hypothesis
hypothesis copied to clipboard
Inquisitor sometimes fails to report arguments as freely varying
Previously discussed in https://github.com/HypothesisWorks/hypothesis/pull/3818#discussion_r1467092386.
>>> @given(st.booleans(), st.booleans(), st.lists(st.none()), st.booleans(), st.booleans())
>>> def f(a, b, c, d, e):
assert not (b and d)
>>> f()
Falsifying example: test_inquisitor_comments_basic_fail_if_either(
# The test always failed when commented parts were varied together.
a=False,
b=True,
c=[], # or any other generated value
d=True,
e=False, # or any other generated value
)
should also mark a as freely varying.
The issue is here: https://github.com/HypothesisWorks/hypothesis/blob/626d45dd2cf8d4021e370d54b2cc4dcfab7eac4e/hypothesis-python/src/hypothesis/internal/conjecture/shrinker.py#L530-L536
where we are trying to be efficient by looking for buffers that prove an arg cannot vary freely, because we saw a buffer where the arg was the only thing that varied and the buffer went from interesting to valid. But this condition may have false positives if the size of the arg buffer changed size and happened to match a buffer where additional things varied.
To solve this, we'll need to check against a more structured representation of inputs than the buffer. We could:
- Check against the
DataTree
representation, which encodes the tree structure of calls. This may run into the same buffer-size-changed problem unless/until the DataTree is migrated to the ir (#3818), but it may also be fine as is. - Check against the final
ConjectureResult
(ConjectureResult.arg_slices
?).
I personally favor a DataTree
-based analysis, which would also be a suitable base to revive https://github.com/HypothesisWorks/hypothesis/pull/3624 to report on sub-argument parts such as elements of st.tuples()
, arguments to st.builds()
, etc.
Yup, I think the correct path forward here is clearly migrating inquisitor to the ir, which should resolve this in passing. Said migration could very likely be worked on at any point in parallel with the rest of the migration. I'll get to it at some point by necessity, of course 🙂 but it's not on my immediate task list if someone wanted to beat me to it.
I've added the explain phase migration as a task to #3921.