Testing: fix unintended interactions between tests, part 2#16429
Testing: fix unintended interactions between tests, part 2#16429scheibelp merged 5 commits intospack:developfrom
Conversation
…pack#16003)" (spack#16420)" This reverts commit 40df44e.
|
From #16420 (comment)
The test Updating |
… fixture) and the config fixture resolves test failures but is likely too heavy-handed
…l_mockery with a separate install_mockery_mutable_config fixture that is exactly the same as install_mockery but uses the mutable_config fixture to avoid conflicts
|
Can we code |
Are you sure PyTest fixtures work like this? I tried updating the but get the same failures as #16429 (comment). For one, I don't think fixtures can "override" one another. That being said it was still possible that executing That's possibly because the "conflicting" fixtures are referenced indirectly (i.e. I also tried having Overall, I think that it merits more investigation, but the current approach may be ideal for now since it works and is straightforward (the downside being that it has code duplication). |
|
For now this is defining an alternative to the For now I'd suggest merging this and I can make it a priority to refactor if we agree that should occur soon. |
|
The last time I ran into something like this it was due to the usage of a contextmanager that was not using yield wrapped in try->finally. (with an exception being thrown during whatever was going on during yield) I note that both use_configuration() and use_store() are lacking that...? |
|
I agree to proceed and have a look at this later. 👍
What I mean is that fixtures may depend on one another, If
Then everything should work cleanly. The one caveat is that a |
I agree that all context managers implemented using the |
That makes sense, although I guessed that was your meaning in #16429 (comment), tried it, and got the same failure:
As of now from reading the code it appears to satisfy constraints (1) and (3) from #16429 (comment) - largely on account of the implementation of the
I see that is an example of fixtures using one another, but I don't think it has been subjected to the same stress (I don't see any tests using the As a side note, I mentioned in #16429 (comment) that it may not be possible to construct an order in all cases based on fixture parameter requests:
I'm wrong there since it must form a DAG and therefore has an order. However, I don't know if PyTest makes the effort to determine the topological order from a list of fixtures (accounting for indirect requests). |
This is essential in general terms since if you have So, if |
This fixture was introduced in spack#16429, and made redundant in spack#39024
This fixture was introduced in spack#16429, and made redundant in spack#39024
This fixture was introduced in spack#16429, and made redundant in spack#39024
#16003 addressed some unintended interference between unit tests. It appears that #16221 was merged during this time and introduces new unit tests which don't interact well with the fixes. This will reintroduce all changes from #16003 plus whatever is needed to get those changes working with #16221
For now this is just the original changes (and I have marked this WIP). But I think it will be useful to create this thread to track the issue.