From 7d6f822fee3f255195c1f1ab745336ffc09871ef Mon Sep 17 00:00:00 2001 From: Aakash Hotchandani Date: Mon, 13 Jul 2026 11:23:34 +0530 Subject: [PATCH] fix(browserstack-service): port 2026-07-10 monorepo accessibility/CLI parity fixes Brings the standalone to parity with the six browserstack-service PRs merged to webdriverio/webdriverio main on 2026-07-10: - #15380 skip the a11y scan for BiDi window/context commands - #15383 route WDIO CLI-flow App Automate sessions to app-accessibility - #15382 finalize orphaned test runs on an interrupted exit (new testOps/openRunsJournal) - #15381 coerce stringified boolean accessibility options (new util.coerceStringBooleans) - #15376 report mocha hooks in the CLI/testHub flow #15379 (a11y Browser type augmentations) was already ported via the parity PR, so its hunks applied as no-ops. Applied as the monorepo net delta; launcher.ts imports and the service.test.ts BrowserstackCLI import were hand-reconciled to the standalone layout. Build + eslint clean; all six PRs' tests pass (the pre-existing offline fetch-mock failures are unchanged from the base branch). Co-Authored-By: Claude Opus 4.8 (1M context) --- .changeset/port-friday-parity-fixes.md | 5 + .../src/accessibility-handler.ts | 48 +++++++ packages/browserstack-service/src/cleanup.ts | 4 + .../cli/frameworks/wdioMochaTestFramework.ts | 62 ++++++++- .../browserstack-service/src/cli/index.ts | 12 ++ .../src/cli/modules/accessibilityModule.ts | 32 ++++- .../src/cli/modules/testHubModule.ts | 5 +- packages/browserstack-service/src/launcher.ts | 14 +- packages/browserstack-service/src/service.ts | 38 +++++- .../src/testOps/listener.ts | 3 + .../src/testOps/openRunsJournal.ts | 97 ++++++++++++++ packages/browserstack-service/src/util.ts | 25 ++++ .../tests/accessibility-handler.test.ts | 37 ++++++ .../cli/modules/accessibilityModule.test.ts | 77 +++++++++++ .../tests/launcher.test.ts | 6 +- .../tests/service.test.ts | 48 +++++++ .../tests/testOps/openRunsJournal.test.ts | 124 ++++++++++++++++++ .../browserstack-service/tests/util.test.ts | 14 ++ 18 files changed, 632 insertions(+), 19 deletions(-) create mode 100644 .changeset/port-friday-parity-fixes.md create mode 100644 packages/browserstack-service/src/testOps/openRunsJournal.ts create mode 100644 packages/browserstack-service/tests/testOps/openRunsJournal.test.ts diff --git a/.changeset/port-friday-parity-fixes.md b/.changeset/port-friday-parity-fixes.md new file mode 100644 index 0000000..c5cce60 --- /dev/null +++ b/.changeset/port-friday-parity-fixes.md @@ -0,0 +1,5 @@ +--- +"@wdio/browserstack-service": patch +--- + +Port the 2026-07-10 monorepo accessibility/CLI fixes to keep the standalone at parity (webdriverio/webdriverio#15380, #15383, #15382, #15381, #15376): skip the accessibility scan for BiDi `window`/`context` commands, route WDIO CLI-flow App Automate sessions to app-accessibility, finalize orphaned test runs on an interrupted exit, coerce stringified boolean accessibility options, and report mocha hooks in the CLI/testHub flow. No user-facing API change. diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index ff1ef90..f46b53e 100644 --- a/packages/browserstack-service/src/accessibility-handler.ts +++ b/packages/browserstack-service/src/accessibility-handler.ts @@ -423,8 +423,10 @@ class _AccessibilityHandler { */ private async commandWrapper (command: CommandInfo, prevImpl: Function, origFunction: Function, ...args: unknown[]) { + const skipScanForBidiWindowCommand = AccessibilityHandler.shouldSkipScanForBidiWindowCommand(this._browser, command) if ( this._sessionId && AccessibilityHandler._a11yScanSessionMap[this._sessionId] && + !skipScanForBidiWindowCommand && ( !command.name.includes('execute') || !AccessibilityHandler.shouldPatchExecuteScript(args.length ? args[0] as string : null) @@ -432,6 +434,8 @@ class _AccessibilityHandler { ) { BStackLogger.debug(`Performing scan for ${command.class} ${command.name}`) await performA11yScan(this.isAppAutomate, this._browser, true, true, command.name) + } else if (skipScanForBidiWindowCommand) { + BStackLogger.debug(`SDK-5047: skipping accessibility scan for BiDi window/context command '${command.name}' to avoid racing the WebdriverIO ContextManager during session-start window churn`) } const impl = prevImpl || origFunction return impl(...args) @@ -503,6 +507,50 @@ class _AccessibilityHandler { ) } + /** + * SDK-5047: Window/context-management commands whose surrounding injected + * accessibility scan (a browser.execute) races the WebdriverIO v9 core + * ContextManager while it (re)binds the browsing context during + * session-start window churn on BiDi sessions (e.g. Chrome), surfacing + * "no such window: target window already closed / web view not found". + */ + private static readonly BIDI_WINDOW_CONTEXT_COMMANDS = new Set([ + 'getWindowHandle', 'getWindowHandles', 'switchToWindow', 'switchWindow', + 'newWindow', 'closeWindow', 'switchFrame', 'switchToFrame', 'switchToParentFrame' + ]) + + /** + * BiDi detection that also covers MultiRemote: the aggregate multiremote + * object does not expose `isBidi` itself — the flag lives on each child + * instance — so the session counts as BiDi when the top-level flag is set + * OR any multiremote child instance reports `isBidi`. + */ + private static isBidiSession(browser: WebdriverIO.Browser | WebdriverIO.MultiRemoteBrowser | undefined): boolean { + const b = browser as (WebdriverIO.Browser & { isBidi?: boolean, instances?: string[] }) | undefined + if (b?.isBidi === true) { + return true + } + if (Array.isArray(b?.instances)) { + const children = b as unknown as Record + return b.instances.some((name) => children[name]?.isBidi === true) + } + return false + } + + /** + * Returns true when the injected pre-command accessibility scan must be + * skipped: only on BiDi sessions, and only for window/context-management + * commands. Non-BiDi sessions and all other commands are unaffected and + * keep scanning exactly as before. + */ + private static shouldSkipScanForBidiWindowCommand(browser: WebdriverIO.Browser | WebdriverIO.MultiRemoteBrowser | undefined, command: CommandInfo): boolean { + return Boolean( + command?.name && + AccessibilityHandler.isBidiSession(browser) && + AccessibilityHandler.BIDI_WINDOW_CONTEXT_COMMANDS.has(command.name) + ) + } + private async _setAnnotation(message: string) { if (this._accessibility && isBrowserstackSession(this._browser)) { await (this._browser as WebdriverIO.Browser).executeScript(`browserstack_executor: ${JSON.stringify({ diff --git a/packages/browserstack-service/src/cleanup.ts b/packages/browserstack-service/src/cleanup.ts index 3f48bfc..034c7b5 100644 --- a/packages/browserstack-service/src/cleanup.ts +++ b/packages/browserstack-service/src/cleanup.ts @@ -1,4 +1,5 @@ import { getErrorString, stopBuildUpstream } from './util.js' +import { finalizeOrphanedRuns } from './testOps/openRunsJournal.js' import { BStackLogger } from './bstackLogger.js' import fs from 'node:fs' import util from 'node:util' @@ -45,6 +46,9 @@ export default class BStackCleanup { } BStackLogger.debug('Executing Test Reporting and Analytics cleanup') try { + // SDK-4671: finalize any test runs orphaned by the interrupted process + // before the build itself is stopped. + await finalizeOrphanedRuns() const result = await stopBuildUpstream() if ((process.env[BROWSERSTACK_OBSERVABILITY]) && process.env[BROWSERSTACK_TESTHUB_UUID]) { BStackLogger.info(`\nVisit https://automation.browserstack.com/builds/${process.env[BROWSERSTACK_TESTHUB_UUID]} to view build report, insights, and many more debugging information all at one place!\n`) diff --git a/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts b/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts index 70de2a4..60c6ddb 100644 --- a/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts +++ b/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts @@ -44,7 +44,11 @@ export default class WdioMochaTestFramework extends TestFramework { } try { - if (CLIUtils.matchHookRegex(testFrameworkState.toString()) && hookState === HookState.PRE) { + // matchHookRegex expects the short state name (e.g. AFTER_EACH); `toString()` yields + // the fully-qualified `TestFrameworkState.AFTER_EACH`, which never matches `^(BEFORE_|AFTER_)`. + // Without this, KEY_HOOK_ID is never set, hook_run.uuid ends up empty, and the backend + // (BigQuery) drops the hook events despite them being sent. + if (CLIUtils.matchHookRegex(testFrameworkState.toString().split('.')[1]) && hookState === HookState.PRE) { instance.updateMultipleEntries({ [TestFrameworkConstants.KEY_HOOK_ID]: uuidv4(), }) @@ -76,7 +80,14 @@ export default class WdioMochaTestFramework extends TestFramework { this.loadTestResult(instance, args) } - await this.trackHookEvents(instance, testFrameworkState, hookState, args) + // Only real hook states accumulate into test_hooks_started/finished. trackEvent runs + // for every state (TEST/INIT_TEST/LOG/...); without this gate those push pseudo-hook + // entries which — now that the Map serializer actually emits them — get flat-mapped into + // TestRunFinished.hooks (duplicating the last hook id / adding '') and accumulate in + // test_hooks_started forever (never popped). + if (CLIUtils.matchHookRegex(testFrameworkState.toString().split('.')[1])) { + await this.trackHookEvents(instance, testFrameworkState, hookState, args) + } logger.debug(`trackEvent: tracked instance data=${JSON.stringify(Object.fromEntries(instance.getAllData()))}`) } catch (error) { logger.error(`trackEvent: Error in tracking events: ${error} hookState=${hookState} testFrameworkState=${testFrameworkState}`) @@ -93,13 +104,49 @@ export default class WdioMochaTestFramework extends TestFramework { * @returns {TestFrameworkInstance} */ resolveInstance(testFrameworkState: State, hookState: State, args: Record = {}): TestFrameworkInstance|null { - let instance = null logger.info(`resolveInstance: resolving instance for testFrameworkState=${testFrameworkState} hookState=${hookState}`) - if (testFrameworkState === TestFrameworkState.INIT_TEST || testFrameworkState === TestFrameworkState.NONE) { + const shortState = testFrameworkState.toString().split('.')[1] + const isHook = CLIUtils.matchHookRegex(shortState) + let instance = TestFramework.getTrackedInstance() + + // Whether the current tracked instance has already run its test body. + const hasRunTest = !!(instance && TestFramework.getState(instance, TestFrameworkConstants.KEY_TEST_ID)) + + // New-test boundary (ports Junit5Framework.resolveInstance): the previous method was a + // test / after-hook (POST) and the current is a before-hook / init (PRE) — the next test + // is starting. At entry, getCurrentTestState()/getCurrentHookState() still hold the PREVIOUS + // method's state (updateInstanceState has not run yet). + const prevState = instance ? instance.getCurrentTestState().toString() : '' + const prevWasTerminal = instance ? (instance.getCurrentTestState() === TestFrameworkState.TEST || prevState.includes('AFTER')) : false + const isNewTestBoundary = instance ? (prevWasTerminal && instance.getCurrentHookState() === HookState.POST && hookState === HookState.PRE) : false + + if (testFrameworkState === TestFrameworkState.NONE) { this.trackWdioMochaInstance(testFrameworkState, args) + } else if (testFrameworkState === TestFrameworkState.INIT_TEST) { + // WDIO fires `before each` BEFORE `beforeTest` (INIT_TEST). Reuse the instance a + // preceding before-hook already opened for this same upcoming test; only start a new + // one if the current instance has already run a test (or none exists). + if (!instance || hasRunTest) { + this.trackWdioMochaInstance(testFrameworkState, args) + } + } else if (isHook && hookState === HookState.PRE) { + // Suite-level (`before all`) and the first `before each` fire before any INIT_TEST, so + // no instance exists yet — create one. Also, a BEFORE hook right after a completed test + // (new-test boundary) starts a new test. The boundary restriction must be limited to + // BEFORE hooks: `after each`/`after all` also fire at a POST->PRE boundary (afterTest + // emits TEST POST just before `after each`), but they belong to the just-finished test. + // Creating a fresh (uuid-less) instance for them would drop test_run_id and orphan the + // hook from TestRunFinished.hooks — so let after-hooks reuse the finished test's instance. + if (!instance || (isNewTestBoundary && shortState.startsWith('BEFORE_'))) { + this.trackWdioMochaInstance(testFrameworkState, args) + } } instance = TestFramework.getTrackedInstance() + if (!instance) { + logger.error(`resolveInstance: unable to resolve/create instance for testFrameworkState=${testFrameworkState} hookState=${hookState}`) + return null + } this.updateInstanceState(instance, testFrameworkState, hookState) return instance @@ -205,7 +252,7 @@ export default class WdioMochaTestFramework extends TestFramework { const logRecord: Record = {} const { level, message, timestamp } = logEntry - if (CLIUtils.matchHookRegex(instance.getCurrentTestState().toString())) { + if (CLIUtils.matchHookRegex(instance.getCurrentTestState().toString().split('.')[1])) { logRecord[TestFrameworkConstants.KEY_HOOK_ID] = TestFramework.getState(instance, TestFrameworkConstants.KEY_HOOK_ID) } logRecord.kind = TestFrameworkConstants.KIND_LOG @@ -321,7 +368,10 @@ export default class WdioMochaTestFramework extends TestFramework { ) { const testResult = args.result as Frameworks.TestResult const test = args.test as Frameworks.Test - const key = testFrameworkState.toString() + // Key hooks by the short state name (e.g. AFTER_EACH), matching how the binary looks them + // up via `event.test_hooks_started[request.testFrameworkState]`. `toString()` yields the + // fully-qualified `TestFrameworkState.AFTER_EACH`, which would never match. + const key = testFrameworkState.toString().split('.')[1] const hooksStarted = TestFramework.getState(instance, TestFrameworkConstants.KEY_HOOKS_STARTED) as Map if (!hooksStarted.has(key)) { diff --git a/packages/browserstack-service/src/cli/index.ts b/packages/browserstack-service/src/cli/index.ts index d7b005c..618ca48 100644 --- a/packages/browserstack-service/src/cli/index.ts +++ b/packages/browserstack-service/src/cli/index.ts @@ -44,6 +44,7 @@ export class BrowserstackCLI { process: ChildProcess | null = null isMainConnected = false isChildConnected = false + modulesLoaded = false binSessionId: string | null = null modules: Record = {} testFramework: WdioMochaTestFramework|null = null @@ -130,6 +131,17 @@ export class BrowserstackCLI { this.binSessionId = startBinResponse.binSessionId this.logger.info(`loadModules: binSessionId=${this.binSessionId}`) + // Idempotency guard: startMain() and startChild() can both run in the same process + // (e.g. local runner, single instance). Each call constructs the modules again, whose + // constructors register observers on the shared eventDispatcher singleton — and + // registerObserver does not dedupe. Loading twice therefore double-registers every + // observer, so each test/hook event is dispatched (and uploaded) twice. Load once. + if (this.modulesLoaded) { + this.logger.info('loadModules: modules already loaded in this process; skipping to avoid duplicate observer registration') + return + } + this.modulesLoaded = true + this.setConfig(startBinResponse) // Surface any build errors the binary populated on testhub.errors diff --git a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts index 7ed1b58..5b7399e 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -78,6 +78,15 @@ export default class AccessibilityModule extends BaseModule { platform_name: browserCaps?.platformName, platform_version: this.getCapability(browserCaps, 'appium:platformVersion', 'platformVersion'), } + + // App Automate sessions must run the app-accessibility flow, not the + // Chrome-only web path. The binary's isAppAccessibility flag is not + // guaranteed to be set on the CLI path, so derive app-ness from caps the + // same way the classic flow does (service._isAppAutomate). + if (this.isAppAutomateSession(inputCaps, browserCaps)) { + this.isAppAccessibility = true + } + if (this.isAppAccessibility) { this.accessibility = validateCapsWithAppA11y(platformA11yMeta) } else { @@ -129,12 +138,11 @@ export default class AccessibilityModule extends BaseModule { return } - if (!('overwriteCommand' in browser && Array.isArray(this.scriptInstance.commandsToWrap))) { - return - } - - // Wrap commands if accessibility scripts are available - if (this.scriptInstance.commandsToWrap && this.scriptInstance.commandsToWrap.length > 0) { + // Web command wrapping (overwriteCommand) only applies to the web a11y + // flow. App Automate a11y scans run via the performScan/test-lifecycle + // path, and appium drivers don't register these commands, so + // overwriteCommand would throw and abort onBeforeExecute. + if (!this.isAppAccessibility && 'overwriteCommand' in browser && Array.isArray(this.scriptInstance.commandsToWrap)) { this.scriptInstance.commandsToWrap .filter((command) => command.name && command.class) .forEach((command) => { @@ -365,6 +373,18 @@ export default class AccessibilityModule extends BaseModule { } + // Mirrors service._isAppAutomate: an App Automate session is identified by the + // presence of an app capability (appium:app / appium:options.app). + private isAppAutomateSession(inputCaps?: WebdriverIO.Capabilities, browserCaps?: WebdriverIO.Capabilities): boolean { + for (const caps of [inputCaps, browserCaps]) { + const c = (caps ?? {}) as Record + if (c['appium:app'] || (c['appium:options'] as { app?: unknown } | undefined)?.app) { + return true + } + } + return false + } + private async performScanCli( browser: WebdriverIO.Browser | WebdriverIO.MultiRemoteBrowser, commandName?: string diff --git a/packages/browserstack-service/src/cli/modules/testHubModule.ts b/packages/browserstack-service/src/cli/modules/testHubModule.ts index 24915e8..d10fb90 100644 --- a/packages/browserstack-service/src/cli/modules/testHubModule.ts +++ b/packages/browserstack-service/src/cli/modules/testHubModule.ts @@ -114,7 +114,10 @@ export default class TestHubModule extends BaseModule { this.logger.debug(`sendTestFrameworkEvent for testState: ${testFrameworkState} hookState: ${testHookState}`) const platformIndex = process.env.WDIO_WORKER_ID ? parseInt(process.env.WDIO_WORKER_ID.split('-')[0]) : 0 const uuid = TestFramework.getState(instance, TestFrameworkConstants.KEY_TEST_UUID) || instance.getRef() - const eventJson = Buffer.from(JSON.stringify(Object.fromEntries(testData))) + // Nested values such as test_hooks_started/test_hooks_finished are JS Maps, which + // JSON.stringify would serialise to `{}` and strip the hook data. Convert any Map to + // a plain object so the binary receives populated hook maps. + const eventJson = Buffer.from(JSON.stringify(Object.fromEntries(testData), (_key, value) => value instanceof Map ? Object.fromEntries(value) : value)) const executionContext = { hash: trackedContext.getId(), threadId: trackedContext.getThreadId().toString(), processId: trackedContext.getProcessId().toString() } const payload: Omit = { platformIndex, diff --git a/packages/browserstack-service/src/launcher.ts b/packages/browserstack-service/src/launcher.ts index bcd1e7b..a1c2380 100644 --- a/packages/browserstack-service/src/launcher.ts +++ b/packages/browserstack-service/src/launcher.ts @@ -45,9 +45,11 @@ import { validateCapsWithNonBstackA11y, mergeChromeOptions, isValidEnabledValue, - isMultiRemoteCaps + isMultiRemoteCaps, + coerceStringBooleans } from './util.js' import CrashReporter from './crash-reporter.js' +import { finalizeOrphanedRuns } from './testOps/openRunsJournal.js' import { BStackLogger } from './bstackLogger.js' import { PercyLogger } from './Percy/PercyLogger.js' import type Percy from './Percy/Percy.js' @@ -438,14 +440,17 @@ export default class BrowserstackLauncherService implements Services.ServiceInst this.browserStackConfig.accessibility = this._accessibilityAutomation if (this._accessibilityAutomation && this._options.accessibilityOptions) { - const filteredOpts = Object.keys(this._options.accessibilityOptions) + // SDK-3737: coerce stringified booleans (e.g. autoScanning: 'false') to real + // booleans so boolean-typed accessibility options are honoured instead of + // being dropped by W3C caps validation. + const filteredOpts = coerceStringBooleans(Object.keys(this._options.accessibilityOptions) .filter(key => !NOT_ALLOWED_KEYS_IN_CAPS.includes(key)) .reduce((opts, key) => { return { ...opts, [key]: this._options.accessibilityOptions?.[key] } - }, {}) + }, {} as Record)) this._updateObjectTypeCaps(capabilities as Capabilities.TestrunnerCapabilities, 'accessibilityOptions', filteredOpts) } else if (isAccessibilityAutomationSession(this._accessibilityAutomation)) { @@ -588,6 +593,9 @@ export default class BrowserstackLauncherService implements Services.ServiceInst const isCLIEnabled = BrowserstackCLI.getInstance().isRunning() BStackLogger.debug('Inside OnComplete hook..') BStackLogger.debug('Sending stop launch event') + // SDK-4671: before stopping the build, synthesize TestRunFinished for any + // test runs whose worker died mid-test, else they stay 'in progress' on TRA. + await finalizeOrphanedRuns() try { await (isCLIEnabled ? BrowserstackCLI.getInstance().stop() : stopBuildUpstream()) PerformanceTester.end(PERFORMANCE_SDK_EVENTS.FRAMEWORK_EVENTS.STOP) diff --git a/packages/browserstack-service/src/service.ts b/packages/browserstack-service/src/service.ts index 9a29000..c5f21d1 100644 --- a/packages/browserstack-service/src/service.ts +++ b/packages/browserstack-service/src/service.ts @@ -8,7 +8,8 @@ import { isBrowserstackSession, patchConsoleLogs, isTrue, - getUniqueIdentifier + getUniqueIdentifier, + getHookType } from './util.js' import type { BrowserstackConfig, BrowserstackOptions, MultiRemoteAction } from './types.js' import type { Pickle, Feature, ITestCaseHookParameter, CucumberHook } from './cucumber-types.js' @@ -388,6 +389,26 @@ export default class BrowserstackService implements Services.ServiceInstance { if (this._config.framework !== 'cucumber') { this._currentTest = test as Frameworks.Test // not update currentTest when this is called for cucumber step } + + // CLI flow: route hook lifecycle to the binary via the TestFramework tracker (gRPC), + // mirroring beforeTest/afterTest. Without this, hook events fall through to the legacy + // Listener -> api/v1/batch path, which is no longer functional in the CLI pipeline, so + // HookRunStarted/HookRunFinished never reach the dashboard. + if (BrowserstackCLI.getInstance().isRunning()) { + // Null-check the tracker rather than asserting: getTestFramework() is null for + // non-mocha frameworks (setupTestFramework only wires it for webdriverio-mocha) and + // during any startup race, so a `!` here could throw a TypeError inside this awaited + // WDIO hook and break the user's suite. Instrumentation must degrade quietly. + const framework = BrowserstackCLI.getInstance().getTestFramework() + if (framework) { + const hookFrameworkState = TestFrameworkState[getHookType((test as Frameworks.Test).title) as keyof typeof TestFrameworkState] + if (hookFrameworkState) { + await framework.trackEvent(hookFrameworkState, HookState.PRE, { test }) + } + } + return + } + await this._insightsHandler?.beforeHook(test, context) } @@ -403,6 +424,21 @@ export default class BrowserstackService implements Services.ServiceInstance { this._failReasons.push(hookError) } } + + // CLI flow: mirror beforeHook — close the hook via the TestFramework tracker (gRPC). + if (BrowserstackCLI.getInstance().isRunning()) { + // Null-check the tracker rather than asserting (see beforeHook) so a missing tracker + // degrades quietly instead of throwing inside this awaited hook. + const framework = BrowserstackCLI.getInstance().getTestFramework() + if (framework) { + const hookFrameworkState = TestFrameworkState[getHookType((test as Frameworks.Test).title) as keyof typeof TestFrameworkState] + if (hookFrameworkState) { + await framework.trackEvent(hookFrameworkState, HookState.POST, { test, result }) + } + } + return + } + await this._insightsHandler?.afterHook(test, result) } diff --git a/packages/browserstack-service/src/testOps/listener.ts b/packages/browserstack-service/src/testOps/listener.ts index 110a020..160a2ae 100644 --- a/packages/browserstack-service/src/testOps/listener.ts +++ b/packages/browserstack-service/src/testOps/listener.ts @@ -12,6 +12,7 @@ import { TEST_ANALYTICS_ID } from '../constants.js' import { sendScreenshots } from './requestUtils.js' +import { recordOpenRun, clearOpenRun } from './openRunsJournal.js' import { BStackLogger } from '../bstackLogger.js' import { shouldProcessEventForTesthub } from '../testHub/utils.js' @@ -105,6 +106,7 @@ class Listener { return } process.env[TEST_ANALYTICS_ID] = testData.uuid + recordOpenRun(testData) this.testStartedStats.triggered() testData.product_map = { @@ -128,6 +130,7 @@ class Listener { accessibility: Listener._testRunAccessibilityVar } + clearOpenRun(testData.uuid) this.testFinishedStats.triggered(testData.result) this.sendBatchEvents(this.getEventForHook('TestRunFinished', testData)) } catch (e) { diff --git a/packages/browserstack-service/src/testOps/openRunsJournal.ts b/packages/browserstack-service/src/testOps/openRunsJournal.ts new file mode 100644 index 0000000..b4119e8 --- /dev/null +++ b/packages/browserstack-service/src/testOps/openRunsJournal.ts @@ -0,0 +1,97 @@ +import path from 'node:path' +import fs from 'node:fs' + +import type { TestData, UploadType } from '../types.js' +import { batchAndPostEvents } from '../util.js' +import { DATA_BATCH_ENDPOINT } from '../constants.js' +import { BStackLogger } from '../bstackLogger.js' + +/** + * Crash-resilient journal of test runs that have sent TestRunStarted but not yet + * TestRunFinished. Each open run is persisted as a small file so that if the worker + * (or the whole wdio process tree) is killed mid-test, the launcher's shutdown path + * or the detached exit cleanup can still send a synthetic TestRunFinished — otherwise + * the test case stays "in progress" on the Test Reporting & Analytics dashboard forever. + */ + +const OPEN_RUNS_DIR = path.join(process.cwd(), 'logs', 'bstack_open_runs') + +export const ORPHAN_FAILURE_REASON = 'Test run did not complete: the process was interrupted or terminated before the test finished (finalized by BrowserStack SDK exit cleanup)' + +function openRunFilePath(uuid: string) { + return path.join(OPEN_RUNS_DIR, `open-run-${uuid}.json`) +} + +export function recordOpenRun(testData: TestData) { + if (!testData || !testData.uuid) { + return + } + try { + fs.mkdirSync(OPEN_RUNS_DIR, { recursive: true }) + fs.writeFileSync(openRunFilePath(testData.uuid), JSON.stringify(testData)) + } catch (e) { + BStackLogger.debug('openRunsJournal: failed to record open run: ' + e) + } +} + +export function clearOpenRun(uuid?: string) { + if (!uuid) { + return + } + try { + fs.rmSync(openRunFilePath(uuid), { force: true }) + } catch (e) { + BStackLogger.debug('openRunsJournal: failed to clear open run: ' + e) + } +} + +export function collectOrphanedRuns(): TestData[] { + const orphans: TestData[] = [] + try { + if (!fs.existsSync(OPEN_RUNS_DIR)) { + return orphans + } + for (const file of fs.readdirSync(OPEN_RUNS_DIR)) { + try { + orphans.push(JSON.parse(fs.readFileSync(path.join(OPEN_RUNS_DIR, file), 'utf8'))) + } catch (e) { + BStackLogger.debug(`openRunsJournal: skipping unreadable journal file ${file}: ${e}`) + } + } + fs.rmSync(OPEN_RUNS_DIR, { recursive: true, force: true }) + } catch (e) { + BStackLogger.debug('openRunsJournal: failed to collect orphaned runs: ' + e) + } + return orphans +} + +export async function finalizeOrphanedRuns(): Promise { + try { + const orphans = collectOrphanedRuns() + if (!orphans.length) { + return 0 + } + const finishedAt = new Date().toISOString() + const events: UploadType[] = orphans.map((testData) => { + const startedAtMs = testData.started_at ? new Date(testData.started_at).getTime() : NaN + return { + event_type: 'TestRunFinished', + test_run: { + ...testData, + finished_at: finishedAt, + result: 'failed', + duration_in_ms: Number.isNaN(startedAtMs) ? undefined : Math.max(0, Date.now() - startedAtMs), + failure: [{ backtrace: [ORPHAN_FAILURE_REASON] }], + failure_reason: ORPHAN_FAILURE_REASON, + failure_type: 'UnhandledError' + } + } + }) + await batchAndPostEvents(DATA_BATCH_ENDPOINT, 'ORPHANED_TEST_RUN_FINALIZATION', events) + BStackLogger.info(`Finalized ${events.length} orphaned test run(s) left behind by an interrupted run`) + return events.length + } catch (e) { + BStackLogger.debug('openRunsJournal: failed to finalize orphaned runs: ' + e) + return 0 + } +} diff --git a/packages/browserstack-service/src/util.ts b/packages/browserstack-service/src/util.ts index c2bd196..136eebd 100644 --- a/packages/browserstack-service/src/util.ts +++ b/packages/browserstack-service/src/util.ts @@ -1880,6 +1880,31 @@ export function getBooleanValueFromString(value: string | undefined): boolean { return ['true'].includes(value.trim().toLowerCase()) } +/** + * SDK-3737: Coerce stringified booleans ('true'/'false', any case) in a flat + * options object to real booleans, leaving every other value untouched. Boolean- + * typed accessibility options (e.g. autoScanning) are commonly supplied as + * strings from JS config / env; without coercion they fail W3C caps validation + * and are silently dropped. Only the exact strings 'true'/'false' are converted. + */ +export function coerceStringBooleans>(obj: T): T { + if (!obj || typeof obj !== 'object' || Array.isArray(obj)) { + return obj + } + const out: Record = {} + for (const [key, value] of Object.entries(obj)) { + if (typeof value === 'string') { + const normalised = value.trim().toLowerCase() + if (normalised === 'true' || normalised === 'false') { + out[key] = normalised === 'true' + continue + } + } + out[key] = value + } + return out as T +} + /** * Checks if a key is safe to use for object property assignment to prevent prototype pollution * @param key - The key to check diff --git a/packages/browserstack-service/tests/accessibility-handler.test.ts b/packages/browserstack-service/tests/accessibility-handler.test.ts index a381b4c..ddaf472 100644 --- a/packages/browserstack-service/tests/accessibility-handler.test.ts +++ b/packages/browserstack-service/tests/accessibility-handler.test.ts @@ -28,6 +28,43 @@ vi.mock('uuid', () => ({ v4: () => '123456789' })) const bstackLoggerSpy = vi.spyOn(bstackLogger.BStackLogger, 'logToFile') bstackLoggerSpy.mockImplementation(() => {}) +describe('shouldSkipScanForBidiWindowCommand (SDK-5047)', () => { + const skip = (b: any, c: any) => (AccessibilityHandler as any).shouldSkipScanForBidiWindowCommand(b, c) + + it('skips the injected a11y scan for window/context commands on BiDi sessions', () => { + for (const name of ['getWindowHandle', 'getWindowHandles', 'switchToWindow', 'switchWindow', 'newWindow', 'closeWindow', 'switchFrame', 'switchToFrame', 'switchToParentFrame']) { + expect(skip({ isBidi: true }, { name, class: 'Browser' })).toBe(true) + } + }) + + it('does not skip for non-window commands on BiDi sessions', () => { + expect(skip({ isBidi: true }, { name: 'click', class: 'Element' })).toBe(false) + expect(skip({ isBidi: true }, { name: 'url', class: 'Browser' })).toBe(false) + expect(skip({ isBidi: true }, { name: 'execute', class: 'Browser' })).toBe(false) + }) + + it('does not skip on non-BiDi sessions even for window commands', () => { + expect(skip({ isBidi: false }, { name: 'getWindowHandle', class: 'Browser' })).toBe(false) + expect(skip({}, { name: 'switchToWindow', class: 'Browser' })).toBe(false) + }) + + it('is safe with undefined browser or missing command name', () => { + expect(skip(undefined, { name: 'getWindowHandle', class: 'Browser' })).toBe(false) + expect(skip({ isBidi: true }, {})).toBe(false) + }) + + it('skips for window commands when any multiremote child instance is BiDi', () => { + const multi = { instances: ['chromeA', 'chromeB'], chromeA: { isBidi: false }, chromeB: { isBidi: true } } + expect(skip(multi, { name: 'getWindowHandle', class: 'Browser' })).toBe(true) + expect(skip(multi, { name: 'switchToWindow', class: 'Browser' })).toBe(true) + }) + + it('does not skip on multiremote when no child instance is BiDi', () => { + const multi = { instances: ['chromeA', 'chromeB'], chromeA: { isBidi: false }, chromeB: {} } + expect(skip(multi, { name: 'getWindowHandle', class: 'Browser' })).toBe(false) + }) +}) + beforeEach(() => { vi.mocked(log.info).mockClear() vi.mocked(fetch).mockClear() diff --git a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts index d755e0e..00c4443 100644 --- a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts +++ b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts @@ -51,6 +51,8 @@ vi.mock('../../../src/cli/grpcClient.js', () => ({ })) import AccessibilityModule from '../../../src/cli/modules/accessibilityModule.js' +import { validateCapsWithA11y, validateCapsWithAppA11y } from '../../../src/util.js' +import accessibilityScripts from '../../../src/scripts/accessibility-scripts.js' import TestFramework from '../../../src/cli/frameworks/testFramework.js' import AutomationFramework from '../../../src/cli/frameworks/automationFramework.js' import { AutomationFrameworkState } from '../../../src/cli/states/automationFrameworkState.js' @@ -366,4 +368,79 @@ describe('AccessibilityModule', () => { expect(result).toEqual({}) }) }) + + // SDK-3813: App Automate + App Accessibility sessions were mis-routed onto the + // web a11y path (Chrome-only gate + overwriteCommand on commands absent from the + // appium driver), so App A11y scans never ran. The CLI module must detect app + // sessions from caps (like the classic flow) and skip web command wrapping. + describe('onBeforeExecute - App Automate (SDK-3813)', () => { + const appGetState = (instance: any, key: string) => { + if (key.includes('input_capabilities')) { + return { 'appium:app': 'bs://BrowserStackMobileAppId' } + } + if (key.includes('capabilities')) { + return { platformName: 'android', platformVersion: '13' } + } + if (key.includes('session_id')) { + return 12345 + } + return {} + } + + afterEach(() => { + accessibilityScripts.commandsToWrap = [] + }) + + it('detects app session from caps and skips web command-overwrite', async () => { + // binary flag is false; caps say app -> module must still take app path + vi.mocked(validateCapsWithAppA11y).mockReturnValue(true) + vi.mocked(validateCapsWithA11y).mockReturnValue(true) + accessibilityScripts.commandsToWrap = [ + { name: 'click', class: 'Browser' }, + { name: 'startA11yScanning', class: 'Browser' } + ] + vi.mocked(AutomationFramework.getState).mockImplementation(appGetState as any) + + await accessibilityModule.onBeforeExecute() + + expect(accessibilityModule.isAppAccessibility).toBe(true) + // Symptom 1: no web command-overwrite attempted on an app session + expect(mockBrowser.overwriteCommand).not.toHaveBeenCalled() + // Symptom 2: Chrome-only web gate never consulted; app validation used + expect(validateCapsWithAppA11y).toHaveBeenCalled() + expect(validateCapsWithA11y).not.toHaveBeenCalled() + }) + + it('engages the app performScan path (execute, not web executeAsync)', async () => { + vi.mocked(validateCapsWithAppA11y).mockReturnValue(true) + vi.mocked(validateCapsWithA11y).mockReturnValue(true) + vi.mocked(AutomationFramework.getState).mockImplementation(appGetState as any) + mockBrowser.execute.mockResolvedValue({ scanned: true }) + + await accessibilityModule.onBeforeExecute() + const result = await mockBrowser.performScan() + + expect(mockBrowser.execute).toHaveBeenCalled() + expect(mockBrowser.executeAsync).not.toHaveBeenCalled() + expect(result).toEqual({ scanned: true }) + }) + + it('does not abort onBeforeExecute when web command wrapping would throw', async () => { + vi.mocked(validateCapsWithAppA11y).mockReturnValue(true) + vi.mocked(validateCapsWithA11y).mockReturnValue(true) + accessibilityScripts.commandsToWrap = [{ name: 'startA11yScanning', class: 'Browser' }] + mockBrowser.overwriteCommand = vi.fn(() => { + throw new Error('overwriteCommand: no command to be overwritten: startA11yScanning') + }) + const errorSpy = vi.spyOn(accessibilityModule.logger, 'error') + vi.mocked(AutomationFramework.getState).mockImplementation(appGetState as any) + + await accessibilityModule.onBeforeExecute() + + expect(mockBrowser.overwriteCommand).not.toHaveBeenCalled() + expect(errorSpy).not.toHaveBeenCalledWith( + expect.stringContaining('Error in onBeforeExecute') + ) + }) + }) }) \ No newline at end of file diff --git a/packages/browserstack-service/tests/launcher.test.ts b/packages/browserstack-service/tests/launcher.test.ts index 16cfe8b..e2219a7 100644 --- a/packages/browserstack-service/tests/launcher.test.ts +++ b/packages/browserstack-service/tests/launcher.test.ts @@ -657,12 +657,14 @@ describe('onComplete', () => { .then(() => expect(service.browserstackLocal?.stop).toHaveBeenCalled()) }) - it('should stop accessibility test run on complete', () => { + it('should stop accessibility test run on complete', async () => { const stopBuildUpstreamSpy = vi.spyOn(utils, 'stopBuildUpstream') vi.spyOn(utils, 'isAccessibilityAutomationSession').mockReturnValue(true) const service = new BrowserstackLauncher({} as any, [{}] as any, {} as any) - service.onComplete() + // onComplete is async (and now awaits orphaned-run finalization before + // stopping the build) — await it instead of relying on its sync prefix. + await service.onComplete() expect(stopBuildUpstreamSpy).toHaveBeenCalledTimes(1) }) }) diff --git a/packages/browserstack-service/tests/service.test.ts b/packages/browserstack-service/tests/service.test.ts index b34fc70..7cfd42e 100644 --- a/packages/browserstack-service/tests/service.test.ts +++ b/packages/browserstack-service/tests/service.test.ts @@ -6,6 +6,7 @@ import logger from '@wdio/logger' import BrowserstackService from '../src/service.js' import * as utils from '../src/util.js' import InsightsHandler from '../src/insights-handler.js' +import { BrowserstackCLI } from '../src/cli/index.js' import * as bstackLogger from '../src/bstackLogger.js' const jasmineSuiteTitle = 'Jasmine__TopLevel__Suite' @@ -2332,3 +2333,50 @@ describe('ignoreHooksStatus feature', () => { }) }) }) + +describe('beforeHook (CLI hook reporting)', () => { + const hookTest = { title: '"before all" hook for "spec"' } as any + let getInstanceSpy: ReturnType + + // Restore only our own spy so the suite's shared mocks stay intact. + afterEach(() => { + getInstanceSpy?.mockRestore() + }) + + it('routes the hook to the CLI TestFramework tracker when the CLI is running', async () => { + const trackEvent = vi.fn().mockResolvedValue(undefined) + getInstanceSpy = vi.spyOn(BrowserstackCLI, 'getInstance').mockReturnValue({ + isRunning: () => true, + getTestFramework: () => ({ trackEvent }) + } as any) + service['_insightsHandler'] = { beforeHook: vi.fn() } as any + + await service.beforeHook(hookTest, {}) + + expect(trackEvent).toHaveBeenCalledTimes(1) + expect(service['_insightsHandler']!.beforeHook).not.toHaveBeenCalled() + }) + + it('degrades quietly (no throw, no legacy fallback) when the CLI is running but the tracker is null', async () => { + getInstanceSpy = vi.spyOn(BrowserstackCLI, 'getInstance').mockReturnValue({ + isRunning: () => true, + getTestFramework: () => null + } as any) + service['_insightsHandler'] = { beforeHook: vi.fn() } as any + + await expect(service.beforeHook(hookTest, {})).resolves.toBeUndefined() + expect(service['_insightsHandler']!.beforeHook).not.toHaveBeenCalled() + }) + + it('falls back to the legacy insights handler when the CLI is not running', async () => { + getInstanceSpy = vi.spyOn(BrowserstackCLI, 'getInstance').mockReturnValue({ + isRunning: () => false, + getTestFramework: () => null + } as any) + service['_insightsHandler'] = { beforeHook: vi.fn() } as any + + await service.beforeHook(hookTest, {}) + + expect(service['_insightsHandler']!.beforeHook).toHaveBeenCalledTimes(1) + }) +}) diff --git a/packages/browserstack-service/tests/testOps/openRunsJournal.test.ts b/packages/browserstack-service/tests/testOps/openRunsJournal.test.ts new file mode 100644 index 0000000..d7b2dfa --- /dev/null +++ b/packages/browserstack-service/tests/testOps/openRunsJournal.test.ts @@ -0,0 +1,124 @@ +import { expect, vi, it, describe, beforeEach, afterEach } from 'vitest' +import fs from 'node:fs' + +import * as OpenRunsJournal from '../../src/testOps/openRunsJournal.js' +import * as utils from '../../src/util.js' + +vi.mock('../../src/bstackLogger.js', () => ({ + BStackLogger: { + debug: vi.fn(), + info: vi.fn(), + }, +})) + +vi.mock('fs', () => ({ + default: { + readdirSync: vi.fn(), + existsSync: vi.fn(), + readFileSync: vi.fn(), + rmSync: vi.fn(), + mkdirSync: vi.fn(), + writeFileSync: vi.fn() + } +})) + +describe('openRunsJournal', () => { + let mockFs: any + beforeEach(() => { + vi.clearAllMocks() + mockFs = vi.mocked(fs) + }) + + afterEach(() => { + vi.clearAllMocks() + }) + + describe('recordOpenRun', () => { + it('persists the started test data keyed by uuid', () => { + const testData = { uuid: 'uuid-1', name: 'my test', started_at: new Date().toISOString() } + OpenRunsJournal.recordOpenRun(testData) + expect(mockFs.mkdirSync).toHaveBeenCalledOnce() + expect(mockFs.writeFileSync).toHaveBeenCalledWith(expect.stringContaining('open-run-uuid-1.json'), JSON.stringify(testData)) + }) + + it('is a no-op without a uuid and never throws on fs errors', () => { + OpenRunsJournal.recordOpenRun({} as any) + expect(mockFs.writeFileSync).not.toHaveBeenCalled() + + mockFs.writeFileSync.mockImplementationOnce(() => { throw new Error('disk full') }) + expect(() => OpenRunsJournal.recordOpenRun({ uuid: 'uuid-err' })).not.toThrow() + }) + }) + + describe('clearOpenRun', () => { + it('removes the journal entry for the finished test', () => { + OpenRunsJournal.clearOpenRun('uuid-2') + expect(mockFs.rmSync).toHaveBeenCalledWith(expect.stringContaining('open-run-uuid-2.json'), { force: true }) + }) + + it('is a no-op without a uuid', () => { + OpenRunsJournal.clearOpenRun(undefined) + expect(mockFs.rmSync).not.toHaveBeenCalled() + }) + }) + + describe('collectOrphanedRuns', () => { + it('returns [] when the journal dir does not exist', () => { + mockFs.existsSync.mockReturnValueOnce(false) + expect(OpenRunsJournal.collectOrphanedRuns()).toEqual([]) + }) + + it('reads every journal file, removes the dir and skips unreadable entries', () => { + mockFs.existsSync.mockReturnValueOnce(true) + mockFs.readdirSync.mockReturnValueOnce(['open-run-a.json', 'open-run-bad.json'] as any) + mockFs.readFileSync.mockImplementation((filePath: string) => { + if (filePath.includes('open-run-a.json')) { + return JSON.stringify({ uuid: 'a' }) + } + throw new Error('corrupt') + }) + const orphans = OpenRunsJournal.collectOrphanedRuns() + expect(orphans).toEqual([{ uuid: 'a' }]) + expect(mockFs.rmSync).toHaveBeenCalledWith(expect.stringContaining('bstack_open_runs'), { recursive: true, force: true }) + }) + }) + + describe('finalizeOrphanedRuns', () => { + it('does nothing when there are no orphans', async () => { + mockFs.existsSync.mockReturnValueOnce(false) + const batchSpy = vi.spyOn(utils, 'batchAndPostEvents').mockResolvedValueOnce(undefined as any) + expect(await OpenRunsJournal.finalizeOrphanedRuns()).toBe(0) + expect(batchSpy).not.toHaveBeenCalled() + }) + + it('posts a failed TestRunFinished per orphan and reports the count', async () => { + const startedAt = new Date(Date.now() - 5000).toISOString() + mockFs.existsSync.mockReturnValueOnce(true) + mockFs.readdirSync.mockReturnValueOnce(['open-run-a.json'] as any) + mockFs.readFileSync.mockReturnValueOnce(JSON.stringify({ uuid: 'a', name: 'stuck test', started_at: startedAt })) + const batchSpy = vi.spyOn(utils, 'batchAndPostEvents').mockResolvedValueOnce(undefined as any) + + expect(await OpenRunsJournal.finalizeOrphanedRuns()).toBe(1) + + expect(batchSpy).toHaveBeenCalledOnce() + const [, kind, events] = batchSpy.mock.calls[0] + expect(kind).toBe('ORPHANED_TEST_RUN_FINALIZATION') + expect(events).toHaveLength(1) + const event = (events as any)[0] + expect(event.event_type).toBe('TestRunFinished') + expect(event.test_run.uuid).toBe('a') + expect(event.test_run.result).toBe('failed') + expect(event.test_run.failure_reason).toBe(OpenRunsJournal.ORPHAN_FAILURE_REASON) + expect(event.test_run.finished_at).toBeTruthy() + expect(event.test_run.duration_in_ms).toBeGreaterThanOrEqual(5000) + }) + + it('swallows upload failures and returns 0', async () => { + mockFs.existsSync.mockReturnValueOnce(true) + mockFs.readdirSync.mockReturnValueOnce(['open-run-a.json'] as any) + mockFs.readFileSync.mockReturnValueOnce(JSON.stringify({ uuid: 'a' })) + vi.spyOn(utils, 'batchAndPostEvents').mockRejectedValueOnce(new Error('no jwt')) + expect(await OpenRunsJournal.finalizeOrphanedRuns()).toBe(0) + }) + }) +}) diff --git a/packages/browserstack-service/tests/util.test.ts b/packages/browserstack-service/tests/util.test.ts index f3d4690..1260156 100644 --- a/packages/browserstack-service/tests/util.test.ts +++ b/packages/browserstack-service/tests/util.test.ts @@ -2255,3 +2255,17 @@ describe('getAppA11yResultsSummary', () => { expect(result).toEqual({ }) }) }) + +describe('coerceStringBooleans (SDK-3737)', () => { + it("coerces exact 'true'/'false' strings (any case) to booleans", () => { + expect(utils.coerceStringBooleans({ autoScanning: 'false' })).toEqual({ autoScanning: false }) + expect(utils.coerceStringBooleans({ autoScanning: 'true', other: 'TRUE' })).toEqual({ autoScanning: true, other: true }) + }) + it('leaves non-boolean strings, real booleans and numbers untouched', () => { + expect(utils.coerceStringBooleans({ wcagVersion: 'wcag21a', autoScanning: true, n: 5 })) + .toEqual({ wcagVersion: 'wcag21a', autoScanning: true, n: 5 }) + }) + it('is safe with empty input', () => { + expect(utils.coerceStringBooleans({})).toEqual({}) + }) +})