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

Fixes a bug that breaks virtual modules on Windows #746

Merged
merged 4 commits into from
Jan 10, 2024

Conversation

seifertm
Copy link
Contributor

@seifertm seifertm commented Jan 10, 2024

pytest-asyncio v0.23.4a0 introduced "virtual modules". These are temporary files in Python packages which contain a fixture definition. The files are removed after the virtual module was collected by pytest.

The use of temporary files has platform-specific behavior. Apparently, pytest opens the NamedTemporaryFile again, which causes issues on Windows. The the docs describe the problem and possible mitigations:

Opening the temporary file again by its name while it is still open works as follows:
On POSIX the file can always be opened again.
On Windows, make sure that at least one of the following conditions are fulfilled:
* delete is false
* additional open shares delete access (e.g. by calling os.open() with the flag O_TEMPORARY)
* delete is true but delete_on_close is false. Note, that in this case the additional opens that do not share delete access (e.g. created via builtin open()) must be closed before exiting the context manager, else the os.unlink() call on context manager exit will fail with a PermissionError.

The delete_on_close flag has only been added in Python 3.12, so pytest-asyncio cannot use it at the moment. Neither does pytest-asyncio have the means to influence how pytest opens a file for collection. This leaves us with setting delete=False in the constructor of the NamedTemporaryFile and clean it up manually.

This PR also enabled CI tests on Windows to reproduce the problem (and validate the fix).

See #729

@seifertm seifertm added this to the v0.23 milestone Jan 10, 2024
@seifertm seifertm force-pushed the fix-virtual-modules-on-windows branch 7 times, most recently from d660b00 to f39fed0 Compare January 10, 2024 18:41
The use of temporary files for injecting fixtures into packages shows platform-specific behavior. As a result, it makes sense to run the tests on Windows as well, at least as long as the virtual module workaround is in place.

Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de>
Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de>
Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de>
Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de>
@seifertm seifertm force-pushed the fix-virtual-modules-on-windows branch from f39fed0 to 705f745 Compare January 10, 2024 18:42
@seifertm seifertm changed the title Run tests on Windows Fixes a bug that breaks virtual modules on Windows Jan 10, 2024
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (aecfd01) 94.76% compared to head (705f745) 94.78%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #746      +/-   ##
==========================================
+ Coverage   94.76%   94.78%   +0.02%     
==========================================
  Files           2        2              
  Lines         497      499       +2     
  Branches       99       99              
==========================================
+ Hits          471      473       +2     
  Misses         19       19              
  Partials        7        7              

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

@seifertm seifertm marked this pull request as ready for review January 10, 2024 18:51
@seifertm seifertm added this pull request to the merge queue Jan 10, 2024
Merged via the queue into pytest-dev:main with commit 8ba9bd0 Jan 10, 2024
14 checks passed
@seifertm seifertm deleted the fix-virtual-modules-on-windows branch January 10, 2024 18:54
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.

None yet

2 participants