Updating tests to use real and fake broker_settings.yaml #216
Conversation
Griffin-Sullivan
force-pushed
the
test-update
branch
from
bb95169
to
5203ec8
Compare
Griffin-Sullivan
force-pushed
the
test-update
branch
3 times, most recently
from
bff4974
to
ce6c35b
Compare
| settings.clean() | ||
| settings.update(_settings.as_dict()) |
There was a problem hiding this comment.
Is there a reason to have settings and _settings alongside when one is already living in the module context?
There was a problem hiding this comment.
I'm not sure I understand the question. The reason for having the two here is that settings lives in the module context and we use this function to update that settings object with what we get from _settings. I'm pretty sure we were not able to just reset settings to another Dynaconf object because then it would lose context in places that it was already imported in, but I honestly can't remember at this point.
| _sensitive_attrs = [] | ||
|
|
||
| def __init__(self, **kwargs): | ||
| self._construct_params = [] | ||
| cls_name = self.__class__.__name__ | ||
| logger.debug(f"{cls_name} provider instantiated with {kwargs=}") | ||
| self.instance = kwargs.pop(f"{cls_name}", None) | ||
| if not self._settings_id == settings._id: | ||
| logger.debug("Settings object has been reloaded, reloading provider settings") |
There was a problem hiding this comment.
What is the case when we need to reload the settings?
There was a problem hiding this comment.
For running the test suite we need to change the settings file being used. Now that we automatically look for settings in ~/.broker we need to change it to our broker_settings for testing when the tests are running.
broker/settings.py
Outdated
| ) | ||
| settings.clean() | ||
| settings.update(_settings.as_dict()) | ||
| settings._id = str(uuid.uuid4()) |
There was a problem hiding this comment.
Could that be renamed to something like _broker_settings_id to be sure that _id does not collide with anything private of Dynaconf?
| @@ -15,3 +17,21 @@ def set_envars(request): | |||
| os.environ[request.param[0]] = request.param[1] | |||
| yield | |||
| del os.environ[request.param[0]] | |||
|
|
|||
| @pytest.fixture(scope="module") | |||
| def temp_settings_and_inventory(): | |||
There was a problem hiding this comment.
What about using the tmp_path fixture and creating the new files in it? Then setting BROKER_DIRECTORY to tmp_path and unsettling it once the test is done?
There was a problem hiding this comment.
We could try it if you think that's an improvement over this implementation. @JacobCallahan any thoughts since you helped me with our current solution?
|
@Griffin-Sullivan will you have a chance to revisit this sometime soon? |
|
Yeah I'll try to do it later today. I just need to get those 2 tests fixed and add @ogajduse's suggestions. |
Griffin-Sullivan
force-pushed
the
test-update
branch
from
ce6c35b
to
fcb8db7
Compare
Griffin-Sullivan
force-pushed
the
test-update
branch
from
fcb8db7
to
5ed72d8
Compare
|
Screw this |
Fixes #212. All of the tests that are non-functional will make backup files for your settings and inventory and copy in test settings. Once the test is finished they are reset to normal.