Replace the use of private monkeypatch fixture API#6095
Open
mgorny wants to merge 1 commit into
Open
Conversation
Replace the use of private `monkeypatch` API with the standard invocations to fix compatibility with pytest 9.1, as well as to make the tests more reliable in the future. The previous code operated on `monkeypatch` fixture internals to trigger clearing environment variables expected by tests at the beginning of tests, and reverting any changes made by the tests. However, this does not seem strictly valid, and it actually lead to tests making wrong assumptions. For example, `test_disable_dotenv_from_env` relied on a previous test unsetting `FOO` in the environment, and failed if `FOO` was set while it was run alone. The new logic uses public API only: it cleans up the standard set of environment variables for every test, and additionally cleans up variables specifically used by dotenv tests. With this approach, it is entirely possible for a test to leave the environment "dirty"; however, that's fine since other tests need to clean up the environment anyway to account for user-set environment variables. Fixes pallets#6071 Signed-off-by: Michał Górny <mgorny@gentoo.org>
| standard values. Returns a list of operations that is used by | ||
| :func:`._reset_os_environ` after each test. | ||
| """ | ||
| mp = monkeypatch.MonkeyPatch() |
Contributor
There was a problem hiding this comment.
actualy keep this fixture and use
with pytest.MonkeyPatch.context() as mp:
...
yield ....
Author
There was a problem hiding this comment.
Sorry; actually, does that change anything? I mean, either way we need a function scope fixture, and my understanding is that it will undo the changes just the same.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace the use of private
monkeypatchAPI with the standard invocations to fix compatibility with pytest 9.1, as well as to make the tests more reliable in the future.The previous code operated on
monkeypatchfixture internals to trigger clearing environment variables expected by tests at the beginning of tests, and reverting any changes made by the tests. However, this does not seem strictly valid, and it actually lead to tests making wrong assumptions. For example,test_disable_dotenv_from_envrelied on a previous test unsettingFOOin the environment, and failed ifFOOwas set while it was run alone.The new logic uses public API only: it cleans up the standard set of environment variables for every test, and additionally cleans up variables specifically used by dotenv tests. With this approach, it is entirely possible for a test to leave the environment "dirty"; however, that's fine since other tests need to clean up the environment anyway to account for user-set environment variables.
Fixes #6071