Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/pr-165.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@wdio/browserstack-service": patch
---

- Fixed accessibility scanning stopping for the remainder of a test after `browser.reloadSession()`.
22 changes: 22 additions & 0 deletions packages/browserstack-service/src/accessibility-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,39 @@ 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: unknown, newSessionId: unknown) {
Comment thread
kamal-kaur04 marked this conversation as resolved.
Outdated
try {
if (!oldSessionId || !newSessionId || oldSessionId === newSessionId) {
return
}
const oldKey = oldSessionId as number
const newKey = newSessionId as number
Comment thread
kamal-kaur04 marked this conversation as resolved.
Outdated

if (this.accessibilityMap.has(oldKey)) {
Comment thread
kamal-kaur04 marked this conversation as resolved.
Outdated
this.accessibilityMap.set(newKey, this.accessibilityMap.get(oldKey) as boolean)
this.accessibilityMap.delete(oldKey)
this.logger.debug(`Accessibility scan gate migrated across session reload to ${String(newSessionId)}`)
}
if (this.LOG_DISABLED_SHOWN.has(oldKey)) {
this.LOG_DISABLED_SHOWN.set(newKey, this.LOG_DISABLED_SHOWN.get(oldKey) as boolean)
this.LOG_DISABLED_SHOWN.delete(oldKey)
}
} catch (error) {
this.logger.error(`Exception in accessibility onSessionReload: ${error}`)
}
}

async onBeforeExecute() {
try {
const autoInstance: AutomationFrameworkInstance = AutomationFramework.getTrackedInstance()
Expand Down
11 changes: 11 additions & 0 deletions packages/browserstack-service/src/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -896,6 +897,16 @@ 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)
} else {
// Classic path only. The handler is constructed in both flows, but `before(sessionId)`
// — which is what records `_sessionId` and populates the scan map — runs only when the
// binary is not up, so in the CLI flow this object holds no state to migrate.
this._accessibilityHandler?.onSessionReload(oldSessionId, newSessionId)
Comment thread
kamal-kaur04 marked this conversation as resolved.
}

const { setSessionName, setSessionStatus } = this._options
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,33 @@ 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('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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,42 @@ describe('AccessibilityModule', () => {
})
})

describe('onSessionReload', () => {
it('carries the scan gate over to the new session id', () => {
accessibilityModule.accessibilityMap.set('old' as never, true)

accessibilityModule.onSessionReload('old', 'new')

expect(accessibilityModule.accessibilityMap.get('new' as never)).toBe(true)
expect(accessibilityModule.accessibilityMap.has('old' as never)).toBe(false)
})

it('preserves a gate the user had closed with stopA11yScanning', () => {
accessibilityModule.accessibilityMap.set('old' as never, false)

accessibilityModule.onSessionReload('old', 'new')

expect(accessibilityModule.accessibilityMap.get('new' as never)).toBe(false)
})

it('does nothing when the old session was never registered', () => {
accessibilityModule.onSessionReload('old', 'new')

expect(accessibilityModule.accessibilityMap.has('new' as never)).toBe(false)
})

it('ignores a no-op reload and missing ids', () => {
accessibilityModule.accessibilityMap.set('same' as never, true)

accessibilityModule.onSessionReload('same', 'same')
accessibilityModule.onSessionReload(undefined, 'new')
accessibilityModule.onSessionReload('old', undefined)

expect(accessibilityModule.accessibilityMap.get('same' as never)).toBe(true)
expect(accessibilityModule.accessibilityMap.has('new' as never)).toBe(false)
})
})

describe('onBeforeTest', () => {
it('should set up accessibility metadata for test', async () => {
const mockArgs = {
Expand Down
26 changes: 26 additions & 0 deletions packages/browserstack-service/tests/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,32 @@ describe('onReload()', () => {
expect(service['_failReasons']).toEqual([])
})

it('migrates the classic accessibility state, but only when the binary is not running', async () => {
// The handler is constructed in both flows, yet `before(sessionId)` — which records
// _sessionId and fills the scan map — runs only in the non-CLI flow, so migrating it
// under the CLI would be operating on an object holding no state.
service['_browser'] = browser
// _printSessionURL does a live fetch, which this suite does not stub for every path —
// several of the pre-existing failures in this file are exactly that. Not what is under
// test here.
const printSpy = vi.spyOn(service as any, '_printSessionURL').mockResolvedValue(undefined)
const handler = { onSessionReload: vi.fn() }
service['_accessibilityHandler'] = handler as any

await service.onReload('1', '2')
expect(handler.onSessionReload).toHaveBeenCalledWith('1', '2')

handler.onSessionReload.mockClear()
const getInstanceSpy = vi.spyOn(BrowserstackCLI, 'getInstance')
.mockReturnValue({ isRunning: () => true, modules: {} } as any)
Comment thread
kamal-kaur04 marked this conversation as resolved.
Outdated

await service.onReload('1', '2')
expect(handler.onSessionReload).not.toHaveBeenCalled()

getInstanceSpy.mockRestore()
printSpy.mockRestore()
})

it('should return if no browser object', async () => {
const updateSpy = vi.spyOn(service, '_update')
service['_browser'] = undefined
Expand Down
Loading