From 41372c3f7ef5fb673a7bcb0921ebf39a08e681fa Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Mon, 31 Aug 2026 14:30:43 +0530 Subject: [PATCH 1/9] feat(accessibility): scan the WDIO config-level hook window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A driver command issued from a config-level before()/beforeSuite was never scanned. The scan gate opened at the first test, and WDIO runs those hooks before any test or framework hook exists, so setup screens went uncovered. The gate now opens as soon as the session is known a11y-capable. Every validation the per-test gate applies still applies here — an a11y-capable session, autoScanning, a supported framework (mocha, cucumber; jasmine untouched), a real session id, and non-multiremote. The include/exclude tag filter is the one exception: it matches on suite and test titles, and in this window neither exists yet. onBeforeTest/beforeScenario re-computes the per-test gate, tags included, so the window governs nothing beyond itself. Scans from the window carry no test run uuid. TEST_ANALYTICS_ID there holds a uuid the framework minted at instance creation — a test that has not started — so sending it attributed the scan to a test it did not come from. Two existing tests pinned the exact argument list of the scan helpers, so appending a parameter made them silently follow it; both now assert by position. SDK-7422 --- .../src/accessibility-handler.ts | 28 ++++++++- .../src/cli/modules/accessibilityModule.ts | 28 +++++++-- packages/browserstack-service/src/util.ts | 11 ++-- .../tests/accessibility-handler.test.ts | 61 ++++++++++++++++++- .../cli/modules/accessibilityModule.test.ts | 51 +++++++++++++++- .../browserstack-service/tests/util.test.ts | 13 ++++ 6 files changed, 178 insertions(+), 14 deletions(-) diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index 69c3350..856ab90 100644 --- a/packages/browserstack-service/src/accessibility-handler.ts +++ b/packages/browserstack-service/src/accessibility-handler.ts @@ -84,6 +84,10 @@ class _AccessibilityHandler { * cucumber goes through beforeScenario/afterScenario instead. */ private static readonly TEST_HOOK_FRAMEWORKS = ['mocha', 'jasmine'] + // Frameworks whose config-level hooks are covered by the pre-test window. Jasmine is + // excluded deliberately — App Accessibility is not supported there and its behaviour must + // not change; multiremote is excluded in the guard below, having no session id to gate on. + private static readonly PRE_TEST_SCAN_FRAMEWORKS = ['mocha', 'cucumber'] private _platformA11yMeta: PlatformA11yMeta private _caps: Capabilities.ResolvedTestrunnerCapabilities private _suiteFile?: string @@ -93,6 +97,8 @@ class _AccessibilityHandler { private _config: Options.Testrunner private _accessibilityOptions?: AccessibilityOptions private _autoScanning: boolean = true + // True from before() until the first test or scenario — see the CLI module's equivalent. + private _preTestWindowActive: boolean = false private _testIdentifier: string | null = null private _testMetadata: TestMetadata = {} /* Set while a supported hook is executing; scans fired in this window are stamped with it. */ @@ -282,6 +288,17 @@ class _AccessibilityHandler { if (!this._accessibility) { return } + + // WDIO's config-level hooks run before any test exists, so the per-test gate below has + // not been computed yet and driver commands issued there went unscanned. Every other + // validation still applies — an a11y-capable session (returned above), autoScanning, a + // supported framework, a real session id. The include/exclude tag filter is the one + // exception: it matches on suite and test titles, and neither exists in this window. + if (this._autoScanning && this.supportsPreTestWindow() && sessionId) { + AccessibilityHandler._a11yScanSessionMap[sessionId] = true + this._preTestWindowActive = true + BStackLogger.debug('Accessibility scan gate opened for the pre-test window') + } if (!('overwriteCommand' in this._browser && Array.isArray(accessibilityScripts.commandsToWrap))) { return } @@ -304,6 +321,11 @@ class _AccessibilityHandler { } + private supportsPreTestWindow(): boolean { + return AccessibilityHandler.PRE_TEST_SCAN_FRAMEWORKS.includes(this._framework as string) && + !this._browser?.isMultiremote + } + async beforeTest (suiteTitle: string | undefined, test: Frameworks.Test) { try { if ( @@ -317,6 +339,8 @@ class _AccessibilityHandler { /* jasmine test objects carry the spec name in `description` (`title` is unset) */ const testTitle = test.title ?? test.description + // Window over: the per-test gate owns the decision from here, tags included. + this._preTestWindowActive = false // @ts-expect-error fix type const shouldScanTest = this._autoScanning && shouldScanTestForAccessibility(suiteTitle, testTitle, this._accessibilityOptions) const testIdentifier = this.getIdentifier(test) @@ -397,6 +421,8 @@ class _AccessibilityHandler { } try { + // Window over: the per-test gate owns the decision from here, tags included. + this._preTestWindowActive = false // @ts-expect-error fix type const shouldScanScenario = this._autoScanning && shouldScanTestForAccessibility(featureData?.name, pickleData.name, this._accessibilityOptions, world, true) this._testMetadata[uniqueId] = { @@ -513,7 +539,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 8e44b11..817671f 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -27,6 +27,9 @@ export default class AccessibilityModule extends BaseModule { scriptInstance: typeof accessibilityScripts accessibility: boolean = false autoScanning: boolean = true + // 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 carry no test run uuid. + preTestWindowActive: boolean = false isAppAccessibility: boolean isNonBstackA11y: boolean accessibilityConfig: Accessibility @@ -216,7 +219,7 @@ export default class AccessibilityModule extends BaseModule { return } // If invoked from inside a hook, currentHookRunUuid stamps the scan for the hook. - return await this.performScanCli(browser, undefined, this.currentHookRunUuid) + return await this.performScanCli(browser, undefined, this.currentHookRunUuid, this.preTestWindowActive) } (browser as WebdriverIO.Browser).startA11yScanning = async () => { @@ -263,6 +266,20 @@ export default class AccessibilityModule extends BaseModule { }) } + // WDIO's config-level hooks (before, beforeSuite) run before any test or framework + // hook exists, so onHookStart — which returns early without a test instance — cannot + // cover them, and driver commands issued there went unscanned. Every validation + // onHookStart applies still applies here: an a11y-capable session (returned above) + // and autoScanning. The include/exclude tag filter is the one exception — it matches + // on suite and test titles, and in this window neither exists yet. onBeforeTest + // re-computes the per-test gate, tags included, so this only affects the window. + const preTestSessionId = this.currentSessionId() + if (this.autoScanning && preTestSessionId !== undefined && preTestSessionId !== null) { + this.accessibilityMap.set(preTestSessionId, true) + this.preTestWindowActive = true + this.logger.debug('Accessibility scan gate opened for the pre-test window') + } + } catch (error) { this.logger.error(`Error in onBeforeExecute: ${error}`) } @@ -282,7 +299,7 @@ export default class AccessibilityModule extends BaseModule { !this.shouldPatchExecuteScript(args.length ? args[0] as string : null) ) { 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}`) @@ -316,6 +333,8 @@ export default class AccessibilityModule extends BaseModule { const accessibilityOptions = this.config.accessibilityOptions const shouldScanTest = this.autoScanning && shouldScanTestForAccessibility(suiteTitle, test.title || '', accessibilityOptions as Record | undefined) && this.accessibility + // Window over: the per-test gate below owns the decision from here, tags included. + this.preTestWindowActive = false this.accessibilityMap.set(sessionId, shouldScanTest) // Create test metadata similar to accessibility-handler @@ -498,7 +517,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, @@ -511,7 +531,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..c78ac37 100644 --- a/packages/browserstack-service/src/util.ts +++ b/packages/browserstack-service/src/util.ts @@ -595,9 +595,12 @@ 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 } => { +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, + // A scan from a WDIO config-level hook belongs to no test. TEST_ANALYTICS_ID in that + // window holds a uuid the framework minted at instance creation — a test that has not + // started — so sending it would attribute the scan to a test it did not come from. + '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 +614,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 +623,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 1d75f54..f847d15 100644 --- a/packages/browserstack-service/tests/accessibility-handler.test.ts +++ b/packages/browserstack-service/tests/accessibility-handler.test.ts @@ -433,6 +433,60 @@ describe('afterScenario', () => { }) }) +describe('pre-test window (config-level hooks)', () => { + const handlerFor = (framework: string) => { + const handler = new AccessibilityHandler(browser, caps, options, false, config, framework, true, false, accessibilityOpts) + vi.spyOn(utils, 'isBrowserstackSession').mockReturnValue(true) + vi.spyOn(utils, 'isAccessibilityAutomationSession').mockReturnValue(true) + return handler + } + + it('opens the scan gate in before(), so config-level hook commands are scanned', async () => { + const handler = handlerFor('mocha') + + await handler.before('session-window') + + expect(AccessibilityHandler['_a11yScanSessionMap']['session-window']).toBe(true) + expect(handler['_preTestWindowActive']).toBe(true) + }) + + it('covers cucumber too', async () => { + const handler = handlerFor('cucumber') + + await handler.before('session-cuke') + + expect(handler['_preTestWindowActive']).toBe(true) + }) + + it('leaves jasmine exactly as it was — App Accessibility is not supported there', async () => { + const handler = handlerFor('jasmine') + + await handler.before('session-jasmine') + + expect(handler['_preTestWindowActive']).toBe(false) + }) + + it('skips multiremote, which has no session id to gate on', async () => { + const handler = handlerFor('mocha') + handler['_browser'] = { ...browser, isMultiremote: true } as any + + await handler.before('session-multi') + + expect(handler['_preTestWindowActive']).toBe(false) + }) + + it('closes the window at the first test', async () => { + const handler = handlerFor('mocha') + await handler.before('session-close') + expect(handler['_preTestWindowActive']).toBe(true) + + vi.spyOn(utils, 'shouldScanTestForAccessibility').mockReturnValue(true) + await handler.beforeTest('suite', { title: 'test', parent: 'suite' } as any) + + expect(handler['_preTestWindowActive']).toBe(false) + }) +}) + describe('beforeHook / afterHook (hook scans)', () => { beforeEach(() => { accessibilityHandler = new AccessibilityHandler(browser, caps, options, false, config, 'mocha', true, false, accessibilityOpts) @@ -483,8 +537,9 @@ 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: reading the last argument silently follows anything appended here + 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 +555,7 @@ 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('_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 0c77511..3d96d75 100644 --- a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts +++ b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts @@ -162,6 +162,53 @@ describe('AccessibilityModule', () => { }) }) + describe('pre-test window gate', () => { + // afterEach's vi.resetAllMocks() drops the factory's mockReturnValue, so the caps + // validators return undefined and onBeforeExecute bails before the gate. Re-arm them. + const withA11yCaps = () => { + vi.mocked(validateCapsWithA11y).mockReturnValue(true) + vi.mocked(validateCapsWithAppA11y).mockReturnValue(true) + return vi.mocked(AutomationFramework.getState).mockImplementation((instance, key) => { + if (key.includes('INPUT_CAPABILITIES')) { + return {} + } + if (key.includes('CAPABILITIES')) { + return { browserName: 'chrome' } + } + return 'session-w' + }) + } + + it('opens the scan gate at driver creation, before any test exists', async () => { + withA11yCaps() + + await accessibilityModule.onBeforeExecute() + + expect(accessibilityModule.accessibilityMap.get('session-w')).toBe(true) + expect(accessibilityModule.preTestWindowActive).toBe(true) + }) + + it('respects autoScanning — the one validation the window still owns', async () => { + withA11yCaps() + accessibilityModule.autoScanning = false + + await accessibilityModule.onBeforeExecute() + + expect(accessibilityModule.accessibilityMap.get('session-w')).toBeUndefined() + expect(accessibilityModule.preTestWindowActive).toBe(false) + }) + + it('closes the window at the first test, handing the gate back to the tag filter', async () => { + withA11yCaps() + await accessibilityModule.onBeforeExecute() + expect(accessibilityModule.preTestWindowActive).toBe(true) + + await accessibilityModule.onBeforeTest({ suiteTitle: 'suite', test: { title: 'test' } }) + + expect(accessibilityModule.preTestWindowActive).toBe(false) + }) + }) + describe('onBeforeExecute', () => { it('should patch browser methods when automation instance exists', async () => { vi.mocked(AutomationFramework.getState).mockImplementation((instance, key) => { @@ -570,7 +617,7 @@ describe('AccessibilityModule', () => { await (accessibilityModule as any).performScanCli(mockBrowser, 'click', 'hook-uuid-99') - expect(_getParamsForAppAccessibility).toHaveBeenCalledWith('click', undefined, 'hook-uuid-99') + expect(_getParamsForAppAccessibility).toHaveBeenCalledWith('click', undefined, 'hook-uuid-99', undefined) }) it('passes no hook uuid for an ordinary (non-hook) app scan', async () => { @@ -580,7 +627,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 diff --git a/packages/browserstack-service/tests/util.test.ts b/packages/browserstack-service/tests/util.test.ts index 6ef86c8..64b02c0 100644 --- a/packages/browserstack-service/tests/util.test.ts +++ b/packages/browserstack-service/tests/util.test.ts @@ -2156,6 +2156,19 @@ describe('_getParamsForAppAccessibility', () => { }) }) + it('omits the test run uuid for a global-hook scan, keeping the hook uuid', () => { + const result = _getParamsForAppAccessibility('click', undefined, 'hook-uuid-1', true) + + expect(result.thTestRunUuid).toBeUndefined() + expect(result.thHookRunUuid).toBe('hook-uuid-1') + expect(result.thBuildUuid).toBe('build-456') + }) + + it('sends the test run uuid when the scan is not from a global hook', () => { + expect(_getParamsForAppAccessibility('click', undefined, null, false).thTestRunUuid).toBe('test-123') + expect(_getParamsForAppAccessibility('click').thTestRunUuid).toBe('test-123') + }) + it('should handle missing environment variables', () => { process.env = {} From 2ab9eb089be0f92c0e7d5fdbe636b3c1056eb411 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 31 Aug 2026 09:36:10 +0000 Subject: [PATCH 2/9] chore(changeset): auto-generate from PR template (minor) --- .changeset/pr-176.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/pr-176.md diff --git a/.changeset/pr-176.md b/.changeset/pr-176.md new file mode 100644 index 0000000..0592c0a --- /dev/null +++ b/.changeset/pr-176.md @@ -0,0 +1,5 @@ +--- +"@wdio/browserstack-service": minor +--- + +- Accessibility scans now run for driver commands issued from your WDIO config's `before()` and `beforeSuite` hooks, so screens visited during setup are covered. From 2a872db18cf548bcd3e165cb10745667fe3bf879 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Mon, 31 Aug 2026 17:05:53 +0530 Subject: [PATCH 3/9] fix(accessibility): end the window at the first framework hook, not the first test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The window targets WDIO's own config hooks. Closing it at the first test swept in mocha's before all / before each #1, which run inside that span, and stripped their test run uuid — baseline carried it, the first cut showed none. That is a behaviour change to framework hooks, which are not this feature's business. It now closes the moment the framework signals a hook: onHookStart on the CLI path, beforeHook on the classic one. The CLI clear sits ahead of that method's own early returns — if a framework hook started at all, the config-level window is over, whether or not that hook goes on to scan. The existing clears at the first test/scenario stay as a backstop for a spec with no framework hooks. Four tests cover it, two per flow: a framework hook closes the window, and a framework-hook scan still carries its test run uuid. Reverting either clear fails exactly those four. Verified on device — repro-inline-hooks (framework hooks only) is now structurally identical to baseline, every scan carrying its test uuid, while repro.conf.ts still gains its 7 parentless config-level scans. SDK-7422 --- .../src/accessibility-handler.ts | 3 +++ .../src/cli/modules/accessibilityModule.ts | 6 +++++ .../tests/accessibility-handler.test.ts | 27 +++++++++++++++++++ .../cli/modules/accessibilityModule.test.ts | 22 +++++++++++++++ 4 files changed, 58 insertions(+) diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index 856ab90..064cd82 100644 --- a/packages/browserstack-service/src/accessibility-handler.ts +++ b/packages/browserstack-service/src/accessibility-handler.ts @@ -491,6 +491,9 @@ class _AccessibilityHandler { */ async beforeHook (test: Frameworks.Test | undefined, context: unknown, hookRunUuid?: string | null) { try { + // Framework hook running ⇒ the config-level window is over. Framework hooks are left + // exactly as they were; only WDIO's own config hooks are targeted. + this._preTestWindowActive = false if (!this._accessibility || !this.shouldRunTestHooks(this._browser, this._accessibility)) { return } diff --git a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts index 817671f..2128331 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -80,6 +80,12 @@ export default class AccessibilityModule extends BaseModule { const hookRunUuid = testInstance ? (TestFramework.getState(testInstance, TestFrameworkConstants.KEY_HOOK_ID) as string | undefined) : undefined this.currentHookRunUuid = hookRunUuid || null + // A framework hook is running, so the config-level window is over. Framework hooks keep + // the treatment they always had, test run uuid included — the window targets WDIO's own + // config hooks only. Cleared ahead of the guards below: if the framework signalled a + // hook at all, the window has ended, whether or not this hook goes on to scan. + this.preTestWindowActive = false + const autoInstance: AutomationFrameworkInstance = AutomationFramework.getTrackedInstance() if (!testInstance || !autoInstance) { return diff --git a/packages/browserstack-service/tests/accessibility-handler.test.ts b/packages/browserstack-service/tests/accessibility-handler.test.ts index f847d15..70a89e5 100644 --- a/packages/browserstack-service/tests/accessibility-handler.test.ts +++ b/packages/browserstack-service/tests/accessibility-handler.test.ts @@ -475,6 +475,33 @@ describe('pre-test window (config-level hooks)', () => { expect(handler['_preTestWindowActive']).toBe(false) }) + it('closes the window when a framework hook starts', async () => { + const handler = handlerFor('mocha') + await handler.before('session-fwhook') + expect(handler['_preTestWindowActive']).toBe(true) + + await handler.beforeHook({ title: '"before all" hook', parent: 'suite' } as any, {}, 'hook-uuid-fw') + + expect(handler['_preTestWindowActive']).toBe(false) + }) + + it('leaves a framework-hook scan carrying its test run uuid', async () => { + const handler = handlerFor('mocha') + handler['_sessionId'] = 'session-fwscan' + await handler.before('session-fwscan') + await handler.beforeHook({ title: '"before all" hook', parent: 'suite' } as any, {}, 'hook-uuid-fw2') + + vi.spyOn(utils, 'shouldScanTestForAccessibility').mockReturnValue(true) + const scanSpy = vi.spyOn(utils, 'performA11yScan').mockResolvedValue(undefined) + const orig = vi.fn().mockResolvedValue('ok') + await handler['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] + // isGlobalHook (index 7) must be false, so the params helper sends the test run uuid + expect(lastCall[7]).toBe(false) + }) + it('closes the window at the first test', async () => { const handler = handlerFor('mocha') await handler.before('session-close') diff --git a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts index 3d96d75..f503a77 100644 --- a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts +++ b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts @@ -198,6 +198,28 @@ describe('AccessibilityModule', () => { expect(accessibilityModule.preTestWindowActive).toBe(false) }) + it('closes the window when a framework hook starts, leaving framework hooks untouched', async () => { + withA11yCaps() + await accessibilityModule.onBeforeExecute() + expect(accessibilityModule.preTestWindowActive).toBe(true) + + await accessibilityModule.onHookStart({ instance: mockTestInstance }) + + expect(accessibilityModule.preTestWindowActive).toBe(false) + }) + + it('keeps the test run uuid on a framework-hook scan', async () => { + withA11yCaps() + accessibilityModule.isAppAccessibility = true + await accessibilityModule.onBeforeExecute() + await accessibilityModule.onHookStart({ instance: mockTestInstance }) + + await (accessibilityModule as any).performScanCli(mockBrowser, 'click', 'hook-uuid-1', accessibilityModule.preTestWindowActive) + + // 4th arg false => _getParamsForAppAccessibility keeps process.env.TEST_ANALYTICS_ID + expect(_getParamsForAppAccessibility).toHaveBeenCalledWith('click', undefined, 'hook-uuid-1', false) + }) + it('closes the window at the first test, handing the gate back to the tag filter', async () => { withA11yCaps() await accessibilityModule.onBeforeExecute() From 2b81d10f88fdf42eb08a2faa81f9b72930d2870a Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Tue, 1 Sep 2026 14:39:30 +0530 Subject: [PATCH 4/9] refactor(accessibility): decide scan parentage per scan, not per hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback: the command wrapper fires performScan for any wrapped command and never knows which hook it is in, so a window state machine keyed on hook boundaries was the wrong shape for the question being asked. The question is whether anything can own the scan, which is a property of the moment: hasNoParent = no framework hook run uuid && no test context Framework hook runs are reported to TRA and keep their test uuid, so they are untouched without needing a hook boundary to protect them. This deletes preTestWindowActive, its three clears, and the onHookStart/beforeHook plumbing added for it. Two things fall out. A beforeSuite between suites is now parentless instead of inheriting the finished test's uuid — the reviewer's second finding, fixed without reopening anything. And the CLI "window never closes" concern stops being expressible, there being no window to leave open. _testIdentifier was set at beforeTest/beforeScenario and never cleared, so on the classic path it meant "has any test started" rather than "is one running". It is now cleared at afterTest and afterScenario, which also makes startA11yScanning's "cannot be started from outside the test" guard effective between tests instead of only before the first one. The framework allowlist stays on the gate: it governs SCANNING, not attribution, and App Accessibility is unsupported on jasmine, which must gain no scans it did not have before. Tests: the CLI cases now drive commandWrapper rather than hand-feeding the flag, so they pin the production call site — the reviewer's fifth finding. Added the between-tests case. Missing trailing newline restored. All 12 fail against origin/main src. SDK-7422 --- .../src/accessibility-handler.ts | 28 ++++---- .../src/cli/modules/accessibilityModule.ts | 22 ++----- .../tests/accessibility-handler.test.ts | 66 +++++++++++-------- .../cli/modules/accessibilityModule.test.ts | 40 ++++++----- 4 files changed, 83 insertions(+), 73 deletions(-) diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index 064cd82..f5ec12f 100644 --- a/packages/browserstack-service/src/accessibility-handler.ts +++ b/packages/browserstack-service/src/accessibility-handler.ts @@ -97,8 +97,6 @@ class _AccessibilityHandler { private _config: Options.Testrunner private _accessibilityOptions?: AccessibilityOptions private _autoScanning: boolean = true - // True from before() until the first test or scenario — see the CLI module's equivalent. - private _preTestWindowActive: boolean = false private _testIdentifier: string | null = null private _testMetadata: TestMetadata = {} /* Set while a supported hook is executing; scans fired in this window are stamped with it. */ @@ -293,11 +291,13 @@ class _AccessibilityHandler { // not been computed yet and driver commands issued there went unscanned. Every other // validation still applies — an a11y-capable session (returned above), autoScanning, a // supported framework, a real session id. The include/exclude tag filter is the one - // exception: it matches on suite and test titles, and neither exists in this window. + // exception: it matches on suite and test titles, and neither exists yet. + // + // The framework allowlist is about SCANNING, not about attribution: App Accessibility is + // not supported on jasmine, so it must gain no scans it did not have before. if (this._autoScanning && this.supportsPreTestWindow() && sessionId) { AccessibilityHandler._a11yScanSessionMap[sessionId] = true - this._preTestWindowActive = true - BStackLogger.debug('Accessibility scan gate opened for the pre-test window') + BStackLogger.debug('Accessibility scan gate opened ahead of the first test') } if (!('overwriteCommand' in this._browser && Array.isArray(accessibilityScripts.commandsToWrap))) { return @@ -339,8 +339,6 @@ class _AccessibilityHandler { /* jasmine test objects carry the spec name in `description` (`title` is unset) */ const testTitle = test.title ?? test.description - // Window over: the per-test gate owns the decision from here, tags included. - this._preTestWindowActive = false // @ts-expect-error fix type const shouldScanTest = this._autoScanning && shouldScanTestForAccessibility(suiteTitle, testTitle, this._accessibilityOptions) const testIdentifier = this.getIdentifier(test) @@ -371,6 +369,9 @@ class _AccessibilityHandler { async afterTest (suiteTitle: string | undefined, test: Frameworks.Test) { BStackLogger.debug('Accessibility after test hook. Before sending test stop event') + // The test is over, so scans after this point have no test to belong to until the next one + // starts. Cleared ahead of the guards below: that is true whatever this method goes on to do. + this._testIdentifier = null if ( !AccessibilityHandler.TEST_HOOK_FRAMEWORKS.includes(this._framework as string) || !this.shouldRunTestHooks(this._browser, this._accessibility) @@ -421,8 +422,6 @@ class _AccessibilityHandler { } try { - // Window over: the per-test gate owns the decision from here, tags included. - this._preTestWindowActive = false // @ts-expect-error fix type const shouldScanScenario = this._autoScanning && shouldScanTestForAccessibility(featureData?.name, pickleData.name, this._accessibilityOptions, world, true) this._testMetadata[uniqueId] = { @@ -449,6 +448,8 @@ class _AccessibilityHandler { async afterScenario (world: ITestCaseHookParameter) { BStackLogger.debug('Accessibility after scenario hook. Before sending test stop event') + // See afterTest: the scenario is over, so nothing parents a scan until the next one starts. + this._testIdentifier = null if (!this.shouldRunTestHooks(this._browser, this._accessibility)) { return } @@ -491,9 +492,6 @@ class _AccessibilityHandler { */ async beforeHook (test: Frameworks.Test | undefined, context: unknown, hookRunUuid?: string | null) { try { - // Framework hook running ⇒ the config-level window is over. Framework hooks are left - // exactly as they were; only WDIO's own config hooks are targeted. - this._preTestWindowActive = false if (!this._accessibility || !this.shouldRunTestHooks(this._browser, this._accessibility)) { return } @@ -542,7 +540,11 @@ class _AccessibilityHandler { ) ) { BStackLogger.debug(`Performing scan for ${command.class} ${command.name}`) - await performA11yScan(this.isAppAutomate, this._browser, true, true, command.name, undefined, this._currentHookRunUuid, this._preTestWindowActive) + // A scan belongs to no test only when nothing can parent it: no framework hook run + // (those are reported to TRA and keep their test uuid) and no test started. That is a + // property of the moment, not of which hook is running — the wrapper never knows that. + const hasNoParent = !this._currentHookRunUuid && this._testIdentifier === null + await performA11yScan(this.isAppAutomate, this._browser, true, true, command.name, undefined, this._currentHookRunUuid, hasNoParent) } 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 2128331..d93003b 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -27,9 +27,6 @@ export default class AccessibilityModule extends BaseModule { scriptInstance: typeof accessibilityScripts accessibility: boolean = false autoScanning: boolean = true - // 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 carry no test run uuid. - preTestWindowActive: boolean = false isAppAccessibility: boolean isNonBstackA11y: boolean accessibilityConfig: Accessibility @@ -80,12 +77,6 @@ export default class AccessibilityModule extends BaseModule { const hookRunUuid = testInstance ? (TestFramework.getState(testInstance, TestFrameworkConstants.KEY_HOOK_ID) as string | undefined) : undefined this.currentHookRunUuid = hookRunUuid || null - // A framework hook is running, so the config-level window is over. Framework hooks keep - // the treatment they always had, test run uuid included — the window targets WDIO's own - // config hooks only. Cleared ahead of the guards below: if the framework signalled a - // hook at all, the window has ended, whether or not this hook goes on to scan. - this.preTestWindowActive = false - const autoInstance: AutomationFrameworkInstance = AutomationFramework.getTrackedInstance() if (!testInstance || !autoInstance) { return @@ -225,7 +216,7 @@ export default class AccessibilityModule extends BaseModule { return } // If invoked from inside a hook, currentHookRunUuid stamps the scan for the hook. - return await this.performScanCli(browser, undefined, this.currentHookRunUuid, this.preTestWindowActive) + return await this.performScanCli(browser, undefined, this.currentHookRunUuid, !this.currentHookRunUuid && !this.currentTestName) } (browser as WebdriverIO.Browser).startA11yScanning = async () => { @@ -282,8 +273,7 @@ export default class AccessibilityModule extends BaseModule { const preTestSessionId = this.currentSessionId() if (this.autoScanning && preTestSessionId !== undefined && preTestSessionId !== null) { this.accessibilityMap.set(preTestSessionId, true) - this.preTestWindowActive = true - this.logger.debug('Accessibility scan gate opened for the pre-test window') + this.logger.debug('Accessibility scan gate opened ahead of the first test') } } catch (error) { @@ -305,7 +295,11 @@ export default class AccessibilityModule extends BaseModule { !this.shouldPatchExecuteScript(args.length ? args[0] as string : null) ) { try { - await this.performScanCli(browser, command.name, this.currentHookRunUuid, this.preTestWindowActive) + // Parentless only when nothing can own the scan: no framework hook run + // (reported to TRA, keeps its test uuid) and no test running. The wrapper + // never knows which hook it is in, and does not need to. + const hasNoParent = !this.currentHookRunUuid && !this.currentTestName + await this.performScanCli(browser, command.name, this.currentHookRunUuid, hasNoParent) this.logger.debug(`Accessibility scan performed after ${command.name} command`) } catch (scanError) { this.logger.debug(`Error performing accessibility scan after ${command.name}: ${scanError}`) @@ -339,8 +333,6 @@ export default class AccessibilityModule extends BaseModule { const accessibilityOptions = this.config.accessibilityOptions const shouldScanTest = this.autoScanning && shouldScanTestForAccessibility(suiteTitle, test.title || '', accessibilityOptions as Record | undefined) && this.accessibility - // Window over: the per-test gate below owns the decision from here, tags included. - this.preTestWindowActive = false this.accessibilityMap.set(sessionId, shouldScanTest) // Create test metadata similar to accessibility-handler diff --git a/packages/browserstack-service/tests/accessibility-handler.test.ts b/packages/browserstack-service/tests/accessibility-handler.test.ts index 70a89e5..86a86e6 100644 --- a/packages/browserstack-service/tests/accessibility-handler.test.ts +++ b/packages/browserstack-service/tests/accessibility-handler.test.ts @@ -433,7 +433,7 @@ describe('afterScenario', () => { }) }) -describe('pre-test window (config-level hooks)', () => { +describe('scans ahead of the first test (config-level hooks)', () => { const handlerFor = (framework: string) => { const handler = new AccessibilityHandler(browser, caps, options, false, config, framework, true, false, accessibilityOpts) vi.spyOn(utils, 'isBrowserstackSession').mockReturnValue(true) @@ -441,13 +441,21 @@ describe('pre-test window (config-level hooks)', () => { return handler } + const scanArgs = async (handler: any) => { + vi.spyOn(utils, 'shouldScanTestForAccessibility').mockReturnValue(true) + const scanSpy = vi.spyOn(utils, 'performA11yScan').mockResolvedValue(undefined) + const orig = vi.fn().mockResolvedValue('ok') + await handler['commandWrapper']({ name: 'click', class: 'Element' } as any, undefined as any, orig, 'arg') + expect(scanSpy).toHaveBeenCalled() + return scanSpy.mock.calls[scanSpy.mock.calls.length - 1] + } + it('opens the scan gate in before(), so config-level hook commands are scanned', async () => { const handler = handlerFor('mocha') await handler.before('session-window') expect(AccessibilityHandler['_a11yScanSessionMap']['session-window']).toBe(true) - expect(handler['_preTestWindowActive']).toBe(true) }) it('covers cucumber too', async () => { @@ -455,7 +463,7 @@ describe('pre-test window (config-level hooks)', () => { await handler.before('session-cuke') - expect(handler['_preTestWindowActive']).toBe(true) + expect(AccessibilityHandler['_a11yScanSessionMap']['session-cuke']).toBe(true) }) it('leaves jasmine exactly as it was — App Accessibility is not supported there', async () => { @@ -463,7 +471,7 @@ describe('pre-test window (config-level hooks)', () => { await handler.before('session-jasmine') - expect(handler['_preTestWindowActive']).toBe(false) + expect(AccessibilityHandler['_a11yScanSessionMap']['session-jasmine']).toBeUndefined() }) it('skips multiremote, which has no session id to gate on', async () => { @@ -472,45 +480,45 @@ describe('pre-test window (config-level hooks)', () => { await handler.before('session-multi') - expect(handler['_preTestWindowActive']).toBe(false) + expect(AccessibilityHandler['_a11yScanSessionMap']['session-multi']).toBeUndefined() }) - it('closes the window when a framework hook starts', async () => { + // Stateless rule: parentless only when neither a framework hook run nor a test can own it. + it('sends no test run uuid when no hook run and no test can own the scan', async () => { const handler = handlerFor('mocha') - await handler.before('session-fwhook') - expect(handler['_preTestWindowActive']).toBe(true) + handler['_sessionId'] = 'session-noparent' + await handler.before('session-noparent') + expect((await scanArgs(handler))[7]).toBe(true) + }) + + it('keeps the test run uuid once a framework hook is running', async () => { + const handler = handlerFor('mocha') + handler['_sessionId'] = 'session-fwhook' + await handler.before('session-fwhook') await handler.beforeHook({ title: '"before all" hook', parent: 'suite' } as any, {}, 'hook-uuid-fw') - expect(handler['_preTestWindowActive']).toBe(false) + expect((await scanArgs(handler))[7]).toBe(false) }) - it('leaves a framework-hook scan carrying its test run uuid', async () => { + it('keeps the test run uuid once a test is running', async () => { const handler = handlerFor('mocha') - handler['_sessionId'] = 'session-fwscan' - await handler.before('session-fwscan') - await handler.beforeHook({ title: '"before all" hook', parent: 'suite' } as any, {}, 'hook-uuid-fw2') - - vi.spyOn(utils, 'shouldScanTestForAccessibility').mockReturnValue(true) - const scanSpy = vi.spyOn(utils, 'performA11yScan').mockResolvedValue(undefined) - const orig = vi.fn().mockResolvedValue('ok') - await handler['commandWrapper']({ name: 'click', class: 'Element' } as any, undefined as any, orig, 'arg') + handler['_sessionId'] = 'session-intest' + await handler.before('session-intest') + handler['_testIdentifier'] = 'test-1' - expect(scanSpy).toHaveBeenCalled() - const lastCall = scanSpy.mock.calls[scanSpy.mock.calls.length - 1] - // isGlobalHook (index 7) must be false, so the params helper sends the test run uuid - expect(lastCall[7]).toBe(false) + expect((await scanArgs(handler))[7]).toBe(false) }) - it('closes the window at the first test', async () => { + it('goes back to parentless after the test ends, so a later beforeSuite is not misattributed', async () => { const handler = handlerFor('mocha') - await handler.before('session-close') - expect(handler['_preTestWindowActive']).toBe(true) - - vi.spyOn(utils, 'shouldScanTestForAccessibility').mockReturnValue(true) - await handler.beforeTest('suite', { title: 'test', parent: 'suite' } as any) + handler['_sessionId'] = 'session-between' + await handler.before('session-between') + handler['_testIdentifier'] = 'test-1' + await handler.afterTest('suite', { title: 'test-1' } as any) - expect(handler['_preTestWindowActive']).toBe(false) + expect(handler['_testIdentifier']).toBeNull() + expect((await scanArgs(handler))[7]).toBe(true) }) }) diff --git a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts index f503a77..be8ffc4 100644 --- a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts +++ b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts @@ -162,7 +162,7 @@ describe('AccessibilityModule', () => { }) }) - describe('pre-test window gate', () => { + describe('scan gate ahead of the first test', () => { // afterEach's vi.resetAllMocks() drops the factory's mockReturnValue, so the caps // validators return undefined and onBeforeExecute bails before the gate. Re-arm them. const withA11yCaps = () => { @@ -179,13 +179,17 @@ describe('AccessibilityModule', () => { }) } + const fireWrappedCommand = async () => { + const orig = vi.fn().mockResolvedValue('ok') + await (accessibilityModule as any).commandWrapper({ name: 'click', class: 'Element' }, orig, 'arg') + } + it('opens the scan gate at driver creation, before any test exists', async () => { withA11yCaps() await accessibilityModule.onBeforeExecute() expect(accessibilityModule.accessibilityMap.get('session-w')).toBe(true) - expect(accessibilityModule.preTestWindowActive).toBe(true) }) it('respects autoScanning — the one validation the window still owns', async () => { @@ -195,39 +199,43 @@ describe('AccessibilityModule', () => { await accessibilityModule.onBeforeExecute() expect(accessibilityModule.accessibilityMap.get('session-w')).toBeUndefined() - expect(accessibilityModule.preTestWindowActive).toBe(false) }) - it('closes the window when a framework hook starts, leaving framework hooks untouched', async () => { + // The rule is stateless: a scan is parentless only when no framework hook run and no test + // can own it. These drive the REAL call site (commandWrapper), so they pin the wiring. + it('sends no test run uuid for a scan with no hook run and no test', async () => { withA11yCaps() + accessibilityModule.isAppAccessibility = true await accessibilityModule.onBeforeExecute() - expect(accessibilityModule.preTestWindowActive).toBe(true) - await accessibilityModule.onHookStart({ instance: mockTestInstance }) + await fireWrappedCommand() - expect(accessibilityModule.preTestWindowActive).toBe(false) + expect(_getParamsForAppAccessibility).toHaveBeenCalledWith('click', undefined, null, true) }) - it('keeps the test run uuid on a framework-hook scan', async () => { + it('keeps the test run uuid once a framework hook is running', async () => { withA11yCaps() accessibilityModule.isAppAccessibility = true await accessibilityModule.onBeforeExecute() + vi.mocked(TestFramework.getState).mockReturnValue('hook-uuid-1') await accessibilityModule.onHookStart({ instance: mockTestInstance }) - await (accessibilityModule as any).performScanCli(mockBrowser, 'click', 'hook-uuid-1', accessibilityModule.preTestWindowActive) + await fireWrappedCommand() - // 4th arg false => _getParamsForAppAccessibility keeps process.env.TEST_ANALYTICS_ID - expect(_getParamsForAppAccessibility).toHaveBeenCalledWith('click', undefined, 'hook-uuid-1', false) + const call = vi.mocked(_getParamsForAppAccessibility).mock.calls.at(-1) + expect(call?.[2]).toBe('hook-uuid-1') + expect(call?.[3]).toBe(false) }) - it('closes the window at the first test, handing the gate back to the tag filter', async () => { + it('keeps the test run uuid once a test is running', async () => { withA11yCaps() + accessibilityModule.isAppAccessibility = true await accessibilityModule.onBeforeExecute() - expect(accessibilityModule.preTestWindowActive).toBe(true) + accessibilityModule.currentTestName = 'a test' - await accessibilityModule.onBeforeTest({ suiteTitle: 'suite', test: { title: 'test' } }) + await fireWrappedCommand() - expect(accessibilityModule.preTestWindowActive).toBe(false) + expect(_getParamsForAppAccessibility).toHaveBeenCalledWith('click', 'a test', null, false) }) }) @@ -652,4 +660,4 @@ describe('AccessibilityModule', () => { expect(_getParamsForAppAccessibility).toHaveBeenCalledWith('click', undefined, undefined, undefined) }) }) -}) \ No newline at end of file +}) From 5d7d6ac4805fc9d652abbf15c7be9d32032e9f06 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Tue, 1 Sep 2026 15:12:17 +0530 Subject: [PATCH 5/9] fix(accessibility): latch the parentless window at the first framework hook or test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The all-hooks E2E matrix showed the per-scan rule reaching further than intended. Scans in cfg afterHook, the head of cfg beforeTest, and the trailing cfg afterScenario carried a test uuid on main and lost it here, because at those moments no hook run is active and no test is registered — true of every gap between tests, not just the one before the first. The rule is now latched: _testContextSeen is set at the first framework hook or test of the session and never reset, so only scans that precede anything the framework does are parentless. Everything from the first framework hook onward is byte-for-byte main's behaviour. That also makes the earlier _testIdentifier clear unnecessary, so it is reverted along with its side effect on startA11yScanning's "cannot be started from outside the test" guard. The cost, accepted deliberately: a beforeSuite between suites goes back to inheriting the finished test's uuid, exactly as on main. Verified across the full matrix — app and web, mocha and cucumber, every session-scoped config hook, two describes so beforeSuite fires twice. App mocha: baseline 17 scans, branch 19 — the 17 identical, +2 parentless in cfg before / cfg beforeSuite. SDK-7422 --- .../src/accessibility-handler.ts | 21 +++++++++++-------- .../src/cli/modules/accessibilityModule.ts | 14 ++++++++----- .../tests/accessibility-handler.test.ts | 12 ++++++----- .../cli/modules/accessibilityModule.test.ts | 3 ++- 4 files changed, 30 insertions(+), 20 deletions(-) diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index f5ec12f..b5e5567 100644 --- a/packages/browserstack-service/src/accessibility-handler.ts +++ b/packages/browserstack-service/src/accessibility-handler.ts @@ -88,6 +88,11 @@ class _AccessibilityHandler { // excluded deliberately — App Accessibility is not supported there and its behaviour must // not change; multiremote is excluded in the guard below, having no session id to gate on. private static readonly PRE_TEST_SCAN_FRAMEWORKS = ['mocha', 'cucumber'] + + // Latched at the first framework hook or test of the session and never reset. Before it, a + // scan can only have come from a WDIO config hook; after it everything behaves as it always + // has, so nothing downstream of the first test changes. + private _testContextSeen = false private _platformA11yMeta: PlatformA11yMeta private _caps: Capabilities.ResolvedTestrunnerCapabilities private _suiteFile?: string @@ -328,6 +333,7 @@ class _AccessibilityHandler { async beforeTest (suiteTitle: string | undefined, test: Frameworks.Test) { try { + this._testContextSeen = true if ( !AccessibilityHandler.TEST_HOOK_FRAMEWORKS.includes(this._framework as string) || !this.shouldRunTestHooks(this._browser, this._accessibility) @@ -369,9 +375,6 @@ class _AccessibilityHandler { async afterTest (suiteTitle: string | undefined, test: Frameworks.Test) { BStackLogger.debug('Accessibility after test hook. Before sending test stop event') - // The test is over, so scans after this point have no test to belong to until the next one - // starts. Cleared ahead of the guards below: that is true whatever this method goes on to do. - this._testIdentifier = null if ( !AccessibilityHandler.TEST_HOOK_FRAMEWORKS.includes(this._framework as string) || !this.shouldRunTestHooks(this._browser, this._accessibility) @@ -410,6 +413,7 @@ class _AccessibilityHandler { * Cucumber Only */ async beforeScenario (world: ITestCaseHookParameter) { + this._testContextSeen = true const pickleData = world.pickle const gherkinDocument = world.gherkinDocument const featureData = gherkinDocument.feature @@ -448,8 +452,6 @@ class _AccessibilityHandler { async afterScenario (world: ITestCaseHookParameter) { BStackLogger.debug('Accessibility after scenario hook. Before sending test stop event') - // See afterTest: the scenario is over, so nothing parents a scan until the next one starts. - this._testIdentifier = null if (!this.shouldRunTestHooks(this._browser, this._accessibility)) { return } @@ -492,6 +494,7 @@ class _AccessibilityHandler { */ async beforeHook (test: Frameworks.Test | undefined, context: unknown, hookRunUuid?: string | null) { try { + this._testContextSeen = true if (!this._accessibility || !this.shouldRunTestHooks(this._browser, this._accessibility)) { return } @@ -540,10 +543,10 @@ class _AccessibilityHandler { ) ) { BStackLogger.debug(`Performing scan for ${command.class} ${command.name}`) - // A scan belongs to no test only when nothing can parent it: no framework hook run - // (those are reported to TRA and keep their test uuid) and no test started. That is a - // property of the moment, not of which hook is running — the wrapper never knows that. - const hasNoParent = !this._currentHookRunUuid && this._testIdentifier === null + // Parentless only before the framework has started anything: no hook run to own the + // scan and no test seen yet in this session. Once either has happened the latch stays + // set, so every later hook keeps the attribution it has always had. + const hasNoParent = !this._currentHookRunUuid && !this._testContextSeen await performA11yScan(this.isAppAutomate, this._browser, true, true, command.name, undefined, this._currentHookRunUuid, hasNoParent) } 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 d93003b..29a973e 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -27,6 +27,8 @@ export default class AccessibilityModule extends BaseModule { scriptInstance: typeof accessibilityScripts accessibility: boolean = false autoScanning: boolean = true + // Latched at the first framework hook or test and never reset — see the classic handler. + testContextSeen: boolean = false isAppAccessibility: boolean isNonBstackA11y: boolean accessibilityConfig: Accessibility @@ -76,6 +78,7 @@ export default class AccessibilityModule extends BaseModule { // capture on autoInstance would silently drop app hook-scan stamping. const hookRunUuid = testInstance ? (TestFramework.getState(testInstance, TestFrameworkConstants.KEY_HOOK_ID) as string | undefined) : undefined this.currentHookRunUuid = hookRunUuid || null + this.testContextSeen = true const autoInstance: AutomationFrameworkInstance = AutomationFramework.getTrackedInstance() if (!testInstance || !autoInstance) { @@ -216,7 +219,7 @@ export default class AccessibilityModule extends BaseModule { return } // If invoked from inside a hook, currentHookRunUuid stamps the scan for the hook. - return await this.performScanCli(browser, undefined, this.currentHookRunUuid, !this.currentHookRunUuid && !this.currentTestName) + return await this.performScanCli(browser, undefined, this.currentHookRunUuid, !this.currentHookRunUuid && !this.testContextSeen) } (browser as WebdriverIO.Browser).startA11yScanning = async () => { @@ -295,10 +298,10 @@ export default class AccessibilityModule extends BaseModule { !this.shouldPatchExecuteScript(args.length ? args[0] as string : null) ) { try { - // Parentless only when nothing can own the scan: no framework hook run - // (reported to TRA, keeps its test uuid) and no test running. The wrapper - // never knows which hook it is in, and does not need to. - const hasNoParent = !this.currentHookRunUuid && !this.currentTestName + // Parentless only before the framework has started anything: no hook run + // and no test seen yet. The wrapper never knows which hook it is in, and + // does not need to. + const hasNoParent = !this.currentHookRunUuid && !this.testContextSeen await this.performScanCli(browser, command.name, this.currentHookRunUuid, hasNoParent) this.logger.debug(`Accessibility scan performed after ${command.name} command`) } catch (scanError) { @@ -326,6 +329,7 @@ export default class AccessibilityModule extends BaseModule { const test = (args.test && typeof args.test === 'object' ? args.test as { title?: string } : {}) || {} this.currentTestName = test.title || null + this.testContextSeen = true const autoInstance: AutomationFrameworkInstance = AutomationFramework.getTrackedInstance() const testInstance: TestFrameworkInstance = TestFramework.getTrackedInstance() diff --git a/packages/browserstack-service/tests/accessibility-handler.test.ts b/packages/browserstack-service/tests/accessibility-handler.test.ts index 86a86e6..4a74ad0 100644 --- a/packages/browserstack-service/tests/accessibility-handler.test.ts +++ b/packages/browserstack-service/tests/accessibility-handler.test.ts @@ -505,20 +505,22 @@ describe('scans ahead of the first test (config-level hooks)', () => { const handler = handlerFor('mocha') handler['_sessionId'] = 'session-intest' await handler.before('session-intest') - handler['_testIdentifier'] = 'test-1' + vi.spyOn(utils, 'shouldScanTestForAccessibility').mockReturnValue(true) + await handler.beforeTest('suite', { title: 'test-1' } as any) expect((await scanArgs(handler))[7]).toBe(false) }) - it('goes back to parentless after the test ends, so a later beforeSuite is not misattributed', async () => { + it('stays attributed after the first test, so later hooks behave exactly as before', async () => { const handler = handlerFor('mocha') handler['_sessionId'] = 'session-between' await handler.before('session-between') - handler['_testIdentifier'] = 'test-1' + vi.spyOn(utils, 'shouldScanTestForAccessibility').mockReturnValue(true) + await handler.beforeTest('suite', { title: 'test-1' } as any) await handler.afterTest('suite', { title: 'test-1' } as any) - expect(handler['_testIdentifier']).toBeNull() - expect((await scanArgs(handler))[7]).toBe(true) + // the latch never resets: a scan between tests keeps the attribution main gives it + expect((await scanArgs(handler))[7]).toBe(false) }) }) diff --git a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts index be8ffc4..34842a1 100644 --- a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts +++ b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts @@ -231,7 +231,8 @@ describe('AccessibilityModule', () => { withA11yCaps() accessibilityModule.isAppAccessibility = true await accessibilityModule.onBeforeExecute() - accessibilityModule.currentTestName = 'a test' + vi.mocked(shouldScanTestForAccessibility).mockReturnValue(true) + await accessibilityModule.onBeforeTest({ suiteTitle: 'suite', test: { title: 'a test' } }) await fireWrappedCommand() From 027b140789f4c06855e8c52b30df3b8a8d0562f7 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Tue, 1 Sep 2026 18:47:25 +0530 Subject: [PATCH 6/9] fix(accessibility): stop tearing the scan gate down after every test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit onAfterTest deleted the session's entry from accessibilityMap, and only the next test's onBeforeTest re-created it. Nothing scanned in the gap: afterSuite and after went unscanned on this flow for the life of the session, and a driver command in either was invisible. The classic flow never deleted, so the same customer code scanned on cucumber and was silently skipped on mocha. This removes the delete rather than forcing the entry true, so a tag-excluded test and a user's stopA11yScanning() still decide what happens next. Post-test scans carry the last test's uuid, which is what the classic flow has always done for cucumber's after hooks. Verified on device: app mocha now scans cfg afterSuite and cfg after (21 scans, up from 19), both stamped with the finished test's uuid. Web is unchanged at 3 scans — back() is not a wrapped command there — so nothing is gained or lost on that path. The test drives onAfterTest to completion, stubbing the stop-event internals that throw against these mocks; without that the assertion passes with the delete restored and proves nothing. SDK-7422 --- .../src/cli/modules/accessibilityModule.ts | 7 +++++-- .../cli/modules/accessibilityModule.test.ts | 21 +++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts index 29a973e..e1c2b55 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -412,7 +412,6 @@ export default class AccessibilityModule extends BaseModule { const autoInstance: AutomationFrameworkInstance = AutomationFramework.getTrackedInstance() const testInstance: TestFrameworkInstance = TestFramework.getTrackedInstance() - const sessionId = AutomationFramework.getState(autoInstance, AutomationFrameworkConstants.KEY_FRAMEWORK_SESSION_ID) if (!autoInstance || !testInstance) { this.logger.error('No tracked instances found for accessibility after test') @@ -456,7 +455,11 @@ export default class AccessibilityModule extends BaseModule { } else { this.logger.warn('No driver found to send accessibility test stop event') } - this.accessibilityMap.delete(sessionId) + // The gate deliberately stays as the test left it. Deleting it here stopped every + // scan between tests — afterSuite, after, and the next suite's beforeSuite all went + // unscanned on this flow, while the classic flow (which never deleted) scanned them. + // Leaving the entry rather than forcing it true keeps a tag-excluded test and a + // user's stopA11yScanning() in effect. // Clean up test metadata TestFramework.setState(testInstance, `accessibility_metadata_${testIdentifier}`, null) diff --git a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts index 34842a1..e4776bd 100644 --- a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts +++ b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts @@ -227,6 +227,27 @@ describe('AccessibilityModule', () => { expect(call?.[3]).toBe(false) }) + it('leaves the gate open after a test, so afterSuite/after still scan', async () => { + withA11yCaps() + vi.mocked(shouldScanTestForAccessibility).mockReturnValue(true) + await accessibilityModule.onBeforeExecute() + await accessibilityModule.onBeforeTest({ suiteTitle: 'suite', test: { title: 'a test' } }) + expect(accessibilityModule.accessibilityMap.get('session-w')).toBe(true) + + // drive onAfterTest all the way to the end: its guards, and then the stop-event + // internals, which throw against these mocks and would swallow the line under test + vi.mocked(mockTestInstance.getData).mockReturnValue({ + accessibilityScanStarted: true, + scanTestForAccessibility: true + }) + vi.spyOn(accessibilityModule as any, 'getDriverExecuteParams').mockResolvedValue({}) + vi.spyOn(accessibilityModule as any, 'sendTestStopEvent').mockResolvedValue(undefined) + await accessibilityModule.onAfterTest() + + // deleting it here used to silence every scan between tests and after the last one + expect(accessibilityModule.accessibilityMap.get('session-w')).toBe(true) + }) + it('keeps the test run uuid once a test is running', async () => { withA11yCaps() accessibilityModule.isAppAccessibility = true From c6591a907e3048d28d026e024e290e73501e978f Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Tue, 1 Sep 2026 21:00:35 +0530 Subject: [PATCH 7/9] fix(accessibility): apply the parentage rule to manual performScan() too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding: the auto path got the rule on both flows, but the classic user-facing browser.performScan() still called with four arguments, so hookRunUuid and isGlobalHook both arrived undefined and the scan carried TEST_ANALYTICS_ID. The CLI equivalent was threaded. Same customer code — performScan() from a config-level before() — was parentless on mocha and misattributed on cucumber, and the manual path is the only route to a config-hook scan for a setup that issues no wrapped command. The rule is now a single getter per flow, used at every scan site: the auto path, the manual performScan(), and the CLI's per-test re-patch, which passed the hook uuid but not the flag. That last one was harmless — it is installed after the latch sets, so undefined and false agreed — but it would have diverged silently the moment the rule changed. Threading _currentHookRunUuid into the classic manual path also closes a pre-existing gap the reviewer noted: a manual scan inside a framework hook used to land as a NULL hook row. Two tests, both failing if the manual path is un-threaded: a manual scan from a config hook is parentless, and one inside a framework hook carries that hook's uuid. SDK-7422 --- .../src/accessibility-handler.ts | 13 +++++++--- .../src/cli/modules/accessibilityModule.ts | 12 ++++++--- .../tests/accessibility-handler.test.ts | 26 +++++++++++++++++++ 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index b5e5567..c4588ca 100644 --- a/packages/browserstack-service/src/accessibility-handler.ts +++ b/packages/browserstack-service/src/accessibility-handler.ts @@ -252,7 +252,9 @@ class _AccessibilityHandler { } browserWithA11y.performScan = async () => { - const results = await performA11yScan(this.isAppAutomate, (this._browser as WebdriverIO.Browser), isBrowserstackSession(this._browser), this._accessibility) + // Same parentage rule as the auto path, and the hook uuid the manual path never carried + // — a manual scan inside a framework hook used to land as a NULL hook row. + const results = await performA11yScan(this.isAppAutomate, (this._browser as WebdriverIO.Browser), isBrowserstackSession(this._browser), this._accessibility, undefined, undefined, this._currentHookRunUuid, this.hasNoParent) if (results) { this._testMetadata[this._testIdentifier as string] = { scanTestForAccessibility : true, @@ -326,6 +328,12 @@ class _AccessibilityHandler { } + // Nothing can own a scan before the framework has started anything. Defined once: the auto + // path and the user-facing performScan() must not answer this differently. + private get hasNoParent(): boolean { + return !this._currentHookRunUuid && !this._testContextSeen + } + private supportsPreTestWindow(): boolean { return AccessibilityHandler.PRE_TEST_SCAN_FRAMEWORKS.includes(this._framework as string) && !this._browser?.isMultiremote @@ -546,8 +554,7 @@ class _AccessibilityHandler { // Parentless only before the framework has started anything: no hook run to own the // scan and no test seen yet in this session. Once either has happened the latch stays // set, so every later hook keeps the attribution it has always had. - const hasNoParent = !this._currentHookRunUuid && !this._testContextSeen - await performA11yScan(this.isAppAutomate, this._browser, true, true, command.name, undefined, this._currentHookRunUuid, hasNoParent) + await performA11yScan(this.isAppAutomate, this._browser, true, true, command.name, undefined, this._currentHookRunUuid, this.hasNoParent) } 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 e1c2b55..15324d3 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -219,7 +219,7 @@ export default class AccessibilityModule extends BaseModule { return } // If invoked from inside a hook, currentHookRunUuid stamps the scan for the hook. - return await this.performScanCli(browser, undefined, this.currentHookRunUuid, !this.currentHookRunUuid && !this.testContextSeen) + return await this.performScanCli(browser, undefined, this.currentHookRunUuid, this.hasNoParent) } (browser as WebdriverIO.Browser).startA11yScanning = async () => { @@ -301,8 +301,7 @@ export default class AccessibilityModule extends BaseModule { // Parentless only before the framework has started anything: no hook run // and no test seen yet. The wrapper never knows which hook it is in, and // does not need to. - const hasNoParent = !this.currentHookRunUuid && !this.testContextSeen - await this.performScanCli(browser, command.name, this.currentHookRunUuid, hasNoParent) + await this.performScanCli(browser, command.name, this.currentHookRunUuid, this.hasNoParent) this.logger.debug(`Accessibility scan performed after ${command.name} command`) } catch (scanError) { this.logger.debug(`Error performing accessibility scan after ${command.name}: ${scanError}`) @@ -375,7 +374,7 @@ export default class AccessibilityModule extends BaseModule { if (!this.accessibility && !this.isAppAccessibility){ return } - const results = await this.performScanCli(browser, undefined, this.currentHookRunUuid) + const results = await this.performScanCli(browser, undefined, this.currentHookRunUuid, this.hasNoParent) if (results){ const testIdentifier = String(testInstance.getContext().getId()) this.testMetadata[testIdentifier] = { @@ -519,6 +518,11 @@ export default class AccessibilityModule extends BaseModule { return false } + // See the classic handler: one definition, used at every scan site. + private get hasNoParent(): boolean { + return !this.currentHookRunUuid && !this.testContextSeen + } + private async performScanCli( browser: WebdriverIO.Browser | WebdriverIO.MultiRemoteBrowser, commandName?: string, diff --git a/packages/browserstack-service/tests/accessibility-handler.test.ts b/packages/browserstack-service/tests/accessibility-handler.test.ts index 4a74ad0..8f2f692 100644 --- a/packages/browserstack-service/tests/accessibility-handler.test.ts +++ b/packages/browserstack-service/tests/accessibility-handler.test.ts @@ -492,6 +492,32 @@ describe('scans ahead of the first test (config-level hooks)', () => { expect((await scanArgs(handler))[7]).toBe(true) }) + it('sends no test run uuid for a MANUAL performScan() from a config hook', async () => { + const handler = handlerFor('mocha') + handler['_sessionId'] = 'session-manual' + const scanSpy = vi.spyOn(utils, 'performA11yScan').mockResolvedValue(undefined) + await handler.before('session-manual') + + await (browser as any).performScan() + + const call = scanSpy.mock.calls[scanSpy.mock.calls.length - 1] + expect(call[7]).toBe(true) + }) + + it('gives a MANUAL performScan() inside a framework hook its hook uuid', async () => { + const handler = handlerFor('mocha') + handler['_sessionId'] = 'session-manual-hook' + const scanSpy = vi.spyOn(utils, 'performA11yScan').mockResolvedValue(undefined) + await handler.before('session-manual-hook') + await handler.beforeHook({ title: '"before all" hook', parent: 'suite' } as any, {}, 'hook-uuid-manual') + + await (browser as any).performScan() + + const call = scanSpy.mock.calls[scanSpy.mock.calls.length - 1] + expect(call[6]).toBe('hook-uuid-manual') + expect(call[7]).toBe(false) + }) + it('keeps the test run uuid once a framework hook is running', async () => { const handler = handlerFor('mocha') handler['_sessionId'] = 'session-fwhook' From 02b1d7a36d14856261ab9b453f79079b408e7ac5 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 15:31:16 +0000 Subject: [PATCH 8/9] chore(changeset): auto-generate from PR template (minor) --- .changeset/pr-176.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.changeset/pr-176.md b/.changeset/pr-176.md index 0592c0a..c0ee13a 100644 --- a/.changeset/pr-176.md +++ b/.changeset/pr-176.md @@ -2,4 +2,5 @@ "@wdio/browserstack-service": minor --- -- Accessibility scans now run for driver commands issued from your WDIO config's `before()` and `beforeSuite` hooks, so screens visited during setup are covered. +- Accessibility scans now run for driver commands issued from your WDIO config's hooks — `before()` and `beforeSuite` before the run starts, and `afterSuite`/`after` during teardown — so screens visited outside your tests are covered. +- `browser.performScan()` called from a config hook is no longer attributed to a test that has not started. From 64e8682569706fb633c7de3aa4375e9edaf61190 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Tue, 1 Sep 2026 21:07:03 +0530 Subject: [PATCH 9/9] fix(accessibility): do not attempt a scan once the session has ended MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer question on the gate-teardown change: with the map entry outliving the last test, does a wrapped command in teardown turn a silent no-op into a logged failure? It does. Reproduced with a driver command in afterSession, which runs after deleteSession: ERROR @wdio/browserstack-service/cli: Accessibility Scan could not be performed : Error: A sessionId is required for this command The customer's own command fails identically a moment later, so nothing breaks — but main was silent there and this is an error-level line in ordinary teardown, which is support-ticket material. Both wrappers now check the one thing that actually knows whether the session is alive — the driver's sessionId — and skip the scan without touching the command. afterSuite and after still scan, since those run before the session is deleted. Verified on device: the error line is gone, the customer command still throws as it does on main, and the all-hooks matrix is unchanged at 21 scans with cfg afterSuite and cfg after present. The CLI mock browser gained a sessionId, since it was modelling a dead session and three tests were passing for the wrong reason. SDK-7422 --- .../src/accessibility-handler.ts | 8 +++++++- .../src/cli/modules/accessibilityModule.ts | 9 +++++++++ .../tests/cli/modules/accessibilityModule.test.ts | 15 +++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index c4588ca..1055bb5 100644 --- a/packages/browserstack-service/src/accessibility-handler.ts +++ b/packages/browserstack-service/src/accessibility-handler.ts @@ -554,7 +554,13 @@ class _AccessibilityHandler { // Parentless only before the framework has started anything: no hook run to own the // scan and no test seen yet in this session. Once either has happened the latch stays // set, so every later hook keeps the attribution it has always had. - await performA11yScan(this.isAppAutomate, this._browser, true, true, command.name, undefined, this._currentHookRunUuid, this.hasNoParent) + // See the CLI module: the gate outlives the session now, and a scan attempted after + // the session is gone logs an error where main was silent. + if (!(this._browser as WebdriverIO.Browser)?.sessionId) { + BStackLogger.debug('Skipping accessibility scan: the session has ended') + } else { + await performA11yScan(this.isAppAutomate, this._browser, true, true, command.name, undefined, this._currentHookRunUuid, this.hasNoParent) + } } 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 15324d3..6c38c7e 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -292,6 +292,15 @@ export default class AccessibilityModule extends BaseModule { if (sessionId && this.accessibilityMap.get(sessionId)) { const browser = AutomationFramework.getDriver(autoInstance) as WebdriverIO.Browser + // The gate now outlives the last test, so a command issued once the session has + // been deleted (afterSession) would reach the scan and fail with "A sessionId is + // required for this command" — an error log where there used to be silence. The + // driver is the only thing that knows: no sessionId, no session. + if (!browser?.sessionId) { + this.logger.debug('Skipping accessibility scan: the session has ended') + return await originFunction(...args) + } + // Perform accessibility scan before command if script is available if ( !command.name.includes('execute') || diff --git a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts index e4776bd..11cf8a9 100644 --- a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts +++ b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts @@ -77,6 +77,8 @@ describe('AccessibilityModule', () => { } mockBrowser = { + // a live session: the wrapper skips scanning when the driver has no sessionId + sessionId: 'session-w', executeAsync: vi.fn().mockResolvedValue([]), execute: vi.fn().mockResolvedValue({}), overwriteCommand: vi.fn() @@ -227,6 +229,19 @@ describe('AccessibilityModule', () => { expect(call?.[3]).toBe(false) }) + it('skips the scan once the session is gone, instead of logging a failure', async () => { + withA11yCaps() + await accessibilityModule.onBeforeExecute() + const orig = vi.fn().mockResolvedValue('ok') + mockBrowser.sessionId = undefined + + await (accessibilityModule as any).commandWrapper({ name: 'click', class: 'Element' }, orig, 'arg') + + // the command still runs; only the scan is skipped + expect(orig).toHaveBeenCalled() + expect(_getParamsForAppAccessibility).not.toHaveBeenCalled() + }) + it('leaves the gate open after a test, so afterSuite/after still scan', async () => { withA11yCaps() vi.mocked(shouldScanTestForAccessibility).mockReturnValue(true)