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

Improve Hypothesis's ability to shrink sums #1403

Merged
merged 3 commits into from
Jul 19, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions hypothesis-python/RELEASE.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
RELEASE_TYPE: patch

This release improves the shrinker's ability to handle situations where there
is an additive constraint between two values.

For example, consider the following test:


.. code-block:: python

import hypothesis.strategies as st
from hypothesis import given

@given(st.integers(), st.integers())
def test_does_not_exceed_100(m, n):
assert m + n <= 100
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To fail with m=0, n=100, this must use < not <=

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, yes, thanks.


Previously this could have failed with almost any pair ``(m, n)`` with
``0 <= m <= n`` and ``m + n == 100``. Now it should almost always fail with
``m=0, n=100``.

This is a relatively niche specialisation, but can be useful in situations
where e.g. a bug is triggered by an integer overflow.
56 changes: 56 additions & 0 deletions hypothesis-python/src/hypothesis/internal/conjecture/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -1339,6 +1339,7 @@ def greedy_shrink(self):
self.interval_deletion_with_block_lowering()
self.pass_to_interval()
self.reorder_bytes()
self.minimize_block_pairs_retaining_sum()

@property
def blocks(self):
Expand Down Expand Up @@ -2279,3 +2280,58 @@ def attempt(new_ordering):
break
else:
i += 1

def minimize_block_pairs_retaining_sum(self):
"""This pass minimizes pairs of blocks subject to the constraint that
their sum when interpreted as integers remains the same. This allow us
to normalize a number of examples that we would otherwise struggle on.
e.g. consider the following:

m = data.draw_bits(8)
n = data.draw_bits(8)
if m + n >= 256:
data.mark_interesting()

The ideal example for this is m=1, n=255, but we will almost never
find that without a pass like this - we would only do so if we
happened to draw n=255 by chance.

This kind of scenario comes up reasonably often in the context of e.g.
triggering overflow behaviour.
"""
i = 0
while i < len(self.shrink_target.blocks):
if self.is_payload_block(i):
j = i + 1
while j < len(self.shrink_target.blocks):
u, v = self.shrink_target.blocks[i]
m = int_from_bytes(self.shrink_target.buffer[u:v])
if m == 0:
break
r, s = self.shrink_target.blocks[j]
n = int_from_bytes(self.shrink_target.buffer[r:s])

if (
s - r == v - u and
self.is_payload_block(j)
):
def trial(x, y):
if s > len(self.shrink_target.buffer):
return False
attempt = bytearray(self.shrink_target.buffer)
try:
attempt[u:v] = int_to_bytes(x, v - u)
attempt[r:s] = int_to_bytes(y, s - r)
except OverflowError:
return False
return self.incorporate_new_buffer(attempt)
if trial(m - 1, n + 1) and m > 1:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be m >= 1, so that m can be driven all the way to 0, right? Also looks like putting the comparison before the call to trial might avoid trying to incorporate a bad buffer. (these probably cancel each other in practice but I'd find it easier to follow the other way)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is correct. We know m > 0. If m = 1 we want to run trial(0, n + 1) and then stop regardless of what it returns. I'll add a comment to clarify the logic.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aaah, right. That does make sense, but a comment would certainly be good!

m = int_from_bytes(self.shrink_target.buffer[u:v])
n = int_from_bytes(self.shrink_target.buffer[r:s])

tot = m + n
minimize_int(
m, lambda x: trial(x, tot - x)
)
j += 1
i += 1
42 changes: 42 additions & 0 deletions hypothesis-python/tests/cover/test_conjecture_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -1330,3 +1330,45 @@ def f(data):
runner.run()

assert runner.exit_reason == ExitReason.finished


@pytest.mark.parametrize('lo', [0, 1, 50])
def test_can_shrink_additively(monkeypatch, lo):
monkeypatch.setattr(
ConjectureRunner, 'generate_new_examples',
lambda self: self.test_function(
ConjectureData.for_buffer(hbytes([100, 100]))))

@run_to_buffer
def x(data):
m = data.draw_bits(8)
n = data.draw_bits(8)
if m >= lo and m + n == 200:
data.mark_interesting()

assert list(x) == [lo, 200 - lo]


def test_can_shrink_additively_losing_size(monkeypatch):
monkeypatch.setattr(
ConjectureRunner, 'generate_new_examples',
lambda self: self.test_function(
ConjectureData.for_buffer(hbytes([100, 100]))))

monkeypatch.setattr(
Shrinker, 'shrink', lambda self: (
self.minimize_block_pairs_retaining_sum(),
)
)

@run_to_buffer
def x(data):
m = data.draw_bits(8)
if m >= 10:
if m <= 50:
data.mark_interesting()
else:
n = data.draw_bits(8)
if m + n == 200:
data.mark_interesting()
assert len(x) == 1