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

Warnings for side effects during import #3837

Merged
merged 14 commits into from
Jan 16, 2024
Merged
1 change: 1 addition & 0 deletions hypothesis-python/.coveragerc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions hypothesis-python/RELEASE.rst
Original file line number Diff line number Diff line change
@@ -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.
6 changes: 5 additions & 1 deletion hypothesis-python/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down
28 changes: 28 additions & 0 deletions hypothesis-python/src/_hypothesis_globals.py
Original file line number Diff line number Diff line change
@@ -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."""
Zac-HD marked this conversation as resolved.
Show resolved Hide resolved

if os.environ.get("HYPOTHESIS_EXTEND_INITIALIZATION"):
in_initialization += 1
25 changes: 25 additions & 0 deletions hypothesis-python/src/_hypothesis_pytestplugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@

import base64
import json
import os
import sys
import warnings
from inspect import signature

import _hypothesis_globals
import pytest

try:
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions hypothesis-python/src/hypothesis/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
failing examples it finds.
"""

import _hypothesis_globals

from hypothesis._settings import HealthCheck, Phase, Verbosity, settings
from hypothesis.control import (
assume,
Expand Down Expand Up @@ -54,3 +56,6 @@

run()
del run

_hypothesis_globals.in_initialization -= 1
del _hypothesis_globals
62 changes: 61 additions & 1 deletion hypothesis-python/src/hypothesis/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -21,11 +26,66 @@ 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"):
__hypothesis_home_directory = Path(where)
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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to stored the formatted what? Slightly concerned about holding onto a potentially mutable object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OTOH we know exactly what comes in, no user will call this function. Thanks for getting to all green 😁

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to help!



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.",
)
4 changes: 3 additions & 1 deletion hypothesis-python/src/hypothesis/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand Down Expand Up @@ -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)

Expand Down
7 changes: 7 additions & 0 deletions hypothesis-python/src/hypothesis/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down
20 changes: 16 additions & 4 deletions hypothesis-python/src/hypothesis/strategies/_internal/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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} "
Expand Down
3 changes: 3 additions & 0 deletions hypothesis-python/src/hypothesis/strategies/_internal/lazy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down
59 changes: 59 additions & 0 deletions hypothesis-python/tests/cover/test_sideeffect_warnings.py
Original file line number Diff line number Diff line change
@@ -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()
Loading
Loading