rootdir for pytester identified as /tmp when passing a tmpfile in /tmp as part of a argument

[BeyondEvil]

• a detailed description of the bug or problem you are having
• output of pip list from the virtual environment you are using
• 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

[RonnyPfannschmidt]

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

[bluetech]

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

1. Explicitly pass --rootdir=.

2. Change --metadata-from-json /tmp/somefile.json to --metadata-from-json=/tmp/somefile.json

3. Put the somefile.json in the testdir instead of using tempfile which puts it in /tmp.

Possible solutions in pytest

1. Somehow change pytest bootstrapping so it doesn't consider random arguments for rootdir

2. (Not a root cause solution) Handle permission errors and ignore the files (I don't like this one... but can be convinced)

3. (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.

[BeyondEvil]

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.

[RonnyPfannschmidt]

Collection should be limited to the invocations directory in the case we hit here

[juliangilbey]

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

pytest/src/_pytest/config/findpaths.py

Line 172 in 12e061e

def determine_setup(

but I cannot fathom why it doesn't work as expected.

[bluetech]

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.

[juliangilbey]

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-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.

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:

pytest/src/_pytest/config/findpaths.py

Line 183 in 33f694f

ancestor = get_common_ancestor(dirs)

The source of the change was two months ago:
676f38d
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

pytest/src/_pytest/config/findpaths.py

Line 198 in 33f694f

rootdir = get_common_ancestor([cwd, ancestor])

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 /tmp and 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 /tmp even 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?

[juliangilbey]

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.

[juliangilbey]

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

[juliangilbey]

Here's a suggestion: change

pytest/src/_pytest/config/findpaths.py

Lines 210 to 215 in 12e061e

	if rootdir is None:
	rootdir = get_common_ancestor(
	invocation_dir, [invocation_dir, ancestor]
	)
	if is_fs_root(rootdir):
	rootdir = ancestor

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 behaviour
• TestInvocationVariants.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 bad
• test_collection_hierarchy: this loses the initial <Dir test_collection_hierarchy>; this may be bad
• TestFillFixtures.test_extend_fixture_conftest_conftest: this gave an error "recursive dependency involving fixture 'spam' detected"; this is clearly bad
• TestLastFailed.test_lastfailed_args_with_deselected: this gave unexpected output
• test_module_full_path_without_drive: oh dear, conftest.py is not picked up, as that is in the invocation directory, and that is not picked up by the new logic
• TestImportLibMode.test_import_using_normal_mechanism_first_integration: completely different paths generated
• test_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?