Adjust post_test_case_hook scope#4022
Conversation
Zac-HD
left a comment
There was a problem hiding this comment.
This looks great - thanks for the fix @tybug!
Should we also try to run with crosshair in CI? That would have caught this at the time, and I think we could probably run most of our coverage tests with crosshair...
| # should we raise Flaky here instead? | ||
| if data.status != Status.INTERESTING: | ||
| self.exit_with(ExitReason.flaky) |
There was a problem hiding this comment.
I think exit-flaky is correct, but we'll also need a covering test 🙂
There was a problem hiding this comment.
just exiting is a silent conclusion of the run, so after looking at it I think we do not want this. I put a stop-gap flaky raise but we lose eg the nice custom deadline reporting we get at the StateForActualGivenExecution level. I don't see an easy way to thread things through at this lower level but I am also having a hard time keeping the engine control flow graph straight. I think this is acceptable for now.
(also added a test)
Would definitely like to work towards this. I've started taking a look at the remaining blockers here, of which pschanely/hypothesis-crosshair#6 is one. |
Zac-HD
left a comment
There was a problem hiding this comment.
Looks good; as you say it'd be nice to upgrade this later but the PR is a clear improvement on status quo, so lets merge!
(I'd like to have this out before I give a talk about it tomorrow 😉)
e53bd2f to
9f5a9ed
Compare
|
hold off on this for a sec, the new check doesn't play nicely with symbolic-typed values from crosshair. |
9f5a9ed to
1f86be4
Compare
assert type(node.value) is crosshair.libimpl.builtinslib.SymbolicBoolAnd this is a good reason to be carrying around |
Move
post_test_case_hookhigher up so it correctly handles symbolic lifetimes. Reported in pschanely/CrossHair#263, regressed in #3977.Also includes a bandaid for flakiness when replaying interesting non-hypothesis backend examples. I ran into this when we flakily raised
DeadlineExceededwith the crosshair backend. I think probably we want to raiseFlakyhere instead of silently exiting though.