pytest-dev / pytest-dev/pytest

What should `raises` and `RaisesGroup` raise?

Open
#13,286 7 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

topic: reporting type: backward compatibility
Dominant language
Python
Stars
14.5k
Forks
3.4k
Avg merge
2d 9h
Merged PRs (30d)
35

Description

The current behavior on the released pytest version is:

  1. if match fails, pytest.raises raises an AssertionError
  2. if the type differs, the exception propagates through.
  3. if no exception was raised, pytest.raises raises pytest.Failed

#13192 complicated this for several reasons, and having RaisesGroup adhere to point 2 was untenable. It became non-trivial to figure out why the "type" of an exceptiongroup with several nested exceptions and groups couldn't get matched against another group. It also became very weird when the "type" started to include matches itself, and it would've necessitated extra logic to figure out if RaisesGroup(RaisesExc(ValueError, match="foo")) failed because of the type or the match in RaisesExc. Or in even weirder cases where it fails for both reasons:

with RaisesGroup(RaisesExc(ValueError), RaisesExc(match="foo")):
  raise ExceptionGroup(TypeError("bar"), TypeError("bar"))

So RaisesGroup will raise an exception in all cases.

We're then faced with how this should affect pytest.raises. #13192 made it raise an AssertionError upon type difference (and if check fails), but this would likely be considered a breaking change. I personally think it's very different if a test fails because I got a completely unexpected exception, versus a different exception than the one I specifically expected in a raises block; but I don't think that's a huge win.
The other downside of keeping current behavior is it becoming an arbitrary-but-for-historic-reasons difference between raises and RaisesGroup.
In either case, the original exception is accessible in the __cause__ attribute and adapting code should be straightforward.

I do also find the distinction between raising pytest.Fail versus AssertionError quite arbitrary, and think they should be unified. My instinct would be for all failures to be pytest.Fail, as AssertionError usually signals to users that an explicit assert has failed. We could make this change backward-compatible by having the exception raised inherit from both AssertionError and pytest.Fail if we really want to.

This also matters for ExceptionInfo.match, which ideally should share logic with raises for string matching - but currently doesn't. It currently raises an AssertionError on failure.

tl;dr

  1. Should raises propagate the exception, or raise AssertionError, or Fail?
  2. Is the distinction between raising different exception types useful, or confusing?
  3. Should RaisesGroup raise pytest.Fail or AssertionError when failing a match, regardless of reason?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing pytest.raises, RaisesGroup, RaisesExc, and ExceptionInfo.match, along with issue #13192. The issue needs a settled policy for failure and exception types before implementation can begin; done would mean the behavior is consistently defined across these APIs.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
testing-qa
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.