diff --git a/hypothesis-python/.coveragerc b/hypothesis-python/.coveragerc index 7732c620a2..89be7942bc 100644 --- a/hypothesis-python/.coveragerc +++ b/hypothesis-python/.coveragerc @@ -3,6 +3,7 @@ branch = True omit = **/_hypothesis_ftz_detector.py **/_hypothesis_pytestplugin.py + **/_hypothesis_globals.py **/extra/array_api.py **/extra/cli.py **/extra/django/*.py diff --git a/hypothesis-python/RELEASE.rst b/hypothesis-python/RELEASE.rst new file mode 100644 index 0000000000..40c0548562 --- /dev/null +++ b/hypothesis-python/RELEASE.rst @@ -0,0 +1,6 @@ +RELEASE_TYPE: minor + +This release avoids creating a ``.hypothesis`` directory when using +:func:`~hypothesis.strategies.register_type_strategy` (:issue:`3836`), +and adds warnings for plugins which do so by other means or have +other unintended side-effects. diff --git a/hypothesis-python/setup.py b/hypothesis-python/setup.py index a668b76531..40b2e31ed8 100644 --- a/hypothesis-python/setup.py +++ b/hypothesis-python/setup.py @@ -124,7 +124,11 @@ def local_file(name): "Topic :: Software Development :: Testing", "Typing :: Typed", ], - py_modules=["_hypothesis_pytestplugin", "_hypothesis_ftz_detector"], + py_modules=[ + "_hypothesis_pytestplugin", + "_hypothesis_ftz_detector", + "_hypothesis_globals", + ], entry_points={ "pytest11": ["hypothesispytest = _hypothesis_pytestplugin"], "console_scripts": ["hypothesis = hypothesis.extra.cli:main"], diff --git a/hypothesis-python/src/_hypothesis_globals.py b/hypothesis-python/src/_hypothesis_globals.py new file mode 100644 index 0000000000..e97e091879 --- /dev/null +++ b/hypothesis-python/src/_hypothesis_globals.py @@ -0,0 +1,28 @@ +# This file is part of Hypothesis, which may be found at +# https://github.com/HypothesisWorks/hypothesis/ +# +# Copyright the Hypothesis Authors. +# Individual contributors are listed in AUTHORS.rst and the git log. +# +# This Source Code Form is subject to the terms of the Mozilla Public License, +# v. 2.0. If a copy of the MPL was not distributed with this file, You can +# obtain one at https://mozilla.org/MPL/2.0/. + +""" +Module for globals shared between plugin(s) and the main hypothesis module, without +depending on either. This file should have no imports outside of stdlib. +""" + +import os + +in_initialization = 1 +"""If nonzero, indicates that hypothesis is still initializing (importing or loading +the test environment). `import hypothesis` will cause this number to be decremented, +and the pytest plugin increments at load time, then decrements it just before the test +session starts. However, this leads to a hole in coverage if another pytest plugin +imports hypothesis before our plugin is loaded. HYPOTHESIS_EXTEND_INITIALIZATION may +be set to pre-increment the value on behalf of _hypothesis_pytestplugin, plugging the +hole.""" + +if os.environ.get("HYPOTHESIS_EXTEND_INITIALIZATION"): + in_initialization += 1 diff --git a/hypothesis-python/src/_hypothesis_pytestplugin.py b/hypothesis-python/src/_hypothesis_pytestplugin.py index 9875e067f5..944304ccdb 100644 --- a/hypothesis-python/src/_hypothesis_pytestplugin.py +++ b/hypothesis-python/src/_hypothesis_pytestplugin.py @@ -21,9 +21,12 @@ import base64 import json +import os import sys +import warnings from inspect import signature +import _hypothesis_globals import pytest try: @@ -94,6 +97,19 @@ def __call__(self, msg): warnings.warn(PYTEST_TOO_OLD_MESSAGE % (pytest.__version__,), stacklevel=1) else: + # Restart side-effect detection as early as possible, to maximize coverage. We + # need balanced increment/decrement in configure/sessionstart to support nested + # pytest (e.g. runpytest_inprocess), so this early increment in effect replaces + # the first one in pytest_configure. + _configured = False + if not os.environ.get("HYPOTHESIS_EXTEND_INITIALIZATION"): + _hypothesis_globals.in_initialization += 1 + if "hypothesis" in sys.modules: + # Some other plugin has imported hypothesis, so we'll check if there + # have been undetected side-effects and warn if so. + from hypothesis.configuration import notice_initialization_restarted + + notice_initialization_restarted() def pytest_addoption(parser): group = parser.getgroup("hypothesis", "Hypothesis") @@ -147,6 +163,12 @@ def pytest_report_header(config): return f"hypothesis profile {settings._current_profile!r}{settings_str}" def pytest_configure(config): + global _configured + # skip first increment because we pre-incremented at import time + if _configured: + _hypothesis_globals.in_initialization += 1 + _configured = True + config.addinivalue_line("markers", "hypothesis: Tests which use hypothesis.") if not _any_hypothesis_option(config): return @@ -407,6 +429,9 @@ def pytest_collection_modifyitems(items): if isinstance(item, pytest.Function) and is_hypothesis_test(item.obj): item.add_marker("hypothesis") + def pytest_sessionstart(session): + _hypothesis_globals.in_initialization -= 1 + # Monkeypatch some internals to prevent applying @pytest.fixture() to a # function which has already been decorated with @hypothesis.given(). # (the reverse case is already an explicit error in Hypothesis) diff --git a/hypothesis-python/src/hypothesis/__init__.py b/hypothesis-python/src/hypothesis/__init__.py index db140b8165..cfb55119f7 100644 --- a/hypothesis-python/src/hypothesis/__init__.py +++ b/hypothesis-python/src/hypothesis/__init__.py @@ -15,6 +15,8 @@ failing examples it finds. """ +import _hypothesis_globals + from hypothesis._settings import HealthCheck, Phase, Verbosity, settings from hypothesis.control import ( assume, @@ -54,3 +56,6 @@ run() del run + +_hypothesis_globals.in_initialization -= 1 +del _hypothesis_globals diff --git a/hypothesis-python/src/hypothesis/configuration.py b/hypothesis-python/src/hypothesis/configuration.py index 6e6ab29516..2586c729f7 100644 --- a/hypothesis-python/src/hypothesis/configuration.py +++ b/hypothesis-python/src/hypothesis/configuration.py @@ -9,8 +9,13 @@ # obtain one at https://mozilla.org/MPL/2.0/. import os +import warnings from pathlib import Path +import _hypothesis_globals + +from hypothesis.errors import HypothesisSideeffectWarning + __hypothesis_home_directory_default = Path.cwd() / ".hypothesis" __hypothesis_home_directory = None @@ -21,7 +26,12 @@ def set_hypothesis_home_dir(directory): __hypothesis_home_directory = None if directory is None else Path(directory) -def storage_directory(*names): +def storage_directory(*names, intent_to_write=True): + if intent_to_write: + check_sideeffect_during_initialization( + "accessing storage for {}", "/".join(names) + ) + global __hypothesis_home_directory if not __hypothesis_home_directory: if where := os.getenv("HYPOTHESIS_STORAGE_DIRECTORY"): @@ -29,3 +39,53 @@ def storage_directory(*names): if not __hypothesis_home_directory: __hypothesis_home_directory = __hypothesis_home_directory_default return __hypothesis_home_directory.joinpath(*names) + + +_first_postinit_what = None + + +def check_sideeffect_during_initialization( + what: str, *fmt_args: object, extra: str = "" +) -> None: + """Called from locations that should not be executed during initialization, for example + touching disk or materializing lazy/deferred strategies from plugins. If initialization + is in progress, a warning is emitted. + + Note that computing the repr can take nontrivial time or memory, so we avoid doing so + unless (and until) we're actually emitting the warning. + """ + global _first_postinit_what + # This is not a particularly hot path, but neither is it doing productive work, so we want to + # minimize the cost by returning immediately. The drawback is that we require + # notice_initialization_restarted() to be called if in_initialization changes away from zero. + if _first_postinit_what is not None: + return + elif _hypothesis_globals.in_initialization: + # Note: -Werror is insufficient under pytest, as doesn't take effect until + # test session start. + msg = what.format(*fmt_args) + warnings.warn( + f"Slow code in plugin: avoid {msg} at import time! Set PYTHONWARNINGS=error " + "to get a traceback and show which plugin is responsible." + extra, + HypothesisSideeffectWarning, + stacklevel=3, + ) + else: + _first_postinit_what = (what, fmt_args) + + +def notice_initialization_restarted(*, warn: bool = True) -> None: + """Reset _first_postinit_what, so that we don't think we're in post-init. Additionally, if it + was set that means that there has been a sideeffect that we haven't warned about, so do that + now (the warning text will be correct, and we also hint that the stacktrace can be improved). + """ + global _first_postinit_what + if _first_postinit_what is not None: + what, *fmt_args = _first_postinit_what + _first_postinit_what = None + if warn: + check_sideeffect_during_initialization( + what, + *fmt_args, + extra=" Additionally, set HYPOTHESIS_EXTEND_INITIALIZATION=1 to pinpoint the exact location.", + ) diff --git a/hypothesis-python/src/hypothesis/database.py b/hypothesis-python/src/hypothesis/database.py index 4abce10c01..9fbdd31597 100644 --- a/hypothesis-python/src/hypothesis/database.py +++ b/hypothesis-python/src/hypothesis/database.py @@ -59,7 +59,7 @@ def _db_for_path(path=None): "https://hypothesis.readthedocs.io/en/latest/settings.html#settings-profiles" ) - path = storage_directory("examples") + path = storage_directory("examples", intent_to_write=False) if not _usable_dir(path): # pragma: no cover warnings.warn( "The database setting is not configured, and the default " @@ -495,6 +495,8 @@ def _prepare_for_io(self) -> None: self._initialized = True def _initialize_db(self) -> None: + # Trigger warning that we suppressed earlier by intent_to_write=False + storage_directory(self.path.name) # Create the cache directory if it doesn't exist self.path.mkdir(exist_ok=True, parents=True) diff --git a/hypothesis-python/src/hypothesis/errors.py b/hypothesis-python/src/hypothesis/errors.py index 8387a87586..0d376a7493 100644 --- a/hypothesis-python/src/hypothesis/errors.py +++ b/hypothesis-python/src/hypothesis/errors.py @@ -117,6 +117,13 @@ class HypothesisDeprecationWarning(HypothesisWarning, FutureWarning): """ +class HypothesisSideeffectWarning(HypothesisWarning): + """A warning issued by Hypothesis when it sees actions that are + discouraged at import or initialization time because they are + slow or have user-visible side effects. + """ + + class Frozen(HypothesisException): """Raised when a mutation method has been called on a ConjectureData object after freeze() has been called.""" diff --git a/hypothesis-python/src/hypothesis/strategies/_internal/core.py b/hypothesis-python/src/hypothesis/strategies/_internal/core.py index 804901268b..028d8405c7 100644 --- a/hypothesis-python/src/hypothesis/strategies/_internal/core.py +++ b/hypothesis-python/src/hypothesis/strategies/_internal/core.py @@ -55,6 +55,7 @@ from hypothesis._settings import note_deprecation from hypothesis.control import cleanup, current_build_context, note from hypothesis.errors import ( + HypothesisSideeffectWarning, HypothesisWarning, InvalidArgument, ResolutionFailed, @@ -2196,14 +2197,25 @@ def register_type_strategy( f"{custom_type=} is not allowed to be registered, " f"because there is no such thing as a runtime instance of {custom_type!r}" ) - elif not (isinstance(strategy, SearchStrategy) or callable(strategy)): + if not (isinstance(strategy, SearchStrategy) or callable(strategy)): raise InvalidArgument( f"{strategy=} must be a SearchStrategy, or a function that takes " "a generic type and returns a specific SearchStrategy" ) - elif isinstance(strategy, SearchStrategy) and strategy.is_empty: - raise InvalidArgument(f"{strategy=} must not be empty") - elif types.has_type_arguments(custom_type): + if isinstance(strategy, SearchStrategy): + with warnings.catch_warnings(): + warnings.simplefilter("error", HypothesisSideeffectWarning) + + # Calling is_empty forces materialization of lazy strategies. If this is done at import + # time, lazy strategies will warn about it; here, we force that warning to raise to + # avoid the materialization. Ideally, we'd just check if the strategy is lazy, but the + # lazy strategy may be wrapped underneath another strategy so that's complicated. + try: + if strategy.is_empty: + raise InvalidArgument(f"{strategy=} must not be empty") + except HypothesisSideeffectWarning: # pragma: no cover + pass + if types.has_type_arguments(custom_type): raise InvalidArgument( f"Cannot register generic type {custom_type!r}, because it has type " "arguments which would not be handled. Instead, register a function " diff --git a/hypothesis-python/src/hypothesis/strategies/_internal/deferred.py b/hypothesis-python/src/hypothesis/strategies/_internal/deferred.py index 489b4d7b7a..f7dae9a1e5 100644 --- a/hypothesis-python/src/hypothesis/strategies/_internal/deferred.py +++ b/hypothesis-python/src/hypothesis/strategies/_internal/deferred.py @@ -10,6 +10,7 @@ import inspect +from hypothesis.configuration import check_sideeffect_during_initialization from hypothesis.errors import InvalidArgument from hypothesis.internal.reflection import get_pretty_function_description from hypothesis.strategies._internal.strategies import SearchStrategy, check_strategy @@ -27,6 +28,8 @@ def __init__(self, definition): @property def wrapped_strategy(self): if self.__wrapped_strategy is None: + check_sideeffect_during_initialization("deferred evaluation of {!r}", self) + if not inspect.isfunction(self.__definition): raise InvalidArgument( f"Expected definition to be a function but got {self.__definition!r} " diff --git a/hypothesis-python/src/hypothesis/strategies/_internal/lazy.py b/hypothesis-python/src/hypothesis/strategies/_internal/lazy.py index 5e493a9099..d6bb13c7c1 100644 --- a/hypothesis-python/src/hypothesis/strategies/_internal/lazy.py +++ b/hypothesis-python/src/hypothesis/strategies/_internal/lazy.py @@ -12,6 +12,7 @@ from typing import MutableMapping from weakref import WeakKeyDictionary +from hypothesis.configuration import check_sideeffect_during_initialization from hypothesis.internal.reflection import ( convert_keyword_arguments, convert_positional_arguments, @@ -100,6 +101,8 @@ def calc_is_cacheable(self, recur): @property def wrapped_strategy(self): if self.__wrapped_strategy is None: + check_sideeffect_during_initialization("lazy evaluation of {!r}", self) + unwrapped_args = tuple(unwrap_strategies(s) for s in self.__args) unwrapped_kwargs = { k: unwrap_strategies(v) for k, v in self.__kwargs.items() diff --git a/hypothesis-python/tests/cover/test_sideeffect_warnings.py b/hypothesis-python/tests/cover/test_sideeffect_warnings.py new file mode 100644 index 0000000000..dd5fdc31b0 --- /dev/null +++ b/hypothesis-python/tests/cover/test_sideeffect_warnings.py @@ -0,0 +1,59 @@ +# This file is part of Hypothesis, which may be found at +# https://github.com/HypothesisWorks/hypothesis/ +# +# Copyright the Hypothesis Authors. +# Individual contributors are listed in AUTHORS.rst and the git log. +# +# This Source Code Form is subject to the terms of the Mozilla Public License, +# v. 2.0. If a copy of the MPL was not distributed with this file, You can +# obtain one at https://mozilla.org/MPL/2.0/. + +import _hypothesis_globals +import pytest + +from hypothesis import configuration as fs, strategies as st +from hypothesis.errors import HypothesisSideeffectWarning + +IN_INITIALIZATION_ATTR = "in_initialization" + +# These tests use the pytest plugin enabling infrastructure to restart the side-effect warnings, +# rather than trying to induce side-effects during import (and entrypoint loading) itself, which is +# hard to do. Manual verification of behaviour during initial import can be done by just injecting +# one of the side-effect-inducing statements below directly into hypothesis.entry_points.run(). +# Manual validation can also be done by inspecting the relevant state during import and verify that +# it is the same as tested here +# (_hypothesis_globals.in_initialization > 0, hypothesis.configuration._first_postinit_what is None) + + +@pytest.fixture +def _extend_initialization(monkeypatch): + assert getattr(_hypothesis_globals, IN_INITIALIZATION_ATTR) == 0 + monkeypatch.setattr(_hypothesis_globals, IN_INITIALIZATION_ATTR, 1) + fs.notice_initialization_restarted(warn=False) + assert fs._first_postinit_what is None # validates state as given in comment above + + +@pytest.mark.parametrize( + "sideeffect, warning_text", + [ + # the inner lambda ensures the lazy strategy can't be cached + (lambda: st.builds(lambda: None).wrapped_strategy, "lazy evaluation"), + (lambda: st.deferred(st.none).wrapped_strategy, "deferred evaluation"), + (fs.storage_directory, "accessing storage"), + ], +) +def test_sideeffect_warning(sideeffect, warning_text, _extend_initialization): + with pytest.warns(HypothesisSideeffectWarning, match=warning_text): + sideeffect() + + +def test_sideeffect_delayed_warning(monkeypatch, _extend_initialization): + what = "synthetic side-effect" + # _extend_initialization ensures we start at known clean slate (no delayed warnings). + # Then: stop initialization, check a side-effect operation, and restart it. + monkeypatch.setattr(_hypothesis_globals, IN_INITIALIZATION_ATTR, 0) + fs.check_sideeffect_during_initialization(what) + fs.check_sideeffect_during_initialization("ignored since not first") + with pytest.warns(HypothesisSideeffectWarning, match=what): + monkeypatch.setattr(_hypothesis_globals, IN_INITIALIZATION_ATTR, 1) + fs.notice_initialization_restarted() diff --git a/hypothesis-python/tests/pytest/test_sideeffect_warnings.py b/hypothesis-python/tests/pytest/test_sideeffect_warnings.py new file mode 100644 index 0000000000..20a2c9c97a --- /dev/null +++ b/hypothesis-python/tests/pytest/test_sideeffect_warnings.py @@ -0,0 +1,65 @@ +# This file is part of Hypothesis, which may be found at +# https://github.com/HypothesisWorks/hypothesis/ +# +# Copyright the Hypothesis Authors. +# Individual contributors are listed in AUTHORS.rst and the git log. +# +# This Source Code Form is subject to the terms of the Mozilla Public License, +# v. 2.0. If a copy of the MPL was not distributed with this file, You can +# obtain one at https://mozilla.org/MPL/2.0/. + +import pytest + +pytest_plugins = "pytester" + +TEST_SCRIPT = """ +def test_noop(): + pass +""" + +LAZY_STRATEGY = "integers()" + +SIDEEFFECT_STATEMENT = f"st.{LAZY_STRATEGY}.wrapped_strategy" + +SIDEEFFECT_SCRIPT = f""" +from hypothesis import strategies as st + +{SIDEEFFECT_STATEMENT} +""" + + +@pytest.mark.skipif( + tuple(map(int, pytest.__version__.split(".")[:2])) <= (6, 1), + reason="Older pytest don't capture these warnings during runpytest setup", +) +def test_sideeffect_warning(testdir): + testdir.makeconftest(SIDEEFFECT_SCRIPT) + script = testdir.makepyfile(TEST_SCRIPT) + result = testdir.runpytest_subprocess(script) + assert "HypothesisSideeffectWarning" in "\n".join(result.outlines) + assert LAZY_STRATEGY in "\n".join(result.outlines) + + +def test_conftest_sideeffect_pinpoint_error(testdir, monkeypatch): + # -Werror is not sufficient since warning is emitted before session start. Additionally, we + # don't want to raise errors from other plugins. Due to limited filtering capabilities of + # PYTHONWARNINGS/-W ("message is a literal string that the start of the warning message must + # contain" and only built-in categories), we must fall back to the actual message text. + monkeypatch.setenv("PYTHONWARNINGS", "error:Slow code in plugin") + testdir.makeconftest(SIDEEFFECT_SCRIPT) + script = testdir.makepyfile(TEST_SCRIPT) + result = testdir.runpytest_subprocess(script) + assert "HypothesisSideeffectWarning" in "\n".join(result.errlines) + assert SIDEEFFECT_STATEMENT in "\n".join(result.errlines) + + +def test_plugin_sideeffect_pinpoint_error(testdir, monkeypatch): + # See comment above regarding this line + monkeypatch.setenv("PYTHONWARNINGS", "error:Slow code in plugin") + # Ensure we see the correct stacktrace regardless of plugin load order + monkeypatch.setenv("HYPOTHESIS_EXTEND_INITIALIZATION", "1") + testdir.makepyfile(sideeffect_plugin=SIDEEFFECT_SCRIPT) + script = testdir.makepyfile(TEST_SCRIPT) + result = testdir.runpytest_subprocess(script, "-p", "sideeffect_plugin") + assert "HypothesisSideeffectWarning" in "\n".join(result.errlines) + assert SIDEEFFECT_STATEMENT in "\n".join(result.errlines)