diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 0fe8f3cd..e9b34981 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -32,7 +32,9 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: ['3.9', '3.12'] + # The floor the package declares, and the newest — 3.10 because selenium + # 4.44 requires it, and network capture requires 4.44+. + python-version: ['3.10', '3.13'] steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 @@ -42,11 +44,6 @@ jobs: # pin its BiDi surface (tests/test_selenium_surface.py) skip without it — # so a bump that moved those internals used to pass CI green and degrade # at runtime. Installing the package is what makes those guards real. - # - # `[test]`, not `[selenium]`: the test extra is the capped one. The user- - # facing extra is uncapped on purpose (see pyproject), so installing it - # here would resolve a selenium the adapter cannot fully capture on and - # fail the guards for a reason CI cannot fix. - name: Install the adapter and its runtime dependency run: pip install -e '.[test]' diff --git a/CLAUDE.md b/CLAUDE.md index dd4e4f90..e364b8c8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -286,7 +286,7 @@ Documented divergences from the conventions above. They exist today as debt to b - **Nightwatch cucumber: traces generate, capture asserts with real pass/fail, and carry DOM mutations** (via the `test/` harness, 2026-07; both remaining gaps closed and re-verified 2026-08-06). Three fixes made the cucumber runner emit a useful trace: **(1) build** — `packages/nightwatch-devtools/tsup.config.ts` now compiles `src/helpers/cucumberHooks.cts` → `dist/helpers/cucumberHooks.cjs` as a self-contained CJS bundle (`@cucumber/cucumber` external). Previously the build ran only `tsup src/index.ts --clean`, so that file never existed and Cucumber's `require:[cucumberHooksPath]` registered *no* hooks (glob matched nothing) → zero capture. `PLUGIN_GLOBAL_KEY` moved to a leaf module `plugin-global-key.ts` so the hooks bundle stays tiny and CJS-safe (importing it via `constants.ts` dragged in core → `createRequire(import.meta.url)`, which throws when bundled to CJS). **(2) capture ordering** — a pre-quit cucumber `After` hook (`order:1000`, `captureCucumberScenarioBeforeQuit`) runs the trace capture + slice flush while the per-scenario browser session is live (the `order:-1` finalize is post browser-quit, so the flush bailed on the absent `sessionId`). Requires `traceGranularity:'test'` (per-scenario slices). **(3) native asserts** — the same pre-quit hook drains `browserProxy.drainNativeAssertCalls()` and calls `captureNativeAssertions` (the `afterEach` path early-returns for cucumber), so `assert.*` rows now appear. Console/network (BiDi) + commands + asserts + frames + sources + transcript are captured. BDD and live mode are unaffected. - **Assert outcomes are correlated off the assertion's own promise, because the results bag does not exist.** Cucumber's Nightwatch client is built by `createClient` with **no reporter**, so `SimplifiedReporter.logAssertResult` no-ops and `results.assertions`/`results.testcases` are *never* populated — no scenario-level reconcile can recover them, which is why `currentTest: undefined` was a dead end. `nativeAssertions.ts` `observedAssertOutcome` reads the outcome from the returned promise instead (`lib/core/asynctree.js` `shouldRejectNodePromise`: a failing `assert.*` rejects its deferred, a failing `verify.*` resolves *with* the AssertionError, a pass resolves with the command value). A fulfilment of `undefined` stays **neutral** — that is an assertion enqueued but never executed after an earlier `assert.*` emptied the queue, and reading it as a pass would paint a never-run assertion green. The results bag still wins where it exists and the row's window comes from whichever source supplied the outcome, so the describe/it timeline is byte-stable. Measured: **4 of 4 assert rows with a real pass/fail** (was 2 of 4 rows, 0 correlated), spanning real 44–372 ms windows instead of a synthetic 1 ms. - Relatedly, cucumber's **per-step** `resetCommandTracking()` was wiping the native-assert buffer, so each scenario kept only its *last* step's assertions (measured 1 of 2). The buffer is per **test unit** — `resetTestTracking()` at `wrapBrowserOnce` now clears it. - - **DOM `mutations` ARE captured**, and the old `ECONNRESET` / "collector not found" attribution is **wrong** — neither appears any more. The gap closed itself with document-start injection (`core/bidi-preload.ts`), confirmed on a *baseline* build so the credit is A10's. Measured per scenario with `traceGranularity:'test'` + `webSocketUrl: true`: 39 and 19 mutation entries, 2 DOM anchors each, **0 of 19 rows on the wrong document**. What remains is noise, not a gap: the screencast poller issues `/screenshot` every 200 ms into the session Nightwatch quits per scenario, logging 9–26 `WARN webdriverHttp: … socket hang up` per run. Fix is to stop the recorder before the per-scenario quit, or suppress the warn for a session in teardown. + - **DOM `mutations` ARE captured**, and the old `ECONNRESET` / "collector not found" attribution is **wrong** — neither appears any more. The gap closed itself with document-start injection (`core/bidi-preload.ts`), confirmed on a *baseline* build so the credit belongs to that change and not to anything here. Measured per scenario with `traceGranularity:'test'` + `webSocketUrl: true`: 39 and 19 mutation entries, 2 DOM anchors each, **0 of 19 rows on the wrong document**. What remains is noise, not a gap: the screencast poller issues `/screenshot` every 200 ms into the session Nightwatch quits per scenario, logging 9–26 `WARN webdriverHttp: … socket hang up` per run. Fix is to stop the recorder before the per-scenario quit, or suppress the warn for a session in teardown. - **A tsup entry that another entry imports is not a leaf, so an `import.meta.url === process.argv[1]` self-start check inside it is dead code; a CLI has to be its own leaf entry.** tsup hoists a module body shared by two entries into `dist/chunk-*.js`, and there `import.meta.url` is the chunk's path, which can never equal `process.argv[1]`. `packages/backend/src/show-trace.ts` imports `start` from `index.ts`, which is what made index shared, so index's old "start if run directly" guard was dead in **every** build: `node dist/index.js` exited 0 without ever serving, while `dist/show-trace.js` self-started correctly for exactly the same reason inverted, being a leaf whose body stays in its own output file (it also compares realpaths, because the invoked path is the `node_modules/.bin` symlink). The live dashboard server is therefore its own leaf entry, `packages/backend/src/server.ts` (shebang, built to an executable `dist/server.js`, shipped as the `devtools-backend` bin, accepting `--port`, `--hostname`, `-h`/`--help`), and `index.ts` stays library-only for the three adapters' in-process `start`/`stop`. Same family as the `cucumberHooks.cts` entry above: the tsup entry list is part of the contract, and in both cases the symptom was silence rather than an error. - **A captured text locator is generated in the recording runner's dialect; every other branch is portable CSS.** `shared/locator-dialect.ts` `locatorDialect(runner)` is the one fact table — WDIO runners (`mocha`/`jasmine`/`cucumber`) get `a*=Logout`, `nightwatch*`/`selenium-webdriver` and an **unidentified recorder** get `//a[contains(., "Logout")]`. The id reaches the page script as `CaptureActionSnapshotInput.runner` and the zip as an extension field on `context-options`, read back through `isTestRunnerId` onto `Metadata.runner`; absent in older and foreign zips, where the player shows no hint. Under WDIO a text carrying a `"` still emits XPath — WDIO compiles `tag*=` to XPath with `"` quoting and would build a broken expression — and WDIO resolves `//` itself. `locatorsMatch` decomposes **both** sides from either dialect, so `after.point` survives whichever way round the two grammars fall; the `concat()`-stitched literal is still left to exact comparison. `@wdio/elements`' standalone `getSnapshot` deliberately keeps the portable XPath default (its output is pasted into arbitrary tools), so a WDIO run's `browser.getSnapshot()` and its trace A11y tab disagree on that one branch. - `Metadata.runner` (typed `TestRunnerId`) and `metadata.options.framework` (untyped `string`, read by the sidebar's `getFramework`) are two carriers of the same fact. All three adapters now set both; the next change to either should collapse `getFramework` onto `Metadata.runner`. diff --git a/packages/selenium-devtools-py/README.md b/packages/selenium-devtools-py/README.md index 8ad4f1cc..7ff1dbbd 100644 --- a/packages/selenium-devtools-py/README.md +++ b/packages/selenium-devtools-py/README.md @@ -19,6 +19,12 @@ pip install -e packages/selenium-devtools-py # or: pip install selenium-devtoo The transport is **dependency-free** (stdlib WebSocket client). The only thing on top of your own `selenium` install is this package; `pytest` is optional. +**Requires Python 3.10+ and selenium 4.44+.** Network capture subscribes through +the public BiDi event API that selenium regenerated in 4.44 — before that the +only way to observe requests without pausing them was a private connection, +which the same release removed. 4.44 requires Python 3.10, which sets the +Python floor too. + ## Use **With pytest (recommended) — no code changes to your tests:** @@ -159,7 +165,7 @@ DEVTOOLS_PORT=3000 PYTHONPATH=src pytest e2e/test_smoke.py -p selenium_devtools. Two workflows, mirroring the JS split (`ci.yml` tests / `release.yml` publish): - **`python.yml`** — runs on PRs + pushes touching this package or `shared`: - unit tests on Python 3.9 + 3.12, and a contract-drift check (regenerate + unit tests on Python 3.10 + 3.13, and a contract-drift check (regenerate `_contract.py`, fail on any diff). Zero repo config needed. - **`python-release.yml`** — **manual** (`workflow_dispatch`, like the JS "Manual NPM Publish"), target `pypi` or `testpypi`. Builds the sdist + wheel diff --git a/packages/selenium-devtools-py/pyproject.toml b/packages/selenium-devtools-py/pyproject.toml index 0268bf5d..75752369 100644 --- a/packages/selenium-devtools-py/pyproject.toml +++ b/packages/selenium-devtools-py/pyproject.toml @@ -7,7 +7,9 @@ name = "selenium-devtools-py" version = "0.1.0" description = "Python Selenium adapter for the WebdriverIO DevTools dashboard" readme = "README.md" -requires-python = ">=3.9" +# >=3.10 because selenium 4.44 requires it, and network capture needs 4.44+. +# Not an independent choice: 3.9 left security support in October 2025. +requires-python = ">=3.10" license = { text = "MIT" } authors = [{ name = "WebdriverIO" }] keywords = ["selenium", "webdriver", "devtools", "pytest", "debugging"] @@ -17,17 +19,13 @@ keywords = ["selenium", "webdriver", "devtools", "pytest", "debugging"] dependencies = [] [project.optional-dependencies] -# Deliberately UNCAPPED, even though network capture needs selenium < 4.44 (that -# release regenerated the BiDi layer, moving the internals `bidi.py` subscribes -# through; issue #293). A cap here would downgrade anyone already on a newer -# selenium, and hard-fail the install for a project that requires one — a cost -# paid by every user to describe a partial degradation that only affects the -# Network tab. `bidi.py` reports it at runtime instead, naming the version. -selenium = ["selenium>=4.6"] -# The TEST environment is capped, because the surface guards assert against -# whatever is installed and only mean something on a version the adapter fully -# supports. Constraining the dev environment costs no user anything. -test = ["pytest>=7", "selenium>=4.6,<4.44"] +# 4.44 is where selenium regenerated the BiDi layer, and with it the public +# event API network capture subscribes through. Earlier versions offered only a +# private connection, which 4.44 removed — supporting both would mean carrying +# two subscription paths for releases that predate this package's own first one. +# Uncapped above: 4.44+ is a floor, not a range. +selenium = ["selenium>=4.44"] +test = ["pytest>=7", "selenium>=4.44"] # 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/bidi.py b/packages/selenium-devtools-py/src/selenium_devtools/bidi.py index 0b966a53..8ff550fc 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/bidi.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/bidi.py @@ -6,24 +6,32 @@ without selenium; ``attach`` does the selenium wiring and is defensive — a BiDi failure is a logged no-op, never a raised error into the user's test. -Two selenium-version realities shape this module (selenium 4.36): +Two constants shape this module: * BiDi only opens when the session was created with ``webSocketUrl`` truthy (``options.web_socket_url = True`` at build). We can't set that from inside the ``execute`` wrapper — the session already exists — so attach() checks the capability and degrades if it's missing. -* selenium's high-level ``network.add_request_handler`` *intercepts* (pauses) - requests. We deliberately avoid it: we subscribe to the network events via - the low-level connection so requests are observed but never stalled. - -The network half of that has a ceiling: selenium 4.44 regenerated the BiDi layer -from a schema, dropping ``NetworkEvent`` and renaming ``Network.conn`` to -``_conn``, so ``_attach_network`` degrades to a warning there and the Network tab -stays empty. ``pyproject.toml`` caps the extra below it. The observe-only -replacement is ``Network._event_manager.add_event_handler``, whose callback -receives a deserialized event rather than one carrying ``.params`` — a port, not -a rename, tracked as issue #293. Console capture and the preload are unaffected; -``tests/test_selenium_surface.py`` guards all of it against the installed version. +* Network events are OBSERVED, never intercepted. Every selenium API that takes + a request or response handler registers an intercept, which pauses each + request until selenium continues it; that would change the timing of the page + under test. Both paths below subscribe instead. + +Network capture subscribes through ``Network.add_event_handler``, the public +observe-only API in the BiDi layer selenium regenerated in 4.44. Before that +release the only way to observe was a private connection plus ``NetworkEvent``, +both of which 4.44 removed; the adapter requires 4.44+ rather than carrying a +second path for versions predating its own first release. + +Its generated event dataclasses cannot be used as delivered: +``BeforeRequestSentParameters`` declares only ``initiator`` and the deserializer +DROPS every param not declared, taking the request, its id and the timestamp +with it — which leaves a response with nothing to correlate against. So +``_add_raw_event_handler`` registers the callback with a pass-through +deserializer of its own, writing nothing that another subscriber reads. + +``tests/test_selenium_surface.py`` guards that surface against the installed +selenium. """ from __future__ import annotations @@ -38,7 +46,7 @@ BIDI_NET_BEFORE_REQUEST, BIDI_NET_RESPONSE_COMPLETED, LOGGER_NAME, - SELENIUM_NETWORK_SURFACE_MOVED_AT, + SELENIUM_MINIMUM_VERSION, ) from .utils import now_ms, selenium_version @@ -170,8 +178,8 @@ def _format_stacktrace(stacktrace: Any) -> str: def request_sent_kwargs(params: Dict[str, Any]) -> Optional[Dict[str, Any]]: """kwargs for the initial (pending) network frame, or None if unidentifiable. - ``params`` is the BiDi ``network.beforeRequestSent`` event params — the - ``.params`` dict on selenium's NetworkEvent. + ``params`` is the raw BiDi ``network.beforeRequestSent`` event params, as + delivered by ``event_params``. """ request = params.get("request") or {} request_id = str(request.get("request") or params.get("id") or "") @@ -353,47 +361,196 @@ def on_js_error(entry: Any) -> None: def network_unavailable_reason(exc: Exception) -> str: - """Why network capture is off, naming the selenium version when that is why. - - The bare exception is unreadable as a cause: on selenium 4.44+ it surfaces as - ``cannot import name 'NetworkEvent'``, which reads like a broken install - rather than a version the adapter has not caught up with. Console capture and - the DOM preload keep working, so this warning is the only signal the user - gets that the Network tab will stay empty. + """Why network capture could not attach, naming a too-old selenium as the + cause when it is one. + + The adapter needs the BiDi layer selenium regenerated in 4.44, and pyproject + requires it, but a user can still end up below that — an existing + environment, a transitive pin, a resolver that had to back off. The bare + exception is then an AttributeError about a generated attribute, which reads + like a broken install rather than a version floor. """ version = selenium_version() - if version >= SELENIUM_NETWORK_SURFACE_MOVED_AT: + required = ".".join(str(p) for p in SELENIUM_MINIMUM_VERSION) + if version < SELENIUM_MINIMUM_VERSION: installed = ".".join(str(part) for part in version) - moved_at = ".".join(str(p) for p in SELENIUM_NETWORK_SURFACE_MOVED_AT) return ( - f"network capture is unavailable on selenium {installed}: selenium " - f"{moved_at} regenerated the BiDi layer and moved the internals this " - "subscribes through. Console, DOM and command capture are " - f"unaffected, and selenium < {moved_at} captures network. Tracked at " - "https://github.com/webdriverio/devtools/issues/293" + f"network capture needs selenium >= {required} and {installed} is " + "installed: the BiDi event API it subscribes through arrived in " + f"{required}. Console, DOM and command capture are unaffected. " + f"`pip install --upgrade 'selenium>={required}'` restores it." ) - return f"network channel unavailable: {exc}" + return ( + f"network capture could not attach on selenium {'.'.join(str(p) for p in version)}" + f" ({exc}). Console, DOM and command capture are unaffected. Please " + "report this: https://github.com/webdriverio/devtools/issues" + ) -def _attach_network(driver: Any, capturer: SessionCapturer) -> bool: - """Subscribe to network events WITHOUT interception (see module docstring). +def event_params(event: Any) -> Dict[str, Any]: + """The BiDi event params dict. + + ``_RawEvent`` passes selenium's params straight through, so this is already a + dict on every healthy path. It stays as the boundary check because the value + comes from selenium's dispatch rather than from us: a future release that + hands the callback something else is then an empty dict and a warning from + ``_incomplete_event``, not an AttributeError inside the handler. + """ + return event if isinstance(event, dict) else {} + + +class _RawEvent: + """The deserializer selenium's dispatch expects, passing params through. + + Selenium identifies an event by ``event_class`` and deserializes it with + ``from_json``; its own wrapper builds a generated dataclass there, which for + these two events silently discards the request and the timestamp. + """ + + def __init__(self, bidi_event: str) -> None: + self.event_class = bidi_event + + def from_json(self, params: Any) -> Any: + return params + + +def supports_event_handler_api() -> bool: + """True when selenium presents the regenerated BiDi network layer. - Uses the low-level connection so requests are only observed. Returns False - (and logs) on any failure — network BiDi is best-effort. + Detection only — nothing is mutated. ``add_event_handler`` is the marker + rather than something this calls: it is public, it arrived with the layer, + and the event manager behind it is per-instance so there is nothing to check + for on the class. A manager that is then missing its parts raises inside + ``_add_raw_event_handler`` and is reported there. """ try: - conn = driver.network.conn - from selenium.webdriver.common.bidi.network import NetworkEvent # lazy - from selenium.webdriver.common.bidi.session import Session # lazy - except Exception as exc: # noqa: BLE001 - _warn(network_unavailable_reason(exc)) + from selenium.webdriver.common.bidi.network import Network + except ImportError: return False + return hasattr(Network, "add_event_handler") + + +def _add_raw_event_handler( + network: Any, bidi_event: str, callback: Any, registered: List[Any] +) -> None: + """Subscribe ``callback`` to ``bidi_event`` with the RAW params, appending + what it installed to ``registered`` so a failed attach can undo it. + + This is selenium's own ``add_event_handler`` body with one substitution: it + looks its deserializer up in a per-event map shared by every subscriber, and + we hand ours in directly. ``add_callback`` closes over the deserializer it is + given, so ours holds for the life of the session. + + Writing ours into that map instead would keep the registration on the public + API, but there is no safe window in which to do it: the swap has to stay in + place across ``add_event_handler``, which subscribes over the websocket, and + any handler another thread registers for the same event during that round + trip closes over OUR deserializer and receives dicts where it expects + selenium's generated objects. Passing it in writes nothing shared, so no such + window exists. + """ + manager = network._event_manager + wrapper = _RawEvent(bidi_event) + callback_id = manager.conn.add_callback(wrapper, callback) + # Recorded the moment the callback exists, BEFORE the two steps that can + # raise. Recording it after them instead leaves the one registration in + # flight invisible to the unwind — precisely the one that failed. + registered.append((bidi_event, wrapper, callback_id)) + manager.subscribe_to_event(bidi_event) + # Selenium counts callbacks per event to decide when a subscription is no + # longer needed. Ours is registered on the connection directly, so without + # this it is invisible to that count and another consumer removing their + # handler would unsubscribe the event out from under us. + manager.add_callback_to_tracking(bidi_event, callback_id) + + +def _undo_registration( + manager: Any, bidi_event: str, wrapper: Any, callback_id: Any +) -> None: + """Best-effort unwind of one registration. + + Being counted in selenium's bookkeeping is what makes leaving one behind + harmful: an abandoned callback keeps the event's callback count above zero, + so a later consumer removing THEIR handler no longer unsubscribes, and the + browser keeps sending the event for the rest of the session. + + Each step is guarded on its own. The registration may have failed part-way, + and a step with nothing to undo must not stop the ones that do. The callback + and its count go first so ``unsubscribe_from_event`` sees an empty list — + it only unsubscribes when no callbacks remain, so this cannot take down a + subscription another consumer is still using. + """ + for label, undo in ( + ("callback", lambda: manager.conn.remove_callback(wrapper, callback_id)), + ("count", lambda: manager.remove_callback_from_tracking(bidi_event, callback_id)), + ("subscription", lambda: manager.unsubscribe_from_event(bidi_event)), + ): + try: + undo() + except Exception as exc: # noqa: BLE001 — unwinding must not raise + _log.debug("could not unwind the %s for %s: %s", label, bidi_event, exc) + + +_reported_incomplete: set = set() + +def _incomplete_event(params: Dict[str, Any], label: str) -> bool: + """True (and warns ONCE per event type) when an event arrived without the + fields capture needs. + + The 4.44+ path relies on selenium deserializing to the raw params, so a + future release that reinstates a typed class for these events would strip + the request and timestamp and leave every entry uncorrelated. That would + otherwise show up as a quietly empty Network tab — the exact failure this + port exists to end — so it is stated instead. + + Once, because the condition is a property of the selenium build rather than + of a request: warning per event would put one line per HTTP request of the + run into the user's console and bury the message it is trying to deliver. + """ + if params.get("request") is not None: + return False + if label not in _reported_incomplete: + _reported_incomplete.add(label) + _warn( + f"{label} arrived without a request field, so it cannot be " + f"correlated — selenium delivered {sorted(params) or 'nothing'}. " + "Network capture is degraded; please report this with your selenium " + "version. Further occurrences are not logged." + ) + return True + + +def _attach_network( + driver: Any, capturer: SessionCapturer, stats: Optional[Dict[str, Any]] = None +) -> bool: + """Subscribe to network events WITHOUT interception (see module docstring). + + Returns False (and logs) on any failure — network BiDi is best-effort. + + ``stats`` is filled in for the caller to report at teardown, rather than + logged per event: the dashboard's Network tab already lists every request, so + a running count is one Console line per request saying what the UI beside it + already shows. What the tab cannot show is a request whose response never + arrived, which is what ``pending`` still holding entries at the end means. + """ + stats = stats if stats is not None else {} + stats["captured"] = 0 pending: Dict[str, Dict[str, Any]] = {} + stats["pending"] = pending + # Both handlers are inert until every subscription is in place — see + # _subscribe_via_event_manager for why a half-subscribed pair is worse than + # no capture at all. + active = {"ok": False} def on_request_sent(event: Any) -> None: + if not active["ok"]: + return try: - kwargs = request_sent_kwargs(getattr(event, "params", {}) or {}) + params = event_params(event) + if _incomplete_event(params, BIDI_NET_BEFORE_REQUEST): + return + kwargs = request_sent_kwargs(params) if kwargs is not None: pending[kwargs["request_id"]] = kwargs capturer.capture_network(**kwargs) @@ -401,38 +558,104 @@ def on_request_sent(event: Any) -> None: _warn(f"beforeRequestSent handler threw: {exc}") def on_response_completed(event: Any) -> None: + if not active["ok"]: + return try: - kwargs = response_completed_kwargs( - getattr(event, "params", {}) or {}, pending - ) + params = event_params(event) + if _incomplete_event(params, BIDI_NET_RESPONSE_COMPLETED): + return + kwargs = response_completed_kwargs(params, pending) if kwargs is not None: pending.pop(kwargs["request_id"], None) capturer.capture_network(**kwargs) + stats["captured"] += 1 except Exception as exc: # noqa: BLE001 _warn(f"responseCompleted handler threw: {exc}") + return _subscribe_via_event_manager( + driver, + { + BIDI_NET_BEFORE_REQUEST: on_request_sent, + BIDI_NET_RESPONSE_COMPLETED: on_response_completed, + }, + active, + ) + + +def _subscribe_via_event_manager( + driver: Any, handlers: Dict[str, Any], active: Dict[str, bool] +) -> bool: + """selenium 4.44+: subscribe each handler to its BiDi event. + + Deliberately not ``add_request_handler``/``add_response_handler``: both + register an intercept even in their high-level form, which pauses every + request until selenium continues it. This only observes. + + A failure part-way through is handled twice over, because the two problems + are different. ``active`` is flipped only once EVERY handler is registered, + and the handlers consult it, so no incomplete DATA is ever produced: + ``beforeRequestSent`` on its own emits a pending frame per request that only + ``responseCompleted`` finalizes, which would fill the Network tab with + requests stuck pending and grow ``pending`` for the rest of the session. That + also covers the gap between the two subscribes, not just an outright failure. + Then the registrations already made are unwound, so no STATE is left behind + either — see ``_undo_registration`` for why an abandoned one is not inert. + The flag is the guarantee; the unwind is best-effort and may find nothing. + """ + if not supports_event_handler_api(): + # Checked rather than caught, so a too-old selenium is reported as a + # version floor instead of an AttributeError about a generated attribute. + _warn(network_unavailable_reason(AttributeError("no BiDi event API"))) + return False + manager = None + registered = [] try: - conn.execute( - Session(conn).subscribe( - BIDI_NET_BEFORE_REQUEST, BIDI_NET_RESPONSE_COMPLETED - ) - ) - conn.add_callback(NetworkEvent(BIDI_NET_BEFORE_REQUEST), on_request_sent) - conn.add_callback( - NetworkEvent(BIDI_NET_RESPONSE_COMPLETED), on_response_completed - ) - return True + network = driver.network + manager = network._event_manager + for bidi_event, callback in handlers.items(): + _add_raw_event_handler(network, bidi_event, callback, registered) except Exception as exc: # noqa: BLE001 - _warn(f"network subscribe failed: {exc}") + _warn(f"network subscribe failed, no network events captured: {exc}") + for bidi_event, wrapper, callback_id in registered: + _undo_registration(manager, bidi_event, wrapper, callback_id) return False + active["ok"] = True + return True + + +def network_summary(stats: Optional[Dict[str, Any]]) -> Optional[str]: + """One line describing what network capture actually did, or None when there + is nothing worth saying. + + Reported once at teardown rather than per event. The count is the cheap half; + the useful half is requests still pending, which means a response never + arrived and is the one thing the Network tab cannot show on its own. + """ + if not stats: + return None + captured = stats.get("captured", 0) + unanswered = len(stats.get("pending") or ()) + if not captured and not unanswered: + return None + if unanswered: + return ( + f"network: {captured} request(s) captured, " + f"{unanswered} still awaiting a response at teardown" + ) + return f"network: {captured} request(s) captured" -def attach(driver: Any, capturer: SessionCapturer) -> bool: +def attach( + driver: Any, capturer: SessionCapturer, stats: Optional[Dict[str, Any]] = None +) -> bool: """Wire BiDi console + network capture onto ``driver``. Returns True if at least one channel attached. A driver without the ``webSocketUrl`` capability (BiDi not enabled at build time) is skipped with a one-line warning — capture continues via the command stream only. + + ``stats`` is an optional bag the caller keeps, to be passed to + ``network_summary`` when the session ends. """ if not _bidi_enabled(driver): _warn( @@ -443,6 +666,6 @@ def attach(driver: Any, capturer: SessionCapturer) -> bool: attached = 0 if _attach_console(driver, capturer): attached += 1 - if _attach_network(driver, capturer): + if _attach_network(driver, capturer, stats): attached += 1 return attached > 0 diff --git a/packages/selenium-devtools-py/src/selenium_devtools/constants.py b/packages/selenium-devtools-py/src/selenium_devtools/constants.py index fa9e3e30..5e68ce2a 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/constants.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/constants.py @@ -100,12 +100,13 @@ # would stall the user's page loads if a callback failed to continue them). BIDI_NET_BEFORE_REQUEST = "network.beforeRequestSent" BIDI_NET_RESPONSE_COMPLETED = "network.responseCompleted" -# selenium 4.44 regenerated the BiDi layer from a schema: ``NetworkEvent`` left -# ``bidi.network`` and ``Network.conn`` became ``_conn``, so the subscribe above -# cannot be built there and network capture degrades to nothing. Console capture -# and the preload are unaffected. One source of truth for that version, read by -# bidi.py (to explain the degradation at runtime) and by the surface guards. -SELENIUM_NETWORK_SURFACE_MOVED_AT = (4, 44) +# The adapter targets the regenerated BiDi layer, which selenium 4.44 shipped — +# it is what ``Network.add_event_handler`` and ``EVENT_CONFIGS`` arrived in, and +# the pre-4.44 internals network capture used instead were removed in the same +# release. Declared in pyproject too; here so a wrong install is explained rather +# than raising an AttributeError, and so the surface guards know what they apply +# to. Also the reason ``requires-python`` is >=3.10: selenium 4.44 requires it. +SELENIUM_MINIMUM_VERSION = (4, 44) # selenium's BiDi log entries already carry lowercase levels; this normalizes # the stragglers to the shared LogLevel union. Unmapped levels fall back to log. BIDI_LEVEL_MAP = { diff --git a/packages/selenium-devtools-py/src/selenium_devtools/instrumentation.py b/packages/selenium-devtools-py/src/selenium_devtools/instrumentation.py index 477e0ed3..140d94b7 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/instrumentation.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/instrumentation.py @@ -281,6 +281,9 @@ def _close_entry(capturer: SessionCapturer, entry: dict) -> None: _flush_mutations(capturer, entry) entry["snapshot"] = None _finalize_screencast(capturer, entry["session_id"], entry) + summary = bidi.network_summary(entry.get("network")) + if summary: + _log.info(summary) def _finalize_screencast( @@ -342,7 +345,10 @@ def _ensure_session_setup(driver: Any, capturer: SessionCapturer) -> Optional[di _log.info("session %s started", session_id) _send_default_suite(capturer, "running") # tree entry for plain-script runs try: - if bidi.attach(driver, capturer): + # Filled in as events arrive; reported once by _on_quit, because the + # Network tab already lists every request as it happens. + entry["network"] = {} + if bidi.attach(driver, capturer, entry["network"]): _log.info("BiDi attached — capturing console + network") except Exception as exc: # noqa: BLE001 — capture must never break the test _log.warning("BiDi attach threw: %s", exc) @@ -536,7 +542,9 @@ def patched_execute(self, driver_command: str, params: Any = None): # noqa: ANN call_source=src, screenshot=shot, ) - _log.debug("command: %s", driver_command) + # No per-command line here: the Actions timeline lists every command as + # it happens, and `_WATCH` puts this logger's debug records in the same + # Console the user is reading, so it was one duplicate line per command. # Keep the snapshot iframe current after every command (a click can # navigate too, not just get/back/…), re-injecting if the page changed. if entry is not None: diff --git a/packages/selenium-devtools-py/tests/test_bidi.py b/packages/selenium-devtools-py/tests/test_bidi.py index 0cca117f..bf152ee1 100644 --- a/packages/selenium-devtools-py/tests/test_bidi.py +++ b/packages/selenium-devtools-py/tests/test_bidi.py @@ -1,3 +1,5 @@ +import sys +import types import unittest from unittest import mock @@ -371,40 +373,551 @@ def network(self): self.assertTrue(bidi.attach(Driver(), cap)) +def fake_network_module(with_event_manager=True): + """A stand-in for selenium 4.44+'s regenerated `bidi.network`. + + Faked rather than skipped-without-selenium: the installed selenium here is + 4.36, so the 4.44+ path has no other way to be exercised, and it is the path + every new user is on. + """ + module = types.ModuleType("selenium.webdriver.common.bidi.network") + + class EventConfig: + def __init__(self, event_key, bidi_event, event_class): + self.event_key = event_key + self.bidi_event = bidi_event + self.event_class = event_class + + class TypedWrapper: + """selenium's own deserializer: builds the generated dataclass, which + for these events keeps only its one declared field.""" + + def __init__(self, bidi_event, event_class): + self.event_class = bidi_event + self._python_class = event_class + + def from_json(self, params): + if self._python_class is dict: + return params + declared = getattr(self._python_class, "DECLARED", ()) + return self._python_class( + **{k: v for k, v in params.items() if k in declared} + ) + + class BeforeRequestSentParameters: + DECLARED = ("initiator",) + + def __init__(self, initiator=None): + self.initiator = initiator + + class Conn: + """The websocket connection. `add_callback` CLOSES OVER the deserializer + it is handed, which is what lets the adapter pass its own in rather than + publish it to a shared map.""" + + def __init__(self): + self.callbacks = {} # event name -> [(callback_id, fn)] + self.next_id = 0 + + def add_callback(self, event, callback): + self.next_id += 1 + self.callbacks.setdefault(event.event_class, []).append( + (self.next_id, lambda params: callback(event.from_json(params))) + ) + return self.next_id + + def remove_callback(self, event, callback_id): + entries = self.callbacks.get(event.event_class, []) + self.callbacks[event.event_class] = [ + entry for entry in entries if entry[0] != callback_id + ] + + class EventManager: + def __init__(self, configs): + self.conn = Conn() + self.subscribed = [] + self.tracked = [] + # One deserializer per BiDi event, shared by every subscriber. + self._event_wrappers = { + config.bidi_event: TypedWrapper(config.bidi_event, config.event_class) + for config in configs.values() + } + + def subscribe_to_event(self, bidi_event, contexts=None): + self.subscribed.append(bidi_event) + + def add_callback_to_tracking(self, bidi_event, callback_id): + self.tracked.append((bidi_event, callback_id)) + + def remove_callback_from_tracking(self, bidi_event, callback_id): + self.tracked.remove((bidi_event, callback_id)) + + def unsubscribe_from_event(self, bidi_event): + # Selenium only unsubscribes when no callbacks remain, so a live + # consumer's subscription cannot be taken down by someone else's + # unwind. Mirrored, or the test would pass on a wrong implementation. + if any(event == bidi_event for event, _ in self.tracked): + return + while bidi_event in self.subscribed: + self.subscribed.remove(bidi_event) + + class Network: + EVENT_CONFIGS = { + "before_request_sent": EventConfig( + "before_request_sent", + "network.beforeRequestSent", + BeforeRequestSentParameters, + ), + "response_completed": EventConfig( + "response_completed", "network.responseCompleted", dict + ), + } + + def __init__(self): + self._event_manager = EventManager(self.EVENT_CONFIGS) + + def add_event_handler(self, event, callback, contexts=None): + """Selenium's own registration, which the adapter does NOT use: it + takes the deserializer from the shared map. Kept so a test can + register through it and prove it still gets selenium's own.""" + config = self.EVENT_CONFIGS.get(event) + if config is None: + raise ValueError(f"Event '{event}' not found") + wrapper = self._event_manager._event_wrappers[config.bidi_event] + return self._event_manager.conn.add_callback(wrapper, callback) + + if not with_event_manager: + del Network.add_event_handler + Network.EVENT_CONFIGS = None + + module.EventConfig = EventConfig + module.Network = Network + module.BeforeRequestSentParameters = BeforeRequestSentParameters + return module + + +class NewSeleniumDriver: + """A 4.44+ driver. Records WHEN `.network` is first touched, because the + deserializers are built in `Network.__init__` — registering after that is + too late, and the failure would be silent.""" + + def __init__(self, network, log): + self.caps = {"webSocketUrl": "ws://x"} + self._network = network + self._log = log + + @property + def network(self): + self._log.append("network accessed") + return self._network + + @property + def script(self): + raise RuntimeError("console not under test") + + +class TestTheEventManagerPath(unittest.TestCase): + """selenium 4.44+ — registering against the regenerated BiDi layer.""" + + @staticmethod + def _dispatch(network, bidi_event, params): + """Deliver an event the way selenium's connection does.""" + for _callback_id, fn in network._event_manager.conn.callbacks[bidi_event]: + fn(params) + + def test_raw_params_reach_the_handlers_and_are_captured(self): + module = fake_network_module() + network = module.Network() + capturer = SessionCapturer(FakeTransport()) + + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + self.assertTrue( + bidi._attach_network(NewSeleniumDriver(network, []), capturer) + ) + + self._dispatch(network, "network.beforeRequestSent", { + "request": {"request": "R1", "url": "https://x/a.js", "method": "GET"}, + "timestamp": 1000, + }) + self._dispatch(network, "network.responseCompleted", { + "request": {"request": "R1"}, "timestamp": 1200, + "response": {"status": 200, "statusText": "OK", + "mimeType": "text/javascript", "bytesReceived": 12}, + }) + + # capture_network sends one batch per call, each a list of one frame. + frames = [ + batch[0] for scope, batch in capturer._tx.sent + if scope == "networkRequests" + ] + self.assertEqual(len(frames), 2) + self.assertEqual(frames[0]["url"], "https://x/a.js") + self.assertEqual(frames[1]["status"], 200) # correlated with the request + + def test_both_events_are_subscribed_and_counted(self): + # Registering on the connection directly bypasses selenium's own + # bookkeeping, so the subscribe and the callback count are done + # explicitly — without the count, another consumer removing their + # handler would unsubscribe the event out from under us. + module = fake_network_module() + network = module.Network() + + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + bidi._attach_network( + NewSeleniumDriver(network, []), SessionCapturer(FakeTransport()) + ) + + manager = network._event_manager + self.assertEqual( + sorted(manager.subscribed), + ["network.beforeRequestSent", "network.responseCompleted"], + ) + self.assertEqual( + sorted(event for event, _ in manager.tracked), + ["network.beforeRequestSent", "network.responseCompleted"], + ) + + def test_nothing_another_subscriber_reads_is_written(self): + """The adapter's deserializer must never be visible to anyone else. + + Publishing it into the shared per-event map would be the public-API + route, but there is no safe window: the swap has to span the websocket + subscribe inside `add_event_handler`, and any handler registered during + that round trip would close over ours and receive dicts where it expects + selenium's generated objects. + """ + module = fake_network_module() + network = module.Network() + + configs_before = dict(module.Network.EVENT_CONFIGS) + wrappers = network._event_manager._event_wrappers + wrappers_before = dict(wrappers) + + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + self.assertTrue( + bidi._attach_network( + NewSeleniumDriver(network, []), SessionCapturer(FakeTransport()) + ) + ) + + self.assertEqual(module.Network.EVENT_CONFIGS, configs_before) + # Identity, not equality: the very objects are untouched, so a handler + # registered at ANY time behaves exactly as it would have. + self.assertEqual(wrappers, wrappers_before) + for event, wrapper in wrappers_before.items(): + self.assertIs(wrappers[event], wrapper) + + def test_a_concurrent_subscriber_still_gets_selenium_deserializer(self): + """The race Greptile raised, made concrete: someone else registering for + the SAME event still gets the generated object, not our dict.""" + module = fake_network_module() + network = module.Network() + seen = [] + + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + bidi._attach_network( + NewSeleniumDriver(network, []), SessionCapturer(FakeTransport()) + ) + # Selenium's own registration, for an event the adapter also holds. + network.add_event_handler("before_request_sent", seen.append) + + self._dispatch(network, "network.beforeRequestSent", { + "request": {"request": "R1", "url": "https://x/a.js", "method": "GET"}, + "initiator": {"type": "script"}, "timestamp": 1000, + }) + + self.assertEqual(len(seen), 1) + # The generated dataclass, with attribute access intact — NOT a dict. + self.assertIsInstance(seen[0], module.BeforeRequestSentParameters) + self.assertEqual(seen[0].initiator, {"type": "script"}) + + def test_a_half_subscribed_pair_captures_nothing(self): + """The first registration succeeding and the second failing must not + leave capture half-on. + + beforeRequestSent alone emits a pending frame per request that only + responseCompleted finalizes, so the Network tab would fill with requests + stuck pending and the pending map would grow for the rest of the session + — while attach() reported failure and the caller believed nothing was + capturing. + """ + module = fake_network_module() + network = module.Network() + capturer = SessionCapturer(FakeTransport()) + + real_subscribe = network._event_manager.subscribe_to_event + + def failing_subscribe(bidi_event, contexts=None): + if bidi_event == "network.responseCompleted": + raise RuntimeError("websocket went away") + return real_subscribe(bidi_event, contexts) + + network._event_manager.subscribe_to_event = failing_subscribe + + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + with self.assertLogs("selenium_devtools.bidi", level="WARNING"): + attached = bidi._attach_network( + NewSeleniumDriver(network, []), capturer + ) + + self.assertFalse(attached) + + manager = network._event_manager + # Nothing left behind. An abandoned callback would keep the event's + # callback count above zero, so a later consumer removing THEIR handler + # would no longer unsubscribe and the browser would keep sending it. + self.assertEqual(manager.conn.callbacks.get("network.beforeRequestSent"), []) + self.assertEqual(manager.tracked, []) + self.assertNotIn("network.beforeRequestSent", manager.subscribed) + + # And no data even if something did survive: the flag is the guarantee, + # the unwind is best-effort. + self._dispatch(network, "network.beforeRequestSent", { + "request": {"request": "R1", "url": "https://x/a.js", "method": "GET"}, + "timestamp": 1000, + }) + frames = [s for s, _ in capturer._tx.sent if s == "networkRequests"] + self.assertEqual(frames, []) + + def test_a_failure_mid_registration_unwinds_the_in_flight_callback(self): + """The callback is installed before the steps that can raise, so the + registration that FAILED is the one most easily missed. + + Recording it only after subscribing leaves it on the connection with + nothing referencing it, and a session that re-attaches accumulates one + more on every failure. + """ + module = fake_network_module() + network = module.Network() + manager = network._event_manager + + # Fails on the FIRST event, i.e. part-way through its own registration + # rather than after an earlier one completed. + def failing_subscribe(bidi_event, contexts=None): + raise RuntimeError("websocket went away") + + manager.subscribe_to_event = failing_subscribe + + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + with self.assertLogs("selenium_devtools.bidi", level="WARNING"): + attached = bidi._attach_network( + NewSeleniumDriver(network, []), SessionCapturer(FakeTransport()) + ) + + self.assertFalse(attached) + self.assertEqual( + [entries for entries in manager.conn.callbacks.values() if entries], [] + ) + # Never tracked, because add_callback_to_tracking is after the failure — + # so the unwind's own steps must tolerate having nothing to remove. + self.assertEqual(manager.tracked, []) + + def test_an_unwind_never_takes_down_a_live_subscription(self): + """`unsubscribe_from_event` only fires with no callbacks left, so another + consumer already listening to the same event keeps theirs.""" + module = fake_network_module() + network = module.Network() + manager = network._event_manager + + # A consumer subscribed before the adapter attaches. + manager.subscribe_to_event("network.beforeRequestSent") + manager.add_callback_to_tracking("network.beforeRequestSent", 999) + + def failing_subscribe(bidi_event, contexts=None): + if bidi_event == "network.responseCompleted": + raise RuntimeError("websocket went away") + manager.subscribed.append(bidi_event) + + manager.subscribe_to_event = failing_subscribe + + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + with self.assertLogs("selenium_devtools.bidi", level="WARNING"): + bidi._attach_network( + NewSeleniumDriver(network, []), SessionCapturer(FakeTransport()) + ) + + self.assertIn("network.beforeRequestSent", manager.subscribed) + self.assertIn(("network.beforeRequestSent", 999), manager.tracked) + + def test_a_selenium_without_the_event_api_is_reported_not_silent(self): + """There is no second path to fall back to, so a selenium that cannot + provide this has to say so. Silence here is an empty Network tab with no + stated cause, which is the failure this whole port exists to end.""" + module = fake_network_module(with_event_manager=False) + + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + self.assertFalse(bidi.supports_event_handler_api()) + with self.assertLogs("selenium_devtools.bidi", level="WARNING") as logs: + # `.network` is never reached: the check happens before it. + attached = bidi._attach_network( + NewSeleniumDriver(None, []), SessionCapturer(FakeTransport()) + ) + + self.assertFalse(attached) + self.assertIn("network capture", "\n".join(logs.output)) + + def test_the_regenerated_layer_selects_the_event_handler_path(self): + module = fake_network_module() + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + self.assertTrue(bidi.supports_event_handler_api()) + + # Detection alone must not touch anything — the caller uses it to pick. + self.assertNotIn("devtools_before_request_sent", module.Network.EVENT_CONFIGS) + + +class TestTheTeardownSummary(unittest.TestCase): + """One line at the end instead of one per request. + + `_WATCH` raises this package's logger to DEBUG so its records reach the + dashboard Console, which means a per-event debug line is a Console line per + request — beside a Network tab already listing every one of them.""" + + def test_a_clean_run_reports_only_the_count(self): + self.assertEqual( + bidi.network_summary({"captured": 13, "pending": {}}), + "network: 13 request(s) captured", + ) + + def test_requests_without_a_response_are_called_out(self): + # The count is the cheap half. This is the half the Network tab cannot + # show: a request whose response never arrived. + summary = bidi.network_summary({"captured": 5, "pending": {"R7": {}, "R8": {}}}) + + self.assertIn("5 request(s) captured", summary) + self.assertIn("2 still awaiting a response", summary) + + def test_nothing_captured_and_nothing_pending_says_nothing(self): + # A session that made no requests should not add a line to the Console. + self.assertIsNone(bidi.network_summary({"captured": 0, "pending": {}})) + self.assertIsNone(bidi.network_summary({})) + self.assertIsNone(bidi.network_summary(None)) + + def test_the_counts_come_from_real_capture(self): + module = fake_network_module() + network = module.Network() + stats = {} + + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + bidi._attach_network( + NewSeleniumDriver(network, []), SessionCapturer(FakeTransport()), stats + ) + + for request_id in ("R1", "R2"): + TestTheEventManagerPath._dispatch( + network, "network.beforeRequestSent", + {"request": {"request": request_id, "url": "https://x/a", "method": "GET"}, + "timestamp": 1000}, + ) + # Only one of the two answers. + TestTheEventManagerPath._dispatch( + network, "network.responseCompleted", + {"request": {"request": "R1"}, "timestamp": 1200, + "response": {"status": 200}}, + ) + + self.assertEqual(bidi.network_summary(stats), + "network: 1 request(s) captured, 1 still awaiting a response " + "at teardown") + + +class TestEventParams(unittest.TestCase): + def test_a_raw_dict_is_its_own_params(self): + self.assertEqual(bidi.event_params({"request": {}}), {"request": {}}) + + def test_anything_else_degrades_to_empty(self): + # The value comes from selenium's dispatch, so a release that hands the + # callback an object must become a warning from _incomplete_event, not + # an AttributeError raised inside the handler. + self.assertEqual(bidi.event_params(object()), {}) + self.assertEqual( + bidi.event_params(types.SimpleNamespace(params={"request": {}})), {} + ) + + +class TestADegradedEventIsReported(unittest.TestCase): + """If a future selenium reinstates a typed class for these events, the + request is stripped and nothing can be correlated. That must not look like + a quiet network-free page — it is the failure this port exists to end.""" + + def setUp(self): + bidi._reported_incomplete.clear() + + def test_an_event_without_a_request_warns(self): + with self.assertLogs("selenium_devtools.bidi", level="WARNING") as logs: + self.assertTrue(bidi._incomplete_event({"initiator": {}}, "network.x")) + + self.assertIn("cannot be correlated", "\n".join(logs.output)) + + def test_it_warns_once_per_event_not_once_per_request(self): + # The condition is a property of the selenium build, not of a request: + # warning per event would put one line per HTTP request into the user's + # console and bury the message. + with self.assertLogs("selenium_devtools.bidi", level="WARNING") as logs: + for _ in range(5): + self.assertTrue(bidi._incomplete_event({}, "network.x")) + + self.assertEqual(len(logs.output), 1) + + def test_each_event_type_reports_separately(self): + with self.assertLogs("selenium_devtools.bidi", level="WARNING") as logs: + bidi._incomplete_event({}, "network.beforeRequestSent") + bidi._incomplete_event({}, "network.responseCompleted") + + self.assertEqual(len(logs.output), 2) + + def test_a_complete_event_is_silent(self): + self.assertFalse(bidi._incomplete_event({"request": {"request": "R"}}, "network.x")) + + class TestWhyNetworkCaptureIsOff(unittest.TestCase): - """The user-facing extra is uncapped, so a user CAN be on a selenium whose - BiDi layer moved. This warning is then the only signal that the Network tab - will stay empty, so it has to name the cause rather than echo an ImportError - that reads like a broken install.""" + """pyproject requires the selenium that carries the BiDi event API, but a + user can still be below it — an existing environment, a transitive pin, a + resolver that backed off. The bare failure is then an AttributeError about a + generated attribute, which reads like a broken install rather than a floor.""" - def test_a_moved_surface_is_reported_as_a_version_gap(self): - major, minor = bidi.SELENIUM_NETWORK_SURFACE_MOVED_AT + def test_a_selenium_below_the_floor_is_named_as_the_cause(self): + major, minor = bidi.SELENIUM_MINIMUM_VERSION with mock.patch.object( - bidi, "selenium_version", return_value=(major, minor + 1) + bidi, "selenium_version", return_value=(major, minor - 1) ): - reason = bidi.network_unavailable_reason( - ImportError("cannot import name 'NetworkEvent'") - ) + reason = bidi.network_unavailable_reason(AttributeError("no attribute")) - # BOTH versions, and they are different things: what the user has, and - # where the surface moved. Asserted on the ATTRIBUTION, not on the - # version appearing somewhere — the moved-at version is also named in - # the "selenium < X captures network" advice, so a bare assertIn passes - # even when the sentence blames the installed version for the move. - self.assertIn(f"on selenium {major}.{minor + 1}", reason) - self.assertIn(f"selenium {major}.{minor} regenerated", reason) - self.assertIn("293", reason) # where the fix is tracked - # The half that still works must be said, or this reads as total loss. + self.assertIn(f"{major}.{minor - 1} is installed", reason) # what they have + self.assertIn(f">= {major}.{minor}", reason) # what is needed + self.assertIn("pip install --upgrade", reason) # how to fix it + # The half that still works, or this reads as total loss. self.assertIn("Console", reason) - def test_an_ordinary_failure_still_reports_the_exception(self): - # Below the moved version the cause is NOT the selenium release, so - # blaming it would send the reader somewhere with no answer. - with mock.patch.object(bidi, "selenium_version", return_value=(4, 36)): + def test_a_failure_on_a_supported_selenium_reports_the_exception(self): + # At or above the floor the version is NOT the cause, so blaming it would + # send the reader somewhere with no answer. + with mock.patch.object( + bidi, "selenium_version", return_value=bidi.SELENIUM_MINIMUM_VERSION + ): reason = bidi.network_unavailable_reason(RuntimeError("no bidi socket")) self.assertIn("no bidi socket", reason) - self.assertNotIn("293", reason) + self.assertNotIn("pip install --upgrade", reason) if __name__ == "__main__": diff --git a/packages/selenium-devtools-py/tests/test_instrumentation.py b/packages/selenium-devtools-py/tests/test_instrumentation.py index df503ed3..e820bc31 100644 --- a/packages/selenium-devtools-py/tests/test_instrumentation.py +++ b/packages/selenium-devtools-py/tests/test_instrumentation.py @@ -476,7 +476,11 @@ def setUp(self): self.attached = [] self._bidi = mock.patch.object( instrumentation.bidi, "attach", - side_effect=lambda d, c: self.attached.append(d.session_id) or True, + # `stats` is the bag attach fills for the teardown summary; accepted + # here so the stub keeps matching the real signature. + side_effect=lambda d, c, stats=None: ( + self.attached.append(d.session_id) or True + ), ) self._bidi.start() instrumentation.install(self.cap, MultiSessionDriver) diff --git a/packages/selenium-devtools-py/tests/test_selenium_surface.py b/packages/selenium-devtools-py/tests/test_selenium_surface.py index 25d5b086..e36a5e4e 100644 --- a/packages/selenium-devtools-py/tests/test_selenium_surface.py +++ b/packages/selenium-devtools-py/tests/test_selenium_surface.py @@ -11,18 +11,20 @@ rename or relocation, not to test selenium's behaviour. That prediction was already history when these first ran: selenium 4.44 shipped -the regenerated layer, and CI (which resolves a newer selenium than a local 3.9 -can install) failed on the first run. `NetworkEvent` is gone from `bidi.network` -and `Network.conn` is now `_conn`, so network capture has been silently dead on -4.44+ — no error, just an empty Network tab behind one warning. The observe-only -replacement is `Network._event_manager.add_event_handler`, whose callback takes a -deserialized event object rather than `.params`, so adopting it is a port and not -a rename (issue #293). Until that lands the extra is capped below 4.44. +the regenerated layer, and CI (which resolves a newer selenium than the local +python could install) failed on the first run, on a breakage that had already +shipped. The adapter now targets that layer, and these guard it. + +Skipped below the version the package requires, rather than failing: the floor +is declared in `pyproject.toml`, and a developer whose environment predates it +should be told these did not run, not handed a wall of failures about attributes +their selenium was never going to have. CI installs from that floor, so they run +where they matter. Skipped when selenium is absent. That is not free: the CI job must install the adapter's own runtime dependency or these never run where they are meant to protect. selenium is an OPTIONAL extra of this package, so the job must select it — -`pip install -e '.[selenium]'` in `.github/workflows/python.yml`. A plain +`pip install -e '.[test]'` in `.github/workflows/python.yml`. A plain `pip install -e .` installs nothing and every guard here silently skips. """ @@ -30,89 +32,98 @@ import inspect import unittest -from selenium_devtools.constants import SELENIUM_NETWORK_SURFACE_MOVED_AT +from selenium_devtools.constants import SELENIUM_MINIMUM_VERSION from selenium_devtools.utils import selenium_version _HAS_SELENIUM = importlib.util.find_spec("selenium") is not None - -# The version fact itself lives in constants.py, because bidi.py needs it too — -# it is what turns the runtime degradation into a warning naming the version. -_NETWORK_SURFACE_MOVED = selenium_version() >= SELENIUM_NETWORK_SURFACE_MOVED_AT - - -@unittest.skipUnless(_HAS_SELENIUM, "selenium is not installed") -class TestTheSupportedSeleniumRange(unittest.TestCase): - def test_the_installed_selenium_still_carries_the_network_surface(self): - """One legible failure for the whole network breakage. - - The two guards below are skipped past that version so this is the only - thing that reports it — a reader gets the version and the reason, not two - stack traces about a missing name and a missing attribute. - - This failing is NOT a broken install: the user-facing extra is uncapped, - so a developer can legitimately have a newer selenium here. It means the - adapter has not caught up, and `bidi.py` says the same thing at runtime - to the user who is actually losing the Network tab.""" - installed = selenium_version() - - self.assertLess( - installed, - SELENIUM_NETWORK_SURFACE_MOVED_AT, - f"selenium {'.'.join(str(p) for p in installed)} is at or past " - f"{'.'.join(str(p) for p in SELENIUM_NETWORK_SURFACE_MOVED_AT)}, " - "which regenerated the BiDi layer: bidi.py subscribes to network " - "events through NetworkEvent and Network.conn, and neither exists " - "any more, so network capture silently degrades to nothing. Console " - "capture and the preload are unaffected. Porting to the new " - "_event_manager surface is issue #293; the `test` extra pins below " - "this release so CI runs the guards on a supported selenium.", - ) +_BELOW_MINIMUM = selenium_version() < SELENIUM_MINIMUM_VERSION +_TOO_OLD = ( + f"selenium is below the {'.'.join(str(p) for p in SELENIUM_MINIMUM_VERSION)} " + "this package requires" +) @unittest.skipUnless(_HAS_SELENIUM, "selenium is not installed") -@unittest.skipIf( - _NETWORK_SURFACE_MOVED, - "selenium is past the supported range — TestTheSupportedSeleniumRange reports it", -) -class TestTheNetworkInternalsTheAdapterUses(unittest.TestCase): - """`bidi.py` reaches through `driver.network.conn` to subscribe WITHOUT - interception — selenium's `add_request_handler` registers an intercept even - in its high-level style, which pauses each request until selenium continues - it. That is a deliberate trade of public API for not stalling a user's page - loads, and it is what these pin. - - Only this class is gated on the cap: the console, preload and driver-channel - guards below still hold on newer selenium, and skipping them there would - drop the coverage exactly where a bump is most likely to move something.""" - - def test_the_network_channel_still_carries_the_low_level_connection(self): +@unittest.skipIf(_BELOW_MINIMUM, _TOO_OLD) +class TestTheRegeneratedNetworkSurface(unittest.TestCase): + """What `_subscribe_via_event_manager` needs to exist. + + `EVENT_CONFIGS` is a public class attribute and `add_event_handler` a public + method, so this is a supported-API dependency rather than reaching inside. + What is NOT public is that one deserializer is built per BiDi event and held + in a map, which is what `_add_raw_event_handler` swaps and restores. That is + the fragile part, and the shape assertions below are what would catch it + changing.""" + + def test_the_public_event_handler_api_is_present(self): from selenium.webdriver.common.bidi.network import Network - # `driver.network.conn` is the whole reason this module reaches inside. - # Asserted against the constructed object rather than the source text: - # what the adapter depends on is that `.conn` is reachable and is the - # connection it was given, not how selenium happens to write the - # assignment. A sentinel stands in for the connection — no session is - # needed to answer the question. - self.assertIn("conn", inspect.signature(Network.__init__).parameters) - - sentinel = object() - self.assertIs(Network(sentinel).conn, sentinel) - - def test_the_event_and_session_types_are_where_the_adapter_imports_them(self): - from selenium.webdriver.common.bidi.network import NetworkEvent - from selenium.webdriver.common.bidi.session import Session + self.assertTrue(callable(getattr(Network, "add_event_handler", None))) + self.assertIsInstance(getattr(Network, "EVENT_CONFIGS", None), dict) + + def test_the_event_manager_carries_what_registration_calls(self): + from selenium.webdriver.common.bidi._event_manager import _EventManager + + # `_add_raw_event_handler` is this class's own add_event_handler body + # with the deserializer passed in rather than looked up, so it calls + # exactly these. Private, and pinned for that reason. + self.assertIn("conn", inspect.signature(_EventManager.__init__).parameters) + for method in ("subscribe_to_event", "add_callback_to_tracking"): + self.assertTrue(callable(getattr(_EventManager, method, None)), method) + + def test_a_registration_can_be_unwound(self): + """A failed attach undoes what it did. Without these, an abandoned + callback keeps the event's count above zero and a later consumer can + never unsubscribe it.""" + from selenium.webdriver.common.bidi._event_manager import _EventManager + from selenium.webdriver.remote.websocket_connection import WebSocketConnection + + self.assertTrue(callable(getattr(WebSocketConnection, "remove_callback", None))) + for method in ("remove_callback_from_tracking", "unsubscribe_from_event"): + self.assertTrue(callable(getattr(_EventManager, method, None)), method) + + # The unwind relies on this staying conditional: it must not tear down a + # subscription another consumer still has callbacks on. + source = inspect.getsource(_EventManager.unsubscribe_from_event) + self.assertIn('entry["callbacks"]', source) + + def test_the_connection_deserializes_per_callback(self): + """The property the whole design rests on: `add_callback` closes over the + deserializer it is HANDED. If it ever resolved one per event instead, the + adapter could no longer keep raw params to itself and would be back to + publishing its own into shared state.""" + from selenium.webdriver.remote.websocket_connection import WebSocketConnection + + source = inspect.getsource(WebSocketConnection.add_callback) + + # The callback body must call from_json on the passed-in event object. + self.assertIn("event.from_json", source) + self.assertIn("event.event_class", source) + + def test_the_generated_event_classes_are_still_lossy(self): + """The reason raw `dict` configs are registered at all. + + If selenium ever models the full event, this fails and the registration + can go — the typed object would then carry the request and timestamp. It + failing is good news, not a break.""" + import dataclasses + + from selenium.webdriver.common.bidi.network import ( + BeforeRequestSentParameters, + ) - # Constructed as `NetworkEvent(name)` and `Session(conn).subscribe(...)`. - self.assertTrue(callable(NetworkEvent)) - self.assertTrue(hasattr(Session, "subscribe")) + if not dataclasses.is_dataclass(BeforeRequestSentParameters): + self.skipTest("no longer a dataclass — registration needs rechecking") + declared = {f.name for f in dataclasses.fields(BeforeRequestSentParameters)} + self.assertNotIn("request", declared) + self.assertNotIn("timestamp", declared) @unittest.skipUnless(_HAS_SELENIUM, "selenium is not installed") class TestTheChannelsThatSurvivedTheBiDiRegeneration(unittest.TestCase): """Console capture and the document-start preload go through `driver.script`, - which 4.44's regeneration left alone. Deliberately NOT gated on the cap, so - these keep guarding on whatever selenium is installed.""" + which 4.44's regeneration left alone. Deliberately NOT gated on the version + floor, so these keep guarding on whatever selenium is installed.""" def test_the_driver_exposes_the_bidi_channels(self): from selenium.webdriver.remote.webdriver import WebDriver