Merge pull request #3820 from vreuter/vr/warn-sampling-stategies-issu…

…e3819

Add note to TypeError when each element to `sampled_from` is a strategy

AUTHORS.rst

@@ -181,6 +181,7 @@ their individual contributions.
 * `Tyler Gibbons <https://www.github.com/kavec>`_ ([email protected])
 * `Tyler Nickerson <https://www.github.com/nmbrgts>`_
 * `Vidya Rani <https://www.github.com/vidyarani-dg>`_ ([email protected])
+* `Vince Reuter <https://github.com/vreuter>`_ ([email protected])
 * `Vincent Michel <https://www.github.com/vxgmichel>`_ ([email protected])
 * `Viorel Pluta <https://github.com/viopl>`_ ([email protected])
 * `Vytautas Strimaitis <https://www.github.com/vstrimaitis>`_

hypothesis-python/RELEASE.rst

@@ -0,0 +1,7 @@
+RELEASE_TYPE: patch
+
+If a test uses :func:`~hypothesis.strategies.sampled_from` on a sequence of
+strategies, and raises a ``TypeError``, we now :pep:`add a note <678>` asking
+whether you meant to use :func:`~hypothesis.strategies.one_of`.
+
+Thanks to Vince Reuter for suggesting and implementing this hint!

hypothesis-python/src/hypothesis/core.py

@@ -917,7 +917,18 @@ def run(data):
                     **dict(enumerate(map(to_jsonable, args))),
                     **{k: to_jsonable(v) for k, v in kwargs.items()},
                 }
-            return test(*args, **kwargs)
+
+            try:
+                return test(*args, **kwargs)
+            except TypeError as e:
+                # If we sampled from a sequence of strategies, AND failed with a
+                # TypeError, *AND that exception mentions SearchStrategy*, add a note:
+                if "SearchStrategy" in str(e):
+                    try:
+                        add_note(e, data._sampled_from_all_strategies_elements_message)
+                    except AttributeError:
+                        pass
+                raise
 
         # self.test_runner can include the execute_example method, or setup/teardown
         # _example, so it's important to get the PRNG and build context in place first.
@@ -1024,7 +1035,6 @@ def _execute_once_for_engine(self, data: ConjectureData) -> None:
                 # The test failed by raising an exception, so we inform the
                 # engine that this test run was interesting. This is the normal
                 # path for test runs that fail.
-
                 tb = get_trimmed_traceback()
                 info = data.extra_information
                 info._expected_traceback = format_exception(e, tb)  # type: ignore

hypothesis-python/src/hypothesis/strategies/_internal/strategies.py

@@ -528,6 +528,22 @@ def _transform(self, element):
 
     def do_draw(self, data):
         result = self.do_filtered_draw(data)
+        if isinstance(result, SearchStrategy) and all(
+            isinstance(x, SearchStrategy) for x in self.elements
+        ):
+            max_num_strats = 3
+            preamble = (
+                f"(first {max_num_strats}) "
+                if len(self.elements) > max_num_strats
+                else ""
+            )
+            strat_reprs = ", ".join(
+                map(get_pretty_function_description, self.elements[:max_num_strats])
+            )
+            data._sampled_from_all_strategies_elements_message = (
+                "sample_from was given a collection of strategies: "
+                f"{preamble}{strat_reprs}. Was one_of intended?"
+            )
         if result is filter_not_satisfied:
             data.mark_invalid(f"Aborted test because unable to satisfy {self!r}")
         return result

hypothesis-python/tests/cover/test_sampled_from.py

@@ -190,3 +190,115 @@ class AnnotationsInsteadOfElements(enum.Enum):
 def test_suggests_elements_instead_of_annotations():
     with pytest.raises(InvalidArgument, match="Cannot sample.*annotations.*dataclass"):
         st.sampled_from(AnnotationsInsteadOfElements).example()
+
+
+class TestErrorNoteBehavior3819:
+    elements = (st.booleans(), st.decimals(), st.integers(), st.text())
+
+    @staticmethod
+    @given(st.data())
+    def direct_without_error(data):
+        data.draw(st.sampled_from((st.floats(), st.binary())))
+
+    @staticmethod
+    @given(st.data())
+    def direct_with_non_type_error(data):
+        data.draw(st.sampled_from(st.characters(), st.floats()))
+        raise Exception("Contains SearchStrategy, but no note addition!")
+
+    @staticmethod
+    @given(st.data())
+    def direct_with_type_error_without_substring(data):
+        data.draw(st.sampled_from(st.booleans(), st.binary()))
+        raise TypeError("Substring not in message!")
+
+    @staticmethod
+    @given(st.data())
+    def direct_with_type_error_with_substring_but_not_all_strategies(data):
+        data.draw(st.sampled_from(st.booleans(), False, True))
+        raise TypeError("Contains SearchStrategy, but no note addition!")
+
+    @staticmethod
+    @given(st.data())
+    def direct_all_strategies_with_type_error_with_substring(data):
+        data.draw(st.sampled_from((st.dates(), st.datetimes())))
+        raise TypeError("This message contains SearchStrategy as substring!")
+
+    @staticmethod
+    @given(st.lists(st.sampled_from(elements)))
+    def indirect_without_error(_):
+        return
+
+    @staticmethod
+    @given(st.lists(st.sampled_from(elements)))
+    def indirect_with_non_type_error(_):
+        raise Exception("Contains SearchStrategy, but no note addition!")
+
+    @staticmethod
+    @given(st.lists(st.sampled_from(elements)))
+    def indirect_with_type_error_without_substring(_):
+        raise TypeError("Substring not in message!")
+
+    @staticmethod
+    @given(st.lists(st.sampled_from((*elements, False, True))))
+    def indirect_with_type_error_with_substring_but_not_all_strategies(_):
+        raise TypeError("Contains SearchStrategy, but no note addition!")
+
+    @staticmethod
+    @given(st.lists(st.sampled_from(elements), min_size=1))
+    def indirect_all_strategies_with_type_error_with_substring(objs):
+        raise TypeError("Contains SearchStrategy in message, trigger note!")
+
+    @pytest.mark.parametrize(
+        ["func_to_call", "exp_err_cls", "should_exp_msg"],
+        [
+            pytest.param(f.__func__, err, msg_exp, id=f.__func__.__name__)
+            for f, err, msg_exp in [
+                (f, TypeError, True)
+                for f in (
+                    direct_all_strategies_with_type_error_with_substring,
+                    indirect_all_strategies_with_type_error_with_substring,
+                )
+            ]
+            + [
+                (f, TypeError, False)
+                for f in (
+                    direct_with_type_error_without_substring,
+                    direct_with_type_error_with_substring_but_not_all_strategies,
+                    indirect_with_type_error_without_substring,
+                    indirect_with_type_error_with_substring_but_not_all_strategies,
+                )
+            ]
+            + [
+                (f, Exception, False)
+                for f in (
+                    direct_with_non_type_error,
+                    indirect_with_non_type_error,
+                )
+            ]
+            + [
+                (f, None, False)
+                for f in (
+                    direct_without_error,
+                    indirect_without_error,
+                )
+            ]
+        ],
+    )
+    def test_error_appropriate_error_note_3819(
+        self, func_to_call, exp_err_cls, should_exp_msg
+    ):
+        if exp_err_cls is None:
+            # Here we only care that no exception was raised, so nothing to assert.
+            func_to_call()
+        else:
+            with pytest.raises(exp_err_cls) as err_ctx:
+                func_to_call()
+            notes = getattr(err_ctx.value, "__notes__", [])
+            matching_messages = [
+                n
+                for n in notes
+                if n.startswith("sample_from was given a collection of strategies")
+                and n.endswith("Was one_of intended?")
+            ]
+            assert len(matching_messages) == (1 if should_exp_msg else 0)