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

Prevent import of unrealted packages during test collection #741

Merged

Conversation

seifertm
Copy link
Contributor

@seifertm seifertm commented Jan 7, 2024

#729 describes an issue where pytest-asyncio imports packages (i.e. __init__.py modules) which are not imported without pytest-asyncio. The imported packages could be test packages and non-test packages. This leads to two issues:

  1. The import happens in the pytest_collectstart hook just before the collection phase. Pytest isn't equipped to deal with import errors or test skips in this hook, so the import can trigger an INTERNALERROR which aborts the test run completely.
  2. Importing packages that are unrelated to the test run may trigger ImportErrors or ImportWarnings. The greedy import behaviour can even cause issues just by changing the import order (refer to due to a change in the import order (refer to this comment about the torch package)

This PR "tackles" the problem by monkey patching the Package.collect method, which delays any collection errors to the actual collection phase and addresses problem 1.

The patch also adds a call to collector.funcnamefilter, in order to skip packages that don't contain test code. The fixture function for a package-scoped loop is installed into a temporary Python module in the current package rather than into the __init__.py to avoid further problems. This solves problem 2.

This code is expected to be a temporary bandaid. The collection logic has changed in pytest 8, so that each Package only collects its current directory rather than all subdirectories. This will allow pytest-asyncio to get rid of the code that tracks sub-package paths and assembles fixture IDs based on them.

Moreover, there are efforts by the pytest developers to allow fixtures to be attached to a certain node, which would eliminate the monkey patching.

…on module in the package, rather than attaching the fixture to the package's __init__.py.

Signed-off-by: Michael Seifert <[email protected]>
…related packages during test collection.

This was caused by a missing call to collector.funcnamefilter when setting up the packaged-scoped event loop fixture function.

The new code respects funcnamefilter and monkeypatches Package.collect to install a package-scoped loop whenever an __init__.py is encountered.

Signed-off-by: Michael Seifert <[email protected]>
@seifertm seifertm linked an issue Jan 7, 2024 that may be closed by this pull request
@seifertm seifertm added this to the v0.23 milestone Jan 7, 2024
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (260b791) 95.15% compared to head (d8824cc) 94.76%.
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #741      +/-   ##
==========================================
- Coverage   95.15%   94.76%   -0.39%     
==========================================
  Files           2        2              
  Lines         475      497      +22     
  Branches       94       99       +5     
==========================================
+ Hits          452      471      +19     
- Misses         16       19       +3     
  Partials        7        7              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@seifertm seifertm added this pull request to the merge queue Jan 9, 2024
Merged via the queue into pytest-dev:main with commit 84703df Jan 9, 2024
9 checks passed
@seifertm seifertm deleted the pytest-asyncio-loads-packages-more-eagerly branch January 9, 2024 17:22
@seifertm seifertm restored the pytest-asyncio-loads-packages-more-eagerly branch January 10, 2024 17:39
@seifertm seifertm deleted the pytest-asyncio-loads-packages-more-eagerly branch January 10, 2024 18:01
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.

pytest-asyncio-0.23.2 loads packages more eagerly
2 participants