Skip to content

Conversation

@pretty-parrot
Copy link
Owner

@pretty-parrot pretty-parrot commented Jul 14, 2022

What is the purpose of this PR

  • This PR patches test/test_prase_it.py::BaseTests::test_parser_read_all_configuration_variables_raise_allowed_types_error and prevents it from failing when it is run by itself
  • Test is flaky (non-deterministic) and depends on test/test_prase_it.py::BaseTests::test_read_envvar_single_file_config to set up a state to pass, but the test fails when it is run by itself otherwise

Expected Result

  • Test test/test_prase_it.py::BaseTests::test_parser_read_all_configuration_variables_raise_allowed_types_error should pass when run both by itself and after test/test_prase_it.py::BaseTests::test_read_envvar_single_file_config

Actual Result

  • Test test/test_prase_it.py::BaseTests::test_parser_read_all_configuration_variables_raise_allowed_types_error fails when it is run by itself

Reproduce the test failure

  • Run python3 -m pytest test/test_prase_it.py::BaseTests::test_parser_read_all_configuration_variables_raise_allowed_types_error
  • Specifically we get the following:
_ BaseTests.test_parser_read_all_configuration_variables_raise_allowed_types_error _

self = <test.test_prase_it.BaseTests testMethod=test_parser_read_all_configuration_variables_raise_allowed_types_error>

    def test_parser_read_all_configuration_variables_raise_allowed_types_error(self):
        parser = ParseIt(config_location=test_files_location)
        with self.assertRaises(TypeError):
>           parser.read_all_configuration_variables(allowed_types={"file_type": [bool, dict]})

test/test_prase_it.py:1324: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

    def read_all_configuration_variables(self, default_value: Optional[dict] = None, required: Optional[list] = None,
                                         allowed_types: Optional[dict] = None) -> dict:
        """reads all configuration variables from all allowed sources and returns a dict that includes the combined
                        result of all of them, if a configuration variable exists in two (or more) different sources the
                         one with the higher priority will be the only one returned
    
                    Arguments:
                        default_value -- defaults to None, a dict of key/value pairs of a configuration variables & it's
                            value should it not be defined in any of the valid sources
                        required -- defaults to None, if given a list configuration variables it will raise a ValueError
                            if any of the configuration variables is not configured in any of the config
                            files/envvars/cli args
                        allowed_types -- Defaults to None, an optional dict of types that are accepted for a variable to
                            be, if set a check will be preformed and if the variables value given is not of any of the
                            types in said list a TypeError will be raised
                    Returns:
                        config_value_dict -- a dict of the key/value pairs of all the configurations requested
        """
        # first we create an empty config_value_dict
        config_value_dict = {}
    
        # now we fill the config_value_dict with the data of all valid sources in reverse order (from least desired to
        # the most desired source), overwriting each data that is found multiple times with the more desired state
        data_sources = self.config_type_priority
        data_sources.reverse()
        for config_type in data_sources:
            if config_type == "cli_args":
                config_value_dict.update(read_all_cli_args_to_dict())
            elif config_type == "envvars" or config_type == "env_vars":
                if self.nest_envvars is True:
                    config_value_dict.update(split_envvar_combained_dict(divider=self.envvar_divider,
                                                                         force_uppercase=self.force_envvars_uppercase))
                else:
                    config_value_dict.update(read_all_envvars_to_dict(force_uppercase=self.force_envvars_uppercase))
            # will loop over all files of each type until all files of all types are searched, first time the key is
            # found will break outside of both loops
            elif config_type in self.valid_file_type_extension:
                for config_file in self.config_files_dict[config_type]:
                    if self.config_file_type == "file":
                        file_dict = self._parse_file_per_type(config_type, config_file)
                    else:
                        file_dict = self._parse_file_per_type(config_type, os.path.join(self.config_location,
                                                                                        config_file))
                    config_value_dict.update(file_dict)
            else:
                raise ValueError
    
        # now we need to add the default values from the provided "default_value" dict to any configuration variable in
        # said list that wasn't found in any of the valid sources
        if default_value is not None:
            for default_config_key, default_config_value in default_value.items():
                config_found, config_value = self._check_config_in_dict(default_config_key, config_value_dict)
                if config_found is False:
                    config_value_dict[default_config_key] = default_config_value
    
        # and we run the type estimate (which is recursive) on the full dict if it's configured to be used
        if self.type_estimate is True:
            config_value_dict = estimate_type(config_value_dict, none_values=self.none_values)
    
        # now we check that all the required values exist and raise a ValueError otherwise
        if required is not None:
            for required_config in required:
                config_found, config_value = self._check_config_in_dict(required_config, config_value_dict)
                if config_found is False:
                    raise ValueError
    
        # and we also check that the "allowed_types" of all keys in the dict are from the list of allowed types and
        # raise a TypeError
        if allowed_types is not None:
            for allowed_types_key, allowed_types_value in allowed_types.items():
>               if type(config_value_dict[allowed_types_key]) not in allowed_types_value:
E               KeyError: 'file_type'

parse_it/parser.py:354: KeyError

Why the Test Fails

  • The test fails because the test is dependent on some state that is not set when it is run by itself.

Fix

  • The changes in this pull request set the state and make the test pass when it is run by itself.

@winglam
Copy link

winglam commented Aug 3, 2022

In this PR the exception is in the Reproduce failure and not the Actual result part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants