From 67a1d2554c5086787ab6423c9fc1d9099686572d Mon Sep 17 00:00:00 2001 From: Juliya Smith Date: Thu, 12 Sep 2024 13:35:10 -0500 Subject: [PATCH] fix: scope order issue --- src/ape/pytest/fixtures.py | 197 +++++++++++++++++++++++++--------- src/ape/pytest/plugin.py | 9 +- src/ape/pytest/runners.py | 96 ++++++++++------- src/ape/pytest/utils.py | 12 +++ tests/functional/test_test.py | 39 ------- 5 files changed, 222 insertions(+), 131 deletions(-) create mode 100644 src/ape/pytest/utils.py diff --git a/src/ape/pytest/fixtures.py b/src/ape/pytest/fixtures.py index 083e1aae13..9f191124e5 100644 --- a/src/ape/pytest/fixtures.py +++ b/src/ape/pytest/fixtures.py @@ -1,4 +1,6 @@ -from collections.abc import Iterator +import inspect +from collections.abc import Iterable, Iterator +from dataclasses import dataclass, field from fnmatch import fnmatch from functools import cached_property from typing import Optional @@ -14,6 +16,7 @@ from ape.managers.networks import NetworkManager from ape.managers.project import ProjectManager from ape.pytest.config import ConfigWrapper +from ape.pytest.utils import Scope from ape.types import SnapshotID from ape.utils.basemodel import ManagerAccessMixin from ape.utils.rpc import allow_disconnected @@ -24,22 +27,11 @@ class PytestApeFixtures(ManagerAccessMixin): # for fixtures, as they are used in output from the command # `ape test -q --fixture` (`pytest -q --fixture`). - _supports_snapshot: bool = True - receipt_capture: "ReceiptCapture" - - def __init__(self, config_wrapper: ConfigWrapper, receipt_capture: "ReceiptCapture"): + def __init__(self, config_wrapper: ConfigWrapper, isolation_manager: "IsolationManager"): self.config_wrapper = config_wrapper - self.receipt_capture = receipt_capture - - @cached_property - def _track_transactions(self) -> bool: - return ( - self.network_manager.provider is not None - and self.provider.is_connected - and (self.config_wrapper.track_gas or self.config_wrapper.track_coverage) - ) + self.isolation_manager = isolation_manager - @pytest.fixture(scope="session") + @pytest.fixture(scope=Scope.SESSION.value) def accounts(self) -> list[TestAccountAPI]: """ A collection of pre-funded accounts. @@ -53,28 +45,28 @@ def compilers(self): """ return self.compiler_manager - @pytest.fixture(scope="session") + @pytest.fixture(scope=Scope.SESSION.value) def chain(self) -> ChainManager: """ Manipulate the blockchain, such as mine or change the pending timestamp. """ return self.chain_manager - @pytest.fixture(scope="session") + @pytest.fixture(scope=Scope.SESSION.value) def networks(self) -> NetworkManager: """ Connect to other networks in your tests. """ return self.network_manager - @pytest.fixture(scope="session") + @pytest.fixture(scope=Scope.SESSION.value) def project(self) -> ProjectManager: """ Access contract types and dependencies. """ return self.local_project - @pytest.fixture(scope="session") + @pytest.fixture(scope=Scope.SESSION.value) def Contract(self): """ Instantiate a reference to an on-chain contract @@ -82,19 +74,95 @@ def Contract(self): """ return self.chain_manager.contracts.instance_at - def _isolation(self, request=None) -> Iterator[None]: + # isolation fixtures + @pytest.fixture(scope=Scope.SESSION.value) + def _session_isolation(self) -> Iterator[None]: + yield from self.isolation_manager.isolation(Scope.SESSION) + + @pytest.fixture(scope=Scope.PACKAGE.value) + def _package_isolation(self) -> Iterator[None]: + yield from self.isolation_manager.isolation(Scope.PACKAGE) + + @pytest.fixture(scope=Scope.MODULE.value) + def _module_isolation(self) -> Iterator[None]: + yield from self.isolation_manager.isolation(Scope.MODULE) + + @pytest.fixture(scope=Scope.CLASS.value) + def _class_isolation(self) -> Iterator[None]: + yield from self.isolation_manager.isolation(Scope.CLASS) + + @pytest.fixture(scope=Scope.FUNCTION.value) + def _function_isolation(self) -> Iterator[None]: + yield from self.isolation_manager.isolation(Scope.FUNCTION) + + +@dataclass +class Snapshot: + scope: "Scope" # Assuming 'Scope' is defined elsewhere + identifier: Optional[str] = None + fixtures: list = field(default_factory=list) + + +class IsolationManager(ManagerAccessMixin): + INVALID_KEY = "__invalid_snapshot__" + + _supported: bool = True + _snapshot_registry: dict[Scope, Snapshot] = { + Scope.SESSION: Snapshot(Scope.SESSION), + Scope.PACKAGE: Snapshot(Scope.PACKAGE), + Scope.MODULE: Snapshot(Scope.MODULE), + Scope.CLASS: Snapshot(Scope.CLASS), + Scope.FUNCTION: Snapshot(Scope.FUNCTION), + } + + def __init__(self, config_wrapper: ConfigWrapper, receipt_capture: "ReceiptCapture"): + self.config_wrapper = config_wrapper + self.receipt_capture = receipt_capture + + @cached_property + def builtin_ape_fixtures(self) -> tuple[str, ...]: + return tuple( + [ + n + for n, itm in inspect.getmembers(PytestApeFixtures) + if callable(itm) and not n.startswith("_") + ] + ) + + @cached_property + def _track_transactions(self) -> bool: + return ( + self.network_manager.provider is not None + and self.provider.is_connected + and (self.config_wrapper.track_gas or self.config_wrapper.track_coverage) + ) + + def update_fixtures(self, scope: Scope, fixtures: Iterable[str]): + snapshot = self._snapshot_registry[scope] + if not ( + new_fixtures := [ + p + for p in fixtures + if p not in snapshot.fixtures and p not in self.builtin_ape_fixtures + ] + ): + return + + # If the snapshot is already set, we have to invalidate it. + # We need to replace the snapshot with one that happens after + # the new fixtures. + if snapshot is not None: + self._snapshot_registry[scope].identifier = self.INVALID_KEY + + # Add or update peer-fixtures. + self._snapshot_registry[scope].fixtures.extend(new_fixtures) + + def isolation(self, scope: Scope) -> Iterator[None]: """ Isolation logic used to implement isolation fixtures for each pytest scope. When tracing support is available, will also assist in capturing receipts. """ - snapshot_id = None - - if self._supports_snapshot: - try: - snapshot_id = self._snapshot() - except BlockNotFoundError: - self._supports_snapshot = False - + self._set_snapshot(scope) if self._track_transactions: did_yield = False try: @@ -106,33 +174,54 @@ def _isolation(self, request=None) -> Iterator[None]: if not did_yield: # Prevent double yielding. yield - else: yield - # NOTE: self._supports_snapshot may have gotten set to False + # NOTE: self._supported may have gotten set to False # someplace else _after_ snapshotting succeeded. - if self._supports_snapshot and snapshot_id is not None: - self._restore(snapshot_id) + if not self._supported: + return - # isolation fixtures - @pytest.fixture(scope="session") - def _session_isolation(self, request) -> Iterator[None]: - yield from self._isolation(request) + self._restore(scope) - @pytest.fixture(scope="package") - def _package_isolation(self, request) -> Iterator[None]: - yield from self._isolation(request) + def _set_snapshot(self, scope: Scope): + # Also can be used to re-set snapshot. + if not self._supported: + return - @pytest.fixture(scope="module") - def _module_isolation(self, request) -> Iterator[None]: - yield from self._isolation(request) + # Here is something tricky: If a snapshot exists + # already at a lower-level, we must use that one. + # Like if a session comes in _after_ a module, have + # the session just use the module. + # Else, it falls apart. + snapshot_id = None + if scope is not Scope.FUNCTION: + lower_scopes: tuple[Scope, ...] = () + if scope is Scope.SESSION: + lower_scopes = (Scope.PACKAGE, Scope.MODULE, Scope.CLASS, Scope.FUNCTION) + elif scope is Scope.PACKAGE: + lower_scopes = (Scope.MODULE, Scope.CLASS, Scope.FUNCTION) + elif scope is Scope.MODULE: + lower_scopes = (Scope.CLASS, Scope.FUNCTION) + elif scope is Scope.CLASS: + lower_scopes = (Scope.FUNCTION,) + for lower_scope in lower_scopes: + snapshot = self._snapshot_registry[lower_scope] + if snapshot.identifier is not None: + snapshot_id = snapshot.identifier + break + + if snapshot_id is None: + try: + snapshot_id = self._take_snapshot() + except Exception: + self._supported = False - _class_isolation = pytest.fixture(_isolation, scope="class") - _function_isolation = pytest.fixture(_isolation, scope="function") + if snapshot_id is not None: + self._snapshot_registry[scope].identifier = snapshot_id @allow_disconnected - def _snapshot(self) -> Optional[SnapshotID]: + def _take_snapshot(self) -> Optional[SnapshotID]: try: return self.chain_manager.snapshot() except NotImplementedError: @@ -141,14 +230,21 @@ def _snapshot(self) -> Optional[SnapshotID]: "Tests will not be completely isolated." ) # To avoid trying again - self._supports_snapshot = False + self._supported = False return None @allow_disconnected - def _restore(self, snapshot_id: SnapshotID): - if snapshot_id not in self.chain_manager._snapshots: + def _restore(self, scope: Scope): + snapshot_id = self._snapshot_registry[scope].identifier + if snapshot_id is None: return + + elif snapshot_id not in self.chain_manager._snapshots or snapshot_id == self.INVALID_KEY: + # Still clear out. + self._snapshot_registry[scope].identifier = None + return + try: self.chain_manager.restore(snapshot_id) except NotImplementedError: @@ -157,11 +253,12 @@ def _restore(self, snapshot_id: SnapshotID): "Tests will not be completely isolated." ) # To avoid trying again - self._supports_snapshot = False + self._supported = False + + self._snapshot_registry[scope].identifier = None class ReceiptCapture(ManagerAccessMixin): - config_wrapper: ConfigWrapper receipt_map: dict[str, dict[str, ReceiptAPI]] = {} enter_blocks: list[int] = [] diff --git a/src/ape/pytest/plugin.py b/src/ape/pytest/plugin.py index ecb10b7e0e..693e3da7f6 100644 --- a/src/ape/pytest/plugin.py +++ b/src/ape/pytest/plugin.py @@ -4,7 +4,7 @@ from ape.exceptions import ConfigError from ape.pytest.config import ConfigWrapper from ape.pytest.coverage import CoverageTracker -from ape.pytest.fixtures import PytestApeFixtures, ReceiptCapture +from ape.pytest.fixtures import IsolationManager, PytestApeFixtures, ReceiptCapture from ape.pytest.gas import GasTracker from ape.pytest.runners import PytestApeRunner from ape.utils.basemodel import ManagerAccessMixin @@ -77,6 +77,7 @@ def is_module(v): config_wrapper = ConfigWrapper(config) receipt_capture = ReceiptCapture(config_wrapper) + isolation_manager = IsolationManager(config_wrapper, receipt_capture) gas_tracker = GasTracker(config_wrapper) coverage_tracker = CoverageTracker(config_wrapper) @@ -84,14 +85,16 @@ def is_module(v): config.option.verbose = config.getoption("capture") == "no" # Register the custom Ape test runner - runner = PytestApeRunner(config_wrapper, receipt_capture, gas_tracker, coverage_tracker) + runner = PytestApeRunner( + config_wrapper, isolation_manager, receipt_capture, gas_tracker, coverage_tracker + ) config.pluginmanager.register(runner, "ape-test") # Inject runner for access to gas and coverage trackers. ManagerAccessMixin._test_runner = runner # Include custom fixtures for project, accounts etc. - fixtures = PytestApeFixtures(config_wrapper, receipt_capture) + fixtures = PytestApeFixtures(config_wrapper, isolation_manager) config.pluginmanager.register(fixtures, "ape-fixtures") # Add custom markers diff --git a/src/ape/pytest/runners.py b/src/ape/pytest/runners.py index 284816df24..8b6b6a9536 100644 --- a/src/ape/pytest/runners.py +++ b/src/ape/pytest/runners.py @@ -10,8 +10,9 @@ from ape.logging import LogLevel from ape.pytest.config import ConfigWrapper from ape.pytest.coverage import CoverageTracker -from ape.pytest.fixtures import ReceiptCapture +from ape.pytest.fixtures import IsolationManager, ReceiptCapture from ape.pytest.gas import GasTracker +from ape.pytest.utils import Scope from ape.types.coverage import CoverageReport from ape.utils.basemodel import ManagerAccessMixin from ape_console._cli import console @@ -21,11 +22,13 @@ class PytestApeRunner(ManagerAccessMixin): def __init__( self, config_wrapper: ConfigWrapper, + isolation_manager: IsolationManager, receipt_capture: ReceiptCapture, gas_tracker: GasTracker, coverage_tracker: CoverageTracker, ): self.config_wrapper = config_wrapper + self.isolation_manager = isolation_manager self.receipt_capture = receipt_capture self._provider_is_connected = False @@ -140,7 +143,59 @@ def pytest_runtest_setup(self, item): # isolation is disabled via cmdline option or running doc-tests. return - _insert_isolation_fixtures(item) + # Abstracted for testing purposes. + fixture_map = item.session._fixturemanager._arg2fixturedefs + scope_map = { + name: definition.scope + for name, definitions in fixture_map.items() + if name in item.fixturenames + for definition in definitions + } + scopes = [scope_map[n] for n in item.fixturenames if n in scope_map] + for scope in (Scope.SESSION, Scope.PACKAGE, Scope.MODULE, Scope.CLASS): + try: + index = len(scopes) - 1 - scopes[::-1].index(scope) + except ValueError: + # Intermediate scope isolations aren't filled in + continue + + fixtures_used = { + n + for n, scp in scope_map.items() + if scp == scope and n not in self.isolation_manager.builtin_ape_fixtures + } + if not fixtures_used: + # Potentially only using built-ape fixtures and nothing else. + continue + + # For non-function scoped isolation, the fixtures + # must go after the last fixture with that scope + # to ensure contracts deployed in fixtures persist. + name = f"_{scope}_isolation" + if name not in item.fixturenames: + item.fixturenames.insert(index + 1, f"_{scope}_isolation") + + # This line is needed to fix the calculation on subsequent checks + scopes.insert(index, scope) + fixtures_used = self._get_fixtures(scope_map, scope) + self.isolation_manager.update_fixtures(scope, fixtures_used) + + # Function-isolation must go last. + try: + item.fixturenames.insert(scopes.index(Scope.FUNCTION), f"_{Scope.FUNCTION}_isolation") + except ValueError: + # No fixtures with function scope, so append function isolation. + item.fixturenames.append("_function_isolation") + + fixtures_used = self._get_fixtures(scope_map, Scope.FUNCTION) + self.isolation_manager.update_fixtures(Scope.FUNCTION, fixtures_used) + + def _get_fixtures(self, scope_map: dict, scope: Scope) -> set[str]: + return { + n + for n, scp in scope_map.items() + if scp == scope and n not in self.isolation_manager.builtin_ape_fixtures + } def pytest_sessionstart(self): """ @@ -248,40 +303,3 @@ def pytest_unconfigure(self): self.chain_manager.contracts.clear_local_caches() self.gas_tracker.session_gas_report = None self.coverage_tracker.reset() - - -def _insert_isolation_fixtures(item): - # Abstracted for testing purposes. - fixture_map = item.session._fixturemanager._arg2fixturedefs - scope_map = { - name: definition.scope - for name, definitions in fixture_map.items() - if name in item.fixturenames - for definition in definitions - } - scopes = [scope_map[n] for n in item.fixturenames if n in scope_map] - - for scope in ("session", "package", "module", "class"): - try: - index = scopes.index(scope) - except ValueError: - # Intermediate scope isolations aren't filled in - continue - - # For non-function scoped isolation, the fixtures - # must go after the last fixture with that scope - # to ensure contracts deployed in fixtures persist. - name = f"_{scope}_isolation" - if name not in item.fixturenames: - item.fixturenames.insert(index, f"_{scope}_isolation") - - # This line is needed to fix the calculation on subsequent checks - scopes.insert(index, scope) - - # Function-isolation must go last. - - try: - item.fixturenames.insert(scopes.index("function"), "_function_isolation") - except ValueError: - # No fixtures with function scope, so append function isolation. - item.fixturenames.insert(0, "_function_isolation") diff --git a/src/ape/pytest/utils.py b/src/ape/pytest/utils.py new file mode 100644 index 0000000000..dcfdee6132 --- /dev/null +++ b/src/ape/pytest/utils.py @@ -0,0 +1,12 @@ +from enum import Enum + + +class Scope(str, Enum): + SESSION = "session" + PACKAGE = "package" + MODULE = "module" + CLASS = "class" + FUNCTION = "function" + + def __str__(self) -> str: + return self.value diff --git a/tests/functional/test_test.py b/tests/functional/test_test.py index f9a94b93aa..78b2d24930 100644 --- a/tests/functional/test_test.py +++ b/tests/functional/test_test.py @@ -1,4 +1,3 @@ -from ape.pytest.runners import _insert_isolation_fixtures from ape_test import ApeTestConfig @@ -10,41 +9,3 @@ def test_balance_set_from_currency_str(self): actual = cfg.balance expected = 10_000_000_000_000_000_000 # 10 ETH in WEI assert actual == expected - - -def test_insert_isolation_fixtures(mocker): - mock_item = mocker.MagicMock() - - def _create_fixture_entry(name: str, scope: str): - mock_fixture = mocker.MagicMock() - mock_fixture.scope = scope - mock_fixture.name = name - return mock_fixture - - fixtures = { - "fixture_at_function": [_create_fixture_entry("fixture_at_function", "function")], - "fixture_at_session": [_create_fixture_entry("fixture_at_session", "session")], - "fixture_at_module": [_create_fixture_entry("fixture_at_module", "module")], - "fixture_at_class": [_create_fixture_entry("fixture_at_class", "class")], - "other_random_fixture": [_create_fixture_entry("other_random_fixture", "function")], - } - - mock_item.session._fixturemanager._arg2fixturedefs = fixtures - mock_item.fixturenames = [*list(fixtures.keys()), "otheriteminnames"] - _insert_isolation_fixtures(mock_item) - actual_fixturenames = mock_item.fixturenames - - # NOTE: The order of this list is VERY IMPORTANT! - expected_fixturenames = [ - "_function_isolation", - "fixture_at_function", - "_session_isolation", - "fixture_at_session", - "_module_isolation", - "fixture_at_module", - "_class_isolation", - "fixture_at_class", - "other_random_fixture", - "otheriteminnames", - ] - assert actual_fixturenames == expected_fixturenames