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

first draft of handling marker exceptions wrapped in exceptiongroup #4110

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 44 additions & 2 deletions hypothesis-python/src/hypothesis/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
FlakyFailure,
FlakyReplay,
Found,
Frozen,
HypothesisException,
HypothesisWarning,
InvalidArgument,
Expand Down Expand Up @@ -148,6 +149,8 @@
else: # pragma: no cover
EllipsisType = type(Ellipsis)

if sys.version_info < (3, 11):
from exceptiongroup import ExceptionGroup
Comment on lines +152 to +153
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if sys.version_info < (3, 11):
from exceptiongroup import ExceptionGroup
from hypothesis.internal.compat import BaseExceptionGroup


TestFunc = TypeVar("TestFunc", bound=Callable)

Expand Down Expand Up @@ -768,6 +771,45 @@ def execute(data, function):
return default_executor


@contextlib.contextmanager
def unwrap_exception_group():
def _flatten_group(excgroup: BaseExceptionGroup) -> list[BaseException]:
found_exceptions = []
for exc in excgroup.exceptions:
if isinstance(exc, BaseExceptionGroup):
found_exceptions.extend(_flatten_group(exc))
else:
found_exceptions.append(exc)
return found_exceptions

try:
yield
except BaseExceptionGroup as excgroup:
# Found? RewindRecursive? FlakyReplay?
marker_exceptions = (StopTest, UnsatisfiedAssumption)
frozen_exceptions, non_frozen_exceptions = excgroup.split(Frozen)
marker_exceptions, user_exceptions = non_frozen_exceptions.split(lambda e: isinstance(e, marker_exceptions))

# TODO: not a great variable name
if user_exceptions:
raise

if non_frozen_exceptions:
flattened_non_frozen_exceptions = _flatten_group(non_frozen_exceptions)
if len(flattened_non_frozen_exceptions) == 1:
raise flattened_non_frozen_exceptions[0] from None
# multiple non-frozen exceptions, re-raise the entire group
raise

flattened_frozen_exceptions = _flatten_group(frozen_exceptions)
# we only have frozen exceptions
if len(flattened_frozen_exceptions) == 1:
raise flattened_frozen_exceptions[0] from excgroup

# multiple frozen exceptions
# TODO: raise a group? The first one? None of them?
raise frozen_exceptions[0] from excgroup

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, the semantics here (generally) are a good question.

I'd opine:

  • If all exceptions are Frozen, raise the first (as above).
  • Else, if exactly one exception is non-Frozen, raise that one [*].
  • Otherwise, reconstruct and raise a group with just the non-frozen ones (maybe .with_traceback(eg.__traceback__)).

[*] hm, I see, that would cause an undesired unwrapping if user code raises a single-exception ExceptionGroup. Maybe the second step should be "if exactly one ... and that exception is a StopException or HypothesisException.

class StateForActualGivenExecution:
def __init__(self, stuff, test, settings, random, wrapped_test):
self.test_runner = get_executor(stuff.selfy)
Expand Down Expand Up @@ -841,7 +883,7 @@ def execute_once(

@proxies(self.test)
def test(*args, **kwargs):
with ensure_free_stackframes():
with unwrap_exception_group(), ensure_free_stackframes():
return self.test(*args, **kwargs)

else:
Expand All @@ -853,7 +895,7 @@ def test(*args, **kwargs):
arg_gctime = gc_cumulative_time()
start = time.perf_counter()
try:
with ensure_free_stackframes():
with unwrap_exception_group(), ensure_free_stackframes():
result = self.test(*args, **kwargs)
finally:
finish = time.perf_counter()
Expand Down
54 changes: 54 additions & 0 deletions hypothesis-python/tests/cover/test_exceptiongroup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import sys
import asyncio

from trio.testing import RaisesGroup
from hypothesis import errors, given, strategies as st
import pytest

if sys.version_info < (3, 11):
from exceptiongroup import ExceptionGroup

def test_exceptiongroup():
@given(st.data())
def test_function(data):
async def task(pred):
return data.draw(st.booleans().filter(pred))

async def _main():
async with asyncio.TaskGroup() as tg:
tg.create_task(task(bool))
tg.create_task(task(lambda _: False))
asyncio.run(_main())
with pytest.raises(errors.Unsatisfiable):
test_function()

def test_exceptiongroup_nested():
@given(st.data())
def test_function(data):
async def task(pred):
return data.draw(st.booleans().filter(pred))

async def _main():
async with asyncio.TaskGroup():
async with asyncio.TaskGroup() as tg2:
tg2.create_task(task(bool))
tg2.create_task(task(lambda _: False))
asyncio.run(_main())
with pytest.raises(errors.Unsatisfiable):
test_function()


def test_exceptiongroup_user_originated():
@given(st.data())
def test_function(data):
raise ExceptionGroup("foo", [ValueError(), ValueError()])
with RaisesGroup(ValueError, ValueError, match="foo"):
test_function()


@given(st.data())
def test_single_exc_group(data):
raise ExceptionGroup("important message for user", [ValueError()])

with RaisesGroup(ValueError, match="important message for user"):
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd just catch BaseExceptionGroup (via hypothesis.internal.compat.BaseExceptionGroup) and check the subexceptions explicitly, to avoid the trio dependency.

Copy link
Author

Choose a reason for hiding this comment

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

ye that's probably the way to go. I'm incredibly biased on the matter as the person who wrote trio.testing.RaisesGroup and is trying to get it added to pytest 😇

Copy link
Member

Choose a reason for hiding this comment

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

(and as a dual maintainer of Pytest and Trio, among other things, I'm pretty keen to support that when I have time 😅)

test_single_exc_group()
1 change: 1 addition & 0 deletions requirements/test.in
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ attrs==24.1.0 # too early for https://github.com/python-attrs/attrs/pull/1329
pexpect
pytest
pytest-xdist
trio # for trio.testing.RaisesGroup, until pytest merges a version
Loading