Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pytest crashes if there is a subdirectory it does not have read access to #12120

Closed
4 tasks done
jesnie opened this issue Mar 15, 2024 · 6 comments · Fixed by #12311
Closed
4 tasks done

Pytest crashes if there is a subdirectory it does not have read access to #12120

jesnie opened this issue Mar 15, 2024 · 6 comments · Fixed by #12311
Labels
topic: collection related to the collection phase

Comments

@jesnie
Copy link

jesnie commented Mar 15, 2024

Pytest crashes if there is a subdirectory it does not have read access to.

For example:

rm -rf .venv chaff tests
virtualenv -p python3.10 .venv
source .venv/bin/activate
pip install pytest
pip list
mkdir chaff
mkdir tests
cat <<EOF > tests/test_something.py
def test_something() -> None:
    pass
EOF
pytest tests
chmod a-rwx chaff
pytest tests

yields:

created virtual environment CPython3.10.12.final.0-64 in 70ms
  creator CPython3Posix(dest=/home/jn/src/pytesttest/.venv, clear=False, no_vcs_ignore=False, global=False)
  seeder FromAppData(download=False, pip=bundle, setuptools=bundle, wheel=bundle, via=copy, app_data_dir=/home/jn/.local/share/virtualenv)
    added seed packages: pip==22.0.2, setuptools==59.6.0, wheel==0.37.1
  activators BashActivator,CShellActivator,FishActivator,NushellActivator,PowerShellActivator,PythonActivator

<snip pip installation stuff...>

Installing collected packages: tomli, pluggy, packaging, iniconfig, exceptiongroup, pytest
Successfully installed exceptiongroup-1.2.0 iniconfig-2.0.0 packaging-24.0 pluggy-1.4.0 pytest-8.1.1 tomli-2.0.1

Package        Version
-------------- -------
exceptiongroup 1.2.0
iniconfig      2.0.0
packaging      24.0
pip            22.0.2
pluggy         1.4.0
pytest         8.1.1
setuptools     59.6.0
tomli          2.0.1
wheel          0.37.1

=================================================================================================================== test session starts ====================================================================================================================
platform linux -- Python 3.10.12, pytest-8.1.1, pluggy-1.4.0 -- /home/jn/src/pytesttest/.venv/bin/python
cachedir: .pytest_cache
rootdir: /home/jn/src/pytesttest
collected 1 item

tests/test_something.py::test_something PASSED

==================================================================================================================== 1 passed in 0.00s =====================================================================================================================

=================================================================================================================== test session starts ====================================================================================================================
platform linux -- Python 3.10.12, pytest-8.1.1, pluggy-1.4.0 -- /home/jn/src/pytesttest/.venv/bin/python
cachedir: .pytest_cache
rootdir: /home/jn/src/pytesttest
collected 0 items / 1 error

========================================================================================================================== ERRORS ==========================================================================================================================
____________________________________________________________________________________________________________________ ERROR collecting . ____________________________________________________________________________________________________________________
.venv/lib/python3.10/site-packages/pluggy/_hooks.py:501: in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
.venv/lib/python3.10/site-packages/pluggy/_manager.py:119: in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
.venv/lib/python3.10/site-packages/_pytest/python.py:212: in pytest_collect_directory
    if pkginit.is_file():
/usr/lib/python3.10/pathlib.py:1322: in is_file
    return S_ISREG(self.stat().st_mode)
/usr/lib/python3.10/pathlib.py:1097: in stat
    return self._accessor.stat(self, follow_symlinks=follow_symlinks)
E   PermissionError: [Errno 13] Permission denied: '/home/jn/src/pytesttest/chaff/__init__.py'
================================================================================================================= short test summary info ==================================================================================================================
ERROR . - PermissionError: [Errno 13] Permission denied: '/home/jn/src/pytesttest/chaff/__init__.py'
===================================================================================================================== 1 error in 0.04s =====================================================================================================================
ERROR: found no collectors for /home/jn/src/pytesttest/tests

I do not believe pytest should be this sensitive to what other subdirectories there might exist.

This is run under Ubuntu 22.04.3 LTS under WSL.

  • 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
@bluetech
Copy link
Member

What is the reason for having inaccessible files in the workspace? Trying to understand if this is something that you intentionally do.

@bluetech bluetech added the topic: collection related to the collection phase label Mar 17, 2024
@jesnie
Copy link
Author

jesnie commented Mar 20, 2024

It's some Docker containers with some databases we use for testing, that has a mounted volume that ends up belonging to another user. We probably could change that if we were sufficiently motivated, but in either case pytest shouldn't crash just because there's a directory it does not have read access to.

@RonnyPfannschmidt
Copy link
Member

@bluetech the regression (that was added by introducing Directories as is) is that we now collect multiple folders just to get to the given testdirs

@jesnie can you try if a collect ignore is a sufficient workaround for now

@bluetech i believe there is a need to ensure collect-towards for testpaths and/or test ids will in fact not look in other places that may trigger permission errors

@jesnie
Copy link
Author

jesnie commented Apr 8, 2024

Sorry about the late reply, but I've been on a small vacation.
Adding a --ignore for the relevant subdirectory does seem to prevent the crash.

@jesnie
Copy link
Author

jesnie commented Apr 8, 2024

@RonnyPfannschmidt can you not just put a try ... except around the relevant code and catch any permission errors?

@bluetech
Copy link
Member

Problem

  • When running pytest a/b/c.py, the ., a and a/b directories or packages are collected as part of building the collection tree.
  • When a directory is collected, it collects all files/dirs in it, including ones that will be filtered out, e.g. a/mysecretdir (the filtering happens separately later).
  • In the python.py plugin's pytest_collect_directory hookimpl, we are checking if a/mysecretdir is a Package by checking whether it contains an __init__.py file.
  • If the user doesn't have rx permission on a/mysecretdir, the collection fails, even though the user didn't select this directory at all.

We've had a bunch of issues about this, as far as I can see the issue mainly comes up in these two scenarios:

  • Users collecting a test in /tmp, which often has non-user-owned dirs. I think doing this is a bad idea -- some other user can drop a conftest.py file and execute stuff as the pytest user -- but who am I to judge.

  • Some Docker or such tool leaving root or service-user owned files in the project directory.

Proposed Solution

I don't have a great proper solution, so for now I suggest we catch PermissionError in the pytest_collect_directory for Packages, and say it's not a package if so. This means it will be collected as a Dir. I think this is fine:

  • If the directory is selected, it will still fail a bit later when trying to run collect() on the Dir node. This is good, we want it to fail if it's actually selected.
  • If the directory is not selected, then it doesn't matter anyway.

OK, so the last point is not 100% right, it might matter if a plugin hooks into the right place etc. That's why I'm not intellectually satisfied with this solution. But practicality beats purity, and practically I don't think anyone will ever notice.

Rejected Solution

Before pytest 8, the problem didn't happen because directory collection was "centralized" in Session which did the filtering during the collection. In pytest 8 we delegated the collection to the Directory nodes (Dir/Package are the built-in concrete ones).

The new scheme is IMO simpler and more pytest-y, and it unlocked further simplifications and optimizations (mostly around conftest loading), and also custom directory node support. So I don't think we want to go back.

Possibly there's a way to keep the new scheme but also delegate the filtering, thus fixing the problem properly. If a future human or machine reads this and comes up with a proposal to do this, that'd be cool :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: collection related to the collection phase
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants