pytest-dev
rootdir for pytester identified as /tmp when passing a tmpfile in /tmp as part of a argument
- [x] a detailed description of the bug or problem you are having
- [x] output of
pip listfrom the virtual environment you are using - [x] pytest and operating system versions
- [ ] minimal example if possible
Please feel free to edit the title 😅
This PR made some of my tests start to fail with PermissionError: [Errno 13] Permission denied: '/tmp/snap-private-tmp/__init__.py'
https://github.com/pytest-dev/pytest-metadata/actions/runs/7365057317/job/20046008077
The tests pass against c7ee5599, but fail on this merge.
Running it locally I also get some warnings:
warnings
=============================== warnings summary ===============================
../../../../../../../../../Users/jimbrannlund/dev/pytest/pytest/src/_pytest/config/__init__.py:1211
/Users/jimbrannlund/dev/pytest/pytest/src/_pytest/config/__init__.py:1211: PytestAssertRewriteWarning: Module already imported so cannot be rewritten: pytest_metadata
self._mark_plugins_for_rewrite(hook)
../../../../../../../../../Users/jimbrannlund/dev/pytest/pytest/src/_pytest/config/__init__.py:1211
/Users/jimbrannlund/dev/pytest/pytest/src/_pytest/config/__init__.py:1211: PytestAssertRewriteWarning: Module already imported so cannot be rewritten: tests
self._mark_plugins_for_rewrite(hook)
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
I tried to figure out exactly where it goes wrong, but failed. It's entirely possible there's something wrong in my plugin that was just surfaced by the changes in this PR.
Please let me know what information I can contribute with.
pip list
Package Version Editable project location
----------------- ------------------------ ----------------------------------------------
attrs 21.4.0
black 22.1.0
cfgv 3.3.1
click 8.0.4
distlib 0.3.4
exceptiongroup 1.2.0
filelock 3.6.0
flake8 4.0.1
identify 2.4.12
iniconfig 1.1.1
mccabe 0.6.1
mypy-extensions 0.4.3
nodeenv 1.6.0
packaging 21.3
pathspec 0.9.0
pip 22.0.3
platformdirs 2.5.1
pluggy 1.3.0
pre-commit 2.17.0
py 1.11.0
pycodestyle 2.8.0
pyflakes 2.4.0
pyparsing 3.0.7
pytest 7.2.0.dev1110+gacd445a3f /Users/jimbrannlund/dev/pytest/pytest
pytest-metadata 2.0.4 /Users/jimbrannlund/dev/pytest/pytest-metadata
PyYAML 6.0
setuptools 60.6.0
six 1.16.0
toml 0.10.2
tomli 2.0.1
tox 3.24.5
typing_extensions 4.1.1
virtualenv 20.14.0
wheel 0.37.1
I've tried the code on python 3.9...3.13 and it fails on all of them.
CI is Linux Locally I'm on a Mac
rootdir is identified as /tmp - something went deep wrong - most likely related to using named temporary files instead of the testdir
since the metadata file is in /tmp, the folder finding gets confused it seems
Right to add o what @RonnyPfannschmidt said, this is a combination of two things:
Problem
pytest bootstrapping:
pytest uses an elaborate procedure to decide what to use for the rootdir. Part of it depends on the command line arguments passed to pytest, e.g. pytest /my/dir/my_test.py might set /my/dir as the rootdir if there's nothing else which sets it.
The comic part is that this has to happen before plugins are loaded. So if the invocation is pytest --metadata-from-json /tmp/somefile.json, pytest doesn't know yet that /tmp/somefile.json belongs to the --metadata-from-json, but takes it as a test argument. And then it makes /tmp the rootdir.
This is a pretty silly but it's how it is now.
Changes in pytest 8:
The above has always been true so why does it start failing with pytest 8? The reason is that PR #11646 has made pytest scan the rootdir a bit more due to larger reliance on the "genitems" mechanism. I personally think it's fair and should not be an issue in itself, but we'll see...
Workarounds
-
Explicitly pass
--rootdir=. -
Change
--metadata-from-json /tmp/somefile.jsonto--metadata-from-json=/tmp/somefile.json -
Put the
somefile.jsonin the testdir instead of usingtempfilewhich puts it in/tmp.
Possible solutions in pytest
-
Somehow change pytest bootstrapping so it doesn't consider random arguments for rootdir
-
(Not a root cause solution) Handle permission errors and ignore the files (I don't like this one... but can be convinced)
-
(Not a root cause solution) Change collection to be more like before in avoiding trying to touch paths that we're sure will be deselected by the initial paths.
I've implemented workaround number 3.
Probably good hygiene to keep any created files in the temporary test directory anyway.
Let me know if there's anything I can help with.
Collection should be limited to the invocations directory in the case we hit here
Something about this change in rootdir behaviour has broken the test suites in at least pytest-order and pytest-console-scripts. I had also thought it was to do with the use of tmpdir, but Here's a little experiment I ran to explore this a little further. I'm using pytest 8.1.1.
I copied the two scripts from https://github.com/pytest-dev/pytest-order/blob/main/tests/test_xdist_handling.py into two files in /tmp/pytest-order-subtests called first_test.py and second_test.py. Those are the only two files in the directory, as in the test itself. Then this call works:
$ cd /
$ python3.12 -m pytest /tmp/pytest-order-subtests
as does
$ cd /tmp/pytest-order-subtests
$ python3.12 -m pytest /tmp/pytest-order-subtests
and
$ cd
$ python3.12 -m pytest /tmp/pytest-order-subtests
All of these set /tmp/pytest-order-subtests as the rootdir.
However the following test fails:
$ cd /tmp
$ python3.12 -m pytest /tmp/pytest-order-subtests
Test session starts (platform: linux, Python 3.12.2, pytest 8.1.1, pytest-sugar 1.0.0)
PyQt6 6.6.1 -- Qt runtime 6.4.2 -- Qt compiled 6.4.2
rootdir: /tmp
plugins: xdist-3.4.0, dependency-0.5.1, twisted-1.14.0, sugar-1.0.0, trio-0.8.0, mock-3.12.0, anyio-4.2.0, cov-4.1.0, flaky-3.8.1, order-1.2.0, asyncio-0.20.3, timeout-2.2.0, xvfb-3.0.0, typeguard-4.1.5, hypothesis-6.98.4, rerunfailures-12.0, qt-4.3.1
asyncio: mode=Mode.STRICT
collecting ...
―――――――――――――――――――――――――――――― ERROR collecting . ――――――――――――――――――――――――――――――
/usr/lib/python3/dist-packages/pluggy/_hooks.py:501: in __call__
return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
/usr/lib/python3/dist-packages/pluggy/_manager.py:119: in _hookexec
return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
/usr/lib/python3/dist-packages/_pytest/python.py:212: in pytest_collect_directory
if pkginit.is_file():
/usr/lib/python3.12/pathlib.py:894: in is_file
return S_ISREG(self.stat().st_mode)
/usr/lib/python3.12/pathlib.py:842: in stat
return os.stat(self, follow_symlinks=follow_symlinks)
E PermissionError: [Errno 13] Permission denied: '/tmp/aptitude-root.1055139:Eud5Gl/__init__.py'
collected 0 items / 1 error
=========================== short test summary info ============================
FAILED . - PermissionError: [Errno 13] Permission denied: '/tmp/aptitude-root.1055139:...
Results (0.14s):
ERROR: found no collectors for /tmp/pytest-order-subtests
Now I followed the stated algorithm at https://docs.pytest.org/en/8.0.x/reference/customize.html#finding-the-rootdir manually, and none of the criteria for making /tmp the rootdir were satisfied (no configuration files were found, it's not the common root, etc). So I don't understand why this invocation is setting /tmp as the rootdir, and therefore failing (as it starts searching for files anywhere under the rootdir, some of which are not readable.
The following also doesn't work; it seems that --rootdir is ignored too, in this case:
$ cd /tmp
$ python3.12 -m pytest --rootdir=/tmp/pytest-order-subtests
Test session starts (platform: linux, Python 3.12.2, pytest 8.1.1, pytest-sugar 1.0.0)
PyQt6 6.6.1 -- Qt runtime 6.4.2 -- Qt compiled 6.4.2
rootdir: /tmp/pytest-order-subtests
plugins: xdist-3.4.0, dependency-0.5.1, twisted-1.14.0, sugar-1.0.0, trio-0.8.0, mock-3.12.0, anyio-4.2.0, cov-4.1.0, flaky-3.8.1, order-1.2.0, asyncio-0.20.3, timeout-2.2.0, xvfb-3.0.0, typeguard-4.1.5, hypothesis-6.98.4, rerunfailures-12.0, qt-4.3.1
asyncio: mode=Mode.STRICT
collecting ...
―――――――――――――――――――――――― ERROR collecting test session ―――――――――――――――――――――――――
/usr/lib/python3/dist-packages/pluggy/_hooks.py:501: in __call__
return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
/usr/lib/python3/dist-packages/pluggy/_manager.py:119: in _hookexec
return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
/usr/lib/python3/dist-packages/_pytest/python.py:212: in pytest_collect_directory
if pkginit.is_file():
/usr/lib/python3.12/pathlib.py:894: in is_file
return S_ISREG(self.stat().st_mode)
/usr/lib/python3.12/pathlib.py:842: in stat
return os.stat(self, follow_symlinks=follow_symlinks)
E PermissionError: [Errno 13] Permission denied: '/tmp/aptitude-root.1055139:Eud5Gl/__init__.py'
collected 0 items / 1 error
=========================== short test summary info ============================
FAILED pytest-order-subtests - PermissionError: [Errno 13] Permission denied: '/tmp/aptitude-root.1055139:...
!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!
Results (0.18s):
A workaround is to touch a file pytest.ini in the directory /tmp/pytest-order-subtests, but this seems like a bug; it should surely not be needed.
I had a look at the code in https://github.com/pytest-dev/pytest/blob/12e061e2e838fb2c17d54df40a4a11aeb8723c6b/src/_pytest/config/findpaths.py#L172 but I cannot fathom why it doesn't work as expected.
python3.12 -m pytest /tmp/pytest-order-subtests
I am not sure why the rootdir ends up as /tmp. From looking at the code, it would end up here i.e. take common ancestor of /tmp/pytest-order-subtests and the invocation dir which is /tmp, so the result is /tmp. I don't know why it includes the invocation dir here off hand, but there is probably a good reason.
The permission error though is a known issue since pytest 8 that is quite hard to fix but it's certainly on the radar.
python3.12 -m pytest --rootdir=/tmp/pytest-order-subtests
This one is expected, you are collecting /tmp and it's good that it fails since some dirs can't be traversed.
Hi @bluetech, thanks for your detailed comments!
python3.12 -m pytest /tmp/pytest-order-subtests
I am not sure why the rootdir ends up as
/tmp. From looking at the code, it would end up here i.e. take common ancestor of/tmp/pytest-order-subtestsand the invocation dir which is/tmp, so the result is/tmp. I don't know why it includes the invocation dir here off hand, but there is probably a good reason.
Yes, that's what I saw, and I also don't understand it. It's certainly not what is described in the documentation, and it seems to possibly be undesirable behaviour. There are quite possibly arguments for doing this, but it is not described as a breaking change in the Changelog (or even mentioned, for that matter).
In version 7.4.4, this line of code read: https://github.com/pytest-dev/pytest/blob/33f694f4b30c5c502f21f81cb8ab907b12ad2f65/src/_pytest/config/findpaths.py#L183
The source of the change was two months ago:
https://github.com/pytest-dev/pytest/commit/676f38d04aa7fc96ecf06235d855820aba05b83a
which added invocation_dir into locations it didn't previously appear at. So maybe parts of this should be reverted?
I've now checked these same tests with pytest 7.4.4, and discovered that they all pass, so this really is a change of behaviour. (And despite having the line
https://github.com/pytest-dev/pytest/blob/33f694f4b30c5c502f21f81cb8ab907b12ad2f65/src/_pytest/config/findpaths.py#L198
in version 7.4.4, somehow the rootdir is correctly set even when pytest is invoked from /tmp.)
The permission error though is a known issue since pytest 8 that is quite hard to fix but it's certainly on the radar.
The permission error is only triggered in this case because of the rootdir issue.
python3.12 -m pytest --rootdir=/tmp/pytest-order-subtests
This one is expected, you are collecting
/tmpand it's good that it fails since some dirs can't be traversed.
I don't understand why it fails, though; the documentation says:
The --rootdir=path command-line option can be used to force a specific directory. Note that ... so why is it collecting from
/tmpeven though rootdir is/tmp/pytest-order-subtests? But that is clearly a separate question...
EDIT: I was wrong about 7.4.4:
$ cd /tmp
$ python3.12 -m pytest /tmp/pytest-order-subtests
Test session starts (platform: linux, Python 3.12.2, pytest 7.4.4, pytest-sugar 1.0.0)
PyQt6 6.6.1 -- Qt runtime 6.4.2 -- Qt compiled 6.4.2
rootdir: /tmp
plugins: xdist-3.4.0, dependency-0.5.1, twisted-1.14.0, sugar-1.0.0, trio-0.8.0, mock-3.12.0, anyio-4.2.0, cov-4.1.0, flaky-3.8.1, order-1.2.0, asyncio-0.20.3, timeout-2.2.0, xvfb-3.0.0, typeguard-4.1.5, hypothesis-6.98.4, rerunfailures-12.0, qt-4.3.1
asyncio: mode=Mode.STRICT
collected 6 items
pytest-order-subtests/first_test.py ✓✓✓ 83% ████████▍
pytest-order-subtests/second_test.py ✓✓✓ 100% ██████████
Results (0.03s):
6 passed
So the rootdir is still /tmp, but it doesn't break.
Maybe the rootdir is wrong in both cases?
Actually, I'm not sure if that patch is the cause now; get_common_ancestor is now called with the invocation directory as a fallback in place of Path.cwd(). So I'm a bit stumped.
Ah, figured out the comment about --rootdir. The following succeeds:
$ cd /tmp
$ python3.12 -m pytest --rootdir=/tmp/pytest-order-subtests /tmp/pytest-order-subtests
Here's a suggestion: change https://github.com/pytest-dev/pytest/blob/12e061e2e838fb2c17d54df40a4a11aeb8723c6b/src/_pytest/config/findpaths.py#L210-L215 to simply read:
if rootdir is None:
rootdir = ancestor
The reasoning is this: if pytest is called as python -m pytest dir [dir2 ...], then the clear intention is to run pytest on the tests in that directory only. If no config file is found in dir or any of its parents (which would make that the rootdir), then dir should be the rootdir, not the common ancestor of dir and the invocation directory.
Making this change causes several tests to fail, though:
TestRootdir.test_explicit_config_file_sets_rootdir: this just tests the behaviour of the existing code, so would need to change to match the new behaviourTestInvocationVariants.test_cmdline_python_namespace_package: this has changed the presented paths for the tests, and I'm not sure whether this is correct behaviour with the proposed change, or whether this is badtest_collection_hierarchy: this loses the initial<Dir test_collection_hierarchy>; this may be badTestFillFixtures.test_extend_fixture_conftest_conftest: this gave an error "recursive dependency involving fixture 'spam' detected"; this is clearly badTestLastFailed.test_lastfailed_args_with_deselected: this gave unexpected outputtest_module_full_path_without_drive: oh dear,conftest.pyis not picked up, as that is in the invocation directory, and that is not picked up by the new logicTestImportLibMode.test_import_using_normal_mechanism_first_integration: completely different paths generatedtest_collection_args_do_not_duplicate_modules: indentation different, presumably because the rootdir is different
Hmmm. So lots would possibly break with this change. I don't know what the best resolution is; perhaps expect all people calling pytest within tests to explicitly state their rootdir on the command line if that is appropriate?