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

Replace MultipleFailures with ExceptionGroup on Python 3.11+ #3191

Closed
wants to merge 1 commit into from

Conversation

Zac-HD
Copy link
Member

@Zac-HD Zac-HD commented Dec 10, 2021

PEP 654 introduces a new ExceptionGroup type, which is designed to replace Trio's MultiError and our very own MultipleFailures types. There's also neat new except* syntax for handling them, but that's not our problem. Based on our conversation, @iritkatriel also added BaseException.__note__ so that we can attach our Failing example: strings (etc) to the exception objects, instead of printing them by hand.

Closes #3175, related to #2192 (comment).

Example outputs on Python 3.9.8 and 3.11.0a3
from hypothesis import given, strategies as st, target

@given(st.integers())
def test(x):
    target(x)
    assert x < 0
    assert x > 0

Running pytest test.py prints, for respectively Python 3.9.8 and 3.11.0a3:

Traceback (most recent call last):
  File "test.py", line 4, in test
    def test(x):
  File "hypothesis/core.py", line 1202, in wrapped_test
    raise the_error_hypothesis_found
hypothesis.errors.MultipleFailures: Hypothesis found 2 distinct failures.
--------------------------------- Hypothesis ----------------------------------
Highest target score: 1  (label='')

Falsifying example: test(
    x=-1,
)
Traceback (most recent call last):
  File "test.py", line 7, in test
    assert x > 0
AssertionError: assert -1 > 0

Falsifying example: test(
    x=0,
)
Traceback (most recent call last):
  File "test.py", line 6, in test
    assert x < 0
AssertionError: assert 0 < 0
_______________________________________________ test _______________________________________________
  + Exception Group Traceback (most recent call last):
  |   File "test.py", line 4, in test
  |     def test(x):
  |
  |   File "hypothesis/core.py", line 1202, in wrapped_test
  |     raise the_error_hypothesis_found
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  | ExceptionGroup: Hypothesis found 2 distinct failures.
  |
  | Highest target score: 1  (label='')
  |
  +-+---------------- 1 ----------------
    | Traceback (most recent call last):
    |   File "test.py", line 7, in test
    |     assert x > 0
    |     ^^^^^^^^^^^^
    | AssertionError: assert -1 > 0
    |
    | Falsifying example: test(
    |     x=-1,
    | )
    +---------------- 2 ----------------
    | Traceback (most recent call last):
    |   File "test.py", line 6, in test
    |     assert x < 0
    |     ^^^^^^^^^^^^
    | AssertionError: assert 0 < 0
    |
    | Falsifying example: test(
    |     x=0,
    | )
    +------------------------------------

So use of the new __note__ approach does change the relative order of traceback and other information, but I don't think that's a big deal - we can reverse our own printing for consistency if it's important, because there are clear upstream reasons to display the __note__ in relation to the exception object rather than the traceback.

I'm ambivalent about using __note__ for singular errors, but if we want to that's left for future work in order to keep this diff small.

@Zac-HD Zac-HD added the interop how to play nicely with other packages label Dec 10, 2021
@Zac-HD Zac-HD force-pushed the exceptiongroup branch 3 times, most recently from c5512fc to d8e9868 Compare December 11, 2021 03:50
@Zac-HD
Copy link
Member Author

Zac-HD commented Dec 13, 2021

Another future thing: interesting_origin should become ExceptionGroup-aware, adding a field for the interesting-origin-of-inner-exceptions-if-any. Unlikely to come up, but I'm keen to avoid conflating errors if we can distinguish them (and report_multiple_bugs is True, of course).

(implemented in #3232)

@Zac-HD
Copy link
Member Author

Zac-HD commented Dec 26, 2021

Per python/peps#2203, we should be using BaseExceptionGroup because our caught failures might not be Exceptions. We should also look at our catch locations and add a clause to catch BaseExceptionGroup if-and-only-if all the inner exceptions are of types that we want to catch. See:

def failure_exceptions_to_catch():
"""Return a tuple of exceptions meaning 'this test has failed', to catch.
This is intended to cover most common test runners; if you would
like another to be added please open an issue or pull request.
"""
# While SystemExit and GeneratorExit are instances of BaseException, we also
# expect them to be deterministic - unlike KeyboardInterrupt - and so we treat
# them as standard exceptions, check for flakiness, etc.
# See https://github.com/HypothesisWorks/hypothesis/issues/2223 for details.
exceptions = [Exception, SystemExit, GeneratorExit]
if "_pytest" in sys.modules:
exceptions.append(sys.modules["_pytest"].outcomes.Failed)
return tuple(exceptions)

See also pytest-dev/pytest#9680 - a coherent ecosystem approach to this would be nice.

@Zac-HD
Copy link
Member Author

Zac-HD commented Jan 16, 2022

The new exceptiongroup package on PyPI provides a backport for older versions, which I am very happy about from an ecosystem interop perspective.

(once a few questions are resolved around handling in Pytest, and how we print things like falsifying examples before PEP-678 __note__ is available)

@Zac-HD
Copy link
Member Author

Zac-HD commented Apr 16, 2022

Since we now have both an exceptiongroup backport and PEP-678 .add_note(), the new (very similar) plan is described in #3175 (comment). I'm also going to close this instead of updating it in place, since the draft is useful as historical context for the PEP.

@Zac-HD Zac-HD closed this Apr 16, 2022
@Zac-HD Zac-HD deleted the exceptiongroup branch July 3, 2022 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interop how to play nicely with other packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Experiment with ExceptionGroup and BaseException.add_note() (in Python 3.11)
1 participant