Skip to content

Conversation

@dariomesic
Copy link
Contributor

Purpose

Addresses a FIXME in testing/python/raises_group.py that pointed out the inconsistent use of repr(type) vs type.__name__ in error messages.

This PR refactors the RaisesContext.__exit__ method in src/_pytest/raises.py to consistently use type.__name__ (e.g., 'ValueError') instead of repr(type) (e.g., <class 'ValueError'>) in all "DID NOT RAISE" error messages. This improves the readability and consistency of these assertion failures.

Tests

Because the library's error message format was changed, this PR also updates all the tests in testing/python/raises.py and testing/python/raises_group.py that were asserting the old, inconsistent message format.

@dariomesic dariomesic force-pushed the style-use-typename-in-raises-errors branch from dca856e to 581ceb6 Compare October 29, 2025 22:52
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Oct 29, 2025
@dariomesic dariomesic marked this pull request as ready for review October 30, 2025 08:04
with pytest.raises(
Failed,
match=r"DID NOT RAISE <class 'raises(\..*)*ClassLooksIterableException'>",
match=r"DID NOT RAISE 'ClassLooksIterableException'",
Copy link
Member

Choose a reason for hiding this comment

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

This makes me wonder if we should maybe use __qualname__ and possibly __module__ for non-builtins in the output, what do others think? IMHO it's fine without, as it should be clear from context.

Copy link
Member

Choose a reason for hiding this comment

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

i do wonder if this has the potentila to break plugin testsuites

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 it's fair to do so. We've had it before that exception messages changed somehow, and often that required changes in pytest's testsuite and AFAIK sometimes elsewhere. Don't think exception messages are part of our API essentially.

Copy link
Member

Choose a reason for hiding this comment

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

This makes me wonder if we should maybe use qualname and possibly module for non-builtins in the output, what do others think

Good question. It is an interesting idea, but I agree with you that it should be clear from context and has the potential of cluttering the output. I suggest we leave like this for now.

@dariomesic dariomesic force-pushed the style-use-typename-in-raises-errors branch from 241d818 to a395424 Compare October 30, 2025 14:13
@dariomesic dariomesic force-pushed the style-use-typename-in-raises-errors branch from 4838955 to 2072395 Compare October 30, 2025 14:20
if len(self.expected_exceptions) == 1:
fail(f"DID NOT RAISE {self.expected_exceptions[0].__name__}")
else:
names = ", ".join(f"{x.__name__}" for x in self.expected_exceptions)
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: This can be simplified as it's already a string:

Suggested change
names = ", ".join(f"{x.__name__}" for x in self.expected_exceptions)
names = ", ".join(x.__name__ for x in self.expected_exceptions)

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.

4 participants