diff --git a/.changeset/pr-165.md b/.changeset/pr-165.md new file mode 100644 index 0000000..86df4d6 --- /dev/null +++ b/.changeset/pr-165.md @@ -0,0 +1,6 @@ +--- +"@wdio/browserstack-service": patch +--- + +- Fixed accessibility scanning stopping for the remainder of a test after `browser.reloadSession()`. +- Fixed App Automate and Automate session names staying on the static `sessionName` capability when a test reloads the session part-way through for example a deep-link or app-relaunch step. Such sessions now show the test title, and also carry their own pass/fail result. diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index ecb0f93..69c3350 100644 --- a/packages/browserstack-service/src/accessibility-handler.ts +++ b/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') } diff --git a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts index 359d12c..8e44b11 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/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 - LOG_DISABLED_SHOWN: Map + accessibilityMap: Map + LOG_DISABLED_SHOWN: Map testMetadata: Record = {} 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,7 +197,7 @@ 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) } @@ -156,7 +205,7 @@ export default class AccessibilityModule extends BaseModule { //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') } diff --git a/packages/browserstack-service/src/cli/modules/automateModule.ts b/packages/browserstack-service/src/cli/modules/automateModule.ts index 712b8b2..fcdc336 100644 --- a/packages/browserstack-service/src/cli/modules/automateModule.ts +++ b/packages/browserstack-service/src/cli/modules/automateModule.ts @@ -155,7 +155,7 @@ export default class AutomateModule extends BaseModule { const suiteTitle = args.suiteTitle as string const testContextOptions = this.config.testContextOptions as TestContextOptions - if (testContextOptions.skipSessionStatus || !isBrowserstackSession(browser)) { + if (!isBrowserstackSession(browser)) { this.logger.info('Skipping session status update as per configuration') return } @@ -176,14 +176,39 @@ export default class AutomateModule extends BaseModule { name = `${pre}${test.parent}${post}` } + // SDK-7270 (residual): the session can be REPLACED mid-test. @wdio/mocha-framework binds + // beforeTest to the test function itself (wrapGlobalTestMethod), so onBeforeTest is the one + // and only per-test naming opportunity and it has already passed by the time the test body + // calls browser.reloadSession(). The replacement session is therefore never registered in + // sessionMap and keeps its creation-time `sessionName` capability. (A reload in a beforeEach + // hook is fine — that runs before beforeTest.) Re-resolve the live session id here + // (service.onReload has already pointed KEY_FRAMEWORK_SESSION_ID at it) and adopt it while + // it is still open. + // + // Deliberately gated on skipSessionName, NOT skipSessionStatus: naming and status are + // independent options, so a `setSessionStatus: false` user must still get the name repair, + // and a `setSessionName: false` user must not be pulled into sessionMap — that would hand + // onAfterExecute a session to status-mark where it previously had none. + if (sessionId && !testContextOptions.skipSessionName && !this.sessionMap.has(sessionId)) { + this.sessionMap.set(sessionId, { lastTestName: name, testResults: new Map() }) + } + // No-op for the steady state: when no mid-test reload happened, onBeforeTest already + // applied this exact name and `appliedName` de-dupes it away — no extra API call. + await this.flushSessionName(sessionId) + + if (testContextOptions.skipSessionStatus) { + this.logger.info('Skipping session status update as per configuration') + return + } + + const testResult: TestResult = { + testName: name, + status: status, + reason: reason + } + const sessionData = this.sessionMap.get(sessionId) if (sessionData) { - const testResult: TestResult = { - testName: name, - status: status, - reason: reason - } - sessionData.testResults.set(name, testResult) this.sessionMap.set(sessionId, sessionData) } diff --git a/packages/browserstack-service/src/service.ts b/packages/browserstack-service/src/service.ts index d02b839..b996be4 100644 --- a/packages/browserstack-service/src/service.ts +++ b/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 diff --git a/packages/browserstack-service/tests/accessibility-handler.test.ts b/packages/browserstack-service/tests/accessibility-handler.test.ts index a6bcf90..1d75f54 100644 --- a/packages/browserstack-service/tests/accessibility-handler.test.ts +++ b/packages/browserstack-service/tests/accessibility-handler.test.ts @@ -689,6 +689,49 @@ describe('afterTest', () => { }) }) +describe('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 } + 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 diff --git a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts index 2a12eee..0c77511 100644 --- a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts +++ b/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 }).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 = { diff --git a/packages/browserstack-service/tests/cli/modules/automateModule.test.ts b/packages/browserstack-service/tests/cli/modules/automateModule.test.ts index d3f47c4..3fca63a 100644 --- a/packages/browserstack-service/tests/cli/modules/automateModule.test.ts +++ b/packages/browserstack-service/tests/cli/modules/automateModule.test.ts @@ -210,6 +210,114 @@ describe('AutomateModule', () => { expect(TestFramework.setState).toHaveBeenCalledTimes(2) // status and reason }) + it('names the session created by a mid-test reload (SDK-7270)', async () => { + // browser.reloadSession() from the test body runs AFTER mocha's EVENT_TEST_BEGIN, so + // onBeforeTest named the old session and the replacement was never registered. + vi.mocked(fetch).mockResolvedValue({ + json: vi.fn().mockResolvedValue({ success: true }) + } as any) + + const mockArgs = { + instance: mockTestInstance, + result: { error: null, passed: true }, + test: { title: 'test title', parent: 'suite title' }, + suiteTitle: 'suite title' + } + + await automateModule.onBeforeTest(mockArgs) + + // service.onReload repoints KEY_FRAMEWORK_SESSION_ID at the replacement session. + vi.mocked(AutomationFramework.getState).mockImplementation((instance, key) => { + if (key === 'framework_session_id') {return 'reloaded-session-id'} + if (key.includes('CAPABILITIES')) {return { browserName: 'chrome' }} + return {} + }) + vi.mocked(fetch).mockClear() + + await automateModule.onAfterTest(mockArgs) + + expect(fetch).toHaveBeenCalledWith( + expect.stringContaining('sessions/reloaded-session-id.json'), + expect.objectContaining({ method: 'PUT', body: JSON.stringify({ name: 'suite title - test title' }) }) + ) + }) + + it('issues no extra name call when the session is unchanged across the test (SDK-7270 de-dupe)', async () => { + vi.mocked(fetch).mockResolvedValue({ + json: vi.fn().mockResolvedValue({ success: true }) + } as any) + + const mockArgs = { + instance: mockTestInstance, + result: { error: null, passed: true }, + test: { title: 'test title', parent: 'suite title' }, + suiteTitle: 'suite title' + } + + await automateModule.onBeforeTest(mockArgs) + vi.mocked(fetch).mockClear() + + await automateModule.onAfterTest(mockArgs) + + // appliedName de-dupes — steady state must cost zero additional API calls. + expect(fetch).not.toHaveBeenCalled() + }) + + it('still names a mid-test-reload session when skipSessionStatus is true (SDK-7270 guard independence)', async () => { + // Naming and status are independent options; a status flag must not gate the name repair. + (automateModule.config as any).testContextOptions.skipSessionStatus = true + vi.mocked(fetch).mockResolvedValue({ + json: vi.fn().mockResolvedValue({ success: true }) + } as any) + + const mockArgs = { + instance: mockTestInstance, + result: { error: null, passed: true }, + test: { title: 'test title', parent: 'suite title' }, + suiteTitle: 'suite title' + } + + await automateModule.onBeforeTest(mockArgs) + + vi.mocked(AutomationFramework.getState).mockImplementation((instance, key) => { + if (key === 'framework_session_id') {return 'reloaded-session-id'} + if (key.includes('CAPABILITIES')) {return { browserName: 'chrome' }} + return {} + }) + vi.mocked(fetch).mockClear() + + await automateModule.onAfterTest(mockArgs) + + expect(fetch).toHaveBeenCalledWith( + expect.stringContaining('sessions/reloaded-session-id.json'), + expect.objectContaining({ method: 'PUT', body: JSON.stringify({ name: 'suite title - test title' }) }) + ) + }) + + it('adds no status traffic for skipSessionName users (SDK-7270 inverse-leak guard)', async () => { + // With naming off, onBeforeTest never registers the session. onAfterTest must not adopt it + // either, or onAfterExecute would start status-marking sessions it previously ignored. + (automateModule.config as any).testContextOptions.skipSessionName = true + vi.mocked(fetch).mockResolvedValue({ + json: vi.fn().mockResolvedValue({ success: true }) + } as any) + + const mockArgs = { + instance: mockTestInstance, + result: { error: null, passed: true }, + test: { title: 'test title', parent: 'suite title' }, + suiteTitle: 'suite title' + } + + await automateModule.onBeforeTest(mockArgs) + await automateModule.onAfterTest(mockArgs) + vi.mocked(fetch).mockClear() + + await automateModule.onAfterExecute() + + expect(fetch).not.toHaveBeenCalled() + }) + it('should skip session status update when skipSessionStatus is true', async () => { (automateModule.config as any).testContextOptions.skipSessionStatus = true const mockArgs = { diff --git a/packages/browserstack-service/tests/decoratorPlacement.test.ts b/packages/browserstack-service/tests/decoratorPlacement.test.ts new file mode 100644 index 0000000..118a87e --- /dev/null +++ b/packages/browserstack-service/tests/decoratorPlacement.test.ts @@ -0,0 +1,41 @@ +import fs from 'node:fs' +import path from 'node:path' +import { describe, expect, it } from 'vitest' + +/** + * A decorator binds to the next class ELEMENT, not to the next thing that looks related. Insert a + * method between `@PerformanceTester.Measure(..., { hookType: 'onReload' })` and `onReload` and the + * decoration silently moves: the hook loses its telemetry, and the intruding method starts + * reporting measurements under the hook's name. + * + * Nothing else in this repo can catch that. The code stays valid TypeScript, lint has no opinion, + * and the unit suites mock `PerformanceTester.Measure` into a pass-through, so the binding has no + * observable behaviour to assert on. Hence a structural check. + */ +const DECORATED_FILES = ['service.ts', 'launcher.ts', 'insights-handler.ts', 'accessibility-handler.ts'] + +describe('PerformanceTester.Measure placement', () => { + for (const file of DECORATED_FILES) { + it(`binds directly to a method in ${file}`, () => { + const lines = fs.readFileSync(path.join(process.cwd(), 'src', file), 'utf8').split('\n') + const offenders: string[] = [] + + lines.forEach((line, index) => { + if (!line.includes('@PerformanceTester.Measure')) { + return + } + // walk to the next non-blank line; that is what the decorator will attach to + let next = index + 1 + while (next < lines.length && lines[next].trim() === '') { + next++ + } + const target = lines[next]?.trim() ?? '' + if (target.startsWith('/*') || target.startsWith('*') || target.startsWith('//')) { + offenders.push(`${file}:${index + 1} — decorator followed by a comment, not a method: ${target.slice(0, 60)}`) + } + }) + + expect(offenders).toEqual([]) + }) + } +}) diff --git a/packages/browserstack-service/tests/service.test.ts b/packages/browserstack-service/tests/service.test.ts index da656dc..bf544f9 100644 --- a/packages/browserstack-service/tests/service.test.ts +++ b/packages/browserstack-service/tests/service.test.ts @@ -7,6 +7,7 @@ import BrowserstackService from '../src/service.js' import * as utils from '../src/util.js' import InsightsHandler from '../src/insights-handler.js' import { BrowserstackCLI } from '../src/cli/index.js' +import AccessibilityModule from '../src/cli/modules/accessibilityModule.js' import * as bstackLogger from '../src/bstackLogger.js' import AutomationFramework from '../src/cli/frameworks/automationFramework.js' import { AutomationFrameworkConstants } from '../src/cli/frameworks/constants/automationFrameworkConstants.js' @@ -191,6 +192,66 @@ describe('onReload()', () => { expect(service['_failReasons']).toEqual([]) }) + describe('accessibility state migration', () => { + // _printSessionURL does a live fetch this suite does not stub on every path — several of + // the pre-existing failures in this file are exactly that, and it is not under test here. + let printSpy: ReturnType + let handler: { onSessionReload: ReturnType } + + beforeEach(() => { + service['_browser'] = browser + printSpy = vi.spyOn(service as any, '_printSessionURL').mockResolvedValue(undefined) + handler = { onSessionReload: vi.fn() } + service['_accessibilityHandler'] = handler as any + }) + + afterEach(() => { + printSpy.mockRestore() + }) + + it('migrates the classic handler when the binary is not driving accessibility', async () => { + await service.onReload('1', '2') + + expect(handler.onSessionReload).toHaveBeenCalledWith('1', '2') + }) + + it('migrates the module gate in the CLI flow, and leaves the classic handler alone', async () => { + // The registry lookup is the path that actually ships for the binary flow, so it is + // asserted rather than left to an optional chain that would swallow a wrong key. + const moduleSpy = { onSessionReload: vi.fn() } + const getInstanceSpy = vi.spyOn(BrowserstackCLI, 'getInstance').mockReturnValue({ + isRunning: () => true, + modules: { [AccessibilityModule.MODULE_NAME]: moduleSpy } + } as any) + const isBstackSpy = vi.spyOn(utils, 'isBrowserstackSession').mockReturnValue(true) + + await service.onReload('1', '2') + + expect(moduleSpy.onSessionReload).toHaveBeenCalledWith('1', '2') + expect(handler.onSessionReload).not.toHaveBeenCalled() + + getInstanceSpy.mockRestore() + isBstackSpy.mockRestore() + }) + + it('still migrates the classic handler when the binary is up against a non-BrowserStack provider', async () => { + // isRunning() alone is not the discriminator: with a non-BrowserStack provider the + // handler's before() DID run, so it holds the live state and must be migrated. + const getInstanceSpy = vi.spyOn(BrowserstackCLI, 'getInstance').mockReturnValue({ + isRunning: () => true, + modules: {} + } as any) + const isBstackSpy = vi.spyOn(utils, 'isBrowserstackSession').mockReturnValue(false) + + await service.onReload('1', '2') + + expect(handler.onSessionReload).toHaveBeenCalledWith('1', '2') + + getInstanceSpy.mockRestore() + isBstackSpy.mockRestore() + }) + }) + it('should return if no browser object', async () => { const updateSpy = vi.spyOn(service, '_update') service['_browser'] = undefined