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

raisesgroup followups #13279

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

raisesgroup followups #13279

wants to merge 3 commits into from

Conversation

jakkdl
Copy link
Member

@jakkdl jakkdl commented Mar 7, 2025

Followup to #13192, which got increasingly unwieldy towards the end so I'm going to do a couple followup PR's to clean up loose ends

  • renames src/_pytest/raises_group.py to src/_pytest/raises.py
  • moves pytest.raises from src/_pytest/python_api.py to src/_pytest/raises.py
  • adds several newsfragments that should've been bundlen with add RaisesGroup & Matcher #13192
  • add more detailed error message if you try to do RaisesGroup((ValueError, TypeError))
  • spend way too much mental energy debating with myself if passing the wrong type to a function expecting an exception type is a TypeError or a ValueError, or if it matters at all

@jakkdl jakkdl requested review from nicoddemus and Zac-HD March 7, 2025 09:41
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Mar 7, 2025
* renames src/_pytest/raises_group.py to src/_pytest/raises.py
* moves pytest.raises from src/_pytest/python_api.py to src/_pytest/raises.py
* adds several newsfragments that should've been bundlen with
* add RaisesGroup & Matcher pytest-dev#13192
* add more detailed error message if you try to do RaisesGroup((ValueError, TypeError))
* mess around with ValueError vs TypeError on invalid expected exception
@jakkdl jakkdl force-pushed the rgroup_followup branch from d01afa6 to eba3621 Compare March 7, 2025 09:42
Copy link
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

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

looks good, but needs some merge fixes - consider rebasing?

@@ -0,0 +1 @@
The context-manager form of :func:`pytest.raises` now has a ``check`` parameter that takes a callable that should return `True` when passed the raised exception if it should be accepted.
Copy link
Member

Choose a reason for hiding this comment

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

I think we really need to spell out this api, for people who aren't really familiar with Pytest. Something like

You can now pass :func:with pytest.raises(check=fn): <pytest.raises>, where fn is a function which takes a raised exception and returns a boolean. The test fails if no exception was raised (as usual with :func:pytest.raises), passes if an exception is raised and fn returns True, and re-raises the exception if fn returns False (which also fails the test).

@bluetech
Copy link
Member

bluetech commented Mar 9, 2025

Hi, I missed the previous episode of this so I hope you don't mind me asking a question late.

  1. I don't see documentation for the new check parameter in the pytest.raises docstring. Unless I missed it, can it be added?

  2. Would you mind briefly recounting to me the rationale for check? (Feel free to tell me to just go read the previous discussion...). Do I intuit correctly that it is not really meant to be used with the expected_exception overload, but with the 2nd overload when more elaborate logic is needed than just an isinstance?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:provided (automation) changelog entry is part of PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants