-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Save teardown exceptions for further teardowns #13002
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.
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.
FWIW such changes must be always accompanied by respective tests.
@@ -214,6 +214,9 @@ def __init__( | |||
# Deprecated alias. Was never public. Can be removed in a few releases. | |||
self._store = self.stash | |||
|
|||
#: A list of exceptions that happened during teardown | |||
self.teardown_exceptions: list[BaseException] = [] |
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.
Does this need to be a list and not an exception group?
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.
It's better to use a list then pass the list to the exception group, you can't append things to a group
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.
Ah, I didn't realize they were immutable..
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.
Guys, where to put tests? Can u suggest appropriate place? Looks like: testing/test_collection.py and teardown?
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.
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.
Merged, thank you. Now we have a test to check if the fixture fails and exception is added to the list
test: tests for teardown exceptions patch
for more information, see https://pre-commit.ci
This PR adds an ability to access the exceptions that happened during teardown in other teardowns.
Motivation: some plugins (playwright-pytest) must rely on current teardown status to properly execute their hooks. Currently the teardown information isn't available, so it must be extracted with different workarounds.
Doesn't change functionality, only exposes the local variable on Node object.
Allows proper implementation of microsoft/playwright-pytest#207, not relying on traceback hacks but getting the information directly.