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

Issue #11850 - Add sys.last_exc #11934

Closed
wants to merge 10 commits into from
Closed

Conversation

robotherapist
Copy link
Contributor

Closes #11850

Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

Thanks for taking this @robotherapist, looks good to me.

I do wonder about the "Skip this frame" thing, however I don't think we should be touching the exc traceback itself, so it's probably OK.

A couple of things before we can merge this:

  • Add a changelog entry
  • Add a test -- can add a check for last_exc to test_store_except_info_on_error in testing/test_runner.py.

@robotherapist
Copy link
Contributor Author

@bluetech Does this require updating mypy? The error in pre-commit.ci is:

ruff.....................................................................Passed
ruff-format..............................................................Passed
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
fix python encoding pragma...............................................Passed
check yaml...............................................................Passed
debug statements (python)................................................Passed
blacken-docs.............................................................Passed
setup-cfg-fmt............................................................Passed
type annotations not comments............................................Passed
mypy.....................................................................Failed
- hook id: mypy
- exit code: 1

src/_pytest/runner.py:172: error: Module has no attribute "last_exc"  [attr-defined]
src/_pytest/runner.py:182: error: Module has no attribute "last_exc"  [attr-defined]
testing/test_runner.py:930: error: Module has no attribute "last_exc"  [attr-defined]
Found 3 errors in 2 files (checked 232 source files)

rst......................................................................Passed
changelog filenames..................................(no files to check)Skipped
py library is deprecated.................................................Passed
py.path usage is deprecated..............................................Passed

Weird it ran the first time though... I was hoping the if statements for 3.12.0 would satisfy everything.

Copy link
Member

@bluetech bluetech 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! See my comment about the mypy issue.

Two "procedural" notes, in case you'd like to submit more PRs in the future:

  • Prefer to update the branch of PRs using git rebase, not git merge, this makes it a bit easier to handle.
  • Prefer to submit PRs from a branch in your fork, not from the main branch. This allows maintainers to update the PR if needed, and probably cause less of a mess for you as the submitter.

@@ -0,0 +1 @@
Added the new `sys.last_exc` value to `pytest_runtest_call()` in `src/_pytest/runner.py`.
Copy link
Member

Choose a reason for hiding this comment

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

Since the changelog is intended for end-users to read, they don't really need to know about details like pytest_runtest_call or the internal file it's done in. I suggest something like this:

Suggested change
Added the new `sys.last_exc` value to `pytest_runtest_call()` in `src/_pytest/runner.py`.
Added support for :data:`sys.last_exc` for post-mortem debugging on Python>=3.12.

@@ -168,6 +168,8 @@ def pytest_runtest_call(item: Item) -> None:
del sys.last_type
del sys.last_value
del sys.last_traceback
if sys.version_info >= (3, 12, 0):
del sys.last_exc
Copy link
Member

Choose a reason for hiding this comment

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

Turns it the type stub for it is just missing from typeshed (the project which provides type annotations for the standard library). I sent a fix for it, but until it's integrated (in a future mypy version), let's add type-ignores for it, like this:

Suggested change
del sys.last_exc
del sys.last_exc # type: ignore[attr-defined]

@bluetech
Copy link
Member

Merged through #12027, thanks @robotherapist.

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.

Set sys.last_exc in addition to last_type/last_value/last_traceback on Python>=3.12
2 participants