Skip to content

Comments

swev-id: pytest-dev__pytest-7982#42

Open
casey-brooks wants to merge 1 commit intopytest-dev__pytest-7982from
casey/fix-symlinked-dir-collection-7982
Open

swev-id: pytest-dev__pytest-7982#42
casey-brooks wants to merge 1 commit intopytest-dev__pytest-7982from
casey/fix-symlinked-dir-collection-7982

Conversation

@casey-brooks
Copy link

Summary

  • ensure directory traversal follows symlinks without infinite loops
  • add regression coverage for symlinked directory arguments and subdirectories

Testing

  • before fix: PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/pytest repro/tests -> collected 0 items
  • after fix: PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/pytest repro/tests -> 1 passed
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/pytest testing/test_collection.py::test_collect_symlinked_directory_argument testing/test_collection.py::test_collect_symlinked_subdirectory
  • .venv/bin/black --check src/_pytest/pathlib.py testing/test_collection.py
  • .venv/bin/flake8 src/_pytest/pathlib.py testing/test_collection.py

Fixes #41

@casey-brooks casey-brooks requested a review from a team December 26, 2025 12:00
@casey-brooks
Copy link
Author

Test & Lint Summary

  • `PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/pytest repro/tests` → 1 passed
  • `PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/pytest testing/test_collection.py::test_collect_symlinked_directory_argument testing/test_collection.py::test_collect_symlinked_subdirectory` → 2 passed
  • `.venv/bin/black --check src/_pytest/pathlib.py testing/test_collection.py` → no changes
  • `.venv/bin/flake8 src/_pytest/pathlib.py testing/test_collection.py` → clean

`tox -e linting` is not reachable here because pre-commit cannot fetch https://gitlab.com/pycqa/flake8 without credentials; the manual black/flake8 runs above cover the linted files.

Copy link

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tackling the symlink regression and adding focused coverage. One issue remains: the new traversal logic now swallows every that arises from , which means permission errors or other unexpected filesystem failures get muted instead of surfacing. Please narrow that handler to the errno cases we actually expect (e.g. broken links, ELOOP) and let other errors propagate so we don’t hide real problems.

@casey-brooks casey-brooks force-pushed the casey/fix-symlinked-dir-collection-7982 branch from 0525843 to 8345176 Compare December 26, 2025 12:06
@casey-brooks
Copy link
Author

Retested after tightening the errno handling:

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/pytest testing/test_collection.py::test_collect_symlinked_directory_argument testing/test_collection.py::test_collect_symlinked_subdirectory → 2 passed
  • .venv/bin/flake8 src/_pytest/pathlib.py → clean

Copy link

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tightening up the exception handling—limiting it to the expected errno cases keeps traversal robust without muting real errors. The added regression tests exercise both direct invocation and parent directory traversal, so the original issue is covered. LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants