diff --git a/hypothesis-python/RELEASE.rst b/hypothesis-python/RELEASE.rst new file mode 100644 index 0000000000..ca2b2aec7d --- /dev/null +++ b/hypothesis-python/RELEASE.rst @@ -0,0 +1,14 @@ +RELEASE_TYPE: minor + +This release deprecates use of the global random number generator while drawing +from a strategy, because this makes test cases less diverse and prevents us +from reporting minimal counterexamples (:issue:`3810`). + +If you see this new warning, you can get a quick fix by using +:func:`~hypothesis.strategies.randoms`; or use more idiomatic strategies +:func:`~hypothesis.strategies.sampled_from`, :func:`~hypothesis.strategies.floats`, +:func:`~hypothesis.strategies.integers`, and so on. + +Note that the same problem applies to e.g. ``numpy.random``, but +for performance reasons we only check the stdlib :mod:`random` module - +ignoring even other sources passed to :func:`~hypothesis.register_random`. diff --git a/hypothesis-python/src/hypothesis/control.py b/hypothesis-python/src/hypothesis/control.py index f4a400072f..6a9523a463 100644 --- a/hypothesis-python/src/hypothesis/control.py +++ b/hypothesis-python/src/hypothesis/control.py @@ -10,7 +10,9 @@ import inspect import math +import random from collections import defaultdict +from contextlib import contextmanager from typing import Any, NoReturn, Union from weakref import WeakKeyDictionary @@ -91,6 +93,38 @@ def current_build_context() -> "BuildContext": return context +class RandomSeeder: + def __init__(self, seed): + self.seed = seed + + def __repr__(self): + return f"RandomSeeder({self.seed!r})" + + +class _Checker: + def __init__(self) -> None: + self.saw_global_random = False + + def __call__(self, x): + self.saw_global_random |= isinstance(x, RandomSeeder) + return x + + +@contextmanager +def deprecate_random_in_strategy(where, stacklevel=0): + _global_rand_state = random.getstate() + yield (checker := _Checker()) + if _global_rand_state != random.getstate() and not checker.saw_global_random: + # raise InvalidDefinition + note_deprecation( + "Do not use the `random` module inside strategies; instead " + "consider `st.randoms()`, `st.sampled_from()`, etc. " + where, + since="RELEASEDAY", + has_codemod=False, + stacklevel=stacklevel + 1, + ) + + class BuildContext: def __init__(self, data, *, is_final=False, close_on_capture=True): assert isinstance(data, ConjectureData) @@ -119,7 +153,8 @@ def prep_args_kwargs_from_strategies(self, kwarg_strategies): kwargs = {} for k, s in kwarg_strategies.items(): start_idx = self.data.index - obj = self.data.draw(s, observe_as=f"generate:{k}") + with deprecate_random_in_strategy(f"from {k}={s!r}", stacklevel=1) as check: + obj = check(self.data.draw(s, observe_as=f"generate:{k}")) end_idx = self.data.index kwargs[k] = obj diff --git a/hypothesis-python/src/hypothesis/internal/reflection.py b/hypothesis-python/src/hypothesis/internal/reflection.py index e62ab28f36..a829f097be 100644 --- a/hypothesis-python/src/hypothesis/internal/reflection.py +++ b/hypothesis-python/src/hypothesis/internal/reflection.py @@ -23,6 +23,7 @@ from functools import partial, wraps from io import StringIO from keyword import iskeyword +from random import _inst as global_random_instance from tokenize import COMMENT, detect_encoding, generate_tokens, untokenize from types import ModuleType from typing import TYPE_CHECKING, Any, Callable @@ -446,6 +447,8 @@ def get_pretty_function_description(f): # Some objects, like `builtins.abs` are of BuiltinMethodType but have # their module as __self__. This might include c-extensions generally? if not (self is None or inspect.isclass(self) or inspect.ismodule(self)): + if self is global_random_instance: + return f"random.{name}" return f"{self!r}.{name}" elif isinstance(name, str) and getattr(dict, name, object()) is f: # special case for keys/values views in from_type() / ghostwriter output diff --git a/hypothesis-python/src/hypothesis/strategies/_internal/core.py b/hypothesis-python/src/hypothesis/strategies/_internal/core.py index 8b5f1de4f1..b8d4aeb010 100644 --- a/hypothesis-python/src/hypothesis/strategies/_internal/core.py +++ b/hypothesis-python/src/hypothesis/strategies/_internal/core.py @@ -53,7 +53,13 @@ import attr from hypothesis._settings import note_deprecation -from hypothesis.control import cleanup, current_build_context, note +from hypothesis.control import ( + RandomSeeder, + cleanup, + current_build_context, + deprecate_random_in_strategy, + note, +) from hypothesis.errors import ( HypothesisSideeffectWarning, HypothesisWarning, @@ -998,14 +1004,6 @@ def randoms( ) -class RandomSeeder: - def __init__(self, seed): - self.seed = seed - - def __repr__(self): - return f"RandomSeeder({self.seed!r})" - - class RandomModule(SearchStrategy): def do_draw(self, data): # It would be unsafe to do run this method more than once per test case, @@ -2136,7 +2134,8 @@ def draw(self, strategy: SearchStrategy[Ex], label: Any = None) -> Ex: self.count += 1 printer = RepresentationPrinter(context=current_build_context()) desc = f"Draw {self.count}{'' if label is None else f' ({label})'}: " - result = self.conjecture_data.draw(strategy, observe_as=f"generate:{desc}") + with deprecate_random_in_strategy(f"{desc}from {strategy!r}", stacklevel=1): + result = self.conjecture_data.draw(strategy, observe_as=f"generate:{desc}") if TESTCASE_CALLBACKS: self.conjecture_data._observability_args[desc] = to_jsonable(result) diff --git a/hypothesis-python/tests/cover/test_searchstrategy.py b/hypothesis-python/tests/cover/test_searchstrategy.py index 56cdd736a9..28b617e797 100644 --- a/hypothesis-python/tests/cover/test_searchstrategy.py +++ b/hypothesis-python/tests/cover/test_searchstrategy.py @@ -10,18 +10,21 @@ import dataclasses import functools +import random from collections import defaultdict, namedtuple import attr import pytest +from hypothesis import given from hypothesis.errors import InvalidArgument, Unsatisfiable from hypothesis.internal.conjecture.data import ConjectureData from hypothesis.internal.reflection import get_pretty_function_description -from hypothesis.strategies import booleans, integers, just, none, tuples +from hypothesis.strategies import booleans, data, integers, just, lists, none, tuples from hypothesis.strategies._internal.utils import to_jsonable from tests.common.debug import assert_simple_property, check_can_generate_examples +from tests.common.utils import checks_deprecated_behaviour def test_or_errors_when_given_non_strategy(): @@ -92,6 +95,23 @@ def test_flatmap_with_invalid_expand(): check_can_generate_examples(just(100).flatmap(lambda n: "a")) +_bad_random_strategy = lists(integers(), min_size=1).map(random.choice) + + +@checks_deprecated_behaviour +def test_use_of_global_random_is_deprecated_in_given(): + check_can_generate_examples(_bad_random_strategy) + + +@checks_deprecated_behaviour +def test_use_of_global_random_is_deprecated_in_interactive_draws(): + @given(data()) + def inner(d): + d.draw(_bad_random_strategy) + + inner() + + def test_jsonable(): assert isinstance(to_jsonable(object()), str)