clean up

Author: alexluck-sift

python/lib/sift_client/_tests/_pytester_fakes.py

@@ -7,6 +7,7 @@
 
 from __future__ import annotations
 
+import itertools
 import json
 from typing import TYPE_CHECKING, Any
 from unittest.mock import MagicMock
@@ -39,8 +40,6 @@ def update(self, fields: dict[str, Any]) -> None:
 
 
 class FakeReportContext:
-    _counter = 0
-
     def __init__(self, steps_file: Path) -> None:
         self.steps_file = steps_file
         self.report = FakeReport()
@@ -51,6 +50,7 @@ def __init__(self, steps_file: Path) -> None:
         self.any_failures = False
         self.log_file: Path | None = None
         self.steps: list[dict[str, Any]] = []
+        self._ids = itertools.count(1)
 
     def __enter__(self) -> FakeReportContext:
         return self
@@ -90,11 +90,10 @@ def create_step(
         description: str | None = None,
         metadata: dict[str, Any] | None = None,
     ) -> FakeStep:
-        type(self)._counter += 1
         step_path = self.get_next_step_path()
         parent = self.step_stack[-1] if self.step_stack else None
         step = FakeStep(
-            id_=f"step-{type(self)._counter}",
+            id_=f"step-{next(self._ids)}",
             name=name,
             parent_step_id=parent.id_ if parent else None,
             step_path=step_path,

python/lib/sift_client/_tests/test_pytest_plugin_parametrize.py

@@ -12,7 +12,6 @@
 from __future__ import annotations
 
 import json
-import os
 from textwrap import dedent
 from typing import TYPE_CHECKING
 
@@ -58,11 +57,12 @@ def client_has_connection() -> bool:
     return False
 
 
-def _setup(pytester: pytest.Pytester) -> Path:
-    steps_file = pytester.path / "captured_steps.json"
+@pytest.fixture
+def steps_file(pytester: pytest.Pytester, monkeypatch: pytest.MonkeyPatch) -> Path:
+    path = pytester.path / "captured_steps.json"
     pytester.makeconftest(_INNER_CONFTEST)
-    os.environ[_STEPS_FILE_ENV] = str(steps_file)
-    return steps_file
+    monkeypatch.setenv(_STEPS_FILE_ENV, str(path))
+    return path
 
 
 def _by_name(steps: list[dict]) -> dict[str, list[dict]]:
@@ -72,8 +72,9 @@ def _by_name(steps: list[dict]) -> dict[str, list[dict]]:
     return out
 
 
-def test_single_parametrize_clusters_under_originalname(pytester: pytest.Pytester) -> None:
-    steps_file = _setup(pytester)
+def test_single_parametrize_clusters_under_originalname(
+    pytester: pytest.Pytester, steps_file: Path
+) -> None:
     pytester.makepyfile(
         test_rail=dedent(
             """
@@ -99,8 +100,9 @@ def test_rail(v):
     assert by_name["v=5.0"][0]["parent_step_id"] == test_rail_id
 
 
-def test_stacked_parametrize_nests_outer_to_inner(pytester: pytest.Pytester) -> None:
-    steps_file = _setup(pytester)
+def test_stacked_parametrize_nests_outer_to_inner(
+    pytester: pytest.Pytester, steps_file: Path
+) -> None:
     pytester.makepyfile(
         test_iso=dedent(
             """
@@ -137,8 +139,7 @@ def test_iso(voltage, component):
         assert leaf["parent_step_id"] in voltage_ids
 
 
-def test_fixture_parametrization_participates(pytester: pytest.Pytester) -> None:
-    steps_file = _setup(pytester)
+def test_fixture_parametrization_participates(pytester: pytest.Pytester, steps_file: Path) -> None:
     pytester.makepyfile(
         test_widget=dedent(
             """
@@ -163,8 +164,9 @@ def test_widget(widget):
     assert by_name["widget='b'"][0]["parent_step_id"] == parent_id
 
 
-def test_module_boundary_isolates_parametrize_stack(pytester: pytest.Pytester) -> None:
-    steps_file = _setup(pytester)
+def test_module_boundary_isolates_parametrize_stack(
+    pytester: pytest.Pytester, steps_file: Path
+) -> None:
     pytester.makepyfile(
         test_a=dedent(
             """
@@ -196,8 +198,9 @@ def test_two(w):
     assert by_name["test_two"][0]["parent_step_id"] == mod_b["id"]
 
 
-def test_leaf_parent_chain_terminates_at_report(pytester: pytest.Pytester) -> None:
-    steps_file = _setup(pytester)
+def test_leaf_parent_chain_terminates_at_report(
+    pytester: pytest.Pytester, steps_file: Path
+) -> None:
     pytester.makepyfile(
         test_chain=dedent(
             """

python/lib/sift_client/pytest_plugin.py

@@ -19,7 +19,15 @@
 _ENV_VARS = ("SIFT_API_KEY", "SIFT_GRPC_URI", "SIFT_REST_URI")
 
 _PARAMETRIZE_PATH_KEY = pytest.StashKey[tuple[str, ...]]()
-_PARAMETRIZE_STACK: list[NewStep] = []
+# Each frame: (path_key, open NewStep). Frames are shared across sibling test
+# items and drained at module teardown / session end.
+_PARAMETRIZE_STACK: list[tuple[str, NewStep]] = []
+
+
+def _drain_parametrize_stack() -> None:
+    while _PARAMETRIZE_STACK:
+        _, ns = _PARAMETRIZE_STACK.pop()
+        ns.__exit__(None, None, None)
 
 
 def _build_parametrize_path(item: pytest.Item) -> tuple[str, ...]:
@@ -47,8 +55,7 @@ def pytest_collection_modifyitems(config: pytest.Config, items: list[pytest.Item
 
 def pytest_sessionfinish(session: pytest.Session, exitstatus: int) -> None:
     """Drain any parametrize parents still open (e.g. when module_substep was disabled)."""
-    while _PARAMETRIZE_STACK:
-        _PARAMETRIZE_STACK.pop().__exit__(None, None, None)
+    _drain_parametrize_stack()
 
 
 def pytest_addoption(parser: pytest.Parser) -> None:
@@ -242,14 +249,8 @@ def _step_impl(
     report_context: ReportContext, request: pytest.FixtureRequest
 ) -> Generator[NewStep, None, None]:
     item = request.node
-    callspec = getattr(item, "callspec", None)
-    if callspec is not None and callspec.params:
-        # Bottom decorator's axis is first in callspec.params, which matches the
-        # innermost (leaf) frame in the parametrize path.
-        axis, value = next(iter(callspec.params.items()))
-        name = f"{axis}={value!r}"
-    else:
-        name = str(item.name)
+    path = item.stash.get(_PARAMETRIZE_PATH_KEY, ())
+    name = path[-1] if path else str(item.name)
     existing_docstring = item.obj.__doc__ or None
     with report_context.new_step(
         name=name, description=existing_docstring, assertion_as_fail_not_error=False
@@ -278,19 +279,20 @@ def _parametrize_parents(
     """
     desired = request.node.stash.get(_PARAMETRIZE_PATH_KEY, ())
     parents = desired[:-1]
-    open_keys = [getattr(ns, "_parametrize_key", None) for ns in _PARAMETRIZE_STACK]
     common = 0
     while (
-        common < len(open_keys) and common < len(parents) and open_keys[common] == parents[common]
+        common < len(_PARAMETRIZE_STACK)
+        and common < len(parents)
+        and _PARAMETRIZE_STACK[common][0] == parents[common]
     ):
         common += 1
     while len(_PARAMETRIZE_STACK) > common:
-        _PARAMETRIZE_STACK.pop().__exit__(None, None, None)
+        _, ns = _PARAMETRIZE_STACK.pop()
+        ns.__exit__(None, None, None)
     for display in parents[common:]:
         ns = report_context.new_step(name=display, assertion_as_fail_not_error=False)
         ns.__enter__()
-        ns._parametrize_key = display  # type: ignore[attr-defined]
-        _PARAMETRIZE_STACK.append(ns)
+        _PARAMETRIZE_STACK.append((display, ns))
     yield
 
 
@@ -316,8 +318,6 @@ def module_substep(
         name=name, description=existing_docstring, assertion_as_fail_not_error=False
     ) as new_step:
         yield new_step
-        # Drain parametrize parents nested under this module before the module
-        # step exits — otherwise ReportContext.exit_step asserts that the module
-        # step is the stack top.
-        while _PARAMETRIZE_STACK:
-            _PARAMETRIZE_STACK.pop().__exit__(None, None, None)
+        # Drain parametrize parents nested under this module step before it
+        # exits — ReportContext.exit_step asserts the module step is the top.
+        _drain_parametrize_stack()