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

UnsatisfiedAssumption masks runtime exceptions in execute_example #3743

Closed
nickcollins opened this issue Sep 15, 2023 · 4 comments · Fixed by #3751
Closed

UnsatisfiedAssumption masks runtime exceptions in execute_example #3743

nickcollins opened this issue Sep 15, 2023 · 4 comments · Fixed by #3751
Labels
legibility make errors helpful and Hypothesis grokable

Comments

@nickcollins
Copy link
Contributor

If execute_example messes up and causes an unexpected exception, during an example run that involved an assumption failure, the test output shows UnsatisfiedAssumption, rather than focusing on the unexpected exception that is the real source of the bug. This is a case of garbage in garbage out, but it would be nice if we saw the unexpected exception rather than the UnsatisfiedAssumption because the latter doesn't help us find and address the actual bug in our execute_example code.

In the original case of this, I got

...
hypothesis.errors.UnsatisfiedAssumption

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
...

So in at least some cases, the error reporting does acknowledge the actual problem that caused the crash. However, in the minimal cut that I show below, there is no "... another exception occurred:" part, so I'm not sure which cases lead to which output.

Repro:

from hypothesis import assume
import unittest

class UnsatisfiedAssumptionTest(unittest.TestCase):

  def execute_example(self, fn):
    try:
      return fn()
    except Exception as e:
      print([][0]) # unexpected exception, due to invalid array indexing
      raise e

  def testUnsatisfiedAssumption(self):
    assume(False)

if __name__ == "__main__":
  unittest.main()

Result:

E
======================================================================
ERROR: testUnsatisfiedAssumption (__main__.UnsatisfiedAssumptionTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/example.py", line 14, in testUnsatisfiedAssumption
    assume(False)
  File "/home/nick/miniconda3/lib/python3.10/site-packages/hypothesis/control.py", line 38, in assume
    raise UnsatisfiedAssumption
hypothesis.errors.UnsatisfiedAssumption
@Zac-HD Zac-HD added the legibility make errors helpful and Hypothesis grokable label Sep 15, 2023
@Zac-HD
Copy link
Member

Zac-HD commented Sep 16, 2023

I think your original case is OK - given the situation, During handling of the above exception, another exception occurred: is about the best we can do.


Your minimal repro has a different problem: the execute_example() method isn't executed at all, because you're calling assume(False) inside a function that hasn't been wrapped in @given(...), and so Hypothesis is not actually involved.

To resolve that, I think we should deprecate calling assume() or reject() outside of a test, as we already do for note() and event(). Would you be interested in opening a PR for this?

@nickcollins
Copy link
Contributor Author

Oh I see! And yea, I can make a PR. I am a little tempted to factor all these has-context checks into a single ensure_context helper function, or I can just kinda copy the logic from event and should_note into assume and reject

@Zac-HD
Copy link
Member

Zac-HD commented Sep 16, 2023

Thanks! I think we'll want to copy the logic, because these new checks should use note_deprecation(...) rather than raise InvalidArgument(...) until the next major version.

@nickcollins
Copy link
Contributor Author

Created pull request: #3748
Let me know if that looks good or if anything needs tweaking

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legibility make errors helpful and Hypothesis grokable
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants