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

Rare PathTimeout errors in provider.realize(...) #21

Closed
Zac-HD opened this issue Aug 4, 2024 · 3 comments
Closed

Rare PathTimeout errors in provider.realize(...) #21

Zac-HD opened this issue Aug 4, 2024 · 3 comments

Comments

@Zac-HD
Copy link

Zac-HD commented Aug 4, 2024

#20 did indeed fix the ~dozens of returning a symbolic type errors that I was getting before, but in a few rare cases it looks like that's uncovered a new failure mode: this CI job has three tests failing with

  File ".../hypothesis_crosshair_provider/crosshair_provider.py", line 293, in realize
    return self.export_value(value)
  File ".../hypothesis_crosshair_provider/crosshair_provider.py", line 280, in export_value
    return deep_realize(value)
  File ".../crosshair/core.py", line 258, in deep_realize
    return deepcopyext(value, CopyMode.REALIZE, {})
  File ".../crosshair/copyext.py", line 47, in deepcopyext
    obj = obj.__ch_realize__()  # type: ignore
  File ".../crosshair/libimpl/builtinslib.py", line 3819, in __ch_realize__
    return bytes(tracing_iter(self.inner))
  File ".../crosshair/tracers.py", line 463, in tracing_iter
    value = next(itr)
  File ".../crosshair/libimpl/builtinslib.py", line 2537, in __iter__
    if not space.smt_fork(idx < my_smt_len):
  File ".../crosshair/statespace.py", line 1043, in smt_fork
    return self.choose_possible(expr, probability_true)
  File ".../crosshair/statespace.py", line 854, in choose_possible
    self.check_timeout()
  File ".../crosshair/statespace.py", line 841, in check_timeout
    raise PathTimeout
crosshair.util.PathTimeout

I'm not really sure what we should do here. Maybe we should have a known exception to indicate that the current test case can't be realized so that Hypothesis can skip it as if for assume(False)? (raised/caught here) Increasing the timeout will only make the problem less common rather than fixing it, but at the cost of rather slow tests. Maybe that - or removing it entirely - is worth it though?

@pschanely
Copy link
Owner

So, it would be really great to have a designated exception or some other way to indicate that I need to abort the current iteration. And, as you're seeing here, that can happen not only when the test case runs, but also everywhere we realize values. How difficult would this be to support? I'm already doing a few horrible things in the plugin to work around this that have their own edge-case failure scenarios.

In the specific case of this timeout, I suspect we want to retain it, though perhaps ideally it'd be 2x the current test's deadline or something like that.

@Zac-HD
Copy link
Author

Zac-HD commented Aug 9, 2024

So, it would be really great to have a designated exception or some other way to indicate that I need to abort the current iteration. And, as you're seeing here, that can happen not only when the test case runs, but also everywhere we realize values. How difficult would this be to support?

Perfectly reasonable, it'll just be a little fiddly to implement - but easy enough to test using a provider that fails on the 1,2,3,4,5,...th interaction to check that they're all handled properly.

In the specific case of this timeout, I suspect we want to retain it, though perhaps ideally it'd be 2x the current test's deadline or something like that.

Makes sense! With timeout=None defaulting to ten seconds or something.

@Zac-HD
Copy link
Author

Zac-HD commented Oct 9, 2024

I think fixed by dd7d1ae (was #23), will reopen if I see it again 🙂

@Zac-HD Zac-HD closed this as completed Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants