pytester: use monkeypatch.chdir() for dir changing
#11315
Conversation
| @@ -696,15 +688,14 @@ def __init__( | |||
| #: be added to the list. The type of items to add to the list depends on | |||
| #: the method using them so refer to them for details. | |||
| self.plugins: List[Union[str, _PluggyPlugin]] = [] | |||
| self._cwd_snapshot = CwdSnapshot() | |||
| self._sys_path_snapshot = SysPathsSnapshot() | |||
There was a problem hiding this comment.
should we log a followup to ensure sys.path/sys.modules handling in coordination with monkeypatch
There was a problem hiding this comment.
Might be possible but some complications:
SysPathSnapshotalso savessys.meta_path, monkeypatch doesn't.- Monkeypatch doesn't save
sys.pathby default, though pytester can probably force it to do it SysModulesSnapshothas no equivalent in monkeypatch currenty as far as know. Also has somepreserve_modulesexclusion...
bluetech
force-pushed
the
pytest-monkeypatch-chdir
branch
from
a9a1fd6
to
5670792
Compare
|
Updated; there were some calls to |
| last_reprtb = last_p.repr_traceback_entry(excinfo.traceback[-1], excinfo) | ||
| last_lines = last_reprtb.lines | ||
| monkeypatch.undo() | ||
| with monkeypatch.context() as mp: |
There was a problem hiding this comment.
Because others use pytester, I think it would be worth adding a trivial changelog explaining this change, in case others find/have the same pattern in their test suites.
There was a problem hiding this comment.
Agreed - added a changelog.
The current method as the following problem, described by Sadra Barikbin: The tests that request both `pytester` and `monkeypatch` and use `monkeypatch.chdir` without context, relying on `monkeypatch`'s teardown to restore cwd. This doesn't work because the following sequence of actions take place: - `monkeypatch` is set up. - `pytester` is set up. It saves the original cwd and changes it to a new one dedicated to the test function. - Test function calls `monkeypatch.chdir()` without context. `monkeypatch` saves cwd, which is not the original one, before changing it. - `pytester` is torn down. It restores the cwd to the original one. - `monkeypatch` is torn down. It restores cwd to what it has saved. The solution here is to have pytester use `monkeypatch.chdir()` itself, then everything is handled correctly.
bluetech
force-pushed
the
pytest-monkeypatch-chdir
branch
from
5670792
to
81192ca
Compare
|
Should we deprecate monkeypatch.undo as public api now that . context is available |
|
We did add a note warning about its use some time ago. I don't know if we can entirely deprecate it because it doesn't have a replacement if someone wants to undo their own |
|
@bluetech customized contextmanager undo is controlled via |
The current method as the following problem, described by @sadra-barikbin in #11236 (comment):
The tests that request both
pytesterandmonkeypatchand usemonkeypatch.chdirwithout context, relying onmonkeypatch's teardown to restore cwd. This doesn't work because the following sequence of actions take place:monkeypatchis set up.pytesteris set up. It saves the original cwd and changes it to a new one dedicated to the test function.monkeypatch.chdir()without context.monkeypatchsaves cwd, which is not the original one, before changing it.pytesteris torn down. It restores the cwd to the original one.monkeypatchis torn down. It restores cwd to what it has saved.The solution here is to have pytester use
monkeypatch.chdir()itself, then everything is handled correctly.