Fix spurious 'no result set' warnings and dropped execution tags under pytest-forked

.buildkite/pipeline.yml

@@ -17,7 +17,12 @@ steps:
     command: ".buildkite/steps/test"
     env:
       PYTHON_VERSION: "3.13"
-      UV_RUN_FLAGS: "--with pytest>=9"
+      # "--with py" is needed because the uv overlay environment puts
+      # pytest 9's vendored py.py shim (py.error/py.path only) ahead of the
+      # project venv's real py package, which pytest-forked needs for
+      # py.process. Installing py into the same overlay restores normal
+      # package-over-module resolution.
+      UV_RUN_FLAGS: "--with pytest>=9 --with py"
 
   - label: ":python: pylint {{matrix}}"
     command: ".buildkite/steps/lint"

pyproject.toml

@@ -14,7 +14,7 @@ classifiers = ["License :: OSI Approved :: MIT License", "Framework :: Pytest"]
 dependencies = ["requests>=2", "pytest>=7", "filelock>=3"]
 
 [project.optional-dependencies]
-dev = ["mock>=4", "check-manifest", "twine; python_full_version >= '3.9.1'", "responses", "pylint"]
+dev = ["mock>=4", "check-manifest", "twine; python_full_version >= '3.9.1'", "responses", "pylint", "pytest-forked; sys_platform != 'win32'"]
 
 [project.urls]
 Homepage = "https://github.com/buildkite/test-collector-python"

src/buildkite_test_collector/pytest_plugin/buildkite_plugin.py

@@ -2,6 +2,7 @@
 import json
 import os
 from pathlib import Path
+from typing import Dict, Tuple
 from uuid import uuid4
 
 from filelock import FileLock
@@ -29,6 +30,9 @@ def _is_subtest_report(report):
 class BuildkitePlugin:
     """Buildkite test collector plugin for Pytest"""
 
+    # 8 attributes of tracking state seems reasonable for this plugin
+    # pylint: disable=too-many-instance-attributes
+
     def __init__(self, payload, rootpath=None):
         self.payload = payload
         self.rootpath = rootpath
@@ -39,9 +43,30 @@ def __init__(self, payload, rootpath=None):
         # call-phase report from overwriting the failure.
         self._failed_by_subtest = set()
         self._collection_errors_seen = set()
+        # The pid of the process that owns this plugin instance (the one
+        # whose payload is uploaded/saved at pytest_unconfigure).  Runners
+        # that fork per test (e.g. pytest-forked) give each child a copy of
+        # this state that is discarded when the child exits — hooks that
+        # fire in such a child must not finalize tests.  See finalize_test.
+        self._owner_pid = os.getpid()
+        # execution_tag markers captured at collection time, keyed by
+        # nodeid.  Collection happens in the owning process, so these
+        # survive fork-per-test runners where pytest_runtest_makereport
+        # (our usual tagging point) only fires in the discarded child.
+        self._tags_by_nodeid: Dict[str, Tuple[Tuple[str, str], ...]] = {}
 
     def pytest_collection_modifyitems(self, config, items):
-        """pytest_collection_modifyitems hook callback to filter tests by execution_tag markers"""
+        """pytest_collection_modifyitems hook callback to capture execution_tag
+        markers and filter tests by them"""
+        for item in items:
+            tags = tuple(
+                (tag.args[0], tag.args[1])
+                for tag in item.iter_markers("execution_tag")
+                if len(tag.args) == 2
+            )
+            if tags:
+                self._tags_by_nodeid[item.nodeid] = tags
+
         tag_filter = config.getoption("tag_filters")
         if not tag_filter:
             return
@@ -277,11 +302,37 @@ def update_test_result(self, report):
     def finalize_test(self, nodeid):
         """ Attempting to move test data for a nodeid to payload area for upload """
         logger.debug('-> finalize_test nodeid=%s', nodeid)
+
+        # Fork-per-test runners (e.g. pytest-forked) run the test protocol
+        # in a child process holding a copy of this plugin.  That copy is
+        # discarded when the child exits — only the serialized reports are
+        # replayed in the owning process, which finalizes the test with the
+        # real result via pytest_runtest_logreport/logfinish.  Finalizing in
+        # the child would only produce a spurious "no result set" warning
+        # (log=False suppresses in-child logreport), so skip it.
+        if os.getpid() != self._owner_pid:
+            logger.debug(
+                '-> finalize_test: skipping in forked child (pid=%s, owner=%s)',
+                os.getpid(), self._owner_pid
+            )
+            return False
+
         test_data = self.in_flight.get(nodeid)
         if not test_data:
             logger.debug('-> finalize_test: not in flight: %s', nodeid)
             return False
         del self.in_flight[nodeid]
+
+        # Apply tags captured at collection time.  Under fork-per-test
+        # runners this is the only tagging point that runs in the owning
+        # process.  Only fill in keys that are still missing: a marker added
+        # dynamically during the test carries a fresher value (applied in
+        # pytest_runtest_makereport) that must not be overwritten by the
+        # collection-time snapshot.
+        for key, value in self._tags_by_nodeid.get(nodeid, ()):
+            if key not in test_data.tags:
+                test_data = test_data.tag_execution(key, value)
+
         if test_data.result is None:
             logger.warning('Test %s has no result set at finalization', nodeid)
         test_data = test_data.finish()

tests/buildkite_test_collector/pytest_plugin/test_plugin.py

@@ -1,4 +1,7 @@
 import json
+import os
+from types import SimpleNamespace
+
 import pytest
 
 from buildkite_test_collector.collector.payload import Payload, TestData, TestResultFailed, TestResultPassed, TestResultSkipped
@@ -565,3 +568,116 @@ def test_pytest_collectreport_deduplicates(fake_env):
     plugin.pytest_collectreport(report)
 
     assert len(plugin.payload.data) == 1
+
+
+# ---------------------------------------------------------------------------
+# fork-per-test runners (e.g. pytest-forked)
+# ---------------------------------------------------------------------------
+
+def test_finalize_test_skipped_in_forked_child(fake_env):
+    """A fork-per-test child holds a discarded copy of the plugin state, so
+    finalizing there must be a no-op — it would otherwise emit a spurious
+    'no result set at finalization' warning for every test, because the
+    child runs the test protocol with log=False (no logreport in-child).
+    The owning process finalizes from the replayed reports instead.
+    """
+    payload = Payload.init(fake_env)
+    plugin = BuildkitePlugin(payload)
+
+    location = ("", None, "")
+    nodeid = "test_sample.py::test_happy"
+    plugin.pytest_runtest_logstart(nodeid, location)
+
+    # Simulate running in a forked child: the current pid no longer matches
+    # the pid recorded when the plugin was constructed.
+    plugin._owner_pid = os.getpid() + 1
+
+    assert plugin.finalize_test(nodeid) is False
+    assert nodeid in plugin.in_flight
+    assert len(plugin.payload.data) == 0
+
+
+def test_finalize_test_runs_in_owning_process(fake_env):
+    """In the owning process finalize_test behaves as before."""
+    payload = Payload.init(fake_env)
+    plugin = BuildkitePlugin(payload)
+
+    location = ("", None, "")
+    report = TestReport(nodeid="test_sample.py::test_happy", location=location, keywords={}, outcome="passed", longrepr=None, when="call")
+
+    plugin.pytest_runtest_logstart(report.nodeid, location)
+    plugin.pytest_runtest_logreport(report)
+
+    assert plugin.finalize_test(report.nodeid) is True
+    assert report.nodeid not in plugin.in_flight
+    assert len(plugin.payload.data) == 1
+    assert isinstance(plugin.payload.data[0].result, TestResultPassed)
+
+
+def test_execution_tags_captured_at_collection_and_applied_at_finalize(fake_env):
+    """execution_tag markers are captured during collection (which happens in
+    the owning process) and applied at finalization, so they survive
+    fork-per-test runners where pytest_runtest_makereport only fires in the
+    discarded child process.
+    """
+    payload = Payload.init(fake_env)
+    plugin = BuildkitePlugin(payload)
+
+    nodeid = "test_sample.py::test_tagged"
+    marker = SimpleNamespace(args=("team", "backend"))
+    bad_marker = SimpleNamespace(args=("only-one-arg",))
+    item = SimpleNamespace(nodeid=nodeid, iter_markers=lambda name: [marker, bad_marker])
+    untagged_item = SimpleNamespace(nodeid="test_sample.py::test_plain", iter_markers=lambda name: [])
+    config = SimpleNamespace(getoption=lambda name: None)
+
+    plugin.pytest_collection_modifyitems(config, [item, untagged_item])
+
+    assert plugin._tags_by_nodeid == {nodeid: (("team", "backend"),)}
+
+    location = ("", None, "")
+    report = TestReport(nodeid=nodeid, location=location, keywords={}, outcome="passed", longrepr=None, when="call")
+    plugin.pytest_runtest_logstart(nodeid, location)
+    plugin.pytest_runtest_logreport(report)
+    plugin.pytest_runtest_logfinish(nodeid, location)
+
+    assert len(plugin.payload.data) == 1
+    assert plugin.payload.data[0].tags == {"team": "backend"}
+
+
+def test_collection_time_tags_do_not_overwrite_runtime_tags(fake_env):
+    """A marker added dynamically during the test run (applied in
+    pytest_runtest_makereport) must win over the collection-time snapshot
+    for the same key — finalize_test only fills in keys that are still
+    missing.
+    """
+    payload = Payload.init(fake_env)
+    plugin = BuildkitePlugin(payload)
+
+    nodeid = "test_sample.py::test_tagged"
+    collected_marker = SimpleNamespace(args=("team", "backend"))
+    item = SimpleNamespace(nodeid=nodeid, iter_markers=lambda name: [collected_marker])
+    config = SimpleNamespace(getoption=lambda name: None)
+
+    plugin.pytest_collection_modifyitems(config, [item])
+    assert plugin._tags_by_nodeid == {nodeid: (("team", "backend"),)}
+
+    location = ("", None, "")
+    report = TestReport(nodeid=nodeid, location=location, keywords={}, outcome="passed", longrepr=None, when="call")
+    plugin.pytest_runtest_logstart(nodeid, location)
+    plugin.pytest_runtest_logreport(report)
+
+    # Simulate pytest_runtest_makereport at teardown, where iter_markers now
+    # also yields a marker added dynamically during the test: the "team" key
+    # was re-tagged with a new value, and a brand-new key was added.
+    runtime_item = SimpleNamespace(
+        nodeid=nodeid,
+        iter_markers=lambda name: [
+            SimpleNamespace(args=("team", "frontend")),
+            SimpleNamespace(args=("priority", "high")),
+        ],
+    )
+    call = SimpleNamespace(when="teardown")
+    plugin.pytest_runtest_makereport(runtime_item, call)
+
+    assert len(plugin.payload.data) == 1
+    assert plugin.payload.data[0].tags == {"team": "frontend", "priority": "high"}

tests/buildkite_test_collector/test_integration_forked.py

@@ -0,0 +1,72 @@
+"""Integration tests for fork-per-test runners (pytest-forked).
+
+pytest-forked runs each test protocol in a forked child process with
+log=False, then replays the serialized reports in the parent.  The child
+inherits a copy of the plugin state that is discarded on exit; only the
+parent's payload is uploaded.  These tests verify that the collector
+produces correct results (with execution tags, and without spurious
+'no result set at finalization' warnings) in that mode.
+"""
+import json
+import os
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+pytest.importorskip("pytest_forked")
+
+# pytest-forked needs py.process from the real py package.  In layered
+# environments (e.g. `uv run --with pytest`), pytest's vendored py.py shim
+# (py.error/py.path only) can shadow the real package, breaking pytest-forked
+# itself with an INTERNALERROR — nothing our collector can be tested against.
+_py = pytest.importorskip("py")
+if not hasattr(_py, "process"):
+    pytest.skip(
+        "'py' resolves to pytest's vendored shim without py.process; "
+        "pytest-forked cannot run in this environment",
+        allow_module_level=True,
+    )
+
+pytestmark = pytest.mark.skipif(
+    not hasattr(os, "fork"), reason="pytest-forked requires os.fork"
+)
+
+
+def test_forked_run_collects_results_and_tags_without_warnings(tmp_path):
+    """Run pytest with --forked and verify results, tags, and clean logs."""
+    test_file = Path(__file__).parent / "data" / "test_sample_execution_tag.py"
+    json_output_file = tmp_path / "test_results.json"
+
+    cmd = [
+        sys.executable, "-m", "pytest",
+        "--forked",
+        str(test_file),
+        f"--json={json_output_file}",
+    ]
+
+    result = subprocess.run(cmd, capture_output=True, text=True)
+    if result.returncode != 0:
+        print(result.stdout)
+        print(result.stderr)
+        pytest.fail("pytest run failed, see output above")
+
+    output = result.stdout + result.stderr
+    assert "has no result set at finalization" not in output
+
+    with open(json_output_file, "r") as f:
+        test_results = json.load(f)
+
+    tests_by_name = {test["name"]: test for test in test_results}
+
+    assert len(tests_by_name) == 3
+    for test in test_results:
+        assert test["result"] == "passed"
+
+    assert tests_by_name["test_with_multiple_tags"]["tags"] == {
+        "language.version": "3.12",
+        "team": "backend",
+    }
+    assert tests_by_name["test_with_single_tag"]["tags"] == {"team": "frontend"}
+    assert "tags" not in tests_by_name["test_without_tags"]