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.

Sign up for GitHub

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

Jump to bottom

Our internals should handle StopTest and other marker exceptions even when wrapped in an ExceptionGroup #4106

Open
Zac-HD opened this issue Sep 15, 2024 · 3 comments · May be fixed by #4110
Open

Our internals should handle StopTest and other marker exceptions even when wrapped in an ExceptionGroup #4106

Zac-HD opened this issue Sep 15, 2024 · 3 comments · May be fixed by #4110
Labels
bug something is clearly wrong here internals Stuff that only Hypothesis devs should ever see interop how to play nicely with other packages

Comments

@Zac-HD
Copy link
Member

Zac-HD commented Sep 15, 2024

Hypothesis currently misbehaves if the marker exceptions we raise are wrapped up in an ExceptionGroup, e.g.:

import trio
from hypothesis import given, strategies as st

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

    @trio.run
    async def _():
        async with trio.open_nursery() as nursery:
            nursery.start_soon(task, bool)
            nursery.start_soon(task, lambda _: False)
  + Exception Group Traceback (most recent call last):
  |   ...
  +-+---------------- 1 ----------------
    | Traceback (most recent call last):
    |   ...
    | hypothesis.errors.StopTest: 1
    +---------------- 2 ----------------
    | Traceback (most recent call last):
    |   ...
    | hypothesis.errors.Frozen: Cannot call start_example on frozen ConjectureData
    +------------------------------------

In such cases I think it's pretty clear that we should treat an exception group containing a StopTest as we would a StopTest itself. I think it's also pretty clear that any Frozen exceptions inside an ExceptionGroup are causally downstream of some other exception, and can therefore be discarded so long as there's any other exception in the group too.
@Zac-HD Zac-HD added bug something is clearly wrong here interop how to play nicely with other packages internals Stuff that only Hypothesis devs should ever see labels Sep 15, 2024
@jakkdl
Copy link

jakkdl commented Sep 19, 2024

How picky are you about depending on exceptiongroup for newer python versions?
3.12.1 added ExceptionGroup support for contextlib.suppress, so if we wanna use exceptiongroup.suppress we need to bump the install_requirement to python < 3.12.1.
I count 9 instances of suppress usage.

Big try: ... except: ... blocks such as

hypothesis/hypothesis-python/src/hypothesis/core.py

Line 1037 in 4d079f7

try:

Needs to have their exception handlers split out into separate functions. We're then left with the choice of using exceptiongroup.catch on all python versions to invoke them, or rolling our own handler in an except ExceptionGroup.

I don't think rolling our own is super ugly/complicated in either case, so am open to both options.

@jobh
Copy link
Contributor

jobh commented Sep 19, 2024 •
edited
Loading

I suggest it may be simplest to unwrap/discard the exceptiongroup around actual test execution, similar in effect to adding it to the end of ensure_free_stackframes.__exit__ but in a new context manager. Then, everything outside - including the suppress calls - is unaffected.

(i.e., with unwrap_exceptions(), ensure_free_stackframes(): return self.test(...))

[ed: To be clear, the context manager would discard Frozen subexceptions, and if there's exactly one remaining subexception then raise that instead of the group]

@jakkdl jakkdl linked a pull request Sep 19, 2024 that will close this issue
Draft
@jakkdl
Copy link

jakkdl commented Sep 19, 2024

Thanks @jobh! That made it much easier to get started on a PR :)
I've opened #4110, would love any reviews

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug something is clearly wrong here internals Stuff that only Hypothesis devs should ever see interop how to play nicely with other packages
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

first draft of handling marker exceptions wrapped in exceptiongroup jakkdl/hypothesis
3 participants
@jobh @jakkdl @Zac-HD