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

Add note to TypeError when each element to sampled_from is a strategy #3820

Merged
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
1 change: 1 addition & 0 deletions AUTHORS.rst
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ their individual contributions.
* `Tyler Gibbons <https://www.github.com/kavec>`_ (tyler.gibbons@flexport.com)
* `Tyler Nickerson <https://www.github.com/nmbrgts>`_
* `Vidya Rani <https://www.github.com/vidyarani-dg>`_ (vidyarani.d.g@gmail.com)
* `Vince Reuter <https://github.com/vreuter>`_ (vince.reuter@gmail.com)
* `Vincent Michel <https://www.github.com/vxgmichel>`_ (vxgmichel@gmail.com)
* `Viorel Pluta <https://github.com/viopl>`_ (viopluta@gmail.com)
* `Vytautas Strimaitis <https://www.github.com/vstrimaitis>`_
Expand Down
7 changes: 7 additions & 0 deletions hypothesis-python/RELEASE.rst
Original file line number Diff line number Diff line change
@@ -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!
14 changes: 12 additions & 2 deletions hypothesis-python/src/hypothesis/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
112 changes: 112 additions & 0 deletions hypothesis-python/tests/cover/test_sampled_from.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Loading