Skip to content

Have pytest.raises match against exception __notes__#11227

Merged
nicoddemus merged 12 commits intopytest-dev:mainfrom
ivirshup:match-exception-notes
Jul 18, 2023
Merged

Have pytest.raises match against exception __notes__#11227
nicoddemus merged 12 commits intopytest-dev:mainfrom
ivirshup:match-exception-notes

Conversation

@ivirshup
Copy link
Copy Markdown
Contributor

@ivirshup ivirshup commented Jul 17, 2023

Closes #11223

Adds support for matching against __notes__ added to an exception within pytest.raises. See issue for discussion/ design.

Notes

TODO

  • Add yourself to AUTHORS in alphabetical
  • Create a new changelog file in the changelog folder, with a name like <ISSUE NUMBER>.<TYPE>.rst.

Copy link
Copy Markdown
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 to me, thanks again Isaac! Once those todos are done I'd be happy to merge.

Copy link
Copy Markdown
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

LGTM, left some simple suggestions.

>>> with pytest.raises(ValueError, match=r'must be \d+$'):
... raise ValueError("value must be 42")

The ``match`` argument searches the formatted exception string, which includes any ``__notes__``:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perhaps it would be nice to add a link to the relevant PEP?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added!

@ivirshup ivirshup marked this pull request as ready for review July 18, 2023 09:49
@ivirshup
Copy link
Copy Markdown
Contributor Author

Thanks for the quick review! Addressed comments, finished up, and added a couple more test cases.

"""
__tracebackhide__ = True
value = str(self.value)
value = "\n".join(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We might want to work with @agronholm to ensure note helpers in the backport as well as shared formatting as follow up

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The tests here don't seem to involve exception groups though.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@agronholm my impression is that exception notes are closely related and my understanding is that integration with the backport may be helpful as per agronholm/exceptiongroup#31 and since formatting may also be affected my impression is that the most benefit for library users would come from having one integration place instead of multiple

Copy link
Copy Markdown
Contributor Author

@ivirshup ivirshup Jul 18, 2023

Choose a reason for hiding this comment

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

About formatting: I don't think there is a function in traceback that provides the exact rendering pytest expects. I think all the methods that traceback provides will include the exception type.

Copy link
Copy Markdown
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

Im happy now that the design decision is done and we will use match all

Copy link
Copy Markdown
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Nice, thanks @ivirshup!

@nicoddemus nicoddemus merged commit 1de0923 into pytest-dev:main Jul 18, 2023
FazeelUsmani added a commit to FazeelUsmani/astropy that referenced this pull request Nov 12, 2025
- Use pytest.raises(match=...) instead of manual exception capture
- Add towncrier changelog entries for both nddata and coordinates
- Follows pytest-dev/pytest#11227 recommendation
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.

Support __notes__ in pytest.raises

5 participants