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

Fix regression that prevents async fixtures from working in sync tests #287

Merged

Conversation

seifertm
Copy link
Contributor

@seifertm seifertm commented Feb 8, 2022

The function _preprocess_async_fixtures does nothing if the fixture is not an async function. The change simply prepares all async fixtures of a test regardless of whether the test is an async function or not.

Do you see any harm in doing so?

This should fix the issue encountered in #286.

… sync tests.

The commit simply processes all async fixtures of a test regardless of whether the test is an async function or not.

Closes pytest-dev#286

Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de>
@codecov-commenter
Copy link

Codecov Report

Merging #287 (8a86c9e) into master (07e9922) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #287   +/-   ##
=======================================
  Coverage   92.61%   92.61%           
=======================================
  Files           3        3           
  Lines         271      271           
  Branches       39       39           
=======================================
  Hits          251      251           
  Misses         12       12           
  Partials        8        8           
Impacted Files Coverage Δ
plugin.py 92.48% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 07e9922...8a86c9e. Read the comment docs.

Copy link
Contributor

@asvetlov asvetlov left a comment

Choose a reason for hiding this comment

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

LGTM!
Thanks for the fix, my fault.
There are too many combinations between sync and async code :)

@asvetlov
Copy link
Contributor

Please let me merge and make a new release right now.
Sorry for the review delay.

@asvetlov asvetlov merged commit f7c8226 into pytest-dev:master Feb 10, 2022
@seifertm
Copy link
Contributor Author

seifertm commented Feb 10, 2022

Sorry for the review delay.

@asvetlov No worries. If someone has time to look at these things within a week, I'm already pretty happy :)
You're always very responsive, anyway.

A new release sounds good!

@seifertm
Copy link
Contributor Author

seifertm commented Feb 10, 2022

@asvetlov The tag that being built was v18.0.1, instead of v0.18.1. I thought that might cause issues and cancelled the running release workflow before the package landed on PyPI. I hope this was okay!

@seifertm seifertm deleted the fix-async-fixtures-in-sync-tests branch October 23, 2023 06:15
@seifertm seifertm restored the fix-async-fixtures-in-sync-tests branch October 23, 2023 08:17
@seifertm seifertm deleted the fix-async-fixtures-in-sync-tests branch October 23, 2023 08:33
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

3 participants