-
-
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
fixtures,runner: fix tracebacks getting longer and longer #12264
Conversation
@@ -1126,7 +1127,7 @@ def pytest_fixture_setup( | |||
# Don't show the fixture as the skip location, as then the user | |||
# wouldn't know which test skipped. | |||
e._use_item_location = True | |||
fixturedef.cached_result = (None, my_cache_key, e) | |||
fixturedef.cached_result = (None, my_cache_key, (e, e.__traceback__)) |
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.
Would it be possible to just pass the dunder traceback into the method to avoid tracking the concrete one for reraise?
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.
I don't think so, the exception is saved on the FixtureDef
and can be needed again at an arbitrary point in the future without a caller-callee relation to the time it is saved.
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.
LGTM. 👍
This wasn't backported to 8.2.x, right? Because I see a similar issue on the recently released 8.2.1 |
@bluetech was this not backported on purpose, or we just forgot to do it? |
I didn't backport because of the changes that may affect plugins. |
- ``FixtureDef.cached_result[2]`` is now a tuple ``(exc, tb)`` instead of ``exc``. | ||
- ``SetupState.stack`` failures are now a tuple ``(exc, tb)`` instead of ``exc``. |
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.
@bluetech FYI this list seems to need indentation or something like that. See the preview of how it's rendered: https://docs.pytest.org/en/latest/changelog.html#bug-fixes.
P.S. I integrated my changelog draft preview Sphinx extension during the sprint. So now, you're going to be able to preview the rendering in new pull requests, even before merging.
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.
I submitted a fix via #12563. It's better with that patch: https://pytest--12563.org.readthedocs.build/en/12563/changelog.html#bug-fixes.
It was being rendered inline and not it's not.
It was being rendered inline and now it's not.
It was being rendered inline and now it's not.
It was being rendered inline and now it's not.
It was being rendered inline and now it's not.
It was being rendered inline and now it's not.
…64--reraise-with-original-tb 📝🚑 Polish the PR #12264 changelog entry
It was being rendered inline and now it's not.
Fix #12204 - please see the explanation in #12204 (comment)