search

pschanely / hypothesis-crosshair

Level-up your Hypothesis tests with CrossHair
MIT License
8 stars 0 forks source link

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

Closed Zac-HD closed 1 month ago

Zac-HD commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 1 month ago

I think fixed by https://github.com/pschanely/hypothesis-crosshair/commit/dd7d1aeb8714fa7cd34e14fd71b495464a2c0473 (was #23), will reopen if I see it again 🙂