test: fix broken test if user had config files#2173
Conversation
|
PR #2174 demonstrates the issue. |
Use `monkeypatch` to ensure that no config files are reported for the test. Closes: #2172
34d44aa to
864fc12
Compare
nejch
left a comment
There was a problem hiding this comment.
Thanks for the catch @JohnVillalovos! We already have some scaffolding for mocking/prepping a clean config environment but perhaps it was not broad enough in where we placed it. It seems we should have a clean environment for all tests by adding that to the root tests/conftest.py e.g.:
-
Move this
mock_clean_envfixture to the root conftest withautouse=True- no test ever should break from a user-suppliedPYTHON_GITLAB_CFG:
python-gitlab/tests/unit/test_config.py
Lines 132 to 134 in f0152dc
-
Move more logic to that fixture that we already use to ensure an empty list of default files - same here, no test ever should be affected by presence of local files:
python-gitlab/tests/unit/test_config.py
Lines 144 to 145 in f0152dc
-
In any test that does specifically need that, override it again if needed (I think we mostly don't, as we explicitly define things) like we already do here:
python-gitlab/tests/unit/test_config.py
Line 138 in f0152dc
More approaches are discussed here https://stackoverflow.com/questions/38748257/disable-autouse-fixtures-on-specific-pytest-marks in case the simple override is not enough.
WDYT? This would then also be more local/CI-agnostic and consistent across all runs and we can stop worrying about config even before we hit CI.
|
That sounds reasonable but I can't help but think this should be merged now and the improvements you mentioned be done afterwards. As at the moment tests are broken for anyone who has a config. I was going to work on arrays tomorrow and then could try to take a stab at your ideas. |
|
Sounds good also! I'll merge this for now. |
Use
monkeypatchto ensure that no config files are reported for thetest.
Closes: #2172