Skip to content

Commit

Permalink
Deprecate randomness in strategies
Browse files Browse the repository at this point in the history
  • Loading branch information
Zac-HD committed Feb 4, 2024
1 parent e677b04 commit e769f67
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 12 deletions.
14 changes: 14 additions & 0 deletions hypothesis-python/RELEASE.rst
Original file line number Diff line number Diff line change
@@ -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`.
37 changes: 36 additions & 1 deletion hypothesis-python/src/hypothesis/control.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down
3 changes: 3 additions & 0 deletions hypothesis-python/src/hypothesis/internal/reflection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
19 changes: 9 additions & 10 deletions hypothesis-python/src/hypothesis/strategies/_internal/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)

Expand Down
22 changes: 21 additions & 1 deletion hypothesis-python/tests/cover/test_searchstrategy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down Expand Up @@ -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)

Expand Down

0 comments on commit e769f67

Please sign in to comment.