Merge pull request #2185 from HypothesisWorks/DRMacIver/prune-dead

Prune parts of the data tree that have discards in them

hypothesis-python/RELEASE.rst

@@ -0,0 +1,7 @@
+RELEASE_TYPE: patch
+
+This release changes how Hypothesis manages its search space in cases where it
+generates redundant data. This should cause it to generate significantly fewer
+duplicated examples (especially with short integer ranges), and may cause it to
+produce more useful examples in some cases (especially ones where there is a
+significant amount of filtering).

hypothesis-python/src/hypothesis/internal/conjecture/data.py

@@ -684,6 +684,9 @@ def draw_bits(self, n_bits, forced, value):
         * ``value`` is the result that ``draw_bits`` returned.
         """
 
+    def kill_branch(self):
+        """Mark this part of the tree as not worth re-exploring."""
+
 
 @attr.s(slots=True)
 class ConjectureResult(object):
@@ -897,6 +900,32 @@ def stop_example(self, discard=False):
                     self.mark_invalid()
             else:
                 self.consecutive_discard_counts[-1] = 0
+        if discard:
+            # Once we've discarded an example, every test case starting with
+            # this prefix contains discards. We prune the tree at that point so
+            # as to avoid future test cases bothering with this region, on the
+            # assumption that some example that you could have used instead
+            # there would *not* trigger the discard. This greatly speeds up
+            # test case generation in some cases, because it allows us to
+            # ignore large swathes of the search space that are effectively
+            # redundant.
+            #
+            # A scenario that can cause us problems but which we deliberately
+            # have decided not to support is that if there are side effects
+            # during data generation then you may end up with a scenario where
+            # every good test case generates a discard because the discarded
+            # section sets up important things for later. This is not terribly
+            # likely and all that you see in this case is some degradation in
+            # quality of testing, so we don't worry about it.
+            #
+            # Note that killing the branch does *not* mean we will never
+            # explore below this point, and in particular we may do so during
+            # shrinking. Any explicit request for a data object that starts
+            # with the branch here will work just fine, but novel prefix
+            # generation will avoid it, and we can use it to detect when we
+            # have explored the entire tree (up to redundancy).
+
+            self.observer.kill_branch()
 
     def note_event(self, event):
         self.events.add(event)

hypothesis-python/src/hypothesis/internal/conjecture/datatree.py

@@ -46,10 +46,23 @@ def inconsistent_generation():
 EMPTY = frozenset()
 
 
+@attr.s(slots=True)
+class Killed(object):
+    """Represents a transition to part of the tree which has been marked as
+    "killed", meaning we want to treat it as not worth exploring, so it will
+    be treated as if it were completely explored for the purposes of
+    exhaustion."""
+
+    next_node = attr.ib()
+
+
 @attr.s(slots=True)
 class Branch(object):
+    """Represents a transition where multiple choices can be made as to what
+    to drawn."""
+
     bit_length = attr.ib()
-    children = attr.ib()
+    children = attr.ib(repr=False)
 
     @property
     def max_children(self):
@@ -58,6 +71,8 @@ def max_children(self):
 
 @attr.s(slots=True, frozen=True)
 class Conclusion(object):
+    """Represents a transition to a finished state."""
+
     status = attr.ib()
     interesting_origin = attr.ib()
 
@@ -174,7 +189,7 @@ def check_exhausted(self):
             and len(self.forced) == len(self.values)
             and self.transition is not None
         ):
-            if isinstance(self.transition, Conclusion):
+            if isinstance(self.transition, (Conclusion, Killed)):
                 self.is_exhausted = True
             elif len(self.transition.children) == self.transition.max_children:
                 self.is_exhausted = all(
@@ -228,13 +243,14 @@ def append_int(n_bits, value):
                     # vary, so what follows is not fixed.
                     return hbytes(novel_prefix)
             else:
-                assert not isinstance(current_node.transition, Conclusion)
+                assert not isinstance(current_node.transition, (Conclusion, Killed))
                 if current_node.transition is None:
                     return hbytes(novel_prefix)
                 branch = current_node.transition
                 assert isinstance(branch, Branch)
                 n_bits = branch.bit_length
 
+                check_counter = 0
                 while True:
                     k = random.getrandbits(n_bits)
                     try:
@@ -246,6 +262,15 @@ def append_int(n_bits, value):
                         append_int(n_bits, k)
                         current_node = child
                         break
+                    check_counter += 1
+                    # We don't expect this assertion to ever fire, but coverage
+                    # wants the loop inside to run if you have branch checking
+                    # on, hence the pragma.
+                    assert (  # pragma: no cover
+                        check_counter != 1000
+                        or len(branch.children) < (2 ** n_bits)
+                        or any([not v.is_exhausted for v in branch.children.values()])
+                    )
 
     def rewrite(self, buffer):
         """Use previously seen ConjectureData objects to return a tuple of
@@ -282,12 +307,15 @@ def simulate_test_function(self, data):
                     data.conclude_test(t.status, t.interesting_origin)
                 elif node.transition is None:
                     raise PreviouslyUnseenBehaviour()
-                else:
+                elif isinstance(node.transition, Branch):
                     v = data.draw_bits(node.transition.bit_length)
                     try:
                         node = node.transition.children[v]
                     except KeyError:
                         raise PreviouslyUnseenBehaviour()
+                else:
+                    assert isinstance(node.transition, Killed)
+                    node = node.transition.next_node
         except StopTest:
             pass
 
@@ -300,6 +328,7 @@ def __init__(self, tree):
         self.__current_node = tree.root
         self.__index_in_current_node = 0
         self.__trail = [self.__current_node]
+        self.__killed = False
 
     def draw_bits(self, n_bits, forced, value):
         i = self.__index_in_current_node
@@ -349,6 +378,27 @@ def draw_bits(self, n_bits, forced, value):
         if self.__trail[-1] is not self.__current_node:
             self.__trail.append(self.__current_node)
 
+    def kill_branch(self):
+        """Mark this part of the tree as not worth re-exploring."""
+        if self.__killed:
+            return
+
+        self.__killed = True
+
+        if self.__index_in_current_node < len(self.__current_node.values) or (
+            self.__current_node.transition is not None
+            and not isinstance(self.__current_node.transition, Killed)
+        ):
+            inconsistent_generation()
+
+        if self.__current_node.transition is None:
+            self.__current_node.transition = Killed(TreeNode())
+            self.__update_exhausted()
+
+        self.__current_node = self.__current_node.transition.next_node
+        self.__index_in_current_node = 0
+        self.__trail.append(self.__current_node)
+
     def conclude_test(self, status, interesting_origin):
         """Says that ``status`` occurred at node ``node``. This updates the
         node if necessary and checks for consistency."""
@@ -381,6 +431,10 @@ def conclude_test(self, status, interesting_origin):
         node.check_exhausted()
         assert len(node.values) > 0 or node.check_exhausted()
 
+        if not self.__killed:
+            self.__update_exhausted()
+
+    def __update_exhausted(self):
         for t in reversed(self.__trail):
             # Any node we've traversed might have now become exhausted.
             # We check from the right. As soon as we hit a node that

hypothesis-python/src/hypothesis/internal/conjecture/engine.py

@@ -694,7 +694,7 @@ def check_result(result):
             data = self.new_conjecture_data_for_buffer(buffer)
             self.test_function(data)
             result = check_result(data.as_result())
-            assert status is None or result.status == status
+            assert status is None or result.status == status, (status, result.status)
             status = result.status
         if status == Status.OVERRUN:
             result = Overrun

hypothesis-python/tests/cover/test_conjecture_data_tree.py

@@ -356,3 +356,20 @@ def test_will_generate_novel_prefix_to_avoid_exhausted_branches():
 
     assert len(prefix) == 2
     assert prefix[0] == 0
+
+
+def test_will_mark_changes_in_discard_as_flaky():
+    tree = DataTree()
+    data = ConjectureData.for_buffer([1, 1], observer=tree.new_observer())
+    data.start_example(10)
+    data.draw_bits(1)
+    data.stop_example()
+    data.draw_bits(1)
+    data.freeze()
+
+    data = ConjectureData.for_buffer([1, 1], observer=tree.new_observer())
+    data.start_example(10)
+    data.draw_bits(1)
+
+    with pytest.raises(Flaky):
+        data.stop_example(discard=True)

hypothesis-python/tests/cover/test_conjecture_engine.py

@@ -37,7 +37,11 @@
 )
 from hypothesis.internal.conjecture.shrinker import Shrinker, block_program
 from hypothesis.internal.conjecture.shrinking import Float
-from hypothesis.internal.conjecture.utils import Sampler, calc_label_from_name
+from hypothesis.internal.conjecture.utils import (
+    Sampler,
+    calc_label_from_name,
+    integer_range,
+)
 from hypothesis.internal.entropy import deterministic_PRNG
 from tests.common.strategies import SLOW, HardToShrink
 from tests.common.utils import no_shrink
@@ -727,12 +731,12 @@ def shrinker(data):
 
 
 def test_discarding_iterates_to_fixed_point():
-    @shrinking_from(hbytes([1] * 10) + hbytes([0]))
+    @shrinking_from(hbytes(list(hrange(100, -1, -1))))
     def shrinker(data):
         data.start_example(0)
-        data.draw_bits(1)
+        data.draw_bits(8)
         data.stop_example(discard=True)
-        while data.draw_bits(1):
+        while data.draw_bits(8):
             pass
         data.mark_interesting()
 
@@ -1397,3 +1401,41 @@ def test_exhaust_space():
         runner.run()
         assert runner.tree.is_exhausted
         assert runner.valid_examples == 2
+
+
+SMALL_COUNT_SETTINGS = settings(TEST_SETTINGS, max_examples=500)
+
+
+def test_discards_kill_branches():
+    starts = set()
+
+    with deterministic_PRNG():
+
+        def test(data):
+            assert runner.call_count <= 256
+            while True:
+                data.start_example(1)
+                b = data.draw_bits(8)
+                data.stop_example(b != 0)
+                if len(data.buffer) == 1:
+                    s = hbytes(data.buffer)
+                    assert s not in starts
+                    starts.add(s)
+                if b == 0:
+                    break
+
+        runner = ConjectureRunner(test, settings=SMALL_COUNT_SETTINGS)
+        runner.run()
+        assert runner.call_count == 256
+
+
+@pytest.mark.parametrize("n", range(1, 32))
+def test_number_of_examples_in_integer_range_is_bounded(n):
+    with deterministic_PRNG():
+
+        def test(data):
+            assert runner.call_count <= 2 * n
+            integer_range(data, 0, n)
+
+        runner = ConjectureRunner(test, settings=SMALL_COUNT_SETTINGS)
+        runner.run()

hypothesis-python/tests/cover/test_regex.py

@@ -26,7 +26,7 @@
 import hypothesis.strategies as st
 from hypothesis import assume, given, settings
 from hypothesis.errors import InvalidArgument
-from hypothesis.internal.compat import PY3, PYPY, hrange, hunichr
+from hypothesis.internal.compat import PY2, PY3, PYPY, hrange, hunichr
 from hypothesis.searchstrategy.regex import (
     SPACE_CHARS,
     UNICODE_DIGIT_CATEGORIES,
@@ -288,8 +288,8 @@ def test_groupref_not_shared_between_regex():
 
 
 @pytest.mark.skipif(
-    PYPY and sys.version_info[:2] == (3, 6),  # Skip for now so we can test the rest
-    reason=r"Under PyPy3.6, the pattern generates but does not match \x80\x80",
+    PYPY or PY2,  # Skip for now so we can test the rest
+    reason=r"Triggers bugs in poor handling of unicode in re for these implementations",
 )
 @given(st.data())
 def test_group_ref_is_not_shared_between_identical_regex(data):

hypothesis-python/tests/nocover/test_recursive.py

@@ -111,7 +111,7 @@ def flatten(x):
                     break
             return result
 
-    x = find_any(nested_sets, lambda x: len(flatten(x)) == 2, settings(deadline=None))
+    x = minimal(nested_sets, lambda x: len(flatten(x)) == 2, settings(deadline=None))
     assert x in (
         frozenset((False, True)),
         frozenset((False, frozenset((True,)))),