diff --git a/.changeset/pr-186.md b/.changeset/pr-186.md new file mode 100644 index 0000000..73392a7 --- /dev/null +++ b/.changeset/pr-186.md @@ -0,0 +1,6 @@ +--- +"@wdio/browserstack-service": patch +--- + +- Fixed accessibility scans being sent twice for a single command in some environments, which made scan counts inaccurate. +- Fixed the SDK silently falling back to a non-binary flow when the configuration response was incomplete. diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index 69c3350..4e93284 100644 --- a/packages/browserstack-service/src/accessibility-handler.ts +++ b/packages/browserstack-service/src/accessibility-handler.ts @@ -78,6 +78,17 @@ import * as PERFORMANCE_SDK_EVENTS from './instrumentation/performance/constants import { BStackLogger } from './bstackLogger.js' class _AccessibilityHandler { + // Evaluated AT SCAN TIME, not at session start. service.ts decides which flow owns + // accessibility once, before the binary has necessarily finished booting; on a slow + // environment that decision lands on this handler and the binary then comes up and wraps the + // same commands through the CLI module, so every command gets scanned twice. Asking again + // when a scan is about to fire is the only check that can be right. + private _cliOwnsAccessibility: () => boolean = () => false + + setCliOwnershipCheck(check: () => boolean) { + this._cliOwnsAccessibility = check + } + /** * Frameworks whose per-test lifecycle flows through beforeTest/afterTest. * WDIO's jasmine adapter emits the same service hooks as mocha (SDK-7190); @@ -512,8 +523,12 @@ class _AccessibilityHandler { !AccessibilityHandler.shouldPatchExecuteScript(args.length ? args[0] as string : null) ) ) { - BStackLogger.debug(`Performing scan for ${command.class} ${command.name}`) - await performA11yScan(this.isAppAutomate, this._browser, true, true, command.name, undefined, this._currentHookRunUuid) + if (this._cliOwnsAccessibility()) { + BStackLogger.debug('Skipping accessibility scan: the binary flow owns accessibility for this session') + } else { + BStackLogger.debug(`Performing scan for ${command.class} ${command.name}`) + await performA11yScan(this.isAppAutomate, this._browser, true, true, command.name, undefined, this._currentHookRunUuid) + } } else if (skipScanForBidiWindowCommand) { BStackLogger.debug(`SDK-5047: skipping accessibility scan for BiDi window/context command '${command.name}' to avoid racing the WebdriverIO ContextManager during session-start window churn`) } diff --git a/packages/browserstack-service/src/cli/index.ts b/packages/browserstack-service/src/cli/index.ts index b1b3cc3..c1dd171 100644 --- a/packages/browserstack-service/src/cli/index.ts +++ b/packages/browserstack-service/src/cli/index.ts @@ -151,7 +151,16 @@ export class BrowserstackCLI { // credentials) before any downstream error. this.logBuildErrors(startBinResponse) - APIUtils.updateURLSForGRR(this.config.apis as GRRUrls) + // A degenerate config carries no apis block — an auth failure, or a config server that + // never answered (measured: a 60s hang against an internal environment, after which the + // binary echoes the input config straight back). Dereferencing it throws, and the caller + // then tears the binary down, silently dropping the whole run to the Direct flow. Keeping + // the default endpoints is strictly better than that. + if (this.config.apis) { + APIUtils.updateURLSForGRR(this.config.apis as GRRUrls) + } else { + this.logger.warn('loadModules: config carries no apis block; keeping default endpoints') + } this.setupTestFramework() this.setupAutomationFramework() diff --git a/packages/browserstack-service/src/service.ts b/packages/browserstack-service/src/service.ts index b996be4..c043c22 100644 --- a/packages/browserstack-service/src/service.ts +++ b/packages/browserstack-service/src/service.ts @@ -289,6 +289,19 @@ export default class BrowserstackService implements Services.ServiceInstance { this._options.accessibilityOptions ) + // Re-asked at scan time, and deliberately NOT via _isCliAccessibilityFlow(): + // that predicate requires isBrowserstackSession(), which decides on + // `hostname.includes('browserstack')` and so is FALSE on every internal + // environment (hub-.bsstag.com). There the service takes the classic + // branch while the binary still runs the CLI module, and both wrap the same + // commands — measured on one internal-env run as 22 classic + 19 CLI scans. + // Asking the module registry is hostname-independent: the module exists only + // when the binary owns accessibility for this session. + this._accessibilityHandler.setCliOwnershipCheck(() => { + const cliA11y = BrowserstackCLI.getInstance().modules?.[AccessibilityModule.MODULE_NAME] as AccessibilityModule | undefined + return Boolean(cliA11y && (cliA11y.accessibility || cliA11y.isAppAccessibility)) + }) + if (this._isCliAccessibilityFlow()){ BStackLogger.info(`CLI is running, tracking accessibility event for before: ${sessionId}`) // BrowserstackCLI.getInstance().getTestFramework()!.trackEvent(AutomationFrameworkState.CREATE, HookState.POST, { sessionId }) diff --git a/packages/browserstack-service/tests/accessibility-handler.test.ts b/packages/browserstack-service/tests/accessibility-handler.test.ts index 1d75f54..99c5546 100644 --- a/packages/browserstack-service/tests/accessibility-handler.test.ts +++ b/packages/browserstack-service/tests/accessibility-handler.test.ts @@ -503,6 +503,26 @@ describe('beforeHook / afterHook (hook scans)', () => { expect(lastCall[lastCall.length - 1]).toBeNull() }) + it('performs NO scan when the binary flow owns accessibility for the session', async () => { + // service.ts can pick the classic branch before the binary has booted; the CLI module then + // wraps the same commands, and without this check every command is scanned twice. + vi.spyOn(utils, 'shouldScanTestForAccessibility').mockReturnValue(true) + const scanSpy = vi.spyOn(utils, 'performA11yScan').mockResolvedValue(undefined) + await accessibilityHandler.beforeHook( + { title: '"before each" hook', parent: 'suite' } as any, + { currentTest: { parent: 'suite', title: 'test' } }, + 'hook-uuid-cli' + ) + accessibilityHandler.setCliOwnershipCheck(() => true) + + const orig = vi.fn().mockResolvedValue('ok') + await accessibilityHandler['commandWrapper']({ name: 'click', class: 'Element' } as any, undefined as any, orig, 'arg') + + // the command still runs — only the duplicate scan is suppressed + expect(orig).toHaveBeenCalled() + expect(scanSpy).not.toHaveBeenCalled() + }) + it('_getParamsForAppAccessibility puts the hook uuid on the scan payload as thHookRunUuid', () => { expect(utils._getParamsForAppAccessibility('click', 'testName', 'hook-uuid-9').thHookRunUuid).toBe('hook-uuid-9') expect(utils._getParamsForAppAccessibility('click', 'testName').thHookRunUuid).toBeUndefined()