-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
runner: re-raise pytest.exit inside ExceptionGroups (fixes #13650) #13736
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @gomri15 for the PR!
Left some small suggestions, please take a look.
@nicoddemus You guys squash all the changes or leave them?, i assumed squash but want to make sure. |
…ssion as expected.
aeec7d8
to
e56af36
Compare
@nicoddemus Is this something were planning to eventually merge you think?. |
The change looks good and clean to me, thanks! I think we need to make sure we agree on the semantics we want though, I will post a comment on the issue.
If the author makes the commits "atomic" i.e. each commit stands on its own (contains one logical change, passes tests etc.), then we merge. That's how I like to do it, but most people don't bother with this, instead the PR should be one single commit, then we squash. You can work whichever way you feel comfortable. |
Should be fine in a follow up, but would be good to see if we can make it work, if only for pedantic correctness. I think we could do it like this:
As for flattening, there seems to not be a built-in way to do it, but I see there is a proposed PEP adding |
Fix issue: 13650, fixes regression when adding exception group which meant pytest raises in teardown wouldn't stop execution
Scope note: This PR intentionally does not handle nested ExceptionGroups (EGs containing EGs). That can be discussed in a follow-up if desired.
Handle pytest.exit() in exception group by searching in exception group if pytest.exit() found and reraising it.
Rationale:
Before Python 3.11 (no EGs), CallInfo.from_call re-raised certain exceptions (e.g., Exit, KeyboardInterrupt) so they bypassed reporting. With EGs, those exceptions could be inside a group, and the simple isinstance(excinfo.value, reraise) check no longer triggered. The session would continue to the next test instead of stopping. This PR restores the original contract in an EG world, without introducing new flows or flags.
Fixes #13650