fix(app-a11y): keep scanning across browser.reloadSession()

.changeset/pr-165.md

@@ -0,0 +1,5 @@
+---
+"@wdio/browserstack-service": patch
+---
+
+- Fixed accessibility scanning stopping for the remainder of a test after `browser.reloadSession()`.

packages/browserstack-service/src/accessibility-handler.ts

@@ -166,6 +166,28 @@ class _AccessibilityHandler {
         }
     }
 
+    /**
+     * A reload replaces the session id under a driver object that otherwise carries on
+     * unchanged. `_sessionId` is captured once in `before()`, so without this every later
+     * scan-gate lookup and every results call keeps addressing the session that already ended.
+     */
+    onSessionReload(oldSessionId: string, newSessionId: string) {
+        try {
+            if (!newSessionId || oldSessionId === newSessionId) {
+                return
+            }
+            if (oldSessionId in AccessibilityHandler._a11yScanSessionMap) {
+                AccessibilityHandler._a11yScanSessionMap[newSessionId] = AccessibilityHandler._a11yScanSessionMap[oldSessionId]
+                delete AccessibilityHandler._a11yScanSessionMap[oldSessionId]
+            }
+            if (this._sessionId === oldSessionId) {
+                this._sessionId = newSessionId
+            }
+        } catch (error) {
+            BStackLogger.debug(`Exception while migrating accessibility state across session reload: ${error}`)
+        }
+    }
+
     async before(sessionId: string) {
         PerformanceTester.start(PERFORMANCE_SDK_EVENTS.CONFIG_EVENTS.ACCESSIBILITY)
 
@@ -237,7 +259,10 @@ class _AccessibilityHandler {
                 BStackLogger.warn('Accessibility scanning cannot be started from outside the test')
                 return
             }
-            AccessibilityHandler._a11yScanSessionMap[sessionId] = true
+            // `this._sessionId` rather than the captured `sessionId`: commandWrapper reads the
+            // former, and a reload moves it, so writing the captured id would leave the user's
+            // start/stop addressing a session that has ended.
+            AccessibilityHandler._a11yScanSessionMap[this._sessionId ?? sessionId] = true
             this._testMetadata[this._testIdentifier as string] = {
                 scanTestForAccessibility : true,
                 accessibilityScanStarted : true
@@ -250,7 +275,7 @@ class _AccessibilityHandler {
                 BStackLogger.warn('Accessibility scanning cannot be stopped from outside the test')
                 return
             }
-            AccessibilityHandler._a11yScanSessionMap[sessionId] = false
+            AccessibilityHandler._a11yScanSessionMap[this._sessionId ?? sessionId] = false
             await this._setAnnotation('Accessibility scanning has stopped')
         }
 

packages/browserstack-service/src/cli/modules/accessibilityModule.ts

@@ -31,8 +31,8 @@ export default class AccessibilityModule extends BaseModule {
     isNonBstackA11y: boolean
     accessibilityConfig: Accessibility
     static MODULE_NAME = 'AccessibilityModule'
-    accessibilityMap: Map<number, boolean>
-    LOG_DISABLED_SHOWN: Map<number, boolean>
+    accessibilityMap: Map<string, boolean>
+    LOG_DISABLED_SHOWN: Map<string, boolean>
     testMetadata: Record<string, { [key: string]: unknown; }> = {}
     currentTestName: string | null = null
     // The run uuid of the hook currently executing (set at hook PRE, cleared at hook POST).
@@ -102,6 +102,56 @@ export default class AccessibilityModule extends BaseModule {
         this.currentHookRunUuid = null
     }
 
+    /**
+     * 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
+     * is keyed on the session id, so the entry registered for the old id is orphaned the moment
+     * the reload lands: every command issued for the rest of that test looks up a key that does
+     * not exist and is silently not scanned. Nothing re-registers until the NEXT onBeforeTest,
+     * so a test that reloads mid-way loses all coverage after the reload.
+     *
+     * Migrating the entry rather than re-deriving it preserves whatever the gate currently says,
+     * including a stopA11yScanning() the user called before reloading.
+     */
+    onSessionReload(oldSessionId: string, newSessionId: string) {
+        try {
+            if (!oldSessionId || !newSessionId || oldSessionId === newSessionId) {
+                return
+            }
+
+            if (this.accessibilityMap.has(oldSessionId)) {
+                this.accessibilityMap.set(newSessionId, this.accessibilityMap.get(oldSessionId) as boolean)
+                this.accessibilityMap.delete(oldSessionId)
+                this.logger.debug(`Accessibility scan gate migrated across session reload to ${newSessionId}`)
+            }
+            if (this.LOG_DISABLED_SHOWN.has(oldSessionId)) {
+                this.LOG_DISABLED_SHOWN.set(newSessionId, this.LOG_DISABLED_SHOWN.get(oldSessionId) as boolean)
+                this.LOG_DISABLED_SHOWN.delete(oldSessionId)
+            }
+        } catch (error) {
+            this.logger.error(`Exception in accessibility onSessionReload: ${error}`)
+        }
+    }
+
+    /**
+     * The session id as of RIGHT NOW, read from framework state rather than captured.
+     *
+     * Anything installed on the driver — the scanning toggles, the results getters — outlives the
+     * session it was created in, because `browser.reloadSession()` swaps the session underneath a
+     * driver object that carries on unchanged. A captured id makes those closures address a
+     * session that has ended, while `commandWrapper` re-reads state and addresses the live one.
+     */
+    private currentSessionId(): string | undefined {
+        try {
+            const autoInstance: AutomationFrameworkInstance = AutomationFramework.getTrackedInstance()
+            return autoInstance
+                ? AutomationFramework.getState(autoInstance, AutomationFrameworkConstants.KEY_FRAMEWORK_SESSION_ID) as string
+                : undefined
+        } catch {
+            return undefined
+        }
+    }
+
     async onBeforeExecute() {
         try {
             const autoInstance: AutomationFrameworkInstance = AutomationFramework.getTrackedInstance()
@@ -121,7 +171,6 @@ export default class AccessibilityModule extends BaseModule {
             const isBrowserstackSession = AutomationFramework.getState(autoInstance, AutomationFrameworkConstants.KEY_IS_BROWSERSTACK_HUB)
             const browserCaps = AutomationFramework.getState(autoInstance, AutomationFrameworkConstants.KEY_CAPABILITIES)
             const inputCaps = AutomationFramework.getState(autoInstance, AutomationFrameworkConstants.KEY_INPUT_CAPABILITIES)
-            const sessionId = AutomationFramework.getState(autoInstance, AutomationFrameworkConstants.KEY_FRAMEWORK_SESSION_ID)
             const platformA11yMeta = {
                 browser_name: browserCaps.browserName,
                 browser_version: browserCaps?.browserVersion || 'latest',
@@ -148,15 +197,15 @@ export default class AccessibilityModule extends BaseModule {
             //patching getA11yResultsSummary
             (browser as WebdriverIO.Browser).getAccessibilityResultsSummary = async () => {
                 if (this.isAppAccessibility) {
-                    return await getAppA11yResultsSummary(true, browser, this.currentTestName, isBrowserstackSession, this.accessibility, sessionId)
+                    return await getAppA11yResultsSummary(true, browser, this.currentTestName, isBrowserstackSession, this.accessibility, this.currentSessionId())
                 }
                 return await this.getA11yResultsSummary(browser)
             }
 
             //patching getA11yResults
             (browser as WebdriverIO.Browser).getAccessibilityResults = async () => {
                 if (this.isAppAccessibility) {
-                    return await getAppA11yResults(true, browser, this.currentTestName, isBrowserstackSession, this.accessibility, sessionId)
+                    return await getAppA11yResults(true, browser, this.currentTestName, isBrowserstackSession, this.accessibility, this.currentSessionId())
                 }
                 return await this.getA11yResults(browser)
             }
@@ -282,7 +331,9 @@ export default class AccessibilityModule extends BaseModule {
                 if (!this.accessibility && !this.isAppAccessibility){
                     return
                 }
-                this.accessibilityMap.set(sessionId, true)
+                // Resolved at call time: a reload mid-test moves the gate to a new key, and
+                // writing to the captured one would leave the user's start/stop with no effect.
+                this.accessibilityMap.set(this.currentSessionId() ?? sessionId, true)
                 this.testMetadata[testIdentifier] = {
                     scanTestForAccessibility : true,
                     accessibilityScanStarted : true
@@ -295,7 +346,7 @@ export default class AccessibilityModule extends BaseModule {
                 if (!this.accessibility && !this.isAppAccessibility){
                     return
                 }
-                this.accessibilityMap.set(sessionId, false)
+                this.accessibilityMap.set(this.currentSessionId() ?? sessionId, false)
                 await this._setAnnotation('Accessibility scanning has stopped')
             }
 

packages/browserstack-service/src/service.ts

@@ -46,6 +46,7 @@ import { AutomationFrameworkConstants } from './cli/frameworks/constants/automat
 import TestFramework from './cli/frameworks/testFramework.js'
 import { TestFrameworkState } from './cli/states/testFrameworkState.js'
 import { TestFrameworkConstants } from './cli/frameworks/constants/testFrameworkConstants.js'
+import AccessibilityModule from './cli/modules/accessibilityModule.js'
 
 import util from 'node:util'
 
@@ -288,7 +289,7 @@ export default class BrowserstackService implements Services.ServiceInstance {
                         this._options.accessibilityOptions
                     )
 
-                    if (isBrowserstackSession(this._browser) && BrowserstackCLI.getInstance().isRunning()){
+                    if (this._isCliAccessibilityFlow()){
                         BStackLogger.info(`CLI is running, tracking accessibility event for before: ${sessionId}`)
                         // BrowserstackCLI.getInstance().getTestFramework()!.trackEvent(AutomationFrameworkState.CREATE, HookState.POST, { sessionId })
                     } else {
@@ -884,6 +885,26 @@ export default class BrowserstackService implements Services.ServiceInstance {
         await this._insightsHandler?.afterStep(step, scenario, result)
     }
 
+    /**
+     * Whether accessibility is being driven by the binary rather than the classic handler.
+     *
+     * 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) {
@@ -896,6 +917,18 @@ export default class BrowserstackService implements Services.ServiceInstance {
             if (instance) {
                 AutomationFramework.setState(instance, AutomationFrameworkConstants.KEY_FRAMEWORK_SESSION_ID, newSessionId)
             }
+            // Carry the accessibility scan gate over to the new session id. Updating the state
+            // above without this is what orphans it: the gate is keyed on the session id, so the
+            // rest of the reloading test would go unscanned.
+            const accessibilityModule = BrowserstackCLI.getInstance().modules?.[AccessibilityModule.MODULE_NAME] as AccessibilityModule | undefined
+            accessibilityModule?.onSessionReload(oldSessionId, newSessionId)
+        }
+
+        // The classic handler holds state exactly when its `before()` ran, which is the inverse of
+        // _isCliAccessibilityFlow() — not of isRunning(). Gating on the same predicate as that
+        // call site is what keeps the binary-up-but-non-BrowserStack-provider case covered.
+        if (!this._isCliAccessibilityFlow()) {
+            this._accessibilityHandler?.onSessionReload(oldSessionId, newSessionId)
         }
 
         const { setSessionName, setSessionStatus } = this._options

packages/browserstack-service/tests/accessibility-handler.test.ts

@@ -689,6 +689,49 @@ describe('afterTest', () => {
     })
 })
 
+describe('onSessionReload', () => {
+    beforeEach(() => {
+        accessibilityHandler = new AccessibilityHandler(browser, caps, options, false, config, 'framework', true, false, accessibilityOpts)
+    })
+
+    it('moves the scan flag and the tracked session id onto the reloaded session', () => {
+        accessibilityHandler['_sessionId'] = 'old-session'
+        AccessibilityHandler['_a11yScanSessionMap']['old-session'] = true
+
+        accessibilityHandler.onSessionReload('old-session', 'new-session')
+
+        expect(AccessibilityHandler['_a11yScanSessionMap']['new-session']).toBe(true)
+        expect(AccessibilityHandler['_a11yScanSessionMap']['old-session']).toBeUndefined()
+        expect(accessibilityHandler['_sessionId']).toBe('new-session')
+    })
+
+    it('makes the scanning toggles address the reloaded session', async () => {
+        // commandWrapper reads `this._sessionId`, so a toggle writing the id captured in before()
+        // would set the dead key — stop would not stop, start would not start.
+        // before() is what installs the toggles on the driver, so it has to run first
+        vi.spyOn(utils, 'isBrowserstackSession').mockReturnValue(true)
+        await accessibilityHandler.before('old-session')
+        accessibilityHandler['_testIdentifier'] = 'test-1'
+        AccessibilityHandler['_a11yScanSessionMap']['old-session'] = true
+
+        accessibilityHandler.onSessionReload('old-session', 'new-session')
+        const browserWithA11y = browser as unknown as { stopA11yScanning: () => Promise<void> }
+        await browserWithA11y.stopA11yScanning()
+
+        expect(AccessibilityHandler['_a11yScanSessionMap']['new-session']).toBe(false)
+    })
+
+    it('is a no-op for a reload that changes nothing', () => {
+        accessibilityHandler['_sessionId'] = 'same'
+        AccessibilityHandler['_a11yScanSessionMap']['same'] = true
+
+        accessibilityHandler.onSessionReload('same', 'same')
+
+        expect(AccessibilityHandler['_a11yScanSessionMap']['same']).toBe(true)
+        expect(accessibilityHandler['_sessionId']).toBe('same')
+    })
+})
+
 describe('getIdentifier', () => {
     let getUniqueIdentifierSpy: any
     let getUniqueIdentifierForCucumberSpy: any

packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts

@@ -36,7 +36,8 @@ vi.mock('../../../src/util.js', () => ({
     getAppA11yResultsSummary: vi.fn().mockResolvedValue({}),
     _getParamsForAppAccessibility: vi.fn().mockReturnValue('{}'),
     formatString: vi.fn().mockReturnValue('formatted-script'),
-    o11yClassErrorHandler: vi.fn().mockImplementation((cls) => cls)
+    o11yClassErrorHandler: vi.fn().mockImplementation((cls) => cls),
+    isBrowserstackSession: vi.fn().mockReturnValue(true)
 }))
 
 vi.mock('../../../src/cli/grpcClient.js', () => ({
@@ -51,7 +52,7 @@ vi.mock('../../../src/cli/grpcClient.js', () => ({
 }))
 
 import AccessibilityModule from '../../../src/cli/modules/accessibilityModule.js'
-import { validateCapsWithA11y, validateCapsWithAppA11y, _getParamsForAppAccessibility } from '../../../src/util.js'
+import { validateCapsWithA11y, validateCapsWithAppA11y, _getParamsForAppAccessibility, shouldScanTestForAccessibility } from '../../../src/util.js'
 import accessibilityScripts from '../../../src/scripts/accessibility-scripts.js'
 import TestFramework from '../../../src/cli/frameworks/testFramework.js'
 import AutomationFramework from '../../../src/cli/frameworks/automationFramework.js'
@@ -199,6 +200,73 @@ describe('AccessibilityModule', () => {
         })
     })
 
+    describe('onSessionReload', () => {
+        it('carries the scan gate over to the new session id', () => {
+            accessibilityModule.accessibilityMap.set('old', true)
+
+            accessibilityModule.onSessionReload('old', 'new')
+
+            expect(accessibilityModule.accessibilityMap.get('new')).toBe(true)
+            expect(accessibilityModule.accessibilityMap.has('old')).toBe(false)
+        })
+
+        it('preserves a gate the user had closed with stopA11yScanning', () => {
+            accessibilityModule.accessibilityMap.set('old', false)
+
+            accessibilityModule.onSessionReload('old', 'new')
+
+            expect(accessibilityModule.accessibilityMap.get('new')).toBe(false)
+        })
+
+        it('does nothing when the old session was never registered', () => {
+            accessibilityModule.onSessionReload('old', 'new')
+
+            expect(accessibilityModule.accessibilityMap.has('new')).toBe(false)
+        })
+
+        it('ignores a no-op reload and missing ids', () => {
+            accessibilityModule.accessibilityMap.set('same', true)
+
+            accessibilityModule.onSessionReload('same', 'same')
+            accessibilityModule.onSessionReload(undefined, 'new')
+            accessibilityModule.onSessionReload('old', undefined)
+
+            expect(accessibilityModule.accessibilityMap.get('same')).toBe(true)
+            expect(accessibilityModule.accessibilityMap.has('new')).toBe(false)
+        })
+    })
+
+    describe('scanning toggles after a reload', () => {
+        it('writes the LIVE session id, not the one captured when the test started', async () => {
+            // The gate is read per command from framework state, so a toggle that wrote the
+            // captured id would set the dead key: stopA11yScanning() after a reload would leave
+            // scanning on, and startA11yScanning() would be a silent no-op.
+            vi.mocked(validateCapsWithA11y).mockReturnValue(true)
+            // resetAllMocks in afterEach strips module-level implementations, so the per-test gate
+            // has to be re-stated or shouldScanTest comes out undefined
+            vi.mocked(shouldScanTestForAccessibility).mockReturnValue(true)
+            let liveSessionId = 'session-before-reload'
+            vi.mocked(AutomationFramework.getState).mockImplementation((instance, key) => {
+                if (key.includes('CAPABILITIES')) {
+                    return { browserName: 'chrome' }
+                }
+                return liveSessionId
+            })
+
+            await accessibilityModule.onBeforeTest({ suiteTitle: 'S', test: { title: 't' } })
+            expect(accessibilityModule.accessibilityMap.get('session-before-reload')).toBe(true)
+
+            // reload: framework state moves on, and onReload migrates the gate
+            liveSessionId = 'session-after-reload'
+            accessibilityModule.onSessionReload('session-before-reload', 'session-after-reload')
+
+            await (mockBrowser as { stopA11yScanning: () => Promise<void> }).stopA11yScanning()
+
+            expect(accessibilityModule.accessibilityMap.get('session-after-reload')).toBe(false)
+            expect(accessibilityModule.accessibilityMap.has('session-before-reload')).toBe(false)
+        })
+    })
+
     describe('onBeforeTest', () => {
         it('should set up accessibility metadata for test', async () => {
             const mockArgs = {