From 2b7f7e9120ec3fa72ec45aa308c1d1591d375ed5 Mon Sep 17 00:00:00 2001 From: Harshit Date: Mon, 24 Aug 2026 17:20:26 +0530 Subject: [PATCH 01/15] fix(automate): name the session created by a mid-test reload (SDK-7270) Follow-up to #148. @wdio/mocha-framework binds beforeTest to the test function itself (wrapGlobalTestMethod), so onBeforeTest is the single per-test naming opportunity and it has already passed by the time the test body calls browser.reloadSession(). The replacement session is never registered in sessionMap, so flushSessionName() returns early on !sessionData and the onAfterExecute sweep -- which iterates the same map -- misses it too. service.onReload learns the new session id but only renames the outgoing one, leaving the live session on its creation-time sessionName capability. onAfterTest now re-resolves the live session id and adopts it into sessionMap before flushing the name, while that session is still open. The naming block is gated on skipSessionName rather than skipSessionStatus and sits above the status gate: gating a name repair behind a status flag would skip it for setSessionStatus:false users, and adopting unconditionally would pull setSessionName:false users into sessionMap and start issuing them a status PUT per session where they previously had none. Adoption also records the test result, so the post-reload session's status is marked at teardown instead of being dropped by the old `if (sessionData)` guard. Steady state costs nothing extra -- appliedName de-dupes the flush when the session did not change. Co-Authored-By: Claude Opus 5 --- .../src/cli/modules/automateModule.ts | 39 +++++-- .../tests/cli/modules/automateModule.test.ts | 108 ++++++++++++++++++ 2 files changed, 140 insertions(+), 7 deletions(-) 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/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 = { From d2a0326b8752f700b2313674f64f73511ab31644 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 13:44:01 +0530 Subject: [PATCH 02/15] fix(app-a11y): register scans for pre-test and reloaded sessions, capture aliased configs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects, each reproduced on an App Automate device run and verified against the session's own appAllyScan telemetry. App-A11y scanning is gated on a per-session entry that only onBeforeTest and onHookStart ever wrote, and that entry is keyed on the session id: - Commands issued from WDIO's config-level before()/beforeSession() ran before any entry existed, so a hook that launches the app and signs in produced no scans at all while still counting as expected coverage. The gate now opens at driver creation. - browser.reloadSession() leaves the entry stranded under the old session id, so nothing scanned for the rest of the reloading test. onReload now migrates it, on both the CLI and the classic handler paths. Config auto-capture followed only relative import specifiers, so a project that wires its split configs through tsconfig `paths` uploaded its entry config alone — the file carrying the hooks was never in the bundle. Aliases are now resolved from the nearest tsconfig/jsconfig (following `extends`, tolerating JSONC); the *.conf.* name filter still keeps application source out. SDK-7422 --- .../src/accessibility-handler.ts | 22 ++ .../src/cli/modules/accessibilityModule.ts | 49 +++++ .../browserstack-service/src/configCapture.ts | 207 +++++++++++++++++- .../browserstack-service/src/constants.ts | 5 + packages/browserstack-service/src/service.ts | 7 + .../tests/accessibility-handler.test.ts | 27 +++ .../cli/modules/accessibilityModule.test.ts | 69 ++++++ .../tests/configCapture.test.ts | 77 +++++++ 8 files changed, 455 insertions(+), 8 deletions(-) diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index ecb0f93..1936f22 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 === null) { + 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) diff --git a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts index 359d12c..640ed26 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -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) { + try { + if (!oldSessionId || !newSessionId || oldSessionId === newSessionId) { + return + } + const oldKey = oldSessionId as number + const newKey = newSessionId as number + + if (this.accessibilityMap.has(oldKey)) { + 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() @@ -214,6 +247,22 @@ export default class AccessibilityModule extends BaseModule { }) } + // Open the scan gate for the window between driver creation and the first test. + // WDIO's own config-level hooks (`before`, `beforeSession`) run here, and they are + // not test-framework hooks, so neither onHookStart nor onBeforeTest has fired yet + // and the gate would have no entry for this session at all: every command issued + // from a config-level before() went unscanned, while still counting as expected + // coverage. Real suites do substantial UI work in that hook (app launch, login, + // account selection), so the screens it walks through are exactly the ones a + // customer expects covered. + // + // Unconditionally true, matching onHookStart: per-test include/exclude filters need + // a test to evaluate and there is none yet, and the following onBeforeTest recomputes + // the gate for the test proper. + if (this.autoScanning && sessionId !== undefined && sessionId !== null) { + this.accessibilityMap.set(sessionId, true) + } + } catch (error) { this.logger.error(`Error in onBeforeExecute: ${error}`) } diff --git a/packages/browserstack-service/src/configCapture.ts b/packages/browserstack-service/src/configCapture.ts index 521560b..e7e04ab 100644 --- a/packages/browserstack-service/src/configCapture.ts +++ b/packages/browserstack-service/src/configCapture.ts @@ -15,9 +15,11 @@ import { BROWSERSTACK_WDIO_CONFIG_STRATEGY, CAPTURE_CONFIG_IMPORT_DEPTH, DEFAULT_WDIO_CONFIG_BASENAME, + MAX_ALIAS_MANIFEST_EXTENDS_DEPTH, MAX_CAPTURED_CONFIG_FILES, MAX_CAPTURED_CONFIG_FILE_BYTES, MAX_PACKAGE_JSON_WALK_UP, + PATH_ALIAS_MANIFESTS, REDACTED_KEYS, SUPPORTED_WDIO_CONFIG_EXTENSIONS, WDIO_CLI_SUBCOMMANDS @@ -313,9 +315,7 @@ const readCappedFile = (filePath: string): { content?: string, reason?: string } */ const isConfigFileName = (filePath: string): boolean => /\.conf(ig)?\.[^.]+$/i.test(path.basename(filePath)) -const resolveRelativeImport = (specifier: string, fromFile: string): string | undefined => { - const base = path.resolve(path.dirname(fromFile), specifier) - +const probeConfigCandidate = (base: string): string | undefined => { if (isReadableFile(base) && SUPPORTED_WDIO_CONFIG_EXTENSIONS.includes(path.extname(base))) { return isConfigFileName(base) ? base : undefined } @@ -339,31 +339,222 @@ const resolveRelativeImport = (specifier: string, fromFile: string): string | un return undefined } +const resolveRelativeImport = (specifier: string, fromFile: string): string | undefined => + probeConfigCandidate(path.resolve(path.dirname(fromFile), specifier)) + +/** + * Strip comments and trailing commas so a tsconfig can be JSON.parse'd. + * + * tsconfig is JSONC and real ones are full of comments, so a plain JSON.parse fails on the + * majority of them. String literals are tracked because a Windows path (`"C:\\a"`) and a URL + * in a comment both contain sequences that a naive strip would treat as delimiters. + */ +const parseJsonc = (text: string): Record | undefined => { + let out = '' + let inString = false + let inLine = false + let inBlock = false + + for (let i = 0; i < text.length; i++) { + const char = text[i] + const next = text[i + 1] + + if (inLine) { + if (char === '\n') { + inLine = false + out += char + } + continue + } + if (inBlock) { + if (char === '*' && next === '/') { + inBlock = false + i++ + } + continue + } + if (inString) { + out += char + if (char === '\\') { + out += next ?? '' + i++ + } else if (char === '"') { + inString = false + } + continue + } + if (char === '"') { + inString = true + out += char + continue + } + if (char === '/' && next === '/') { + inLine = true + i++ + continue + } + if (char === '/' && next === '*') { + inBlock = true + i++ + continue + } + out += char + } + + try { + return JSON.parse(out.replace(/,(\s*[}\]])/g, '$1')) as Record + } catch { + return undefined + } +} + +interface AliasPattern { + /* text before the single `*`, or the whole pattern for an exact alias */ + prefix: string + /* text after the `*`; empty for a prefix-only or exact alias */ + suffix: string + /* absolute target paths, `*` still in place */ + targets: string[] + /* exact aliases must match the specifier in full */ + exact: boolean +} + +/** + * `compilerOptions.paths` from the nearest tsconfig/jsconfig, following `extends`. + * + * Split configs are routinely wired with aliases rather than relative paths — the customer + * whose bundle prompted this imports every one of its configs as `@wdioConfs/...` — and an + * alias-only project used to hand us a single entry config that spreads files we never see. + */ +const collectAliasPatterns = (manifestPath: string, depth = 0): AliasPattern[] => { + const parsed = parseJsonc(readCappedFile(manifestPath).content || '') + if (!parsed) { + return [] + } + + const manifestDir = path.dirname(manifestPath) + const compilerOptions = (parsed.compilerOptions || {}) as Record + const paths = (compilerOptions.paths || {}) as Record + // No baseUrl is legal since TS 4.4, and then targets resolve against the tsconfig itself. + const baseUrl = typeof compilerOptions.baseUrl === 'string' + ? path.resolve(manifestDir, compilerOptions.baseUrl) + : manifestDir + + const patterns: AliasPattern[] = [] + for (const [pattern, rawTargets] of Object.entries(paths)) { + const targets = (Array.isArray(rawTargets) ? rawTargets : []) + .filter((target): target is string => typeof target === 'string') + .map((target) => path.resolve(baseUrl, target)) + if (targets.length === 0) { + continue + } + const star = pattern.indexOf('*') + patterns.push(star === -1 + ? { prefix: pattern, suffix: '', targets, exact: true } + : { prefix: pattern.slice(0, star), suffix: pattern.slice(star + 1), targets, exact: false }) + } + + // `extends` is resolved relative to the extending file; a bare specifier is a shared + // config package, which is worth one node_modules probe and no more. + const extendsList = Array.isArray(parsed.extends) + ? parsed.extends.filter((entry): entry is string => typeof entry === 'string') + : (typeof parsed.extends === 'string' ? [parsed.extends] : []) + + if (depth < MAX_ALIAS_MANIFEST_EXTENDS_DEPTH) { + for (const entry of extendsList) { + const candidates = entry.startsWith('.') + ? [path.resolve(manifestDir, entry), path.resolve(manifestDir, `${entry}.json`)] + : [path.join(manifestDir, 'node_modules', entry), path.join(manifestDir, 'node_modules', entry, 'tsconfig.json')] + const parent = candidates.find(isReadableFile) + if (parent) { + // Own patterns win: the extending file overrides its base. + patterns.push(...collectAliasPatterns(parent, depth + 1)) + } + } + } + + return patterns +} + +const findAliasPatterns = (entryPath: string): AliasPattern[] => { + for (const manifest of PATH_ALIAS_MANIFESTS) { + const found = findUpwards(path.dirname(entryPath), manifest) + if (found) { + const patterns = collectAliasPatterns(found) + if (patterns.length > 0) { + return patterns + } + } + } + return [] +} + +const resolveAliasImport = (specifier: string, patterns: AliasPattern[]): string | undefined => { + for (const { prefix, suffix, targets, exact } of patterns) { + let wildcard: string | undefined + if (exact) { + if (specifier !== prefix) { + continue + } + wildcard = '' + } else { + if (!specifier.startsWith(prefix) || !specifier.endsWith(suffix) || + specifier.length < prefix.length + suffix.length) { + continue + } + wildcard = specifier.slice(prefix.length, specifier.length - (suffix.length || 0)) + } + + for (const target of targets) { + const resolved = probeConfigCandidate(target.replace('*', wildcard)) + if (resolved) { + return resolved + } + } + } + return undefined +} + /** * Collect local files a config pulls in (`wdio.shared.conf.ts` style splits), so a * captured config is not just a two-line file that spreads a base config we never see. * - * Only RELATIVE specifiers are followed — bare specifiers are npm packages, never the - * customer's own config. Depth and count are capped so this can never walk a source tree. + * Two kinds of specifier are followed: RELATIVE ones, and bare ones that a tsconfig + * `paths` alias maps back into the project. Anything else is an npm package. Every + * candidate still has to be named like a config (`*.conf.*` / `*.config.*`), so an + * alias table pointing at `src/*` cannot turn this into a source-tree crawl, and depth + * and count stay capped. */ const collectLocalImports = (entryPath: string, entryContent: string, budget: number): Array<{ filePath: string, content: string }> => { const found: Array<{ filePath: string, content: string }> = [] const seen = new Set([entryPath]) let frontier: Array<{ filePath: string, content: string }> = [{ filePath: entryPath, content: entryContent }] + // Resolved lazily: only an unresolved bare specifier needs the alias table, so a + // project that splits its config relatively never reads a tsconfig at all. + let aliasPatterns: AliasPattern[] | undefined for (let depth = 0; depth < CAPTURE_CONFIG_IMPORT_DEPTH && found.length < budget; depth++) { const next: Array<{ filePath: string, content: string }> = [] for (const { filePath, content } of frontier) { - // `from './x'`, `import './x'`, `import('./x')`, `require('./x')` - const importRegex = /(?:from|import|require)\s*\(?\s*['"](\.[^'"]*)['"]/g + // `from 'x'`, `import 'x'`, `import('x')`, `require('x')` + const importRegex = /(?:from|import|require)\s*\(?\s*['"]([^'"]+)['"]/g let match: RegExpExecArray | null while ((match = importRegex.exec(content)) !== null) { if (found.length >= budget) { break } - const resolved = resolveRelativeImport(match[1], filePath) + const specifier = match[1] + let resolved: string | undefined + if (specifier.startsWith('.')) { + resolved = resolveRelativeImport(specifier, filePath) + } else { + if (aliasPatterns === undefined) { + aliasPatterns = findAliasPatterns(entryPath) + } + resolved = aliasPatterns.length > 0 ? resolveAliasImport(specifier, aliasPatterns) : undefined + } if (!resolved || seen.has(resolved) || resolved.includes(`${path.sep}node_modules${path.sep}`)) { continue } diff --git a/packages/browserstack-service/src/constants.ts b/packages/browserstack-service/src/constants.ts index c64bfa8..b88cd7c 100644 --- a/packages/browserstack-service/src/constants.ts +++ b/packages/browserstack-service/src/constants.ts @@ -85,6 +85,11 @@ export const MAX_CAPTURED_CONFIG_FILES = 6 export const CAPTURE_CONFIG_IMPORT_DEPTH = 1 /* How far to walk up from the config dir looking for the project's package.json */ export const MAX_PACKAGE_JSON_WALK_UP = 5 +/* JSONC manifests read for `compilerOptions.paths`, so a config that imports its siblings + through an alias (`@confs/wdio-main.conf`) is followed like a relative import */ +export const PATH_ALIAS_MANIFESTS = ['tsconfig.json', 'jsconfig.json'] +/* How many `extends` links to follow out of an alias manifest (a base plus overrides) */ +export const MAX_ALIAS_MANIFEST_EXTENDS_DEPTH = 3 /** * Keys whose line is scrubbed before a config file enters the archive. diff --git a/packages/browserstack-service/src/service.ts b/packages/browserstack-service/src/service.ts index d02b839..f1d04a2 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' @@ -896,7 +897,13 @@ 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) } + this._accessibilityHandler?.onSessionReload(oldSessionId, newSessionId) const { setSessionName, setSessionStatus } = this._options const ignoreHooksStatus = this._options.testObservabilityOptions?.ignoreHooksStatus === true diff --git a/packages/browserstack-service/tests/accessibility-handler.test.ts b/packages/browserstack-service/tests/accessibility-handler.test.ts index a6bcf90..7982ece 100644 --- a/packages/browserstack-service/tests/accessibility-handler.test.ts +++ b/packages/browserstack-service/tests/accessibility-handler.test.ts @@ -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 diff --git a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts index 2a12eee..c3c2003 100644 --- a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts +++ b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts @@ -199,6 +199,75 @@ describe('AccessibilityModule', () => { }) }) + describe('onBeforeExecute - pre-test scan gate (config-level before hook)', () => { + it('opens the scan gate at driver creation so config-level before() commands scan', async () => { + // WDIO's config-level before()/beforeSession() are not test-framework hooks, so + // neither onHookStart nor onBeforeTest has run when they fire their commands. + vi.mocked(validateCapsWithA11y).mockReturnValue(true) + vi.mocked(AutomationFramework.getState).mockImplementation((instance, key) => { + if (key.includes('CAPABILITIES')) { + return { browserName: 'chrome' } + } + return 12345 + }) + + await accessibilityModule.onBeforeExecute() + + expect(accessibilityModule.accessibilityMap.get(12345 as never)).toBe(true) + }) + + it('leaves the gate closed when autoScanning is off', async () => { + vi.mocked(validateCapsWithA11y).mockReturnValue(true) + vi.mocked(AutomationFramework.getState).mockImplementation((instance, key) => { + if (key.includes('CAPABILITIES')) { + return { browserName: 'chrome' } + } + return 12345 + }) + accessibilityModule.autoScanning = false + + await accessibilityModule.onBeforeExecute() + + expect(accessibilityModule.accessibilityMap.has(12345 as never)).toBe(false) + }) + }) + + 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 = { diff --git a/packages/browserstack-service/tests/configCapture.test.ts b/packages/browserstack-service/tests/configCapture.test.ts index 0224db9..5b50b8a 100644 --- a/packages/browserstack-service/tests/configCapture.test.ts +++ b/packages/browserstack-service/tests/configCapture.test.ts @@ -513,6 +513,83 @@ describe('collectConfigFilesForUpload', () => { expect(files.map((f) => f.name)).toEqual(['wdio.conf.ts']) }) + it('follows a bare specifier that a tsconfig paths alias maps back into the project', () => { + // The shape that prompted this: a customer whose entry config imports every other + // config through an alias, so relative-only following captured the entry alone. + write('tsconfig.json', JSON.stringify({ + compilerOptions: { baseUrl: '.', paths: { '@confs/*': ['configs/*'] } } + })) + write('wdio.conf.ts', [ + "import { main } from '@confs/wdio-main.conf'", + 'export const config = { ...main }' + ].join('\n')) + write('configs/wdio-main.conf.ts', 'export const main = { maxInstances: 7 }') + + const { files } = collectConfigFilesForUpload({} as never) + + expect(files.map((f) => f.name).sort()).toEqual(['wdio-main.conf.ts', 'wdio.conf.ts']) + expect(files.find((f) => f.name === 'wdio-main.conf.ts')!.content).toContain('maxInstances: 7') + }) + + it('resolves an alias target against baseUrl, not the config directory', () => { + write('tsconfig.json', JSON.stringify({ + compilerOptions: { baseUrl: 'src', paths: { '@confs/*': ['wdio/*'] } } + })) + write('wdio.conf.ts', "import '@confs/base.conf'\nexport const config = {}") + write('src/wdio/base.conf.ts', 'export const base = { maxInstances: 3 }') + + const { files } = collectConfigFilesForUpload({} as never) + + expect(files.map((f) => f.name).sort()).toEqual(['base.conf.ts', 'wdio.conf.ts']) + }) + + it('reads an alias table out of JSONC (comments and trailing commas)', () => { + write('tsconfig.json', [ + '{', + ' // real tsconfigs are commented', + ' "compilerOptions": {', + ' /* block comment */', + ' "baseUrl": ".",', + ' "paths": { "@confs/*": ["configs/*"], },', + ' },', + '}' + ].join('\n')) + write('wdio.conf.ts', "import '@confs/base.conf'\nexport const config = {}") + write('configs/base.conf.ts', 'export const base = {}') + + const { files } = collectConfigFilesForUpload({} as never) + + expect(files.map((f) => f.name).sort()).toEqual(['base.conf.ts', 'wdio.conf.ts']) + }) + + it('picks up an alias table inherited through extends', () => { + write('tsconfig.base.json', JSON.stringify({ + compilerOptions: { baseUrl: '.', paths: { '@confs/*': ['configs/*'] } } + })) + write('tsconfig.json', JSON.stringify({ extends: './tsconfig.base.json' })) + write('wdio.conf.ts', "import '@confs/base.conf'\nexport const config = {}") + write('configs/base.conf.ts', 'export const base = {}') + + const { files } = collectConfigFilesForUpload({} as never) + + expect(files.map((f) => f.name).sort()).toEqual(['base.conf.ts', 'wdio.conf.ts']) + }) + + it('does not let an alias table widen capture to ordinary source', () => { + // `@app/*` -> `src/*` is a normal alias, and following it would archive application + // code. The config-name filter is what keeps this a config capture. + write('tsconfig.json', JSON.stringify({ + compilerOptions: { baseUrl: '.', paths: { '@app/*': ['src/*'] } } + })) + write('wdio.conf.ts', "import { helper } from '@app/helpers/utils'\nexport const config = {}") + write('src/helpers/utils.ts', 'export const helper = 1 // ORDINARY_SOURCE') + + const { files } = collectConfigFilesForUpload({} as never) + + expect(files.map((f) => f.name)).toEqual(['wdio.conf.ts']) + expect(files.map((f) => f.content).join('\n')).not.toContain('ORDINARY_SOURCE') + }) + it('de-duplicates archive names when two captured files share a basename', () => { write('wdio.conf.ts', 'import "./nested/wdio.conf.ts"\nexport const config = {}') write('nested/wdio.conf.ts', 'export const config = {}') From 2354ae4d994a57cdd5233de29f548dfa3f7182f7 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2026 08:34:23 +0000 Subject: [PATCH 03/15] chore(changeset): auto-generate from PR template (patch) --- .changeset/pr-165.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/pr-165.md diff --git a/.changeset/pr-165.md b/.changeset/pr-165.md new file mode 100644 index 0000000..9e4e40d --- /dev/null +++ b/.changeset/pr-165.md @@ -0,0 +1,7 @@ +--- +"@wdio/browserstack-service": patch +--- + +- Fixed accessibility scans not running for driver commands issued from a WDIO config-level `before()` hook. +- Fixed accessibility scanning stopping for the rest of a test after `browser.reloadSession()`. +- Configuration files imported through TypeScript path aliases are now included in uploaded SDK logs, so support can see the config that actually ran. From e943a788f1b8f84121bf5ca7b432408cf4c057b9 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 14:37:39 +0530 Subject: [PATCH 04/15] fix(config-capture): expand an alias wildcard by slicing, not String.replace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL flagged the substitution as an incomplete replacement. The sharper problem is the other half of `String.replace` with a string pattern: `$&` / `$'` in the REPLACEMENT are interpreted, so a specifier carrying those characters resolved to a corrupted path and the imported config was silently dropped from the bundle. Slicing at the first `*` is immune to both, and TypeScript allows at most one `*` per target, so the first is the substitution point by definition. Test pins it — it fails on the previous implementation, which resolved `@confs/a$&b.conf` to `configs/a*b.conf.ts`. SDK-7422 --- packages/browserstack-service/src/configCapture.ts | 10 +++++++++- .../tests/configCapture.test.ts | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/browserstack-service/src/configCapture.ts b/packages/browserstack-service/src/configCapture.ts index e7e04ab..a0f23cf 100644 --- a/packages/browserstack-service/src/configCapture.ts +++ b/packages/browserstack-service/src/configCapture.ts @@ -506,7 +506,15 @@ const resolveAliasImport = (specifier: string, patterns: AliasPattern[]): string } for (const target of targets) { - const resolved = probeConfigCandidate(target.replace('*', wildcard)) + // Substituted by slicing rather than String.replace: `replace` with a string + // pattern interprets `$&` / `$'` in the REPLACEMENT, so a specifier carrying + // those characters would resolve to a corrupted path. TypeScript allows at most + // one `*` per target, so the first one is the substitution point by definition. + const star = target.indexOf('*') + const expanded = star === -1 + ? target + : target.slice(0, star) + wildcard + target.slice(star + 1) + const resolved = probeConfigCandidate(expanded) if (resolved) { return resolved } diff --git a/packages/browserstack-service/tests/configCapture.test.ts b/packages/browserstack-service/tests/configCapture.test.ts index 5b50b8a..0e4c99c 100644 --- a/packages/browserstack-service/tests/configCapture.test.ts +++ b/packages/browserstack-service/tests/configCapture.test.ts @@ -575,6 +575,20 @@ describe('collectConfigFilesForUpload', () => { expect(files.map((f) => f.name).sort()).toEqual(['base.conf.ts', 'wdio.conf.ts']) }) + it('substitutes the wildcard literally, even when the specifier carries $ patterns', () => { + // String.replace interprets `$&` in the REPLACEMENT, which would have resolved this + // to `configs/a*b.conf.ts` — a path that does not exist. + write('tsconfig.json', JSON.stringify({ + compilerOptions: { baseUrl: '.', paths: { '@confs/*': ['configs/*'] } } + })) + write('wdio.conf.ts', "import '@confs/a$&b.conf'\nexport const config = {}") + write('configs/a$&b.conf.ts', 'export const base = {}') + + const { files } = collectConfigFilesForUpload({} as never) + + expect(files.map((f) => f.name).sort()).toEqual(['a$&b.conf.ts', 'wdio.conf.ts']) + }) + it('does not let an alias table widen capture to ordinary source', () => { // `@app/*` -> `src/*` is a normal alias, and following it would archive application // code. The config-name filter is what keeps this a config capture. From f2271ca8d7818940a42ba64047b773efb11b13e2 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 15:55:08 +0530 Subject: [PATCH 05/15] feat(accessibility): give pre-test scans a hook run TRA can resolve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scans fired from WDIO's config-level before() had neither thTestRunUuid (no test exists yet) nor thHookRunUuid, so App-A11y's lookup of the scan's parent in BTCER had nothing to find — the scan reached the hub and then belonged to nothing. The config-level window is now reported as a BEFORE_ALL hook run through the existing framework path, so the framework mints the uuid, the binary emits HookRunStarted/Finished for it, and this module's own onHookStart observer stamps it onto the scans. The instance the tracked hook creates is the one the first test reuses, so the hook also lands parented to a real test_run_id rather than orphaned. Opened ON DEMAND, from the scan path only. WDIO gives a service no event for a user's config-level before() — ConfigParser.addService folds the user's config hooks and every service's hooks into one config.before array that the runner fires with Promise.all — so there is nothing to detect, and opening it from the lifecycle would report a hook that never ran on every build in the fleet. Opening it when a scan needs a parent is exactly the condition that matters, and it self-limits: verified against a control config with no config-level hook, which reports only its four real Mocha hooks. SDK-7422 --- .../src/cli/modules/accessibilityModule.ts | 71 +++++++++++++++++++ .../cli/modules/accessibilityModule.test.ts | 66 +++++++++++++++++ 2 files changed, 137 insertions(+) diff --git a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts index 640ed26..b952479 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -12,6 +12,7 @@ import type { Command } from '../../scripts/accessibility-scripts.js' import accessibilityScripts from '../../scripts/accessibility-scripts.js' import { _getParamsForAppAccessibility, formatString, getAppA11yResults, getAppA11yResultsSummary, shouldScanTestForAccessibility, validateCapsWithA11y, validateCapsWithAppA11y, isBrowserstackSession } from '../../util.js' import { AutomationFrameworkConstants } from '../frameworks/constants/automationFrameworkConstants.js' +import { BROWSERSTACK_WDIO_CONFIG_FILE_PATH } from '../../constants.js' import util from 'node:util' import type { Accessibility } from '../../grpc/index.js' import PerformanceTester from '../../instrumentation/performance/performance-tester.js' @@ -39,6 +40,15 @@ export default class AccessibilityModule extends BaseModule { // Any scan fired while this is set is stamped with it as thHookRunUuid so the backend // (SeleniumHub appAllyScan → hook_run_uuid) can reconcile the scan onto the wrapping test. currentHookRunUuid: string | null = null + // Lifecycle of the hook run that carries scans fired BEFORE any test exists — i.e. from + // WDIO's config-level before()/beforeSession(), which are not test-framework hooks and get + // no hook run of their own. + // + // Opened LAZILY, on the first such scan, and never otherwise: a suite with no config-level + // hook (or one that touches no driver) must not report a phantom hook to TRA, and that is + // the overwhelming majority of suites. 'attempted' is distinct from 'open' so a failed PRE + // is neither retried on every command nor closed by a POST that would pair with nothing. + private preTestHookState: 'idle' | 'attempted' | 'open' | 'closed' = 'idle' constructor(accessibilityConfig: Accessibility, isNonBstackA11y: boolean) { super() @@ -102,6 +112,60 @@ export default class AccessibilityModule extends BaseModule { this.currentHookRunUuid = null } + /** + * Open a BEFORE_ALL hook run for the pre-test window, if one is warranted. + * + * Called from the scan path, so it fires only when there is actually a scan needing a parent. + * The framework stamps the run uuid (KEY_HOOK_ID) and this module's own onHookStart observer + * picks it up, which is what puts thHookRunUuid on the scan; the binary turns the same event + * into HookRunStarted, so App-A11y's lookup of that uuid in BTCER resolves. + */ + private async ensurePreTestHookRun() { + if (this.preTestHookState !== 'idle' || this.currentHookRunUuid !== null) { + return + } + // Mark before awaiting: concurrent commands must not each open their own hook run. + this.preTestHookState = 'attempted' + try { + const framework = BrowserstackCLI.getInstance()?.getTestFramework() + if (!framework) { + return + } + await framework.trackEvent(TestFrameworkState.BEFORE_ALL, HookState.PRE, this.preTestHookArgs()) + this.preTestHookState = 'open' + } catch (error) { + this.logger.debug(`Could not open a hook run for the pre-test window: ${error}`) + } + } + + private async closePreTestHookRun() { + if (this.preTestHookState !== 'open') { + this.preTestHookState = 'closed' + return + } + this.preTestHookState = 'closed' + try { + const framework = BrowserstackCLI.getInstance()?.getTestFramework() + await framework?.trackEvent(TestFrameworkState.BEFORE_ALL, HookState.POST, { ...this.preTestHookArgs(), result: { passed: true } }) + } catch (error) { + this.logger.debug(`Could not close the hook run for the pre-test window: ${error}`) + } + } + + /** + * The binary runs path.relative() over this file path when building the hook record, so an + * absent path would throw there and drop the event. configCapture has already resolved the + * wdio config path onto the environment by this point. + */ + private preTestHookArgs() { + return { + test: { + file: process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] || process.cwd(), + title: 'wdio config-level before hook (pre-test window)' + } + } + } + /** * browser.reloadSession() hands the worker a NEW session id while the driver object, the * wrapped commands and the currently running test all stay exactly the same. The scan gate @@ -281,6 +345,7 @@ export default class AccessibilityModule extends BaseModule { !command.name.includes('execute') || !this.shouldPatchExecuteScript(args.length ? args[0] as string : null) ) { + await this.ensurePreTestHookRun() try { await this.performScanCli(browser, command.name, this.currentHookRunUuid) this.logger.debug(`Accessibility scan performed after ${command.name} command`) @@ -305,6 +370,12 @@ export default class AccessibilityModule extends BaseModule { async onBeforeTest(args: Record) { try { this.logger.debug('Accessibility before test hook. Starting accessibility scan for this test case.') + // The pre-test window is over: close its hook run (if one was opened) before clearing + // the uuid, so the POST pairs against the record the PRE pushed. onHookEnd is the only + // other thing that clears this, so without the clear a spec with no framework hooks + // would stamp every test-body scan with the pre-test hook uuid. + await this.closePreTestHookRun() + this.currentHookRunUuid = null const suiteTitle = (typeof args.suiteTitle === 'string' ? args.suiteTitle : '') || '' const test = (args.test && typeof args.test === 'object' ? args.test as { title?: string } : {}) || {} diff --git a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts index c3c2003..2e87cc5 100644 --- a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts +++ b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts @@ -39,6 +39,19 @@ vi.mock('../../../src/util.js', () => ({ o11yClassErrorHandler: vi.fn().mockImplementation((cls) => cls) })) +// hoisted: vi.mock factories are lifted above module scope, and `getInstance` is a plain +// function rather than a spy so the suite-wide clearAllMocks/resetAllMocks cannot strip its +// return value out from under the module constructor. +const { mockTrackEvent } = vi.hoisted(() => ({ mockTrackEvent: vi.fn() })) +vi.mock('../../../src/cli/index.js', () => ({ + BrowserstackCLI: { + getInstance: () => ({ + options: {}, + getTestFramework: () => ({ trackEvent: mockTrackEvent }) + }) + } +})) + vi.mock('../../../src/cli/grpcClient.js', () => ({ GrpcClient: { getInstance: vi.fn().mockReturnValue({ @@ -109,6 +122,7 @@ describe('AccessibilityModule', () => { afterEach(() => { vi.resetAllMocks() + mockTrackEvent.mockReset() }) describe('constructor', () => { @@ -216,6 +230,18 @@ describe('AccessibilityModule', () => { expect(accessibilityModule.accessibilityMap.get(12345 as never)).toBe(true) }) + it('does not leave a hook run uuid set once the first test starts', async () => { + // The config-level hook window is tracked as a real BEFORE_ALL hook (service.before), + // so currentHookRunUuid comes from the framework. It must not survive into the test + // body: onHookEnd is the only other thing that clears it, and a tracked config hook + // whose POST never landed would otherwise stamp every test-body scan as a hook scan. + accessibilityModule.currentHookRunUuid = 'config-level-hook-uuid' + + await accessibilityModule.onBeforeTest({ suiteTitle: 'S', test: { title: 't' } }) + + expect(accessibilityModule.currentHookRunUuid).toBeNull() + }) + it('leaves the gate closed when autoScanning is off', async () => { vi.mocked(validateCapsWithA11y).mockReturnValue(true) vi.mocked(AutomationFramework.getState).mockImplementation((instance, key) => { @@ -232,6 +258,46 @@ describe('AccessibilityModule', () => { }) }) + describe('pre-test window hook run', () => { + // The config-level before() window has no test-framework hook of its own, so scans from it + // had no parent. One is opened on demand — and ONLY on demand, or every suite in the fleet + // would report a hook it never ran. + it('opens a BEFORE_ALL hook run the first time a scan needs a parent, once only', async () => { + await accessibilityModule['ensurePreTestHookRun']() + await accessibilityModule['ensurePreTestHookRun']() + + const opens = mockTrackEvent.mock.calls.filter((c) => c[1] === HookState.PRE) + expect(opens).toHaveLength(1) + expect(opens[0][0]).toBe(TestFrameworkState.BEFORE_ALL) + expect((opens[0][2] as { test: { title: string } }).test.title).toContain('pre-test window') + }) + + it('opens nothing while a framework hook is already the scan parent', async () => { + accessibilityModule.currentHookRunUuid = 'framework-hook-uuid' + + await accessibilityModule['ensurePreTestHookRun']() + + expect(mockTrackEvent).not.toHaveBeenCalled() + }) + + it('closes the hook run when the first test starts', async () => { + await accessibilityModule['ensurePreTestHookRun']() + mockTrackEvent.mockClear() + + await accessibilityModule.onBeforeTest({ suiteTitle: 'S', test: { title: 't' } }) + + const closes = mockTrackEvent.mock.calls.filter((c) => c[1] === HookState.POST) + expect(closes).toHaveLength(1) + expect((closes[0][2] as { result: { passed: boolean } }).result.passed).toBe(true) + }) + + it('never closes a hook run it did not open', async () => { + await accessibilityModule.onBeforeTest({ suiteTitle: 'S', test: { title: 't' } }) + + expect(mockTrackEvent.mock.calls.filter((c) => c[1] === HookState.POST)).toHaveLength(0) + }) + }) + describe('onSessionReload', () => { it('carries the scan gate over to the new session id', () => { accessibilityModule.accessibilityMap.set('old' as never, true) From cca3e9df5952e307a217dc38343d6d37eda8f379 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 16:56:41 +0530 Subject: [PATCH 06/15] feat(hooks): log the start and finish of every session-scoped config hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wraps the handlers registered for each WDIO hook that runs with a live driver, so the window a driver command falls into is observable. WDIO tells a service nothing about the other handlers in a hook array — ConfigParser folds the user's config hooks and every service's hooks into one array the runner fires with Promise.all — so patching the array in place is the only way to see a non-ours handler start and finish. Logging only; no events, no behaviour change. Sync handlers stay sync so a sync throw is not converted into a rejection. Verified: 'before' appears only when the user declares one (control config with no hooks shows none), while the beforeTest/afterTest entries come from expect-webdriverio's snapshot and soft-assert services — so 'registered at construction' means the user's hooks plus any service constructed before ours, not user code alone. SDK-7422 --- .../src/hookInstrumentation.ts | 117 ++++++++++++++++++ packages/browserstack-service/src/service.ts | 5 + 2 files changed, 122 insertions(+) create mode 100644 packages/browserstack-service/src/hookInstrumentation.ts diff --git a/packages/browserstack-service/src/hookInstrumentation.ts b/packages/browserstack-service/src/hookInstrumentation.ts new file mode 100644 index 0000000..43c3f6b --- /dev/null +++ b/packages/browserstack-service/src/hookInstrumentation.ts @@ -0,0 +1,117 @@ +import type { Options } from '@wdio/types' + +import { BStackLogger } from './bstackLogger.js' + +/** + * WDIO config hooks that execute in the worker process WITH a live session, so a driver + * command issued inside them is meaningful. + * + * Deliberately excluded: + * - `onPrepare` / `onWorkerStart` / `onWorkerEnd` / `onComplete` — launcher process, no driver. + * - `beforeSession` — runs before the session is created; there is no browser yet. + * - `afterSession` — the session is already being deleted; commands there fail. + * - `beforeCommand` / `afterCommand` — a live driver, but per-command: their window is the + * command, which is already observable, and wrapping them would log on every interaction. + */ +export const BROWSER_CONTEXT_HOOKS = [ + 'before', + 'beforeSuite', + 'beforeHook', + 'beforeTest', + 'afterTest', + 'afterHook', + 'afterSuite', + 'after' +] as const + +type HookFn = (...args: unknown[]) => unknown + +const INSTRUMENTED = Symbol('bstackHookInstrumented') + +/** + * Wrap one handler so its entry and exit are observable, preserving its shape exactly. + * + * Sync handlers stay sync: an `async` wrapper would turn every sync hook into a promise and a + * synchronous throw into a rejection, which changes ordering for anything that calls a hook + * without awaiting it. So the wrapper only attaches to the promise when the handler actually + * returned one. + */ +const instrument = (hookName: string, index: number, fn: HookFn): HookFn => { + if ((fn as unknown as Record)[INSTRUMENTED]) { + return fn + } + + // `fn.name` is the only identity WDIO leaves: ConfigParser binds every handler it folds in + // (`hook.bind(service)`), so both the user's config hooks and other services' hooks read as + // `bound `. Logged anyway — it separates a named user function from an anonymous one. + const label = `${hookName}#${index}${fn.name ? ` (${fn.name})` : ''}` + const wrapped = function (this: unknown, ...args: unknown[]) { + const startedAt = Date.now() + const finish = (outcome: string) => BStackLogger.debug(`[hook-window] ${label} ${outcome} in ${Date.now() - startedAt}ms`) + + BStackLogger.debug(`[hook-window] ${label} started`) + let result: unknown + try { + result = fn.apply(this, args) + } catch (error) { + finish(`threw: ${(error as Error)?.message}`) + throw error + } + + if (result && typeof (result as Promise).then === 'function') { + return (result as Promise).then( + (value) => { + finish('finished') + return value + }, + (error) => { + finish(`rejected: ${(error as Error)?.message}`) + throw error + } + ) + } + + finish('finished') + return result + } as HookFn + + Object.defineProperty(wrapped, INSTRUMENTED, { value: true }) + return wrapped +} + +/** + * Make the boundaries of every session-scoped config hook observable. + * + * WDIO tells a service nothing about the other handlers registered for a hook: `ConfigParser` + * folds the user's config hooks and every service's hooks into one array per hook name, and the + * runner fires them together (`executeHooksWithArgs` → `Promise.all`). Patching the array in + * place is therefore the only way to see when a handler that is not ours starts and finishes — + * which is what bounds the windows a driver command can fall into. + * + * The index in the label is the handler's position in that merged array, so it includes this + * service's own handlers; it identifies a handler across its start/finish pair, nothing more. + * + * Logging only, for now: no events are emitted and no behaviour changes. Idempotent, and any + * failure is swallowed — instrumentation must never be the reason a suite cannot start. + */ +export function instrumentBrowserContextHooks(config?: Options.Testrunner): void { + if (!config) { + return + } + + try { + const record = config as unknown as Record + for (const hookName of BROWSER_CONTEXT_HOOKS) { + const handlers = record[hookName] + if (!Array.isArray(handlers) || handlers.length === 0) { + continue + } + BStackLogger.debug(`[hook-window] instrumenting ${hookName}: ${handlers.length} handler(s) registered at service construction`) + record[hookName] = handlers.map((handler, index) => + typeof handler === 'function' ? instrument(hookName, index, handler as HookFn) : handler + ) + } + } catch (error) { + BStackLogger.debug(`Could not instrument config hooks: ${error}`) + } +} diff --git a/packages/browserstack-service/src/service.ts b/packages/browserstack-service/src/service.ts index f1d04a2..4ce78cb 100644 --- a/packages/browserstack-service/src/service.ts +++ b/packages/browserstack-service/src/service.ts @@ -24,6 +24,7 @@ import CustomTagsHandler from './custom-tags-handler.js' import { classifyMochaHookTitle, setCurrentMochaHookWindow } from './customTags.js' import type TestHubModule from './cli/modules/testHubModule.js' import { BStackLogger } from './bstackLogger.js' +import { instrumentBrowserContextHooks } from './hookInstrumentation.js' import PercyHandler from './Percy/Percy-Handler.js' import Listener from './testOps/listener.js' import { saveWorkerData } from './data-store.js' @@ -119,6 +120,10 @@ export default class BrowserstackService implements Services.ServiceInstance { const bailOpt: unknown = this._config?.mochaOpts?.bail this._mochaBail = this._config?.framework === 'mocha' && Boolean(bailOpt) && !isFalse(bailOpt) + // Make the boundaries of the user's own session-scoped config hooks observable. Logging + // only; patched here because the runner reads these arrays after the service is built. + instrumentBrowserContextHooks(this._config) + PerformanceTester.startMonitoring('performance-report-service.csv') if (shouldProcessEventForTesthub('')) { this._config.reporters?.push(TestReporter) From ef9953abeab461678df5f74aec49260460f7ef79 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 17:06:17 +0530 Subject: [PATCH 07/15] revert(config-capture): drop alias-import following, log hook sources instead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts the tsconfig `paths` alias resolution added earlier in this PR, restoring configCapture to what it was: relative imports only, entry config plus package.json. Following aliases widened what gets uploaded from a customer's project, and the actual need — seeing the hook code behind a failure — does not require uploading anything. Instead the launcher reads the hook sources out of the config file and logs them. The parsed config cannot supply them: ConfigParser folds every hook into an array as `hook.bind(service)`, and a bound function stringifies to `[native code]`, so the file is the only place the bodies survive. Also records which hooks call reloadSession and publishes the list on the environment as BROWSERSTACK_HOOKS_WITH_RELOAD_SESSION — a reload inside a hook swaps the session under the driver, which is the shape behind the scan-gate defect this PR fixes. Parsing notes: brace matching is lexical, so a `}` inside a string, comment or template literal does not truncate a body; and the parameter list is skipped by balancing its parentheses before looking for the body brace, because a typed parameter carries its own braces (`beforeTest: function (test: { title?: string })`) and taking the first one captured the signature instead of the hook. Known limit, unchanged from before: only hooks written in the entry config are found. A split config that imports its hooks elsewhere yields nothing — the same blind spot the alias work was aimed at, now without uploading files. The config-level before() scan fix moves to SDK-7422-config-level-before-hook. SDK-7422 --- .../src/cli/modules/accessibilityModule.ts | 87 ------- .../browserstack-service/src/configCapture.ts | 215 +----------------- .../browserstack-service/src/constants.ts | 17 +- .../src/hookInstrumentation.ts | 117 ---------- .../browserstack-service/src/hookSources.ts | 188 +++++++++++++++ packages/browserstack-service/src/launcher.ts | 51 ++++- packages/browserstack-service/src/service.ts | 5 - .../cli/modules/accessibilityModule.test.ts | 99 -------- .../tests/configCapture.test.ts | 91 -------- .../tests/hookSources.test.ts | 122 ++++++++++ 10 files changed, 380 insertions(+), 612 deletions(-) delete mode 100644 packages/browserstack-service/src/hookInstrumentation.ts create mode 100644 packages/browserstack-service/src/hookSources.ts create mode 100644 packages/browserstack-service/tests/hookSources.test.ts diff --git a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts index b952479..5e3dadb 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -12,7 +12,6 @@ import type { Command } from '../../scripts/accessibility-scripts.js' import accessibilityScripts from '../../scripts/accessibility-scripts.js' import { _getParamsForAppAccessibility, formatString, getAppA11yResults, getAppA11yResultsSummary, shouldScanTestForAccessibility, validateCapsWithA11y, validateCapsWithAppA11y, isBrowserstackSession } from '../../util.js' import { AutomationFrameworkConstants } from '../frameworks/constants/automationFrameworkConstants.js' -import { BROWSERSTACK_WDIO_CONFIG_FILE_PATH } from '../../constants.js' import util from 'node:util' import type { Accessibility } from '../../grpc/index.js' import PerformanceTester from '../../instrumentation/performance/performance-tester.js' @@ -40,15 +39,6 @@ export default class AccessibilityModule extends BaseModule { // Any scan fired while this is set is stamped with it as thHookRunUuid so the backend // (SeleniumHub appAllyScan → hook_run_uuid) can reconcile the scan onto the wrapping test. currentHookRunUuid: string | null = null - // Lifecycle of the hook run that carries scans fired BEFORE any test exists — i.e. from - // WDIO's config-level before()/beforeSession(), which are not test-framework hooks and get - // no hook run of their own. - // - // Opened LAZILY, on the first such scan, and never otherwise: a suite with no config-level - // hook (or one that touches no driver) must not report a phantom hook to TRA, and that is - // the overwhelming majority of suites. 'attempted' is distinct from 'open' so a failed PRE - // is neither retried on every command nor closed by a POST that would pair with nothing. - private preTestHookState: 'idle' | 'attempted' | 'open' | 'closed' = 'idle' constructor(accessibilityConfig: Accessibility, isNonBstackA11y: boolean) { super() @@ -112,60 +102,6 @@ export default class AccessibilityModule extends BaseModule { this.currentHookRunUuid = null } - /** - * Open a BEFORE_ALL hook run for the pre-test window, if one is warranted. - * - * Called from the scan path, so it fires only when there is actually a scan needing a parent. - * The framework stamps the run uuid (KEY_HOOK_ID) and this module's own onHookStart observer - * picks it up, which is what puts thHookRunUuid on the scan; the binary turns the same event - * into HookRunStarted, so App-A11y's lookup of that uuid in BTCER resolves. - */ - private async ensurePreTestHookRun() { - if (this.preTestHookState !== 'idle' || this.currentHookRunUuid !== null) { - return - } - // Mark before awaiting: concurrent commands must not each open their own hook run. - this.preTestHookState = 'attempted' - try { - const framework = BrowserstackCLI.getInstance()?.getTestFramework() - if (!framework) { - return - } - await framework.trackEvent(TestFrameworkState.BEFORE_ALL, HookState.PRE, this.preTestHookArgs()) - this.preTestHookState = 'open' - } catch (error) { - this.logger.debug(`Could not open a hook run for the pre-test window: ${error}`) - } - } - - private async closePreTestHookRun() { - if (this.preTestHookState !== 'open') { - this.preTestHookState = 'closed' - return - } - this.preTestHookState = 'closed' - try { - const framework = BrowserstackCLI.getInstance()?.getTestFramework() - await framework?.trackEvent(TestFrameworkState.BEFORE_ALL, HookState.POST, { ...this.preTestHookArgs(), result: { passed: true } }) - } catch (error) { - this.logger.debug(`Could not close the hook run for the pre-test window: ${error}`) - } - } - - /** - * The binary runs path.relative() over this file path when building the hook record, so an - * absent path would throw there and drop the event. configCapture has already resolved the - * wdio config path onto the environment by this point. - */ - private preTestHookArgs() { - return { - test: { - file: process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] || process.cwd(), - title: 'wdio config-level before hook (pre-test window)' - } - } - } - /** * browser.reloadSession() hands the worker a NEW session id while the driver object, the * wrapped commands and the currently running test all stay exactly the same. The scan gate @@ -311,22 +247,6 @@ export default class AccessibilityModule extends BaseModule { }) } - // Open the scan gate for the window between driver creation and the first test. - // WDIO's own config-level hooks (`before`, `beforeSession`) run here, and they are - // not test-framework hooks, so neither onHookStart nor onBeforeTest has fired yet - // and the gate would have no entry for this session at all: every command issued - // from a config-level before() went unscanned, while still counting as expected - // coverage. Real suites do substantial UI work in that hook (app launch, login, - // account selection), so the screens it walks through are exactly the ones a - // customer expects covered. - // - // Unconditionally true, matching onHookStart: per-test include/exclude filters need - // a test to evaluate and there is none yet, and the following onBeforeTest recomputes - // the gate for the test proper. - if (this.autoScanning && sessionId !== undefined && sessionId !== null) { - this.accessibilityMap.set(sessionId, true) - } - } catch (error) { this.logger.error(`Error in onBeforeExecute: ${error}`) } @@ -345,7 +265,6 @@ export default class AccessibilityModule extends BaseModule { !command.name.includes('execute') || !this.shouldPatchExecuteScript(args.length ? args[0] as string : null) ) { - await this.ensurePreTestHookRun() try { await this.performScanCli(browser, command.name, this.currentHookRunUuid) this.logger.debug(`Accessibility scan performed after ${command.name} command`) @@ -370,12 +289,6 @@ export default class AccessibilityModule extends BaseModule { async onBeforeTest(args: Record) { try { this.logger.debug('Accessibility before test hook. Starting accessibility scan for this test case.') - // The pre-test window is over: close its hook run (if one was opened) before clearing - // the uuid, so the POST pairs against the record the PRE pushed. onHookEnd is the only - // other thing that clears this, so without the clear a spec with no framework hooks - // would stamp every test-body scan with the pre-test hook uuid. - await this.closePreTestHookRun() - this.currentHookRunUuid = null const suiteTitle = (typeof args.suiteTitle === 'string' ? args.suiteTitle : '') || '' const test = (args.test && typeof args.test === 'object' ? args.test as { title?: string } : {}) || {} diff --git a/packages/browserstack-service/src/configCapture.ts b/packages/browserstack-service/src/configCapture.ts index a0f23cf..521560b 100644 --- a/packages/browserstack-service/src/configCapture.ts +++ b/packages/browserstack-service/src/configCapture.ts @@ -15,11 +15,9 @@ import { BROWSERSTACK_WDIO_CONFIG_STRATEGY, CAPTURE_CONFIG_IMPORT_DEPTH, DEFAULT_WDIO_CONFIG_BASENAME, - MAX_ALIAS_MANIFEST_EXTENDS_DEPTH, MAX_CAPTURED_CONFIG_FILES, MAX_CAPTURED_CONFIG_FILE_BYTES, MAX_PACKAGE_JSON_WALK_UP, - PATH_ALIAS_MANIFESTS, REDACTED_KEYS, SUPPORTED_WDIO_CONFIG_EXTENSIONS, WDIO_CLI_SUBCOMMANDS @@ -315,7 +313,9 @@ const readCappedFile = (filePath: string): { content?: string, reason?: string } */ const isConfigFileName = (filePath: string): boolean => /\.conf(ig)?\.[^.]+$/i.test(path.basename(filePath)) -const probeConfigCandidate = (base: string): string | undefined => { +const resolveRelativeImport = (specifier: string, fromFile: string): string | undefined => { + const base = path.resolve(path.dirname(fromFile), specifier) + if (isReadableFile(base) && SUPPORTED_WDIO_CONFIG_EXTENSIONS.includes(path.extname(base))) { return isConfigFileName(base) ? base : undefined } @@ -339,230 +339,31 @@ const probeConfigCandidate = (base: string): string | undefined => { return undefined } -const resolveRelativeImport = (specifier: string, fromFile: string): string | undefined => - probeConfigCandidate(path.resolve(path.dirname(fromFile), specifier)) - -/** - * Strip comments and trailing commas so a tsconfig can be JSON.parse'd. - * - * tsconfig is JSONC and real ones are full of comments, so a plain JSON.parse fails on the - * majority of them. String literals are tracked because a Windows path (`"C:\\a"`) and a URL - * in a comment both contain sequences that a naive strip would treat as delimiters. - */ -const parseJsonc = (text: string): Record | undefined => { - let out = '' - let inString = false - let inLine = false - let inBlock = false - - for (let i = 0; i < text.length; i++) { - const char = text[i] - const next = text[i + 1] - - if (inLine) { - if (char === '\n') { - inLine = false - out += char - } - continue - } - if (inBlock) { - if (char === '*' && next === '/') { - inBlock = false - i++ - } - continue - } - if (inString) { - out += char - if (char === '\\') { - out += next ?? '' - i++ - } else if (char === '"') { - inString = false - } - continue - } - if (char === '"') { - inString = true - out += char - continue - } - if (char === '/' && next === '/') { - inLine = true - i++ - continue - } - if (char === '/' && next === '*') { - inBlock = true - i++ - continue - } - out += char - } - - try { - return JSON.parse(out.replace(/,(\s*[}\]])/g, '$1')) as Record - } catch { - return undefined - } -} - -interface AliasPattern { - /* text before the single `*`, or the whole pattern for an exact alias */ - prefix: string - /* text after the `*`; empty for a prefix-only or exact alias */ - suffix: string - /* absolute target paths, `*` still in place */ - targets: string[] - /* exact aliases must match the specifier in full */ - exact: boolean -} - -/** - * `compilerOptions.paths` from the nearest tsconfig/jsconfig, following `extends`. - * - * Split configs are routinely wired with aliases rather than relative paths — the customer - * whose bundle prompted this imports every one of its configs as `@wdioConfs/...` — and an - * alias-only project used to hand us a single entry config that spreads files we never see. - */ -const collectAliasPatterns = (manifestPath: string, depth = 0): AliasPattern[] => { - const parsed = parseJsonc(readCappedFile(manifestPath).content || '') - if (!parsed) { - return [] - } - - const manifestDir = path.dirname(manifestPath) - const compilerOptions = (parsed.compilerOptions || {}) as Record - const paths = (compilerOptions.paths || {}) as Record - // No baseUrl is legal since TS 4.4, and then targets resolve against the tsconfig itself. - const baseUrl = typeof compilerOptions.baseUrl === 'string' - ? path.resolve(manifestDir, compilerOptions.baseUrl) - : manifestDir - - const patterns: AliasPattern[] = [] - for (const [pattern, rawTargets] of Object.entries(paths)) { - const targets = (Array.isArray(rawTargets) ? rawTargets : []) - .filter((target): target is string => typeof target === 'string') - .map((target) => path.resolve(baseUrl, target)) - if (targets.length === 0) { - continue - } - const star = pattern.indexOf('*') - patterns.push(star === -1 - ? { prefix: pattern, suffix: '', targets, exact: true } - : { prefix: pattern.slice(0, star), suffix: pattern.slice(star + 1), targets, exact: false }) - } - - // `extends` is resolved relative to the extending file; a bare specifier is a shared - // config package, which is worth one node_modules probe and no more. - const extendsList = Array.isArray(parsed.extends) - ? parsed.extends.filter((entry): entry is string => typeof entry === 'string') - : (typeof parsed.extends === 'string' ? [parsed.extends] : []) - - if (depth < MAX_ALIAS_MANIFEST_EXTENDS_DEPTH) { - for (const entry of extendsList) { - const candidates = entry.startsWith('.') - ? [path.resolve(manifestDir, entry), path.resolve(manifestDir, `${entry}.json`)] - : [path.join(manifestDir, 'node_modules', entry), path.join(manifestDir, 'node_modules', entry, 'tsconfig.json')] - const parent = candidates.find(isReadableFile) - if (parent) { - // Own patterns win: the extending file overrides its base. - patterns.push(...collectAliasPatterns(parent, depth + 1)) - } - } - } - - return patterns -} - -const findAliasPatterns = (entryPath: string): AliasPattern[] => { - for (const manifest of PATH_ALIAS_MANIFESTS) { - const found = findUpwards(path.dirname(entryPath), manifest) - if (found) { - const patterns = collectAliasPatterns(found) - if (patterns.length > 0) { - return patterns - } - } - } - return [] -} - -const resolveAliasImport = (specifier: string, patterns: AliasPattern[]): string | undefined => { - for (const { prefix, suffix, targets, exact } of patterns) { - let wildcard: string | undefined - if (exact) { - if (specifier !== prefix) { - continue - } - wildcard = '' - } else { - if (!specifier.startsWith(prefix) || !specifier.endsWith(suffix) || - specifier.length < prefix.length + suffix.length) { - continue - } - wildcard = specifier.slice(prefix.length, specifier.length - (suffix.length || 0)) - } - - for (const target of targets) { - // Substituted by slicing rather than String.replace: `replace` with a string - // pattern interprets `$&` / `$'` in the REPLACEMENT, so a specifier carrying - // those characters would resolve to a corrupted path. TypeScript allows at most - // one `*` per target, so the first one is the substitution point by definition. - const star = target.indexOf('*') - const expanded = star === -1 - ? target - : target.slice(0, star) + wildcard + target.slice(star + 1) - const resolved = probeConfigCandidate(expanded) - if (resolved) { - return resolved - } - } - } - return undefined -} - /** * Collect local files a config pulls in (`wdio.shared.conf.ts` style splits), so a * captured config is not just a two-line file that spreads a base config we never see. * - * Two kinds of specifier are followed: RELATIVE ones, and bare ones that a tsconfig - * `paths` alias maps back into the project. Anything else is an npm package. Every - * candidate still has to be named like a config (`*.conf.*` / `*.config.*`), so an - * alias table pointing at `src/*` cannot turn this into a source-tree crawl, and depth - * and count stay capped. + * Only RELATIVE specifiers are followed — bare specifiers are npm packages, never the + * customer's own config. Depth and count are capped so this can never walk a source tree. */ const collectLocalImports = (entryPath: string, entryContent: string, budget: number): Array<{ filePath: string, content: string }> => { const found: Array<{ filePath: string, content: string }> = [] const seen = new Set([entryPath]) let frontier: Array<{ filePath: string, content: string }> = [{ filePath: entryPath, content: entryContent }] - // Resolved lazily: only an unresolved bare specifier needs the alias table, so a - // project that splits its config relatively never reads a tsconfig at all. - let aliasPatterns: AliasPattern[] | undefined for (let depth = 0; depth < CAPTURE_CONFIG_IMPORT_DEPTH && found.length < budget; depth++) { const next: Array<{ filePath: string, content: string }> = [] for (const { filePath, content } of frontier) { - // `from 'x'`, `import 'x'`, `import('x')`, `require('x')` - const importRegex = /(?:from|import|require)\s*\(?\s*['"]([^'"]+)['"]/g + // `from './x'`, `import './x'`, `import('./x')`, `require('./x')` + const importRegex = /(?:from|import|require)\s*\(?\s*['"](\.[^'"]*)['"]/g let match: RegExpExecArray | null while ((match = importRegex.exec(content)) !== null) { if (found.length >= budget) { break } - const specifier = match[1] - let resolved: string | undefined - if (specifier.startsWith('.')) { - resolved = resolveRelativeImport(specifier, filePath) - } else { - if (aliasPatterns === undefined) { - aliasPatterns = findAliasPatterns(entryPath) - } - resolved = aliasPatterns.length > 0 ? resolveAliasImport(specifier, aliasPatterns) : undefined - } + const resolved = resolveRelativeImport(match[1], filePath) if (!resolved || seen.has(resolved) || resolved.includes(`${path.sep}node_modules${path.sep}`)) { continue } diff --git a/packages/browserstack-service/src/constants.ts b/packages/browserstack-service/src/constants.ts index b88cd7c..74a08e6 100644 --- a/packages/browserstack-service/src/constants.ts +++ b/packages/browserstack-service/src/constants.ts @@ -85,11 +85,18 @@ export const MAX_CAPTURED_CONFIG_FILES = 6 export const CAPTURE_CONFIG_IMPORT_DEPTH = 1 /* How far to walk up from the config dir looking for the project's package.json */ export const MAX_PACKAGE_JSON_WALK_UP = 5 -/* JSONC manifests read for `compilerOptions.paths`, so a config that imports its siblings - through an alias (`@confs/wdio-main.conf`) is followed like a relative import */ -export const PATH_ALIAS_MANIFESTS = ['tsconfig.json', 'jsconfig.json'] -/* How many `extends` links to follow out of an alias manifest (a base plus overrides) */ -export const MAX_ALIAS_MANIFEST_EXTENDS_DEPTH = 3 + +/* WDIO hook names read out of the config file for diagnostics, in lifecycle order */ +export const WDIO_HOOK_NAMES = [ + 'onPrepare', 'onWorkerStart', 'onWorkerEnd', 'beforeSession', 'before', 'beforeSuite', + 'beforeHook', 'afterHook', 'beforeTest', 'afterTest', 'afterSuite', 'after', 'afterSession', + 'onComplete', 'onReload', 'beforeCommand', 'afterCommand', 'beforeFeature', 'afterFeature', + 'beforeScenario', 'afterScenario', 'beforeStep', 'afterStep' +] as const +/* Per-hook cap: enough for a real hook, small enough that the log stays readable */ +export const MAX_HOOK_SOURCE_CHARS = 8000 +/* Names the hooks-with-reloadSession list is published under, for the worker processes */ +export const BROWSERSTACK_HOOKS_WITH_RELOAD_SESSION = 'BROWSERSTACK_HOOKS_WITH_RELOAD_SESSION' /** * Keys whose line is scrubbed before a config file enters the archive. diff --git a/packages/browserstack-service/src/hookInstrumentation.ts b/packages/browserstack-service/src/hookInstrumentation.ts deleted file mode 100644 index 43c3f6b..0000000 --- a/packages/browserstack-service/src/hookInstrumentation.ts +++ /dev/null @@ -1,117 +0,0 @@ -import type { Options } from '@wdio/types' - -import { BStackLogger } from './bstackLogger.js' - -/** - * WDIO config hooks that execute in the worker process WITH a live session, so a driver - * command issued inside them is meaningful. - * - * Deliberately excluded: - * - `onPrepare` / `onWorkerStart` / `onWorkerEnd` / `onComplete` — launcher process, no driver. - * - `beforeSession` — runs before the session is created; there is no browser yet. - * - `afterSession` — the session is already being deleted; commands there fail. - * - `beforeCommand` / `afterCommand` — a live driver, but per-command: their window is the - * command, which is already observable, and wrapping them would log on every interaction. - */ -export const BROWSER_CONTEXT_HOOKS = [ - 'before', - 'beforeSuite', - 'beforeHook', - 'beforeTest', - 'afterTest', - 'afterHook', - 'afterSuite', - 'after' -] as const - -type HookFn = (...args: unknown[]) => unknown - -const INSTRUMENTED = Symbol('bstackHookInstrumented') - -/** - * Wrap one handler so its entry and exit are observable, preserving its shape exactly. - * - * Sync handlers stay sync: an `async` wrapper would turn every sync hook into a promise and a - * synchronous throw into a rejection, which changes ordering for anything that calls a hook - * without awaiting it. So the wrapper only attaches to the promise when the handler actually - * returned one. - */ -const instrument = (hookName: string, index: number, fn: HookFn): HookFn => { - if ((fn as unknown as Record)[INSTRUMENTED]) { - return fn - } - - // `fn.name` is the only identity WDIO leaves: ConfigParser binds every handler it folds in - // (`hook.bind(service)`), so both the user's config hooks and other services' hooks read as - // `bound `. Logged anyway — it separates a named user function from an anonymous one. - const label = `${hookName}#${index}${fn.name ? ` (${fn.name})` : ''}` - const wrapped = function (this: unknown, ...args: unknown[]) { - const startedAt = Date.now() - const finish = (outcome: string) => BStackLogger.debug(`[hook-window] ${label} ${outcome} in ${Date.now() - startedAt}ms`) - - BStackLogger.debug(`[hook-window] ${label} started`) - let result: unknown - try { - result = fn.apply(this, args) - } catch (error) { - finish(`threw: ${(error as Error)?.message}`) - throw error - } - - if (result && typeof (result as Promise).then === 'function') { - return (result as Promise).then( - (value) => { - finish('finished') - return value - }, - (error) => { - finish(`rejected: ${(error as Error)?.message}`) - throw error - } - ) - } - - finish('finished') - return result - } as HookFn - - Object.defineProperty(wrapped, INSTRUMENTED, { value: true }) - return wrapped -} - -/** - * Make the boundaries of every session-scoped config hook observable. - * - * WDIO tells a service nothing about the other handlers registered for a hook: `ConfigParser` - * folds the user's config hooks and every service's hooks into one array per hook name, and the - * runner fires them together (`executeHooksWithArgs` → `Promise.all`). Patching the array in - * place is therefore the only way to see when a handler that is not ours starts and finishes — - * which is what bounds the windows a driver command can fall into. - * - * The index in the label is the handler's position in that merged array, so it includes this - * service's own handlers; it identifies a handler across its start/finish pair, nothing more. - * - * Logging only, for now: no events are emitted and no behaviour changes. Idempotent, and any - * failure is swallowed — instrumentation must never be the reason a suite cannot start. - */ -export function instrumentBrowserContextHooks(config?: Options.Testrunner): void { - if (!config) { - return - } - - try { - const record = config as unknown as Record - for (const hookName of BROWSER_CONTEXT_HOOKS) { - const handlers = record[hookName] - if (!Array.isArray(handlers) || handlers.length === 0) { - continue - } - BStackLogger.debug(`[hook-window] instrumenting ${hookName}: ${handlers.length} handler(s) registered at service construction`) - record[hookName] = handlers.map((handler, index) => - typeof handler === 'function' ? instrument(hookName, index, handler as HookFn) : handler - ) - } - } catch (error) { - BStackLogger.debug(`Could not instrument config hooks: ${error}`) - } -} diff --git a/packages/browserstack-service/src/hookSources.ts b/packages/browserstack-service/src/hookSources.ts new file mode 100644 index 0000000..488c6da --- /dev/null +++ b/packages/browserstack-service/src/hookSources.ts @@ -0,0 +1,188 @@ +import fs from 'node:fs' + +import { MAX_CAPTURED_CONFIG_FILE_BYTES, MAX_HOOK_SOURCE_CHARS, WDIO_HOOK_NAMES } from './constants.js' + +export type HookSources = Record + +/** + * Walk forward from an opening brace to its match, so a hook body is captured whole. + * + * Brace counting has to be lexical or it breaks on ordinary config code: a `'}'` inside a + * selector string, a `//` comment mentioning a brace, or a template literal all close the body + * early and hand back a truncated function. + */ +const sliceBalanced = (text: string, openIndex: number): string | undefined => { + let depth = 0 + let quote: string | undefined + let lineComment = false + let blockComment = false + + for (let i = openIndex; i < text.length; i++) { + const char = text[i] + const next = text[i + 1] + + if (lineComment) { + if (char === '\n') { + lineComment = false + } + continue + } + if (blockComment) { + if (char === '*' && next === '/') { + blockComment = false + i++ + } + continue + } + if (quote) { + if (char === '\\') { + i++ + } else if (char === quote) { + quote = undefined + } + continue + } + if (char === '/' && next === '/') { + lineComment = true + i++ + continue + } + if (char === '/' && next === '*') { + blockComment = true + i++ + continue + } + if (char === '\'' || char === '"' || char === '`') { + quote = char + continue + } + if (char === '{') { + depth++ + continue + } + if (char === '}') { + depth-- + if (depth === 0) { + return text.slice(openIndex, i + 1) + } + } + } + return undefined +} + +/** + * Index of the brace that opens a hook's BODY, starting from its declaration. + * + * Not simply the next `{`: a typed parameter carries its own braces + * (`beforeTest: function (test: { title?: string })`), and taking the first one captures the + * type annotation instead of the body. So the parameter list is skipped by balancing its + * parentheses first. + */ +const findBodyBrace = (text: string, fromIndex: number): number | undefined => { + const limit = Math.min(text.length, fromIndex + 600) + let i = fromIndex + + const openParen = text.indexOf('(', fromIndex) + if (openParen !== -1 && openParen < limit) { + let depth = 0 + let quote: string | undefined + for (let j = openParen; j < text.length; j++) { + const char = text[j] + if (quote) { + if (char === '\\') { + j++ + } else if (char === quote) { + quote = undefined + } + continue + } + if (char === '\'' || char === '"' || char === '`') { + quote = char + continue + } + if (char === '(') { + depth++ + } else if (char === ')') { + depth-- + if (depth === 0) { + i = j + 1 + break + } + } + } + } + + const brace = text.indexOf('{', i) + return brace === -1 ? undefined : brace +} + +/** + * Source text of every WDIO hook the config file declares itself. + * + * Read from the file rather than from the parsed config on purpose: `ConfigParser` folds every + * hook it finds into an array as `hook.bind(service)`, and a bound function stringifies to + * `function () { [native code] }` — so the hooks are unreadable by the time a service can see + * them. The file is the only place the bodies survive. + * + * Limitation worth knowing when reading the output: only hooks written IN this file are found. + * A config that imports its hooks from elsewhere (`...require('./hooks.conf')`) yields the + * reference, not the body. + */ +export function extractUserHookSources(configPath: string): HookSources { + const sources: HookSources = {} + + let text: string + try { + if (fs.statSync(configPath).size > MAX_CAPTURED_CONFIG_FILE_BYTES) { + return sources + } + text = fs.readFileSync(configPath, 'utf8') + } catch { + return sources + } + + for (const hookName of WDIO_HOOK_NAMES) { + // `before:` as a property, or `before (` / `async before (` as a method shorthand. + // The negative lookbehind keeps `beforeTest` from matching a search for `before`. + const declaration = new RegExp(`(?`. A `;` or a stray `=` means we + // matched a mention of the name rather than its declaration. + if (!/^[A-Za-z0-9_$]+\s*:?\s*(?:async\s+)?(?:function\s*)?[A-Za-z0-9_$]*\s*(?:\([^;]*\))?\s*(?::[^;{]*)?\s*(?:=>)?\s*$/.test(head)) { + continue + } + const body = sliceBalanced(text, openIndex) + if (!body) { + continue + } + const captured = `${head.trim()}${body}` + sources[hookName] = captured.length > MAX_HOOK_SOURCE_CHARS + ? `${captured.slice(0, MAX_HOOK_SOURCE_CHARS)}… [truncated]` + : captured + break + } + } + + return sources +} + +/** + * Hook names whose source mentions `identifier` — e.g. which hooks call `reloadSession`. + */ +export function hooksUsing(sources: HookSources, identifier: string): string[] { + const needle = new RegExp(`(? needle.test(source)) + .map(([hookName]) => hookName) +} diff --git a/packages/browserstack-service/src/launcher.ts b/packages/browserstack-service/src/launcher.ts index 390e4e8..725af04 100644 --- a/packages/browserstack-service/src/launcher.ts +++ b/packages/browserstack-service/src/launcher.ts @@ -26,7 +26,8 @@ import { BROWSERSTACK_OBSERVABILITY, WDIO_NAMING_PREFIX, BROWSERSTACK_TEST_REPORTING, - TEST_REPORTING_PROJECT_NAME + TEST_REPORTING_PROJECT_NAME, + BROWSERSTACK_HOOKS_WITH_RELOAD_SESSION } from './constants.js' import { launchTestSession, @@ -52,6 +53,7 @@ import { } from './util.js' import CrashReporter from './crash-reporter.js' import { initWdioConfigPath, isAutoCaptureLogsDisabled, publishAutoCaptureDisabled } from './configCapture.js' +import { extractUserHookSources, hooksUsing, type HookSources } from './hookSources.js' import { finalizeOrphanedRuns } from './testOps/openRunsJournal.js' import { BStackLogger } from './bstackLogger.js' import { PercyLogger } from './Percy/PercyLogger.js' @@ -73,6 +75,7 @@ type BrowserstackLocal = BrowserstackLocalLauncher.Local & { } export default class BrowserstackLauncherService implements Services.ServiceInstance { + private _hookSources: HookSources = {} browserstackLocal?: BrowserstackLocal private _buildName?: string private _projectName?: string @@ -113,6 +116,8 @@ export default class BrowserstackLauncherService implements Services.ServiceInst process.env.TEST_OBSERVABILITY_PROJECT_NAME = process.env[TEST_REPORTING_PROJECT_NAME] } + this._hookSources = this._captureHookSources() + if (!isUndefined(process.env.TEST_REPORTING_BUILD_NAME)) { process.env.TEST_OBSERVABILITY_BUILD_NAME = process.env.TEST_REPORTING_BUILD_NAME } @@ -249,6 +254,50 @@ export default class BrowserstackLauncherService implements Services.ServiceInst } @PerformanceTester.Measure(PERFORMANCE_SDK_EVENTS.EVENTS.SDK_PRE_TEST) + /** + * Log the source of the hooks the user's config declares. + * + * Debugging an SDK issue that lives in a customer's hook currently means guessing: the + * uploaded logs carry the config file only when it is the entry file, and the parsed hooks + * are bound functions, which stringify to `[native code]`. Reading them out of the file puts + * the actual bodies in the log next to the failure. + * + * Also records which hooks call `reloadSession`, published on the environment so a worker can + * read it: a reload inside a hook changes the session under the driver, which is the shape + * behind several session-attribution defects. + */ + private _captureHookSources (): HookSources { + try { + const { configPath } = initWdioConfigPath(this._config) + if (!configPath) { + return {} + } + + const sources = extractUserHookSources(configPath) + const declared = Object.keys(sources) + if (declared.length === 0) { + BStackLogger.debug('user hook sources: none declared in the config file') + return sources + } + + BStackLogger.debug(`user hook sources (${declared.length}): ${declared.join(', ')}`) + for (const [hookName, source] of Object.entries(sources)) { + BStackLogger.debug(`user hook source [${hookName}]:\n${source}`) + } + + const reloadHooks = hooksUsing(sources, 'reloadSession') + if (reloadHooks.length > 0) { + BStackLogger.debug(`reloadSession used in hooks: ${reloadHooks.join(',')}`) + process.env[BROWSERSTACK_HOOKS_WITH_RELOAD_SESSION] = reloadHooks.join(',') + } + + return sources + } catch (error) { + BStackLogger.debug(`hook source capture failed: ${(error as Error).message}`) + return {} + } + } + async onPrepare (config: Options.Testrunner, capabilities: Capabilities.TestrunnerCapabilities | WebdriverIO.Capabilities) { PerformanceTester.start(PERFORMANCE_SDK_EVENTS.FRAMEWORK_EVENTS.INIT) diff --git a/packages/browserstack-service/src/service.ts b/packages/browserstack-service/src/service.ts index 4ce78cb..f1d04a2 100644 --- a/packages/browserstack-service/src/service.ts +++ b/packages/browserstack-service/src/service.ts @@ -24,7 +24,6 @@ import CustomTagsHandler from './custom-tags-handler.js' import { classifyMochaHookTitle, setCurrentMochaHookWindow } from './customTags.js' import type TestHubModule from './cli/modules/testHubModule.js' import { BStackLogger } from './bstackLogger.js' -import { instrumentBrowserContextHooks } from './hookInstrumentation.js' import PercyHandler from './Percy/Percy-Handler.js' import Listener from './testOps/listener.js' import { saveWorkerData } from './data-store.js' @@ -120,10 +119,6 @@ export default class BrowserstackService implements Services.ServiceInstance { const bailOpt: unknown = this._config?.mochaOpts?.bail this._mochaBail = this._config?.framework === 'mocha' && Boolean(bailOpt) && !isFalse(bailOpt) - // Make the boundaries of the user's own session-scoped config hooks observable. Logging - // only; patched here because the runner reads these arrays after the service is built. - instrumentBrowserContextHooks(this._config) - PerformanceTester.startMonitoring('performance-report-service.csv') if (shouldProcessEventForTesthub('')) { this._config.reporters?.push(TestReporter) diff --git a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts index 2e87cc5..869ddd5 100644 --- a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts +++ b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts @@ -39,19 +39,6 @@ vi.mock('../../../src/util.js', () => ({ o11yClassErrorHandler: vi.fn().mockImplementation((cls) => cls) })) -// hoisted: vi.mock factories are lifted above module scope, and `getInstance` is a plain -// function rather than a spy so the suite-wide clearAllMocks/resetAllMocks cannot strip its -// return value out from under the module constructor. -const { mockTrackEvent } = vi.hoisted(() => ({ mockTrackEvent: vi.fn() })) -vi.mock('../../../src/cli/index.js', () => ({ - BrowserstackCLI: { - getInstance: () => ({ - options: {}, - getTestFramework: () => ({ trackEvent: mockTrackEvent }) - }) - } -})) - vi.mock('../../../src/cli/grpcClient.js', () => ({ GrpcClient: { getInstance: vi.fn().mockReturnValue({ @@ -122,7 +109,6 @@ describe('AccessibilityModule', () => { afterEach(() => { vi.resetAllMocks() - mockTrackEvent.mockReset() }) describe('constructor', () => { @@ -213,91 +199,6 @@ describe('AccessibilityModule', () => { }) }) - describe('onBeforeExecute - pre-test scan gate (config-level before hook)', () => { - it('opens the scan gate at driver creation so config-level before() commands scan', async () => { - // WDIO's config-level before()/beforeSession() are not test-framework hooks, so - // neither onHookStart nor onBeforeTest has run when they fire their commands. - vi.mocked(validateCapsWithA11y).mockReturnValue(true) - vi.mocked(AutomationFramework.getState).mockImplementation((instance, key) => { - if (key.includes('CAPABILITIES')) { - return { browserName: 'chrome' } - } - return 12345 - }) - - await accessibilityModule.onBeforeExecute() - - expect(accessibilityModule.accessibilityMap.get(12345 as never)).toBe(true) - }) - - it('does not leave a hook run uuid set once the first test starts', async () => { - // The config-level hook window is tracked as a real BEFORE_ALL hook (service.before), - // so currentHookRunUuid comes from the framework. It must not survive into the test - // body: onHookEnd is the only other thing that clears it, and a tracked config hook - // whose POST never landed would otherwise stamp every test-body scan as a hook scan. - accessibilityModule.currentHookRunUuid = 'config-level-hook-uuid' - - await accessibilityModule.onBeforeTest({ suiteTitle: 'S', test: { title: 't' } }) - - expect(accessibilityModule.currentHookRunUuid).toBeNull() - }) - - it('leaves the gate closed when autoScanning is off', async () => { - vi.mocked(validateCapsWithA11y).mockReturnValue(true) - vi.mocked(AutomationFramework.getState).mockImplementation((instance, key) => { - if (key.includes('CAPABILITIES')) { - return { browserName: 'chrome' } - } - return 12345 - }) - accessibilityModule.autoScanning = false - - await accessibilityModule.onBeforeExecute() - - expect(accessibilityModule.accessibilityMap.has(12345 as never)).toBe(false) - }) - }) - - describe('pre-test window hook run', () => { - // The config-level before() window has no test-framework hook of its own, so scans from it - // had no parent. One is opened on demand — and ONLY on demand, or every suite in the fleet - // would report a hook it never ran. - it('opens a BEFORE_ALL hook run the first time a scan needs a parent, once only', async () => { - await accessibilityModule['ensurePreTestHookRun']() - await accessibilityModule['ensurePreTestHookRun']() - - const opens = mockTrackEvent.mock.calls.filter((c) => c[1] === HookState.PRE) - expect(opens).toHaveLength(1) - expect(opens[0][0]).toBe(TestFrameworkState.BEFORE_ALL) - expect((opens[0][2] as { test: { title: string } }).test.title).toContain('pre-test window') - }) - - it('opens nothing while a framework hook is already the scan parent', async () => { - accessibilityModule.currentHookRunUuid = 'framework-hook-uuid' - - await accessibilityModule['ensurePreTestHookRun']() - - expect(mockTrackEvent).not.toHaveBeenCalled() - }) - - it('closes the hook run when the first test starts', async () => { - await accessibilityModule['ensurePreTestHookRun']() - mockTrackEvent.mockClear() - - await accessibilityModule.onBeforeTest({ suiteTitle: 'S', test: { title: 't' } }) - - const closes = mockTrackEvent.mock.calls.filter((c) => c[1] === HookState.POST) - expect(closes).toHaveLength(1) - expect((closes[0][2] as { result: { passed: boolean } }).result.passed).toBe(true) - }) - - it('never closes a hook run it did not open', async () => { - await accessibilityModule.onBeforeTest({ suiteTitle: 'S', test: { title: 't' } }) - - expect(mockTrackEvent.mock.calls.filter((c) => c[1] === HookState.POST)).toHaveLength(0) - }) - }) - describe('onSessionReload', () => { it('carries the scan gate over to the new session id', () => { accessibilityModule.accessibilityMap.set('old' as never, true) diff --git a/packages/browserstack-service/tests/configCapture.test.ts b/packages/browserstack-service/tests/configCapture.test.ts index 0e4c99c..0224db9 100644 --- a/packages/browserstack-service/tests/configCapture.test.ts +++ b/packages/browserstack-service/tests/configCapture.test.ts @@ -513,97 +513,6 @@ describe('collectConfigFilesForUpload', () => { expect(files.map((f) => f.name)).toEqual(['wdio.conf.ts']) }) - it('follows a bare specifier that a tsconfig paths alias maps back into the project', () => { - // The shape that prompted this: a customer whose entry config imports every other - // config through an alias, so relative-only following captured the entry alone. - write('tsconfig.json', JSON.stringify({ - compilerOptions: { baseUrl: '.', paths: { '@confs/*': ['configs/*'] } } - })) - write('wdio.conf.ts', [ - "import { main } from '@confs/wdio-main.conf'", - 'export const config = { ...main }' - ].join('\n')) - write('configs/wdio-main.conf.ts', 'export const main = { maxInstances: 7 }') - - const { files } = collectConfigFilesForUpload({} as never) - - expect(files.map((f) => f.name).sort()).toEqual(['wdio-main.conf.ts', 'wdio.conf.ts']) - expect(files.find((f) => f.name === 'wdio-main.conf.ts')!.content).toContain('maxInstances: 7') - }) - - it('resolves an alias target against baseUrl, not the config directory', () => { - write('tsconfig.json', JSON.stringify({ - compilerOptions: { baseUrl: 'src', paths: { '@confs/*': ['wdio/*'] } } - })) - write('wdio.conf.ts', "import '@confs/base.conf'\nexport const config = {}") - write('src/wdio/base.conf.ts', 'export const base = { maxInstances: 3 }') - - const { files } = collectConfigFilesForUpload({} as never) - - expect(files.map((f) => f.name).sort()).toEqual(['base.conf.ts', 'wdio.conf.ts']) - }) - - it('reads an alias table out of JSONC (comments and trailing commas)', () => { - write('tsconfig.json', [ - '{', - ' // real tsconfigs are commented', - ' "compilerOptions": {', - ' /* block comment */', - ' "baseUrl": ".",', - ' "paths": { "@confs/*": ["configs/*"], },', - ' },', - '}' - ].join('\n')) - write('wdio.conf.ts', "import '@confs/base.conf'\nexport const config = {}") - write('configs/base.conf.ts', 'export const base = {}') - - const { files } = collectConfigFilesForUpload({} as never) - - expect(files.map((f) => f.name).sort()).toEqual(['base.conf.ts', 'wdio.conf.ts']) - }) - - it('picks up an alias table inherited through extends', () => { - write('tsconfig.base.json', JSON.stringify({ - compilerOptions: { baseUrl: '.', paths: { '@confs/*': ['configs/*'] } } - })) - write('tsconfig.json', JSON.stringify({ extends: './tsconfig.base.json' })) - write('wdio.conf.ts', "import '@confs/base.conf'\nexport const config = {}") - write('configs/base.conf.ts', 'export const base = {}') - - const { files } = collectConfigFilesForUpload({} as never) - - expect(files.map((f) => f.name).sort()).toEqual(['base.conf.ts', 'wdio.conf.ts']) - }) - - it('substitutes the wildcard literally, even when the specifier carries $ patterns', () => { - // String.replace interprets `$&` in the REPLACEMENT, which would have resolved this - // to `configs/a*b.conf.ts` — a path that does not exist. - write('tsconfig.json', JSON.stringify({ - compilerOptions: { baseUrl: '.', paths: { '@confs/*': ['configs/*'] } } - })) - write('wdio.conf.ts', "import '@confs/a$&b.conf'\nexport const config = {}") - write('configs/a$&b.conf.ts', 'export const base = {}') - - const { files } = collectConfigFilesForUpload({} as never) - - expect(files.map((f) => f.name).sort()).toEqual(['a$&b.conf.ts', 'wdio.conf.ts']) - }) - - it('does not let an alias table widen capture to ordinary source', () => { - // `@app/*` -> `src/*` is a normal alias, and following it would archive application - // code. The config-name filter is what keeps this a config capture. - write('tsconfig.json', JSON.stringify({ - compilerOptions: { baseUrl: '.', paths: { '@app/*': ['src/*'] } } - })) - write('wdio.conf.ts', "import { helper } from '@app/helpers/utils'\nexport const config = {}") - write('src/helpers/utils.ts', 'export const helper = 1 // ORDINARY_SOURCE') - - const { files } = collectConfigFilesForUpload({} as never) - - expect(files.map((f) => f.name)).toEqual(['wdio.conf.ts']) - expect(files.map((f) => f.content).join('\n')).not.toContain('ORDINARY_SOURCE') - }) - it('de-duplicates archive names when two captured files share a basename', () => { write('wdio.conf.ts', 'import "./nested/wdio.conf.ts"\nexport const config = {}') write('nested/wdio.conf.ts', 'export const config = {}') diff --git a/packages/browserstack-service/tests/hookSources.test.ts b/packages/browserstack-service/tests/hookSources.test.ts new file mode 100644 index 0000000..501f2a1 --- /dev/null +++ b/packages/browserstack-service/tests/hookSources.test.ts @@ -0,0 +1,122 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { describe, expect, it, beforeEach, afterEach } from 'vitest' + +import { extractUserHookSources, hooksUsing } from '../src/hookSources.js' + +let tmpRoot: string + +const write = (content: string): string => { + const file = path.join(tmpRoot, 'wdio.conf.ts') + fs.writeFileSync(file, content) + return file +} + +beforeEach(() => { + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'bstack-hooksrc-')) +}) + +afterEach(() => { + fs.rmSync(tmpRoot, { recursive: true, force: true }) +}) + +describe('extractUserHookSources', () => { + it('captures a function-expression hook whole', () => { + const file = write([ + 'export const config = {', + ' before: async function () {', + ' await browser.execute("mobile: startActivity", {})', + ' },', + ' specs: ["./a.js"]', + '}' + ].join('\n')) + + const sources = extractUserHookSources(file) + + expect(Object.keys(sources)).toEqual(['before']) + expect(sources.before).toContain('mobile: startActivity') + expect(sources.before).not.toContain('specs') + }) + + it('captures arrow and method-shorthand hooks', () => { + const file = write([ + 'export const config = {', + ' beforeTest: (test) => { console.log(test.title) },', + ' async afterSuite (suite) { await browser.deleteSession() }', + '}' + ].join('\n')) + + const sources = extractUserHookSources(file) + + expect(sources.beforeTest).toContain('test.title') + expect(sources.afterSuite).toContain('deleteSession') + }) + + it('skips a typed parameter list rather than capturing the type as the body', () => { + // `function (test: { title?: string })` — the first brace in this declaration belongs to + // the type annotation, not the hook. Taking it captured the signature and nothing else. + const file = write([ + 'export const config = {', + ' beforeTest: function (test: { title?: string }) {', + ' console.log(test.title)', + ' }', + '}' + ].join('\n')) + + const sources = extractUserHookSources(file) + + expect(sources.beforeTest).toContain('console.log(test.title)') + }) + + it('does not stop at a brace inside a string, comment or template literal', () => { + const file = write([ + 'export const config = {', + ' before: async function () {', + ' await $(\'//div[@id="a}b"]\').click() // a } in a comment', + ' const q = `template } literal`', + ' await browser.reloadSession()', + ' }', + '}' + ].join('\n')) + + const sources = extractUserHookSources(file) + + expect(sources.before).toContain('reloadSession') + expect(sources.before.trim().endsWith('}')).toBe(true) + }) + + it('does not let a prefix hook name match a longer one', () => { + const file = write([ + 'export const config = {', + ' beforeTest: function () { console.log("only beforeTest here") }', + '}' + ].join('\n')) + + const sources = extractUserHookSources(file) + + expect(Object.keys(sources)).toEqual(['beforeTest']) + }) + + it('returns nothing for a config that declares no hooks, and never throws on a bad path', () => { + expect(extractUserHookSources(write('export const config = { specs: ["./a.js"] }'))).toEqual({}) + expect(extractUserHookSources(path.join(tmpRoot, 'missing.conf.ts'))).toEqual({}) + }) +}) + +describe('hooksUsing', () => { + it('reports the hooks that call the identifier', () => { + const sources = { + before: 'before: async function () { await browser.reloadSession() }', + beforeTest: 'beforeTest: function () { console.log(1) }' + } + + expect(hooksUsing(sources, 'reloadSession')).toEqual(['before']) + }) + + it('does not match the identifier as part of a longer name', () => { + const sources = { before: 'before: function () { myReloadSessionHelper() }' } + + expect(hooksUsing(sources, 'reloadSession')).toEqual([]) + }) +}) From 516cae0c38d2bc306ddf215c798527169796733c Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2026 11:40:58 +0000 Subject: [PATCH 08/15] chore(changeset): auto-generate from PR template (patch) --- .changeset/pr-165.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.changeset/pr-165.md b/.changeset/pr-165.md index 9e4e40d..c083b66 100644 --- a/.changeset/pr-165.md +++ b/.changeset/pr-165.md @@ -2,6 +2,5 @@ "@wdio/browserstack-service": patch --- -- Fixed accessibility scans not running for driver commands issued from a WDIO config-level `before()` hook. - Fixed accessibility scanning stopping for the rest of a test after `browser.reloadSession()`. -- Configuration files imported through TypeScript path aliases are now included in uploaded SDK logs, so support can see the config that actually ran. +- The hooks defined in your WDIO config are now included in the debug logs, so support can see the code behind a failure. From 2ddef19adb81e16a9ce0e57e6b14b134ec8d109e Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 17:45:40 +0530 Subject: [PATCH 09/15] fix(hooks): redact captured hook sources before they reach the log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hook bodies are customer code and the debug log is uploaded, so the captured source now goes through redactSensitiveContent — the same routine that guards an uploaded config file. BStackLogger's own scrub is not enough on its own. Measured against a hook body carrying nine sensitive shapes, it leaves six readable: authToken, password, clientSecret, a snake_case AWS secret, URL userinfo and an inline `token:` value. It knows only user/key/userName/accessKey in a key-value or query-string position. The identifier scan moved ahead of redaction, which is not cosmetic: redaction replaces the whole matching line, so `await browser.reloadSession({ userName, accessKey })` — reloading with fresh credentials, a real pattern — collapses to [REDACTED] and the call would disappear from the detection. Raw text is scanned for the tracked identifiers, then the stored copy is redacted; extractUserHookSources returns both, and hooksUsing documents that it can only be as complete as the text handed to it. Test fixtures use neutral sentinels rather than realistic secrets — the redactor keys on the field name, not the value shape, so nothing is lost by not committing token-shaped strings. SDK-7422 --- .../browserstack-service/src/constants.ts | 2 + .../browserstack-service/src/hookSources.ts | 52 +++++++++++++---- packages/browserstack-service/src/launcher.ts | 8 ++- .../tests/hookSources.test.ts | 57 ++++++++++++++++--- 4 files changed, 99 insertions(+), 20 deletions(-) diff --git a/packages/browserstack-service/src/constants.ts b/packages/browserstack-service/src/constants.ts index 74a08e6..fb026e8 100644 --- a/packages/browserstack-service/src/constants.ts +++ b/packages/browserstack-service/src/constants.ts @@ -93,6 +93,8 @@ export const WDIO_HOOK_NAMES = [ 'onComplete', 'onReload', 'beforeCommand', 'afterCommand', 'beforeFeature', 'afterFeature', 'beforeScenario', 'afterScenario', 'beforeStep', 'afterStep' ] as const +/* Identifiers whose use inside a hook changes how a session behaves, so worth recording */ +export const TRACKED_HOOK_IDENTIFIERS = ['reloadSession'] as const /* Per-hook cap: enough for a real hook, small enough that the log stays readable */ export const MAX_HOOK_SOURCE_CHARS = 8000 /* Names the hooks-with-reloadSession list is published under, for the worker processes */ diff --git a/packages/browserstack-service/src/hookSources.ts b/packages/browserstack-service/src/hookSources.ts index 488c6da..a6943d6 100644 --- a/packages/browserstack-service/src/hookSources.ts +++ b/packages/browserstack-service/src/hookSources.ts @@ -1,9 +1,17 @@ import fs from 'node:fs' -import { MAX_CAPTURED_CONFIG_FILE_BYTES, MAX_HOOK_SOURCE_CHARS, WDIO_HOOK_NAMES } from './constants.js' +import { redactSensitiveContent } from './configCapture.js' +import { MAX_CAPTURED_CONFIG_FILE_BYTES, MAX_HOOK_SOURCE_CHARS, TRACKED_HOOK_IDENTIFIERS, WDIO_HOOK_NAMES } from './constants.js' export type HookSources = Record +export interface ExtractedHooks { + /** hook name -> source, REDACTED and therefore safe to log and upload */ + sources: HookSources + /** tracked identifier -> the hooks that call it, derived BEFORE redaction */ + identifiers: Record +} + /** * Walk forward from an opening brace to its match, so a hook body is captured whole. * @@ -128,17 +136,19 @@ const findBodyBrace = (text: string, fromIndex: number): number | undefined => { * A config that imports its hooks from elsewhere (`...require('./hooks.conf')`) yields the * reference, not the body. */ -export function extractUserHookSources(configPath: string): HookSources { +export function extractUserHookSources(configPath: string): ExtractedHooks { const sources: HookSources = {} + const identifiers: Record = {} + const empty = { sources, identifiers } let text: string try { if (fs.statSync(configPath).size > MAX_CAPTURED_CONFIG_FILE_BYTES) { - return sources + return empty } text = fs.readFileSync(configPath, 'utf8') } catch { - return sources + return empty } for (const hookName of WDIO_HOOK_NAMES) { @@ -167,22 +177,44 @@ export function extractUserHookSources(configPath: string): HookSources { continue } const captured = `${head.trim()}${body}` - sources[hookName] = captured.length > MAX_HOOK_SOURCE_CHARS - ? `${captured.slice(0, MAX_HOOK_SOURCE_CHARS)}… [truncated]` - : captured + + // Identifier scan runs on the RAW body, before redaction. redactSensitiveContent + // replaces a whole matching LINE, so `await browser.reloadSession({ userName, accessKey })` + // — reloading with fresh credentials, a real pattern — collapses to `[REDACTED]` and the + // call disappears. Scanning first keeps the detection while the stored copy stays safe. + for (const identifier of TRACKED_HOOK_IDENTIFIERS) { + if (mentions(captured, identifier)) { + identifiers[identifier] = [...(identifiers[identifier] || []), hookName] + } + } + + // Hook bodies are customer code and this log is uploaded, so the stored copy is + // redacted with the same routine that guards an uploaded config file. The logger's own + // scrub is narrower — it misses authToken, password, clientSecret, URL userinfo and PEM + // blocks — so relying on it would publish those in the clear. + const safe = redactSensitiveContent(captured) + sources[hookName] = safe.length > MAX_HOOK_SOURCE_CHARS + ? `${safe.slice(0, MAX_HOOK_SOURCE_CHARS)}… [truncated]` + : safe break } } - return sources + return { sources, identifiers } } +const mentions = (source: string, identifier: string): boolean => + new RegExp(`(? needle.test(source)) + .filter(([, source]) => mentions(source, identifier)) .map(([hookName]) => hookName) } diff --git a/packages/browserstack-service/src/launcher.ts b/packages/browserstack-service/src/launcher.ts index 725af04..5e91b84 100644 --- a/packages/browserstack-service/src/launcher.ts +++ b/packages/browserstack-service/src/launcher.ts @@ -53,7 +53,7 @@ import { } from './util.js' import CrashReporter from './crash-reporter.js' import { initWdioConfigPath, isAutoCaptureLogsDisabled, publishAutoCaptureDisabled } from './configCapture.js' -import { extractUserHookSources, hooksUsing, type HookSources } from './hookSources.js' +import { extractUserHookSources, type HookSources } from './hookSources.js' import { finalizeOrphanedRuns } from './testOps/openRunsJournal.js' import { BStackLogger } from './bstackLogger.js' import { PercyLogger } from './Percy/PercyLogger.js' @@ -273,7 +273,7 @@ export default class BrowserstackLauncherService implements Services.ServiceInst return {} } - const sources = extractUserHookSources(configPath) + const { sources, identifiers } = extractUserHookSources(configPath) const declared = Object.keys(sources) if (declared.length === 0) { BStackLogger.debug('user hook sources: none declared in the config file') @@ -285,7 +285,9 @@ export default class BrowserstackLauncherService implements Services.ServiceInst BStackLogger.debug(`user hook source [${hookName}]:\n${source}`) } - const reloadHooks = hooksUsing(sources, 'reloadSession') + // Read from `identifiers`, not from the redacted sources: redaction drops a whole + // line, so a reloadSession call sharing a line with credentials would vanish. + const reloadHooks = identifiers.reloadSession || [] if (reloadHooks.length > 0) { BStackLogger.debug(`reloadSession used in hooks: ${reloadHooks.join(',')}`) process.env[BROWSERSTACK_HOOKS_WITH_RELOAD_SESSION] = reloadHooks.join(',') diff --git a/packages/browserstack-service/tests/hookSources.test.ts b/packages/browserstack-service/tests/hookSources.test.ts index 501f2a1..36be7a8 100644 --- a/packages/browserstack-service/tests/hookSources.test.ts +++ b/packages/browserstack-service/tests/hookSources.test.ts @@ -32,7 +32,7 @@ describe('extractUserHookSources', () => { '}' ].join('\n')) - const sources = extractUserHookSources(file) + const { sources } = extractUserHookSources(file) expect(Object.keys(sources)).toEqual(['before']) expect(sources.before).toContain('mobile: startActivity') @@ -47,7 +47,7 @@ describe('extractUserHookSources', () => { '}' ].join('\n')) - const sources = extractUserHookSources(file) + const { sources } = extractUserHookSources(file) expect(sources.beforeTest).toContain('test.title') expect(sources.afterSuite).toContain('deleteSession') @@ -64,7 +64,7 @@ describe('extractUserHookSources', () => { '}' ].join('\n')) - const sources = extractUserHookSources(file) + const { sources } = extractUserHookSources(file) expect(sources.beforeTest).toContain('console.log(test.title)') }) @@ -80,7 +80,7 @@ describe('extractUserHookSources', () => { '}' ].join('\n')) - const sources = extractUserHookSources(file) + const { sources } = extractUserHookSources(file) expect(sources.before).toContain('reloadSession') expect(sources.before.trim().endsWith('}')).toBe(true) @@ -93,14 +93,57 @@ describe('extractUserHookSources', () => { '}' ].join('\n')) - const sources = extractUserHookSources(file) + const { sources } = extractUserHookSources(file) expect(Object.keys(sources)).toEqual(['beforeTest']) }) it('returns nothing for a config that declares no hooks, and never throws on a bad path', () => { - expect(extractUserHookSources(write('export const config = { specs: ["./a.js"] }'))).toEqual({}) - expect(extractUserHookSources(path.join(tmpRoot, 'missing.conf.ts'))).toEqual({}) + expect(extractUserHookSources(write('export const config = { specs: ["./a.js"] }')).sources).toEqual({}) + expect(extractUserHookSources(path.join(tmpRoot, 'missing.conf.ts')).sources).toEqual({}) + }) +}) + +describe('redaction of captured hook sources', () => { + it('redacts credentials the logger\'s own scrub misses', () => { + // BStackLogger.redactCredentials only knows user/key/userName/accessKey, so these six + // shapes reached the uploaded log in the clear before this went through + // redactSensitiveContent. + const file = write([ + 'export const config = {', + ' before: async function () {', + ' const authToken = "SENTINEL_authToken_value"', + ' const password = "SENTINEL_password_value"', + ' const clientSecret = "SENTINEL_clientSecret_value"', + ' const AWS_SECRET_ACCESS_KEY = "SENTINEL_awsSecret_value"', + ' await browser.url("https://admin:SENTINEL_userinfo@internal.corp/login")', + ' await browser.execute("login", { token: "SENTINEL_token_value" })', + ' }', + '}' + ].join('\n')) + + const { sources } = extractUserHookSources(file) + + for (const secret of ['SENTINEL_authToken_value', 'SENTINEL_password_value', 'SENTINEL_clientSecret_value', 'SENTINEL_awsSecret_value', 'SENTINEL_userinfo', 'SENTINEL_token_value']) { + expect(sources.before).not.toContain(secret) + } + expect(sources.before).toContain('[REDACTED]') + }) + + it('still reports reloadSession when its line carries credentials', () => { + // Redaction replaces the whole line, so scanning the redacted copy would lose this call. + const file = write([ + 'export const config = {', + ' before: async function () {', + ' await browser.reloadSession({ userName: "u", accessKey: "k" })', + ' }', + '}' + ].join('\n')) + + const { sources, identifiers } = extractUserHookSources(file) + + expect(identifiers.reloadSession).toEqual(['before']) + expect(sources.before).not.toContain('"k"') }) }) From 71b396666aaca914695ee4c4c266be9e51afec72 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 17:48:53 +0530 Subject: [PATCH 10/15] revert(hooks): move hook-source logging out of this PR Everything that writes to the uploaded logs leaves this branch, so the PR is the reloadSession scan-gate fix and nothing else. configCapture, constants and launcher are identical to main again. Parked on SDK-7422-hook-source-logging, complete with the redaction fix: reading a customer's hook bodies into an uploaded log needs its own review, on its own timeline, not as a rider on a scan-gate fix. SDK-7422 --- .../browserstack-service/src/constants.ts | 14 -- .../browserstack-service/src/hookSources.ts | 220 ------------------ packages/browserstack-service/src/launcher.ts | 53 +---- .../tests/hookSources.test.ts | 165 ------------- 4 files changed, 1 insertion(+), 451 deletions(-) delete mode 100644 packages/browserstack-service/src/hookSources.ts delete mode 100644 packages/browserstack-service/tests/hookSources.test.ts diff --git a/packages/browserstack-service/src/constants.ts b/packages/browserstack-service/src/constants.ts index fb026e8..c64bfa8 100644 --- a/packages/browserstack-service/src/constants.ts +++ b/packages/browserstack-service/src/constants.ts @@ -86,20 +86,6 @@ export const CAPTURE_CONFIG_IMPORT_DEPTH = 1 /* How far to walk up from the config dir looking for the project's package.json */ export const MAX_PACKAGE_JSON_WALK_UP = 5 -/* WDIO hook names read out of the config file for diagnostics, in lifecycle order */ -export const WDIO_HOOK_NAMES = [ - 'onPrepare', 'onWorkerStart', 'onWorkerEnd', 'beforeSession', 'before', 'beforeSuite', - 'beforeHook', 'afterHook', 'beforeTest', 'afterTest', 'afterSuite', 'after', 'afterSession', - 'onComplete', 'onReload', 'beforeCommand', 'afterCommand', 'beforeFeature', 'afterFeature', - 'beforeScenario', 'afterScenario', 'beforeStep', 'afterStep' -] as const -/* Identifiers whose use inside a hook changes how a session behaves, so worth recording */ -export const TRACKED_HOOK_IDENTIFIERS = ['reloadSession'] as const -/* Per-hook cap: enough for a real hook, small enough that the log stays readable */ -export const MAX_HOOK_SOURCE_CHARS = 8000 -/* Names the hooks-with-reloadSession list is published under, for the worker processes */ -export const BROWSERSTACK_HOOKS_WITH_RELOAD_SESSION = 'BROWSERSTACK_HOOKS_WITH_RELOAD_SESSION' - /** * Keys whose line is scrubbed before a config file enters the archive. * `user` / `key` are WDIO's own top-level credential options, hence the bare entries. diff --git a/packages/browserstack-service/src/hookSources.ts b/packages/browserstack-service/src/hookSources.ts deleted file mode 100644 index a6943d6..0000000 --- a/packages/browserstack-service/src/hookSources.ts +++ /dev/null @@ -1,220 +0,0 @@ -import fs from 'node:fs' - -import { redactSensitiveContent } from './configCapture.js' -import { MAX_CAPTURED_CONFIG_FILE_BYTES, MAX_HOOK_SOURCE_CHARS, TRACKED_HOOK_IDENTIFIERS, WDIO_HOOK_NAMES } from './constants.js' - -export type HookSources = Record - -export interface ExtractedHooks { - /** hook name -> source, REDACTED and therefore safe to log and upload */ - sources: HookSources - /** tracked identifier -> the hooks that call it, derived BEFORE redaction */ - identifiers: Record -} - -/** - * Walk forward from an opening brace to its match, so a hook body is captured whole. - * - * Brace counting has to be lexical or it breaks on ordinary config code: a `'}'` inside a - * selector string, a `//` comment mentioning a brace, or a template literal all close the body - * early and hand back a truncated function. - */ -const sliceBalanced = (text: string, openIndex: number): string | undefined => { - let depth = 0 - let quote: string | undefined - let lineComment = false - let blockComment = false - - for (let i = openIndex; i < text.length; i++) { - const char = text[i] - const next = text[i + 1] - - if (lineComment) { - if (char === '\n') { - lineComment = false - } - continue - } - if (blockComment) { - if (char === '*' && next === '/') { - blockComment = false - i++ - } - continue - } - if (quote) { - if (char === '\\') { - i++ - } else if (char === quote) { - quote = undefined - } - continue - } - if (char === '/' && next === '/') { - lineComment = true - i++ - continue - } - if (char === '/' && next === '*') { - blockComment = true - i++ - continue - } - if (char === '\'' || char === '"' || char === '`') { - quote = char - continue - } - if (char === '{') { - depth++ - continue - } - if (char === '}') { - depth-- - if (depth === 0) { - return text.slice(openIndex, i + 1) - } - } - } - return undefined -} - -/** - * Index of the brace that opens a hook's BODY, starting from its declaration. - * - * Not simply the next `{`: a typed parameter carries its own braces - * (`beforeTest: function (test: { title?: string })`), and taking the first one captures the - * type annotation instead of the body. So the parameter list is skipped by balancing its - * parentheses first. - */ -const findBodyBrace = (text: string, fromIndex: number): number | undefined => { - const limit = Math.min(text.length, fromIndex + 600) - let i = fromIndex - - const openParen = text.indexOf('(', fromIndex) - if (openParen !== -1 && openParen < limit) { - let depth = 0 - let quote: string | undefined - for (let j = openParen; j < text.length; j++) { - const char = text[j] - if (quote) { - if (char === '\\') { - j++ - } else if (char === quote) { - quote = undefined - } - continue - } - if (char === '\'' || char === '"' || char === '`') { - quote = char - continue - } - if (char === '(') { - depth++ - } else if (char === ')') { - depth-- - if (depth === 0) { - i = j + 1 - break - } - } - } - } - - const brace = text.indexOf('{', i) - return brace === -1 ? undefined : brace -} - -/** - * Source text of every WDIO hook the config file declares itself. - * - * Read from the file rather than from the parsed config on purpose: `ConfigParser` folds every - * hook it finds into an array as `hook.bind(service)`, and a bound function stringifies to - * `function () { [native code] }` — so the hooks are unreadable by the time a service can see - * them. The file is the only place the bodies survive. - * - * Limitation worth knowing when reading the output: only hooks written IN this file are found. - * A config that imports its hooks from elsewhere (`...require('./hooks.conf')`) yields the - * reference, not the body. - */ -export function extractUserHookSources(configPath: string): ExtractedHooks { - const sources: HookSources = {} - const identifiers: Record = {} - const empty = { sources, identifiers } - - let text: string - try { - if (fs.statSync(configPath).size > MAX_CAPTURED_CONFIG_FILE_BYTES) { - return empty - } - text = fs.readFileSync(configPath, 'utf8') - } catch { - return empty - } - - for (const hookName of WDIO_HOOK_NAMES) { - // `before:` as a property, or `before (` / `async before (` as a method shorthand. - // The negative lookbehind keeps `beforeTest` from matching a search for `before`. - const declaration = new RegExp(`(?`. A `;` or a stray `=` means we - // matched a mention of the name rather than its declaration. - if (!/^[A-Za-z0-9_$]+\s*:?\s*(?:async\s+)?(?:function\s*)?[A-Za-z0-9_$]*\s*(?:\([^;]*\))?\s*(?::[^;{]*)?\s*(?:=>)?\s*$/.test(head)) { - continue - } - const body = sliceBalanced(text, openIndex) - if (!body) { - continue - } - const captured = `${head.trim()}${body}` - - // Identifier scan runs on the RAW body, before redaction. redactSensitiveContent - // replaces a whole matching LINE, so `await browser.reloadSession({ userName, accessKey })` - // — reloading with fresh credentials, a real pattern — collapses to `[REDACTED]` and the - // call disappears. Scanning first keeps the detection while the stored copy stays safe. - for (const identifier of TRACKED_HOOK_IDENTIFIERS) { - if (mentions(captured, identifier)) { - identifiers[identifier] = [...(identifiers[identifier] || []), hookName] - } - } - - // Hook bodies are customer code and this log is uploaded, so the stored copy is - // redacted with the same routine that guards an uploaded config file. The logger's own - // scrub is narrower — it misses authToken, password, clientSecret, URL userinfo and PEM - // blocks — so relying on it would publish those in the clear. - const safe = redactSensitiveContent(captured) - sources[hookName] = safe.length > MAX_HOOK_SOURCE_CHARS - ? `${safe.slice(0, MAX_HOOK_SOURCE_CHARS)}… [truncated]` - : safe - break - } - } - - return { sources, identifiers } -} - -const mentions = (source: string, identifier: string): boolean => - new RegExp(`(? mentions(source, identifier)) - .map(([hookName]) => hookName) -} diff --git a/packages/browserstack-service/src/launcher.ts b/packages/browserstack-service/src/launcher.ts index 5e91b84..390e4e8 100644 --- a/packages/browserstack-service/src/launcher.ts +++ b/packages/browserstack-service/src/launcher.ts @@ -26,8 +26,7 @@ import { BROWSERSTACK_OBSERVABILITY, WDIO_NAMING_PREFIX, BROWSERSTACK_TEST_REPORTING, - TEST_REPORTING_PROJECT_NAME, - BROWSERSTACK_HOOKS_WITH_RELOAD_SESSION + TEST_REPORTING_PROJECT_NAME } from './constants.js' import { launchTestSession, @@ -53,7 +52,6 @@ import { } from './util.js' import CrashReporter from './crash-reporter.js' import { initWdioConfigPath, isAutoCaptureLogsDisabled, publishAutoCaptureDisabled } from './configCapture.js' -import { extractUserHookSources, type HookSources } from './hookSources.js' import { finalizeOrphanedRuns } from './testOps/openRunsJournal.js' import { BStackLogger } from './bstackLogger.js' import { PercyLogger } from './Percy/PercyLogger.js' @@ -75,7 +73,6 @@ type BrowserstackLocal = BrowserstackLocalLauncher.Local & { } export default class BrowserstackLauncherService implements Services.ServiceInstance { - private _hookSources: HookSources = {} browserstackLocal?: BrowserstackLocal private _buildName?: string private _projectName?: string @@ -116,8 +113,6 @@ export default class BrowserstackLauncherService implements Services.ServiceInst process.env.TEST_OBSERVABILITY_PROJECT_NAME = process.env[TEST_REPORTING_PROJECT_NAME] } - this._hookSources = this._captureHookSources() - if (!isUndefined(process.env.TEST_REPORTING_BUILD_NAME)) { process.env.TEST_OBSERVABILITY_BUILD_NAME = process.env.TEST_REPORTING_BUILD_NAME } @@ -254,52 +249,6 @@ export default class BrowserstackLauncherService implements Services.ServiceInst } @PerformanceTester.Measure(PERFORMANCE_SDK_EVENTS.EVENTS.SDK_PRE_TEST) - /** - * Log the source of the hooks the user's config declares. - * - * Debugging an SDK issue that lives in a customer's hook currently means guessing: the - * uploaded logs carry the config file only when it is the entry file, and the parsed hooks - * are bound functions, which stringify to `[native code]`. Reading them out of the file puts - * the actual bodies in the log next to the failure. - * - * Also records which hooks call `reloadSession`, published on the environment so a worker can - * read it: a reload inside a hook changes the session under the driver, which is the shape - * behind several session-attribution defects. - */ - private _captureHookSources (): HookSources { - try { - const { configPath } = initWdioConfigPath(this._config) - if (!configPath) { - return {} - } - - const { sources, identifiers } = extractUserHookSources(configPath) - const declared = Object.keys(sources) - if (declared.length === 0) { - BStackLogger.debug('user hook sources: none declared in the config file') - return sources - } - - BStackLogger.debug(`user hook sources (${declared.length}): ${declared.join(', ')}`) - for (const [hookName, source] of Object.entries(sources)) { - BStackLogger.debug(`user hook source [${hookName}]:\n${source}`) - } - - // Read from `identifiers`, not from the redacted sources: redaction drops a whole - // line, so a reloadSession call sharing a line with credentials would vanish. - const reloadHooks = identifiers.reloadSession || [] - if (reloadHooks.length > 0) { - BStackLogger.debug(`reloadSession used in hooks: ${reloadHooks.join(',')}`) - process.env[BROWSERSTACK_HOOKS_WITH_RELOAD_SESSION] = reloadHooks.join(',') - } - - return sources - } catch (error) { - BStackLogger.debug(`hook source capture failed: ${(error as Error).message}`) - return {} - } - } - async onPrepare (config: Options.Testrunner, capabilities: Capabilities.TestrunnerCapabilities | WebdriverIO.Capabilities) { PerformanceTester.start(PERFORMANCE_SDK_EVENTS.FRAMEWORK_EVENTS.INIT) diff --git a/packages/browserstack-service/tests/hookSources.test.ts b/packages/browserstack-service/tests/hookSources.test.ts deleted file mode 100644 index 36be7a8..0000000 --- a/packages/browserstack-service/tests/hookSources.test.ts +++ /dev/null @@ -1,165 +0,0 @@ -import fs from 'node:fs' -import os from 'node:os' -import path from 'node:path' -import { describe, expect, it, beforeEach, afterEach } from 'vitest' - -import { extractUserHookSources, hooksUsing } from '../src/hookSources.js' - -let tmpRoot: string - -const write = (content: string): string => { - const file = path.join(tmpRoot, 'wdio.conf.ts') - fs.writeFileSync(file, content) - return file -} - -beforeEach(() => { - tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'bstack-hooksrc-')) -}) - -afterEach(() => { - fs.rmSync(tmpRoot, { recursive: true, force: true }) -}) - -describe('extractUserHookSources', () => { - it('captures a function-expression hook whole', () => { - const file = write([ - 'export const config = {', - ' before: async function () {', - ' await browser.execute("mobile: startActivity", {})', - ' },', - ' specs: ["./a.js"]', - '}' - ].join('\n')) - - const { sources } = extractUserHookSources(file) - - expect(Object.keys(sources)).toEqual(['before']) - expect(sources.before).toContain('mobile: startActivity') - expect(sources.before).not.toContain('specs') - }) - - it('captures arrow and method-shorthand hooks', () => { - const file = write([ - 'export const config = {', - ' beforeTest: (test) => { console.log(test.title) },', - ' async afterSuite (suite) { await browser.deleteSession() }', - '}' - ].join('\n')) - - const { sources } = extractUserHookSources(file) - - expect(sources.beforeTest).toContain('test.title') - expect(sources.afterSuite).toContain('deleteSession') - }) - - it('skips a typed parameter list rather than capturing the type as the body', () => { - // `function (test: { title?: string })` — the first brace in this declaration belongs to - // the type annotation, not the hook. Taking it captured the signature and nothing else. - const file = write([ - 'export const config = {', - ' beforeTest: function (test: { title?: string }) {', - ' console.log(test.title)', - ' }', - '}' - ].join('\n')) - - const { sources } = extractUserHookSources(file) - - expect(sources.beforeTest).toContain('console.log(test.title)') - }) - - it('does not stop at a brace inside a string, comment or template literal', () => { - const file = write([ - 'export const config = {', - ' before: async function () {', - ' await $(\'//div[@id="a}b"]\').click() // a } in a comment', - ' const q = `template } literal`', - ' await browser.reloadSession()', - ' }', - '}' - ].join('\n')) - - const { sources } = extractUserHookSources(file) - - expect(sources.before).toContain('reloadSession') - expect(sources.before.trim().endsWith('}')).toBe(true) - }) - - it('does not let a prefix hook name match a longer one', () => { - const file = write([ - 'export const config = {', - ' beforeTest: function () { console.log("only beforeTest here") }', - '}' - ].join('\n')) - - const { sources } = extractUserHookSources(file) - - expect(Object.keys(sources)).toEqual(['beforeTest']) - }) - - it('returns nothing for a config that declares no hooks, and never throws on a bad path', () => { - expect(extractUserHookSources(write('export const config = { specs: ["./a.js"] }')).sources).toEqual({}) - expect(extractUserHookSources(path.join(tmpRoot, 'missing.conf.ts')).sources).toEqual({}) - }) -}) - -describe('redaction of captured hook sources', () => { - it('redacts credentials the logger\'s own scrub misses', () => { - // BStackLogger.redactCredentials only knows user/key/userName/accessKey, so these six - // shapes reached the uploaded log in the clear before this went through - // redactSensitiveContent. - const file = write([ - 'export const config = {', - ' before: async function () {', - ' const authToken = "SENTINEL_authToken_value"', - ' const password = "SENTINEL_password_value"', - ' const clientSecret = "SENTINEL_clientSecret_value"', - ' const AWS_SECRET_ACCESS_KEY = "SENTINEL_awsSecret_value"', - ' await browser.url("https://admin:SENTINEL_userinfo@internal.corp/login")', - ' await browser.execute("login", { token: "SENTINEL_token_value" })', - ' }', - '}' - ].join('\n')) - - const { sources } = extractUserHookSources(file) - - for (const secret of ['SENTINEL_authToken_value', 'SENTINEL_password_value', 'SENTINEL_clientSecret_value', 'SENTINEL_awsSecret_value', 'SENTINEL_userinfo', 'SENTINEL_token_value']) { - expect(sources.before).not.toContain(secret) - } - expect(sources.before).toContain('[REDACTED]') - }) - - it('still reports reloadSession when its line carries credentials', () => { - // Redaction replaces the whole line, so scanning the redacted copy would lose this call. - const file = write([ - 'export const config = {', - ' before: async function () {', - ' await browser.reloadSession({ userName: "u", accessKey: "k" })', - ' }', - '}' - ].join('\n')) - - const { sources, identifiers } = extractUserHookSources(file) - - expect(identifiers.reloadSession).toEqual(['before']) - expect(sources.before).not.toContain('"k"') - }) -}) - -describe('hooksUsing', () => { - it('reports the hooks that call the identifier', () => { - const sources = { - before: 'before: async function () { await browser.reloadSession() }', - beforeTest: 'beforeTest: function () { console.log(1) }' - } - - expect(hooksUsing(sources, 'reloadSession')).toEqual(['before']) - }) - - it('does not match the identifier as part of a longer name', () => { - const sources = { before: 'before: function () { myReloadSessionHelper() }' } - - expect(hooksUsing(sources, 'reloadSession')).toEqual([]) - }) -}) From 4803e0f2eed6f7c25d4e490a765c280b50846c26 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2026 12:20:41 +0000 Subject: [PATCH 11/15] chore(changeset): auto-generate from PR template (patch) --- .changeset/pr-165.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.changeset/pr-165.md b/.changeset/pr-165.md index c083b66..b911381 100644 --- a/.changeset/pr-165.md +++ b/.changeset/pr-165.md @@ -2,5 +2,4 @@ "@wdio/browserstack-service": patch --- -- Fixed accessibility scanning stopping for the rest of a test after `browser.reloadSession()`. -- The hooks defined in your WDIO config are now included in the debug logs, so support can see the code behind a failure. +- Fixed accessibility scanning stopping for the remainder of a test after `browser.reloadSession()`. From 5da67af0fdb5172d2ddea7b06eea12d5a21a032a Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 18:04:32 +0530 Subject: [PATCH 12/15] fix(app-a11y): migrate the classic scan state only in the non-CLI flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review catch: the classic handler's migration ran in both flows, and under the CLI it is redundant. The handler is constructed either way, but `before(sessionId)` — which records _sessionId and populates the scan map — runs only in the else branch of the CLI check, so in the binary flow the object holds nothing to migrate. Worse than redundant, in fact: the old `_sessionId === null` fallback meant the CLI flow would adopt the new id onto an otherwise inert handler. Now an explicit either/or: the CLI flow migrates the AccessibilityModule gate, the classic flow migrates the handler. The handler also only migrates the session it was actually tracking, rather than treating null as "adopt this one". SDK-7422 --- .../src/accessibility-handler.ts | 2 +- packages/browserstack-service/src/service.ts | 6 ++++- .../tests/service.test.ts | 26 +++++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index 1936f22..340f878 100644 --- a/packages/browserstack-service/src/accessibility-handler.ts +++ b/packages/browserstack-service/src/accessibility-handler.ts @@ -180,7 +180,7 @@ class _AccessibilityHandler { AccessibilityHandler._a11yScanSessionMap[newSessionId] = AccessibilityHandler._a11yScanSessionMap[oldSessionId] delete AccessibilityHandler._a11yScanSessionMap[oldSessionId] } - if (this._sessionId === oldSessionId || this._sessionId === null) { + if (this._sessionId === oldSessionId) { this._sessionId = newSessionId } } catch (error) { diff --git a/packages/browserstack-service/src/service.ts b/packages/browserstack-service/src/service.ts index f1d04a2..736b6a8 100644 --- a/packages/browserstack-service/src/service.ts +++ b/packages/browserstack-service/src/service.ts @@ -902,8 +902,12 @@ export default class BrowserstackService implements Services.ServiceInstance { // 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) } - this._accessibilityHandler?.onSessionReload(oldSessionId, newSessionId) const { setSessionName, setSessionStatus } = this._options const ignoreHooksStatus = this._options.testObservabilityOptions?.ignoreHooksStatus === true diff --git a/packages/browserstack-service/tests/service.test.ts b/packages/browserstack-service/tests/service.test.ts index da656dc..d1324a2 100644 --- a/packages/browserstack-service/tests/service.test.ts +++ b/packages/browserstack-service/tests/service.test.ts @@ -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) + + 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 From cec234c198b3ce6fa60f8b6a3f1864185a1c1184 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 19:09:36 +0530 Subject: [PATCH 13/15] fix(app-a11y): resolve the session at call time, not at capture time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five review findings, all the same underlying shape: the scan gate is read per command from framework state, which a reload updates, while several writers captured the session id once and kept writing to it. Migrating the gate turned that latent disagreement into a live one. Before the migration both sides were stale, so they agreed and nothing scanned; after it, commandWrapper reads the new key while a captured writer sets the old one. Concretely: browser.stopA11yScanning() called AFTER a reload set false on the dead key while the live key stayed true, so scanning carried on against an explicit instruction, and startA11yScanning() was a silent no-op — in exactly the window this PR exists to fix. - accessibilityModule: the scanning toggles and the results getters resolve the session at call time via currentSessionId(). getAccessibilityResults/Summary previously reported the PRE-reload session's results, while the classic path reported the live one, so the two flows disagreed on the same operation. - accessibility-handler: the toggles write this._sessionId, which commandWrapper reads and this PR migrates, instead of the id captured in before(). - service: one _isCliAccessibilityFlow() predicate, used both where AccessibilityHandler.before() is gated and where the reload migration chooses a path. isRunning() alone is NOT equivalent — with the binary up against a non-BrowserStack provider, before() DID run, so the handler holds the live state and must still be migrated. - accessibilityMap / LOG_DISABLED_SHOWN are Map: session ids are strings, the Map declaration was a mis-declaration, and the casts it forced (including `as never` in the tests) are gone. Tests: the CLI branch's positive path is now asserted through the module registry rather than left to an optional chain; the non-BrowserStack-provider case is covered; and both toggle paths are pinned against the post-reload inversion. SDK-7422 --- .../src/accessibility-handler.ts | 7 +- .../src/cli/modules/accessibilityModule.ts | 52 ++++++++----- packages/browserstack-service/src/service.ts | 26 +++++-- .../tests/accessibility-handler.test.ts | 16 ++++ .../cli/modules/accessibilityModule.test.ts | 54 ++++++++++--- .../tests/service.test.ts | 75 ++++++++++++++----- 6 files changed, 175 insertions(+), 55 deletions(-) diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index 340f878..69c3350 100644 --- a/packages/browserstack-service/src/accessibility-handler.ts +++ b/packages/browserstack-service/src/accessibility-handler.ts @@ -259,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 @@ -272,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 5e3dadb..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). @@ -113,28 +113,45 @@ export default class AccessibilityModule extends BaseModule { * 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) { + onSessionReload(oldSessionId: string, newSessionId: string) { try { if (!oldSessionId || !newSessionId || oldSessionId === newSessionId) { return } - const oldKey = oldSessionId as number - const newKey = newSessionId as number - if (this.accessibilityMap.has(oldKey)) { - 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.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(oldKey)) { - this.LOG_DISABLED_SHOWN.set(newKey, this.LOG_DISABLED_SHOWN.get(oldKey) as boolean) - this.LOG_DISABLED_SHOWN.delete(oldKey) + 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() @@ -154,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', @@ -181,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) } @@ -189,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) } @@ -315,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 @@ -328,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/service.ts b/packages/browserstack-service/src/service.ts index 736b6a8..52cdc3d 100644 --- a/packages/browserstack-service/src/service.ts +++ b/packages/browserstack-service/src/service.ts @@ -289,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 { @@ -886,6 +886,20 @@ export default class BrowserstackService implements Services.ServiceInstance { } @PerformanceTester.Measure(PERFORMANCE_SDK_EVENTS.EVENTS.SDK_HOOK, { hookType: 'onReload' }) + /** + * Whether accessibility is being driven by the binary rather than the classic handler. + * + * The two are mutually exclusive and this exact condition decides which: it is what gates + * `AccessibilityHandler.before()` in the `before` hook. Keeping it in one place matters — + * `isRunning()` alone is NOT equivalent. With the binary up against a non-BrowserStack + * provider, `isBrowserstackSession` is false, so the classic handler DID initialise and holds + * the live state; splitting on `isRunning()` there would migrate an empty module gate and + * leave the handler stale. + */ + private _isCliAccessibilityFlow (): boolean { + return Boolean(isBrowserstackSession(this._browser)) && BrowserstackCLI.getInstance().isRunning() + } + async onReload(oldSessionId: string, newSessionId: string) { if (!this._browser) { return Promise.resolve() @@ -902,10 +916,12 @@ export default class BrowserstackService implements Services.ServiceInstance { // 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. + } + + // 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) } diff --git a/packages/browserstack-service/tests/accessibility-handler.test.ts b/packages/browserstack-service/tests/accessibility-handler.test.ts index 7982ece..1d75f54 100644 --- a/packages/browserstack-service/tests/accessibility-handler.test.ts +++ b/packages/browserstack-service/tests/accessibility-handler.test.ts @@ -705,6 +705,22 @@ describe('onSessionReload', () => { 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 diff --git a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts index 869ddd5..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' @@ -201,37 +202,68 @@ describe('AccessibilityModule', () => { describe('onSessionReload', () => { it('carries the scan gate over to the new session id', () => { - accessibilityModule.accessibilityMap.set('old' as never, true) + accessibilityModule.accessibilityMap.set('old', true) accessibilityModule.onSessionReload('old', 'new') - expect(accessibilityModule.accessibilityMap.get('new' as never)).toBe(true) - expect(accessibilityModule.accessibilityMap.has('old' as never)).toBe(false) + 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' as never, false) + accessibilityModule.accessibilityMap.set('old', false) accessibilityModule.onSessionReload('old', 'new') - expect(accessibilityModule.accessibilityMap.get('new' as never)).toBe(false) + 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' as never)).toBe(false) + expect(accessibilityModule.accessibilityMap.has('new')).toBe(false) }) it('ignores a no-op reload and missing ids', () => { - accessibilityModule.accessibilityMap.set('same' as never, true) + accessibilityModule.accessibilityMap.set('same', 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) + 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) }) }) diff --git a/packages/browserstack-service/tests/service.test.ts b/packages/browserstack-service/tests/service.test.ts index d1324a2..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,30 +192,64 @@ 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 + 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 } - await service.onReload('1', '2') - expect(handler.onSessionReload).toHaveBeenCalledWith('1', '2') + 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() + }) - handler.onSessionReload.mockClear() - const getInstanceSpy = vi.spyOn(BrowserstackCLI, 'getInstance') - .mockReturnValue({ isRunning: () => true, modules: {} } as any) + it('migrates the classic handler when the binary is not driving accessibility', async () => { + await service.onReload('1', '2') - await service.onReload('1', '2') - expect(handler.onSessionReload).not.toHaveBeenCalled() + expect(handler.onSessionReload).toHaveBeenCalledWith('1', '2') + }) - getInstanceSpy.mockRestore() - printSpy.mockRestore() + 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 () => { From bd75537965a2f99bdd31cde161abf569decef33f Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 26 Aug 2026 20:02:54 +0530 Subject: [PATCH 14/15] fix(service): keep @Measure bound to onReload, and guard the placement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The predicate added in cec234c landed between the decorator and the method, so the decoration moved to it: onReload stopped emitting its SDK_HOOK measurement, and the predicate started reporting one under hookType 'onReload' on every call — including from the before hook, so every session reported an onReload measurement whether or not it ever reloaded. Behaviour was unaffected (PerformanceTester.measure returns the raw value on the synchronous path), which is why nothing surfaced it: valid TypeScript, lint has no opinion, and the unit suites mock Measure into a pass-through so the binding has no observable behaviour to assert. Hence a structural test over the decorated files instead — verified to fail with the mistake reintroduced and pass without it. Doc comment corrected too: the two accessibility paths are NOT strictly exclusive. The module is registered on startBinResponse.accessibility?.success alone, independent of provider, so under a non-BrowserStack provider it can hold real state while the classic handler's before() has also run. What the predicate decides is which one owns the CLASSIC handler's state. SDK-7422 --- packages/browserstack-service/src/service.ts | 20 +++++---- .../tests/decoratorPlacement.test.ts | 41 +++++++++++++++++++ 2 files changed, 54 insertions(+), 7 deletions(-) create mode 100644 packages/browserstack-service/tests/decoratorPlacement.test.ts diff --git a/packages/browserstack-service/src/service.ts b/packages/browserstack-service/src/service.ts index 52cdc3d..b996be4 100644 --- a/packages/browserstack-service/src/service.ts +++ b/packages/browserstack-service/src/service.ts @@ -885,21 +885,27 @@ export default class BrowserstackService implements Services.ServiceInstance { await this._insightsHandler?.afterStep(step, scenario, result) } - @PerformanceTester.Measure(PERFORMANCE_SDK_EVENTS.EVENTS.SDK_HOOK, { hookType: 'onReload' }) /** * Whether accessibility is being driven by the binary rather than the classic handler. * - * The two are mutually exclusive and this exact condition decides which: it is what gates - * `AccessibilityHandler.before()` in the `before` hook. Keeping it in one place matters — - * `isRunning()` alone is NOT equivalent. With the binary up against a non-BrowserStack - * provider, `isBrowserstackSession` is false, so the classic handler DID initialise and holds - * the live state; splitting on `isRunning()` there would migrate an empty module gate and - * leave the handler stale. + * Precisely: it decides whether the CLASSIC handler owns the session's accessibility state. + * It is the same condition that gates `AccessibilityHandler.before()` in the `before` hook, so + * keeping it in one place is what stops the two drifting. + * + * The two paths are not strictly exclusive. `AccessibilityModule` is registered on + * `startBinResponse.accessibility?.success` alone, independent of provider — `isNonBstackA11y` + * exists for exactly the turboscale / non-BrowserStack case — so with the binary up against a + * non-BrowserStack provider the module can hold real state AND the handler's `before()` will + * have run. Both then need migrating, which is what the caller does. + * + * `isRunning()` alone is NOT equivalent, and that is the trap: under a non-BrowserStack + * provider it is true while the classic handler is the one holding the live state. */ private _isCliAccessibilityFlow (): boolean { return Boolean(isBrowserstackSession(this._browser)) && BrowserstackCLI.getInstance().isRunning() } + @PerformanceTester.Measure(PERFORMANCE_SDK_EVENTS.EVENTS.SDK_HOOK, { hookType: 'onReload' }) async onReload(oldSessionId: string, newSessionId: string) { if (!this._browser) { return Promise.resolve() diff --git a/packages/browserstack-service/tests/decoratorPlacement.test.ts b/packages/browserstack-service/tests/decoratorPlacement.test.ts new file mode 100644 index 0000000..118a87e --- /dev/null +++ b/packages/browserstack-service/tests/decoratorPlacement.test.ts @@ -0,0 +1,41 @@ +import fs from 'node:fs' +import path from 'node:path' +import { describe, expect, it } from 'vitest' + +/** + * A decorator binds to the next class ELEMENT, not to the next thing that looks related. Insert a + * method between `@PerformanceTester.Measure(..., { hookType: 'onReload' })` and `onReload` and the + * decoration silently moves: the hook loses its telemetry, and the intruding method starts + * reporting measurements under the hook's name. + * + * Nothing else in this repo can catch that. The code stays valid TypeScript, lint has no opinion, + * and the unit suites mock `PerformanceTester.Measure` into a pass-through, so the binding has no + * observable behaviour to assert on. Hence a structural check. + */ +const DECORATED_FILES = ['service.ts', 'launcher.ts', 'insights-handler.ts', 'accessibility-handler.ts'] + +describe('PerformanceTester.Measure placement', () => { + for (const file of DECORATED_FILES) { + it(`binds directly to a method in ${file}`, () => { + const lines = fs.readFileSync(path.join(process.cwd(), 'src', file), 'utf8').split('\n') + const offenders: string[] = [] + + lines.forEach((line, index) => { + if (!line.includes('@PerformanceTester.Measure')) { + return + } + // walk to the next non-blank line; that is what the decorator will attach to + let next = index + 1 + while (next < lines.length && lines[next].trim() === '') { + next++ + } + const target = lines[next]?.trim() ?? '' + if (target.startsWith('/*') || target.startsWith('*') || target.startsWith('//')) { + offenders.push(`${file}:${index + 1} — decorator followed by a comment, not a method: ${target.slice(0, 60)}`) + } + }) + + expect(offenders).toEqual([]) + }) + } +}) From d39b1109e2a3a83f726e873780906a3a016b8a09 Mon Sep 17 00:00:00 2001 From: harshit-browserstack Date: Thu, 27 Aug 2026 17:31:43 +0530 Subject: [PATCH 15/15] Update pr-165.md --- .changeset/pr-165.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.changeset/pr-165.md b/.changeset/pr-165.md index b911381..86df4d6 100644 --- a/.changeset/pr-165.md +++ b/.changeset/pr-165.md @@ -3,3 +3,4 @@ --- - 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.