From 9cc3d4a6a5d272e39f55a3204da3a74fd9df30a9 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 19:51:58 +0530 Subject: [PATCH 01/17] feat(app-a11y): scan the config-level before() window, with a hook run TRA can resolve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebuilt on top of the reloadSession fix and its review corrections, so this branch is now the before() work alone — the earlier parked version predated those and still carried the reverted alias config-capture. Driver commands issued from WDIO's config-level before()/beforeSession() were never scanned. Those hooks are not test-framework hooks, so neither onHookStart nor onBeforeTest had run and the scan gate had no entry for the session at all — while the rake still counted those commands as expected coverage. The gate now opens at driver creation, unconditionally when autoScanning is on, matching what onHookStart already does for Mocha hooks. Scans from that window also had no parent: no thTestRunUuid (no test exists yet) and no thHookRunUuid, so App-A11y's lookup of the scan's parent in BTCER found nothing. The window is now reported as a BEFORE_ALL hook run through the existing framework path, so the framework mints the uuid, the binary emits HookRunStarted/Finished, and onHookStart stamps it onto the scans. Opened ON DEMAND, from the scan path only. WDIO gives a service no event for a user's config-level before() — ConfigParser folds the user's config hooks and every service's hooks into one array the runner fires with Promise.all — so there is nothing to detect, and opening it from the lifecycle would report a hook that never ran on every build in the fleet. hookInstrumentation logs the start and finish of every session-scoped config hook, which is how that window becomes observable at all. Logging only, no events. SDK-7422 --- .../src/cli/modules/accessibilityModule.ts | 87 +++++++++++++ .../src/hookInstrumentation.ts | 117 ++++++++++++++++++ packages/browserstack-service/src/service.ts | 5 + .../cli/modules/accessibilityModule.test.ts | 79 ++++++++++++ 4 files changed, 288 insertions(+) create mode 100644 packages/browserstack-service/src/hookInstrumentation.ts diff --git a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts index 8e44b11..fd33b42 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -12,6 +12,7 @@ import type { Command } from '../../scripts/accessibility-scripts.js' import accessibilityScripts from '../../scripts/accessibility-scripts.js' import { _getParamsForAppAccessibility, formatString, getAppA11yResults, getAppA11yResultsSummary, shouldScanTestForAccessibility, validateCapsWithA11y, validateCapsWithAppA11y, isBrowserstackSession } from '../../util.js' import { AutomationFrameworkConstants } from '../frameworks/constants/automationFrameworkConstants.js' +import { BROWSERSTACK_WDIO_CONFIG_FILE_PATH } from '../../constants.js' import util from 'node:util' import type { Accessibility } from '../../grpc/index.js' import PerformanceTester from '../../instrumentation/performance/performance-tester.js' @@ -39,6 +40,15 @@ export default class AccessibilityModule extends BaseModule { // Any scan fired while this is set is stamped with it as thHookRunUuid so the backend // (SeleniumHub appAllyScan → hook_run_uuid) can reconcile the scan onto the wrapping test. currentHookRunUuid: string | null = null + // Lifecycle of the hook run that carries scans fired BEFORE any test exists — i.e. from + // WDIO's config-level before()/beforeSession(), which are not test-framework hooks and get no + // hook run of their own. + // + // Opened LAZILY, on the first such scan, and never otherwise: a suite with no config-level + // hook (or one that touches no driver) must not report a phantom hook to TRA, and that is the + // overwhelming majority of suites. 'attempted' is distinct from 'open' so a failed PRE is + // neither retried on every command nor closed by a POST that would pair with nothing. + private preTestHookState: 'idle' | 'attempted' | 'open' | 'closed' = 'idle' constructor(accessibilityConfig: Accessibility, isNonBstackA11y: boolean) { super() @@ -102,6 +112,60 @@ export default class AccessibilityModule extends BaseModule { this.currentHookRunUuid = null } + /** + * Open a BEFORE_ALL hook run for the pre-test window, if one is warranted. + * + * Called from the scan path, so it fires only when there is actually a scan needing a parent. + * The framework stamps the run uuid (KEY_HOOK_ID) and this module's own onHookStart observer + * picks it up, which is what puts thHookRunUuid on the scan; the binary turns the same event + * into HookRunStarted, so App-A11y's lookup of that uuid in BTCER resolves. + */ + private async ensurePreTestHookRun() { + if (this.preTestHookState !== 'idle' || this.currentHookRunUuid !== null) { + return + } + // Mark before awaiting: concurrent commands must not each open their own hook run. + this.preTestHookState = 'attempted' + try { + const framework = BrowserstackCLI.getInstance()?.getTestFramework() + if (!framework) { + return + } + await framework.trackEvent(TestFrameworkState.BEFORE_ALL, HookState.PRE, this.preTestHookArgs()) + this.preTestHookState = 'open' + } catch (error) { + this.logger.debug(`Could not open a hook run for the pre-test window: ${error}`) + } + } + + private async closePreTestHookRun() { + if (this.preTestHookState !== 'open') { + this.preTestHookState = 'closed' + return + } + this.preTestHookState = 'closed' + try { + const framework = BrowserstackCLI.getInstance()?.getTestFramework() + await framework?.trackEvent(TestFrameworkState.BEFORE_ALL, HookState.POST, { ...this.preTestHookArgs(), result: { passed: true } }) + } catch (error) { + this.logger.debug(`Could not close the hook run for the pre-test window: ${error}`) + } + } + + /** + * The binary runs path.relative() over this file path when building the hook record, so an + * absent path would throw there and drop the event. configCapture has already resolved the + * wdio config path onto the environment by this point. + */ + private preTestHookArgs() { + return { + test: { + file: process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] || process.cwd(), + title: 'wdio config-level before hook (pre-test window)' + } + } + } + /** * browser.reloadSession() hands the worker a NEW session id while the driver object, the * wrapped commands and the currently running test all stay exactly the same. The scan gate @@ -263,6 +327,22 @@ export default class AccessibilityModule extends BaseModule { }) } + // Open the scan gate for the window between driver creation and the first test. + // WDIO's own config-level hooks (`before`, `beforeSession`) run here, and they are not + // test-framework hooks, so neither onHookStart nor onBeforeTest has fired yet and the + // gate would have no entry for this session at all: every command issued from a + // config-level before() went unscanned while still counting as expected coverage. Real + // suites do substantial UI work there (app launch, login, account selection), so those + // screens are exactly the ones a customer expects covered. + // + // Unconditionally true, matching onHookStart: per-test include/exclude filters need a + // test to evaluate and there is none yet, and the following onBeforeTest recomputes the + // gate for the test proper. + const sessionId = this.currentSessionId() + if (this.autoScanning && sessionId !== undefined && sessionId !== null) { + this.accessibilityMap.set(sessionId, true) + } + } catch (error) { this.logger.error(`Error in onBeforeExecute: ${error}`) } @@ -281,6 +361,7 @@ export default class AccessibilityModule extends BaseModule { !command.name.includes('execute') || !this.shouldPatchExecuteScript(args.length ? args[0] as string : null) ) { + await this.ensurePreTestHookRun() try { await this.performScanCli(browser, command.name, this.currentHookRunUuid) this.logger.debug(`Accessibility scan performed after ${command.name} command`) @@ -305,6 +386,12 @@ export default class AccessibilityModule extends BaseModule { async onBeforeTest(args: Record) { try { this.logger.debug('Accessibility before test hook. Starting accessibility scan for this test case.') + // The pre-test window is over: close its hook run (if one was opened) before clearing + // the uuid, so the POST pairs against the record the PRE pushed. onHookEnd is the only + // other thing that clears this, so without the clear a spec with no framework hooks + // would stamp every test-body scan with the pre-test hook uuid. + await this.closePreTestHookRun() + this.currentHookRunUuid = null const suiteTitle = (typeof args.suiteTitle === 'string' ? args.suiteTitle : '') || '' const test = (args.test && typeof args.test === 'object' ? args.test as { title?: string } : {}) || {} diff --git a/packages/browserstack-service/src/hookInstrumentation.ts b/packages/browserstack-service/src/hookInstrumentation.ts new file mode 100644 index 0000000..43c3f6b --- /dev/null +++ b/packages/browserstack-service/src/hookInstrumentation.ts @@ -0,0 +1,117 @@ +import type { Options } from '@wdio/types' + +import { BStackLogger } from './bstackLogger.js' + +/** + * WDIO config hooks that execute in the worker process WITH a live session, so a driver + * command issued inside them is meaningful. + * + * Deliberately excluded: + * - `onPrepare` / `onWorkerStart` / `onWorkerEnd` / `onComplete` — launcher process, no driver. + * - `beforeSession` — runs before the session is created; there is no browser yet. + * - `afterSession` — the session is already being deleted; commands there fail. + * - `beforeCommand` / `afterCommand` — a live driver, but per-command: their window is the + * command, which is already observable, and wrapping them would log on every interaction. + */ +export const BROWSER_CONTEXT_HOOKS = [ + 'before', + 'beforeSuite', + 'beforeHook', + 'beforeTest', + 'afterTest', + 'afterHook', + 'afterSuite', + 'after' +] as const + +type HookFn = (...args: unknown[]) => unknown + +const INSTRUMENTED = Symbol('bstackHookInstrumented') + +/** + * Wrap one handler so its entry and exit are observable, preserving its shape exactly. + * + * Sync handlers stay sync: an `async` wrapper would turn every sync hook into a promise and a + * synchronous throw into a rejection, which changes ordering for anything that calls a hook + * without awaiting it. So the wrapper only attaches to the promise when the handler actually + * returned one. + */ +const instrument = (hookName: string, index: number, fn: HookFn): HookFn => { + if ((fn as unknown as Record)[INSTRUMENTED]) { + return fn + } + + // `fn.name` is the only identity WDIO leaves: ConfigParser binds every handler it folds in + // (`hook.bind(service)`), so both the user's config hooks and other services' hooks read as + // `bound `. Logged anyway — it separates a named user function from an anonymous one. + const label = `${hookName}#${index}${fn.name ? ` (${fn.name})` : ''}` + const wrapped = function (this: unknown, ...args: unknown[]) { + const startedAt = Date.now() + const finish = (outcome: string) => BStackLogger.debug(`[hook-window] ${label} ${outcome} in ${Date.now() - startedAt}ms`) + + BStackLogger.debug(`[hook-window] ${label} started`) + let result: unknown + try { + result = fn.apply(this, args) + } catch (error) { + finish(`threw: ${(error as Error)?.message}`) + throw error + } + + if (result && typeof (result as Promise).then === 'function') { + return (result as Promise).then( + (value) => { + finish('finished') + return value + }, + (error) => { + finish(`rejected: ${(error as Error)?.message}`) + throw error + } + ) + } + + finish('finished') + return result + } as HookFn + + Object.defineProperty(wrapped, INSTRUMENTED, { value: true }) + return wrapped +} + +/** + * Make the boundaries of every session-scoped config hook observable. + * + * WDIO tells a service nothing about the other handlers registered for a hook: `ConfigParser` + * folds the user's config hooks and every service's hooks into one array per hook name, and the + * runner fires them together (`executeHooksWithArgs` → `Promise.all`). Patching the array in + * place is therefore the only way to see when a handler that is not ours starts and finishes — + * which is what bounds the windows a driver command can fall into. + * + * The index in the label is the handler's position in that merged array, so it includes this + * service's own handlers; it identifies a handler across its start/finish pair, nothing more. + * + * Logging only, for now: no events are emitted and no behaviour changes. Idempotent, and any + * failure is swallowed — instrumentation must never be the reason a suite cannot start. + */ +export function instrumentBrowserContextHooks(config?: Options.Testrunner): void { + if (!config) { + return + } + + try { + const record = config as unknown as Record + for (const hookName of BROWSER_CONTEXT_HOOKS) { + const handlers = record[hookName] + if (!Array.isArray(handlers) || handlers.length === 0) { + continue + } + BStackLogger.debug(`[hook-window] instrumenting ${hookName}: ${handlers.length} handler(s) registered at service construction`) + record[hookName] = handlers.map((handler, index) => + typeof handler === 'function' ? instrument(hookName, index, handler as HookFn) : handler + ) + } + } catch (error) { + BStackLogger.debug(`Could not instrument config hooks: ${error}`) + } +} diff --git a/packages/browserstack-service/src/service.ts b/packages/browserstack-service/src/service.ts index 52cdc3d..a40babf 100644 --- a/packages/browserstack-service/src/service.ts +++ b/packages/browserstack-service/src/service.ts @@ -24,6 +24,7 @@ import CustomTagsHandler from './custom-tags-handler.js' import { classifyMochaHookTitle, setCurrentMochaHookWindow } from './customTags.js' import type TestHubModule from './cli/modules/testHubModule.js' import { BStackLogger } from './bstackLogger.js' +import { instrumentBrowserContextHooks } from './hookInstrumentation.js' import PercyHandler from './Percy/Percy-Handler.js' import Listener from './testOps/listener.js' import { saveWorkerData } from './data-store.js' @@ -119,6 +120,10 @@ export default class BrowserstackService implements Services.ServiceInstance { const bailOpt: unknown = this._config?.mochaOpts?.bail this._mochaBail = this._config?.framework === 'mocha' && Boolean(bailOpt) && !isFalse(bailOpt) + // Make the boundaries of the user's own session-scoped config hooks observable. Logging + // only; patched here because the runner reads these arrays after the service is built. + instrumentBrowserContextHooks(this._config) + PerformanceTester.startMonitoring('performance-report-service.csv') if (shouldProcessEventForTesthub('')) { this._config.reporters?.push(TestReporter) diff --git a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts index 0c77511..6fc75b3 100644 --- a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts +++ b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts @@ -40,6 +40,19 @@ vi.mock('../../../src/util.js', () => ({ isBrowserstackSession: vi.fn().mockReturnValue(true) })) +// hoisted: vi.mock factories are lifted above module scope, and `getInstance` is a plain +// function rather than a spy so the suite-wide clearAllMocks/resetAllMocks cannot strip its +// return value out from under the module constructor. +const { mockTrackEvent } = vi.hoisted(() => ({ mockTrackEvent: vi.fn() })) +vi.mock('../../../src/cli/index.js', () => ({ + BrowserstackCLI: { + getInstance: () => ({ + options: {}, + getTestFramework: () => ({ trackEvent: mockTrackEvent }) + }) + } +})) + vi.mock('../../../src/cli/grpcClient.js', () => ({ GrpcClient: { getInstance: vi.fn().mockReturnValue({ @@ -110,6 +123,7 @@ describe('AccessibilityModule', () => { afterEach(() => { vi.resetAllMocks() + mockTrackEvent.mockReset() }) describe('constructor', () => { @@ -200,6 +214,71 @@ describe('AccessibilityModule', () => { }) }) + describe('pre-test scan gate and its hook run', () => { + const withA11yOn = () => { + vi.mocked(validateCapsWithA11y).mockReturnValue(true) + vi.mocked(AutomationFramework.getState).mockImplementation((instance, key) => { + if (key.includes('CAPABILITIES')) { + return { browserName: 'chrome' } + } + return 'live-session' + }) + } + + it('opens the scan gate at driver creation, so config-level before() commands scan', async () => { + // WDIO's config-level before()/beforeSession() are not test-framework hooks, so + // neither onHookStart nor onBeforeTest has run when they fire their commands. + withA11yOn() + + await accessibilityModule.onBeforeExecute() + + expect(accessibilityModule.accessibilityMap.get('live-session')).toBe(true) + }) + + it('leaves the gate closed when autoScanning is off', async () => { + withA11yOn() + accessibilityModule.autoScanning = false + + await accessibilityModule.onBeforeExecute() + + expect(accessibilityModule.accessibilityMap.has('live-session')).toBe(false) + }) + + it('opens a BEFORE_ALL hook run the first time a scan needs a parent, once only', async () => { + await accessibilityModule['ensurePreTestHookRun']() + await accessibilityModule['ensurePreTestHookRun']() + + const opens = mockTrackEvent.mock.calls.filter((c) => c[1] === HookState.PRE) + expect(opens).toHaveLength(1) + expect(opens[0][0]).toBe(TestFrameworkState.BEFORE_ALL) + expect((opens[0][2] as { test: { title: string } }).test.title).toContain('pre-test window') + }) + + it('opens nothing while a framework hook is already the scan parent', async () => { + accessibilityModule.currentHookRunUuid = 'framework-hook-uuid' + + await accessibilityModule['ensurePreTestHookRun']() + + expect(mockTrackEvent).not.toHaveBeenCalled() + }) + + it('closes the hook run when the first test starts, and never closes one it did not open', async () => { + await accessibilityModule['ensurePreTestHookRun']() + mockTrackEvent.mockClear() + + await accessibilityModule.onBeforeTest({ suiteTitle: 'S', test: { title: 't' } }) + + const closes = mockTrackEvent.mock.calls.filter((c) => c[1] === HookState.POST) + expect(closes).toHaveLength(1) + expect((closes[0][2] as { result: { passed: boolean } }).result.passed).toBe(true) + expect(accessibilityModule.currentHookRunUuid).toBeNull() + + mockTrackEvent.mockClear() + await accessibilityModule.onBeforeTest({ suiteTitle: 'S', test: { title: 't2' } }) + expect(mockTrackEvent.mock.calls.filter((c) => c[1] === HookState.POST)).toHaveLength(0) + }) + }) + describe('onSessionReload', () => { it('carries the scan gate over to the new session id', () => { accessibilityModule.accessibilityMap.set('old', true) From adb8b2316bc03c2de770dd64fcedc15ea6c31644 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 19:59:28 +0530 Subject: [PATCH 02/17] fix(service): keep @Measure bound to onReload, and guard the placement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The predicate added in cec234c landed between the decorator and the method, so the decoration moved to it: onReload stopped emitting its SDK_HOOK measurement, and the predicate started reporting one under hookType 'onReload' on every call — including from the before hook, so every session reported an onReload measurement whether or not it ever reloaded. Behaviour was unaffected (PerformanceTester.measure returns the raw value on the synchronous path), which is why nothing surfaced it: valid TypeScript, lint has no opinion, and the unit suites mock Measure into a pass-through so the binding has no observable behaviour to assert. Hence a structural test over the decorated files instead — verified to fail with the mistake reintroduced and pass without it. Doc comment corrected too: the two accessibility paths are NOT strictly exclusive. The module is registered on startBinResponse.accessibility?.success alone, independent of provider, so under a non-BrowserStack provider it can hold real state while the classic handler's before() has also run. What the predicate decides is which one owns the CLASSIC handler's state. SDK-7422 --- packages/browserstack-service/src/service.ts | 20 +++++---- .../tests/decoratorPlacement.test.ts | 41 +++++++++++++++++++ 2 files changed, 54 insertions(+), 7 deletions(-) create mode 100644 packages/browserstack-service/tests/decoratorPlacement.test.ts diff --git a/packages/browserstack-service/src/service.ts b/packages/browserstack-service/src/service.ts index a40babf..a66a2d1 100644 --- a/packages/browserstack-service/src/service.ts +++ b/packages/browserstack-service/src/service.ts @@ -890,21 +890,27 @@ export default class BrowserstackService implements Services.ServiceInstance { await this._insightsHandler?.afterStep(step, scenario, result) } - @PerformanceTester.Measure(PERFORMANCE_SDK_EVENTS.EVENTS.SDK_HOOK, { hookType: 'onReload' }) /** * Whether accessibility is being driven by the binary rather than the classic handler. * - * The two are mutually exclusive and this exact condition decides which: it is what gates - * `AccessibilityHandler.before()` in the `before` hook. Keeping it in one place matters — - * `isRunning()` alone is NOT equivalent. With the binary up against a non-BrowserStack - * provider, `isBrowserstackSession` is false, so the classic handler DID initialise and holds - * the live state; splitting on `isRunning()` there would migrate an empty module gate and - * leave the handler stale. + * Precisely: it decides whether the CLASSIC handler owns the session's accessibility state. + * It is the same condition that gates `AccessibilityHandler.before()` in the `before` hook, so + * keeping it in one place is what stops the two drifting. + * + * The two paths are not strictly exclusive. `AccessibilityModule` is registered on + * `startBinResponse.accessibility?.success` alone, independent of provider — `isNonBstackA11y` + * exists for exactly the turboscale / non-BrowserStack case — so with the binary up against a + * non-BrowserStack provider the module can hold real state AND the handler's `before()` will + * have run. Both then need migrating, which is what the caller does. + * + * `isRunning()` alone is NOT equivalent, and that is the trap: under a non-BrowserStack + * provider it is true while the classic handler is the one holding the live state. */ private _isCliAccessibilityFlow (): boolean { return Boolean(isBrowserstackSession(this._browser)) && BrowserstackCLI.getInstance().isRunning() } + @PerformanceTester.Measure(PERFORMANCE_SDK_EVENTS.EVENTS.SDK_HOOK, { hookType: 'onReload' }) async onReload(oldSessionId: string, newSessionId: string) { if (!this._browser) { return Promise.resolve() diff --git a/packages/browserstack-service/tests/decoratorPlacement.test.ts b/packages/browserstack-service/tests/decoratorPlacement.test.ts new file mode 100644 index 0000000..118a87e --- /dev/null +++ b/packages/browserstack-service/tests/decoratorPlacement.test.ts @@ -0,0 +1,41 @@ +import fs from 'node:fs' +import path from 'node:path' +import { describe, expect, it } from 'vitest' + +/** + * A decorator binds to the next class ELEMENT, not to the next thing that looks related. Insert a + * method between `@PerformanceTester.Measure(..., { hookType: 'onReload' })` and `onReload` and the + * decoration silently moves: the hook loses its telemetry, and the intruding method starts + * reporting measurements under the hook's name. + * + * Nothing else in this repo can catch that. The code stays valid TypeScript, lint has no opinion, + * and the unit suites mock `PerformanceTester.Measure` into a pass-through, so the binding has no + * observable behaviour to assert on. Hence a structural check. + */ +const DECORATED_FILES = ['service.ts', 'launcher.ts', 'insights-handler.ts', 'accessibility-handler.ts'] + +describe('PerformanceTester.Measure placement', () => { + for (const file of DECORATED_FILES) { + it(`binds directly to a method in ${file}`, () => { + const lines = fs.readFileSync(path.join(process.cwd(), 'src', file), 'utf8').split('\n') + const offenders: string[] = [] + + lines.forEach((line, index) => { + if (!line.includes('@PerformanceTester.Measure')) { + return + } + // walk to the next non-blank line; that is what the decorator will attach to + let next = index + 1 + while (next < lines.length && lines[next].trim() === '') { + next++ + } + const target = lines[next]?.trim() ?? '' + if (target.startsWith('/*') || target.startsWith('*') || target.startsWith('//')) { + offenders.push(`${file}:${index + 1} — decorator followed by a comment, not a method: ${target.slice(0, 60)}`) + } + }) + + expect(offenders).toEqual([]) + }) + } +}) From f4c85685a86a1b0b83657b42c1d05393200f2bd8 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 20:57:55 +0530 Subject: [PATCH 03/17] fix(hooks): report a failed config-level before() as failed, not passed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by running the case rather than reasoning about it: with a config-level before() that throws, WDIO leaves the exit code at 0 (executeHooksWithArgs resolves WITH the error instead of rejecting — deliberately, so a throwing config hook cannot fail the run), every reporter shows green, and before this commit the hook run this branch emits said result=passed. That is worse than the pre-existing silence. Unreported means the dashboard omits the failure; reporting `passed` means it asserts something false about it. The instrumentation wrapper is the only thing in the process that sees the rejection, so it now records the message per hook name, and closePreTestHookRun reads it and reports { passed: false, error: { message } }. Measured on the same bench, one line apart in the same log: before: HookRunFinished BEFORE_ALL result=passed uuid=1bcd33f4 after: HookRunFinished BEFORE_ALL result=failed uuid=d9c3ccb7 and the TRA build verdict moves from passed {passed: 2} to failed {passed: 1, failed: 1}. SDK-7422 --- .../src/cli/modules/accessibilityModule.ts | 9 ++++++++- .../src/hookInstrumentation.ts | 17 ++++++++++++++++ .../cli/modules/accessibilityModule.test.ts | 20 +++++++++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts index fd33b42..d24548a 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -13,6 +13,7 @@ import accessibilityScripts from '../../scripts/accessibility-scripts.js' import { _getParamsForAppAccessibility, formatString, getAppA11yResults, getAppA11yResultsSummary, shouldScanTestForAccessibility, validateCapsWithA11y, validateCapsWithAppA11y, isBrowserstackSession } from '../../util.js' import { AutomationFrameworkConstants } from '../frameworks/constants/automationFrameworkConstants.js' import { BROWSERSTACK_WDIO_CONFIG_FILE_PATH } from '../../constants.js' +import { getHookFailure } from '../../hookInstrumentation.js' import util from 'node:util' import type { Accessibility } from '../../grpc/index.js' import PerformanceTester from '../../instrumentation/performance/performance-tester.js' @@ -146,7 +147,13 @@ export default class AccessibilityModule extends BaseModule { this.preTestHookState = 'closed' try { const framework = BrowserstackCLI.getInstance()?.getTestFramework() - await framework?.trackEvent(TestFrameworkState.BEFORE_ALL, HookState.POST, { ...this.preTestHookArgs(), result: { passed: true } }) + // A config-level before() that threw must not be reported green. WDIO swallows the + // error (executeHooksWithArgs resolves WITH it rather than rejecting), so the run + // stays exit-0 and every reporter shows success — reporting `passed` here would make + // the dashboard actively assert something false rather than merely omit it. + const failure = getHookFailure('before') + const result = failure ? { passed: false, error: { message: failure } } : { passed: true } + await framework?.trackEvent(TestFrameworkState.BEFORE_ALL, HookState.POST, { ...this.preTestHookArgs(), result }) } catch (error) { this.logger.debug(`Could not close the hook run for the pre-test window: ${error}`) } diff --git a/packages/browserstack-service/src/hookInstrumentation.ts b/packages/browserstack-service/src/hookInstrumentation.ts index 43c3f6b..ea4fb09 100644 --- a/packages/browserstack-service/src/hookInstrumentation.ts +++ b/packages/browserstack-service/src/hookInstrumentation.ts @@ -28,6 +28,18 @@ type HookFn = (...args: unknown[]) => unknown const INSTRUMENTED = Symbol('bstackHookInstrumented') +/** + * Failure message per hook name, for the hooks WDIO reports to nobody. + * + * `executeHooksWithArgs` resolves with the error instead of rejecting — deliberately, so a + * throwing config hook cannot fail the run — which means a failed `before()` leaves the exit code + * at 0, the reporters green and the dashboard reading "passed". Anything that reports on that + * window needs a way to know it actually blew up, and this wrapper is the only thing that sees it. + */ +const hookFailures: Record = {} + +export const getHookFailure = (hookName: string): string | undefined => hookFailures[hookName] + /** * Wrap one handler so its entry and exit are observable, preserving its shape exactly. * @@ -45,6 +57,9 @@ const instrument = (hookName: string, index: number, fn: HookFn): HookFn => { // (`hook.bind(service)`), so both the user's config hooks and other services' hooks read as // `bound `. Logged anyway — it separates a named user function from an anonymous one. const label = `${hookName}#${index}${fn.name ? ` (${fn.name})` : ''}` + const recordFailure = (error: unknown) => { + hookFailures[hookName] = (error as Error)?.message || String(error) + } const wrapped = function (this: unknown, ...args: unknown[]) { const startedAt = Date.now() const finish = (outcome: string) => BStackLogger.debug(`[hook-window] ${label} ${outcome} in ${Date.now() - startedAt}ms`) @@ -54,6 +69,7 @@ const instrument = (hookName: string, index: number, fn: HookFn): HookFn => { try { result = fn.apply(this, args) } catch (error) { + recordFailure(error) finish(`threw: ${(error as Error)?.message}`) throw error } @@ -65,6 +81,7 @@ const instrument = (hookName: string, index: number, fn: HookFn): HookFn => { return value }, (error) => { + recordFailure(error) finish(`rejected: ${(error as Error)?.message}`) throw error } diff --git a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts index 6fc75b3..f0bbb13 100644 --- a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts +++ b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts @@ -53,6 +53,9 @@ vi.mock('../../../src/cli/index.js', () => ({ } })) +const { mockGetHookFailure } = vi.hoisted(() => ({ mockGetHookFailure: vi.fn() })) +vi.mock('../../../src/hookInstrumentation.js', () => ({ getHookFailure: mockGetHookFailure })) + vi.mock('../../../src/cli/grpcClient.js', () => ({ GrpcClient: { getInstance: vi.fn().mockReturnValue({ @@ -254,6 +257,23 @@ describe('AccessibilityModule', () => { expect((opens[0][2] as { test: { title: string } }).test.title).toContain('pre-test window') }) + it('reports the hook run as FAILED when the config-level before() threw', async () => { + // WDIO swallows a throwing config hook (executeHooksWithArgs resolves with the error), + // so the run stays exit-0 and every reporter is green. Reporting `passed` here would + // make the dashboard assert something false rather than merely omit it. + mockGetHookFailure.mockReturnValue('BOOM: config-level before hook failed') + await accessibilityModule['ensurePreTestHookRun']() + mockTrackEvent.mockClear() + + await accessibilityModule.onBeforeTest({ suiteTitle: 'S', test: { title: 't' } }) + + const close = mockTrackEvent.mock.calls.find((c) => c[1] === HookState.POST) + expect(close).toBeDefined() + const result = (close![2] as { result: { passed: boolean, error?: { message: string } } }).result + expect(result.passed).toBe(false) + expect(result.error?.message).toContain('BOOM') + }) + it('opens nothing while a framework hook is already the scan parent', async () => { accessibilityModule.currentHookRunUuid = 'framework-hook-uuid' From d5b9405d36c643db53d505841d776e0b60b8f852 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 21:10:44 +0530 Subject: [PATCH 04/17] fix(hooks): judge the whole pre-test window, not just before() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keying the failure lookup on the literal 'before' was too narrow. `beforeSuite` is a config hook with a live driver that the Mocha adapter registers as a root before-all (`this._runner.suite.beforeAll(this.wrapHook('beforeSuite'))`), so it runs after the config-level before and still ahead of the first test — inside the same window this hook run covers. A beforeSuite that threw was reported as a passed window. PRE_TEST_WINDOW_HOOKS names the hooks that can run there, and the message is prefixed with the hook that failed. Mocha's own hooks stay excluded: each has its own hook run with its own result, so folding them in would report the same failure twice. Instrumenting a config now also clears the record for those hooks. The map is process state for the worker's lifetime, so without it a failure could be reported against a later config. Verified on device with a clean before() and a throwing beforeSuite (build eq3p9ktfb1uiiq5mtxqj670cu42o6vf82z6nzcdx): [hook-window] before#0 finished in 2695ms [hook-window] beforeSuite#0 rejected: BOOM: beforeSuite failed in 4132ms HookRunFinished BEFORE_ALL result=failed uuid=187517a8 SDK-7422 --- .../src/cli/modules/accessibilityModule.ts | 4 +- .../src/hookInstrumentation.ts | 34 ++++++++++++ .../cli/modules/accessibilityModule.test.ts | 4 +- .../tests/hookInstrumentation.test.ts | 53 +++++++++++++++++++ 4 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 packages/browserstack-service/tests/hookInstrumentation.test.ts diff --git a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts index d24548a..0329f43 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -13,7 +13,7 @@ import accessibilityScripts from '../../scripts/accessibility-scripts.js' import { _getParamsForAppAccessibility, formatString, getAppA11yResults, getAppA11yResultsSummary, shouldScanTestForAccessibility, validateCapsWithA11y, validateCapsWithAppA11y, isBrowserstackSession } from '../../util.js' import { AutomationFrameworkConstants } from '../frameworks/constants/automationFrameworkConstants.js' import { BROWSERSTACK_WDIO_CONFIG_FILE_PATH } from '../../constants.js' -import { getHookFailure } from '../../hookInstrumentation.js' +import { getPreTestWindowFailure } from '../../hookInstrumentation.js' import util from 'node:util' import type { Accessibility } from '../../grpc/index.js' import PerformanceTester from '../../instrumentation/performance/performance-tester.js' @@ -151,7 +151,7 @@ export default class AccessibilityModule extends BaseModule { // error (executeHooksWithArgs resolves WITH it rather than rejecting), so the run // stays exit-0 and every reporter shows success — reporting `passed` here would make // the dashboard actively assert something false rather than merely omit it. - const failure = getHookFailure('before') + const failure = getPreTestWindowFailure() const result = failure ? { passed: false, error: { message: failure } } : { passed: true } await framework?.trackEvent(TestFrameworkState.BEFORE_ALL, HookState.POST, { ...this.preTestHookArgs(), result }) } catch (error) { diff --git a/packages/browserstack-service/src/hookInstrumentation.ts b/packages/browserstack-service/src/hookInstrumentation.ts index ea4fb09..ea3a068 100644 --- a/packages/browserstack-service/src/hookInstrumentation.ts +++ b/packages/browserstack-service/src/hookInstrumentation.ts @@ -40,6 +40,34 @@ const hookFailures: Record = {} export const getHookFailure = (hookName: string): string | undefined => hookFailures[hookName] +/** + * The config hooks that can run inside the pre-test window — driver alive, no test started yet. + * + * `beforeSuite` belongs here and is easy to miss: the Mocha adapter registers it as a root + * before-all (`this._runner.suite.beforeAll(this.wrapHook('beforeSuite'))`), so it runs after the + * config-level `before` and still ahead of the first test. Hardcoding `'before'` reported a window + * as passed when it was `beforeSuite` that threw. + * + * Mocha's own hooks are excluded deliberately: each already has its own hook run with its own + * result, so folding their failures in here would report the same failure twice. + */ +export const PRE_TEST_WINDOW_HOOKS = ['before', 'beforeSuite'] as const + +/** + * First failure recorded by any handler that ran in the pre-test window. + * + * Any handler in those arrays counts, including another service's — from the window's point of + * view the setup did fail, whoever owned the throw. The message identifies which. + */ +export const getPreTestWindowFailure = (): string | undefined => { + for (const hookName of PRE_TEST_WINDOW_HOOKS) { + if (hookFailures[hookName]) { + return `${hookName}: ${hookFailures[hookName]}` + } + } + return undefined +} + /** * Wrap one handler so its entry and exit are observable, preserving its shape exactly. * @@ -118,6 +146,12 @@ export function instrumentBrowserContextHooks(config?: Options.Testrunner): void try { const record = config as unknown as Record + // Instrumenting a config starts a fresh record. Without this the map is process-global + // for the worker's lifetime, so a failure from one instrumented config could be reported + // against a later one. + for (const hookName of BROWSER_CONTEXT_HOOKS) { + delete hookFailures[hookName] + } for (const hookName of BROWSER_CONTEXT_HOOKS) { const handlers = record[hookName] if (!Array.isArray(handlers) || handlers.length === 0) { diff --git a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts index f0bbb13..30b2fe8 100644 --- a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts +++ b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts @@ -54,7 +54,7 @@ vi.mock('../../../src/cli/index.js', () => ({ })) const { mockGetHookFailure } = vi.hoisted(() => ({ mockGetHookFailure: vi.fn() })) -vi.mock('../../../src/hookInstrumentation.js', () => ({ getHookFailure: mockGetHookFailure })) +vi.mock('../../../src/hookInstrumentation.js', () => ({ getPreTestWindowFailure: mockGetHookFailure })) vi.mock('../../../src/cli/grpcClient.js', () => ({ GrpcClient: { @@ -261,7 +261,7 @@ describe('AccessibilityModule', () => { // WDIO swallows a throwing config hook (executeHooksWithArgs resolves with the error), // so the run stays exit-0 and every reporter is green. Reporting `passed` here would // make the dashboard assert something false rather than merely omit it. - mockGetHookFailure.mockReturnValue('BOOM: config-level before hook failed') + mockGetHookFailure.mockReturnValue('before: BOOM: config-level before hook failed') await accessibilityModule['ensurePreTestHookRun']() mockTrackEvent.mockClear() diff --git a/packages/browserstack-service/tests/hookInstrumentation.test.ts b/packages/browserstack-service/tests/hookInstrumentation.test.ts new file mode 100644 index 0000000..2e59fc7 --- /dev/null +++ b/packages/browserstack-service/tests/hookInstrumentation.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it, beforeEach, vi } from 'vitest' + +vi.mock('../src/bstackLogger.js', () => ({ BStackLogger: { debug: vi.fn() } })) + +import { instrumentBrowserContextHooks, getPreTestWindowFailure, getHookFailure } from '../src/hookInstrumentation.js' + +describe('pre-test window failures', () => { + beforeEach(() => { + // the recorder is module state; clear it by re-running the hooks that write to it + instrumentBrowserContextHooks({ before: [], beforeSuite: [] } as never) + }) + + it('records a rejection from the config-level before hook', async () => { + const config = { before: [async () => { throw new Error('BOOM from before') }] } as never + + instrumentBrowserContextHooks(config) + await expect((config as { before: Array<() => Promise> }).before[0]()).rejects.toThrow('BOOM from before') + + expect(getHookFailure('before')).toBe('BOOM from before') + expect(getPreTestWindowFailure()).toBe('before: BOOM from before') + }) + + it('also catches beforeSuite, which the Mocha adapter runs inside the same window', async () => { + // registered as a root before-all by @wdio/mocha-framework, so it runs after the + // config-level before and still ahead of the first test + const config = { beforeSuite: [async () => { throw new Error('BOOM from beforeSuite') }] } as never + + instrumentBrowserContextHooks(config) + await expect((config as { beforeSuite: Array<() => Promise> }).beforeSuite[0]()).rejects.toThrow('BOOM from beforeSuite') + + expect(getPreTestWindowFailure()).toBe('beforeSuite: BOOM from beforeSuite') + }) + + it('keeps a synchronous handler synchronous, and still records its throw', () => { + const config = { before: [() => { throw new Error('sync BOOM') }] } as never + + instrumentBrowserContextHooks(config) + const handler = (config as { before: Array<() => unknown> }).before[0] + + // not a promise: an async wrapper would turn this throw into an unhandled rejection + expect(() => handler()).toThrow('sync BOOM') + expect(getPreTestWindowFailure()).toBe('before: sync BOOM') + }) + + it('reports nothing when the window ran clean', async () => { + const config = { before: [async () => 'fine'] } as never + + instrumentBrowserContextHooks(config) + await (config as { before: Array<() => Promise> }).before[0]() + + expect(getPreTestWindowFailure()).toBeUndefined() + }) +}) From 2585962ec2c48f99d77e81f41c2aa174f5ad1d2d Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 21:44:17 +0530 Subject: [PATCH 05/17] feat(hooks): name the pre-test hook run after the hook that opened it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The label was fixed at 'wdio config-level before hook', which names the wrong hook whenever the window opens somewhere else — and it can: beforeSuite runs in the same window, so a suite whose before() touches no driver gets its window opened from beforeSuite instead. The instrumentation already wraps every session-scoped hook, so it now also keeps a stack of the hooks currently executing and exposes the innermost one. The name is captured once, when the window opens, because the binary takes a hook's name from the record pushed at PRE and by POST time a different hook is running. Verified on device, two shapes: before() does the first scannable command -> 'wdio before() hook (pre-test window)' before() touches nothing, beforeSuite does -> 'wdio beforeSuite() hook (pre-test window)' (build eestbqwlk8mvjhzqkijfl0lc4gnzqxredgofblrw) Note the name says where the window OPENED, not where it failed — those differ, and the failure message carries the failing hook's name, so both facts survive. SDK-7422 --- .../src/cli/modules/accessibilityModule.ts | 15 ++++++++-- .../src/hookInstrumentation.ts | 24 +++++++++++++++ .../cli/modules/accessibilityModule.test.ts | 30 +++++++++++++++++-- .../tests/hookInstrumentation.test.ts | 29 +++++++++++++++++- 4 files changed, 93 insertions(+), 5 deletions(-) diff --git a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts index 0329f43..ef7aa7b 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -13,7 +13,7 @@ import accessibilityScripts from '../../scripts/accessibility-scripts.js' import { _getParamsForAppAccessibility, formatString, getAppA11yResults, getAppA11yResultsSummary, shouldScanTestForAccessibility, validateCapsWithA11y, validateCapsWithAppA11y, isBrowserstackSession } from '../../util.js' import { AutomationFrameworkConstants } from '../frameworks/constants/automationFrameworkConstants.js' import { BROWSERSTACK_WDIO_CONFIG_FILE_PATH } from '../../constants.js' -import { getPreTestWindowFailure } from '../../hookInstrumentation.js' +import { getActiveHookName, getPreTestWindowFailure } from '../../hookInstrumentation.js' import util from 'node:util' import type { Accessibility } from '../../grpc/index.js' import PerformanceTester from '../../instrumentation/performance/performance-tester.js' @@ -50,6 +50,10 @@ export default class AccessibilityModule extends BaseModule { // overwhelming majority of suites. 'attempted' is distinct from 'open' so a failed PRE is // neither retried on every command nor closed by a POST that would pair with nothing. private preTestHookState: 'idle' | 'attempted' | 'open' | 'closed' = 'idle' + // The config hook that was executing when the window was opened. Captured once: the binary + // takes the hook's name from the record pushed at PRE, and by POST time a different hook is + // running, so recomputing it there would describe the wrong one. + private preTestHookLabel: string | undefined constructor(accessibilityConfig: Accessibility, isNonBstackA11y: boolean) { super() @@ -127,6 +131,7 @@ export default class AccessibilityModule extends BaseModule { } // Mark before awaiting: concurrent commands must not each open their own hook run. this.preTestHookState = 'attempted' + this.preTestHookLabel = getActiveHookName() try { const framework = BrowserstackCLI.getInstance()?.getTestFramework() if (!framework) { @@ -165,10 +170,16 @@ export default class AccessibilityModule extends BaseModule { * wdio config path onto the environment by this point. */ private preTestHookArgs() { + // Named after the hook that was actually running when the window opened — `before`, + // `beforeSuite`, whatever it was — because a fixed label would name the wrong hook half + // the time, and this name is what a person reads on the dashboard. + const hook = this.preTestHookLabel return { test: { file: process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] || process.cwd(), - title: 'wdio config-level before hook (pre-test window)' + title: hook + ? `wdio ${hook}() hook (pre-test window)` + : 'wdio config-level hook (pre-test window)' } } } diff --git a/packages/browserstack-service/src/hookInstrumentation.ts b/packages/browserstack-service/src/hookInstrumentation.ts index ea3a068..8e86633 100644 --- a/packages/browserstack-service/src/hookInstrumentation.ts +++ b/packages/browserstack-service/src/hookInstrumentation.ts @@ -38,6 +38,18 @@ const INSTRUMENTED = Symbol('bstackHookInstrumented') */ const hookFailures: Record = {} +/** + * Names of the hooks currently executing, innermost last. + * + * A stack rather than a single value because WDIO fires same-named handlers concurrently + * (`Promise.all`) and nests hook kinds — so "which hook am I inside right now" is only answerable + * at the moment it is asked. Anything reporting on a hook window uses this to name it after the + * hook that was actually running, instead of a fixed label that may name the wrong one. + */ +const activeHooks: string[] = [] + +export const getActiveHookName = (): string | undefined => activeHooks[activeHooks.length - 1] + export const getHookFailure = (hookName: string): string | undefined => hookFailures[hookName] /** @@ -88,16 +100,24 @@ const instrument = (hookName: string, index: number, fn: HookFn): HookFn => { const recordFailure = (error: unknown) => { hookFailures[hookName] = (error as Error)?.message || String(error) } + const leave = () => { + const at = activeHooks.lastIndexOf(hookName) + if (at !== -1) { + activeHooks.splice(at, 1) + } + } const wrapped = function (this: unknown, ...args: unknown[]) { const startedAt = Date.now() const finish = (outcome: string) => BStackLogger.debug(`[hook-window] ${label} ${outcome} in ${Date.now() - startedAt}ms`) BStackLogger.debug(`[hook-window] ${label} started`) + activeHooks.push(hookName) let result: unknown try { result = fn.apply(this, args) } catch (error) { recordFailure(error) + leave() finish(`threw: ${(error as Error)?.message}`) throw error } @@ -105,17 +125,20 @@ const instrument = (hookName: string, index: number, fn: HookFn): HookFn => { if (result && typeof (result as Promise).then === 'function') { return (result as Promise).then( (value) => { + leave() finish('finished') return value }, (error) => { recordFailure(error) + leave() finish(`rejected: ${(error as Error)?.message}`) throw error } ) } + leave() finish('finished') return result } as HookFn @@ -152,6 +175,7 @@ export function instrumentBrowserContextHooks(config?: Options.Testrunner): void for (const hookName of BROWSER_CONTEXT_HOOKS) { delete hookFailures[hookName] } + activeHooks.length = 0 for (const hookName of BROWSER_CONTEXT_HOOKS) { const handlers = record[hookName] if (!Array.isArray(handlers) || handlers.length === 0) { diff --git a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts index 30b2fe8..97f5241 100644 --- a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts +++ b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts @@ -53,8 +53,14 @@ vi.mock('../../../src/cli/index.js', () => ({ } })) -const { mockGetHookFailure } = vi.hoisted(() => ({ mockGetHookFailure: vi.fn() })) -vi.mock('../../../src/hookInstrumentation.js', () => ({ getPreTestWindowFailure: mockGetHookFailure })) +const { mockGetHookFailure, mockGetActiveHookName } = vi.hoisted(() => ({ + mockGetHookFailure: vi.fn(), + mockGetActiveHookName: vi.fn() +})) +vi.mock('../../../src/hookInstrumentation.js', () => ({ + getPreTestWindowFailure: mockGetHookFailure, + getActiveHookName: mockGetActiveHookName +})) vi.mock('../../../src/cli/grpcClient.js', () => ({ GrpcClient: { @@ -257,6 +263,26 @@ describe('AccessibilityModule', () => { expect((opens[0][2] as { test: { title: string } }).test.title).toContain('pre-test window') }) + it('names the hook run after the hook that was executing, not a fixed label', async () => { + // beforeSuite runs in the same window as before(), so a fixed 'before' label would + // name the wrong hook whenever the first scan lands in beforeSuite. + mockGetActiveHookName.mockReturnValue('beforeSuite') + + await accessibilityModule['ensurePreTestHookRun']() + + const open = mockTrackEvent.mock.calls.find((c) => c[1] === HookState.PRE) + expect((open![2] as { test: { title: string } }).test.title).toBe('wdio beforeSuite() hook (pre-test window)') + }) + + it('falls back to a neutral name when no hook is identifiable', async () => { + mockGetActiveHookName.mockReturnValue(undefined) + + await accessibilityModule['ensurePreTestHookRun']() + + const open = mockTrackEvent.mock.calls.find((c) => c[1] === HookState.PRE) + expect((open![2] as { test: { title: string } }).test.title).toBe('wdio config-level hook (pre-test window)') + }) + it('reports the hook run as FAILED when the config-level before() threw', async () => { // WDIO swallows a throwing config hook (executeHooksWithArgs resolves with the error), // so the run stays exit-0 and every reporter is green. Reporting `passed` here would diff --git a/packages/browserstack-service/tests/hookInstrumentation.test.ts b/packages/browserstack-service/tests/hookInstrumentation.test.ts index 2e59fc7..ad814bb 100644 --- a/packages/browserstack-service/tests/hookInstrumentation.test.ts +++ b/packages/browserstack-service/tests/hookInstrumentation.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it, beforeEach, vi } from 'vitest' vi.mock('../src/bstackLogger.js', () => ({ BStackLogger: { debug: vi.fn() } })) -import { instrumentBrowserContextHooks, getPreTestWindowFailure, getHookFailure } from '../src/hookInstrumentation.js' +import { instrumentBrowserContextHooks, getPreTestWindowFailure, getHookFailure, getActiveHookName } from '../src/hookInstrumentation.js' describe('pre-test window failures', () => { beforeEach(() => { @@ -51,3 +51,30 @@ describe('pre-test window failures', () => { expect(getPreTestWindowFailure()).toBeUndefined() }) }) + +describe('active hook name', () => { + it('reports which hook is executing, and nothing once it has finished', async () => { + const seen: Array = [] + const config = { + before: [async () => { seen.push(getActiveHookName()) }], + beforeSuite: [async () => { seen.push(getActiveHookName()) }] + } as never + + instrumentBrowserContextHooks(config) + const c = config as unknown as { before: Array<() => Promise>, beforeSuite: Array<() => Promise> } + await c.before[0]() + await c.beforeSuite[0]() + + expect(seen).toEqual(['before', 'beforeSuite']) + expect(getActiveHookName()).toBeUndefined() + }) + + it('unwinds even when the hook throws', async () => { + const config = { before: [async () => { throw new Error('BOOM') }] } as never + + instrumentBrowserContextHooks(config) + await expect((config as { before: Array<() => Promise> }).before[0]()).rejects.toThrow('BOOM') + + expect(getActiveHookName()).toBeUndefined() + }) +}) From f70e05215250a1d7bb360490041ef8430e664c1f Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 22:10:09 +0530 Subject: [PATCH 06/17] style(hooks): quote the hook name in the reported title, drop the window suffix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `wdio "before" hook` / `wdio "beforeSuite" hook` instead of `wdio before() hook (pre-test window)`. Quoting matches how Mocha names its own hooks ("before all" hook: ...), so the dashboard reads consistently across both kinds, and the suffix was internal vocabulary that meant nothing to someone looking at a hook in a build. Verified on device: wdio "before" hook result=passed 3a0c7942 (build mu2hqmwrk4ygyh1yp8queidnxfdeeju7dk4ubbdx) wdio "beforeSuite" hook result=failed 1184570a (build xprccsrzaofseerwsevv5pgcec55h9jufvjzzf2s) Scans in that window are unchanged — 7 in the config-level hook on the normal bench. SDK-7422 --- .../src/cli/modules/accessibilityModule.ts | 4 ++-- .../tests/cli/modules/accessibilityModule.test.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts index ef7aa7b..af3401d 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -178,8 +178,8 @@ export default class AccessibilityModule extends BaseModule { test: { file: process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] || process.cwd(), title: hook - ? `wdio ${hook}() hook (pre-test window)` - : 'wdio config-level hook (pre-test window)' + ? `wdio "${hook}" hook` + : 'wdio config-level hook' } } } diff --git a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts index 97f5241..f26dfd7 100644 --- a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts +++ b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts @@ -260,7 +260,7 @@ describe('AccessibilityModule', () => { const opens = mockTrackEvent.mock.calls.filter((c) => c[1] === HookState.PRE) expect(opens).toHaveLength(1) expect(opens[0][0]).toBe(TestFrameworkState.BEFORE_ALL) - expect((opens[0][2] as { test: { title: string } }).test.title).toContain('pre-test window') + expect((opens[0][2] as { test: { title: string } }).test.title).toContain('hook') }) it('names the hook run after the hook that was executing, not a fixed label', async () => { @@ -271,7 +271,7 @@ describe('AccessibilityModule', () => { await accessibilityModule['ensurePreTestHookRun']() const open = mockTrackEvent.mock.calls.find((c) => c[1] === HookState.PRE) - expect((open![2] as { test: { title: string } }).test.title).toBe('wdio beforeSuite() hook (pre-test window)') + expect((open![2] as { test: { title: string } }).test.title).toBe('wdio "beforeSuite" hook') }) it('falls back to a neutral name when no hook is identifiable', async () => { @@ -280,7 +280,7 @@ describe('AccessibilityModule', () => { await accessibilityModule['ensurePreTestHookRun']() const open = mockTrackEvent.mock.calls.find((c) => c[1] === HookState.PRE) - expect((open![2] as { test: { title: string } }).test.title).toBe('wdio config-level hook (pre-test window)') + expect((open![2] as { test: { title: string } }).test.title).toBe('wdio config-level hook') }) it('reports the hook run as FAILED when the config-level before() threw', async () => { From 74db214c728b7605e89ad895dbf81df3bb0b48c1 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 22:22:22 +0530 Subject: [PATCH 07/17] test(hooks): pin debug-log coverage across the whole patched set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only four of the eight session-scoped hooks had ever been seen firing in a device run, because earlier benches did not define the rest. A bench that defines all eight confirms every one is instrumented and logs its start and finish: instrumenting before: 1 · beforeSuite: 1 · beforeHook: 1 · beforeTest: 3 afterTest: 2 · afterHook: 1 · afterSuite: 1 · after: 2 start/finish lines: before 2 · beforeSuite 2 · beforeHook 8 · beforeTest 6 afterTest 4 · afterHook 8 · afterSuite 2 · after 4 (beforeHook/afterHook fire four times each — once per Mocha hook. The handler counts above 1 are expect-webdriverio's snapshot and soft-assert services sharing the array, which the index in the label distinguishes.) The test asserts it over BROWSER_CONTEXT_HOOKS itself, so adding a name to that list without the wrapping actually reaching it fails here rather than showing up as a silence in a device log. SDK-7422 --- .../tests/hookInstrumentation.test.ts | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/packages/browserstack-service/tests/hookInstrumentation.test.ts b/packages/browserstack-service/tests/hookInstrumentation.test.ts index ad814bb..161f79b 100644 --- a/packages/browserstack-service/tests/hookInstrumentation.test.ts +++ b/packages/browserstack-service/tests/hookInstrumentation.test.ts @@ -2,7 +2,8 @@ import { describe, expect, it, beforeEach, vi } from 'vitest' vi.mock('../src/bstackLogger.js', () => ({ BStackLogger: { debug: vi.fn() } })) -import { instrumentBrowserContextHooks, getPreTestWindowFailure, getHookFailure, getActiveHookName } from '../src/hookInstrumentation.js' +import { BStackLogger } from '../src/bstackLogger.js' +import { instrumentBrowserContextHooks, getPreTestWindowFailure, getHookFailure, getActiveHookName, BROWSER_CONTEXT_HOOKS } from '../src/hookInstrumentation.js' describe('pre-test window failures', () => { beforeEach(() => { @@ -78,3 +79,26 @@ describe('active hook name', () => { expect(getActiveHookName()).toBeUndefined() }) }) + +describe('coverage of the patched set', () => { + it('logs a start and a finish for every hook in BROWSER_CONTEXT_HOOKS', async () => { + // Guards the list against drift: adding a hook name without it actually being wrapped + // would otherwise show up only as a silence in a device log. + const config = Object.fromEntries( + BROWSER_CONTEXT_HOOKS.map((name) => [name, [async () => undefined]]) + ) as unknown as never + + instrumentBrowserContextHooks(config) + vi.mocked(BStackLogger.debug).mockClear() + + for (const name of BROWSER_CONTEXT_HOOKS) { + await (config as unknown as Record Promise>>)[name][0]() + } + + const logged = vi.mocked(BStackLogger.debug).mock.calls.map((c) => String(c[0])) + for (const name of BROWSER_CONTEXT_HOOKS) { + expect(logged.some((l) => l.includes(`[hook-window] ${name}#0`) && l.includes('started'))).toBe(true) + expect(logged.some((l) => l.includes(`[hook-window] ${name}#0`) && l.includes('finished'))).toBe(true) + } + }) +}) From d9d64696a7e69d7460255694e6168b726d895c61 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2026 16:55:09 +0000 Subject: [PATCH 08/17] chore(changeset): auto-generate from PR template (minor) --- .changeset/pr-168.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/pr-168.md diff --git a/.changeset/pr-168.md b/.changeset/pr-168.md new file mode 100644 index 0000000..209f634 --- /dev/null +++ b/.changeset/pr-168.md @@ -0,0 +1,6 @@ +--- +"@wdio/browserstack-service": minor +--- + +- Accessibility scans now run for driver commands issued from your WDIO config's `before()` hook, so screens visited during setup are covered. +- Hooks in your WDIO config that touch the driver now appear on the build, including when they fail — previously a failing `before()` was reported nowhere. From 51dc3c9c2106f506ff9bf27cdba292dd44148add Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 22:33:39 +0530 Subject: [PATCH 09/17] refactor(hooks): tighten comments, drop dead export, make instrumentation unable to break a hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Comments cut to the file's density: hookInstrumentation 192 -> 147 lines with no code removed beyond `getHookFailure`, which only tests used. Comment share of the accessibilityModule diff drops from 47/106 lines to 20/79. Exception safety, which matters because this wrapper runs inside the user's hook: - every log and every bookkeeping step goes through `quietly()`, so a throwing logger or a hostile object cannot surface as the customer's hook failing - the entry point's own catch-block log is contained too — that one could escape the function - `isThenable` tolerates a throwing `.then` getter - one unwrappable handler is handed back untouched instead of costing the rest their wrapping The handler's own error is never routed through any of that: it propagates unchanged, verified by asserting identity of the thrown object. Four tests for it, two of which found real holes while being written — the unprotected catch-block log, and a wrong premise about Object.freeze (freezing a handler does not prevent wrapping it, since defineProperty runs on the new wrapper). SDK-7422 --- .../src/cli/modules/accessibilityModule.ts | 65 +++----- .../src/hookInstrumentation.ts | 156 +++++++++--------- .../tests/hookInstrumentation.test.ts | 65 +++++++- 3 files changed, 157 insertions(+), 129 deletions(-) diff --git a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts index af3401d..f8edc77 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -41,18 +41,12 @@ export default class AccessibilityModule extends BaseModule { // Any scan fired while this is set is stamped with it as thHookRunUuid so the backend // (SeleniumHub appAllyScan → hook_run_uuid) can reconcile the scan onto the wrapping test. currentHookRunUuid: string | null = null - // Lifecycle of the hook run that carries scans fired BEFORE any test exists — i.e. from - // WDIO's config-level before()/beforeSession(), which are not test-framework hooks and get no - // hook run of their own. - // - // Opened LAZILY, on the first such scan, and never otherwise: a suite with no config-level - // hook (or one that touches no driver) must not report a phantom hook to TRA, and that is the - // overwhelming majority of suites. 'attempted' is distinct from 'open' so a failed PRE is - // neither retried on every command nor closed by a POST that would pair with nothing. + // Hook run covering scans fired before any test exists (WDIO's config-level hooks, which get + // no hook run of their own). 'attempted' is distinct from 'open' so a failed PRE is neither + // retried per command nor closed by a POST that would pair with nothing. private preTestHookState: 'idle' | 'attempted' | 'open' | 'closed' = 'idle' - // The config hook that was executing when the window was opened. Captured once: the binary - // takes the hook's name from the record pushed at PRE, and by POST time a different hook is - // running, so recomputing it there would describe the wrong one. + // Captured at PRE: the binary names a hook from the record pushed there, and by POST a + // different hook is running. private preTestHookLabel: string | undefined constructor(accessibilityConfig: Accessibility, isNonBstackA11y: boolean) { @@ -118,21 +112,18 @@ export default class AccessibilityModule extends BaseModule { } /** - * Open a BEFORE_ALL hook run for the pre-test window, if one is warranted. - * - * Called from the scan path, so it fires only when there is actually a scan needing a parent. - * The framework stamps the run uuid (KEY_HOOK_ID) and this module's own onHookStart observer - * picks it up, which is what puts thHookRunUuid on the scan; the binary turns the same event - * into HookRunStarted, so App-A11y's lookup of that uuid in BTCER resolves. + * Open a BEFORE_ALL hook run for the pre-test window, on demand from the scan path — so a + * suite with no config-level hook reports nothing extra. The framework stamps the uuid, + * onHookStart picks it up onto the scan, and the binary emits HookRunStarted for it. */ private async ensurePreTestHookRun() { if (this.preTestHookState !== 'idle' || this.currentHookRunUuid !== null) { return } - // Mark before awaiting: concurrent commands must not each open their own hook run. + // marked before awaiting: concurrent commands must not each open their own hook run this.preTestHookState = 'attempted' - this.preTestHookLabel = getActiveHookName() try { + this.preTestHookLabel = getActiveHookName() const framework = BrowserstackCLI.getInstance()?.getTestFramework() if (!framework) { return @@ -152,10 +143,8 @@ export default class AccessibilityModule extends BaseModule { this.preTestHookState = 'closed' try { const framework = BrowserstackCLI.getInstance()?.getTestFramework() - // A config-level before() that threw must not be reported green. WDIO swallows the - // error (executeHooksWithArgs resolves WITH it rather than rejecting), so the run - // stays exit-0 and every reporter shows success — reporting `passed` here would make - // the dashboard actively assert something false rather than merely omit it. + // WDIO swallows a throwing config hook, so nothing else marks it failed; reporting + // `passed` here would assert something false rather than merely omit it. const failure = getPreTestWindowFailure() const result = failure ? { passed: false, error: { message: failure } } : { passed: true } await framework?.trackEvent(TestFrameworkState.BEFORE_ALL, HookState.POST, { ...this.preTestHookArgs(), result }) @@ -164,15 +153,8 @@ export default class AccessibilityModule extends BaseModule { } } - /** - * The binary runs path.relative() over this file path when building the hook record, so an - * absent path would throw there and drop the event. configCapture has already resolved the - * wdio config path onto the environment by this point. - */ + /** The binary runs path.relative() over `file`, so an absent path would drop the event. */ private preTestHookArgs() { - // Named after the hook that was actually running when the window opened — `before`, - // `beforeSuite`, whatever it was — because a fixed label would name the wrong hook half - // the time, and this name is what a person reads on the dashboard. const hook = this.preTestHookLabel return { test: { @@ -345,17 +327,10 @@ export default class AccessibilityModule extends BaseModule { }) } - // Open the scan gate for the window between driver creation and the first test. - // WDIO's own config-level hooks (`before`, `beforeSession`) run here, and they are not - // test-framework hooks, so neither onHookStart nor onBeforeTest has fired yet and the - // gate would have no entry for this session at all: every command issued from a - // config-level before() went unscanned while still counting as expected coverage. Real - // suites do substantial UI work there (app launch, login, account selection), so those - // screens are exactly the ones a customer expects covered. - // - // Unconditionally true, matching onHookStart: per-test include/exclude filters need a - // test to evaluate and there is none yet, and the following onBeforeTest recomputes the - // gate for the test proper. + // Gate for the window between driver creation and the first test: WDIO's config-level + // hooks run there, and being no test-framework hooks, nothing else registers the + // session — so their commands went unscanned while still counting as expected + // coverage. Unconditional like onHookStart; onBeforeTest recomputes it per test. const sessionId = this.currentSessionId() if (this.autoScanning && sessionId !== undefined && sessionId !== null) { this.accessibilityMap.set(sessionId, true) @@ -404,10 +379,8 @@ export default class AccessibilityModule extends BaseModule { async onBeforeTest(args: Record) { try { this.logger.debug('Accessibility before test hook. Starting accessibility scan for this test case.') - // The pre-test window is over: close its hook run (if one was opened) before clearing - // the uuid, so the POST pairs against the record the PRE pushed. onHookEnd is the only - // other thing that clears this, so without the clear a spec with no framework hooks - // would stamp every test-body scan with the pre-test hook uuid. + // Window over: close its hook run before clearing the uuid, or a spec with no + // framework hooks would stamp every test-body scan as a hook scan. await this.closePreTestHookRun() this.currentHookRunUuid = null const suiteTitle = (typeof args.suiteTitle === 'string' ? args.suiteTitle : '') || '' diff --git a/packages/browserstack-service/src/hookInstrumentation.ts b/packages/browserstack-service/src/hookInstrumentation.ts index 8e86633..f19237d 100644 --- a/packages/browserstack-service/src/hookInstrumentation.ts +++ b/packages/browserstack-service/src/hookInstrumentation.ts @@ -3,15 +3,10 @@ import type { Options } from '@wdio/types' import { BStackLogger } from './bstackLogger.js' /** - * WDIO config hooks that execute in the worker process WITH a live session, so a driver - * command issued inside them is meaningful. + * Config hooks that run in the worker with a live session. * - * Deliberately excluded: - * - `onPrepare` / `onWorkerStart` / `onWorkerEnd` / `onComplete` — launcher process, no driver. - * - `beforeSession` — runs before the session is created; there is no browser yet. - * - `afterSession` — the session is already being deleted; commands there fail. - * - `beforeCommand` / `afterCommand` — a live driver, but per-command: their window is the - * command, which is already observable, and wrapping them would log on every interaction. + * Excluded: launcher hooks (no driver), `beforeSession` (no session yet), `afterSession` (session + * being deleted), and `beforeCommand`/`afterCommand` (per-command, already observable). */ export const BROWSER_CONTEXT_HOOKS = [ 'before', @@ -24,52 +19,49 @@ export const BROWSER_CONTEXT_HOOKS = [ 'after' ] as const +/** Config hooks that can run before the first test, while a driver is alive. */ +export const PRE_TEST_WINDOW_HOOKS = ['before', 'beforeSuite'] as const + type HookFn = (...args: unknown[]) => unknown const INSTRUMENTED = Symbol('bstackHookInstrumented') /** - * Failure message per hook name, for the hooks WDIO reports to nobody. + * Run our own bookkeeping so it can never affect the hook it wraps. * - * `executeHooksWithArgs` resolves with the error instead of rejecting — deliberately, so a - * throwing config hook cannot fail the run — which means a failed `before()` leaves the exit code - * at 0, the reporters green and the dashboard reading "passed". Anything that reports on that - * window needs a way to know it actually blew up, and this wrapper is the only thing that sees it. + * This code executes inside the user's hook. A throw from anything here — a logger whose stream + * has gone, a Proxy that dislikes String() — would surface as their hook failing, in a hook WDIO + * reports to nobody. The handler's own error is never routed through this. */ -const hookFailures: Record = {} +const quietly = (fn: () => void): void => { + try { + fn() + } catch { + // instrumentation is not worth a failed hook + } +} -/** - * Names of the hooks currently executing, innermost last. - * - * A stack rather than a single value because WDIO fires same-named handlers concurrently - * (`Promise.all`) and nests hook kinds — so "which hook am I inside right now" is only answerable - * at the moment it is asked. Anything reporting on a hook window uses this to name it after the - * hook that was actually running, instead of a fixed label that may name the wrong one. - */ +/** `.then` may be a throwing getter on a hostile object, and we only need to know if it is callable. */ +const isThenable = (value: unknown): boolean => { + try { + return Boolean(value) && typeof (value as Promise).then === 'function' + } catch { + return false + } +} + +const hookFailures: Record = {} +/** Innermost last: WDIO fires same-named handlers concurrently and nests hook kinds. */ const activeHooks: string[] = [] export const getActiveHookName = (): string | undefined => activeHooks[activeHooks.length - 1] -export const getHookFailure = (hookName: string): string | undefined => hookFailures[hookName] - /** - * The config hooks that can run inside the pre-test window — driver alive, no test started yet. + * First failure from a hook that ran before the first test, prefixed with its hook name. * - * `beforeSuite` belongs here and is easy to miss: the Mocha adapter registers it as a root - * before-all (`this._runner.suite.beforeAll(this.wrapHook('beforeSuite'))`), so it runs after the - * config-level `before` and still ahead of the first test. Hardcoding `'before'` reported a window - * as passed when it was `beforeSuite` that threw. - * - * Mocha's own hooks are excluded deliberately: each already has its own hook run with its own - * result, so folding their failures in here would report the same failure twice. - */ -export const PRE_TEST_WINDOW_HOOKS = ['before', 'beforeSuite'] as const - -/** - * First failure recorded by any handler that ran in the pre-test window. - * - * Any handler in those arrays counts, including another service's — from the window's point of - * view the setup did fail, whoever owned the throw. The message identifies which. + * `executeHooksWithArgs` resolves WITH a hook's error rather than rejecting, so a throwing config + * hook leaves the exit code at 0 and every reporter green. This wrapper is the only thing that + * sees it. Any handler counts, including another service's — the window failed either way. */ export const getPreTestWindowFailure = (): string | undefined => { for (const hookName of PRE_TEST_WINDOW_HOOKS) { @@ -81,21 +73,16 @@ export const getPreTestWindowFailure = (): string | undefined => { } /** - * Wrap one handler so its entry and exit are observable, preserving its shape exactly. - * - * Sync handlers stay sync: an `async` wrapper would turn every sync hook into a promise and a - * synchronous throw into a rejection, which changes ordering for anything that calls a hook - * without awaiting it. So the wrapper only attaches to the promise when the handler actually - * returned one. + * Sync handlers stay sync — an `async` wrapper would turn a synchronous throw into a rejection and + * defer every hook body by a microtask, which is observable to anything not awaiting the hook. */ const instrument = (hookName: string, index: number, fn: HookFn): HookFn => { if ((fn as unknown as Record)[INSTRUMENTED]) { return fn } - // `fn.name` is the only identity WDIO leaves: ConfigParser binds every handler it folds in - // (`hook.bind(service)`), so both the user's config hooks and other services' hooks read as - // `bound `. Logged anyway — it separates a named user function from an anonymous one. + // ConfigParser binds every handler it folds in, so `fn.name` reads as `bound ` for the + // user's hooks and other services' alike; the index is what identifies one across its pair. const label = `${hookName}#${index}${fn.name ? ` (${fn.name})` : ''}` const recordFailure = (error: unknown) => { hookFailures[hookName] = (error as Error)?.message || String(error) @@ -106,39 +93,48 @@ const instrument = (hookName: string, index: number, fn: HookFn): HookFn => { activeHooks.splice(at, 1) } } + const wrapped = function (this: unknown, ...args: unknown[]) { const startedAt = Date.now() - const finish = (outcome: string) => BStackLogger.debug(`[hook-window] ${label} ${outcome} in ${Date.now() - startedAt}ms`) + const finish = (outcome: string) => quietly(() => + BStackLogger.debug(`[hook-window] ${label} ${outcome} in ${Date.now() - startedAt}ms`)) + + quietly(() => { + BStackLogger.debug(`[hook-window] ${label} started`) + activeHooks.push(hookName) + }) - BStackLogger.debug(`[hook-window] ${label} started`) - activeHooks.push(hookName) let result: unknown try { result = fn.apply(this, args) } catch (error) { - recordFailure(error) - leave() + quietly(() => { + recordFailure(error) + leave() + }) finish(`threw: ${(error as Error)?.message}`) throw error } - if (result && typeof (result as Promise).then === 'function') { + if (isThenable(result)) { return (result as Promise).then( (value) => { - leave() + quietly(leave) finish('finished') return value }, (error) => { - recordFailure(error) - leave() + quietly(() => { + recordFailure(error) + leave() + }) finish(`rejected: ${(error as Error)?.message}`) throw error } ) } - leave() + quietly(leave) finish('finished') return result } as HookFn @@ -150,17 +146,11 @@ const instrument = (hookName: string, index: number, fn: HookFn): HookFn => { /** * Make the boundaries of every session-scoped config hook observable. * - * WDIO tells a service nothing about the other handlers registered for a hook: `ConfigParser` - * folds the user's config hooks and every service's hooks into one array per hook name, and the - * runner fires them together (`executeHooksWithArgs` → `Promise.all`). Patching the array in - * place is therefore the only way to see when a handler that is not ours starts and finishes — - * which is what bounds the windows a driver command can fall into. - * - * The index in the label is the handler's position in that merged array, so it includes this - * service's own handlers; it identifies a handler across its start/finish pair, nothing more. + * WDIO tells a service nothing about the other handlers on a hook — ConfigParser folds the user's + * and every service's into one array per hook name, fired together via `Promise.all` — so patching + * the array in place is the only way to see a handler that is not ours start and finish. * - * Logging only, for now: no events are emitted and no behaviour changes. Idempotent, and any - * failure is swallowed — instrumentation must never be the reason a suite cannot start. + * Logging only. Idempotent, and never throws: instrumentation must not stop a suite starting. */ export function instrumentBrowserContextHooks(config?: Options.Testrunner): void { if (!config) { @@ -169,24 +159,30 @@ export function instrumentBrowserContextHooks(config?: Options.Testrunner): void try { const record = config as unknown as Record - // Instrumenting a config starts a fresh record. Without this the map is process-global - // for the worker's lifetime, so a failure from one instrumented config could be reported - // against a later one. - for (const hookName of BROWSER_CONTEXT_HOOKS) { - delete hookFailures[hookName] - } activeHooks.length = 0 for (const hookName of BROWSER_CONTEXT_HOOKS) { + // fresh record per config: the maps live as long as the worker + delete hookFailures[hookName] + const handlers = record[hookName] if (!Array.isArray(handlers) || handlers.length === 0) { continue } - BStackLogger.debug(`[hook-window] instrumenting ${hookName}: ${handlers.length} handler(s) registered at service construction`) - record[hookName] = handlers.map((handler, index) => - typeof handler === 'function' ? instrument(hookName, index, handler as HookFn) : handler - ) + quietly(() => BStackLogger.debug(`[hook-window] instrumenting ${hookName}: ${handlers.length} handler(s)`)) + record[hookName] = handlers.map((handler, index) => { + if (typeof handler !== 'function') { + return handler + } + try { + return instrument(hookName, index, handler as HookFn) + } catch { + // hand back the original rather than losing the hook, or the ones after it + return handler + } + }) } } catch (error) { - BStackLogger.debug(`Could not instrument config hooks: ${error}`) + // including this one: a throwing logger here would escape the function entirely + quietly(() => BStackLogger.debug(`Could not instrument config hooks: ${error}`)) } } diff --git a/packages/browserstack-service/tests/hookInstrumentation.test.ts b/packages/browserstack-service/tests/hookInstrumentation.test.ts index 161f79b..a0a4afb 100644 --- a/packages/browserstack-service/tests/hookInstrumentation.test.ts +++ b/packages/browserstack-service/tests/hookInstrumentation.test.ts @@ -1,9 +1,9 @@ -import { describe, expect, it, beforeEach, vi } from 'vitest' +import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest' vi.mock('../src/bstackLogger.js', () => ({ BStackLogger: { debug: vi.fn() } })) import { BStackLogger } from '../src/bstackLogger.js' -import { instrumentBrowserContextHooks, getPreTestWindowFailure, getHookFailure, getActiveHookName, BROWSER_CONTEXT_HOOKS } from '../src/hookInstrumentation.js' +import { instrumentBrowserContextHooks, getPreTestWindowFailure, getActiveHookName, BROWSER_CONTEXT_HOOKS } from '../src/hookInstrumentation.js' describe('pre-test window failures', () => { beforeEach(() => { @@ -17,7 +17,6 @@ describe('pre-test window failures', () => { instrumentBrowserContextHooks(config) await expect((config as { before: Array<() => Promise> }).before[0]()).rejects.toThrow('BOOM from before') - expect(getHookFailure('before')).toBe('BOOM from before') expect(getPreTestWindowFailure()).toBe('before: BOOM from before') }) @@ -102,3 +101,63 @@ describe('coverage of the patched set', () => { } }) }) + +describe('instrumentation cannot break the hook it wraps', () => { + afterEach(() => { + vi.mocked(BStackLogger.debug).mockReset() + }) + + it('runs the handler even when our own logging throws', async () => { + vi.mocked(BStackLogger.debug).mockImplementation(() => { throw new Error('logger is gone') }) + let ran = false + const config = { before: [async () => { ran = true; return 'value' }] } as never + + instrumentBrowserContextHooks(config) + const handler = (config as unknown as { before: Array<() => Promise> }).before[0] + + await expect(handler()).resolves.toBe('value') + expect(ran).toBe(true) + }) + + it('still propagates the handler\'s own error, unchanged', async () => { + const boom = new Error('user hook failed') + const config = { before: [async () => { throw boom }] } as never + + instrumentBrowserContextHooks(config) + const handler = (config as unknown as { before: Array<() => Promise> }).before[0] + + await expect(handler()).rejects.toBe(boom) + }) + + it('does not choke on a return value with a hostile then getter', () => { + const hostile = { get then() { throw new Error('no touching') } } + const config = { before: [() => hostile] } as never + + instrumentBrowserContextHooks(config) + const handler = (config as unknown as { before: Array<() => unknown> }).before[0] + + expect(handler()).toBe(hostile) + }) + + it('keeps instrumenting the remaining handlers when one cannot be wrapped', () => { + // reading `.name` is enough to break wrapping for this one + const hostile = new Proxy(() => 'hostile', { + get(target, key) { + if (key === 'name') { + throw new Error('no name for you') + } + return Reflect.get(target, key) + } + }) + const normal = () => 'normal' + const config = { before: [hostile, normal] } as never + + instrumentBrowserContextHooks(config) + const handlers = (config as unknown as { before: Array<() => string> }).before + + expect(handlers[0]).toBe(hostile) // handed back untouched + expect(handlers[1]).not.toBe(normal) // the one after it still got wrapped + expect(handlers[0]()).toBe('hostile') + expect(handlers[1]()).toBe('normal') + }) +}) From c27900bd7990b8f027451b20b07c2df5cd86b193 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 22:46:10 +0530 Subject: [PATCH 10/17] feat(app-a11y): cover the non-CLI flow and cucumber's hook lifecycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate fix and the instrumentation were both mocha/CLI-shaped. Two gaps closed. Non-CLI flow: the classic AccessibilityHandler had the same defect the CLI module did — nothing registered the session before the first test, so commands from a config-level hook went unscanned while still counting as expected coverage. `before()` now opens the gate when autoScanning is on; beforeTest/beforeScenario/beforeHook each recompute it. This is the only path cucumber and jasmine ever take (CLISupportedFrameworks is ['mocha']), and the path mocha takes under multiremote. Cucumber lifecycle: beforeFeature/beforeScenario/beforeStep/afterStep/afterScenario/afterFeature were not instrumented at all, so a cucumber suite's hook windows were invisible. Added, and beforeFeature joins PRE_TEST_WINDOW_HOOKS since it precedes the first scenario. Measured, cucumber on device (Direct flow by construction): prod 9.35.0 config-level before() window: 0 scans (kfribnppwbyt98ob8xc9lftkjvbm3xeroffaqjy1) this branch config-level before() window: 1 scan (4ouu5t3ci52ouoo3pmxcczehbzxatuln4fqhxv08) all 7 cucumber hooks + before/after logged start/finish a throwing before(): exit 0, "1 passed", and the only record is ours — [hook-window] before#0 rejected: BOOM: cucumber config-level before failed in 3050ms (tupaf5ev3rr3qclpvq2sx9628hs7ot6lvzev2phd) Remaining asymmetry, deliberately not papered over: the hook RUN is still reported only on the CLI path. Direct flow reports hooks through insights-handler, whose hook state is shared with the real hooks (setCurrentHook drives log parenting), so injecting a synthetic hook there needs its own design and testing rather than a rider on this change. SDK-7422 --- .../src/accessibility-handler.ts | 9 +++++ .../src/hookInstrumentation.ts | 19 +++++++++-- .../tests/accessibility-handler.test.ts | 34 +++++++++++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index 69c3350..a6dbb98 100644 --- a/packages/browserstack-service/src/accessibility-handler.ts +++ b/packages/browserstack-service/src/accessibility-handler.ts @@ -282,6 +282,15 @@ class _AccessibilityHandler { if (!this._accessibility) { return } + + // Gate for the window between driver creation and the first test/scenario. WDIO's + // config-level hooks run there and are no test-framework hooks, so nothing else registers + // the session and their commands went unscanned. beforeTest/beforeScenario/beforeHook each + // recompute it. Same fix as the CLI flow's onBeforeExecute, for mocha-direct and cucumber. + if (this._autoScanning) { + AccessibilityHandler._a11yScanSessionMap[sessionId] = true + } + if (!('overwriteCommand' in this._browser && Array.isArray(accessibilityScripts.commandsToWrap))) { return } diff --git a/packages/browserstack-service/src/hookInstrumentation.ts b/packages/browserstack-service/src/hookInstrumentation.ts index f19237d..12f09dc 100644 --- a/packages/browserstack-service/src/hookInstrumentation.ts +++ b/packages/browserstack-service/src/hookInstrumentation.ts @@ -16,11 +16,24 @@ export const BROWSER_CONTEXT_HOOKS = [ 'afterTest', 'afterHook', 'afterSuite', - 'after' + 'after', + // cucumber's own lifecycle: same driver, different names + 'beforeFeature', + 'beforeScenario', + 'beforeStep', + 'afterStep', + 'afterScenario', + 'afterFeature' ] as const -/** Config hooks that can run before the first test, while a driver is alive. */ -export const PRE_TEST_WINDOW_HOOKS = ['before', 'beforeSuite'] as const +/** + * Config hooks that can run before the first test/scenario, while a driver is alive. + * + * `beforeSuite` (mocha) and `beforeFeature` (cucumber) both land inside that window: the Mocha + * adapter registers beforeSuite as a root before-all, and beforeFeature precedes the first + * scenario. + */ +export const PRE_TEST_WINDOW_HOOKS = ['before', 'beforeSuite', 'beforeFeature'] as const type HookFn = (...args: unknown[]) => unknown diff --git a/packages/browserstack-service/tests/accessibility-handler.test.ts b/packages/browserstack-service/tests/accessibility-handler.test.ts index 1d75f54..be42005 100644 --- a/packages/browserstack-service/tests/accessibility-handler.test.ts +++ b/packages/browserstack-service/tests/accessibility-handler.test.ts @@ -689,6 +689,40 @@ describe('afterTest', () => { }) }) +describe('pre-test scan gate (non-CLI flow)', () => { + beforeEach(() => { + accessibilityHandler = new AccessibilityHandler(browser, caps, options, false, config, 'mocha', true, false, accessibilityOpts) + vi.spyOn(utils, 'isBrowserstackSession').mockReturnValue(true) + vi.spyOn(utils, 'isAccessibilityAutomationSession').mockReturnValue(true) + vi.spyOn(utils, 'validateCapsWithA11y').mockReturnValue(true) + }) + + it('opens the gate in before(), so config-level hook commands scan', async () => { + await accessibilityHandler.before('session-direct') + + expect(AccessibilityHandler['_a11yScanSessionMap']['session-direct']).toBe(true) + }) + + it('leaves it closed when autoScanning is off', async () => { + const handler = new AccessibilityHandler(browser, caps, options, false, config, 'mocha', true, false, + { ...accessibilityOpts, autoScanning: false } as never) + + await handler.before('session-noauto') + + expect(AccessibilityHandler['_a11yScanSessionMap']['session-noauto']).toBeUndefined() + }) + + it('is recomputed per test, so a filtered test still closes it', async () => { + await accessibilityHandler.before('session-direct-2') + expect(AccessibilityHandler['_a11yScanSessionMap']['session-direct-2']).toBe(true) + + vi.spyOn(utils, 'shouldScanTestForAccessibility').mockReturnValue(false) + await accessibilityHandler.beforeTest('suite', { title: 'excluded', parent: 'suite' } as never) + + expect(AccessibilityHandler['_a11yScanSessionMap']['session-direct-2']).toBe(false) + }) +}) + describe('onSessionReload', () => { beforeEach(() => { accessibilityHandler = new AccessibilityHandler(browser, caps, options, false, config, 'framework', true, false, accessibilityOpts) From 56c816096d67ece3dc420b89975b6c5efb02334b Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 22:57:22 +0530 Subject: [PATCH 11/17] refactor: move the hook-window constants into constants.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the repo convention that constants live in one place: BROWSER_CONTEXT_HOOKS, PRE_TEST_WINDOW_HOOKS, the [hook-window] log tag and the reported hook title now come from constants.ts, with hookInstrumentation and accessibilityModule importing them. That also removes the last string literals duplicated between the two — the 'wdio' title prefix appeared in both a template and its fallback. Tests import BROWSER_CONTEXT_HOOKS from constants too, so the list has a single definition. Re-verified on device after the move, both flows unchanged: mocha CLI config-level before() window: 7 scans (7wtmrijn2s3fchavf5bs6p0uprfd0uxjrboenkeg) cucumber non-CLI config-level before() window: 1 scan (7tzkeeepk1x80zwtczn8gqu67kv2vcpqbvqvmbec) SDK-7422 --- .../src/cli/modules/accessibilityModule.ts | 6 +-- .../browserstack-service/src/constants.ts | 31 ++++++++++++++ .../src/hookInstrumentation.ts | 40 ++----------------- .../tests/hookInstrumentation.test.ts | 3 +- 4 files changed, 40 insertions(+), 40 deletions(-) diff --git a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts index f8edc77..817b309 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -12,7 +12,7 @@ import type { Command } from '../../scripts/accessibility-scripts.js' import accessibilityScripts from '../../scripts/accessibility-scripts.js' import { _getParamsForAppAccessibility, formatString, getAppA11yResults, getAppA11yResultsSummary, shouldScanTestForAccessibility, validateCapsWithA11y, validateCapsWithAppA11y, isBrowserstackSession } from '../../util.js' import { AutomationFrameworkConstants } from '../frameworks/constants/automationFrameworkConstants.js' -import { BROWSERSTACK_WDIO_CONFIG_FILE_PATH } from '../../constants.js' +import { BROWSERSTACK_WDIO_CONFIG_FILE_PATH, PRE_TEST_HOOK_TITLE_FALLBACK, PRE_TEST_HOOK_TITLE_PREFIX } from '../../constants.js' import { getActiveHookName, getPreTestWindowFailure } from '../../hookInstrumentation.js' import util from 'node:util' import type { Accessibility } from '../../grpc/index.js' @@ -160,8 +160,8 @@ export default class AccessibilityModule extends BaseModule { test: { file: process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] || process.cwd(), title: hook - ? `wdio "${hook}" hook` - : 'wdio config-level hook' + ? `${PRE_TEST_HOOK_TITLE_PREFIX} "${hook}" hook` + : PRE_TEST_HOOK_TITLE_FALLBACK } } } diff --git a/packages/browserstack-service/src/constants.ts b/packages/browserstack-service/src/constants.ts index c64bfa8..78bb4c8 100644 --- a/packages/browserstack-service/src/constants.ts +++ b/packages/browserstack-service/src/constants.ts @@ -86,6 +86,37 @@ export const CAPTURE_CONFIG_IMPORT_DEPTH = 1 /* How far to walk up from the config dir looking for the project's package.json */ export const MAX_PACKAGE_JSON_WALK_UP = 5 +/* WDIO config hooks that run in the worker with a live session, so a driver command inside them + is meaningful. Excludes launcher hooks (no driver), beforeSession (no session yet), + afterSession (session being deleted) and beforeCommand/afterCommand (per-command). */ +export const BROWSER_CONTEXT_HOOKS = [ + 'before', + 'beforeSuite', + 'beforeHook', + 'beforeTest', + 'afterTest', + 'afterHook', + 'afterSuite', + 'after', + 'beforeFeature', + 'beforeScenario', + 'beforeStep', + 'afterStep', + 'afterScenario', + 'afterFeature' +] as const + +/* Of those, the ones that can run before the first test/scenario: the Mocha adapter registers + beforeSuite as a root before-all, and beforeFeature precedes the first scenario. */ +export const PRE_TEST_WINDOW_HOOKS = ['before', 'beforeSuite', 'beforeFeature'] as const + +/* Tag on hook-window diagnostics, so they can be grepped out of an uploaded log */ +export const HOOK_WINDOW_LOG_PREFIX = '[hook-window]' + +/* Title reported for the hook run covering the pre-test window */ +export const PRE_TEST_HOOK_TITLE_PREFIX = 'wdio' +export const PRE_TEST_HOOK_TITLE_FALLBACK = `${PRE_TEST_HOOK_TITLE_PREFIX} config-level hook` + /** * Keys whose line is scrubbed before a config file enters the archive. * `user` / `key` are WDIO's own top-level credential options, hence the bare entries. diff --git a/packages/browserstack-service/src/hookInstrumentation.ts b/packages/browserstack-service/src/hookInstrumentation.ts index 12f09dc..f219f3a 100644 --- a/packages/browserstack-service/src/hookInstrumentation.ts +++ b/packages/browserstack-service/src/hookInstrumentation.ts @@ -1,39 +1,7 @@ import type { Options } from '@wdio/types' import { BStackLogger } from './bstackLogger.js' - -/** - * Config hooks that run in the worker with a live session. - * - * Excluded: launcher hooks (no driver), `beforeSession` (no session yet), `afterSession` (session - * being deleted), and `beforeCommand`/`afterCommand` (per-command, already observable). - */ -export const BROWSER_CONTEXT_HOOKS = [ - 'before', - 'beforeSuite', - 'beforeHook', - 'beforeTest', - 'afterTest', - 'afterHook', - 'afterSuite', - 'after', - // cucumber's own lifecycle: same driver, different names - 'beforeFeature', - 'beforeScenario', - 'beforeStep', - 'afterStep', - 'afterScenario', - 'afterFeature' -] as const - -/** - * Config hooks that can run before the first test/scenario, while a driver is alive. - * - * `beforeSuite` (mocha) and `beforeFeature` (cucumber) both land inside that window: the Mocha - * adapter registers beforeSuite as a root before-all, and beforeFeature precedes the first - * scenario. - */ -export const PRE_TEST_WINDOW_HOOKS = ['before', 'beforeSuite', 'beforeFeature'] as const +import { BROWSER_CONTEXT_HOOKS, HOOK_WINDOW_LOG_PREFIX, PRE_TEST_WINDOW_HOOKS } from './constants.js' type HookFn = (...args: unknown[]) => unknown @@ -110,10 +78,10 @@ const instrument = (hookName: string, index: number, fn: HookFn): HookFn => { const wrapped = function (this: unknown, ...args: unknown[]) { const startedAt = Date.now() const finish = (outcome: string) => quietly(() => - BStackLogger.debug(`[hook-window] ${label} ${outcome} in ${Date.now() - startedAt}ms`)) + BStackLogger.debug(`${HOOK_WINDOW_LOG_PREFIX} ${label} ${outcome} in ${Date.now() - startedAt}ms`)) quietly(() => { - BStackLogger.debug(`[hook-window] ${label} started`) + BStackLogger.debug(`${HOOK_WINDOW_LOG_PREFIX} ${label} started`) activeHooks.push(hookName) }) @@ -181,7 +149,7 @@ export function instrumentBrowserContextHooks(config?: Options.Testrunner): void if (!Array.isArray(handlers) || handlers.length === 0) { continue } - quietly(() => BStackLogger.debug(`[hook-window] instrumenting ${hookName}: ${handlers.length} handler(s)`)) + quietly(() => BStackLogger.debug(`${HOOK_WINDOW_LOG_PREFIX} instrumenting ${hookName}: ${handlers.length} handler(s)`)) record[hookName] = handlers.map((handler, index) => { if (typeof handler !== 'function') { return handler diff --git a/packages/browserstack-service/tests/hookInstrumentation.test.ts b/packages/browserstack-service/tests/hookInstrumentation.test.ts index a0a4afb..a659e90 100644 --- a/packages/browserstack-service/tests/hookInstrumentation.test.ts +++ b/packages/browserstack-service/tests/hookInstrumentation.test.ts @@ -3,7 +3,8 @@ import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest' vi.mock('../src/bstackLogger.js', () => ({ BStackLogger: { debug: vi.fn() } })) import { BStackLogger } from '../src/bstackLogger.js' -import { instrumentBrowserContextHooks, getPreTestWindowFailure, getActiveHookName, BROWSER_CONTEXT_HOOKS } from '../src/hookInstrumentation.js' +import { instrumentBrowserContextHooks, getPreTestWindowFailure, getActiveHookName } from '../src/hookInstrumentation.js' +import { BROWSER_CONTEXT_HOOKS } from '../src/constants.js' describe('pre-test window failures', () => { beforeEach(() => { From 0fafe7979fa82503df221b22d7d85b6b806265e5 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 23:57:00 +0530 Subject: [PATCH 12/17] feat(app-a11y): report the pre-test hook run for cucumber too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the CLI/non-CLI asymmetry for the framework that always takes the non-CLI path. Jasmine and mocha-multiremote are deliberately left out — neither reports hooks on a path this can reuse, so they would need their own design. Direct flow reports cucumber hooks through insights-handler, so that is where the pre-test window now goes: reportPreTestHookStarted/Finished build the same TestMeta shape the real cucumber hooks use and go out via listener.hookStarted/hookFinished. Two small things made it fit — getHookRunDataForCucumber now prefers a stored `name` over deriving one from hookType, so a synthesised hook can carry its own label, and the run is stashed under a fixed key so its finish can find its start. The accessibility handler drives it, on demand from the scan path and closed at the first test/scenario, exactly as the CLI module does. It takes the reporter by injection rather than reaching for insights-handler itself, so it stays unaware of which framework it is serving and the service decides who gets one. Measured on device, cucumber: happy [BEFORE_ALL] wdio "before" hook status=passed build zzppjaz9vfv1tsexw1nkdbuguqyqhfxsyzopgyrm failing [BEFORE_ALL] wdio "before" hook status=failed build czrlfmfadwt32xjgdtsmzuup46wmutqqftujmje7 build verdict follows: passed {passed: 2} vs failed {passed: 1, failed: 1} the window's scan still lands (1x back), and the scenario itself still reports passed — WDIO swallows the hook error either way, which is why the hook row is the only place it shows SDK-7422 --- .../src/accessibility-handler.ts | 53 ++++++++++++++++++ .../browserstack-service/src/constants.ts | 3 + .../src/insights-handler.ts | 55 ++++++++++++++++++- packages/browserstack-service/src/service.ts | 13 +++++ .../tests/accessibility-handler.test.ts | 43 +++++++++++++++ 5 files changed, 165 insertions(+), 2 deletions(-) diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index a6dbb98..d510721 100644 --- a/packages/browserstack-service/src/accessibility-handler.ts +++ b/packages/browserstack-service/src/accessibility-handler.ts @@ -75,7 +75,9 @@ import accessibilityScripts from './scripts/accessibility-scripts.js' import PerformanceTester from './instrumentation/performance/performance-tester.js' import * as PERFORMANCE_SDK_EVENTS from './instrumentation/performance/constants.js' +import { PRE_TEST_HOOK_TITLE_FALLBACK, PRE_TEST_HOOK_TITLE_PREFIX } from './constants.js' import { BStackLogger } from './bstackLogger.js' +import { getActiveHookName, getPreTestWindowFailure } from './hookInstrumentation.js' class _AccessibilityHandler { /** @@ -93,6 +95,15 @@ class _AccessibilityHandler { private _config: Options.Testrunner private _accessibilityOptions?: AccessibilityOptions private _autoScanning: boolean = true + // Reports the pre-test window as a hook run. Injected by the service, and only for the + // frameworks whose Direct-flow hook reporting supports it, so this handler stays unaware of + // which one it is serving. + private _preTestHookReporter?: { + open: (name: string) => Promise, + close: (uuid: string, passed: boolean, message?: string) => Promise + } + private _preTestHookState: 'idle' | 'attempted' | 'open' | 'closed' = 'idle' + private _preTestHookUuid?: string private _testIdentifier: string | null = null private _testMetadata: TestMetadata = {} /* Set while a supported hook is executing; scans fired in this window are stamped with it. */ @@ -188,6 +199,45 @@ class _AccessibilityHandler { } } + setPreTestHookReporter(reporter: { + open: (name: string) => Promise, + close: (uuid: string, passed: boolean, message?: string) => Promise + }) { + this._preTestHookReporter = reporter + } + + /** On demand from the scan path, so a suite with no config-level hook reports nothing extra. */ + private async ensurePreTestHookRun() { + if (this._preTestHookState !== 'idle' || !this._preTestHookReporter) { + return + } + this._preTestHookState = 'attempted' + try { + const hook = getActiveHookName() + const name = hook ? `${PRE_TEST_HOOK_TITLE_PREFIX} "${hook}" hook` : PRE_TEST_HOOK_TITLE_FALLBACK + this._preTestHookUuid = await this._preTestHookReporter.open(name) + if (this._preTestHookUuid) { + this._preTestHookState = 'open' + } + } catch (error) { + BStackLogger.debug(`Could not open a hook run for the pre-test window: ${error}`) + } + } + + private async closePreTestHookRun() { + if (this._preTestHookState !== 'open' || !this._preTestHookUuid || !this._preTestHookReporter) { + this._preTestHookState = 'closed' + return + } + this._preTestHookState = 'closed' + try { + const failure = getPreTestWindowFailure() + await this._preTestHookReporter.close(this._preTestHookUuid, !failure, failure) + } catch (error) { + BStackLogger.debug(`Could not close the hook run for the pre-test window: ${error}`) + } + } + async before(sessionId: string) { PerformanceTester.start(PERFORMANCE_SDK_EVENTS.CONFIG_EVENTS.ACCESSIBILITY) @@ -314,6 +364,7 @@ class _AccessibilityHandler { } async beforeTest (suiteTitle: string | undefined, test: Frameworks.Test) { + await this.closePreTestHookRun() try { if ( !AccessibilityHandler.TEST_HOOK_FRAMEWORKS.includes(this._framework as string) || @@ -394,6 +445,7 @@ class _AccessibilityHandler { * Cucumber Only */ async beforeScenario (world: ITestCaseHookParameter) { + await this.closePreTestHookRun() const pickleData = world.pickle const gherkinDocument = world.gherkinDocument const featureData = gherkinDocument.feature @@ -513,6 +565,7 @@ class _AccessibilityHandler { private async commandWrapper (command: CommandInfo, prevImpl: Function, origFunction: Function, ...args: unknown[]) { const skipScanForBidiWindowCommand = AccessibilityHandler.shouldSkipScanForBidiWindowCommand(this._browser, command) + await this.ensurePreTestHookRun() if ( this._sessionId && AccessibilityHandler._a11yScanSessionMap[this._sessionId] && !skipScanForBidiWindowCommand && diff --git a/packages/browserstack-service/src/constants.ts b/packages/browserstack-service/src/constants.ts index 78bb4c8..fe6d9ce 100644 --- a/packages/browserstack-service/src/constants.ts +++ b/packages/browserstack-service/src/constants.ts @@ -113,6 +113,9 @@ export const PRE_TEST_WINDOW_HOOKS = ['before', 'beforeSuite', 'beforeFeature'] /* Tag on hook-window diagnostics, so they can be grepped out of an uploaded log */ export const HOOK_WINDOW_LOG_PREFIX = '[hook-window]' +/* Key the pre-test hook run is stashed under, so its finish can find its start */ +export const PRE_TEST_HOOK_ID = 'bstack:pre-test-hook' + /* Title reported for the hook run covering the pre-test window */ export const PRE_TEST_HOOK_TITLE_PREFIX = 'wdio' export const PRE_TEST_HOOK_TITLE_FALLBACK = `${PRE_TEST_HOOK_TITLE_PREFIX} config-level hook` diff --git a/packages/browserstack-service/src/insights-handler.ts b/packages/browserstack-service/src/insights-handler.ts index e64a312..69d1a97 100644 --- a/packages/browserstack-service/src/insights-handler.ts +++ b/packages/browserstack-service/src/insights-handler.ts @@ -41,7 +41,7 @@ import type { import { BStackLogger } from './bstackLogger.js' import type { Capabilities } from '@wdio/types' import Listener from './testOps/listener.js' -import { TESTOPS_SCREENSHOT_ENV } from './constants.js' +import { TESTOPS_SCREENSHOT_ENV, PRE_TEST_HOOK_ID } from './constants.js' import { BrowserstackCLI } from './cli/index.js' import { TestFrameworkState } from './cli/states/testFrameworkState.js' import { HookState } from './cli/states/hookState.js' @@ -409,13 +409,64 @@ class _InsightsHandler { } } + /** + * Report a hook run for the pre-test window — the stretch between the driver existing and the + * first scenario, where WDIO's config-level hooks run and emit no framework event of their own. + * + * Cucumber only: it is the framework whose Direct-flow hook reporting this mirrors, and the + * one that always takes that flow. Returns the uuid to close with, or undefined if nothing + * was reported. + */ + async reportPreTestHookStarted(name: string): Promise { + try { + if (this._framework !== 'cucumber') { + return undefined + } + const hookMetaData: TestMeta = { + uuid: uuidv4(), + startedAt: (new Date()).toISOString(), + // absent before the first scenario; the hook uuid is what the scan is joined on + testRunId: InsightsHandler.currentTest?.uuid, + hookType: 'BEFORE_ALL', + kind: 'hook', + name, + scopes: [this._cucumberData.feature?.name || ''], + fileName: this._cucumberData.uri + } + this._tests[PRE_TEST_HOOK_ID] = hookMetaData + this.setCurrentHook({ uuid: hookMetaData.uuid }) + this.listener.hookStarted(this.getHookRunDataForCucumber(hookMetaData, 'HookRunStarted')) + return hookMetaData.uuid + } catch (error) { + BStackLogger.debug(`Could not report the pre-test hook run: ${error}`) + return undefined + } + } + + async reportPreTestHookFinished(uuid: string, passed: boolean, message?: string): Promise { + try { + const hookMetaData = this._tests[PRE_TEST_HOOK_ID] + if (!hookMetaData || hookMetaData.uuid !== uuid) { + return + } + hookMetaData.finishedAt = (new Date()).toISOString() + this.setCurrentHook({ uuid, finished: true }) + const result = { passed, error: message ? { message } : undefined } as Frameworks.TestResult + this.listener.hookFinished(this.getHookRunDataForCucumber(hookMetaData, 'HookRunFinished', result)) + delete this._tests[PRE_TEST_HOOK_ID] + } catch (error) { + BStackLogger.debug(`Could not close the pre-test hook run: ${error}`) + } + } + public getHookRunDataForCucumber(hookData: TestMeta, eventType: string, result?: Frameworks.TestResult) { const { uri, feature } = this._cucumberData const testData: TestData = { uuid: hookData.uuid, type: 'hook', - name: this.getCucumberHookName(hookData.hookType), + // stored name wins: callers that synthesise a hook carry their own label + name: hookData.name || this.getCucumberHookName(hookData.hookType), body: { lang: 'webdriverio', code: null diff --git a/packages/browserstack-service/src/service.ts b/packages/browserstack-service/src/service.ts index a66a2d1..1d32b1e 100644 --- a/packages/browserstack-service/src/service.ts +++ b/packages/browserstack-service/src/service.ts @@ -323,6 +323,19 @@ export default class BrowserstackService implements Services.ServiceInstance { return } await this._insightsHandler.before() + + // Cucumber only, per its Direct-flow hook reporting: let the accessibility + // handler report the pre-test window as a hook run, so scans fired there have + // a parent the backend can resolve. Jasmine and mocha-multiremote are left + // out — neither reports hooks on a path this can reuse. + if (this._config.framework === 'cucumber' && this._accessibilityHandler) { + const insightsHandler = this._insightsHandler + this._accessibilityHandler.setPreTestHookReporter({ + open: (name: string) => insightsHandler.reportPreTestHookStarted(name), + close: (uuid: string, passed: boolean, message?: string) => + insightsHandler.reportPreTestHookFinished(uuid, passed, message) + }) + } } /** diff --git a/packages/browserstack-service/tests/accessibility-handler.test.ts b/packages/browserstack-service/tests/accessibility-handler.test.ts index be42005..a171f77 100644 --- a/packages/browserstack-service/tests/accessibility-handler.test.ts +++ b/packages/browserstack-service/tests/accessibility-handler.test.ts @@ -689,6 +689,49 @@ describe('afterTest', () => { }) }) +describe('pre-test hook run (cucumber, Direct flow)', () => { + let reporter: { open: ReturnType, close: ReturnType } + + beforeEach(() => { + accessibilityHandler = new AccessibilityHandler(browser, caps, options, false, config, 'cucumber', true, false, accessibilityOpts) + reporter = { open: vi.fn().mockResolvedValue('hook-uuid'), close: vi.fn().mockResolvedValue(undefined) } + accessibilityHandler.setPreTestHookReporter(reporter) + }) + + it('opens once on demand and closes when the first scenario starts', async () => { + await accessibilityHandler['ensurePreTestHookRun']() + await accessibilityHandler['ensurePreTestHookRun']() + expect(reporter.open).toHaveBeenCalledTimes(1) + + await accessibilityHandler['closePreTestHookRun']() + + expect(reporter.close).toHaveBeenCalledWith('hook-uuid', true, undefined) + }) + + it('never closes a run it did not open', async () => { + await accessibilityHandler['closePreTestHookRun']() + + expect(reporter.close).not.toHaveBeenCalled() + }) + + it('reports nothing when no reporter is installed', async () => { + const bare = new AccessibilityHandler(browser, caps, options, false, config, 'cucumber', true, false, accessibilityOpts) + + await bare['ensurePreTestHookRun']() + + expect(bare['_preTestHookState']).toBe('idle') + }) + + it('stays closed-but-unreported when open() yields no uuid', async () => { + reporter.open.mockResolvedValue(undefined) + + await accessibilityHandler['ensurePreTestHookRun']() + await accessibilityHandler['closePreTestHookRun']() + + expect(reporter.close).not.toHaveBeenCalled() + }) +}) + describe('pre-test scan gate (non-CLI flow)', () => { beforeEach(() => { accessibilityHandler = new AccessibilityHandler(browser, caps, options, false, config, 'mocha', true, false, accessibilityOpts) From e3b0d1f4dcfd35de44695ea3381c8033151bdfb4 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Thu, 27 Aug 2026 00:13:09 +0530 Subject: [PATCH 13/17] fix(app-a11y): scope the pre-test window to mocha and cucumber, leaving jasmine untouched MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both the gate and the hook instrumentation were framework-agnostic, so a jasmine or multiremote run would have picked them up. Now gated on PRE_TEST_SCAN_FRAMEWORKS (mocha, cucumber) and not-multiremote, at the two places that act: the classic handler's before() and the service's call to instrumentBrowserContextHooks. Checked against the code rather than taking the premise on trust, and it contradicts it twice: TEST_HOOK_FRAMEWORKS is ['mocha', 'jasmine'] with a comment that WDIO's jasmine adapter emits the same service hooks as mocha (SDK-7190), and util.ts states plainly that every WDIO framework (mocha/jasmine/cucumber) supports App Automate. There is no framework gate on App-A11y anywhere, and no multiremote gate outside the launcher's CLI check. So "unsupported" is a product position, not something the code enforces — worth knowing, because it means jasmine is only unscanned today by accident of provisioning rather than by design. Measured on device, same jasmine bench with a driver-touching config-level before(): prod 9.35.0 0 scans, whole session (jhiej0xnkfxdigpk29psmzykmldy96mvysjzxirs) this branch 0 scans, whole session (ca39y63f7h2qmuck5tylhcaajlxj2fxhtlkv1vya) 0 [hook-window] lines and 0 hook runs in the jasmine build segment test passes identically on both So jasmine is byte-for-byte unchanged, which is the requirement. SDK-7422 --- .../src/accessibility-handler.ts | 18 +++++++-- .../browserstack-service/src/constants.ts | 6 +++ packages/browserstack-service/src/service.ts | 12 ++++-- .../tests/accessibility-handler.test.ts | 38 +++++++++++++++++++ 4 files changed, 67 insertions(+), 7 deletions(-) diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index d510721..dba4352 100644 --- a/packages/browserstack-service/src/accessibility-handler.ts +++ b/packages/browserstack-service/src/accessibility-handler.ts @@ -75,7 +75,7 @@ import accessibilityScripts from './scripts/accessibility-scripts.js' import PerformanceTester from './instrumentation/performance/performance-tester.js' import * as PERFORMANCE_SDK_EVENTS from './instrumentation/performance/constants.js' -import { PRE_TEST_HOOK_TITLE_FALLBACK, PRE_TEST_HOOK_TITLE_PREFIX } from './constants.js' +import { PRE_TEST_HOOK_TITLE_FALLBACK, PRE_TEST_HOOK_TITLE_PREFIX, PRE_TEST_SCAN_FRAMEWORKS } from './constants.js' import { BStackLogger } from './bstackLogger.js' import { getActiveHookName, getPreTestWindowFailure } from './hookInstrumentation.js' @@ -199,6 +199,12 @@ class _AccessibilityHandler { } } + /** mocha and cucumber only — see PRE_TEST_SCAN_FRAMEWORKS. */ + private supportsPreTestWindow(): boolean { + return PRE_TEST_SCAN_FRAMEWORKS.includes(this._framework as typeof PRE_TEST_SCAN_FRAMEWORKS[number]) && + !this._browser?.isMultiremote + } + setPreTestHookReporter(reporter: { open: (name: string) => Promise, close: (uuid: string, passed: boolean, message?: string) => Promise @@ -208,7 +214,7 @@ class _AccessibilityHandler { /** On demand from the scan path, so a suite with no config-level hook reports nothing extra. */ private async ensurePreTestHookRun() { - if (this._preTestHookState !== 'idle' || !this._preTestHookReporter) { + if (this._preTestHookState !== 'idle' || !this._preTestHookReporter || !this.supportsPreTestWindow()) { return } this._preTestHookState = 'attempted' @@ -336,8 +342,12 @@ class _AccessibilityHandler { // Gate for the window between driver creation and the first test/scenario. WDIO's // config-level hooks run there and are no test-framework hooks, so nothing else registers // the session and their commands went unscanned. beforeTest/beforeScenario/beforeHook each - // recompute it. Same fix as the CLI flow's onBeforeExecute, for mocha-direct and cucumber. - if (this._autoScanning) { + // recompute it. Same fix as the CLI flow's onBeforeExecute. + // + // Scoped to mocha and cucumber. Jasmine is in the per-test path too, so opening this for + // it would add scans in a window it never scanned — a behaviour change on a framework + // where App-A11y is not supported. Multiremote likewise. + if (this._autoScanning && this.supportsPreTestWindow()) { AccessibilityHandler._a11yScanSessionMap[sessionId] = true } diff --git a/packages/browserstack-service/src/constants.ts b/packages/browserstack-service/src/constants.ts index fe6d9ce..284a987 100644 --- a/packages/browserstack-service/src/constants.ts +++ b/packages/browserstack-service/src/constants.ts @@ -113,6 +113,12 @@ export const PRE_TEST_WINDOW_HOOKS = ['before', 'beforeSuite', 'beforeFeature'] /* Tag on hook-window diagnostics, so they can be grepped out of an uploaded log */ export const HOOK_WINDOW_LOG_PREFIX = '[hook-window]' +/* Frameworks the pre-test window work applies to. Jasmine is excluded deliberately: it is in + the per-test a11y path (TEST_HOOK_FRAMEWORKS) and keeps that behaviour untouched, but App-A11y + is not supported there, so nothing new is switched on for it. Multiremote is excluded the same + way, and separately never reaches the CLI flow. */ +export const PRE_TEST_SCAN_FRAMEWORKS = ['mocha', 'cucumber'] as const + /* Key the pre-test hook run is stashed under, so its finish can find its start */ export const PRE_TEST_HOOK_ID = 'bstack:pre-test-hook' diff --git a/packages/browserstack-service/src/service.ts b/packages/browserstack-service/src/service.ts index 1d32b1e..cd1402d 100644 --- a/packages/browserstack-service/src/service.ts +++ b/packages/browserstack-service/src/service.ts @@ -11,13 +11,14 @@ import { isFalse, getUniqueIdentifier, getHookType, - isBrowserstackExecutorScript + isBrowserstackExecutorScript, + isMultiRemoteCaps } from './util.js' import type { BrowserstackConfig, BrowserstackOptions, MultiRemoteAction } from './types.js' import type { Pickle, Feature, ITestCaseHookParameter, CucumberHook } from './cucumber-types.js' import InsightsHandler from './insights-handler.js' import TestReporter from './reporter.js' -import { DEFAULT_OPTIONS, NOT_ALLOWED_KEYS_IN_CAPS, PERF_MEASUREMENT_ENV } from './constants.js' +import { DEFAULT_OPTIONS, NOT_ALLOWED_KEYS_IN_CAPS, PERF_MEASUREMENT_ENV, PRE_TEST_SCAN_FRAMEWORKS } from './constants.js' import CrashReporter from './crash-reporter.js' import AccessibilityHandler from './accessibility-handler.js' import CustomTagsHandler from './custom-tags-handler.js' @@ -122,7 +123,12 @@ export default class BrowserstackService implements Services.ServiceInstance { // Make the boundaries of the user's own session-scoped config hooks observable. Logging // only; patched here because the runner reads these arrays after the service is built. - instrumentBrowserContextHooks(this._config) + // Scoped to the frameworks the pre-test window work applies to, so a jasmine or multiremote + // run has its handlers left exactly as WDIO registered them. + if (PRE_TEST_SCAN_FRAMEWORKS.includes(this._config?.framework as typeof PRE_TEST_SCAN_FRAMEWORKS[number]) && + !isMultiRemoteCaps(this._caps as Capabilities.TestrunnerCapabilities)) { + instrumentBrowserContextHooks(this._config) + } PerformanceTester.startMonitoring('performance-report-service.csv') if (shouldProcessEventForTesthub('')) { diff --git a/packages/browserstack-service/tests/accessibility-handler.test.ts b/packages/browserstack-service/tests/accessibility-handler.test.ts index a171f77..e879bff 100644 --- a/packages/browserstack-service/tests/accessibility-handler.test.ts +++ b/packages/browserstack-service/tests/accessibility-handler.test.ts @@ -689,6 +689,44 @@ describe('afterTest', () => { }) }) +describe('frameworks the pre-test window does NOT apply to', () => { + beforeEach(() => { + vi.spyOn(utils, 'isBrowserstackSession').mockReturnValue(true) + vi.spyOn(utils, 'isAccessibilityAutomationSession').mockReturnValue(true) + vi.spyOn(utils, 'validateCapsWithA11y').mockReturnValue(true) + }) + + it('leaves jasmine exactly as it was — no pre-test gate', async () => { + // jasmine IS in TEST_HOOK_FRAMEWORKS, so it already scans per test. Opening the window + // for it would add scans where it never had them, on a framework App-A11y does not + // support. Its per-test behaviour is untouched either way. + const handler = new AccessibilityHandler(browser, caps, options, false, config, 'jasmine', true, false, accessibilityOpts) + + await handler.before('session-jasmine') + + expect(AccessibilityHandler['_a11yScanSessionMap']['session-jasmine']).toBeUndefined() + }) + + it('leaves multiremote alone', async () => { + const multiremote = { ...browser, isMultiremote: true } as never + const handler = new AccessibilityHandler(multiremote, caps, options, false, config, 'mocha', true, false, accessibilityOpts) + + await handler.before('session-multiremote') + + expect(AccessibilityHandler['_a11yScanSessionMap']['session-multiremote']).toBeUndefined() + }) + + it('does not open a hook run for jasmine even if a reporter is installed', async () => { + const handler = new AccessibilityHandler(browser, caps, options, false, config, 'jasmine', true, false, accessibilityOpts) + const reporter = { open: vi.fn().mockResolvedValue('uuid'), close: vi.fn() } + handler.setPreTestHookReporter(reporter) + + await handler['ensurePreTestHookRun']() + + expect(reporter.open).not.toHaveBeenCalled() + }) +}) + describe('pre-test hook run (cucumber, Direct flow)', () => { let reporter: { open: ReturnType, close: ReturnType } From c79e2ba180b6dcc31c1a2304821d7e65c595b3b3 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Thu, 27 Aug 2026 11:31:11 +0530 Subject: [PATCH 14/17] test(app-a11y): pin why multiremote never scanned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A multiremote browser has no sessionId — MultiRemoteDriver defines none and the element path deletes it, which is why wdio-runner reads ids per instance instead. The service passes browser.sessionId straight to the handler, so it receives undefined, and the classic scan gate is keyed on session id: commandWrapper's `this._sessionId && map[this._sessionId]` can never match. So multiremote was never scanning by construction, not by policy. Recording it as a test because the reason is three files away from the code that depends on it, and because the pre-test gate would otherwise have written an 'undefined' key into the shared map. SDK-7422 --- .../tests/accessibility-handler.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/browserstack-service/tests/accessibility-handler.test.ts b/packages/browserstack-service/tests/accessibility-handler.test.ts index e879bff..e955520 100644 --- a/packages/browserstack-service/tests/accessibility-handler.test.ts +++ b/packages/browserstack-service/tests/accessibility-handler.test.ts @@ -716,6 +716,19 @@ describe('frameworks the pre-test window does NOT apply to', () => { expect(AccessibilityHandler['_a11yScanSessionMap']['session-multiremote']).toBeUndefined() }) + it('does not key the map on "undefined" when multiremote hands over no session id', async () => { + // A multiremote browser has no sessionId — MultiRemoteDriver defines none and the element + // path deletes it, which is why wdio-runner reads ids per instance. The service passes + // `browser.sessionId` straight through, so the handler receives undefined. + const multiremote = { ...browser, isMultiremote: true } as never + const handler = new AccessibilityHandler(multiremote, caps, options, false, config, 'mocha', true, false, accessibilityOpts) + + await handler.before(undefined as unknown as string) + + expect(AccessibilityHandler['_a11yScanSessionMap']['undefined']).toBeUndefined() + expect(handler['_sessionId']).toBeUndefined() + }) + it('does not open a hook run for jasmine even if a reporter is installed', async () => { const handler = new AccessibilityHandler(browser, caps, options, false, config, 'jasmine', true, false, accessibilityOpts) const reporter = { open: vi.fn().mockResolvedValue('uuid'), close: vi.fn() } From 1987b06eafc360a1252808e0f6020b1aba7acd14 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Thu, 27 Aug 2026 14:09:18 +0530 Subject: [PATCH 15/17] fix(app-a11y): stamp the pre-test hook uuid onto the scan on the classic path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cucumber hook run was reaching TRA while the scans in its window still went out with no parent at all — thTestRunUuid ABSENT and thHookRunUuid ABSENT — so the join the hook run exists for could not happen. Caught by reading a real payload rather than trusting the previous commit's claim that the asymmetry was closed. The mechanism was already there: commandWrapper passes this._currentHookRunUuid to performA11yScan as thHookRunUuid, exactly as beforeHook/afterHook use it for framework hooks. The pre-test window just never set it. Set on open, cleared on close so a test-body scan is never stamped as a hook scan. SDK-7422 --- .../browserstack-service/src/accessibility-handler.ts | 6 ++++++ .../tests/accessibility-handler.test.ts | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index dba4352..93f8c25 100644 --- a/packages/browserstack-service/src/accessibility-handler.ts +++ b/packages/browserstack-service/src/accessibility-handler.ts @@ -224,6 +224,10 @@ class _AccessibilityHandler { this._preTestHookUuid = await this._preTestHookReporter.open(name) if (this._preTestHookUuid) { this._preTestHookState = 'open' + // What actually reaches the scan: commandWrapper passes this to performA11yScan as + // thHookRunUuid. Without it the hook run exists in TRA and the scans in its window + // still arrive with no parent, which is the join this whole thing is for. + this._currentHookRunUuid = this._preTestHookUuid } } catch (error) { BStackLogger.debug(`Could not open a hook run for the pre-test window: ${error}`) @@ -238,6 +242,8 @@ class _AccessibilityHandler { this._preTestHookState = 'closed' try { const failure = getPreTestWindowFailure() + // cleared before the close so a test-body scan is never stamped as a hook scan + this._currentHookRunUuid = null await this._preTestHookReporter.close(this._preTestHookUuid, !failure, failure) } catch (error) { BStackLogger.debug(`Could not close the hook run for the pre-test window: ${error}`) diff --git a/packages/browserstack-service/tests/accessibility-handler.test.ts b/packages/browserstack-service/tests/accessibility-handler.test.ts index e955520..d0fba59 100644 --- a/packages/browserstack-service/tests/accessibility-handler.test.ts +++ b/packages/browserstack-service/tests/accessibility-handler.test.ts @@ -749,6 +749,16 @@ describe('pre-test hook run (cucumber, Direct flow)', () => { accessibilityHandler.setPreTestHookReporter(reporter) }) + it('stamps the hook uuid onto scans, and clears it when the window closes', async () => { + // commandWrapper passes _currentHookRunUuid to performA11yScan as thHookRunUuid; without + // this the hook run exists but the window's scans still arrive with no parent. + await accessibilityHandler['ensurePreTestHookRun']() + expect(accessibilityHandler['_currentHookRunUuid']).toBe('hook-uuid') + + await accessibilityHandler['closePreTestHookRun']() + expect(accessibilityHandler['_currentHookRunUuid']).toBeNull() + }) + it('opens once on demand and closes when the first scenario starts', async () => { await accessibilityHandler['ensurePreTestHookRun']() await accessibilityHandler['ensurePreTestHookRun']() From fbbbaf9b705b137bad181a4c296235cd64dc7466 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Thu, 27 Aug 2026 14:24:11 +0530 Subject: [PATCH 16/17] fix(app-a11y): install the cucumber hook reporter before the window can open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit stamped the hook uuid onto the scan but nothing changed on device: the window's scan still went out with thHookRunUuid ABSENT. The reason was ordering, not the stamp — the reporter was installed after InsightsHandler was constructed, and WDIO runs the user's config-level before() concurrently with ours, so its first command reached the scan path while this method was still setting up. ensurePreTestHookRun found no reporter and did nothing. Installed before AccessibilityHandler.before(), and the callbacks now read _insightsHandler at CALL time instead of capturing it, so being wired before it exists is fine. Measured, cucumber with two commands in the hook (fyiiwkwdl8deghsyanxtq7e0shsrlkdc59xrmg01): scan 1 hookRun=9384b3f5 <- the window, now parented scan 2 hookRun=9384b3f5 scan 3..5 testRun=0032ebfd, hookRun ABSENT <- scenario body, correctly not hook scans Worth noting what the window's scans still lack: thTestRunUuid is ABSENT there, because on the Direct path that env var is only set when a test actually starts. The hook uuid is the join, which is what we agreed was sufficient. SDK-7422 --- packages/browserstack-service/src/service.ts | 28 +++++++++++--------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/packages/browserstack-service/src/service.ts b/packages/browserstack-service/src/service.ts index cd1402d..855842f 100644 --- a/packages/browserstack-service/src/service.ts +++ b/packages/browserstack-service/src/service.ts @@ -300,6 +300,22 @@ export default class BrowserstackService implements Services.ServiceInstance { this._options.accessibilityOptions ) + // Installed before before(), and reading _insightsHandler at CALL time rather + // than capturing it: WDIO runs the user's config-level before() concurrently + // with ours, so its first command can land while this method is still setting + // up. Installing late meant the first scan of the window found no reporter. + // Cucumber only — jasmine and multiremote report hooks on no path this reuses. + if (this._config.framework === 'cucumber') { + this._accessibilityHandler.setPreTestHookReporter({ + open: (name: string) => this._insightsHandler + ? this._insightsHandler.reportPreTestHookStarted(name) + : Promise.resolve(undefined), + close: (uuid: string, passed: boolean, message?: string) => this._insightsHandler + ? this._insightsHandler.reportPreTestHookFinished(uuid, passed, message) + : Promise.resolve() + }) + } + if (this._isCliAccessibilityFlow()){ BStackLogger.info(`CLI is running, tracking accessibility event for before: ${sessionId}`) // BrowserstackCLI.getInstance().getTestFramework()!.trackEvent(AutomationFrameworkState.CREATE, HookState.POST, { sessionId }) @@ -330,18 +346,6 @@ export default class BrowserstackService implements Services.ServiceInstance { } await this._insightsHandler.before() - // Cucumber only, per its Direct-flow hook reporting: let the accessibility - // handler report the pre-test window as a hook run, so scans fired there have - // a parent the backend can resolve. Jasmine and mocha-multiremote are left - // out — neither reports hooks on a path this can reuse. - if (this._config.framework === 'cucumber' && this._accessibilityHandler) { - const insightsHandler = this._insightsHandler - this._accessibilityHandler.setPreTestHookReporter({ - open: (name: string) => insightsHandler.reportPreTestHookStarted(name), - close: (uuid: string, passed: boolean, message?: string) => - insightsHandler.reportPreTestHookFinished(uuid, passed, message) - }) - } } /** From 178c541b93bc75b87dc484b31d01a870d0920fd1 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Thu, 27 Aug 2026 14:37:38 +0530 Subject: [PATCH 17/17] fix(app-a11y): never send a test run uuid for a global-hook scan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A scan fired from a WDIO config-level hook belongs to no test, so it must not carry one. TEST_ANALYTICS_ID in that window holds a uuid mocha minted at instance creation — a test that has not started — and sending it attributed the scan to a test it did not come from. The hook run uuid is the parent there, which is the whole reason it exists. _getParamsForAppAccessibility takes an isGlobalHook flag; both scan paths thread it from an explicit "pre-test window active" flag, set when the gate opens at driver creation and cleared at the first test. Cleared unconditionally on the classic path: the window is over whether or not a hook run was opened, and leaving it set would strip the uuid from every later test-body scan. Two existing tests asserted the LAST argument of performA11yScan, so appending a parameter made them silently follow it — now asserted by position. Two more asserted an exact 3-arg call that is now 4; corrected to expect the forwarded undefined rather than pretending the arity is unchanged. SDK-7422 --- .../src/accessibility-handler.ts | 8 +++++- .../src/cli/modules/accessibilityModule.ts | 12 ++++++--- packages/browserstack-service/src/util.ts | 14 +++++++--- .../tests/accessibility-handler.test.ts | 27 ++++++++++++++++--- .../cli/modules/accessibilityModule.test.ts | 18 +++++++++++-- 5 files changed, 66 insertions(+), 13 deletions(-) diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index 93f8c25..181d668 100644 --- a/packages/browserstack-service/src/accessibility-handler.ts +++ b/packages/browserstack-service/src/accessibility-handler.ts @@ -103,6 +103,8 @@ class _AccessibilityHandler { close: (uuid: string, passed: boolean, message?: string) => Promise } private _preTestHookState: 'idle' | 'attempted' | 'open' | 'closed' = 'idle' + // True from before() until the first test/scenario — see preTestWindowActive on the CLI module. + private _preTestWindowActive = false private _preTestHookUuid?: string private _testIdentifier: string | null = null private _testMetadata: TestMetadata = {} @@ -235,6 +237,9 @@ class _AccessibilityHandler { } private async closePreTestHookRun() { + // cleared unconditionally: the window is over whether or not a hook run was ever opened, + // and leaving it set would strip the test run uuid from every later test-body scan + this._preTestWindowActive = false if (this._preTestHookState !== 'open' || !this._preTestHookUuid || !this._preTestHookReporter) { this._preTestHookState = 'closed' return @@ -355,6 +360,7 @@ class _AccessibilityHandler { // where App-A11y is not supported. Multiremote likewise. if (this._autoScanning && this.supportsPreTestWindow()) { AccessibilityHandler._a11yScanSessionMap[sessionId] = true + this._preTestWindowActive = true } if (!('overwriteCommand' in this._browser && Array.isArray(accessibilityScripts.commandsToWrap))) { @@ -591,7 +597,7 @@ class _AccessibilityHandler { ) ) { BStackLogger.debug(`Performing scan for ${command.class} ${command.name}`) - await performA11yScan(this.isAppAutomate, this._browser, true, true, command.name, undefined, this._currentHookRunUuid) + await performA11yScan(this.isAppAutomate, this._browser, true, true, command.name, undefined, this._currentHookRunUuid, this._preTestWindowActive) } 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`) } diff --git a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts index 817b309..aa091a9 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -45,6 +45,9 @@ export default class AccessibilityModule extends BaseModule { // no hook run of their own). 'attempted' is distinct from 'open' so a failed PRE is neither // retried per command nor closed by a POST that would pair with nothing. private preTestHookState: 'idle' | 'attempted' | 'open' | 'closed' = 'idle' + // True from driver creation until the first test. Scans fired in that window come from WDIO's + // config-level hooks, which belong to no test, so they must not carry a test run uuid. + private preTestWindowActive = false // Captured at PRE: the binary names a hook from the record pushed there, and by POST a // different hook is running. private preTestHookLabel: string | undefined @@ -334,6 +337,7 @@ export default class AccessibilityModule extends BaseModule { const sessionId = this.currentSessionId() if (this.autoScanning && sessionId !== undefined && sessionId !== null) { this.accessibilityMap.set(sessionId, true) + this.preTestWindowActive = true } } catch (error) { @@ -356,7 +360,7 @@ export default class AccessibilityModule extends BaseModule { ) { await this.ensurePreTestHookRun() try { - await this.performScanCli(browser, command.name, this.currentHookRunUuid) + await this.performScanCli(browser, command.name, this.currentHookRunUuid, this.preTestWindowActive) this.logger.debug(`Accessibility scan performed after ${command.name} command`) } catch (scanError) { this.logger.debug(`Error performing accessibility scan after ${command.name}: ${scanError}`) @@ -382,6 +386,7 @@ export default class AccessibilityModule extends BaseModule { // Window over: close its hook run before clearing the uuid, or a spec with no // framework hooks would stamp every test-body scan as a hook scan. await this.closePreTestHookRun() + this.preTestWindowActive = false this.currentHookRunUuid = null const suiteTitle = (typeof args.suiteTitle === 'string' ? args.suiteTitle : '') || '' const test = (args.test && typeof args.test === 'object' ? args.test as { title?: string } : {}) || {} @@ -576,7 +581,8 @@ export default class AccessibilityModule extends BaseModule { private async performScanCli( browser: WebdriverIO.Browser | WebdriverIO.MultiRemoteBrowser, commandName?: string, - hookRunUuid?: string | null + hookRunUuid?: string | null, + isGlobalHook?: boolean ): Promise | undefined> { return await PerformanceTester.measureWrapper( PERFORMANCE_SDK_EVENTS.A11Y_EVENTS.PERFORM_SCAN, @@ -589,7 +595,7 @@ export default class AccessibilityModule extends BaseModule { if (this.isAppAccessibility) { const testName=this.currentTestName || undefined const results: unknown = await (browser as WebdriverIO.Browser).execute( - formatString(this.scriptInstance.performScan, JSON.stringify(_getParamsForAppAccessibility(commandName, testName, hookRunUuid))) as string, + formatString(this.scriptInstance.performScan, JSON.stringify(_getParamsForAppAccessibility(commandName, testName, hookRunUuid, isGlobalHook))) as string, {} ) BStackLogger.debug(util.format(results as string)) diff --git a/packages/browserstack-service/src/util.ts b/packages/browserstack-service/src/util.ts index 8c381e9..600e562 100644 --- a/packages/browserstack-service/src/util.ts +++ b/packages/browserstack-service/src/util.ts @@ -595,9 +595,15 @@ export const formatString = (template: (string | null), ...values: (string | nul } // eslint-disable-next-line @typescript-eslint/no-explicit-any -export const _getParamsForAppAccessibility = ( commandName?: string, testName?: string, hookRunUuid?: string | null ): { thTestRunUuid: any, thHookRunUuid: any, thBuildUuid: any, thJwtToken: any, authHeader: any, scanTimestamp: number, method: string | undefined, testName: string | undefined } => { +/** + * `isGlobalHook` marks a scan fired from a WDIO CONFIG-level hook — before()/beforeSuite()/ + * beforeFeature() — which belongs to no test. TEST_ANALYTICS_ID there holds the uuid of a test + * that has not started (mocha mints it at instance creation), so sending it would attribute the + * scan to a test it did not come from. The hook run uuid is the parent in that case. + */ +export const _getParamsForAppAccessibility = ( commandName?: string, testName?: string, hookRunUuid?: string | null, isGlobalHook?: boolean ): { thTestRunUuid: any, thHookRunUuid: any, thBuildUuid: any, thJwtToken: any, authHeader: any, scanTimestamp: number, method: string | undefined, testName: string | undefined } => { return { - 'thTestRunUuid': process.env.TEST_ANALYTICS_ID, + 'thTestRunUuid': isGlobalHook ? undefined : process.env.TEST_ANALYTICS_ID, // Present only when the scan fires inside a hook (dropped by JSON.stringify when undefined, // so in-test scans are unchanged). SeleniumHub appAllyHandler relays this as `hook_run_uuid`. 'thHookRunUuid': hookRunUuid || undefined, @@ -611,7 +617,7 @@ export const _getParamsForAppAccessibility = ( commandName?: string, testName?: } /* eslint-disable @typescript-eslint/no-explicit-any */ -export const performA11yScan = async (isAppAutomate: boolean, browser: WebdriverIO.Browser | WebdriverIO.MultiRemoteBrowser, isBrowserStackSession?: boolean, isAccessibility?: boolean | string, commandName?: string, testName?: string, hookRunUuid?: string | null,) : Promise<{ [key: string]: any; } | undefined> => { +export const performA11yScan = async (isAppAutomate: boolean, browser: WebdriverIO.Browser | WebdriverIO.MultiRemoteBrowser, isBrowserStackSession?: boolean, isAccessibility?: boolean | string, commandName?: string, testName?: string, hookRunUuid?: string | null, isGlobalHook?: boolean) : Promise<{ [key: string]: any; } | undefined> => { if (!isAccessibilityAutomationSession(isAccessibility)) { BStackLogger.warn('Not an Accessibility Automation session, cannot perform Accessibility scan.') @@ -620,7 +626,7 @@ export const performA11yScan = async (isAppAutomate: boolean, browser: Webdriver try { if (isAppAccessibilityAutomationSession(isAccessibility, isAppAutomate)) { - const results: unknown = await (browser as WebdriverIO.Browser).execute(formatString(AccessibilityScripts.performScan, JSON.stringify(_getParamsForAppAccessibility(commandName, testName, hookRunUuid))) as string, {}) + const results: unknown = await (browser as WebdriverIO.Browser).execute(formatString(AccessibilityScripts.performScan, JSON.stringify(_getParamsForAppAccessibility(commandName, testName, hookRunUuid, isGlobalHook))) as string, {}) BStackLogger.debug(util.format(results as string)) return ( results as { [key: string]: any; } | undefined ) } diff --git a/packages/browserstack-service/tests/accessibility-handler.test.ts b/packages/browserstack-service/tests/accessibility-handler.test.ts index d0fba59..03aac27 100644 --- a/packages/browserstack-service/tests/accessibility-handler.test.ts +++ b/packages/browserstack-service/tests/accessibility-handler.test.ts @@ -483,8 +483,10 @@ describe('beforeHook / afterHook (hook scans)', () => { await accessibilityHandler['commandWrapper']({ name: 'click', class: 'Element' } as any, undefined as any, orig, 'arg') expect(scanSpy).toHaveBeenCalled() const lastCall = scanSpy.mock.calls[scanSpy.mock.calls.length - 1] - // performA11yScan(isAppAutomate, browser, isBS, isA11y, commandName, testName, hookRunUuid) - expect(lastCall[lastCall.length - 1]).toBe('hook-uuid-42') + // performA11yScan(isAppAutomate, browser, isBS, isA11y, commandName, testName, hookRunUuid, + // isGlobalHook) — asserted by POSITION, not "last": the signature grew an + // argument and a last-element assertion silently followed it. + expect(lastCall[6]).toBe('hook-uuid-42') }) it('does NOT stamp a hook uuid on a test-body scan (afterHook cleared it)', async () => { @@ -500,7 +502,26 @@ describe('beforeHook / afterHook (hook scans)', () => { await accessibilityHandler['commandWrapper']({ name: 'click', class: 'Element' } as any, undefined as any, orig, 'arg') expect(scanSpy).toHaveBeenCalled() const lastCall = scanSpy.mock.calls[scanSpy.mock.calls.length - 1] - expect(lastCall[lastCall.length - 1]).toBeNull() + expect(lastCall[6]).toBeNull() + }) + + it('omits thTestRunUuid for a global-hook scan, keeps it for a test-body scan', () => { + // TEST_ANALYTICS_ID holds a uuid minted at instance creation, i.e. a test that has not + // started. Sending it from a config-level hook would attribute the scan to a test it did + // not come from; the hook run uuid is the parent there. + process.env.TEST_ANALYTICS_ID = 'test-run-uuid-1' + + const fromGlobalHook = utils._getParamsForAppAccessibility('back', undefined, 'hook-uuid-1', true) + expect(fromGlobalHook.thTestRunUuid).toBeUndefined() + expect(fromGlobalHook.thHookRunUuid).toBe('hook-uuid-1') + + const fromTestBody = utils._getParamsForAppAccessibility('back', 'a test', null, false) + expect(fromTestBody.thTestRunUuid).toBe('test-run-uuid-1') + + // default (flag omitted) must stay as it was for every existing caller + expect(utils._getParamsForAppAccessibility('back', 'a test').thTestRunUuid).toBe('test-run-uuid-1') + + delete process.env.TEST_ANALYTICS_ID }) it('_getParamsForAppAccessibility puts the hook uuid on the scan payload as thHookRunUuid', () => { diff --git a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts index f26dfd7..23ced77 100644 --- a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts +++ b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts @@ -695,7 +695,21 @@ describe('AccessibilityModule', () => { await (accessibilityModule as any).performScanCli(mockBrowser, 'click', 'hook-uuid-99') - expect(_getParamsForAppAccessibility).toHaveBeenCalledWith('click', undefined, 'hook-uuid-99') + // 4th arg is the global-hook flag; a direct call passes none, so it forwards undefined + expect(_getParamsForAppAccessibility).toHaveBeenCalledWith('click', undefined, 'hook-uuid-99', undefined) + }) + + it('marks a scan from the pre-test window as a global-hook scan', async () => { + // the flag is what strips thTestRunUuid: in that window the env var holds a uuid for a + // test that has not started, so sending it would attribute the scan to the wrong test + accessibilityModule.accessibility = true + accessibilityModule.isAppAccessibility = true + mockBrowser.execute.mockResolvedValue({ scanned: true }) + + await (accessibilityModule as never as { performScanCli: (b: unknown, c?: string, h?: string | null, g?: boolean) => Promise }) + .performScanCli(mockBrowser, 'back', 'hook-uuid-1', true) + + expect(_getParamsForAppAccessibility).toHaveBeenCalledWith('back', undefined, 'hook-uuid-1', true) }) it('passes no hook uuid for an ordinary (non-hook) app scan', async () => { @@ -705,7 +719,7 @@ describe('AccessibilityModule', () => { await (accessibilityModule as any).performScanCli(mockBrowser, 'click') - expect(_getParamsForAppAccessibility).toHaveBeenCalledWith('click', undefined, undefined) + expect(_getParamsForAppAccessibility).toHaveBeenCalledWith('click', undefined, undefined, undefined) }) }) }) \ No newline at end of file