Trace parameter reads per call and stop test reruns leaking module fixtures

.github/workflows/pr.yaml

@@ -77,7 +77,7 @@ jobs:
       - name: Install Core and the legacy country smoke bundle
         # Resolve the checkout and all country dependencies together so a later
         # install cannot replace Core or pair a legacy US model with SPM 1.x.
-        run: uv pip install --system . -r .github/requirements/legacy-country-smoke.txt "pytest-rerunfailures>=10,<15"
+        run: uv pip install --system . -r .github/requirements/legacy-country-smoke.txt "pytest-rerunfailures>=16.2,<17"
       - name: Verify installed dependency compatibility
         run: uv pip check --system
       - name: Verify pytest plugins

changelog.d/rerunfailures-fixture-teardown.changed.md

@@ -0,0 +1 @@
+The dev dependency on pytest-rerunfailures now requires 16.2 or later, whose reruns no longer leave a module-scoped fixture cached for later test modules.

changelog.d/traced-simulation-parameter-isolation.fixed.md

@@ -0,0 +1 @@
+Tracing a simulation no longer switches tracing on in the parameter tree of its tax-benefit system, so other simulations, branches and clones of the system stay untraced, every traced simulation and branch records the parameters its formulas read under its own tracer and branch name, and traced parameter nodes can be copied, deep-copied and pickled.

docs/python_api/tracers.md

@@ -56,6 +56,15 @@ The `policyengine_core.tracers` module contains classes used to represent tracer
     :show-inheritance:
 ```
 
+## TracingParameterNode
+
+```{eval-rst}
+.. autoclass:: policyengine_core.tracers.tracing_parameter_node_at_instant.TracingParameterNode
+    :members:
+    :inherited-members:
+    :show-inheritance:
+```
+
 ## TracingParameterNodeAtInstant
 
 ```{eval-rst}

policyengine_core/simulations/simulation.py

@@ -22,7 +22,7 @@
 from policyengine_core.tracers import (
     FullTracer,
     SimpleTracer,
-    TracingParameterNodeAtInstant,
+    TracingParameterNode,
 )
 import random
 from policyengine_core.tools.hugging_face import *
@@ -1184,14 +1184,16 @@ def _run_formula(
                             )
             return values
 
-        if self.trace and not isinstance(
-            self.tax_benefit_system.parameters, TracingParameterNodeAtInstant
-        ):
-            # Soft-recast
-            self.tax_benefit_system.parameters.branch_name = self.branch_name
-            self.tax_benefit_system.parameters.trace = True
-            self.tax_benefit_system.parameters.tracer = self.tracer
         parameters_at = self.tax_benefit_system.parameters
+        if self.trace:
+            # Trace through a view of the parameter tree that belongs to this
+            # call. Switching tracing on in the tree itself would leave the
+            # shared tax-benefit system traced for every simulation, branch
+            # and clone that uses it afterwards, with this simulation's tracer
+            # and branch name cached in the tree's nodes at each instant.
+            parameters_at = TracingParameterNode(
+                parameters_at, self.tracer, self.branch_name
+            )
 
         # A rules-engine formula must be a pure, deterministic function of its
         # inputs. Randomness is forbidden statically at variable registration

policyengine_core/tracers/__init__.py

@@ -5,4 +5,7 @@
 from .variable_graph import VariableGraph
 from .simple_tracer import SimpleTracer
 from .trace_node import TraceNode
-from .tracing_parameter_node_at_instant import TracingParameterNodeAtInstant
+from .tracing_parameter_node_at_instant import (
+    TracingParameterNode,
+    TracingParameterNodeAtInstant,
+)

policyengine_core/tracers/tracing_parameter_node_at_instant.py

@@ -21,6 +21,66 @@
     Child = Union[ParameterNode, ArrayLike]
 
 
+def _wrapped(wrapper: object, attribute: str, key: str) -> object:
+    """Return the object ``wrapper`` delegates the lookup of ``key`` to.
+
+    ``__getattr__`` only runs when normal lookup fails. Two such lookups must
+    not reach the wrapped object:
+
+    - special names, which ``copy``, ``deepcopy`` and ``pickle`` probe on an
+      instance (``__deepcopy__``, ``__setstate__``, ...). The wrapped
+      object's answer would act on the wrapped object, not on the wrapper;
+    - any name on an instance those protocols have created with ``__new__``
+      and not filled in yet. It has no ``attribute``, so reading it here
+      would call ``__getattr__`` again, without end.
+    """
+    if key.startswith("__") and key.endswith("__"):
+        raise AttributeError(key)
+    try:
+        return wrapper.__dict__[attribute]
+    except KeyError:
+        raise AttributeError(key) from None
+
+
+class TracingParameterNode:
+    """The parameter tree as the formulas of one traced simulation see it.
+
+    Calling it at an instant returns a :class:`TracingParameterNodeAtInstant`
+    that records every parameter a formula reads in ``tracer``, under
+    ``branch_name``. Any other attribute is read from the wrapped node.
+
+    The wrapped node is never modified, so tracing one simulation does not
+    trace the tax-benefit system it shares with other simulations, branches
+    and clones.
+    """
+
+    def __init__(
+        self,
+        parameter_node: parameters.ParameterNode,
+        tracer: tracers.FullTracer,
+        branch_name: str,
+    ) -> None:
+        self.parameter_node = parameter_node
+        self.tracer = tracer
+        self.branch_name = branch_name
+
+    def __call__(self, instant) -> TracingParameterNodeAtInstant:
+        return self.get_at_instant(instant)
+
+    def get_at_instant(self, instant) -> TracingParameterNodeAtInstant:
+        node_at_instant = self.parameter_node.get_at_instant(instant)
+        if isinstance(node_at_instant, TracingParameterNodeAtInstant):
+            # The node traces by itself (its ``trace`` flag is set): record
+            # in this simulation's tracer, not in the one the node holds.
+            node_at_instant = node_at_instant.parameter_node_at_instant
+        return TracingParameterNodeAtInstant(
+            node_at_instant, self.tracer, self.branch_name
+        )
+
+    def __getattr__(self, key: str):
+        return getattr(_wrapped(self, "parameter_node", key), key)
+
+
 class TracingParameterNodeAtInstant:
     def __init__(
         self,
@@ -36,7 +96,7 @@ def __getattr__(
         self,
         key: str,
     ) -> Union[TracingParameterNodeAtInstant, Child]:
-        child = getattr(self.parameter_node_at_instant, key)
+        child = getattr(_wrapped(self, "parameter_node_at_instant", key), key)
         return self.get_traced_child(child, key)
 
     def __getitem__(

pyproject.toml

@@ -48,7 +48,7 @@ dev = [
     "markupsafe==2.0.1",
     "mypy==0.991",
     "pytest-cov",
-    "pytest-rerunfailures>=10,<15",
+    "pytest-rerunfailures>=16.2,<17",
     "sphinx==5.0.0",
     "sphinx-argparse==0.4.0",
     "sphinx-math-dollar==1.2.1",

tests/core/test_rerun_fixture_isolation.py

@@ -0,0 +1,134 @@
+"""A rerun must not carry a module-scoped fixture into later modules.
+
+``make test`` reruns failed tests (``--reruns 2``). pytest-rerunfailures 14.0
+empties pytest's setup stack before a rerun without running the finalizers on
+it, and a cached fixture registers its finalizer only when it is first
+created. So the module-scoped ``tax_benefit_system`` of the rerun test's
+module was never torn down, and pytest handed the same cached object to every
+later module, with whatever earlier modules had done to it. On Windows CI a
+later module then traced it, and ``tests/core/test_parameters.py`` and
+``tests/core/test_reforms.py`` failed on a system that was not theirs.
+
+pytest-rerunfailures 15.0 puts the finalizers back after a rerun
+(pytest-dev/pytest-rerunfailures#278), and the dev dependency now requires a
+release that does. This test runs a small suite with the installed plugin in a
+subprocess: a test in the first module fails and is rerun, and the module
+after it must get a fresh fixture, created after the first module's was torn
+down.
+"""
+
+import os
+import subprocess
+import sys
+import textwrap
+import xml.etree.ElementTree as ElementTree
+
+import pytest
+
+pytest.importorskip("pytest_rerunfailures")
+
+CONFTEST = """
+import pytest
+
+CREATED = []
+
+
+class System:
+    def __init__(self, module):
+        self.module = module
+        self.torn_down = False
+
+
+@pytest.fixture(scope="module")
+def system(request):
+    system = System(request.module.__name__)
+    CREATED.append(system)
+    yield system
+    system.torn_down = True
+"""
+
+PASSES_ON_RERUN = """
+ATTEMPTS = []
+
+
+def test_rerun(system):
+    ATTEMPTS.append(system)
+    assert len(ATTEMPTS) > 1, "the first attempt fails"
+"""
+
+FAILS_EVERY_ATTEMPT = """
+def test_rerun(system):
+    assert False, "every attempt fails"
+"""
+
+LATER_MODULE = """
+from conftest import CREATED
+
+
+def test_gets_a_fresh_fixture(system):
+    assert system.module == __name__
+    earlier = [created for created in CREATED if created is not system]
+    assert earlier
+    assert all(created.torn_down for created in earlier)
+    assert all(created.module != __name__ for created in earlier)
+"""
+
+
+def _outcomes(junit_xml):
+    outcomes = {}
+    for case in ElementTree.parse(junit_xml).iter("testcase"):
+        name = f"{case.get('classname')}.{case.get('name')}"
+        failed = case.find("failure") is not None or case.find("error") is not None
+        outcomes[name] = "failed" if failed else "passed"
+    return outcomes
+
+
+@pytest.mark.parametrize(
+    "rerun_module, expected_rerun_outcome",
+    [(PASSES_ON_RERUN, "passed"), (FAILS_EVERY_ATTEMPT, "failed")],
+    ids=["passes-on-rerun", "fails-every-attempt"],
+)
+def test_rerun_does_not_leak_module_fixture_into_later_modules(
+    tmp_path, rerun_module, expected_rerun_outcome
+):
+    (tmp_path / "pytest.ini").write_text("[pytest]\n")
+    (tmp_path / "conftest.py").write_text(textwrap.dedent(CONFTEST))
+    (tmp_path / "test_a_rerun.py").write_text(textwrap.dedent(rerun_module))
+    (tmp_path / "test_b_later.py").write_text(textwrap.dedent(LATER_MODULE))
+    junit_xml = tmp_path / "junit.xml"
+
+    environment = {
+        key: value
+        for key, value in os.environ.items()
+        if not key.startswith(("PYTEST_", "COV_", "COVERAGE_"))
+    }
+    environment["PYTEST_DISABLE_PLUGIN_AUTOLOAD"] = "1"
+    completed = subprocess.run(
+        [
+            sys.executable,
+            "-m",
+            "pytest",
+            "-p",
+            "pytest_rerunfailures",
+            "-p",
+            "no:cacheprovider",
+            "--reruns",
+            "1",
+            "--reruns-delay",
+            "0",
+            f"--junitxml={junit_xml}",
+            "-q",
+        ],
+        cwd=tmp_path,
+        env=environment,
+        capture_output=True,
+        text=True,
+        timeout=120,
+    )
+    output = completed.stdout + completed.stderr
+
+    assert _outcomes(junit_xml) == {
+        "test_a_rerun.test_rerun": expected_rerun_outcome,
+        "test_b_later.test_gets_a_fresh_fixture": "passed",
+    }, output
+    assert "1 rerun" in output, output