diff --git a/packages/selenium-devtools-py/README.md b/packages/selenium-devtools-py/README.md index 5a1f2f67..f0108258 100644 --- a/packages/selenium-devtools-py/README.md +++ b/packages/selenium-devtools-py/README.md @@ -120,6 +120,24 @@ or a local resolves, an attribute or a call does not, because evaluating `driver.current_url` again would issue another WebDriver command. Those rows carry the condition and the error without values. +### Parallel runs (`pytest -n`) + +**pytest-xdist works with no extra configuration.** Every process reporting into +one run has to agree on a run id, or the backend treats each connect as a new run +and wipes what the previous one captured. With xdist they do agree: the plugin +loads in the **controller** as well, and enabling capture there resolves the id +before xdist spawns any worker — workers are child processes, so they inherit it. + +Measured with the real plugin against a real backend: `-n 2` and `-n 4` gave 3 +and 5 processes and **one** run id, with the backend seeing three worker connects +all carrying it. This is where the JS adapters differ — jest/vitest workers and +nightwatch `test_workers` load their plugin per worker with no launcher-side +hook, so each reads as its own run. + +What still reads as separate runs genuinely is: two independent `pytest` +invocations, or a worker started without the environment. Export +`DEVTOOLS_RUN_ID` yourself to join such processes into one run. + ## Dashboard window lifecycle Like the JS adapters, `enable()` opens the dashboard in a dedicated, closable diff --git a/packages/selenium-devtools-py/pyproject.toml b/packages/selenium-devtools-py/pyproject.toml index 1ce3aa3c..eff2f6be 100644 --- a/packages/selenium-devtools-py/pyproject.toml +++ b/packages/selenium-devtools-py/pyproject.toml @@ -30,7 +30,12 @@ dependencies = ["selenium>=4.44"] [project.optional-dependencies] # Test-only additions. selenium is not repeated: it is a hard dependency above, # so a second declaration here could drift from it. -test = ["pytest>=7"] +# +# pytest-xdist is not optional for CI: tests/test_xdist_run_identity.py skips +# without it, and that file is the only check that parallel workers share one +# run id. Skipping is silent, so omitting it leaves the guarantee unverified +# exactly where it is meant to be enforced. +test = ["pytest>=7", "pytest-xdist>=3"] # Auto-discovered by pytest; inert unless DEVTOOLS_ENABLE / DEVTOOLS_PORT is set. [project.entry-points.pytest11] diff --git a/packages/selenium-devtools-py/src/selenium_devtools/run_id.py b/packages/selenium-devtools-py/src/selenium_devtools/run_id.py index ef2d3948..caf692a7 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/run_id.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/run_id.py @@ -32,11 +32,21 @@ def resolve_run_id() -> str: whatever the environment still holds, and reusing the id would have the backend keep the finished run's commands, logs, network data and baselines. - That is the known limit for multi-process pytest (xdist): the plugin loads - per worker with no launcher-side hook to stamp first, so siblings disagree. - Deriving a fallback from the parent pid would group them, but would also make - two sequential single-process runs share an id and inherit each other's - state — which is why the per-process fallback stands. Tracked as issue #297. + **pytest-xdist needs nothing extra, measured rather than assumed** (#297). + The plugin loads in the CONTROLLER as well, and its ``pytest_configure`` + resolves the id — via ``enable()`` opening the socket — before xdist spawns + any worker. execnet spawns workers as child processes, so they inherit this + variable and adopt it. Measured with the real plugin against a real backend: + ``-n 2`` and ``-n 4`` produced 3 and 5 processes and **one** run id, and the + backend saw three worker connects all carrying it. The controller is the + launcher the JS adapters lack, which is why their equivalent gap (jest and + vitest workers, nightwatch ``test_workers``) does not apply here. + + What still reads as separate runs is genuinely separate: two independent + ``pytest`` invocations, or any worker whose environment does not carry the + variable. Deriving a fallback from the parent pid would group those, but + would also make two sequential single-process runs share an id and inherit + each other's state, so the per-process fallback stands. """ global _run_ended existing = os.environ.get(ENV_RUN_ID) diff --git a/packages/selenium-devtools-py/tests/test_run_id.py b/packages/selenium-devtools-py/tests/test_run_id.py index d7096d06..0ee8fb9f 100644 --- a/packages/selenium-devtools-py/tests/test_run_id.py +++ b/packages/selenium-devtools-py/tests/test_run_id.py @@ -5,6 +5,7 @@ """ import os +import pathlib import unittest from unittest import mock @@ -44,11 +45,12 @@ def test_it_is_stable_within_a_process(self): self.assertEqual(resolve_run_id(), resolve_run_id()) def test_two_processes_that_both_start_cold_disagree(self): - # The known limit, and why it is a limit rather than a bug: with no - # launcher-side hook to stamp first, xdist workers each generate their - # own. Deriving one from the parent pid would group siblings but would - # also make two sequential runs share an id and inherit each other's - # state. Tracked as #297. + # Two processes with no shared environment are genuinely separate runs — + # two independent `pytest` invocations, say. NOT the xdist case: those + # workers inherit the controller's variable and agree (measured, #297). + # Deriving an id from the parent pid would group cold siblings but would + # also make two sequential runs share one and inherit each other's + # state. first = resolve_run_id() os.environ.pop(ENV_RUN_ID, None) # a sibling with a cold environment @@ -108,6 +110,31 @@ def test_reset_after_adopting_then_generating_clears_only_ours(self): self.assertIsNone(os.environ.get(ENV_RUN_ID)) self.assertNotEqual(mine, "inherited") + def test_a_child_process_inherits_the_id_through_the_environment(self): + """The property pytest-xdist relies on, pinned (#297). + + xdist spawns workers as child processes, so they inherit this variable + and adopt the controller's id — which is why parallel pytest needs no + launcher hook. Asserted through a real subprocess rather than by reading + the code, because "the environment propagates" is the whole claim. + """ + import subprocess + import sys + import textwrap + + parent_id = resolve_run_id() + child = subprocess.run( + [sys.executable, "-c", textwrap.dedent(""" + import sys + sys.path.insert(0, %r) + from selenium_devtools.run_id import resolve_run_id + print(resolve_run_id()) + """) % str(pathlib.Path(__file__).resolve().parents[1] / "src")], + capture_output=True, text=True, env=os.environ.copy(), + ) + + self.assertEqual(child.stdout.strip(), parent_id, child.stderr) + def test_the_env_var_is_the_one_the_js_side_publishes(self): # Generated from shared's RUNNER_ENV, so a Python worker and a JS worker # in the same run agree rather than each inventing an identity. diff --git a/packages/selenium-devtools-py/tests/test_xdist_run_identity.py b/packages/selenium-devtools-py/tests/test_xdist_run_identity.py new file mode 100644 index 00000000..1f7af58e --- /dev/null +++ b/packages/selenium-devtools-py/tests/test_xdist_run_identity.py @@ -0,0 +1,120 @@ +"""Run identity across a real pytest-xdist run. + +The guarantee is not "the environment propagates" — it is an ORDERING: whatever +resolves the id in the controller's ``pytest_configure`` does so before xdist +spawns any worker, so the workers inherit it. A subprocess test cannot see that, +because it stamps the variable itself and then spawns something generic. + +So this drives real pytest with real xdist. The temporary project's conftest +deliberately does NOT resolve the id: a conftest's ``pytest_configure`` runs +BEFORE an entry-point plugin's, so resolving there would stamp the environment +earlier than the adapter does and the test would pass however late the adapter +resolved. The resolving is done by a plugin loaded with ``-p``, which is as close +to the adapter's own registration timing as a temp project can get. + +Skipped when pytest or pytest-xdist is absent; both are dev-only. +""" + +import importlib.util +import json +import subprocess +import sys +import tempfile +import textwrap +import unittest +from pathlib import Path + +_HAS_XDIST = all( + importlib.util.find_spec(name) is not None for name in ("pytest", "xdist") +) + +# Resolves the run id at the same lifecycle point the adapter does — its +# `pytest_configure`, which for the adapter is where `enable()` opens the socket. +PLUGIN = ''' +import json, os +from pathlib import Path + +from selenium_devtools._contract import ENV_RUN_ID +from selenium_devtools.run_id import resolve_run_id + +OUT = Path(__file__).parent / "observed" + + +def pytest_configure(config): + resolve_run_id() + + +def pytest_sessionfinish(session, exitstatus): + OUT.mkdir(exist_ok=True) + worker = os.environ.get("PYTEST_XDIST_WORKER", "controller") + (OUT / f"{worker}-{os.getpid()}.json").write_text(json.dumps({ + "worker": worker, "pid": os.getpid(), + "run_id": os.environ.get(ENV_RUN_ID), + })) +''' + +TESTS = """ +def test_a(): + assert True + + +def test_b(): + assert True + + +def test_c(): + assert True + + +def test_d(): + assert True +""" + + +@unittest.skipUnless(_HAS_XDIST, "pytest-xdist is not installed") +class TestEveryWorkerSharesOneRunId(unittest.TestCase): + def _run(self, *extra): + src = str(Path(__file__).resolve().parents[1] / "src") + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + (root / "conftest.py").write_text("") # must not resolve; see docstring + (root / "runid_plugin.py").write_text(textwrap.dedent(PLUGIN)) + (root / "test_parallel.py").write_text(TESTS) + proc = subprocess.run( + [sys.executable, "-m", "pytest", "-p", "no:cacheprovider", + "-p", "runid_plugin", "-q", *extra], + cwd=root, capture_output=True, text=True, + env={"PATH": "/usr/bin:/bin", "PYTHONPATH": f"{src}:{root}"}, + ) + observed = [ + json.loads(p.read_text()) + for p in (root / "observed").glob("*.json") + ] + return proc, observed + + def test_two_workers_and_the_controller_agree(self): + proc, observed = self._run("-n", "2") + + self.assertIn("passed", proc.stdout, proc.stdout + proc.stderr) + # Controller plus two workers — if only one process reported, xdist did + # not run and this would pass for the wrong reason. + self.assertGreaterEqual(len(observed), 3, observed) + self.assertEqual(len({row["run_id"] for row in observed}), 1, observed) + + def test_four_workers_agree(self): + proc, observed = self._run("-n", "4") + + self.assertIn("passed", proc.stdout, proc.stdout + proc.stderr) + self.assertGreaterEqual(len(observed), 5, observed) + self.assertEqual(len({row["run_id"] for row in observed}), 1, observed) + + def test_the_workers_are_really_separate_processes(self): + # Guards the guard: one id across one process proves nothing. + _, observed = self._run("-n", "2") + + self.assertGreaterEqual(len({row["pid"] for row in observed}), 3, observed) + self.assertIn("gw0", {row["worker"] for row in observed}, observed) + + +if __name__ == "__main__": + unittest.main()