From 92af76f815bc26d1c499609e9cac21a9e7e63643 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Thu, 17 Sep 2026 19:32:54 +0530 Subject: [PATCH 1/8] fix: publish a real viewport from every adapter --- .changeset/publish-a-real-viewport.md | 17 +++ packages/core/src/index.ts | 1 + packages/core/src/viewport.ts | 99 +++++++++++++++++ packages/core/tests/viewport.test.ts | 103 ++++++++++++++++++ .../nightwatch-devtools/src/session-init.ts | 22 +++- .../tests/viewport-metadata.test.ts | 75 +++++++++++++ .../src/selenium_devtools/instrumentation.py | 22 +++- .../src/selenium_devtools/types.py | 8 +- .../tests/test_instrumentation.py | 28 ++++- .../src/helpers/driverMetadata.ts | 35 +++++- .../tests/driverMetadata.test.ts | 80 +++++++++++++- packages/service/src/session-metadata.ts | 47 +++----- 12 files changed, 487 insertions(+), 50 deletions(-) create mode 100644 .changeset/publish-a-real-viewport.md create mode 100644 packages/core/src/viewport.ts create mode 100644 packages/core/tests/viewport.test.ts create mode 100644 packages/nightwatch-devtools/tests/viewport-metadata.test.ts diff --git a/.changeset/publish-a-real-viewport.md b/.changeset/publish-a-real-viewport.md new file mode 100644 index 00000000..ce3436ea --- /dev/null +++ b/.changeset/publish-a-real-viewport.md @@ -0,0 +1,17 @@ +--- +"@wdio/devtools-service": patch +"@wdio/selenium-devtools": patch +"@wdio/nightwatch-devtools": patch +--- + +Publish the viewport from every adapter. Selenium and Nightwatch published none at all, so `trace.metadata.viewport` was absent for every trace either produced and the exporter fell back to a hard-coded 1280x720 in three places. That fallback is what the player lays the DOM-replay iframe out at, so **every** Selenium and Nightwatch trace was replayed at 1280x720 regardless of the window the run actually used. Not a mobile problem: a desktop run at 2560x1440 was framed just as wrongly, which is presumably why it went unnoticed — the proportions are plausible. + +The read has one home now, `resolveViewport` in `core`, because all three JS adapters need it. Two probes, only one of which exists at a time: a page measures itself through `visualViewport` — the only read carrying the real scale and offsets — and a native app has no page to ask, so the device's own window is the only answer. `isNativeAppSession` settles which, so the branch was already decided. + +Each adapter supplies its own probes, and the care is in how: Selenium reads through the **unpatched** `getDriverOriginals()` and Nightwatch over its raw WebDriver transport, because both implement these as ordinary commands — through the patched path every run would open with an `executeScript` or `getWindowRect` row of our own making, and Nightwatch's would additionally sit behind the command in flight on its own queue. + +The script reads the `visualViewport` fields one by one rather than returning the object: it is a host object, and a driver that serializes it structurally hands back `{}`, which would read as a successful empty measurement rather than a failed one. A read that answers nothing usable omits the viewport rather than publishing a zero-sized one, and a failure degrades to no viewport rather than failing the session. + +The Python adapter already published one, but only `width`/`height` from `innerWidth`/`innerHeight`, so it lost the scale and offsets the shared `Viewport` declares; it now takes the same `visualViewport` read as the others. + +Also corrects the claim, in the comment that survived, that this field is metadata only. It is load-bearing geometry wherever there is a DOM to replay. diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a97888f5..10bfe00f 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -21,6 +21,7 @@ export * from './screenshot-artifact.js' export * from './video-slice.js' export * from './with-timeout.js' export * from './webdriver-http.js' +export * from './viewport.js' export * from './assert-patcher.js' export * from './element-snapshot.js' export * from './element-scripts.js' diff --git a/packages/core/src/viewport.ts b/packages/core/src/viewport.ts new file mode 100644 index 00000000..25a36ec7 --- /dev/null +++ b/packages/core/src/viewport.ts @@ -0,0 +1,99 @@ +// The geometry a trace is replayed at. +// +// `metadata.viewport` is not descriptive: `trace-exporter` falls back to a +// hard-coded 1280x720 when it is absent, and the player lays the DOM-replay +// iframe out at whatever it finds. An adapter that publishes none therefore +// frames every run — desktop included — at proportions it never had. +// +// Two reads, because only one of them exists at a time: a page measures itself +// through `visualViewport`, and a native app has no page to ask, so the +// device's own window is the only answer. `isNativeAppSession` settles which. + +import { isNativeAppSession, type Viewport } from '@wdio/devtools-shared' + +/** + * A function BODY, which is what every driver's script endpoint takes. + * + * Reads the fields one by one rather than returning `window.visualViewport` + * itself: it is a host object, and a driver that serializes it structurally + * hands back `{}` — the richer read then looks like a successful empty one. + */ +export const VISUAL_VIEWPORT_SCRIPT = ` + var v = window.visualViewport + if (!v) { return null } + return { + width: v.width, + height: v.height, + offsetLeft: v.offsetLeft, + offsetTop: v.offsetTop, + scale: v.scale + } +` + +export interface ViewportProbes { + /** Runs a function body in the page. Omitted when the adapter has no way to. */ + runScript?: (body: string) => Promise + /** The device or OS window — the native session's only measurable surface. */ + getWindowSize?: () => Promise + onWarn?: (message: string) => void +} + +const size = (value: unknown): number | undefined => + typeof value === 'number' && Number.isFinite(value) && value > 0 + ? value + : undefined + +/** A window rect carries no scale or offset — a native app is not scrolled or + * pinch-zoomed, so the neutral values are the true ones rather than padding. */ +function fromWindow(raw: unknown): Viewport | undefined { + const rect = raw as { width?: unknown; height?: unknown } | undefined + const width = size(rect?.width) + const height = size(rect?.height) + return width && height + ? { width, height, offsetLeft: 0, offsetTop: 0, scale: 1 } + : undefined +} + +function fromVisualViewport(raw: unknown): Viewport | undefined { + const v = raw as Record | undefined + const width = size(v?.width) + const height = size(v?.height) + if (!width || !height) { + return undefined + } + return { + width, + height, + offsetLeft: typeof v?.offsetLeft === 'number' ? v.offsetLeft : 0, + offsetTop: typeof v?.offsetTop === 'number' ? v.offsetTop : 0, + scale: size(v?.scale) ?? 1 + } +} + +/** + * The session's viewport, or undefined when it cannot be read. + * + * Degrades rather than throwing: a capture without geometry is still worth + * keeping, and this runs while the session is being brought up. + */ +export async function resolveViewport( + capabilities: unknown, + probes: ViewportProbes +): Promise { + const native = isNativeAppSession(capabilities) + try { + if (native) { + return probes.getWindowSize + ? fromWindow(await probes.getWindowSize()) + : undefined + } + return probes.runScript + ? fromVisualViewport(await probes.runScript(VISUAL_VIEWPORT_SCRIPT)) + : undefined + } catch (err) { + probes.onWarn?.( + `Could not resolve the session viewport: ${(err as Error).message}` + ) + return undefined + } +} diff --git a/packages/core/tests/viewport.test.ts b/packages/core/tests/viewport.test.ts new file mode 100644 index 00000000..0c8e2521 --- /dev/null +++ b/packages/core/tests/viewport.test.ts @@ -0,0 +1,103 @@ +import { describe, it, expect, vi } from 'vitest' +import { resolveViewport, VISUAL_VIEWPORT_SCRIPT } from '../src/viewport.js' + +const WEB = { browserName: 'chrome', platformName: 'linux' } +const NATIVE = { + platformName: 'Android', + 'appium:automationName': 'UiAutomator2' +} + +describe('resolveViewport on a page', () => { + it('reads the visual viewport, keeping scale and offsets', async () => { + const runScript = vi.fn().mockResolvedValue({ + width: 390, + height: 664, + offsetLeft: 0, + offsetTop: 12, + scale: 2 + }) + await expect(resolveViewport(WEB, { runScript })).resolves.toEqual({ + width: 390, + height: 664, + offsetLeft: 0, + offsetTop: 12, + scale: 2 + }) + expect(runScript).toHaveBeenCalledWith(VISUAL_VIEWPORT_SCRIPT) + }) + + it('defaults scale and offsets when the page omits them', async () => { + const runScript = vi.fn().mockResolvedValue({ width: 1280, height: 720 }) + await expect(resolveViewport(WEB, { runScript })).resolves.toEqual({ + width: 1280, + height: 720, + offsetLeft: 0, + offsetTop: 0, + scale: 1 + }) + }) + + // A driver that structurally serializes the host object hands back `{}`, + // which must read as "no viewport" rather than a zero-sized one. + it('answers undefined for an empty or sizeless read', async () => { + for (const value of [{}, null, undefined, { width: 0, height: 0 }]) { + const runScript = vi.fn().mockResolvedValue(value) + await expect(resolveViewport(WEB, { runScript })).resolves.toBeUndefined() + } + }) + + it('never asks the device window for a page', async () => { + const getWindowSize = vi.fn() + await resolveViewport(WEB, { + runScript: vi.fn().mockResolvedValue({ width: 1, height: 1 }), + getWindowSize + }) + expect(getWindowSize).not.toHaveBeenCalled() + }) +}) + +describe('resolveViewport on a native session', () => { + it('measures the device window instead of a page', async () => { + const runScript = vi.fn() + const getWindowSize = vi + .fn() + .mockResolvedValue({ width: 1080, height: 2219 }) + await expect( + resolveViewport(NATIVE, { runScript, getWindowSize }) + ).resolves.toEqual({ + width: 1080, + height: 2219, + offsetLeft: 0, + offsetTop: 0, + scale: 1 + }) + // A native app has no document; the page read can only fail there. + expect(runScript).not.toHaveBeenCalled() + }) + + it('answers undefined when the window cannot be measured', async () => { + const getWindowSize = vi.fn().mockResolvedValue(undefined) + await expect( + resolveViewport(NATIVE, { getWindowSize }) + ).resolves.toBeUndefined() + }) +}) + +describe('resolveViewport degradation', () => { + // The geometry is worth having, but not at the cost of the session. + it('warns and answers undefined rather than throwing', async () => { + const onWarn = vi.fn() + const runScript = vi.fn().mockRejectedValue(new Error('no such window')) + await expect( + resolveViewport(WEB, { runScript, onWarn }) + ).resolves.toBeUndefined() + expect(onWarn).toHaveBeenCalledWith( + expect.stringContaining('Could not resolve the session viewport') + ) + }) + + it('answers undefined when the adapter supplies no probe', async () => { + await expect(resolveViewport(WEB, {})).resolves.toBeUndefined() + await expect(resolveViewport(NATIVE, {})).resolves.toBeUndefined() + }) +}) diff --git a/packages/nightwatch-devtools/src/session-init.ts b/packages/nightwatch-devtools/src/session-init.ts index ae71d652..0bd13200 100644 --- a/packages/nightwatch-devtools/src/session-init.ts +++ b/packages/nightwatch-devtools/src/session-init.ts @@ -15,6 +15,8 @@ */ import logger from '@wdio/logger' +import { resolveViewport } from '@wdio/devtools-core' +import { webdriverExecute, webdriverGet } from './helpers/webdriverHttp.js' import { errorMessage, finalizeScreencast, @@ -114,10 +116,20 @@ function initReporterChain(ctx: SessionInitCtx): void { ) } -function broadcastSessionMetadata( +/** The run's geometry, over the raw WebDriver transport: `browser.*` commands + * are QUEUED, so a read issued here would sit behind the command in flight. */ +function readViewport(browser: NightwatchBrowser) { + return resolveViewport(browser.capabilities || {}, { + runScript: (body) => webdriverExecute(browser, body), + getWindowSize: () => webdriverGet(browser, 'window/rect'), + onWarn: (message) => log.warn(message) + }) +} + +async function broadcastSessionMetadata( ctx: SessionInitCtx, browser: NightwatchBrowser -): void { +): Promise { const capabilities = browser.capabilities || {} const desiredCapabilities = browser.desiredCapabilities || {} const sessionId = browser.sessionId @@ -128,9 +140,11 @@ function broadcastSessionMetadata( ctx.srcFolders = Array.isArray(sf) ? sf : sf ? [sf] : [] } + const viewport = await readViewport(browser) const metadata = { type: TraceType.Testrunner, capabilities, + ...(viewport ? { viewport } : {}), desiredCapabilities, sessionId, testEnv: opts.testEnv, @@ -314,7 +328,7 @@ async function rebindSessionToBrowser( // Also gates `wrapUrlMethod`, which is per browser OBJECT — cucumber hands // over a new one per scenario. ctx.isScriptInjected = false - broadcastSessionMetadata(ctx, browser) + await broadcastSessionMetadata(ctx, browser) await armReplacedSession(ctx, browser) rotateScreencastForSession(ctx, browser) await ctx.screencastRotation @@ -357,7 +371,7 @@ export async function ensureSessionInitialized( } } initReporterChain(ctx) - broadcastSessionMetadata(ctx, browser) + await broadcastSessionMetadata(ctx, browser) await armCaptureForSession(ctx, browser) await startScreencast(ctx, browser, browser.sessionId) } diff --git a/packages/nightwatch-devtools/tests/viewport-metadata.test.ts b/packages/nightwatch-devtools/tests/viewport-metadata.test.ts new file mode 100644 index 00000000..9ffddc44 --- /dev/null +++ b/packages/nightwatch-devtools/tests/viewport-metadata.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it, vi, beforeEach } from 'vitest' + +// The read has to bypass Nightwatch's command queue, so the adapter goes over +// the raw WebDriver transport — mocked here to assert both the geometry and +// that the queue is never touched. +const webdriverExecute = vi.fn() +const webdriverGet = vi.fn() +vi.mock('../src/helpers/webdriverHttp.js', () => ({ + webdriverExecute: (...args: unknown[]) => webdriverExecute(...args), + webdriverGet: (...args: unknown[]) => webdriverGet(...args), + webdriverPost: vi.fn(), + resolveWebDriverAddress: vi.fn() +})) + +const { resolveViewport } = await import('@wdio/devtools-core') + +const WEB_CAPS = { browserName: 'chrome' } +const NATIVE_CAPS = { + platformName: 'Android', + 'appium:automationName': 'UiAutomator2' +} + +/** The adapter's own probe wiring, mirrored so the transport choice is what is + * under test rather than the surrounding session bringup. */ +function readViewport(capabilities: Record) { + return resolveViewport(capabilities, { + runScript: (body: string) => webdriverExecute(body), + getWindowSize: () => webdriverGet('window/rect') + }) +} + +beforeEach(() => { + webdriverExecute.mockReset() + webdriverGet.mockReset() +}) + +describe('nightwatch viewport metadata (#373)', () => { + it('reads the page viewport over the raw transport', async () => { + webdriverExecute.mockResolvedValue({ + width: 1440, + height: 778, + offsetLeft: 0, + offsetTop: 0, + scale: 1 + }) + await expect(readViewport(WEB_CAPS)).resolves.toEqual({ + width: 1440, + height: 778, + offsetLeft: 0, + offsetTop: 0, + scale: 1 + }) + expect(webdriverExecute).toHaveBeenCalledOnce() + expect(webdriverGet).not.toHaveBeenCalled() + }) + + it('asks the device window on a native session', async () => { + webdriverGet.mockResolvedValue({ width: 1080, height: 2219 }) + await expect(readViewport(NATIVE_CAPS)).resolves.toEqual({ + width: 1080, + height: 2219, + offsetLeft: 0, + offsetTop: 0, + scale: 1 + }) + expect(webdriverGet).toHaveBeenCalledWith('window/rect') + expect(webdriverExecute).not.toHaveBeenCalled() + }) + + // The transport answers null for any failure rather than rejecting. + it('answers undefined when the transport returns null', async () => { + webdriverExecute.mockResolvedValue(null) + await expect(readViewport(WEB_CAPS)).resolves.toBeUndefined() + }) +}) diff --git a/packages/selenium-devtools-py/src/selenium_devtools/instrumentation.py b/packages/selenium-devtools-py/src/selenium_devtools/instrumentation.py index 6a81cf9c..e9400a2c 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/instrumentation.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/instrumentation.py @@ -633,18 +633,30 @@ def _viewport(driver: Any) -> Optional[Viewport]: return _driver_window(driver) run = _guarded_execute_script(driver) try: - size = run("return [window.innerWidth, window.innerHeight]") + # `visualViewport`, matching the JS adapters: it is the only read that + # carries the real scale and offsets, which `innerWidth` cannot express. + size = run( + "var v = window.visualViewport;" + " if (!v) { return null }" + " return [v.width, v.height, v.offsetLeft, v.offsetTop, v.scale]" + ) except Exception as exc: # noqa: BLE001 — a default frame, not a failed run _log.debug("viewport read failed: %s", exc) return None - if not isinstance(size, list) or len(size) != 2: + if not isinstance(size, list) or len(size) != 5: return None - width, height = size - if not isinstance(width, int) or not isinstance(height, int): + width, height, offset_left, offset_top, scale = size + if not isinstance(width, (int, float)) or not isinstance(height, (int, float)): return None if width <= 0 or height <= 0: return None - return {"width": width, "height": height} + return { + "width": int(width), + "height": int(height), + "offsetLeft": offset_left if isinstance(offset_left, (int, float)) else 0, + "offsetTop": offset_top if isinstance(offset_top, (int, float)) else 0, + "scale": scale if isinstance(scale, (int, float)) and scale > 0 else 1, + } def _ensure_session_setup(driver: Any, capturer: SessionCapturer) -> Optional[dict]: diff --git a/packages/selenium-devtools-py/src/selenium_devtools/types.py b/packages/selenium-devtools-py/src/selenium_devtools/types.py index 82a421a3..8a00620f 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/types.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/types.py @@ -37,9 +37,15 @@ class CommandLog(TypedDict, total=False): testUid: str -class Viewport(TypedDict): +class Viewport(TypedDict, total=False): + """Mirrors shared's `Viewport`. The offsets and scale are absent on a native + session, which has no page to be scrolled or pinch-zoomed.""" + width: int height: int + offsetLeft: float + offsetTop: float + scale: float class ElementScripts(TypedDict): diff --git a/packages/selenium-devtools-py/tests/test_instrumentation.py b/packages/selenium-devtools-py/tests/test_instrumentation.py index 3ded406f..bbbc0758 100644 --- a/packages/selenium-devtools-py/tests/test_instrumentation.py +++ b/packages/selenium-devtools-py/tests/test_instrumentation.py @@ -1116,11 +1116,13 @@ def __init__(self, size=None): super().__init__() self.session_id = "sess-9" # already-initialized session self.scripts = [] - self._size = size if size is not None else [1280, 1024] + # [width, height, offsetLeft, offsetTop, scale] — the visualViewport + # read, which alone carries the scale a pinch-zoomed page replays at. + self._size = size if size is not None else [1280, 1024, 0, 0, 1] def execute_script(self, script, *args): self.scripts.append(script) - return self._size if "innerWidth" in script else None + return self._size if "visualViewport" in script else None class TestViewportMetadata(unittest.TestCase): @@ -1140,7 +1142,16 @@ def test_the_session_metadata_carries_the_real_viewport(self): driver.execute("get", {"url": "https://x/"}) [meta] = self._metadata() - self.assertEqual(meta["viewport"], {"width": 1280, "height": 1024}) + self.assertEqual( + meta["viewport"], + { + "width": 1280, + "height": 1024, + "offsetLeft": 0, + "offsetTop": 0, + "scale": 1, + }, + ) def test_the_probe_does_not_become_a_command_row(self): # Unguarded it re-enters the same hook and every run opens with an @@ -1212,7 +1223,16 @@ def get_window_size(self): MobileWebDriver().execute("get", {"url": "https://x/"}) [meta] = [d for s, d in tx.sent if s == "metadata"] - self.assertEqual(meta["viewport"], {"width": 1280, "height": 1024}) + self.assertEqual( + meta["viewport"], + { + "width": 1280, + "height": 1024, + "offsetLeft": 0, + "offsetTop": 0, + "scale": 1, + }, + ) def test_a_nonsense_size_is_refused(self): for bad in ([0, 800], [1280, -1], ["1280", 800], [1280], "1280x800"): diff --git a/packages/selenium-devtools/src/helpers/driverMetadata.ts b/packages/selenium-devtools/src/helpers/driverMetadata.ts index 2e506aa9..ec00d4bc 100644 --- a/packages/selenium-devtools/src/helpers/driverMetadata.ts +++ b/packages/selenium-devtools/src/helpers/driverMetadata.ts @@ -1,7 +1,8 @@ import logger from '@wdio/logger' -import { errorMessage } from '@wdio/devtools-core' +import { errorMessage, resolveViewport } from '@wdio/devtools-core' import { TraceType } from '@wdio/devtools-shared' import { SELENIUM_RUNNER_ID } from '../constants.js' +import { getDriverOriginals } from '../driverPatcher.js' import type { SeleniumDriverLike } from '../types.js' const log = logger('@wdio/selenium-devtools:driverMetadata') @@ -92,6 +93,33 @@ function logBrowserBoot( log.info(`Driver session created in ${Date.now() - driverReadyTs}ms`) } +/** + * The run's geometry, read through the UNPATCHED driver methods: selenium + * implements both as ordinary commands, so the patched ones would open every + * run with an `executeScript` or `getWindowRect` row of our own making. + */ +function readViewport(driver: SeleniumDriverLike, capabilities: unknown) { + const orig = getDriverOriginals() + return resolveViewport(capabilities, { + runScript: orig.executeScript + ? (body) => orig.executeScript!(driver, body) + : undefined, + getWindowSize: orig.manage + ? async () => { + // `manage()` is typed as unknown here — the window handle is a + // selenium internal this package deliberately does not model. + const window = ( + orig.manage!(driver) as { + window?: () => { getRect?: () => unknown } + } + ).window?.() + return window?.getRect?.() + } + : undefined, + onWarn: (message) => log.warn(message) + }) +} + /** * Extract session id + a fully-built upstream-metadata payload from a freshly * created Selenium driver. Logs the standard `Browser:`/`Capabilities sent:`/ @@ -111,11 +139,14 @@ export async function buildDriverMetadata( const sessionId = session?.getId?.() ?? undefined const capGet = makeCapGet(capabilities) logBrowserBoot(capGet, sessionId, driverReadyTs) + const caps = serializeCapabilities(capabilities) + const viewport = await readViewport(driver, caps) return { sessionId, metadata: { type: TraceType.Testrunner, - capabilities: serializeCapabilities(capabilities), + capabilities: caps, + ...(viewport ? { viewport } : {}), sessionId, runner: SELENIUM_RUNNER_ID, options: { diff --git a/packages/selenium-devtools/tests/driverMetadata.test.ts b/packages/selenium-devtools/tests/driverMetadata.test.ts index 33b1855d..3044ff21 100644 --- a/packages/selenium-devtools/tests/driverMetadata.test.ts +++ b/packages/selenium-devtools/tests/driverMetadata.test.ts @@ -1,5 +1,6 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi, afterEach } from 'vitest' import { buildDriverMetadata } from '../src/helpers/driverMetadata.js' +import { getDriverOriginals } from '../src/driverPatcher.js' import { SELENIUM_RUNNER_ID } from '../src/constants.js' import type { SeleniumDriverLike } from '../src/types.js' @@ -24,6 +25,19 @@ function driverStub( } as unknown as SeleniumDriverLike } +/** `getDriverOriginals` hands back the module's own object, so a test sets the + * unpatched methods by writing onto it — and must clear them again, since the + * patcher's state is module-level and outlives one test. */ +function setDriverOriginalsForTest(originals: Record) { + const bag = getDriverOriginals() as Record + for (const key of Object.keys(bag)) { + delete bag[key] + } + Object.assign(bag, originals) +} + +afterEach(() => setDriverOriginalsForTest({})) + async function metadataFor(detectedRunner: string | null) { const { metadata } = await buildDriverMetadata({ driver: driverStub(), @@ -131,3 +145,67 @@ describe('buildDriverMetadata capability serialization', () => { expect(metadata?.capabilities).toEqual({ browserName: 'firefox' }) }) }) + +// #373: without this the exporter falls back to 1280x720 and every Selenium +// trace — desktop included — is replayed at proportions the run never had. +describe('buildDriverMetadata viewport', () => { + it('publishes the page viewport through the unpatched executeScript', async () => { + const executeScript = vi.fn().mockResolvedValue({ + width: 1512, + height: 857, + offsetLeft: 0, + offsetTop: 0, + scale: 1 + }) + setDriverOriginalsForTest({ executeScript }) + const { metadata } = await buildDriverMetadata({ + driver: driverStub(), + driverReadyTs: Date.now(), + detectedRunner: 'mocha' + }) + expect((metadata as { viewport?: unknown }).viewport).toEqual({ + width: 1512, + height: 857, + offsetLeft: 0, + offsetTop: 0, + scale: 1 + }) + }) + + it('measures the device window on a native session', async () => { + const executeScript = vi.fn() + const manage = vi.fn().mockReturnValue({ + window: () => ({ getRect: () => ({ width: 1080, height: 2219 }) }) + }) + setDriverOriginalsForTest({ executeScript, manage }) + const { metadata } = await buildDriverMetadata({ + driver: driverStub('sess-native', { + platformName: 'Android', + 'appium:automationName': 'UiAutomator2' + }), + driverReadyTs: Date.now(), + detectedRunner: 'mocha' + }) + expect((metadata as { viewport?: unknown }).viewport).toEqual({ + width: 1080, + height: 2219, + offsetLeft: 0, + offsetTop: 0, + scale: 1 + }) + expect(executeScript).not.toHaveBeenCalled() + }) + + it('omits the viewport rather than failing when the read throws', async () => { + setDriverOriginalsForTest({ + executeScript: vi.fn().mockRejectedValue(new Error('no such window')) + }) + const { metadata } = await buildDriverMetadata({ + driver: driverStub(), + driverReadyTs: Date.now(), + detectedRunner: 'mocha' + }) + expect(metadata).toBeDefined() + expect('viewport' in (metadata as object)).toBe(false) + }) +}) diff --git a/packages/service/src/session-metadata.ts b/packages/service/src/session-metadata.ts index 7747d3e5..1c1a4e75 100644 --- a/packages/service/src/session-metadata.ts +++ b/packages/service/src/session-metadata.ts @@ -3,9 +3,9 @@ // unit-testable and the plugin only forwards its lifecycle hook. import logger from '@wdio/logger' +import { resolveViewport as coreResolveViewport } from '@wdio/devtools-core' import { deviceFromCapabilities, - isNativeAppSession, type Metadata, type TraceType, type Viewport @@ -15,43 +15,24 @@ import type { Capabilities } from '@wdio/types' const log = logger('@wdio/devtools-service') /** - * Size of the captured surface. A page reports its own visual viewport; a - * native app has no DOM to ask, so the driver's window size is the only answer - * — measured at 1080x2219 on a Pixel 7, which is the window minus the - * navigation bar. + * Size of the captured surface, through core's shared reader so all three JS + * adapters answer this the same way. * - * Metadata only for a native app: neither number matches the screenshot's own - * pixels (that Pixel 7 shot is 1080x2400, and iOS reports points rather than - * pixels), so anything sizing a captured image measures the image instead. It - * is load-bearing wherever there IS a DOM to replay — the player sizes the - * replay iframe from it — so a mobile BROWSER session must reach the page read - * below, which alone carries the real scale and offsets. + * A native app's numbers are descriptive only: neither matches the screenshot's + * own pixels (a Pixel 7 reports 1080x2219 — the window minus the navigation bar + * — against a 1080x2400 shot, and iOS reports points), so anything sizing a + * captured image measures the image instead. Wherever there IS a DOM it is + * load-bearing geometry: the player sizes the replay iframe from it, and the + * exporter falls back to 1280x720 without it. */ async function resolveViewport( browser: WebdriverIO.Browser ): Promise { - try { - if (isNativeAppSession(browser.capabilities)) { - const size = await browser.getWindowSize() - return size - ? { - width: size.width, - height: size.height, - offsetLeft: 0, - offsetTop: 0, - scale: 1 - } - : undefined - } - return (await browser.execute(() => window.visualViewport)) || undefined - } catch (err) { - // A viewport is descriptive, not load-bearing — the capture is still worth - // keeping without it, so this degrades rather than failing the session. - log.warn( - `Could not resolve the session viewport: ${(err as Error).message}` - ) - return undefined - } + return coreResolveViewport(browser.capabilities, { + runScript: (body) => browser.execute(body), + getWindowSize: () => browser.getWindowSize(), + onWarn: (message) => log.warn(message) + }) } /** From fb491183e8b1cdfc89b5c58ec72d3de849493bed Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Fri, 18 Sep 2026 15:44:57 +0530 Subject: [PATCH 2/8] fix: make a live native run visible on the dashboard --- packages/core/src/session-capturer.ts | 99 +++++++++- .../tests/session-capturer-outbox.test.ts | 171 ++++++++++++++++++ packages/service/src/session.ts | 14 +- packages/service/tests/session.test.ts | 50 +++++ 4 files changed, 328 insertions(+), 6 deletions(-) create mode 100644 packages/core/tests/session-capturer-outbox.test.ts diff --git a/packages/core/src/session-capturer.ts b/packages/core/src/session-capturer.ts index 7be4f9b7..2a5e37c4 100644 --- a/packages/core/src/session-capturer.ts +++ b/packages/core/src/session-capturer.ts @@ -51,6 +51,11 @@ export interface SessionCapturerOptions { type ConsoleMethod = (typeof CONSOLE_METHODS)[number] export abstract class SessionCapturerBase { + /** Ceiling on messages held while the socket connects. High enough that a + * normal bringup never reaches it, low enough that a dashboard which never + * answers cannot retain a run's worth of traffic. */ + static readonly MAX_PENDING_UPSTREAM = 1000 + // ── State (mostly private; subclasses access shared ws via `this.ws`) ──── /** * Exposed as `protected` so subclasses with framework-specific close/wait @@ -117,10 +122,17 @@ export abstract class SessionCapturerBase { ) this.ws.on('open', () => { this.#hasConnected = true + this.#flushPending() this.onWsOpen() }) - this.ws.on('error', (err: unknown) => this.onWsError(err)) - this.ws.on('close', () => this.onWsClose()) + this.ws.on('error', (err: unknown) => { + this.#discardPending() + this.onWsError(err) + }) + this.ws.on('close', () => { + this.#discardPending() + this.onWsClose() + }) this.ws.on('message', (raw: Buffer | string) => { try { const parsed = JSON.parse(raw.toString()) @@ -147,14 +159,91 @@ export abstract class SessionCapturerBase { * {@link onUpstreamDrop}. */ sendUpstream(event: string, data: unknown): void { - if (!this.ws || this.ws.readyState !== WebSocket.OPEN) { - this.onUpstreamDrop(event, 'closed') + if (!this.ws) { + this.notifyDrop(event, 'closed') + return + } + // A socket that has not opened YET is not a lost dashboard: a session's + // metadata and its first suites are published while the driver is still + // being created, and against Appium that is ~11 s before the worker socket + // opens. Dropping them silently cost the live dashboard the whole run — + // `metadata.type` gates the test-suite pane and `metadata.device` the + // mobile layout, so both were simply absent with nothing logged. + if (this.ws.readyState === WebSocket.CONNECTING) { + this.#buffer(event, data) + return + } + if (this.ws.readyState !== WebSocket.OPEN) { + this.notifyDrop(event, 'closed') return } try { this.ws.send(JSON.stringify({ scope: event, data })) } catch (err) { - this.onUpstreamDrop(event, 'send-error', err) + this.notifyDrop(event, 'send-error', err) + } + } + + /** Messages published before the socket opened, in the order they were + * published. Capped: a dashboard that never connects must not grow this + * without bound for the length of a run. */ + #pending: { event: string; data: unknown }[] = [] + + #buffer(event: string, data: unknown): void { + if (this.#pending.length >= SessionCapturerBase.MAX_PENDING_UPSTREAM) { + this.notifyDrop(event, 'closed') + return + } + this.#pending.push({ event, data }) + } + + #inDropHandler = false + + /** + * Report a drop, at most one level deep. + * + * An adapter's handler naturally wants to log, and `patchConsole` forwards + * console output upstream — so a handler that logs re-enters `sendUpstream`, + * drops again and recurses until the stack blows. Measured as "Maximum call + * stack size exceeded" raised inside the user's own spec, which points + * nowhere near this code. + */ + protected notifyDrop( + event: string, + reason: 'closed' | 'send-error', + err?: unknown + ): void { + if (this.#inDropHandler) { + return + } + this.#inDropHandler = true + try { + this.onUpstreamDrop(event, reason, err) + } finally { + this.#inDropHandler = false + } + } + + /** A socket that dies before it ever opens is a dashboard that is not coming, + * so its buffer is reported as dropped and released. Without this the + * payloads — screenshots among them — were retained for the run's length and + * the adapter's drop warning never fired, which is the silence the buffer + * exists to end, not to relocate. */ + #discardPending(): void { + const pending = this.#pending + this.#pending = [] + for (const { event } of pending) { + this.notifyDrop(event, 'closed') + } + } + + /** Publish what was buffered while connecting, oldest first — order matters, + * since the app folds each metadata message into the previous one. */ + #flushPending(): void { + const pending = this.#pending + this.#pending = [] + for (const { event, data } of pending) { + this.sendUpstream(event, data) } } diff --git a/packages/core/tests/session-capturer-outbox.test.ts b/packages/core/tests/session-capturer-outbox.test.ts new file mode 100644 index 00000000..0f711a95 --- /dev/null +++ b/packages/core/tests/session-capturer-outbox.test.ts @@ -0,0 +1,171 @@ +/** + * Publishing before the worker socket opens. + * + * A session's metadata and its first suites go out while the driver is still + * being created — against Appium that is ~11 s before the socket opens. Dropped + * silently, the live dashboard lost the whole run: `metadata.type` gates the + * test-suite pane and `metadata.device` the mobile layout, so both were absent + * with nothing logged to say why. + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { SessionCapturerBase } from '../src/session-capturer.js' + +interface FakeSocketLike { + readyState: number + sent: string[] + open: () => void + fail: (event: 'close' | 'error') => void +} + +const { sockets, FakeSocket } = vi.hoisted(() => { + const sockets: FakeSocketLike[] = [] + class FakeSocket { + static readonly CONNECTING = 0 + static readonly OPEN = 1 + readyState = 0 + sent: string[] = [] + handlers: Record void)[]> = {} + + constructor() { + sockets.push(this as unknown as FakeSocketLike) + } + on(event: string, handler: (...args: unknown[]) => void) { + ;(this.handlers[event] ??= []).push(handler) + } + send(payload: string) { + this.sent.push(payload) + } + open() { + this.readyState = 1 + for (const handler of this.handlers.open ?? []) { + handler() + } + } + fail(event: 'close' | 'error') { + this.readyState = 3 + for (const handler of this.handlers[event] ?? []) { + handler(new Error('boom')) + } + } + } + return { sockets, FakeSocket } +}) + +vi.mock('ws', () => ({ WebSocket: FakeSocket })) + +class TestSessionCapturer extends SessionCapturerBase { + drops: string[] = [] + protected override onUpstreamDrop(event: string): void { + this.drops.push(event) + } +} + +const scopes = (socket: FakeSocketLike) => + socket.sent.map((raw) => JSON.parse(raw).scope) + +let capturer: TestSessionCapturer +let socket: FakeSocketLike + +beforeEach(() => { + sockets.length = 0 + capturer = new TestSessionCapturer({ hostname: 'localhost', port: 1234 }) + socket = sockets[sockets.length - 1] +}) + +describe('a message published while the socket is still connecting', () => { + it('is delivered once the socket opens, not discarded', () => { + capturer.sendUpstream('metadata', { type: 'testrunner' }) + expect(socket.sent).toHaveLength(0) + + socket.open() + + expect(scopes(socket)).toEqual(['metadata']) + expect(capturer.drops).toEqual([]) + }) + + // The app folds each metadata message into the previous one, so a flush that + // reordered them would let an earlier partial overwrite a later value. + it('preserves publication order across the flush', () => { + capturer.sendUpstream('metadata', { type: 'testrunner' }) + capturer.sendUpstream('suites', { a: 1 }) + capturer.sendUpstream('commands', { b: 2 }) + + socket.open() + + expect(scopes(socket)).toEqual(['metadata', 'suites', 'commands']) + }) + + it('sends straight through once open', () => { + socket.open() + capturer.sendUpstream('commands', { b: 2 }) + expect(scopes(socket)).toEqual(['commands']) + }) +}) + +describe('the buffer is bounded', () => { + it('drops beyond the cap rather than growing for a run that never connects', () => { + const cap = SessionCapturerBase.MAX_PENDING_UPSTREAM + for (let i = 0; i < cap + 5; i++) { + capturer.sendUpstream('commands', { i }) + } + expect(capturer.drops).toHaveLength(5) + + socket.open() + expect(socket.sent).toHaveLength(cap) + }) +}) + +describe('a socket that closes after opening', () => { + it('drops rather than buffering — the dashboard is gone, not pending', () => { + socket.open() + socket.readyState = 3 // CLOSED + capturer.sendUpstream('commands', { b: 2 }) + expect(capturer.drops).toEqual(['commands']) + }) +}) + +// An adapter's drop handler naturally wants to log, and `patchConsole` +// forwards console output upstream — so a logging handler re-enters +// sendUpstream, drops again and recurses until the stack blows. Observed as +// "Maximum call stack size exceeded" raised inside the user's own spec, which +// points nowhere near the capturer. +describe('a drop handler that logs', () => { + class LoggingCapturer extends SessionCapturerBase { + calls = 0 + protected override onUpstreamDrop(event: string): void { + this.calls++ + // Stands in for `log.warn` reaching patched console and being forwarded. + this.sendUpstream('console', { message: `dropped ${event}` }) + } + } + + it('does not recurse', () => { + const capturer = new LoggingCapturer({ hostname: 'localhost', port: 1234 }) + const socket = sockets[sockets.length - 1] + socket.readyState = 3 // CLOSED — every send drops + + expect(() => capturer.sendUpstream('commands', { a: 1 })).not.toThrow() + // The nested send drops too, but is not reported a second time. + expect(capturer.calls).toBe(1) + }) +}) + +// A socket that dies before it ever opens is a dashboard that is not coming. +// Holding its buffer would retain the payloads — screenshots among them — for +// the run's length AND keep the adapter's drop warning silent, which is the +// silence the buffer exists to end, not to relocate. +describe('a socket that never opens', () => { + it('reports and releases what it was holding', () => { + capturer.sendUpstream('metadata', { type: 'testrunner' }) + capturer.sendUpstream('suites', { a: 1 }) + expect(capturer.drops).toEqual([]) + + socket.fail('close') + + expect(capturer.drops).toEqual(['metadata', 'suites']) + // Released, so a later open cannot replay a dashboard that already gave up. + socket.open() + expect(socket.sent).toHaveLength(0) + }) +}) diff --git a/packages/service/src/session.ts b/packages/service/src/session.ts index 591021ff..a374f503 100644 --- a/packages/service/src/session.ts +++ b/packages/service/src/session.ts @@ -37,6 +37,12 @@ import { directProbes } from './direct-probes.js' const log = logger('@wdio/devtools-service:SessionCapturer') export class SessionCapturer extends SessionCapturerBase { + // No `onUpstreamDrop` override here on purpose. `patchConsole` forwards + // console.warn upstream, so warning about a drop re-enters sendUpstream, + // drops again and recurses until the stack blows — measured as "Maximum call + // stack size exceeded" inside the user's own spec. The buffer added in core + // is what makes drops rare; a diagnostic for the ones that remain has to come + // from somewhere that cannot be captured. #isScriptInjected = false /** Session start wall time for trace event timestamps. */ readonly startWallTime = Date.now() @@ -165,7 +171,13 @@ export class SessionCapturer extends SessionCapturerBase { testUid, stepUid } - if (!isAppiumSession(browser)) { + // A native session takes one too: it is the ONLY visual it can have. There + // is no DOM to replay and no per-action snapshot outside trace mode, so + // skipping it left the player with nothing to show for any command and the + // device pane falling back to desktop browser chrome. A mobile BROWSER + // session keeps the old behaviour — it replays from its mutation stream, + // and a screenshot per command on a phone is ~1.2s of round trip. + if (!isAppiumSession(browser) || isNativeAppSession(browser.capabilities)) { try { commandLogEntry.screenshot = await browser.takeScreenshot() } catch (screenshotError) { diff --git a/packages/service/tests/session.test.ts b/packages/service/tests/session.test.ts index 1f02af03..7217aad6 100644 --- a/packages/service/tests/session.test.ts +++ b/packages/service/tests/session.test.ts @@ -74,6 +74,56 @@ describe('SessionCapturer', () => { expect(capturer.commandsLog[0].screenshot).toBe(mockScreenshot) }) + // A native session has no DOM to replay and no per-action snapshot outside + // trace mode, so this screenshot is the only visual any command can carry — + // without it the player shows nothing per command and the device pane falls + // back to desktop browser chrome. + it('captures one for a native session, which has no other visual', async () => { + const capturer = new SessionCapturer() + const nativeBrowser = { + ...mockBrowser, + isMobile: true, + capabilities: { + platformName: 'Android', + 'appium:automationName': 'UiAutomator2' + } + } + mockBrowser.takeScreenshot.mockResolvedValueOnce('native-shot') + + await capturer.afterCommand( + nativeBrowser as never, + 'click' as never, + ['~btn'], + undefined, + undefined, + undefined + ) + + expect(capturer.commandsLog[0].screenshot).toBe('native-shot') + }) + + // It replays from its mutation stream instead, and a screenshot per command + // on a phone is ~1.2s of round trip. + it('skips one for a mobile browser session', async () => { + const capturer = new SessionCapturer() + const mobileWeb = { + ...mockBrowser, + isMobile: true, + capabilities: { platformName: 'Android', browserName: 'chrome' } + } + + await capturer.afterCommand( + mobileWeb as never, + 'click' as never, + ['#btn'], + undefined, + undefined, + undefined + ) + + expect(capturer.commandsLog[0].screenshot).toBeUndefined() + }) + it('should handle screenshot failures gracefully', async () => { const capturer = new SessionCapturer() mockBrowser.takeScreenshot.mockRejectedValueOnce( From 3b0314e25eb832365f93aaf66ee901ca50efdf41 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Fri, 18 Sep 2026 15:45:07 +0530 Subject: [PATCH 3/8] fix(service): record a passing .not assertion as passed --- packages/service/src/assert-capture.ts | 45 ++++++++++++- packages/service/src/assertion-tracker.ts | 3 +- packages/service/tests/assert-capture.test.ts | 63 ++++++++++++++++++- 3 files changed, 107 insertions(+), 4 deletions(-) diff --git a/packages/service/src/assert-capture.ts b/packages/service/src/assert-capture.ts index a62b4cff..f0f200b8 100644 --- a/packages/service/src/assert-capture.ts +++ b/packages/service/src/assert-capture.ts @@ -159,11 +159,51 @@ function unwrapAsymmetricMatcher(value: unknown): unknown { * screenshot come from the matcher's read command it's coalesced into (see * `coalesceAssertionIntoLastRead`), not from a stack walk here. */ +/** + * Whether the matcher was called through `.not`. + * + * It has to be read out of the message, because nothing else carries it: + * `afterAssertion` is handed `{matcherName, options, result}` and the flag + * lives on the matcher's own `this`. `result.pass` is the RAW matcher answer — + * jest's convention is that `pass` describes the positive assertion and the + * framework inverts it for `.not` — so a passing `.not.toBeDisplayed()` arrives + * as `pass: false` and was recorded as a failed row in a green test. + * + * expect-webdriverio builds the message from that same flag: `Expect $(…) not + * to be displayed`, and `Expected [not]:` labelling the diff. Both are checked + * because the label is suppressed by its own `useNotInLabel` option. + */ +export function assertionWasNegated(message: string | undefined): boolean { + if (!message) { + return false + } + return ( + message.includes('Expected [not]') || /\bExpect\b.*\bnot to\b/.test(message) + ) +} + +/** The message is a thunk that formats a diff; a matcher whose own formatting + * throws must not take the assertion row down with it. */ +function readMessage(message?: () => string): string | undefined { + try { + return message?.() + } catch { + return undefined + } +} + export function expectAssertionToCommandLog( params: ExpectAssertion, - testUid: string | undefined + testUid: string | undefined, + /** The caller already knows the outcome — a matcher that hard-threw is a + * failure whatever its message says. Sniffing it for `.not` could invert a + * real failure into a passing row carrying no error. */ + outcomeIsDecided = false ): CommandLog { const { matcherName, expectedValue, result } = params + const rawPass = result.pass ?? result.result ?? false + const negated = + !outcomeIsDecided && assertionWasNegated(readMessage(result.message)) const rawArgs = expectedValue === undefined ? [] @@ -174,7 +214,8 @@ export function expectAssertionToCommandLog( { method: matcherName, args: rawArgs.map(unwrapAsymmetricMatcher), - passed: result.pass ?? result.result ?? false, + // Inverted for `.not`, so a passing negated matcher is a passing row. + passed: negated ? !rawPass : rawPass, message: result.message }, testUid diff --git a/packages/service/src/assertion-tracker.ts b/packages/service/src/assertion-tracker.ts index 240d308e..4d57c700 100644 --- a/packages/service/src/assertion-tracker.ts +++ b/packages/service/src/assertion-tracker.ts @@ -182,7 +182,8 @@ export class AssertionTracker { expectedValue: pending.expectedValue, result: { pass: false, message: () => message } }, - pending.testUid + pending.testUid, + true ) entry.stepUid = pending.stepUid const capturer = this.#ctx.getCapturer() diff --git a/packages/service/tests/assert-capture.test.ts b/packages/service/tests/assert-capture.test.ts index 8f89b460..16c5ad5b 100644 --- a/packages/service/tests/assert-capture.test.ts +++ b/packages/service/tests/assert-capture.test.ts @@ -9,7 +9,8 @@ import { captureExpectFailure, expectAssertionToCommandLog, toCommandError, - wireAssertCapture + wireAssertCapture, + assertionWasNegated } from '../src/assert-capture.js' import type { SessionCapturer } from '../src/session.js' @@ -215,3 +216,63 @@ describe('expectAssertionToCommandLog', () => { expect(entry).toMatchObject({ command: 'expect.toBeClickable', args: [] }) }) }) + +// expect-webdriverio hands `afterAssertion` the RAW matcher result: `pass` +// answers the POSITIVE assertion and the framework inverts it for `.not`. +// Nothing in the hook params carries `isNot` — it lives on the matcher's own +// `this` — so a passing `.not.*` arrived as `pass: false`, was recorded as a +// failed row inside a green test, and landed in the Errors tab. +describe('a negated matcher (.not)', () => { + // Verbatim shape of enhanceError's output: `Expect ${subject} ${not}to …` + // plus the `Expected [not]` diff label. + const negated = `Expect $(\`#gone\`) not to be displayed + +Expected [not]: true +Received : false` + + const positive = `Expect $(\`#here\`) to be displayed + +Expected: true +Received: false` + + const entryFor = (pass: boolean, message: string | (() => string)) => + expectAssertionToCommandLog( + { + matcherName: 'toBeDisplayed', + result: { + pass, + message: typeof message === 'string' ? () => message : message + } + }, + 'test-1' + ) + + it('records a passing .not assertion as passed', () => { + const entry = entryFor(false, negated) + expect(entry.result).toBe('passed') + expect(entry.error).toBeUndefined() + }) + + it('records a failing .not assertion as failed', () => { + const entry = entryFor(true, negated) + expect(entry.error).toBeDefined() + expect(entry.result).toBeUndefined() + }) + + it('leaves a positive matcher alone in both directions', () => { + expect(entryFor(true, positive).result).toBe('passed') + expect(entryFor(false, positive).error).toBeDefined() + }) +}) + +describe('assertionWasNegated', () => { + it('detects the diff label and the phrase independently', () => { + expect(assertionWasNegated('Expected [not]: true')).toBe(true) + expect(assertionWasNegated('Expect $(`#a`) not to have text')).toBe(true) + }) + + it('does not fire on a positive message or an empty one', () => { + expect(assertionWasNegated('Expect $(`#a`) to be displayed')).toBe(false) + expect(assertionWasNegated(undefined)).toBe(false) + }) +}) From bd744b364bb8d469d0d15354bd6110233c90118b Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Fri, 18 Sep 2026 15:45:19 +0530 Subject: [PATCH 4/8] feat(app): give a live device capture its own column --- packages/app/src/components/workbench.ts | 130 ++++++++++++++++++----- 1 file changed, 103 insertions(+), 27 deletions(-) diff --git a/packages/app/src/components/workbench.ts b/packages/app/src/components/workbench.ts index acece8fc..bc4d2367 100644 --- a/packages/app/src/components/workbench.ts +++ b/packages/app/src/components/workbench.ts @@ -585,6 +585,64 @@ export class DevtoolsWorkbench extends Element { } } + /** + * Live device layout: the capture is a full-height column on the right, and + * everything else stacks to its left — the action list above, the dock below. + * + * Live mode only. The player puts the dock BESIDE the capture, which works + * there because the whole window is the trace. A live dashboard has already + * spent its left edge on the suite tree, so a third column squeezed the dock + * into an unreadable strip and the tab row overflowed under the capture. + */ + #renderLiveDeviceLayout() { + const width = basisPx(this.#dragDevice.getPosition()) + return html` +
+
+
+ ${this.#renderActionsSidebar()} +
+ ${ + // Without this the collapse is one-way here: the toggle lives + // inside the sidebar that just went `hidden`, and the state is + // persisted, so the action list stayed gone across reloads. + this.#renderSidebarRestoreButton() + } + ${ + !this.#toolbarCollapsed && !this.#workbenchSidebarCollapsed + ? this.#dragVertical.getSlider('z-[999] pointer-events-auto') + : nothing + } + ${this.#renderWorkbenchTabs()} +
+ ${ + !this.#toolbarCollapsed + ? this.#dragDevice.getSlider('z-[999] pointer-events-auto') + : nothing + } +
+ ${this.#renderBrowserPane(true)} +
+
+ ` + } + #renderStackedSplit() { return html`
-
- ${this.#renderActionsSidebar()} -
- ${this.#renderSidebarRestoreButton()} ${ - !this.#workbenchSidebarCollapsed - ? this.#dragHorizontal.getSlider('z-30') + // The live device layout owns the whole row: it stacks the action + // list and the dock in one column beside the capture, so the sidebar + // is rendered inside it rather than as a sibling here. + this.#liveDeviceLayout ? this.#renderLiveDeviceLayout() : nothing + } + ${this.#liveDeviceLayout ? nothing : this.#renderRowSplit()} +
+ ` + } + + #renderRowSplit() { + return html` +
+ ${this.#renderActionsSidebar()} +
+ ${this.#renderSidebarRestoreButton()} + ${ + !this.#workbenchSidebarCollapsed + ? this.#dragHorizontal.getSlider('z-30') + : nothing + } +
+ ${ + this.playerMode + ? html`` : nothing } -
- ${ - this.playerMode - ? html`` - : nothing - } - ${ - this.#deviceLayout - ? this.#renderDeviceSplit() - : this.#renderStackedSplit() - } -
+ ${ + this.#deviceLayout + ? this.#renderDeviceSplit() + : this.#renderStackedSplit() + }
` } + + /** The capture-as-right-column arrangement, live only — the player keeps the + * dock beside the capture. */ + get #liveDeviceLayout(): boolean { + return this.#deviceLayout && !this.playerMode + } } declare global { From 90e915a97506f500cf0cb9697ce3d81a3da694ce Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Fri, 18 Sep 2026 15:45:32 +0530 Subject: [PATCH 5/8] fix(selenium-devtools-py): never capture this package's own tests --- packages/selenium-devtools-py/pyproject.toml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/selenium-devtools-py/pyproject.toml b/packages/selenium-devtools-py/pyproject.toml index eff2f6be..49aaa8ca 100644 --- a/packages/selenium-devtools-py/pyproject.toml +++ b/packages/selenium-devtools-py/pyproject.toml @@ -43,3 +43,12 @@ selenium_devtools = "selenium_devtools.pytest_plugin" [tool.hatch.build.targets.wheel] packages = ["src/selenium_devtools"] + +# This package's own unit tests must never capture themselves. The plugin is a +# pytest11 entry point, so it loads in every run here, and its last resort is +# the environment — a `DEVTOOLS_PORT` or `DEVTOOLS_ENABLE` exported for a demo +# would otherwise opt the unit suite in and open a dashboard window per run. +# An explicit `false` is the project's answer and the environment cannot +# overturn it, which a bare unset value could not guarantee. +[tool.pytest.ini_options] +devtools = false From e678275eb119c70eef0f415c7a49242a9c77dc2d Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Fri, 18 Sep 2026 16:20:51 +0530 Subject: [PATCH 6/8] fix(service): take the native command screenshot in live mode only --- packages/service/src/index.ts | 1 + packages/service/src/session.ts | 24 ++++++++++++++------- packages/service/tests/session.test.ts | 29 +++++++++++++++++++++++++- 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/packages/service/src/index.ts b/packages/service/src/index.ts index 08116b59..70a13f3d 100644 --- a/packages/service/src/index.ts +++ b/packages/service/src/index.ts @@ -231,6 +231,7 @@ export default class DevToolsHookService implements Services.ServiceInstance { this.#sessionCapturer = new SessionCapturer( wdioCaps['wdio:devtoolsOptions'] ) + this.#sessionCapturer.traceMode = this.#options.mode ?? 'live' stampRunnerMetadata(this.#sessionCapturer, browser, this.captureType) if (this.#options.captureAssertions !== false) { diff --git a/packages/service/src/session.ts b/packages/service/src/session.ts index a374f503..8cd17b5e 100644 --- a/packages/service/src/session.ts +++ b/packages/service/src/session.ts @@ -31,6 +31,7 @@ import { loadInjectableScript, type CapturedPerformancePayload } from '@wdio/devtools-core' +import type { DevToolsMode } from '@wdio/devtools-shared' import type { CommandLog } from './types.js' import { directProbes } from './direct-probes.js' @@ -43,6 +44,11 @@ export class SessionCapturer extends SessionCapturerBase { // stack size exceeded" inside the user's own spec. The buffer added in core // is what makes drops rare; a diagnostic for the ones that remain has to come // from somewhere that cannot be captured. + /** Set by the plugin at session start. Mirrors the field nightwatch's + * capturer already carries, so the capture path can tell the two modes + * apart without reaching back into the plugin's options. */ + traceMode: DevToolsMode = 'live' + #isScriptInjected = false /** Session start wall time for trace event timestamps. */ readonly startWallTime = Date.now() @@ -171,13 +177,17 @@ export class SessionCapturer extends SessionCapturerBase { testUid, stepUid } - // A native session takes one too: it is the ONLY visual it can have. There - // is no DOM to replay and no per-action snapshot outside trace mode, so - // skipping it left the player with nothing to show for any command and the - // device pane falling back to desktop browser chrome. A mobile BROWSER - // session keeps the old behaviour — it replays from its mutation stream, - // and a screenshot per command on a phone is ~1.2s of round trip. - if (!isAppiumSession(browser) || isNativeAppSession(browser.capabilities)) { + // A native session takes one in LIVE mode, where it is the only visual it + // can have: no DOM to replay, and the per-action snapshot is trace-only, so + // skipping it left the player with nothing for any command and the device + // pane falling back to desktop browser chrome. Trace mode is excluded + // because `captureActionResult` already screenshots the same command — two + // Appium round trips at ~1.2s each is the cost #351 exists to remove. A + // mobile BROWSER session keeps the old behaviour throughout: it replays + // from its mutation stream. + const nativeLiveCapture = + this.traceMode !== 'trace' && isNativeAppSession(browser.capabilities) + if (!isAppiumSession(browser) || nativeLiveCapture) { try { commandLogEntry.screenshot = await browser.takeScreenshot() } catch (screenshotError) { diff --git a/packages/service/tests/session.test.ts b/packages/service/tests/session.test.ts index 7217aad6..59de683b 100644 --- a/packages/service/tests/session.test.ts +++ b/packages/service/tests/session.test.ts @@ -78,8 +78,9 @@ describe('SessionCapturer', () => { // trace mode, so this screenshot is the only visual any command can carry — // without it the player shows nothing per command and the device pane falls // back to desktop browser chrome. - it('captures one for a native session, which has no other visual', async () => { + it('captures one for a native session in live mode, its only visual', async () => { const capturer = new SessionCapturer() + capturer.traceMode = 'live' const nativeBrowser = { ...mockBrowser, isMobile: true, @@ -102,6 +103,32 @@ describe('SessionCapturer', () => { expect(capturer.commandsLog[0].screenshot).toBe('native-shot') }) + // `captureActionResult` already screenshots the same command in trace mode; + // two Appium round trips at ~1.2s each is the cost #351 exists to remove. + it('skips one for a native session in trace mode', async () => { + const capturer = new SessionCapturer() + capturer.traceMode = 'trace' + const nativeBrowser = { + ...mockBrowser, + isMobile: true, + capabilities: { + platformName: 'Android', + 'appium:automationName': 'UiAutomator2' + } + } + + await capturer.afterCommand( + nativeBrowser as never, + 'click' as never, + ['~btn'], + undefined, + undefined, + undefined + ) + + expect(capturer.commandsLog[0].screenshot).toBeUndefined() + }) + // It replays from its mutation stream instead, and a screenshot per command // on a phone is ~1.2s of round trip. it('skips one for a mobile browser session', async () => { From 36d20dafd97f3d183a166befafec5d96b41da4a3 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Fri, 18 Sep 2026 16:41:06 +0530 Subject: [PATCH 7/8] fix(service): detect .not from the diff block, not the prose --- packages/service/src/assert-capture.ts | 33 ++++++-- packages/service/tests/assert-capture.test.ts | 80 +++++++++++++------ 2 files changed, 80 insertions(+), 33 deletions(-) diff --git a/packages/service/src/assert-capture.ts b/packages/service/src/assert-capture.ts index f0f200b8..1a27d1ac 100644 --- a/packages/service/src/assert-capture.ts +++ b/packages/service/src/assert-capture.ts @@ -169,17 +169,30 @@ function unwrapAsymmetricMatcher(value: unknown): unknown { * framework inverts it for `.not` — so a passing `.not.toBeDisplayed()` arrives * as `pass: false` and was recorded as a failed row in a green test. * - * expect-webdriverio builds the message from that same flag: `Expect $(…) not - * to be displayed`, and `Expected [not]:` labelling the diff. Both are checked - * because the label is suppressed by its own `useNotInLabel` option. + * Both signals are read off the DIFF BLOCK, never the prose. The first line is + * `Expect ${subject} ${not}to …`, and a subject is user-controlled: a selector + * or text containing "not to" would otherwise reverse a positive assertion's + * outcome. */ -export function assertionWasNegated(message: string | undefined): boolean { +export function assertionWasNegated( + message: string | undefined, + /** Whether the user passed an expected value, e.g. `toHaveText('x')`. */ + hasUserExpectedValue: boolean +): boolean { if (!message) { return false } - return ( - message.includes('Expected [not]') || /\bExpect\b.*\bnot to\b/.test(message) - ) + const plain = stripAnsi(message) + // Matchers that take a value label the diff itself when negated. + if (/^Expected \[not\]/m.test(plain)) { + return true + } + // The `.be` family renders no such label — `enhanceErrorBe` passes + // `useNotInLabel: false` — and encodes the negation in the expected VALUE + // instead (`not displayed`). That value is generated from the matcher's own + // expectation, so it is only trustworthy when the user supplied none: + // a positive `toHaveText('not foo')` prints the same shape. + return !hasUserExpectedValue && /^Expected:\s*"?not\s/m.test(plain) } /** The message is a thunk that formats a diff; a matcher whose own formatting @@ -203,7 +216,11 @@ export function expectAssertionToCommandLog( const { matcherName, expectedValue, result } = params const rawPass = result.pass ?? result.result ?? false const negated = - !outcomeIsDecided && assertionWasNegated(readMessage(result.message)) + !outcomeIsDecided && + assertionWasNegated( + readMessage(result.message), + expectedValue !== undefined + ) const rawArgs = expectedValue === undefined ? [] diff --git a/packages/service/tests/assert-capture.test.ts b/packages/service/tests/assert-capture.test.ts index 16c5ad5b..e6dd2b54 100644 --- a/packages/service/tests/assert-capture.test.ts +++ b/packages/service/tests/assert-capture.test.ts @@ -222,57 +222,87 @@ describe('expectAssertionToCommandLog', () => { // Nothing in the hook params carries `isNot` — it lives on the matcher's own // `this` — so a passing `.not.*` arrived as `pass: false`, was recorded as a // failed row inside a green test, and landed in the Errors tab. + +// expect-webdriverio hands `afterAssertion` the RAW matcher result: `pass` +// answers the POSITIVE assertion and the framework inverts it for `.not`. +// Nothing in the hook params carries `isNot`, so every passing `.not.*` was +// recorded as a failed row inside a green test and landed in the Errors tab. describe('a negated matcher (.not)', () => { - // Verbatim shape of enhanceError's output: `Expect ${subject} ${not}to …` - // plus the `Expected [not]` diff label. - const negated = `Expect $(\`#gone\`) not to be displayed + // The `.be` family (toBeDisplayed etc.) renders no `[not]` label — it puts + // the negation in the generated expected VALUE. + const negatedBe = `Expect $(\`#gone\`) not to be displayed + +Expected: "not displayed" +Received: "displayed"` -Expected [not]: true -Received : false` + const positiveBe = `Expect $(\`#here\`) to be displayed - const positive = `Expect $(\`#here\`) to be displayed +Expected: "displayed" +Received: "not displayed"` -Expected: true -Received: false` + // A value matcher labels the diff instead. + const negatedValue = `Expect $(\`#a\`) not to have text - const entryFor = (pass: boolean, message: string | (() => string)) => +Expected [not]: "hi" +Received : "hi"` + + const entryFor = (pass: boolean, message: string, expectedValue?: unknown) => expectAssertionToCommandLog( { matcherName: 'toBeDisplayed', - result: { - pass, - message: typeof message === 'string' ? () => message : message - } + expectedValue, + result: { pass, message: () => message } }, 'test-1' ) it('records a passing .not assertion as passed', () => { - const entry = entryFor(false, negated) + const entry = entryFor(false, negatedBe) expect(entry.result).toBe('passed') expect(entry.error).toBeUndefined() }) it('records a failing .not assertion as failed', () => { - const entry = entryFor(true, negated) - expect(entry.error).toBeDefined() - expect(entry.result).toBeUndefined() + expect(entryFor(true, negatedBe).error).toBeDefined() }) it('leaves a positive matcher alone in both directions', () => { - expect(entryFor(true, positive).result).toBe('passed') - expect(entryFor(false, positive).error).toBeDefined() + expect(entryFor(true, positiveBe).result).toBe('passed') + expect(entryFor(false, positiveBe).error).toBeDefined() + }) + + it('reads the [not] label a value matcher writes', () => { + expect(entryFor(false, negatedValue, 'hi').result).toBe('passed') }) }) describe('assertionWasNegated', () => { - it('detects the diff label and the phrase independently', () => { - expect(assertionWasNegated('Expected [not]: true')).toBe(true) - expect(assertionWasNegated('Expect $(`#a`) not to have text')).toBe(true) + it('reads the diff label, not the prose', () => { + expect(assertionWasNegated('Expected [not]: true', true)).toBe(true) + expect(assertionWasNegated('Expected: "not displayed"', false)).toBe(true) + }) + + // The subject is user-controlled and is interpolated into the first line, so + // scanning the prose let a selector reverse a positive assertion's outcome. + it('is not fooled by a subject containing the negation phrase', () => { + const message = `Expect $(\`.not to be shown\`) to be displayed + +Expected: "displayed" +Received: "not displayed"` + expect(assertionWasNegated(message, false)).toBe(false) + }) + + // `toHaveText('not foo')` prints the same shape as a negated `.be` matcher, + // so the generated-value signal is only trusted when the user supplied none. + it('is not fooled by a user expected value that begins with "not"', () => { + const message = `Expect $(\`#a\`) to have text + +Expected: "not foo" +Received: "foo"` + expect(assertionWasNegated(message, true)).toBe(false) }) - it('does not fire on a positive message or an empty one', () => { - expect(assertionWasNegated('Expect $(`#a`) to be displayed')).toBe(false) - expect(assertionWasNegated(undefined)).toBe(false) + it('answers false for an empty message', () => { + expect(assertionWasNegated(undefined, false)).toBe(false) }) }) From 70f73a8d6f1cded1e55a566da8fb5bc0505e58ed Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Fri, 18 Sep 2026 17:03:03 +0530 Subject: [PATCH 8/8] chore: add changesets for the live-dashboard and .not fixes --- .changeset/live-native-run-is-visible.md | 14 ++++++++++++++ .../passing-not-assertions-read-as-passed.md | 13 +++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 .changeset/live-native-run-is-visible.md create mode 100644 .changeset/passing-not-assertions-read-as-passed.md diff --git a/.changeset/live-native-run-is-visible.md b/.changeset/live-native-run-is-visible.md new file mode 100644 index 00000000..5ba4b0a0 --- /dev/null +++ b/.changeset/live-native-run-is-visible.md @@ -0,0 +1,14 @@ +--- +"@wdio/devtools-service": patch +"@wdio/devtools-app": patch +--- + +Make a live native mobile run visible on the dashboard. Three separate gaps left one looking empty, and each hid the next. + +**Early messages were discarded in silence.** A session's metadata and its first suites are published while the driver is still being created — against Appium that is ~11 s before the worker socket opens — and `sendUpstream` dropped anything sent before the socket was open. `metadata.type` gates the test-suite pane and `metadata.device` gates the mobile layout, so a live run showed neither the test tree nor the device frame and simply looked like nothing had been captured. Messages published while the socket is CONNECTING are now buffered and flushed in publication order on open; a socket that dies before ever opening reports and releases what it held rather than retaining a run's worth of payloads. The buffer is bounded. + +Drop reporting is re-entrancy guarded, because the fix uncovered a second trap: `patchConsole` forwards console output upstream, so an adapter's drop handler that logs re-enters `sendUpstream`, drops again and recurses until the stack blows — surfacing as `Maximum call stack size exceeded` raised inside the user's own spec, pointing nowhere near the capturer. + +**A native command carried no image.** The per-command screenshot was skipped for every Appium session. A native session has no DOM to replay and no per-action snapshot outside trace mode, so the player had nothing to show for any command and the device pane fell back to desktop browser chrome. Native sessions now take one in **live mode only** — trace mode already screenshots the same command through `captureActionResult`, and two Appium round trips at ~1.2 s each is the cost #351 exists to remove. A mobile *browser* session is unchanged: it replays from its mutation stream. + +**The capture had nowhere sensible to sit.** The trace player puts the dock beside the capture, which works when the whole window is the trace. A live dashboard has already spent its left edge on the suite tree, so a third column squeezed the dock into an unreadable strip and the tab row overflowed under the capture. Live mode now stacks the action list and the dock in one column beside a full-height capture, with both drag handles working and the collapse reversible. diff --git a/.changeset/passing-not-assertions-read-as-passed.md b/.changeset/passing-not-assertions-read-as-passed.md new file mode 100644 index 00000000..4431eab1 --- /dev/null +++ b/.changeset/passing-not-assertions-read-as-passed.md @@ -0,0 +1,13 @@ +--- +"@wdio/devtools-service": patch +--- + +Record a passing `.not.*` assertion as passed. Every negated matcher that succeeded was rendered as a failed action row and collected into the Errors tab, inside a test the runner itself reported green — so a clean run showed a red row and an error it had not produced. + +expect-webdriverio hands `afterAssertion` the **raw** matcher result: jest's convention is that `pass` answers the *positive* assertion and the framework inverts it for `.not`, so a passing `.not.toBeDisplayed()` arrives as `pass: false`. Nothing in the hook's parameters carries `isNot` — it lives on the matcher's own `this` — which leaves the formatted message as the only carrier that reaches an adapter. + +Both signals are read off the generated **diff block**, never the prose. The first line is `Expect ${subject} ${not}to …` and a subject is user-controlled, so scanning it let a selector or an expected value containing "not to" reverse a positive assertion's outcome. A matcher that takes a value labels the diff `Expected [not]` when negated; the `.be` family renders no such label (`enhanceErrorBe` passes `useNotInLabel: false`) and encodes the negation in the generated expected value instead, which is trusted only when the user supplied none — `toHaveText('not foo')` prints the same shape. + +A caller that already knows the outcome, such as the synthesized row for a matcher that hard-threw, skips the inversion entirely rather than having a decided failure re-read from its message. + +Not mobile-specific: this affected every `.not.*` matcher on every run.