Skip to content

Commit

Permalink
narrow the severity and cases for alternate API use suggestion; close #…
Browse files Browse the repository at this point in the history
…3819

Revised per suggestion here: #3820 (comment)
  • Loading branch information
vreuter committed Dec 26, 2023
1 parent 4eb0b50 commit 7706606
Show file tree
Hide file tree
Showing 6 changed files with 128 additions and 17 deletions.
3 changes: 1 addition & 2 deletions hypothesis-python/RELEASE.rst
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
RELEASE_TYPE: patch

This patch causes a warning to be issued when :func:`~hypothesis.strategies.sampled_from` is given a nonempty collection of all strategy values as the argument to its `strategies` parameter (:issue:`3819``).
This patch causes a [note to be included](https://peps.python.org/pep-0678/) when :func:`~hypothesis.strategies.sampled_from` is given a nonempty collection of all strategy values _and_ the `given`-decorated test fails with a `TypeError` (:issue:`3819``).
This is because such a call is suggestive of intent to instead use :func:`~hypothesis.strategies.one_of`.
This closes
14 changes: 14 additions & 0 deletions hypothesis-python/src/hypothesis/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1585,6 +1585,20 @@ def wrapped_test(*arguments, **kwargs):
if isinstance(e, BaseExceptionGroup)
else get_trimmed_traceback()
)
if (
isinstance(e, TypeError)
and "SearchStrategy" in str(e)
and any(
getattr(
s, "sampling_is_from_a_collection_of_strategies", False
)
for s in given_kwargs.values()
)
):
add_note(
e,
"sample_from was given a collection of strategies; was one_of intended?",
)
raise the_error_hypothesis_found

if not (ran_explicit_examples or state.ever_executed):
Expand Down
5 changes: 0 additions & 5 deletions hypothesis-python/src/hypothesis/strategies/_internal/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,6 @@ def sampled_from(
"so maybe you tried to write an enum as if it was a dataclass?"
)
raise InvalidArgument("Cannot sample from a length-zero sequence.")
elif all(isinstance(x, SearchStrategy) for x in values):
warnings.warn(
"sample_from was given a collection of strategies; was one_of intended?",
stacklevel=1,
)
if len(values) == 1:
return just(values[0])
try:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,9 @@ def __init__(self, elements, repr_=None, transformations=()):
super().__init__()
self.elements = cu.check_sample(elements, "sampled_from")
assert self.elements
self.sampling_is_from_a_collection_of_strategies = all(
isinstance(x, SearchStrategy) for x in self.elements
)
self.repr_ = repr_
self._transformations = transformations

Expand Down
4 changes: 1 addition & 3 deletions hypothesis-python/tests/cover/test_lookup.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,10 +311,8 @@ class Baz(Foo):


@pytest.mark.parametrize(
"var,expected,exp_warn",
"var,expected",
[
# Expect a warning exactly when the type constraint/bound should trigger
# passing a strategy to sampled_from.
(typing.TypeVar("V"), object),
(typing.TypeVar("V", bound=int), int),
(typing.TypeVar("V", bound=Foo), (Bar, Baz)),
Expand Down
116 changes: 109 additions & 7 deletions hypothesis-python/tests/cover/test_sampled_from.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,113 @@ def test_suggests_elements_instead_of_annotations():
st.sampled_from(AnnotationsInsteadOfElements).example()


@pytest.mark.parametrize("wrap", [list, tuple])
def test_warns_when_given_entirely_strategies_as_elements(wrap):
elements = wrap([st.booleans(), st.decimals(), st.integers(), st.text()])
with pytest.warns(
UserWarning,
match="sample_from was given a collection of strategies; was one_of intended?",
class TestErrorNoteBehavior3819:
elements = (st.booleans(), st.decimals(), st.integers(), st.text())

def execute_expecting_error(test_func):
with pytest.raises(TypeError) as err_ctx:
test_func()
obs_notes = getattr(err_ctx.value, "__notes__", [])
assert obs_notes[-1] == exp_msg

@staticmethod
@given(st.sampled_from(elements))
def args_with_type_error_with_message_substring(_):
raise TypeError("SearchStrategy")

@staticmethod
@given(all_strat_sample=st.sampled_from(elements))
def kwargs_with_type_error_with_message_substring(all_strat_sample):
raise TypeError("SearchStrategy")

@staticmethod
@given(st.sampled_from(elements))
def args_with_type_error_without_message_substring(_):
raise TypeError("Substring not in message!")

@staticmethod
@given(st.sampled_from(elements))
def kwargs_with_type_error_without_message_substring(_):
raise TypeError("Substring not in message!")

@staticmethod
@given(st.sampled_from(elements))
def type_error_but_not_all_strategies_args(_):
raise TypeError("SearchStrategy, but should not trigger note addition!")

@staticmethod
@given(all_strat_sample=st.sampled_from(elements))
def type_error_but_not_all_strategies_kwargs(all_strat_sample):
raise TypeError("SearchStrategy, but should not trigger note addition!")

@staticmethod
@given(st.sampled_from(elements))
def non_type_error_args(_):
raise Exception("SearchStrategy, but should not trigger note addition!")

@staticmethod
@given(all_strat_sample=st.sampled_from(elements))
def non_type_error_kwargs(all_strat_sample):
raise Exception("SearchStrategy, but should not trigger note addition!")

@staticmethod
@given(st.sampled_from(elements))
def args_without_error(_):
return

@staticmethod
@given(all_strat_sample=st.sampled_from(elements))
def kwargs_without_error(all_strat_sample):
return

@pytest.mark.parametrize(
["func_to_call", "exp_err_cls", "should_exp_msg"],
[
(f, TypeError, True)
for f in (
args_with_type_error_with_message_substring,
kwargs_with_type_error_with_message_substring,
)
]
+ [
(f, TypeError, False)
for f in (
args_with_type_error_without_message_substring,
kwargs_with_type_error_without_message_substring,
)
]
+ [
(f, TypeError, False)
for f in (
type_error_but_not_all_strategies_args,
type_error_but_not_all_strategies_kwargs,
)
]
+ [(f, Exception, False) for f in (non_type_error_args, non_type_error_kwargs)]
+ [(f, None, False) for f in (args_without_error, kwargs_without_error)],
)
def test_error_appropriate_error_note_3819(
self, func_to_call, exp_err_cls, should_exp_msg
):
st.sampled_from(elements)
if exp_err_cls is None:
try:
func_to_call()
except BaseException as e:
pytest.fail(f"Expected call to succeed but got error: {e}")
else:
assert True
else:
with pytest.raises(Exception) as err_ctx:
func_to_call()
err = err_ctx.value
assert type(err) is exp_err_cls
notes = getattr(err, "__notes__", [])
msg_in_notes = (
"sample_from was given a collection of strategies; was one_of intended?"
in notes
)
assert (
should_exp_msg
and msg_in_notes
or (not should_exp_msg and not should_exp_msg)
)

0 comments on commit 7706606

Please sign in to comment.