diff --git a/.changeset/selenium-configure-options.md b/.changeset/selenium-configure-options.md new file mode 100644 index 00000000..fa7b6d07 --- /dev/null +++ b/.changeset/selenium-configure-options.md @@ -0,0 +1,5 @@ +--- +'@wdio/selenium-devtools': patch +--- + +`DevTools.configure()` now applies `port`, `hostname`, `captureScreenshots`, `filmstrip` and `emitArtifactsManifest`. They were documented as configure options but silently ignored, so the defaults always applied. `port`/`hostname` passed after the backend start has begun are ignored with a warning, and an invalid port, a `null` value or a value of the wrong type (for example the string `'false'` for a boolean option) is refused with a warning rather than applied. A rerun keeps its parent's backend whatever they say. diff --git a/packages/selenium-devtools/README.md b/packages/selenium-devtools/README.md index 30829026..801b5239 100644 --- a/packages/selenium-devtools/README.md +++ b/packages/selenium-devtools/README.md @@ -259,6 +259,8 @@ DevTools.configure({ Here's what every option does, in plain language. **All are optional** — the plugin runs fine with `DevTools.configure({})` or no configure call at all. +> `port` and `hostname` only take effect before the backend starts, which happens when the first driver is built, so call `DevTools.configure(...)` before that; a later call ignores them with a warning. A rerun keeps its parent's backend whatever they say. + > **For CI**, set both `headless: true` (hide the test browser) and `openUi: false` (don't try to open the dashboard window). The backend stays running on the configured port so you can still open the UI later. #### `screencast` — record a video of the browser diff --git a/packages/selenium-devtools/src/helpers/configure-options.ts b/packages/selenium-devtools/src/helpers/configure-options.ts new file mode 100644 index 00000000..6026472f --- /dev/null +++ b/packages/selenium-devtools/src/helpers/configure-options.ts @@ -0,0 +1,118 @@ +import type { DevToolsOptions, ServerLock } from '../types.js' + +type OptionKind = 'boolean' | 'choice' | 'port' | 'host' + +// A Record over every option, so a key added to DevToolsOptions fails to compile here instead of being silently ignored. +const OPTION_KIND = { + openUi: 'boolean', + headless: 'boolean', + captureScreenshots: 'boolean', + captureAssertions: 'boolean', + filmstrip: 'boolean', + emitArtifactsManifest: 'boolean', + mode: 'choice', + traceFormat: 'choice', + traceGranularity: 'choice', + tracePolicy: 'choice', + screenshot: 'choice', + video: 'choice', + port: 'port', + hostname: 'host' +} as const satisfies Record< + Exclude, + OptionKind +> + +const EXPECTED: Record = { + boolean: 'a boolean', + choice: 'a non-empty string', + port: 'an integer between 1 and 65535', + host: 'a non-empty string' +} + +type ConfigurableKey = keyof typeof OPTION_KIND + +export type ConfigurableOptions = Pick + +export function serverLock(state: { + isReuse: boolean + backendStarting: boolean +}): ServerLock | undefined { + if (state.isReuse) { + return 'reuse' + } + return state.backendStarting ? 'started' : undefined +} + +const CONFIGURABLE_KEYS = Object.keys(OPTION_KIND) as ConfigurableKey[] + +function isServerKey(key: ConfigurableKey): boolean { + const kind: OptionKind = OPTION_KIND[key] + return kind === 'port' || kind === 'host' +} + +// JS callers bypass the types (e.g. `headless: process.env.HEADLESS`), and the string 'false' is truthy. +function hasExpectedShape(kind: OptionKind, value: unknown): boolean { + switch (kind) { + case 'boolean': + return typeof value === 'boolean' + case 'port': + return ( + typeof value === 'number' && + Number.isInteger(value) && + value > 0 && + value < 65536 + ) + case 'choice': + case 'host': + return typeof value === 'string' && value !== '' + } +} + +function describeValue(value: unknown): string { + return typeof value === 'string' ? `'${value}'` : String(value) +} + +function copyOption( + target: ConfigurableOptions, + opts: DevToolsOptions, + key: K +): void { + target[key] = opts[key] +} + +/** Returns one warning per refused option; a reused backend's port/hostname are skipped silently, as that is the rerun working. */ +export function applyConfigureOptions( + target: ConfigurableOptions, + opts: DevToolsOptions, + lock?: ServerLock +): string[] { + const warnings: string[] = [] + const accepted: ConfigurableKey[] = [] + for (const key of CONFIGURABLE_KEYS) { + const value: unknown = opts[key] + if (value === undefined || value === null) { + continue + } + const kind: OptionKind = OPTION_KIND[key] + if (hasExpectedShape(kind, value)) { + accepted.push(key) + } else { + warnings.push( + `configure: ignoring ${key}: expected ${EXPECTED[kind]}, got ${describeValue(value)}` + ) + } + } + const server = accepted.filter(isServerKey) + if (server.length > 0 && lock === 'started') { + warnings.push( + `configure: ignoring ${server.join('/')}: the backend start has already begun` + ) + } + for (const key of accepted) { + if (!(lock && isServerKey(key))) { + copyOption(target, opts, key) + } + } + return warnings +} diff --git a/packages/selenium-devtools/src/index.ts b/packages/selenium-devtools/src/index.ts index 69b49c0b..f2a071dd 100644 --- a/packages/selenium-devtools/src/index.ts +++ b/packages/selenium-devtools/src/index.ts @@ -43,6 +43,10 @@ import { detectSeleniumVersion } from './helpers/runtime.js' import { findFreePort } from './helpers/utils.js' +import { + applyConfigureOptions, + serverLock +} from './helpers/configure-options.js' import { RetryTracker, errorMessage, @@ -60,15 +64,9 @@ import { import { type ActionSnapshot, type CapturedCommand, - type DevToolsMode, type DevToolsOptions, type ScreencastFrame, type ScreencastOptions, - type TraceFormat, - type TraceGranularity, - type TraceRetentionPolicy, - type TraceScreenshotPolicy, - type TraceVideoPolicy, type SeleniumDriverLike, type TestStats } from './types.js' @@ -299,54 +297,17 @@ class SeleniumDevToolsPlugin { return this.#uiReadyPromise } - // Declarative option-application mapper — one guarded assignment per option; - // splitting it purely to satisfy the line rule hurts readability. - // eslint-disable-next-line max-lines-per-function - configure( - opts: { - rerunCommand?: string - screencast?: ScreencastOptions - headless?: boolean - openUi?: boolean - captureAssertions?: boolean - mode?: DevToolsMode - traceFormat?: TraceFormat - traceGranularity?: TraceGranularity - tracePolicy?: TraceRetentionPolicy - screenshot?: TraceScreenshotPolicy - video?: TraceVideoPolicy - } = {} - ) { + configure(opts: DevToolsOptions = {}) { if ('rerunCommand' in opts) { this.#rerunManager.configure(opts.rerunCommand) this.#options.rerunCommand = opts.rerunCommand } - if (typeof opts.headless === 'boolean') { - this.#options.headless = opts.headless - } - if (typeof opts.captureAssertions === 'boolean') { - this.#options.captureAssertions = opts.captureAssertions - } - if (typeof opts.openUi === 'boolean') { - this.#options.openUi = opts.openUi - } - if (opts.mode) { - this.#options.mode = opts.mode - } - if (opts.traceFormat) { - this.#options.traceFormat = opts.traceFormat - } - if (opts.traceGranularity) { - this.#options.traceGranularity = opts.traceGranularity - } - if (opts.tracePolicy) { - this.#options.tracePolicy = opts.tracePolicy - } - if (opts.screenshot) { - this.#options.screenshot = opts.screenshot - } - if (opts.video) { - this.#options.video = opts.video + const lock = serverLock({ + isReuse: this.#isReuse, + backendStarting: this.#backendStartPromise !== undefined + }) + for (const warning of applyConfigureOptions(this.#options, opts, lock)) { + log.warn(warning) } if (opts.screencast) { if (this.#options.mode === 'trace' && opts.screencast.enabled) { @@ -734,19 +695,7 @@ if (!registerHooks()) { registerProcessHooks(plugin) export const DevTools = { - configure: (opts: { - rerunCommand?: string - screencast?: ScreencastOptions - headless?: boolean - openUi?: boolean - captureAssertions?: boolean - mode?: DevToolsMode - traceFormat?: TraceFormat - traceGranularity?: TraceGranularity - tracePolicy?: TraceRetentionPolicy - screenshot?: TraceScreenshotPolicy - video?: TraceVideoPolicy - }) => plugin.configure(opts), + configure: (opts: DevToolsOptions) => plugin.configure(opts), startTest: (name: string, meta?: { file?: string }) => plugin.startTest(name, meta), endTest: (state: TestStats['state'] = 'passed') => plugin.endTest(state) diff --git a/packages/selenium-devtools/src/types.ts b/packages/selenium-devtools/src/types.ts index 8804a772..09bb33bf 100644 --- a/packages/selenium-devtools/src/types.ts +++ b/packages/selenium-devtools/src/types.ts @@ -22,6 +22,9 @@ export { type TraceVideoPolicy } from '@wdio/devtools-shared' +/** Why `configure()` can no longer change port/hostname: a rerun reusing its parent's backend, or a backend start already under way. */ +export type ServerLock = 'reuse' | 'started' + export interface DevToolsOptions extends BaseDevToolsOptions { /** Open a Chrome window pointing at the UI. Default true. */ openUi?: boolean diff --git a/packages/selenium-devtools/tests/configure-options.test.ts b/packages/selenium-devtools/tests/configure-options.test.ts new file mode 100644 index 00000000..136c5288 --- /dev/null +++ b/packages/selenium-devtools/tests/configure-options.test.ts @@ -0,0 +1,158 @@ +import { describe, it, expect } from 'vitest' +import { + applyConfigureOptions, + serverLock, + type ConfigurableOptions +} from '../src/helpers/configure-options.js' +import type { DevToolsOptions } from '../src/types.js' + +function defaults(): ConfigurableOptions { + return { + port: 3000, + hostname: 'localhost', + openUi: true, + headless: false, + captureScreenshots: true, + filmstrip: true, + mode: 'live' + } +} + +// A JS caller can hand configure() anything, whatever the type says. +function untyped(opts: Record): DevToolsOptions { + return opts as DevToolsOptions +} + +describe('applyConfigureOptions', () => { + it('applies the options configure() used to drop', () => { + const target = defaults() + const warnings = applyConfigureOptions(target, { + port: 4123, + hostname: '127.0.0.1', + captureScreenshots: false, + filmstrip: false, + emitArtifactsManifest: true + }) + expect(warnings).toEqual([]) + expect(target).toMatchObject({ + port: 4123, + hostname: '127.0.0.1', + captureScreenshots: false, + filmstrip: false, + emitArtifactsManifest: true + }) + }) + + it('applies false booleans and leaves unspecified options untouched', () => { + const target = defaults() + applyConfigureOptions(target, { openUi: false, mode: 'trace' }) + expect(target).toEqual({ ...defaults(), openUi: false, mode: 'trace' }) + }) + + it('refuses a string where a boolean belongs, so "false" cannot turn headless on', () => { + const target = defaults() + const warnings = applyConfigureOptions( + target, + untyped({ headless: 'false', openUi: 'true', captureAssertions: 'yes' }) + ) + expect(target.headless).toBe(false) + expect(target.openUi).toBe(true) + expect('captureAssertions' in target).toBe(false) + expect(warnings).toEqual([ + "configure: ignoring openUi: expected a boolean, got 'true'", + "configure: ignoring headless: expected a boolean, got 'false'", + "configure: ignoring captureAssertions: expected a boolean, got 'yes'" + ]) + }) + + it('refuses an empty or non-string choice', () => { + const target = defaults() + const warnings = applyConfigureOptions( + target, + untyped({ mode: '', traceFormat: 1 }) + ) + expect(target.mode).toBe('live') + expect('traceFormat' in target).toBe(false) + expect(warnings).toEqual([ + "configure: ignoring mode: expected a non-empty string, got ''", + 'configure: ignoring traceFormat: expected a non-empty string, got 1' + ]) + }) + + it('treats an explicit undefined or null as "not specified"', () => { + const target = defaults() + const warnings = applyConfigureOptions( + target, + untyped({ + emitArtifactsManifest: undefined, + traceFormat: null, + port: null + }) + ) + expect(warnings).toEqual([]) + expect(target).toEqual(defaults()) + expect('emitArtifactsManifest' in target).toBe(false) + }) + + it('refuses an invalid port but still applies a valid hostname', () => { + const target = defaults() + const warnings = applyConfigureOptions(target, { + port: Number('not-a-port'), + hostname: '0.0.0.0' + }) + expect(warnings).toEqual([ + 'configure: ignoring port: expected an integer between 1 and 65535, got NaN' + ]) + expect(target.port).toBe(3000) + expect(target.hostname).toBe('0.0.0.0') + }) + + it('refuses port/hostname once the backend start has begun, and says why', () => { + const target = defaults() + const warnings = applyConfigureOptions( + target, + { port: 4123, hostname: 'example.test', filmstrip: false }, + 'started' + ) + expect(target.port).toBe(3000) + expect(target.hostname).toBe('localhost') + expect(target.filmstrip).toBe(false) + expect(warnings).toEqual([ + 'configure: ignoring port/hostname: the backend start has already begun' + ]) + }) + + it("keeps a reused parent backend's port/hostname without warning", () => { + const target = defaults() + const warnings = applyConfigureOptions( + target, + { port: 4123, openUi: false }, + 'reuse' + ) + expect(warnings).toEqual([]) + expect(target.port).toBe(3000) + expect(target.openUi).toBe(false) + }) + + it('does not warn under a lock when port/hostname were not asked for', () => { + expect( + applyConfigureOptions(defaults(), { openUi: false }, 'started') + ).toEqual([]) + }) +}) + +describe('serverLock', () => { + it('prefers reuse over a started backend, since a rerun child never binds its own', () => { + expect(serverLock({ isReuse: true, backendStarting: true })).toBe('reuse') + expect(serverLock({ isReuse: true, backendStarting: false })).toBe('reuse') + }) + + it('locks once the backend start has begun, and not before', () => { + expect(serverLock({ isReuse: false, backendStarting: true })).toBe( + 'started' + ) + expect( + serverLock({ isReuse: false, backendStarting: false }) + ).toBeUndefined() + }) +})