Merge pull request #4266 from tybug/stateful-pprint

Improve pprinting of stateful examples

Author: tybug

hypothesis-python/RELEASE.rst

@@ -0,0 +1,19 @@
+RELEASE_TYPE: patch
+
+Improve the clarity of printing counterexamples in :doc:`stateful testing <stateful>`, by avoiding confusing :class:`~hypothesis.stateful.Bundle` references with equivalent values drawn from a regular strategy.
+
+For example, we now print:
+
+.. code-block: python
+
+  a_0 = state.add_to_bundle(a=0)
+  state.unrelated(value=0)
+
+instead of
+
+.. code-block: python
+
+  a_0 = state.add_to_bundle(a=0)
+  state.unrelated(value=a_0)
+
+if the ``unrelated`` rule draws from a regular strategy such as :func:`~hypothesis.strategies.integers` instead of the ``a`` bundle.

hypothesis-python/src/hypothesis/stateful.py

@@ -65,6 +65,20 @@
 SHOULD_CONTINUE_LABEL = cu.calc_label_from_name("should we continue drawing")
 
 
+def _is_singleton(obj: object) -> bool:
+    """
+    Returns True if two separately created instances of v will have the same id
+    (due to interning).
+    """
+    # The range [-5, 256] is a cpython implementation detail. This may not work
+    # well on other platforms.
+    if isinstance(obj, int) and -5 <= obj <= 256:
+        return True
+    # cpython also interns compile-time strings, but let's just ignore those for
+    # now.
+    return isinstance(obj, bool) or obj is None
+
+
 class _OmittedArgument:
     """Sentinel class to prevent overlapping overloads in type hints. See comments
     above the overloads of @rule."""
@@ -395,7 +409,10 @@ def _add_result_to_targets(self, targets, result):
             def printer(obj, p, cycle, name=name):
                 return p.text(name)
 
-            self.__printer.singleton_pprinters.setdefault(id(result), printer)
+            # see
+            # https://github.com/HypothesisWorks/hypothesis/pull/4266#discussion_r1949619102
+            if not _is_singleton(result):
+                self.__printer.singleton_pprinters.setdefault(id(result), printer)
             self.names_to_values[name] = result
             self.bundles.setdefault(target, []).append(VarReference(name))
 

hypothesis-python/tests/nocover/test_stateful.py

@@ -8,11 +8,12 @@
 # v. 2.0. If a copy of the MPL was not distributed with this file, You can
 # obtain one at https://mozilla.org/MPL/2.0/.
 
+import inspect
 from collections import namedtuple
 
 import pytest
 
-from hypothesis import settings as Settings
+from hypothesis import Phase, settings as Settings, strategies as st
 from hypothesis.stateful import (
     Bundle,
     RuleBasedStateMachine,
@@ -21,7 +22,25 @@
     rule,
     run_state_machine_as_test,
 )
-from hypothesis.strategies import booleans, integers, lists
+
+
+def run_to_notes(TestClass):
+    TestCase = TestClass.TestCase
+    # don't add explain phase notes to the error
+    TestCase.settings = Settings(phases=set(Phase) - {Phase.explain})
+    try:
+        TestCase().runTest()
+    except AssertionError as err:
+        return err.__notes__
+
+    raise RuntimeError("Expected an assertion error")
+
+
+def assert_runs_to_output(TestClass, output):
+    # remove the first line, which is always "Falsfying example:"
+    actual = "\n".join(run_to_notes(TestClass)[1:])
+    assert actual == inspect.cleandoc(output.strip())
+
 
 Leaf = namedtuple("Leaf", ("label",))
 Split = namedtuple("Split", ("left", "right"))
@@ -30,7 +49,7 @@
 class BalancedTrees(RuleBasedStateMachine):
     trees = Bundle("BinaryTree")
 
-    @rule(target=trees, x=booleans())
+    @rule(target=trees, x=st.booleans())
     def leaf(self, x):
         return Leaf(x)
 
@@ -81,7 +100,7 @@ def is_not_too_deep(self, check):
 class RoseTreeStateMachine(RuleBasedStateMachine):
     nodes = Bundle("nodes")
 
-    @rule(target=nodes, source=lists(nodes))
+    @rule(target=nodes, source=st.lists(nodes))
     def bunch(self, source):
         return source
 
@@ -149,7 +168,7 @@ def __init__(self):
     # achieve "swarming" by by just restricting the alphabet for single byte
     # decisions, which is a thing the underlying conjecture engine  will
     # happily do on its own without knowledge of the rule structure.
-    @rule(move=integers(0, 255))
+    @rule(move=st.integers(0, 255))
     def ladder(self, move):
         self.seen.add(move)
         assert len(self.seen) <= 15
@@ -213,29 +232,29 @@ class TestMyStatefulMachine(MyStatefulMachine.TestCase):
 def test_multiple_precondition_bug():
     # See https://github.com/HypothesisWorks/hypothesis/issues/2861
     class MultiplePreconditionMachine(RuleBasedStateMachine):
-        @rule(x=integers())
+        @rule(x=st.integers())
         def good_method(self, x):
             pass
 
         @precondition(lambda self: True)
         @precondition(lambda self: False)
-        @rule(x=integers())
+        @rule(x=st.integers())
         def bad_method_a(self, x):
             raise AssertionError("This rule runs, even though it shouldn't.")
 
         @precondition(lambda self: False)
         @precondition(lambda self: True)
-        @rule(x=integers())
+        @rule(x=st.integers())
         def bad_method_b(self, x):
             raise AssertionError("This rule might be skipped for the wrong reason.")
 
         @precondition(lambda self: True)
-        @rule(x=integers())
+        @rule(x=st.integers())
         @precondition(lambda self: False)
         def bad_method_c(self, x):
             raise AssertionError("This rule runs, even though it shouldn't.")
 
-        @rule(x=integers())
+        @rule(x=st.integers())
         @precondition(lambda self: True)
         @precondition(lambda self: False)
         def bad_method_d(self, x):
@@ -266,3 +285,98 @@ def bad_invariant_d(self):
             raise AssertionError("This invariant runs, even though it shouldn't.")
 
     run_state_machine_as_test(MultiplePreconditionMachine)
+
+
+class UnrelatedCall(RuleBasedStateMachine):
+    a = Bundle("a")
+
+    def __init__(self):
+        super().__init__()
+        self.calls = set()
+
+    @rule(target=a, a=st.integers())
+    def add_a(self, a):
+        self.calls.add("add")
+        return a
+
+    @rule(v=a)
+    def f(self, v):
+        self.calls.add("f")
+
+    @precondition(lambda self: "add" in self.calls)
+    @rule(value=st.integers())
+    def unrelated(self, value):
+        self.calls.add("unrelated")
+
+    @rule()
+    def invariant(self):
+        # force all three calls to be made in a particular order (with the
+        # `unrelated` precondition) so we always shrink to a particular counterexample.
+        assert len(self.calls) != 3
+
+
+def test_unrelated_rule_does_not_use_var_reference_repr():
+    # we are specifically looking for state.unrelated(value=0) not being replaced
+    # with state.unrelated(value=a_0). The `unrelated` rule is drawing from
+    # st.integers, not a bundle, so the values should not be conflated even if
+    # they're both 0.
+    assert_runs_to_output(
+        UnrelatedCall,
+        """
+        state = UnrelatedCall()
+        a_0 = state.add_a(a=0)
+        state.f(v=a_0)
+        state.unrelated(value=0)
+        state.invariant()
+        state.teardown()
+        """,
+    )
+
+
+class SourceSameAsTarget(RuleBasedStateMachine):
+    values = Bundle("values")
+
+    @rule(target=values, value=st.lists(values))
+    def f(self, value):
+        assert len(value) == 0
+        return value
+
+
+class SourceSameAsTargetUnclearOrigin(RuleBasedStateMachine):
+    values = Bundle("values")
+
+    def __init__(self):
+        super().__init__()
+        self.called = False
+
+    @rule(target=values, value=st.just([]) | st.lists(values))
+    def f(self, value):
+        assert not self.called
+        # ensure we get two calls to f before failing. In the minimal failing
+        # example, both will be from st.just([]).
+        self.called = True
+        return value
+
+
+def test_replaces_when_same_id():
+    assert_runs_to_output(
+        SourceSameAsTarget,
+        f"""
+        state = {SourceSameAsTarget.__name__}()
+        values_0 = state.f(value=[])
+        state.f(value=[values_0])
+        state.teardown()
+        """,
+    )
+
+
+def test_doesnt_replace_when_different_id():
+    assert_runs_to_output(
+        SourceSameAsTargetUnclearOrigin,
+        f"""
+        state = {SourceSameAsTargetUnclearOrigin.__name__}()
+        values_0 = state.f(value=[])
+        state.f(value=[])
+        state.teardown()
+        """,
+    )