-
Notifications
You must be signed in to change notification settings - Fork 584
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,6 +61,7 @@ | |
FlakyFailure, | ||
FlakyReplay, | ||
Found, | ||
Frozen, | ||
HypothesisException, | ||
HypothesisWarning, | ||
InvalidArgument, | ||
|
@@ -148,6 +149,8 @@ | |
else: # pragma: no cover | ||
EllipsisType = type(Ellipsis) | ||
|
||
if sys.version_info < (3, 11): | ||
from exceptiongroup import ExceptionGroup | ||
|
||
TestFunc = TypeVar("TestFunc", bound=Callable) | ||
|
||
|
@@ -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 | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, the semantics here (generally) are a good question. I'd opine:
[*] hm, I see, that would cause an undesired unwrapping if user code raises a single-exception |
||
class StateForActualGivenExecution: | ||
def __init__(self, stuff, test, settings, random, wrapped_test): | ||
self.test_runner = get_executor(stuff.selfy) | ||
|
@@ -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: | ||
|
@@ -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() | ||
|
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"): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd just catch There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.