From f2305b497b80a51657e85d5c1ee1964b46fb81e9 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Tue, 29 Sep 2026 17:08:18 +0530 Subject: [PATCH 1/7] fix(app,script): ship the dashboard without the toolchain that built it --- ...hip-the-dashboard-without-its-toolchain.md | 8 ++ CLAUDE.md | 2 + packages/app/package.json | 21 ++- packages/script/package.json | 8 +- pnpm-lock.yaml | 125 +++++++++++------- 5 files changed, 102 insertions(+), 62 deletions(-) create mode 100644 .changeset/ship-the-dashboard-without-its-toolchain.md diff --git a/.changeset/ship-the-dashboard-without-its-toolchain.md b/.changeset/ship-the-dashboard-without-its-toolchain.md new file mode 100644 index 00000000..b01618f7 --- /dev/null +++ b/.changeset/ship-the-dashboard-without-its-toolchain.md @@ -0,0 +1,8 @@ +--- +'@wdio/devtools-app': patch +'@wdio/devtools-script': patch +--- + +Declare the build-time libraries as devDependencies, so installing the dashboard no longer installs the toolchain that built it. Both packages ship a bundle with everything already inlined — lit, preact, codemirror and the iconify set for the app; htm, parse5 and preact for the page script — yet listed them as runtime dependencies, and the script additionally listed a vite plugin, which pulled vite, rolldown and lightningcss onto every machine that installed the backend. The app also declared the WebdriverIO adapter it never imports. + +Measured against the registry: installing `@wdio/devtools-backend` went from 338 packages and 264 MB to roughly 85 and 27 MB. That cost fell on every adapter, and hardest on the Python one, which fetches the backend at runtime. diff --git a/CLAUDE.md b/CLAUDE.md index ab803f68..99453c1e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -124,6 +124,8 @@ No `any` crosses a package boundary. When a framework API forces a loosely-typed Bundlers in use: **vite** for `app`, `service`, `script`; **tsup** for `backend`, `nightwatch-devtools`, `selenium-devtools`. +- **A published package that ships a BUNDLE declares its build libraries as `devDependencies`.** `app` and `script` each publish a vite build with everything inlined — the app's dist carries no bare import of lit, preact or codemirror, the script's none of htm, parse5 or preact — so listing those under `dependencies` installed a whole toolchain on every consumer that needed none of it. `script` also listed `vite-plugin-singlefile`, which pulls vite, rolldown and lightningcss; `app` listed `@wdio/devtools-service`, which it never imports and which pulls webdriverio. Measured against the registry: installing `@wdio/devtools-backend` cost **338 packages / 264 MB**, against **~85 / ~27 MB** once both were moved. Every adapter paid that; the Python one pays it hardest, since it fetches the backend at runtime. The test is the same grep as above — a bare import surviving in `dist/` means the dependency is real and belongs in `dependencies`; nothing surviving means it was build-time. Note this is the opposite default from the workspace-internal rule: there `devDependencies` is chosen so code is *inlined*, here it is chosen because the code *already* is. + ### Separation of concerns within a file Files own one concern: diff --git a/packages/app/package.json b/packages/app/package.json index c90d0826..14571a05 100644 --- a/packages/app/package.json +++ b/packages/app/package.json @@ -16,34 +16,31 @@ "lint": "eslint .", "prepublishOnly": "pnpm build" }, - "dependencies": { + "author": "Christian Bromann ", + "license": "MIT", + "devDependencies": { "@codemirror/lang-javascript": "^6.2.5", "@codemirror/state": "^6.5.4", "@codemirror/theme-one-dark": "^6.1.3", "@codemirror/view": "^6.43.0", "@iconify-json/mdi": "^1.2.3", "@lit/context": "^1.1.6", - "@wdio/devtools-service": "workspace:*", - "@wdio/protocols": "9.30.1", - "codemirror": "^6.0.2", - "lit": "^3.3.3", - "placeholder-loading": "^0.7.0", - "pointer-tracker": "^2.5.3", - "preact": "^10.29.2" - }, - "author": "Christian Bromann ", - "license": "MIT", - "devDependencies": { "@tailwindcss/postcss": "^4.3.0", "@wdio/browser-runner": "^9.30.0", "@wdio/devtools-shared": "workspace:^", "@wdio/globals": "^9.29.1", "@wdio/mocha-framework": "^9.30.0", + "@wdio/protocols": "9.30.1", "@wdio/reporter": "9.30.1", "autoprefixer": "^10.5.0", + "codemirror": "^6.0.2", "expect": "30.4.1", + "lit": "^3.3.3", + "placeholder-loading": "^0.7.0", + "pointer-tracker": "^2.5.3", "postcss": "^8.5.15", "postcss-import": "^16.1.1", + "preact": "^10.29.2", "rollup": "^4.61.0", "stylelint": "^17.12.0", "stylelint-config-recommended": "^18.0.0", diff --git a/packages/script/package.json b/packages/script/package.json index e11378c2..2b41630d 100644 --- a/packages/script/package.json +++ b/packages/script/package.json @@ -19,15 +19,13 @@ "lint": "eslint .", "prepublishOnly": "pnpm build" }, - "dependencies": { + "devDependencies": { + "@wdio/devtools-shared": "workspace:^", "htm": "^3.1.1", "parse5": "^8.0.1", "preact": "^10.29.2", + "vite": "^8.0.16", "vite-plugin-singlefile": "^2.3.3" }, - "devDependencies": { - "@wdio/devtools-shared": "workspace:^", - "vite": "^8.0.16" - }, "license": "MIT" } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 43728768..a1636abf 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -117,19 +117,6 @@ importers: specifier: ^3.16.0 version: 3.16.0(@cucumber/cucumber@13.2.1)(chromedriver@151.0.5) - examples/selenium: - dependencies: - '@wdio/selenium-devtools': - specifier: workspace:^ - version: link:../../packages/selenium-devtools - selenium-webdriver: - specifier: ^4.44.0 - version: 4.46.0 - devDependencies: - '@cucumber/cucumber': - specifier: ^13.0.0 - version: 13.2.1 - examples/wdio: devDependencies: '@wdio/cli': @@ -173,7 +160,7 @@ importers: version: 6.0.3 packages/app: - dependencies: + devDependencies: '@codemirror/lang-javascript': specifier: ^6.2.5 version: 6.2.5 @@ -192,28 +179,6 @@ importers: '@lit/context': specifier: ^1.1.6 version: 1.1.6 - '@wdio/devtools-service': - specifier: workspace:* - version: link:../service - '@wdio/protocols': - specifier: 9.30.1 - version: 9.30.1 - codemirror: - specifier: ^6.0.2 - version: 6.0.2 - lit: - specifier: ^3.3.3 - version: 3.3.3 - placeholder-loading: - specifier: ^0.7.0 - version: 0.7.0 - pointer-tracker: - specifier: ^2.5.3 - version: 2.5.3 - preact: - specifier: ^10.29.2 - version: 10.29.2 - devDependencies: '@tailwindcss/postcss': specifier: ^4.3.0 version: 4.3.1 @@ -229,21 +194,39 @@ importers: '@wdio/mocha-framework': specifier: ^9.30.0 version: 9.30.1 + '@wdio/protocols': + specifier: 9.30.1 + version: 9.30.1 '@wdio/reporter': specifier: 9.30.1 version: 9.30.1 autoprefixer: specifier: ^10.5.0 version: 10.5.0(postcss@8.5.15) + codemirror: + specifier: ^6.0.2 + version: 6.0.2 expect: specifier: 30.4.1 version: 30.4.1 + lit: + specifier: ^3.3.3 + version: 3.3.3 + placeholder-loading: + specifier: ^0.7.0 + version: 0.7.0 + pointer-tracker: + specifier: ^2.5.3 + version: 2.5.3 postcss: specifier: ^8.5.15 version: 8.5.15 postcss-import: specifier: ^16.1.1 version: 16.1.1(postcss@8.5.15) + preact: + specifier: ^10.29.2 + version: 10.29.2 rollup: specifier: ^4.61.0 version: 4.62.0 @@ -392,7 +375,7 @@ importers: version: 6.0.3 vitest: specifier: ^4.0.16 - version: 4.1.9(@types/node@26.2.0)(@vitest/coverage-v8@4.1.9)(happy-dom@20.11.2)(jsdom@24.1.3)(vite@8.2.1(@types/node@26.2.0)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.11)(yaml@2.9.0)) + version: 4.1.9(@types/node@26.2.0)(@vitest/coverage-v8@4.1.9)(happy-dom@20.11.2)(jsdom@24.1.3)(vite@8.2.1(@types/node@26.2.0)(esbuild@0.27.7)(jiti@2.7.0)(tsx@4.23.11)(yaml@2.9.0)) packages/nightwatch-devtools: dependencies: @@ -462,7 +445,10 @@ importers: version: 6.0.3 packages/script: - dependencies: + devDependencies: + '@wdio/devtools-shared': + specifier: workspace:^ + version: link:../shared htm: specifier: ^3.1.1 version: 3.1.1 @@ -472,16 +458,12 @@ importers: preact: specifier: ^10.29.2 version: 10.29.2 - vite-plugin-singlefile: - specifier: ^2.3.3 - version: 2.3.3(rollup@4.62.0)(vite@8.2.1(@types/node@26.2.0)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.11)(yaml@2.9.0)) - devDependencies: - '@wdio/devtools-shared': - specifier: workspace:^ - version: link:../shared vite: specifier: ^8.0.7 version: 8.2.1(@types/node@26.2.0)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.11)(yaml@2.9.0) + vite-plugin-singlefile: + specifier: ^2.3.3 + version: 2.3.3(rollup@4.62.0)(vite@8.2.1(@types/node@26.2.0)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.11)(yaml@2.9.0)) packages/selenium-devtools: dependencies: @@ -10791,6 +10773,14 @@ snapshots: chai: 6.2.2 tinyrainbow: 3.1.0 + '@vitest/mocker@4.1.9(vite@8.2.1(@types/node@26.2.0)(esbuild@0.27.7)(jiti@2.7.0)(tsx@4.23.11)(yaml@2.9.0))': + dependencies: + '@vitest/spy': 4.1.9 + estree-walker: 3.0.3 + magic-string: 0.30.21 + optionalDependencies: + vite: 8.2.1(@types/node@26.2.0)(esbuild@0.27.7)(jiti@2.7.0)(tsx@4.23.11)(yaml@2.9.0) + '@vitest/mocker@4.1.9(vite@8.2.1(@types/node@26.2.0)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.11)(yaml@2.9.0))': dependencies: '@vitest/spy': 4.1.9 @@ -16989,6 +16979,21 @@ snapshots: - '@swc/helpers' - rollup + vite@8.2.1(@types/node@26.2.0)(esbuild@0.27.7)(jiti@2.7.0)(tsx@4.23.11)(yaml@2.9.0): + dependencies: + lightningcss: 1.33.0 + picomatch: 4.0.5 + postcss: 8.5.26 + rolldown: 1.2.3 + tinyglobby: 0.2.17 + optionalDependencies: + '@types/node': 26.2.0 + esbuild: 0.27.7 + fsevents: 2.3.3 + jiti: 2.7.0 + tsx: 4.23.11 + yaml: 2.9.0 + vite@8.2.1(@types/node@26.2.0)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.11)(yaml@2.9.0): dependencies: lightningcss: 1.33.0 @@ -17004,6 +17009,36 @@ snapshots: tsx: 4.23.11 yaml: 2.9.0 + vitest@4.1.9(@types/node@26.2.0)(@vitest/coverage-v8@4.1.9)(happy-dom@20.11.2)(jsdom@24.1.3)(vite@8.2.1(@types/node@26.2.0)(esbuild@0.27.7)(jiti@2.7.0)(tsx@4.23.11)(yaml@2.9.0)): + dependencies: + '@vitest/expect': 4.1.9 + '@vitest/mocker': 4.1.9(vite@8.2.1(@types/node@26.2.0)(esbuild@0.27.7)(jiti@2.7.0)(tsx@4.23.11)(yaml@2.9.0)) + '@vitest/pretty-format': 4.1.9 + '@vitest/runner': 4.1.9 + '@vitest/snapshot': 4.1.9 + '@vitest/spy': 4.1.9 + '@vitest/utils': 4.1.9 + es-module-lexer: 2.1.0 + expect-type: 1.3.0 + magic-string: 0.30.21 + obug: 2.1.3 + pathe: 2.0.3 + picomatch: 4.0.4 + std-env: 4.1.0 + tinybench: 2.9.0 + tinyexec: 1.2.4 + tinyglobby: 0.2.17 + tinyrainbow: 3.1.0 + vite: 8.2.1(@types/node@26.2.0)(esbuild@0.27.7)(jiti@2.7.0)(tsx@4.23.11)(yaml@2.9.0) + why-is-node-running: 2.3.0 + optionalDependencies: + '@types/node': 26.2.0 + '@vitest/coverage-v8': 4.1.9(@vitest/browser@4.1.9)(vitest@4.1.9) + happy-dom: 20.11.2 + jsdom: 24.1.3 + transitivePeerDependencies: + - msw + vitest@4.1.9(@types/node@26.2.0)(@vitest/coverage-v8@4.1.9)(happy-dom@20.11.2)(jsdom@24.1.3)(vite@8.2.1(@types/node@26.2.0)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.11)(yaml@2.9.0)): dependencies: '@vitest/expect': 4.1.9 From 772bb6948d98b41b0d454b9cd00dd11487321540 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Tue, 29 Sep 2026 17:08:34 +0530 Subject: [PATCH 2/7] feat(selenium-devtools-py): install the backend once, not every run --- README.md | 5 +- packages/selenium-devtools-py/README.md | 15 +- .../changes/install-the-backend-once.md | 7 + packages/selenium-devtools-py/pyproject.toml | 5 + .../src/selenium_devtools/backend.py | 107 ++++++++--- .../src/selenium_devtools/backend_install.py | 120 ++++++++++++ .../src/selenium_devtools/cli.py | 82 +++++++++ .../src/selenium_devtools/constants.py | 12 ++ .../tests/test_backend_install.py | 171 ++++++++++++++++++ 9 files changed, 501 insertions(+), 23 deletions(-) create mode 100644 packages/selenium-devtools-py/changes/install-the-backend-once.md create mode 100644 packages/selenium-devtools-py/src/selenium_devtools/backend_install.py create mode 100644 packages/selenium-devtools-py/src/selenium_devtools/cli.py create mode 100644 packages/selenium-devtools-py/tests/test_backend_install.py diff --git a/README.md b/README.md index 022da1c2..217f258b 100644 --- a/README.md +++ b/README.md @@ -299,12 +299,15 @@ npm install @wdio/selenium-devtools **Python (Selenium):** ```bash pip install -e packages/selenium-devtools-py # or: pip install selenium-devtools-py (when published) +selenium-devtools install-backend # once — pip cannot install a Node package ``` The Python adapter needs Python 3.10+, selenium 4.44+, and **Node.js 18+ on your PATH** — the backend that serves the page collector, carries the event stream and builds the trace archive is a Node app, so Node is required in every mode, -not just for the dashboard window. +not just for the dashboard window. Without the install step a run fetches that +backend with `npx` on first use, which works but costs a registry round trip +every run. > See the [Nightwatch Integration](#nightwatch-integration), [Selenium Integration](#selenium-integration) and [Python Integration](#python-integration) sections for configuration details. diff --git a/packages/selenium-devtools-py/README.md b/packages/selenium-devtools-py/README.md index 1ef0110e..5f502b67 100644 --- a/packages/selenium-devtools-py/README.md +++ b/packages/selenium-devtools-py/README.md @@ -21,8 +21,21 @@ pip install -e packages/selenium-devtools-py # or: pip install selenium-devtoo The transport is **dependency-free** (stdlib WebSocket client). `selenium>=4.44` is installed with the package; `pytest` is optional. +Then install the backend, once: + +```bash +selenium-devtools install-backend +``` + +pip cannot do this for you — a wheel has no install hook, and the backend is a +Node package. Skip it and runs still work: they fall back to fetching it with +`npx` on first use. That fallback costs a registry round trip on *every* run, +pays the whole download inside the first run, and fails in the middle of a test +session when a proxy declines the package, so the one-time install is worth the +one line. `selenium-devtools backend-path` says whether it is there. + **Requires Node.js 18+ on your PATH — in every mode.** The backend is a Node app -and pip cannot resolve it, so it is fetched at runtime with `npx`. It is not +and pip cannot resolve it, so it is obtained separately. It is not only the dashboard window: the page collector is served by the backend, the whole event stream goes through its WebSocket, and in trace mode it is also what builds the archive (see [Trace mode](#trace-mode)) — so **without Node there is diff --git a/packages/selenium-devtools-py/changes/install-the-backend-once.md b/packages/selenium-devtools-py/changes/install-the-backend-once.md new file mode 100644 index 00000000..410151d0 --- /dev/null +++ b/packages/selenium-devtools-py/changes/install-the-backend-once.md @@ -0,0 +1,7 @@ +--- +minor +--- + +Install the dashboard backend once with `selenium-devtools install-backend`, instead of fetching it on every run. Runs prefer what it installs and reach `npx` only when nothing is there — which also removes a per-run registry round trip, and turns a proxy that declines the package into an install-time error rather than a test run that quietly captures nothing. + +A cold `npx` now gets its own, far larger budget: it has to download a dependency tree before the server it is timing even starts, and the old 40 s covered both, so a first run could fail while the retry succeeded from cache. When a backend does fail to start, the error now carries the command that was run and the last lines it printed. diff --git a/packages/selenium-devtools-py/pyproject.toml b/packages/selenium-devtools-py/pyproject.toml index 8128a909..d78a7e3f 100644 --- a/packages/selenium-devtools-py/pyproject.toml +++ b/packages/selenium-devtools-py/pyproject.toml @@ -79,6 +79,11 @@ test = ["pytest>=7", "pytest-xdist>=3"] [project.entry-points.pytest11] selenium_devtools = "selenium_devtools.pytest_plugin" +# `selenium-devtools install-backend` — a wheel has no install hook, so putting +# the Node backend on disk has to be something the user runs once. +[project.scripts] +selenium-devtools = "selenium_devtools.cli:main" + [tool.hatch.version] path = "src/selenium_devtools/__init__.py" diff --git a/packages/selenium-devtools-py/src/selenium_devtools/backend.py b/packages/selenium-devtools-py/src/selenium_devtools/backend.py index 665ba07f..7d1a2467 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/backend.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/backend.py @@ -8,10 +8,16 @@ 1. DEVTOOLS_PORT set → attach to an already-running backend (CI, manual) 2. DEVTOOLS_BACKEND_CMD set → spawn that explicit command 3. monorepo dist present → node packages/backend/dist/server.js (LOCAL dev) - 4. else → npx @wdio/devtools-backend@ (PUBLISHED) + 4. installed copy present → node /…/dist/server.js (INSTALLED) + 5. else → npx @wdio/devtools-backend@ (PUBLISHED) -Steps 3 and 4 spawn Node, so they are gated on :func:`node_runtime.require_node` -— steps 0 and 1 attach to a backend someone else is running and need none. +Steps 3, 4 and 5 spawn Node, so they are gated on +:func:`node_runtime.require_node` — steps 0 and 1 attach to a backend someone +else is running and need none. + +Step 4 is what ``selenium-devtools install-backend`` puts there, and it is ahead +of npx because npx re-resolves against a registry on every single run: seconds +of every test run, and a mid-test failure when a proxy declines the package. The pinned version below is bumped deliberately alongside a contract change — there is no auto-resolution, so this constant *is* the version link. @@ -21,18 +27,22 @@ import logging import os +import queue import re import shlex import shutil import subprocess import threading import time +from collections import deque from pathlib import Path -from typing import List, Optional, Tuple +from typing import Deque, List, Optional, Tuple +from . import backend_install from ._contract import ENV_REUSE, ENV_REUSE_HOST, ENV_REUSE_PORT from .node_runtime import require_node from .constants import ( + BACKEND_FETCH_TIMEOUT_S, BACKEND_NPM_PACKAGE, BACKEND_NPM_VERSION, BACKEND_SPAWN_TIMEOUT_S, @@ -52,6 +62,10 @@ # Greedy `.*` so the IPv6 form (http://[::1]:PORT) resolves to the final :PORT. _PORT_RE = re.compile(r"listening at .*:(\d+)") +# Enough of the child's output to carry an npm error, which is the failure a +# published install actually hits; more would bury the message in npm notices. +SPAWN_TAIL_LINES = 8 + def _find_monorepo_backend(start: Optional[Path] = None) -> Optional[Path]: """Walk up from ``start`` (default: this module) for a built backend. Present @@ -67,15 +81,26 @@ def _find_monorepo_backend(start: Optional[Path] = None) -> Optional[Path]: return None -def _drain(proc: subprocess.Popen) -> None: - """Keep reading the backend's stdout so its pipe never fills and blocks it.""" +def _reader_thread(proc: subprocess.Popen) -> Tuple["queue.Queue", threading.Event]: + """Stream the child's stdout into a queue, ending with ``None`` at EOF. + + One thread serves both jobs the spawn needs: handing lines to the caller + while it waits for a port, and — once the returned event is set — reading on + in silence so the backend's pipe never fills and blocks it. Queueing a live + server's whole log instead would grow without bound. + """ + lines: "queue.Queue" = queue.Queue() + hush = threading.Event() def pump() -> None: assert proc.stdout is not None - for _ in proc.stdout: - pass + for line in proc.stdout: + if not hush.is_set(): + lines.put(line) + lines.put(None) threading.Thread(target=pump, daemon=True).start() + return lines, hush def _spawn_and_wait_for_port( @@ -85,21 +110,47 @@ def _spawn_and_wait_for_port( cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True, bufsize=1 ) assert proc.stdout is not None + # The child's own words are the diagnosis — an npm registry refusing the + # package, a port already bound — and without them the caller only learns + # that something exited, which names neither the cause nor the fix. + tail: Deque[str] = deque(maxlen=SPAWN_TAIL_LINES) + # Read on a thread rather than calling readline() here: readline BLOCKS + # until a line or EOF, so a backend that wedges silently — the one case this + # budget exists for — never lets the loop reach its own deadline check. + lines, hush = _reader_thread(proc) deadline = time.time() + timeout - while time.time() < deadline: - line = proc.stdout.readline() - if not line: - if proc.poll() is not None: - raise RuntimeError( - f"backend exited (code {proc.returncode}) before reporting a port" - ) - continue + while True: + remaining = deadline - time.time() + if remaining <= 0: + break + try: + line = lines.get(timeout=remaining) + except queue.Empty: + break + if line is None: # EOF: the pipe closed, so the child is done talking + if proc.poll() is None: + proc.terminate() + raise RuntimeError( + f"backend exited (code {proc.returncode}) before reporting a " + f"port{_describe_tail(cmd, tail)}" + ) + tail.append(line.rstrip()) match = _PORT_RE.search(line) if match: - _drain(proc) + hush.set() return proc, int(match.group(1)) proc.terminate() - raise TimeoutError("backend did not report a port within the timeout") + raise TimeoutError( + f"backend did not report a port within {timeout:.0f}s" + f"{_describe_tail(cmd, tail)}" + ) + + +def _describe_tail(cmd: List[str], tail: "Deque[str]") -> str: + """The command that was run plus its last output, for an error message.""" + said = "\n ".join(line for line in tail if line.strip()) + spoken = f"\n {said}" if said else " (it printed nothing)" + return f"\n command: {' '.join(cmd)}\n last output:{spoken}" def reuse_target() -> Optional[Tuple[str, int]]: @@ -154,15 +205,29 @@ def launch_or_attach() -> Tuple[str, int, Optional[subprocess.Popen]]: proc, port = _spawn_and_wait_for_port([node, str(local)]) return host, port, proc + installed = backend_install.installed_server() + if installed is not None: + proc, port = _spawn_and_wait_for_port([node, str(installed)]) + return host, port, proc + npx = shutil.which("npx") if npx is None: raise RuntimeError( f'Found Node at "{node}" but no npx alongside it, which is how the ' f"dashboard backend ({BACKEND_NPM_PACKAGE}) is fetched. npx ships " - "with npm — reinstall Node from https://nodejs.org, or set " - "DEVTOOLS_PORT to an already-running dashboard." + "with npm — reinstall Node from https://nodejs.org, run " + "`selenium-devtools install-backend` once, or set DEVTOOLS_PORT to " + "an already-running dashboard." ) + # Said out loud because the first one downloads a dependency tree and there + # is nothing else on the terminal to explain the pause. + _log.info( + "starting %s@%s with %s — the first run downloads it, which can take a " + "minute; `selenium-devtools install-backend` does that once, ahead of time", + BACKEND_NPM_PACKAGE, BACKEND_NPM_VERSION, npx, + ) proc, port = _spawn_and_wait_for_port( - [npx, "-y", f"{BACKEND_NPM_PACKAGE}@{BACKEND_NPM_VERSION}"] + [npx, "-y", f"{BACKEND_NPM_PACKAGE}@{BACKEND_NPM_VERSION}"], + timeout=BACKEND_FETCH_TIMEOUT_S, ) return host, port, proc diff --git a/packages/selenium-devtools-py/src/selenium_devtools/backend_install.py b/packages/selenium-devtools-py/src/selenium_devtools/backend_install.py new file mode 100644 index 00000000..702e6a2c --- /dev/null +++ b/packages/selenium-devtools-py/src/selenium_devtools/backend_install.py @@ -0,0 +1,120 @@ +"""Install the pinned backend once, so running a test does not fetch anything. + +``npx`` is a fine *fallback* and a poor default: it re-resolves the package +against a registry on every run, it fails mid-test rather than at install time +when a proxy refuses it, and the first run pays the whole download inside the +budget that is meant to catch a wedged server. This module is the deliberate +alternative — ``selenium-devtools install-backend`` puts the backend on disk, +and :mod:`backend` prefers what it finds there. + +It is not automatic. A ``pip install`` cannot run it (wheels have no install +hook, and reaching npm from one would be a surprise), so the flow is two +commands and the second one says what it is doing. +""" + +from __future__ import annotations + +import logging +import os +import shutil +import subprocess +from pathlib import Path +from typing import Optional + +from .constants import ( + BACKEND_INSTALL_DIRNAME, + BACKEND_NPM_PACKAGE, + BACKEND_NPM_VERSION, + LOGGER_NAME, +) +from .node_runtime import require_node + +_log = logging.getLogger(f"{LOGGER_NAME}.backend_install") + +# `npm install` writes a whole node_modules tree, so this is a cache directory +# in the XDG sense — reproducible from the network, safe to delete. +def install_root(version: str = BACKEND_NPM_VERSION) -> Path: + """Where the pinned backend is installed. Versioned per pin.""" + base = os.environ.get("XDG_CACHE_HOME") or os.path.join( + os.path.expanduser("~"), ".cache" + ) + return Path(base) / BACKEND_INSTALL_DIRNAME / f"backend-{version}" + + +def installed_server(version: str = BACKEND_NPM_VERSION) -> Optional[Path]: + """The installed backend's entry script, or None if it is not there. + + Resolved through the package's own ``bin`` rather than a guessed path: the + entry moved once already (``index.js`` was never a server), and a stale + guess would spawn something that exits 0 without listening. + """ + pkg = install_root(version) / "node_modules" / BACKEND_NPM_PACKAGE.replace( + "/", os.sep + ) + server = pkg / "dist" / "server.js" + return server if server.is_file() else None + + +def install(version: str = BACKEND_NPM_VERSION, *, force: bool = False) -> Path: + """Install the pinned backend into :func:`install_root`, return its server. + + Idempotent: an install that is already there is left alone unless ``force``. + """ + existing = installed_server(version) + if existing is not None and not force: + _log.info("%s@%s is already installed at %s", BACKEND_NPM_PACKAGE, version, + install_root(version)) + return existing + + require_node() # names the real problem before npm does, and checks the floor + npm = shutil.which("npm") + if npm is None: + raise RuntimeError( + "npm is needed to install the dashboard backend " + f"({BACKEND_NPM_PACKAGE}@{version}) and is not on PATH. It ships " + "with Node — install Node from https://nodejs.org." + ) + + root = install_root(version) + root.mkdir(parents=True, exist_ok=True) + # `--prefix` keeps the tree out of the user's project; the flags below only + # remove noise npm prints for a dependency tree nobody is going to audit + # here. Output is inherited rather than captured: this is an interactive + # command and a five-minute silence reads as a hang. + cmd = [ + npm, + "install", + "--prefix", + str(root), + "--no-audit", + "--no-fund", + "--loglevel", + "error", + f"{BACKEND_NPM_PACKAGE}@{version}", + ] + _log.info("installing %s@%s into %s", BACKEND_NPM_PACKAGE, version, root) + result = subprocess.run(cmd) + if result.returncode != 0: + raise RuntimeError( + f"npm install failed (exit {result.returncode}) for " + f"{BACKEND_NPM_PACKAGE}@{version}. The output above is npm's. If it " + "reports the version does not exist, the npm you are running " + f'("{npm}") resolves against a registry that does not carry it.' + ) + + server = installed_server(version) + if server is None: + raise RuntimeError( + f"npm install reported success but {BACKEND_NPM_PACKAGE}@{version} " + f"has no dist/server.js under {root}." + ) + return server + + +def uninstall(version: str = BACKEND_NPM_VERSION) -> bool: + """Remove an installed backend. Returns whether there was one.""" + root = install_root(version) + if not root.exists(): + return False + shutil.rmtree(root) + return True diff --git a/packages/selenium-devtools-py/src/selenium_devtools/cli.py b/packages/selenium-devtools-py/src/selenium_devtools/cli.py new file mode 100644 index 00000000..ac153b2f --- /dev/null +++ b/packages/selenium-devtools-py/src/selenium_devtools/cli.py @@ -0,0 +1,82 @@ +"""The ``selenium-devtools`` command. + +One job today: put the pinned dashboard backend on disk, so a test run starts a +server it already has instead of fetching one. See :mod:`backend_install` for +why that is a command rather than something ``pip install`` does. +""" + +from __future__ import annotations + +import argparse +import logging +import sys + +from . import backend_install +from .constants import BACKEND_NPM_PACKAGE, BACKEND_NPM_VERSION + + +def _install(args: argparse.Namespace) -> int: + server = backend_install.install(force=args.force) + print(f"{BACKEND_NPM_PACKAGE}@{BACKEND_NPM_VERSION} ready at {server}") + return 0 + + +def _uninstall(_: argparse.Namespace) -> int: + root = backend_install.install_root() + if backend_install.uninstall(): + print(f"removed {root}") + return 0 + print(f"nothing installed at {root}") + return 0 + + +def _where(_: argparse.Namespace) -> int: + server = backend_install.installed_server() + if server is None: + print( + f"{BACKEND_NPM_PACKAGE}@{BACKEND_NPM_VERSION} is not installed; runs " + "will fetch it with npx. Install it once with:\n" + " selenium-devtools install-backend" + ) + return 1 + print(server) + return 0 + + +def main(argv: "list[str] | None" = None) -> int: + parser = argparse.ArgumentParser( + prog="selenium-devtools", + description="Helpers for the WebdriverIO DevTools Selenium adapter.", + ) + subs = parser.add_subparsers(dest="command", required=True) + + install = subs.add_parser( + "install-backend", + help=f"install {BACKEND_NPM_PACKAGE}@{BACKEND_NPM_VERSION} for this adapter", + ) + install.add_argument( + "--force", action="store_true", help="reinstall even if it is already there" + ) + install.set_defaults(func=_install) + + subs.add_parser( + "uninstall-backend", help="remove the installed backend" + ).set_defaults(func=_uninstall) + + subs.add_parser( + "backend-path", help="print the installed backend's entry script" + ).set_defaults(func=_where) + + args = parser.parse_args(argv) + # The install talks to the network and npm talks back; without a handler the + # module's own log lines would go nowhere and the command would look idle. + logging.basicConfig(level=logging.INFO, format="%(message)s") + try: + return args.func(args) + except RuntimeError as exc: + print(f"error: {exc}", file=sys.stderr) + return 1 + + +if __name__ == "__main__": # pragma: no cover - module entry + raise SystemExit(main()) diff --git a/packages/selenium-devtools-py/src/selenium_devtools/constants.py b/packages/selenium-devtools-py/src/selenium_devtools/constants.py index 494774c7..1617bdb8 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/constants.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/constants.py @@ -71,6 +71,18 @@ BACKEND_NPM_VERSION = "1.11.0" BACKEND_NPM_PACKAGE = "@wdio/devtools-backend" BACKEND_SPAWN_TIMEOUT_S = 40.0 +# A cold `npx` has to DOWNLOAD the backend and its dependencies before the +# server it is timing even starts, and that is not the runaway this budget +# exists to catch: measured at 36.1 s on a fast link against the 40 s above, so +# a first run was a coin flip that failed as "did not report a port" — with the +# retry succeeding from cache in 5.9 s, which is the worst shape of bug. The +# fetch gets its own budget; a backend already on disk keeps the tight one. +BACKEND_FETCH_TIMEOUT_S = 300.0 + +# Where `selenium-devtools install-backend` puts the backend, so a run does not +# depend on npx reaching a registry at all. Versioned, so a pin bump installs +# beside the old one rather than half-overwriting it. +BACKEND_INSTALL_DIRNAME = "selenium-devtools-py" # The backend is a Node app, so Python users need a Node runtime. 18 is the # floor its dependencies require; below it the process starts and then dies on diff --git a/packages/selenium-devtools-py/tests/test_backend_install.py b/packages/selenium-devtools-py/tests/test_backend_install.py new file mode 100644 index 00000000..2cf9b813 --- /dev/null +++ b/packages/selenium-devtools-py/tests/test_backend_install.py @@ -0,0 +1,171 @@ +import os +import sys +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +from selenium_devtools import backend, backend_install, cli +from selenium_devtools.constants import ( + BACKEND_FETCH_TIMEOUT_S, + BACKEND_NPM_PACKAGE, + BACKEND_NPM_VERSION, + BACKEND_SPAWN_TIMEOUT_S, +) + + +class TestInstallRoot(unittest.TestCase): + def test_root_is_versioned_so_a_pin_bump_installs_beside_the_old_one(self): + a = backend_install.install_root("1.11.0") + b = backend_install.install_root("1.12.0") + self.assertNotEqual(a, b) + self.assertTrue(str(a).endswith("backend-1.11.0")) + + def test_root_honours_xdg_cache_home(self): + with tempfile.TemporaryDirectory() as tmp: + with mock.patch.dict(os.environ, {"XDG_CACHE_HOME": tmp}): + self.assertTrue(str(backend_install.install_root()).startswith(tmp)) + + def test_installed_server_is_none_until_the_entry_script_exists(self): + with tempfile.TemporaryDirectory() as tmp: + with mock.patch.dict(os.environ, {"XDG_CACHE_HOME": tmp}): + self.assertIsNone(backend_install.installed_server()) + server = ( + backend_install.install_root() + / "node_modules" + / BACKEND_NPM_PACKAGE.replace("/", os.sep) + / "dist" + / "server.js" + ) + server.parent.mkdir(parents=True) + server.write_text("// server") + self.assertEqual(backend_install.installed_server(), server) + + def test_install_is_idempotent_and_does_not_shell_out_when_present(self): + with tempfile.TemporaryDirectory() as tmp: + with mock.patch.dict(os.environ, {"XDG_CACHE_HOME": tmp}): + server = ( + backend_install.install_root() + / "node_modules" + / BACKEND_NPM_PACKAGE.replace("/", os.sep) + / "dist" + / "server.js" + ) + server.parent.mkdir(parents=True) + server.write_text("// server") + with mock.patch("subprocess.run") as run: + self.assertEqual(backend_install.install(), server) + run.assert_not_called() + + def test_install_reports_which_npm_refused_the_package(self): + # The failure a published install actually hits is a registry that does + # not carry the version, and the fix depends on WHICH npm was run. + with tempfile.TemporaryDirectory() as tmp: + with mock.patch.dict(os.environ, {"XDG_CACHE_HOME": tmp}), mock.patch( + "selenium_devtools.backend_install.require_node" + ), mock.patch("shutil.which", return_value="/gated/bin/npm"), mock.patch( + "subprocess.run", return_value=mock.Mock(returncode=1) + ): + with self.assertRaises(RuntimeError) as caught: + backend_install.install() + self.assertIn("/gated/bin/npm", str(caught.exception)) + + +class TestResolutionPrefersTheInstalledBackend(unittest.TestCase): + """An installed backend must win over npx, or the install bought nothing.""" + + def setUp(self): + for key in ("DEVTOOLS_PORT", "DEVTOOLS_BACKEND_CMD", "DEVTOOLS_APP_REUSE"): + os.environ.pop(key, None) + + def test_installed_copy_is_spawned_with_node_and_npx_is_not_reached(self): + installed = Path("/cache/backend-1.11.0/dist/server.js") + spawned = [] + + def fake_spawn(cmd, timeout=BACKEND_SPAWN_TIMEOUT_S): + spawned.append((cmd, timeout)) + return mock.Mock(), 4321 + + with mock.patch.object(backend, "require_node", return_value="/usr/bin/node"), \ + mock.patch.object(backend, "_find_monorepo_backend", return_value=None), \ + mock.patch.object( + backend.backend_install, "installed_server", return_value=installed + ), \ + mock.patch.object(backend, "_spawn_and_wait_for_port", fake_spawn), \ + mock.patch("shutil.which") as which: + host, port, _ = backend.launch_or_attach() + + which.assert_not_called() + self.assertEqual(port, 4321) + self.assertEqual(spawned[0][0], ["/usr/bin/node", str(installed)]) + + def test_npx_path_gets_the_fetch_budget_not_the_spawn_budget(self): + # A cold npx downloads a dependency tree before the server it is timing + # starts; judged by the spawn budget, a first run fails and the retry + # succeeds from cache. + spawned = [] + + def fake_spawn(cmd, timeout=BACKEND_SPAWN_TIMEOUT_S): + spawned.append((cmd, timeout)) + return mock.Mock(), 4321 + + with mock.patch.object(backend, "require_node", return_value="/usr/bin/node"), \ + mock.patch.object(backend, "_find_monorepo_backend", return_value=None), \ + mock.patch.object( + backend.backend_install, "installed_server", return_value=None + ), \ + mock.patch.object(backend, "_spawn_and_wait_for_port", fake_spawn), \ + mock.patch("shutil.which", return_value="/usr/bin/npx"): + backend.launch_or_attach() + + cmd, timeout = spawned[0] + self.assertIn(f"{BACKEND_NPM_PACKAGE}@{BACKEND_NPM_VERSION}", cmd) + self.assertEqual(timeout, BACKEND_FETCH_TIMEOUT_S) + self.assertGreater(BACKEND_FETCH_TIMEOUT_S, BACKEND_SPAWN_TIMEOUT_S) + + +class TestSpawnFailuresCarryTheChildsOutput(unittest.TestCase): + def test_exit_before_a_port_reports_what_the_child_said(self): + script = ( + "import sys;" + "print('npm error code ETARGET');" + "print('npm error notarget No matching version found');" + "sys.exit(1)" + ) + with self.assertRaises(RuntimeError) as caught: + backend._spawn_and_wait_for_port([sys.executable, "-c", script]) + message = str(caught.exception) + self.assertIn("ETARGET", message) + self.assertIn("No matching version found", message) + self.assertIn("command:", message) + + def test_timeout_names_the_budget_it_exceeded(self): + script = "import time; time.sleep(5)" + with self.assertRaises(TimeoutError) as caught: + backend._spawn_and_wait_for_port([sys.executable, "-c", script], timeout=0.3) + self.assertIn("0s", str(caught.exception)) + + +class TestCli(unittest.TestCase): + def test_backend_path_exits_nonzero_and_says_how_when_not_installed(self): + with mock.patch.object( + cli.backend_install, "installed_server", return_value=None + ): + self.assertEqual(cli.main(["backend-path"]), 1) + + def test_install_backend_delegates_and_reports_success(self): + with mock.patch.object( + cli.backend_install, "install", return_value=Path("/cache/server.js") + ) as install: + self.assertEqual(cli.main(["install-backend"]), 0) + install.assert_called_once_with(force=False) + + def test_a_failed_install_is_an_error_exit_not_a_traceback(self): + with mock.patch.object( + cli.backend_install, "install", side_effect=RuntimeError("npm said no") + ): + self.assertEqual(cli.main(["install-backend"]), 1) + + +if __name__ == "__main__": + unittest.main() From bde735872de3f8f209e5e26305c85a65a5a906fb Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Tue, 29 Sep 2026 18:30:26 +0530 Subject: [PATCH 3/7] fix(runner): keep Run-all whole after a targeted rerun --- .../hand-the-run-all-command-to-a-rerun.md | 5 ++ CLAUDE.md | 2 +- packages/backend/src/runner.ts | 8 ++ packages/backend/tests/runner.test.ts | 28 +++++++ packages/selenium-devtools-py/README.md | 11 ++- .../changes/run-all-after-a-rerun.md | 5 ++ .../scripts/gen_contract.py | 13 ++- .../src/selenium_devtools/_contract.py | 1 + .../src/selenium_devtools/rerun.py | 21 ++++- .../selenium-devtools-py/tests/test_rerun.py | 83 +++++++++++++++++-- packages/shared/src/runner.ts | 17 ++-- 11 files changed, 174 insertions(+), 20 deletions(-) create mode 100644 .changeset/hand-the-run-all-command-to-a-rerun.md create mode 100644 packages/selenium-devtools-py/changes/run-all-after-a-rerun.md diff --git a/.changeset/hand-the-run-all-command-to-a-rerun.md b/.changeset/hand-the-run-all-command-to-a-rerun.md new file mode 100644 index 00000000..8819d360 --- /dev/null +++ b/.changeset/hand-the-run-all-command-to-a-rerun.md @@ -0,0 +1,5 @@ +--- +'@wdio/devtools-backend': patch +--- + +Pass the run-everything command to a rerun child. A child is spawned with one test named on its command line, so an adapter that derives its Run-all command from its own invocation republishes "run that one test" as the command for running everything — after which Run-all reruns only whatever was last reran. The child cannot reconstruct what it was narrowed from, so the spawner now hands the original down alongside the rest of the reuse handshake. diff --git a/CLAUDE.md b/CLAUDE.md index 99453c1e..58e6bf9d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -285,7 +285,7 @@ Documented divergences from the conventions above. They exist today as debt to b - A plain script's tree is one synthetic suite holding one synthetic test, and both denote the whole run, so its launch command doubles as its rerun template (no slot — the backend substitutes nothing) and all three controls are honest. Refusing the row-scoped ones instead would disable the button beside the only row the tree has. - **Two unrelated events share the `clearExecutionData` scope, and the receiver cannot tell them apart from the uid.** A run STARTING (`backend/src/index.ts` `handleTestRun`, one per `POST /api/tests/run`) and ONE ENTRY resetting inside a run already in flight (`nightwatch-devtools/src/cucumber-lifecycle.ts`, which re-emits a scenario suite and must not wipe its siblings) arrive under the same scope with the same shape. The app inferred the difference by comparing the uid against `rerunState.activeRerunSuiteUid` — a latch that outlived its rerun, so the *next* run start at a different scope read as a child clear of the last one and **skipped its wipe entirely**: rerun a suite, then the file or Tests, and the Actions/Console/Network tabs kept the previous run's rows and grew with each rerun. `ClearExecutionDataWsPayload.runStart` now states it on the wire (it has to be on the wire, not local to the clicking window — popouts see only WS events), and the app clears both latches when it is set. A backend test asserts the flag actually ships: the app-side fix reads it, so dropping it would restore the bug with every app test still green. - Still open, same class: `app/src/components/browser/snapshot.ts` `#videos` is only ever pushed to, so the screencast "Recording N" dropdown accumulates every session of every run for the life of the page (observed at 17). That component listens only to the `screencast-ready` window event and never learns a run started. - - **A rerun's process collects a SUBSET, so anything it derives from "this collection" is wrong for the tree it merges into.** Two bugs of that one shape, both found by rerunning a single pytest test: (a) `SuiteStats.order` — which `test-entry-state.ts` `orderedChildren` sorts a suite's tests and child suites by — was pytest's `enumerate(session.items)` index, so a rerun restamped its one test as position 0 and the row jumped above the class it was written below. It is now the item's **source line**, a property of the test rather than of the collection; within a module pytest collects in definition order, so the two agree wherever both are meaningful (a plugin that reorders collection is the exception, and there the line is the more stable answer anyway). (b) `suite-merge.ts` `resetStaleChildrenOnRerun` flipped every settled child *suite* to `pending` whenever an incoming suite arrived `pending` — but a single-test rerun re-emits the parent as `pending` carrying only the one test it collected, so a sibling class suite was set spinning and never reported again, keeping the spinner for the rest of the session with all of its own tests still green. `mergeTests` already froze sibling *tests* on `activeRerunTestUid`; that guard now covers child suites too. A suite on the path to the target is unaffected either way — it re-reports its own state. + - **A rerun's process collects a SUBSET, so anything it derives from "this collection" is wrong for the tree it merges into.** Three bugs of that one shape, all found by rerunning a single pytest test. The third is the one that shows the rule has a limit: (c) the **launch command** — what Run-all spawns — was built from the child's own invocation, which the backend had narrowed to a single nodeid, so one targeted rerun rescoped Run-all to that test permanently and the tree kept showing only what that child collected. Unlike (a) and (b) this is not recoverable inside the child: its arguments no longer mention what it was narrowed from. The original travels down instead, as `REUSE_ENV.LAUNCH_COMMAND` beside the rest of the reuse handshake; the *rerun template* stays locally derived, since only this process can say how its own interpreter selects a test. **Fixed in the Python adapter only** — `selenium-devtools/src/rerunManager.ts` still derives its launch command from `captureLaunchCommand()`, i.e. from the child's own argv, so a mocha/jest rerun carrying an inherited `--grep` republishes that as Run-all. It already strips those filters out of the *template* for this reason; the getter is what is left. A second consumer makes the inherit-or-derive resolution itself core's, with `captureLaunchCommand` staying adapter-local. The other two: (a) `SuiteStats.order` — which `test-entry-state.ts` `orderedChildren` sorts a suite's tests and child suites by — was pytest's `enumerate(session.items)` index, so a rerun restamped its one test as position 0 and the row jumped above the class it was written below. It is now the item's **source line**, a property of the test rather than of the collection; within a module pytest collects in definition order, so the two agree wherever both are meaningful (a plugin that reorders collection is the exception, and there the line is the more stable answer anyway). (b) `suite-merge.ts` `resetStaleChildrenOnRerun` flipped every settled child *suite* to `pending` whenever an incoming suite arrived `pending` — but a single-test rerun re-emits the parent as `pending` carrying only the one test it collected, so a sibling class suite was set spinning and never reported again, keeping the spinner for the rest of the session with all of its own tests still green. `mergeTests` already froze sibling *tests* on `activeRerunTestUid`; that guard now covers child suites too. A suite on the path to the target is unaffected either way — it re-reports its own state. - **A trace archive is a full recording of the page, and there is no redaction policy anywhere in capture.** Whatever the run put on screen or typed is in the zip, usually several times over: measured on the Python login example, its demo credential appears ~103 times across six places — the page's own displayed text (90x, the-internet prints it), the DOM mutation stream, the `Element.fill` command args, the transcript, the captured test source, and `*-elements.json`. `shared/element-scripts.ts` blanks an `` value, which is worth having because nothing downstream reads that field, but it removes **2 of those ~103** and closes nothing on its own. `buildElementScripts` now projects a captured record down to what is actually read (`selector` + `boundingBox` + context), so `value` and `href` leave the archive entirely — justified as dead data, **not** as a redaction: the same archive still carries 15 hrefs in `trace.mutations` independent of `elements.json`, and 29 `value` attribute mutations recording a typed string keystroke by keystroke (`t`, `to`, `tom`, ...). `@wdio/elements` keeps returning the full `BrowserElementInfo` from its own live call, which is its documented API. A real policy has to act at the collector and the command-arg serializer — a masking-selector or `maskInputs` option — not at one resource. Until then, treat a trace zip as sensitive as the run that produced it. - **An `ActionSnapshot` carries no session identity, in any adapter.** `shared`'s type has never had one and `core/action-snapshot.ts` records none, so per-action captures from two concurrently-driven sessions land in one list and are resolved purely by the command's completion timestamp. `claimAfter` is an exact keyed lookup, so the window is narrow — two commands completing in the **same millisecond**, where `trace-frame-snapshots.ts` breaks the tie by "keep the richest capture" (largest screenshot), which is session-blind — plus the documented `latestAtOrBefore` fallback for a command that took no capture of its own. Python is not worse than the JS adapters here and leans on that fallback less, since it stamps each snapshot with its own command's `row["timestamp"]`; reaching the failure at all needs threaded drivers in one process (pytest's function-scoped fixtures are sequential, and `-n` is multi-process). Fixing it is a shared-contract change: `sessionId` on the snapshot, and an index keyed by the pair. - **Chrome discards all WebDriver-synthesized input to a tab after a breached credential is submitted.** The first time a test types a `(username, password)` pair that Chrome's password-leak check finds in a breach corpus into an `` and submits a form whose destination no longer shows that login form, Chrome queries `passwordsleakcheck-pa.googleapis.com` and ~0.3-0.9 s later stops delivering **all** synthesized input — mouse *and* keyboard — to that tab. chromedriver returns HTTP 200 for every subsequent Element Click / Send Keys; nothing reaches the page. Untrusted JS (`element.click()`) still works and direct CDP `Input.dispatchMouseEvent`/`dispatchKeyEvent` are equally dead, so this is Chrome, not chromedriver and not our capture. `tomsmith` / `SuperSecretPassword!` — the-internet's demo credential — triggers it; changing only the *username* does not, nor does a random password. diff --git a/packages/backend/src/runner.ts b/packages/backend/src/runner.ts index a532531f..e1231048 100644 --- a/packages/backend/src/runner.ts +++ b/packages/backend/src/runner.ts @@ -69,6 +69,14 @@ class TestRunner { childEnv[REUSE_ENV.PORT] = String(payload.devtoolsPort) childEnv[REUSE_ENV.REUSE] = '1' } + // Deleted when this payload carries none: `childEnv` starts from our own + // environment, so a backend that inherited the variable would otherwise + // hand a stale command to every child it spawns. + if (payload.launchCommand) { + childEnv[REUSE_ENV.LAUNCH_COMMAND] = payload.launchCommand + } else { + delete childEnv[REUSE_ENV.LAUNCH_COMMAND] + } return childEnv } diff --git a/packages/backend/tests/runner.test.ts b/packages/backend/tests/runner.test.ts index abd114cb..f9415e6e 100644 --- a/packages/backend/tests/runner.test.ts +++ b/packages/backend/tests/runner.test.ts @@ -2,6 +2,8 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import { spawn } from 'node:child_process' import fs from 'node:fs' import path from 'node:path' +import { RERUN_SLOT, REUSE_ENV } from '@wdio/devtools-shared' + import type { RunnerRequestBody } from '../src/types.js' vi.mock('node:child_process') @@ -123,6 +125,32 @@ describe('TestRunner', () => { await firstRun.catch(() => {}) }) + it('hands the run-everything command down to the child', async () => { + // See REUSE_ENV.LAUNCH_COMMAND in shared for why the child cannot work + // this out for itself. + vi.mocked(spawn).mockReturnValue(mockChild) + const payload: RunnerRequestBody = { + uid: 'tests/test_a.py::test_two', + entryType: 'test', + devtoolsHost: 'localhost', + devtoolsPort: 3000, + launchCommand: 'python -m pytest', + rerunCommand: `python -m pytest ${RERUN_SLOT.testId}` + } + + const run = testRunner.run(payload) + await new Promise((resolve) => setTimeout(resolve, 10)) + + const env = vi.mocked(spawn).mock.calls[0][2]?.env as Record< + string, + string + > + expect(env[REUSE_ENV.LAUNCH_COMMAND]).toBe('python -m pytest') + + testRunner.stop() + await run.catch(() => {}) + }) + it('should handle spawn errors', async () => { const errorChild = createMockChild(false, true) vi.mocked(spawn).mockReturnValue(errorChild) diff --git a/packages/selenium-devtools-py/README.md b/packages/selenium-devtools-py/README.md index 5f502b67..44ee8efe 100644 --- a/packages/selenium-devtools-py/README.md +++ b/packages/selenium-devtools-py/README.md @@ -302,9 +302,14 @@ run, so all three controls relaunch the script, which is what that tree means. The rerun reports into **the dashboard you pressed the button in**: the backend points the process it spawns back at itself (`DEVTOOLS_APP_REUSE` /`_HOST` -/`_PORT`), so the child attaches to that backend and opens no second window. - -The command is your own invocation, re-derived: +/`_PORT`), so the child attaches to that backend and opens no second window. It +also hands down the Run-all command (`DEVTOOLS_RERUN_LAUNCH_COMMAND`), because +a child is spawned with one test on its command line and would otherwise +republish *that* as the command for running everything — after which Run-all +would rerun only whatever you last reran. + +The command is your own invocation, re-derived — except Run-all in a rerun +child, which is the one handed down: ``` you ran: pytest examples/ -k login -n 4 diff --git a/packages/selenium-devtools-py/changes/run-all-after-a-rerun.md b/packages/selenium-devtools-py/changes/run-all-after-a-rerun.md new file mode 100644 index 00000000..89df535a --- /dev/null +++ b/packages/selenium-devtools-py/changes/run-all-after-a-rerun.md @@ -0,0 +1,5 @@ +--- +patch +--- + +Keep Run-all meaning "run everything" after a targeted rerun. A rerun child is invoked with one nodeid on its command line, and the launch command was derived from that invocation — so once you reran a single test, Run-all republished as "run that one test" and kept rerunning it alone, with the tree showing only the test that child collected. The backend now hands the original command down, and a rerun child publishes that instead of guessing from its own arguments. diff --git a/packages/selenium-devtools-py/scripts/gen_contract.py b/packages/selenium-devtools-py/scripts/gen_contract.py index 3d7c2ea1..82b243b5 100644 --- a/packages/selenium-devtools-py/scripts/gen_contract.py +++ b/packages/selenium-devtools-py/scripts/gen_contract.py @@ -255,12 +255,18 @@ def main() -> int: "pytest nodeid, and no other slot is substituted verbatim." ) - missing_reuse = [k for k in ("REUSE", "HOST", "PORT") if k not in reuse_env] + missing_reuse = [ + k + for k in ("REUSE", "HOST", "PORT", "LAUNCH_COMMAND") + if k not in reuse_env + ] if missing_reuse: raise SystemExit( f"contract drift: REUSE_ENV key(s) {missing_reuse} no longer in " - f"shared (present: {sorted(reuse_env)}). A rerun child needs all " - "three to report into the dashboard that launched it." + f"shared (present: {sorted(reuse_env)}). REUSE/HOST/PORT are how " + "a rerun child reports into the dashboard that launched it, and " + "LAUNCH_COMMAND is how it publishes a Run-all that is not " + "narrowed to the one test it was spawned for." ) missing_export = [k for k in ("request", "result") if k not in trace_export] @@ -311,6 +317,7 @@ def main() -> int: f'ENV_REUSE = "{reuse_env["REUSE"]}"', f'ENV_REUSE_HOST = "{reuse_env["HOST"]}"', f'ENV_REUSE_PORT = "{reuse_env["PORT"]}"', + f'ENV_LAUNCH_COMMAND = "{reuse_env["LAUNCH_COMMAND"]}"', "", ] out = shared.parent / "selenium-devtools-py" / "src" / "selenium_devtools" / "_contract.py" diff --git a/packages/selenium-devtools-py/src/selenium_devtools/_contract.py b/packages/selenium-devtools-py/src/selenium_devtools/_contract.py index c46e05d0..7f9a4ab7 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/_contract.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/_contract.py @@ -35,3 +35,4 @@ ENV_REUSE = "DEVTOOLS_APP_REUSE" ENV_REUSE_HOST = "DEVTOOLS_APP_HOST" ENV_REUSE_PORT = "DEVTOOLS_APP_PORT" +ENV_LAUNCH_COMMAND = "DEVTOOLS_RERUN_LAUNCH_COMMAND" diff --git a/packages/selenium-devtools-py/src/selenium_devtools/rerun.py b/packages/selenium-devtools-py/src/selenium_devtools/rerun.py index 99d43bba..6bacf507 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/rerun.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/rerun.py @@ -38,7 +38,12 @@ import sys from typing import Dict, List, Optional, Sequence -from ._contract import ENV_RUNNER_CWD, RERUN_SLOT_TEST_ID +from ._contract import ( + ENV_LAUNCH_COMMAND, + ENV_REUSE, + ENV_RUNNER_CWD, + RERUN_SLOT_TEST_ID, +) from .constants import LOGGER_NAME, RUN_CAPABILITIES_NONE _log = logging.getLogger(f"{LOGGER_NAME}.rerun") @@ -134,7 +139,7 @@ def configure_pytest( file/dir/nodeid arguments pytest resolved out of it, and `rootdir` the directory its nodeids are relative to. """ - launch = _quote( + launch = _inherited_launch() or _quote( [sys.executable, "-m", "pytest", *_absolute_positionals(args, positionals)] ) targeted = _drop_positionals(_strip_filters(args), positionals) @@ -144,6 +149,18 @@ def configure_pytest( _publish(launch=launch, rerun=rerun, base_dir=rootdir or os.getcwd()) +def _inherited_launch() -> Optional[str]: + """The Run-all command handed down by the backend that spawned this run. + + Read only while the reuse handshake is live: the variable is inherited like + any other, so a leftover export would otherwise pin Run-all on a first run + that is nobody's child. See REUSE_ENV in shared for why it is passed at all. + """ + if os.environ.get(ENV_REUSE) != "1": + return None + return os.environ.get(ENV_LAUNCH_COMMAND) or None + + def _publish(*, launch: Optional[str], rerun: Optional[str], base_dir: str) -> None: global _options options: Dict[str, object] = { diff --git a/packages/selenium-devtools-py/tests/test_rerun.py b/packages/selenium-devtools-py/tests/test_rerun.py index ea08904f..3b27f3b3 100644 --- a/packages/selenium-devtools-py/tests/test_rerun.py +++ b/packages/selenium-devtools-py/tests/test_rerun.py @@ -10,7 +10,12 @@ import unittest from selenium_devtools import rerun -from selenium_devtools._contract import ENV_RUNNER_CWD, RERUN_SLOT_TEST_ID +from selenium_devtools._contract import ( + ENV_LAUNCH_COMMAND, + ENV_REUSE, + ENV_RUNNER_CWD, + RERUN_SLOT_TEST_ID, +) PY = rerun._quote([sys.executable]) ROOT = "/repo" @@ -24,14 +29,22 @@ def setUp(self) -> None: # `_reset_for_tests`, not `reset`: ownership of ENV_RUNNER_CWD outlives # a reset by design, so each case has to start as a fresh process would. rerun._reset_for_tests() - self._saved_cwd = os.environ.pop(ENV_RUNNER_CWD, None) + # Both are inherited, and both decide what this module publishes — so a + # suite run FROM the dashboard's own rerun (which exports them) would + # otherwise assert against that rerun's environment rather than a fresh + # process's. + self._saved = { + key: os.environ.pop(key, None) + for key in (ENV_RUNNER_CWD, ENV_REUSE, ENV_LAUNCH_COMMAND) + } self.addCleanup(self._restore) def _restore(self) -> None: rerun._reset_for_tests() - os.environ.pop(ENV_RUNNER_CWD, None) - if self._saved_cwd is not None: - os.environ[ENV_RUNNER_CWD] = self._saved_cwd + for key, value in self._saved.items(): + os.environ.pop(key, None) + if value is not None: + os.environ[key] = value def configure_pytest(self, args, positionals=(), rootdir=ROOT): rerun.configure_pytest( @@ -40,6 +53,66 @@ def configure_pytest(self, args, positionals=(), rootdir=ROOT): return rerun.run_options() +class TestARerunChildsRunAll(RerunTestCase): + """Run-all is inherited from the spawner, never derived from a rerun + child's own invocation — see REUSE_ENV.LAUNCH_COMMAND in shared.""" + + def test_the_handed_down_command_wins_over_this_invocation(self): + os.environ[ENV_REUSE] = "1" + os.environ[ENV_LAUNCH_COMMAND] = f"{PY} -m pytest" + # What the backend spawns for a targeted rerun: the template with one + # nodeid substituted in, which pytest reports back as a positional. + options = self.configure_pytest( + ["tests/test_a.py::test_two"], ["tests/test_a.py::test_two"] + ) + + self.assertEqual(options["launchCommand"], f"{PY} -m pytest") + self.assertNotIn("test_two", str(options["launchCommand"])) + + def test_the_rerun_template_is_still_this_processs_own(self): + # Only Run-all is inherited. The template has to describe how THIS + # interpreter selects a test, which the parent's command cannot say. + os.environ[ENV_REUSE] = "1" + os.environ[ENV_LAUNCH_COMMAND] = "python -m pytest" + options = self.configure_pytest(["-p", "no:cacheprovider"]) + + self.assertEqual( + options["rerunCommand"], + f"{PY} -m pytest -p no:cacheprovider {RERUN_SLOT_TEST_ID}", + ) + + def test_a_first_run_still_derives_its_own(self): + # No handshake: this is the original run, and its invocation IS the + # right answer — including a narrowing the user asked for. + options = self.configure_pytest(["tests/unit"], ["tests/unit"]) + + self.assertEqual( + options["launchCommand"], + f"{PY} -m pytest {os.path.abspath('tests/unit')}", + ) + + def test_a_leftover_variable_without_a_handshake_is_ignored(self): + # Inherited like any other variable, so a stale export from an earlier + # rerun would otherwise scope Run-all on a run that is nobody's child. + os.environ[ENV_LAUNCH_COMMAND] = f"{PY} -m pytest one_test.py::only" + options = self.configure_pytest(["tests/unit"], ["tests/unit"]) + + self.assertEqual( + options["launchCommand"], + f"{PY} -m pytest {os.path.abspath('tests/unit')}", + ) + + def test_an_empty_handshake_is_not_a_command(self): + os.environ[ENV_REUSE] = "1" + os.environ[ENV_LAUNCH_COMMAND] = "" + options = self.configure_pytest(["tests/unit"], ["tests/unit"]) + + self.assertEqual( + options["launchCommand"], + f"{PY} -m pytest {os.path.abspath('tests/unit')}", + ) + + class TestPytestCommands(RerunTestCase): def test_the_rerun_template_ends_in_the_slot_the_backend_fills(self): options = self.configure_pytest(["tests/test_a.py"], ["tests/test_a.py"]) diff --git a/packages/shared/src/runner.ts b/packages/shared/src/runner.ts index 372f66c4..822725ce 100644 --- a/packages/shared/src/runner.ts +++ b/packages/shared/src/runner.ts @@ -11,18 +11,23 @@ export const TESTS_API = { /** * Environment variables the backend's rerun spawner sets on the child - * process so the adapter (service/nightwatch/selenium) can detect the - * reuse-mode handshake and connect to the existing dashboard backend - * instead of starting a new one. Single source of truth — typos in any - * leg of the handshake silently break reruns, so all four packages - * (backend writer + three adapter readers) reference this object. + * process: how the adapter (service/nightwatch/selenium/python) detects the + * reuse-mode handshake and connects to the existing dashboard backend instead + * of starting a new one, plus what the child cannot work out for itself. + * `LAUNCH_COMMAND` is that second kind — a child is spawned with one test + * named on its command line, so an adapter deriving its Run-all command from + * its own invocation would republish "run that one test" as the command for + * running everything, and its arguments no longer say what it was narrowed + * from. Single source of truth — typos in any leg of the handshake silently + * break reruns, so every package on both sides references this object. */ export const REUSE_ENV = { REUSE: 'DEVTOOLS_APP_REUSE', HOST: 'DEVTOOLS_APP_HOST', PORT: 'DEVTOOLS_APP_PORT', RERUN_LABEL: 'DEVTOOLS_RERUN_LABEL', - RERUN_ENTRY_TYPE: 'DEVTOOLS_RERUN_ENTRY_TYPE' + RERUN_ENTRY_TYPE: 'DEVTOOLS_RERUN_ENTRY_TYPE', + LAUNCH_COMMAND: 'DEVTOOLS_RERUN_LAUNCH_COMMAND' } as const /** From 3a36ff093e57b03e1a4737cd579134d532677fa5 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Tue, 29 Sep 2026 18:30:43 +0530 Subject: [PATCH 4/7] fix(app): replay a boolean attribute the page itself set --- .changeset/replay-a-checked-box-as-checked.md | 7 ++ .../src/components/browser/vnode-transform.ts | 28 +++++++- packages/app/tests/vnode-transform.test.ts | 69 +++++++++++++++++++ 3 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 .changeset/replay-a-checked-box-as-checked.md diff --git a/.changeset/replay-a-checked-box-as-checked.md b/.changeset/replay-a-checked-box-as-checked.md new file mode 100644 index 00000000..d0be028f --- /dev/null +++ b/.changeset/replay-a-checked-box-as-checked.md @@ -0,0 +1,7 @@ +--- +'@wdio/devtools-app': patch +--- + +Replay a boolean attribute the page itself set. The DOM anchor captures markup, so a page's own `` arrives as `checked=""` — and Preact assigns these as properties, where `''` is falsy, so the box replayed unchecked while the screencast showed it ticked. Every boolean attribute was affected: a disabled control replayed as usable, and a `checked="false"` from a cleared box replayed as ticked, since a non-empty string is truthy. + +The vnode path now resolves them through the same two helpers the mutation path already used, so one policy decides both routes into the replayed DOM. diff --git a/packages/app/src/components/browser/vnode-transform.ts b/packages/app/src/components/browser/vnode-transform.ts index c5e06266..7774543e 100644 --- a/packages/app/src/components/browser/vnode-transform.ts +++ b/packages/app/src/components/browser/vnode-transform.ts @@ -3,6 +3,8 @@ import { type VNode, h } from 'preact' +import { booleanAttributeOn, isBooleanAttribute } from './boolean-attribute.js' + interface SerializedVNode { type?: string props?: { @@ -37,6 +39,30 @@ function withoutInlineHandlers( return kept } +/** + * A boolean attribute's state is its PRESENCE, and the anchor captures markup — + * so a page's own `` arrives as `checked=""`. + * Preact assigns these as properties (`name in dom`), where `''` is falsy, so + * the box replayed unchecked while the screencast showed it ticked; `disabled`, + * `readonly`, `selected` and the rest replayed off the same way, rendering a + * disabled control as usable. Measured: `checked=""` → property `false`. + * + * Resolved through the helpers the mutation path already uses, so one policy + * decides both routes into the replayed DOM. + */ +function withBooleanAttributeState( + props: Record +): Record { + const resolved: Record = {} + for (const [key, value] of Object.entries(props)) { + resolved[key] = + typeof value === 'string' && isBooleanAttribute(key) + ? booleanAttributeOn(key, value) + : value + } + return resolved +} + export function transform(node: TransformInput): VNode<{}> { if (typeof node !== 'object' || node === null) { // Plain string/number text node — return as-is for Preact to render as text. @@ -44,7 +70,7 @@ export function transform(node: TransformInput): VNode<{}> { } const { children, ...rawProps } = node.props ?? {} - const props = withoutInlineHandlers(rawProps) + const props = withBooleanAttributeState(withoutInlineHandlers(rawProps)) /** * ToDo(Christian): fix way we collect data on added nodes in script */ diff --git a/packages/app/tests/vnode-transform.test.ts b/packages/app/tests/vnode-transform.test.ts index f2656b56..49d26876 100644 --- a/packages/app/tests/vnode-transform.test.ts +++ b/packages/app/tests/vnode-transform.test.ts @@ -240,6 +240,75 @@ describe('transform', () => { }) }) + describe('boolean attributes', () => { + /** Renders through Preact, which is where the state is actually decided: + * these attributes are assigned as PROPERTIES, so a prop's truthiness — + * not its presence — is what the replayed page shows. */ + const renderInto = (node: VNode<{}>): HTMLInputElement => { + const host = document.createElement('div') + document.body.appendChild(host) + render(node, host) + return host.querySelector('input') as HTMLInputElement + } + + it('replays a box the PAGE checked, captured as a bare attribute', () => { + // the-internet's /checkboxes ships ``, and + // the anchor serializes markup — so this arrives as `checked=""`, which + // Preact assigns as the property `''`. It rendered unchecked while the + // screencast showed it ticked. + const box = renderInto( + transform(captureFragment('')) + ) + + expect(box.checked).toBe(true) + }) + + it('replays a box the TEST checked, captured as a property state', () => { + // packages/script emits String(el.checked) on input/change, so a click + // reaches the wire as "true" rather than as a bare attribute. A browser + // coerces that string itself, so this pins the resolved boolean rather + // than a defect — the two capture routes must not drift apart. + const box = renderInto( + transform({ + type: 'input', + props: { type: 'checkbox', checked: 'true' } + }) + ) + + expect(box.checked).toBe(true) + }) + + it('replays a box the test CLEARED as unchecked', () => { + // The worst of the three: "false" is a non-empty string, so a browser + // reads the raw prop as truthy and renders a box the test just cleared + // as ticked. + const box = renderInto( + transform({ + type: 'input', + props: { type: 'checkbox', checked: 'false' } + }) + ) + + expect(box.checked).toBe(false) + }) + + it('keeps a disabled control disabled', () => { + // Same class, and worse when wrong: a control the page disabled replayed + // as usable, which reads as the capture having missed the state. + const field = renderInto(transform(captureFragment(''))) + + expect(field.disabled).toBe(true) + }) + + it('leaves a non-boolean attribute alone', () => { + const field = renderInto( + transform({ type: 'input', props: { type: 'text', value: 'tomsmith' } }) + ) + + expect(field.value).toBe('tomsmith') + }) + }) + describe('props', () => { it('spreads the captured attributes onto the rendered node', () => { const node = transform({ From 964725021383f1c9a93aa94f8bdcc7f0071d9413 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Tue, 29 Sep 2026 18:30:55 +0530 Subject: [PATCH 5/7] fix(selenium-devtools-py): pin the installed-backend branch in tests --- .../src/selenium_devtools/backend_install.py | 8 +++++--- .../selenium-devtools-py/tests/test_node_runtime.py | 12 ++++++++++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/selenium-devtools-py/src/selenium_devtools/backend_install.py b/packages/selenium-devtools-py/src/selenium_devtools/backend_install.py index 702e6a2c..9f39043c 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/backend_install.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/backend_install.py @@ -97,9 +97,11 @@ def install(version: str = BACKEND_NPM_VERSION, *, force: bool = False) -> Path: if result.returncode != 0: raise RuntimeError( f"npm install failed (exit {result.returncode}) for " - f"{BACKEND_NPM_PACKAGE}@{version}. The output above is npm's. If it " - "reports the version does not exist, the npm you are running " - f'("{npm}") resolves against a registry that does not carry it.' + f"{BACKEND_NPM_PACKAGE}@{version}. The output above is npm's. A " + "report that the version does not exist is about the npm you are " + f'running ("{npm}") rather than about the version: either its ' + "registry does not carry it, or a proxy in front of it withholds " + "releases below some age, which npm reports the same way." ) server = installed_server(version) diff --git a/packages/selenium-devtools-py/tests/test_node_runtime.py b/packages/selenium-devtools-py/tests/test_node_runtime.py index 9c5d3117..f81de67d 100644 --- a/packages/selenium-devtools-py/tests/test_node_runtime.py +++ b/packages/selenium-devtools-py/tests/test_node_runtime.py @@ -151,17 +151,23 @@ def test_an_explicit_port_never_checks(self): def _spawning_run(self, monorepo_dist, npx="/usr/bin/npx"): """Drive launch_or_attach down a spawning branch, recording the order. - Both branches are pinned explicitly. Letting `_find_monorepo_backend` + EVERY branch is pinned explicitly. Letting `_find_monorepo_backend` answer for real made this test depend on whether `pnpm build` had run — green locally with a built dist, and in CI (which runs the Python job without building) it fell through to the npx branch and died on the - empty `os.environ`, because `shutil.which` reads PATH from it. + empty `os.environ`, because `shutil.which` reads PATH from it. The + installed-backend branch is the same hazard one step further out: it + reads a cache directory under the developer's home, so running + `selenium-devtools install-backend` once would otherwise decide the + outcome of a test about which branch is taken. """ calls = [] with mock.patch.object(backend, "reuse_target", return_value=None), mock.patch( "os.environ", {} ), mock.patch.object( backend, "_find_monorepo_backend", return_value=monorepo_dist + ), mock.patch.object( + backend.backend_install, "installed_server", return_value=None ), mock.patch( "shutil.which", return_value=npx ), mock.patch.object( @@ -193,6 +199,8 @@ def test_node_without_npx_beside_it_is_reported_as_such(self): "os.environ", {} ), mock.patch.object( backend, "_find_monorepo_backend", return_value=None + ), mock.patch.object( + backend.backend_install, "installed_server", return_value=None ), mock.patch( "shutil.which", return_value=None ), mock.patch.object(backend, "require_node", return_value="/n"): From 13fcc18f92db0aeb72f44c8a0791316515b93259 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Tue, 29 Sep 2026 18:42:29 +0530 Subject: [PATCH 6/7] fix(app): replay a boolean attribute the page itself set --- .changeset/replay-a-checked-box-as-checked.md | 4 +-- .../src/components/browser/vnode-transform.ts | 18 ++++++----- packages/app/tests/vnode-transform.test.ts | 30 +++++-------------- 3 files changed, 19 insertions(+), 33 deletions(-) diff --git a/.changeset/replay-a-checked-box-as-checked.md b/.changeset/replay-a-checked-box-as-checked.md index d0be028f..6ebac86b 100644 --- a/.changeset/replay-a-checked-box-as-checked.md +++ b/.changeset/replay-a-checked-box-as-checked.md @@ -2,6 +2,6 @@ '@wdio/devtools-app': patch --- -Replay a boolean attribute the page itself set. The DOM anchor captures markup, so a page's own `` arrives as `checked=""` — and Preact assigns these as properties, where `''` is falsy, so the box replayed unchecked while the screencast showed it ticked. Every boolean attribute was affected: a disabled control replayed as usable, and a `checked="false"` from a cleared box replayed as ticked, since a non-empty string is truthy. +Replay a boolean attribute the page itself set. The DOM anchor captures markup, so a page's own `` arrives as `checked=""` — and Preact assigns these as properties, where `''` is falsy, so the box replayed unchecked while the screencast showed it ticked. Every boolean attribute was affected the same way: a control the page disabled replayed as usable, a selected option as unselected. -The vnode path now resolves them through the same two helpers the mutation path already used, so one policy decides both routes into the replayed DOM. +Captured markup now replays on presence alone, which is what HTML means: `checked="false"` in a page's own markup is a ticked box. That is deliberately NOT the mutation path's rule, where "false" is the collector reporting a cleared field — a signal that only ever arrives as a mutation record, never as markup. diff --git a/packages/app/src/components/browser/vnode-transform.ts b/packages/app/src/components/browser/vnode-transform.ts index 7774543e..ba96fbfb 100644 --- a/packages/app/src/components/browser/vnode-transform.ts +++ b/packages/app/src/components/browser/vnode-transform.ts @@ -3,7 +3,7 @@ import { type VNode, h } from 'preact' -import { booleanAttributeOn, isBooleanAttribute } from './boolean-attribute.js' +import { isBooleanAttribute } from './boolean-attribute.js' interface SerializedVNode { type?: string @@ -40,25 +40,27 @@ function withoutInlineHandlers( } /** - * A boolean attribute's state is its PRESENCE, and the anchor captures markup — + * A boolean attribute's state is its PRESENCE, and everything reaching here is + * MARKUP — the anchor and every added node are serialized with `outerHTML` — * so a page's own `` arrives as `checked=""`. * Preact assigns these as properties (`name in dom`), where `''` is falsy, so * the box replayed unchecked while the screencast showed it ticked; `disabled`, * `readonly`, `selected` and the rest replayed off the same way, rendering a * disabled control as usable. Measured: `checked=""` → property `false`. * - * Resolved through the helpers the mutation path already uses, so one policy - * decides both routes into the replayed DOM. + * Presence alone, never the value: `checked="false"` in markup is a CHECKED box + * (the browser reads the attribute, not what it says), so this deliberately + * does NOT share `booleanAttributeOn` with the mutation path. There "false" is + * the collector reporting a cleared field — a signal that only exists on that + * path, since `#handleAttributeMutation` is where those records land and they + * never come through here. */ function withBooleanAttributeState( props: Record ): Record { const resolved: Record = {} for (const [key, value] of Object.entries(props)) { - resolved[key] = - typeof value === 'string' && isBooleanAttribute(key) - ? booleanAttributeOn(key, value) - : value + resolved[key] = isBooleanAttribute(key) ? true : value } return resolved } diff --git a/packages/app/tests/vnode-transform.test.ts b/packages/app/tests/vnode-transform.test.ts index 49d26876..c93f5e78 100644 --- a/packages/app/tests/vnode-transform.test.ts +++ b/packages/app/tests/vnode-transform.test.ts @@ -263,35 +263,19 @@ describe('transform', () => { expect(box.checked).toBe(true) }) - it('replays a box the TEST checked, captured as a property state', () => { - // packages/script emits String(el.checked) on input/change, so a click - // reaches the wire as "true" rather than as a bare attribute. A browser - // coerces that string itself, so this pins the resolved boolean rather - // than a defect — the two capture routes must not drift apart. + it('replays a box whose markup says checked="false"', () => { + // HTML reads the ATTRIBUTE, not what it says: a page that writes + // checked="false" renders a ticked box, and the replay must agree. This + // is why the mutation path's policy cannot be shared — there "false" is + // the collector reporting a cleared field, a signal that never reaches + // markup, and reusing it here replayed this page's box unticked. const box = renderInto( - transform({ - type: 'input', - props: { type: 'checkbox', checked: 'true' } - }) + transform(captureFragment('')) ) expect(box.checked).toBe(true) }) - it('replays a box the test CLEARED as unchecked', () => { - // The worst of the three: "false" is a non-empty string, so a browser - // reads the raw prop as truthy and renders a box the test just cleared - // as ticked. - const box = renderInto( - transform({ - type: 'input', - props: { type: 'checkbox', checked: 'false' } - }) - ) - - expect(box.checked).toBe(false) - }) - it('keeps a disabled control disabled', () => { // Same class, and worse when wrong: a control the page disabled replayed // as usable, which reads as the capture having missed the state. From 9848e9f17e4f4a722adc2f773c9e5c80c531254e Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Tue, 29 Sep 2026 18:42:41 +0530 Subject: [PATCH 7/7] fix(selenium-devtools-py): never get stuck on a broken installed backend --- .../src/selenium_devtools/backend.py | 17 ++- .../src/selenium_devtools/backend_install.py | 26 ++++- .../src/selenium_devtools/constants.py | 3 + .../tests/test_backend_install.py | 101 ++++++++++++++---- 4 files changed, 122 insertions(+), 25 deletions(-) diff --git a/packages/selenium-devtools-py/src/selenium_devtools/backend.py b/packages/selenium-devtools-py/src/selenium_devtools/backend.py index 7d1a2467..7a1ad231 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/backend.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/backend.py @@ -207,8 +207,21 @@ def launch_or_attach() -> Tuple[str, int, Optional[subprocess.Popen]]: installed = backend_install.installed_server() if installed is not None: - proc, port = _spawn_and_wait_for_port([node, str(installed)]) - return host, port, proc + try: + proc, port = _spawn_and_wait_for_port([node, str(installed)]) + return host, port, proc + except (RuntimeError, TimeoutError, OSError) as exc: + # An `npm install` interrupted after writing the entry script leaves + # a tree that looks installed and cannot run. Preferring it is right; + # being STUCK on it is not — every later run would lose capture too, + # until someone thought to delete a cache directory they were never + # told about. So a broken install costs one failed spawn, not the + # feature. + _log.warning( + "the installed backend at %s did not start (%s); falling back " + "to npx. `selenium-devtools install-backend --force` reinstalls it.", + installed, exc, + ) npx = shutil.which("npx") if npx is None: diff --git a/packages/selenium-devtools-py/src/selenium_devtools/backend_install.py b/packages/selenium-devtools-py/src/selenium_devtools/backend_install.py index 9f39043c..81cf7f4a 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/backend_install.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/backend_install.py @@ -14,6 +14,7 @@ from __future__ import annotations +import json import logging import os import shutil @@ -22,6 +23,7 @@ from typing import Optional from .constants import ( + BACKEND_BIN_NAME, BACKEND_INSTALL_DIRNAME, BACKEND_NPM_PACKAGE, BACKEND_NPM_VERSION, @@ -44,14 +46,27 @@ def install_root(version: str = BACKEND_NPM_VERSION) -> Path: def installed_server(version: str = BACKEND_NPM_VERSION) -> Optional[Path]: """The installed backend's entry script, or None if it is not there. - Resolved through the package's own ``bin`` rather than a guessed path: the - entry moved once already (``index.js`` was never a server), and a stale - guess would spawn something that exits 0 without listening. + Read from the package's own ``bin`` rather than assumed: the entry has moved + once already (``index.js`` was never a server, which is why 1.10.0 is the + floor), so a hard-coded path would make a perfectly good install look absent + the next time it moves — and the run would silently go back to `npx`. """ pkg = install_root(version) / "node_modules" / BACKEND_NPM_PACKAGE.replace( "/", os.sep ) - server = pkg / "dist" / "server.js" + try: + manifest = json.loads((pkg / "package.json").read_text()) + except (OSError, ValueError): + return None + bin_field = manifest.get("bin") + entry = ( + bin_field.get(BACKEND_BIN_NAME) + if isinstance(bin_field, dict) + else bin_field if isinstance(bin_field, str) else None + ) + if not entry: + return None + server = pkg / entry return server if server.is_file() else None @@ -108,7 +123,8 @@ def install(version: str = BACKEND_NPM_VERSION, *, force: bool = False) -> Path: if server is None: raise RuntimeError( f"npm install reported success but {BACKEND_NPM_PACKAGE}@{version} " - f"has no dist/server.js under {root}." + f"under {root} has no runnable server — its package.json names no " + f'"{BACKEND_BIN_NAME}" bin, or the file it names is missing.' ) return server diff --git a/packages/selenium-devtools-py/src/selenium_devtools/constants.py b/packages/selenium-devtools-py/src/selenium_devtools/constants.py index 1617bdb8..d64c8c37 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/constants.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/constants.py @@ -83,6 +83,9 @@ # depend on npx reaching a registry at all. Versioned, so a pin bump installs # beside the old one rather than half-overwriting it. BACKEND_INSTALL_DIRNAME = "selenium-devtools-py" +# The bin the backend package publishes its SERVER under; it also ships +# `show-trace`, so the name is what picks the right one out of `bin`. +BACKEND_BIN_NAME = "devtools-backend" # The backend is a Node app, so Python users need a Node runtime. 18 is the # floor its dependencies require; below it the process starts and then dies on diff --git a/packages/selenium-devtools-py/tests/test_backend_install.py b/packages/selenium-devtools-py/tests/test_backend_install.py index 2cf9b813..bd70d6d6 100644 --- a/packages/selenium-devtools-py/tests/test_backend_install.py +++ b/packages/selenium-devtools-py/tests/test_backend_install.py @@ -1,3 +1,4 @@ +import json import os import sys import tempfile @@ -7,6 +8,7 @@ from selenium_devtools import backend, backend_install, cli from selenium_devtools.constants import ( + BACKEND_BIN_NAME, BACKEND_FETCH_TIMEOUT_S, BACKEND_NPM_PACKAGE, BACKEND_NPM_VERSION, @@ -14,6 +16,30 @@ ) +def _fake_install( + *, + entry: str = "dist/server.js", + bin_field: "dict | None" = None, + write_entry: bool = True, +) -> Path: + """Write the tree `npm install` would leave, as `installed_server` reads it: + a package.json naming the server bin, and the file it names.""" + pkg = ( + backend_install.install_root() + / "node_modules" + / BACKEND_NPM_PACKAGE.replace("/", os.sep) + ) + pkg.mkdir(parents=True, exist_ok=True) + (pkg / "package.json").write_text( + json.dumps({"bin": bin_field or {BACKEND_BIN_NAME: entry}}) + ) + server = pkg / entry + if write_entry: + server.parent.mkdir(parents=True, exist_ok=True) + server.write_text("// server") + return server + + class TestInstallRoot(unittest.TestCase): def test_root_is_versioned_so_a_pin_bump_installs_beside_the_old_one(self): a = backend_install.install_root("1.11.0") @@ -30,29 +56,34 @@ def test_installed_server_is_none_until_the_entry_script_exists(self): with tempfile.TemporaryDirectory() as tmp: with mock.patch.dict(os.environ, {"XDG_CACHE_HOME": tmp}): self.assertIsNone(backend_install.installed_server()) - server = ( - backend_install.install_root() - / "node_modules" - / BACKEND_NPM_PACKAGE.replace("/", os.sep) - / "dist" - / "server.js" - ) - server.parent.mkdir(parents=True) - server.write_text("// server") + server = _fake_install() self.assertEqual(backend_install.installed_server(), server) + def test_the_entry_is_read_from_the_package_not_assumed(self): + # The entry has moved once already, and a hard-coded path would make a + # good install look absent — silently sending every run back to npx. + with tempfile.TemporaryDirectory() as tmp: + with mock.patch.dict(os.environ, {"XDG_CACHE_HOME": tmp}): + server = _fake_install(entry="dist/next-server.js") + self.assertEqual(backend_install.installed_server(), server) + + def test_a_package_naming_no_server_bin_is_not_installed(self): + with tempfile.TemporaryDirectory() as tmp: + with mock.patch.dict(os.environ, {"XDG_CACHE_HOME": tmp}): + _fake_install(bin_field={"show-trace": "dist/show-trace.js"}) + self.assertIsNone(backend_install.installed_server()) + + def test_an_entry_the_manifest_names_but_never_wrote_is_not_installed(self): + # The half-written tree an interrupted `npm install` leaves behind. + with tempfile.TemporaryDirectory() as tmp: + with mock.patch.dict(os.environ, {"XDG_CACHE_HOME": tmp}): + _fake_install(write_entry=False) + self.assertIsNone(backend_install.installed_server()) + def test_install_is_idempotent_and_does_not_shell_out_when_present(self): with tempfile.TemporaryDirectory() as tmp: with mock.patch.dict(os.environ, {"XDG_CACHE_HOME": tmp}): - server = ( - backend_install.install_root() - / "node_modules" - / BACKEND_NPM_PACKAGE.replace("/", os.sep) - / "dist" - / "server.js" - ) - server.parent.mkdir(parents=True) - server.write_text("// server") + server = _fake_install() with mock.patch("subprocess.run") as run: self.assertEqual(backend_install.install(), server) run.assert_not_called() @@ -124,6 +155,40 @@ def fake_spawn(cmd, timeout=BACKEND_SPAWN_TIMEOUT_S): self.assertGreater(BACKEND_FETCH_TIMEOUT_S, BACKEND_SPAWN_TIMEOUT_S) +class TestABrokenInstallDoesNotDisableCapture(unittest.TestCase): + def setUp(self): + for key in ("DEVTOOLS_PORT", "DEVTOOLS_BACKEND_CMD", "DEVTOOLS_APP_REUSE"): + os.environ.pop(key, None) + + def test_an_installed_backend_that_will_not_start_falls_back_to_npx(self): + # An `npm install` interrupted after writing the entry script leaves a + # tree that looks installed and cannot run. Preferring it is right; + # being stuck on it would cost every later run its capture too, until + # someone deleted a cache directory they were never told about. + spawned = [] + + def fake_spawn(cmd, timeout=BACKEND_SPAWN_TIMEOUT_S): + spawned.append(cmd) + if "broken" in cmd[-1]: + raise RuntimeError("backend exited (code 1) before reporting a port") + return mock.Mock(), 4321 + + with mock.patch.object(backend, "require_node", return_value="/usr/bin/node"), \ + mock.patch.object(backend, "_find_monorepo_backend", return_value=None), \ + mock.patch.object( + backend.backend_install, + "installed_server", + return_value=Path("/cache/broken/server.js"), + ), \ + mock.patch.object(backend, "_spawn_and_wait_for_port", fake_spawn), \ + mock.patch("shutil.which", return_value="/usr/bin/npx"): + _, port, _ = backend.launch_or_attach() + + self.assertEqual(port, 4321) + self.assertEqual(len(spawned), 2) + self.assertEqual(spawned[1][0], "/usr/bin/npx") + + class TestSpawnFailuresCarryTheChildsOutput(unittest.TestCase): def test_exit_before_a_port_reports_what_the_child_said(self): script = (