-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Closed
Labels
topic: collectionrelated to the collection phaserelated to the collection phasetopic: reportingrelated to terminal output and user-facing messages and errorsrelated to terminal output and user-facing messages and errors
Milestone
Description
What's the problem this feature will solve?
When writing async unit-tests, it is extremely easy to forget to install the pytest-async extension and end up ignoring the futures.
This can cause unexpected silence:
async def test_should_fail_but_doesnt():
assert False
This passes if pytest-async is not installed. Instead, I'd like to propose we make this fail with a nice message indicating that you need too install python-async.
Describe the solution you'd like
I'd like to inspect tests to see if they return a future. If they return a future, and pytest-async is not installed i'd like to error out and tell users to install it.
Alternative Solutions
Ignore the tests.
Additional context
I've been bit by this once~ know another team member of mine who has also been bitten by this.
I'm happy to contribute this.
Metadata
Metadata
Assignees
Labels
topic: collectionrelated to the collection phaserelated to the collection phasetopic: reportingrelated to terminal output and user-facing messages and errorsrelated to terminal output and user-facing messages and errors
Type
Projects
Milestone
Relationships
Development
Select code repository
Activity
RonnyPfannschmidt commentedon Aug 30, 2023
More details needed,pytest notifies when async tests are there without a async plugin
LukeWood commentedon Aug 30, 2023
Oh interesting! I had no idea- maybe I missed it in the logs?
what does notify mean in this context? Error or warning? If a warning maybe we should upgrade it to an error and gate the warning behind an environment variable?
RonnyPfannschmidt commentedon Aug 30, 2023
Warnings
LukeWood commentedon Aug 30, 2023
RonnyPfannschmidt commentedon Aug 30, 2023
@nicoddemus @asottile @bluetech
I'm +1 on turning those into a failure for 8.x and would like your input
Zac-HD commentedon Sep 7, 2023
Sounds good to me - we could even recommend a particular plugin based on
sys.modules
:Zac-HD commentedon Oct 22, 2024
Relevant code is here. We should convert from a warning to an error, and include the nodeid in the error message - it's not currently included in this warning!
It'd also be good to turn other non-None returns into an error; that's similarly a few years past when we planned to finish the transition.
Merged PR 5358: Update dependency pytest to v8
4 remaining items