diff --git a/.changeset/pr-168.md b/.changeset/pr-168.md new file mode 100644 index 0000000..209f634 --- /dev/null +++ b/.changeset/pr-168.md @@ -0,0 +1,6 @@ +--- +"@wdio/browserstack-service": minor +--- + +- Accessibility scans now run for driver commands issued from your WDIO config's `before()` hook, so screens visited during setup are covered. +- Hooks in your WDIO config that touch the driver now appear on the build, including when they fail — previously a failing `before()` was reported nowhere. diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index 69c3350..181d668 100644 --- a/packages/browserstack-service/src/accessibility-handler.ts +++ b/packages/browserstack-service/src/accessibility-handler.ts @@ -75,7 +75,9 @@ import accessibilityScripts from './scripts/accessibility-scripts.js' import PerformanceTester from './instrumentation/performance/performance-tester.js' import * as PERFORMANCE_SDK_EVENTS from './instrumentation/performance/constants.js' +import { PRE_TEST_HOOK_TITLE_FALLBACK, PRE_TEST_HOOK_TITLE_PREFIX, PRE_TEST_SCAN_FRAMEWORKS } from './constants.js' import { BStackLogger } from './bstackLogger.js' +import { getActiveHookName, getPreTestWindowFailure } from './hookInstrumentation.js' class _AccessibilityHandler { /** @@ -93,6 +95,17 @@ class _AccessibilityHandler { private _config: Options.Testrunner private _accessibilityOptions?: AccessibilityOptions private _autoScanning: boolean = true + // Reports the pre-test window as a hook run. Injected by the service, and only for the + // frameworks whose Direct-flow hook reporting supports it, so this handler stays unaware of + // which one it is serving. + private _preTestHookReporter?: { + open: (name: string) => Promise, + close: (uuid: string, passed: boolean, message?: string) => Promise + } + private _preTestHookState: 'idle' | 'attempted' | 'open' | 'closed' = 'idle' + // True from before() until the first test/scenario — see preTestWindowActive on the CLI module. + private _preTestWindowActive = false + private _preTestHookUuid?: string private _testIdentifier: string | null = null private _testMetadata: TestMetadata = {} /* Set while a supported hook is executing; scans fired in this window are stamped with it. */ @@ -188,6 +201,60 @@ class _AccessibilityHandler { } } + /** mocha and cucumber only — see PRE_TEST_SCAN_FRAMEWORKS. */ + private supportsPreTestWindow(): boolean { + return PRE_TEST_SCAN_FRAMEWORKS.includes(this._framework as typeof PRE_TEST_SCAN_FRAMEWORKS[number]) && + !this._browser?.isMultiremote + } + + setPreTestHookReporter(reporter: { + open: (name: string) => Promise, + close: (uuid: string, passed: boolean, message?: string) => Promise + }) { + this._preTestHookReporter = reporter + } + + /** On demand from the scan path, so a suite with no config-level hook reports nothing extra. */ + private async ensurePreTestHookRun() { + if (this._preTestHookState !== 'idle' || !this._preTestHookReporter || !this.supportsPreTestWindow()) { + return + } + this._preTestHookState = 'attempted' + try { + const hook = getActiveHookName() + const name = hook ? `${PRE_TEST_HOOK_TITLE_PREFIX} "${hook}" hook` : PRE_TEST_HOOK_TITLE_FALLBACK + this._preTestHookUuid = await this._preTestHookReporter.open(name) + if (this._preTestHookUuid) { + this._preTestHookState = 'open' + // What actually reaches the scan: commandWrapper passes this to performA11yScan as + // thHookRunUuid. Without it the hook run exists in TRA and the scans in its window + // still arrive with no parent, which is the join this whole thing is for. + this._currentHookRunUuid = this._preTestHookUuid + } + } catch (error) { + BStackLogger.debug(`Could not open a hook run for the pre-test window: ${error}`) + } + } + + private async closePreTestHookRun() { + // cleared unconditionally: the window is over whether or not a hook run was ever opened, + // and leaving it set would strip the test run uuid from every later test-body scan + this._preTestWindowActive = false + if (this._preTestHookState !== 'open' || !this._preTestHookUuid || !this._preTestHookReporter) { + this._preTestHookState = 'closed' + return + } + this._preTestHookState = 'closed' + try { + const failure = getPreTestWindowFailure() + // cleared before the close so a test-body scan is never stamped as a hook scan + this._currentHookRunUuid = null + await this._preTestHookReporter.close(this._preTestHookUuid, !failure, failure) + } catch (error) { + BStackLogger.debug(`Could not close the hook run for the pre-test window: ${error}`) + } + } + async before(sessionId: string) { PerformanceTester.start(PERFORMANCE_SDK_EVENTS.CONFIG_EVENTS.ACCESSIBILITY) @@ -282,6 +349,20 @@ class _AccessibilityHandler { if (!this._accessibility) { return } + + // Gate for the window between driver creation and the first test/scenario. WDIO's + // config-level hooks run there and are no test-framework hooks, so nothing else registers + // the session and their commands went unscanned. beforeTest/beforeScenario/beforeHook each + // recompute it. Same fix as the CLI flow's onBeforeExecute. + // + // Scoped to mocha and cucumber. Jasmine is in the per-test path too, so opening this for + // it would add scans in a window it never scanned — a behaviour change on a framework + // where App-A11y is not supported. Multiremote likewise. + if (this._autoScanning && this.supportsPreTestWindow()) { + AccessibilityHandler._a11yScanSessionMap[sessionId] = true + this._preTestWindowActive = true + } + if (!('overwriteCommand' in this._browser && Array.isArray(accessibilityScripts.commandsToWrap))) { return } @@ -305,6 +386,7 @@ class _AccessibilityHandler { } async beforeTest (suiteTitle: string | undefined, test: Frameworks.Test) { + await this.closePreTestHookRun() try { if ( !AccessibilityHandler.TEST_HOOK_FRAMEWORKS.includes(this._framework as string) || @@ -385,6 +467,7 @@ class _AccessibilityHandler { * Cucumber Only */ async beforeScenario (world: ITestCaseHookParameter) { + await this.closePreTestHookRun() const pickleData = world.pickle const gherkinDocument = world.gherkinDocument const featureData = gherkinDocument.feature @@ -504,6 +587,7 @@ class _AccessibilityHandler { private async commandWrapper (command: CommandInfo, prevImpl: Function, origFunction: Function, ...args: unknown[]) { const skipScanForBidiWindowCommand = AccessibilityHandler.shouldSkipScanForBidiWindowCommand(this._browser, command) + await this.ensurePreTestHookRun() if ( this._sessionId && AccessibilityHandler._a11yScanSessionMap[this._sessionId] && !skipScanForBidiWindowCommand && @@ -513,7 +597,7 @@ class _AccessibilityHandler { ) ) { BStackLogger.debug(`Performing scan for ${command.class} ${command.name}`) - await performA11yScan(this.isAppAutomate, this._browser, true, true, command.name, undefined, this._currentHookRunUuid) + await performA11yScan(this.isAppAutomate, this._browser, true, true, command.name, undefined, this._currentHookRunUuid, this._preTestWindowActive) } 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/modules/accessibilityModule.ts b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts index 8e44b11..aa091a9 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -12,6 +12,8 @@ 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, PRE_TEST_HOOK_TITLE_FALLBACK, PRE_TEST_HOOK_TITLE_PREFIX } from '../../constants.js' +import { getActiveHookName, getPreTestWindowFailure } from '../../hookInstrumentation.js' import util from 'node:util' import type { Accessibility } from '../../grpc/index.js' import PerformanceTester from '../../instrumentation/performance/performance-tester.js' @@ -39,6 +41,16 @@ 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 + // Hook run covering scans fired before any test exists (WDIO's config-level hooks, which get + // no hook run of their own). 'attempted' is distinct from 'open' so a failed PRE is neither + // retried per command nor closed by a POST that would pair with nothing. + private preTestHookState: 'idle' | 'attempted' | 'open' | 'closed' = 'idle' + // True from driver creation until the first test. Scans fired in that window come from WDIO's + // config-level hooks, which belong to no test, so they must not carry a test run uuid. + private preTestWindowActive = false + // Captured at PRE: the binary names a hook from the record pushed there, and by POST a + // different hook is running. + private preTestHookLabel: string | undefined constructor(accessibilityConfig: Accessibility, isNonBstackA11y: boolean) { super() @@ -102,6 +114,61 @@ export default class AccessibilityModule extends BaseModule { this.currentHookRunUuid = null } + /** + * Open a BEFORE_ALL hook run for the pre-test window, on demand from the scan path — so a + * suite with no config-level hook reports nothing extra. The framework stamps the uuid, + * onHookStart picks it up onto the scan, and the binary emits HookRunStarted for it. + */ + private async ensurePreTestHookRun() { + if (this.preTestHookState !== 'idle' || this.currentHookRunUuid !== null) { + return + } + // marked before awaiting: concurrent commands must not each open their own hook run + this.preTestHookState = 'attempted' + try { + this.preTestHookLabel = getActiveHookName() + 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() + // WDIO swallows a throwing config hook, so nothing else marks it failed; reporting + // `passed` here would assert something false rather than merely omit it. + const failure = getPreTestWindowFailure() + const result = failure ? { passed: false, error: { message: failure } } : { passed: true } + await framework?.trackEvent(TestFrameworkState.BEFORE_ALL, HookState.POST, { ...this.preTestHookArgs(), result }) + } catch (error) { + this.logger.debug(`Could not close the hook run for the pre-test window: ${error}`) + } + } + + /** The binary runs path.relative() over `file`, so an absent path would drop the event. */ + private preTestHookArgs() { + const hook = this.preTestHookLabel + return { + test: { + file: process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] || process.cwd(), + title: hook + ? `${PRE_TEST_HOOK_TITLE_PREFIX} "${hook}" hook` + : PRE_TEST_HOOK_TITLE_FALLBACK + } + } + } + /** * 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 @@ -263,6 +330,16 @@ export default class AccessibilityModule extends BaseModule { }) } + // Gate for the window between driver creation and the first test: WDIO's config-level + // hooks run there, and being no test-framework hooks, nothing else registers the + // session — so their commands went unscanned while still counting as expected + // coverage. Unconditional like onHookStart; onBeforeTest recomputes it per test. + const sessionId = this.currentSessionId() + if (this.autoScanning && sessionId !== undefined && sessionId !== null) { + this.accessibilityMap.set(sessionId, true) + this.preTestWindowActive = true + } + } catch (error) { this.logger.error(`Error in onBeforeExecute: ${error}`) } @@ -281,8 +358,9 @@ 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) + await this.performScanCli(browser, command.name, this.currentHookRunUuid, this.preTestWindowActive) this.logger.debug(`Accessibility scan performed after ${command.name} command`) } catch (scanError) { this.logger.debug(`Error performing accessibility scan after ${command.name}: ${scanError}`) @@ -305,6 +383,11 @@ 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.') + // Window over: close its hook run before clearing the uuid, or a spec with no + // framework hooks would stamp every test-body scan as a hook scan. + await this.closePreTestHookRun() + this.preTestWindowActive = false + this.currentHookRunUuid = null const suiteTitle = (typeof args.suiteTitle === 'string' ? args.suiteTitle : '') || '' const test = (args.test && typeof args.test === 'object' ? args.test as { title?: string } : {}) || {} @@ -498,7 +581,8 @@ export default class AccessibilityModule extends BaseModule { private async performScanCli( browser: WebdriverIO.Browser | WebdriverIO.MultiRemoteBrowser, commandName?: string, - hookRunUuid?: string | null + hookRunUuid?: string | null, + isGlobalHook?: boolean ): Promise | undefined> { return await PerformanceTester.measureWrapper( PERFORMANCE_SDK_EVENTS.A11Y_EVENTS.PERFORM_SCAN, @@ -511,7 +595,7 @@ export default class AccessibilityModule extends BaseModule { if (this.isAppAccessibility) { const testName=this.currentTestName || undefined const results: unknown = await (browser as WebdriverIO.Browser).execute( - formatString(this.scriptInstance.performScan, JSON.stringify(_getParamsForAppAccessibility(commandName, testName, hookRunUuid))) as string, + formatString(this.scriptInstance.performScan, JSON.stringify(_getParamsForAppAccessibility(commandName, testName, hookRunUuid, isGlobalHook))) as string, {} ) BStackLogger.debug(util.format(results as string)) diff --git a/packages/browserstack-service/src/constants.ts b/packages/browserstack-service/src/constants.ts index c64bfa8..284a987 100644 --- a/packages/browserstack-service/src/constants.ts +++ b/packages/browserstack-service/src/constants.ts @@ -86,6 +86,46 @@ 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 config hooks that run in the worker with a live session, so a driver command inside them + is meaningful. Excludes launcher hooks (no driver), beforeSession (no session yet), + afterSession (session being deleted) and beforeCommand/afterCommand (per-command). */ +export const BROWSER_CONTEXT_HOOKS = [ + 'before', + 'beforeSuite', + 'beforeHook', + 'beforeTest', + 'afterTest', + 'afterHook', + 'afterSuite', + 'after', + 'beforeFeature', + 'beforeScenario', + 'beforeStep', + 'afterStep', + 'afterScenario', + 'afterFeature' +] as const + +/* Of those, the ones that can run before the first test/scenario: the Mocha adapter registers + beforeSuite as a root before-all, and beforeFeature precedes the first scenario. */ +export const PRE_TEST_WINDOW_HOOKS = ['before', 'beforeSuite', 'beforeFeature'] as const + +/* Tag on hook-window diagnostics, so they can be grepped out of an uploaded log */ +export const HOOK_WINDOW_LOG_PREFIX = '[hook-window]' + +/* Frameworks the pre-test window work applies to. Jasmine is excluded deliberately: it is in + the per-test a11y path (TEST_HOOK_FRAMEWORKS) and keeps that behaviour untouched, but App-A11y + is not supported there, so nothing new is switched on for it. Multiremote is excluded the same + way, and separately never reaches the CLI flow. */ +export const PRE_TEST_SCAN_FRAMEWORKS = ['mocha', 'cucumber'] as const + +/* Key the pre-test hook run is stashed under, so its finish can find its start */ +export const PRE_TEST_HOOK_ID = 'bstack:pre-test-hook' + +/* Title reported for the hook run covering the pre-test window */ +export const PRE_TEST_HOOK_TITLE_PREFIX = 'wdio' +export const PRE_TEST_HOOK_TITLE_FALLBACK = `${PRE_TEST_HOOK_TITLE_PREFIX} config-level hook` + /** * 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/hookInstrumentation.ts b/packages/browserstack-service/src/hookInstrumentation.ts new file mode 100644 index 0000000..f219f3a --- /dev/null +++ b/packages/browserstack-service/src/hookInstrumentation.ts @@ -0,0 +1,169 @@ +import type { Options } from '@wdio/types' + +import { BStackLogger } from './bstackLogger.js' +import { BROWSER_CONTEXT_HOOKS, HOOK_WINDOW_LOG_PREFIX, PRE_TEST_WINDOW_HOOKS } from './constants.js' + +type HookFn = (...args: unknown[]) => unknown + +const INSTRUMENTED = Symbol('bstackHookInstrumented') + +/** + * Run our own bookkeeping so it can never affect the hook it wraps. + * + * This code executes inside the user's hook. A throw from anything here — a logger whose stream + * has gone, a Proxy that dislikes String() — would surface as their hook failing, in a hook WDIO + * reports to nobody. The handler's own error is never routed through this. + */ +const quietly = (fn: () => void): void => { + try { + fn() + } catch { + // instrumentation is not worth a failed hook + } +} + +/** `.then` may be a throwing getter on a hostile object, and we only need to know if it is callable. */ +const isThenable = (value: unknown): boolean => { + try { + return Boolean(value) && typeof (value as Promise).then === 'function' + } catch { + return false + } +} + +const hookFailures: Record = {} +/** Innermost last: WDIO fires same-named handlers concurrently and nests hook kinds. */ +const activeHooks: string[] = [] + +export const getActiveHookName = (): string | undefined => activeHooks[activeHooks.length - 1] + +/** + * First failure from a hook that ran before the first test, prefixed with its hook name. + * + * `executeHooksWithArgs` resolves WITH a hook's error rather than rejecting, so a throwing config + * hook leaves the exit code at 0 and every reporter green. This wrapper is the only thing that + * sees it. Any handler counts, including another service's — the window failed either way. + */ +export const getPreTestWindowFailure = (): string | undefined => { + for (const hookName of PRE_TEST_WINDOW_HOOKS) { + if (hookFailures[hookName]) { + return `${hookName}: ${hookFailures[hookName]}` + } + } + return undefined +} + +/** + * Sync handlers stay sync — an `async` wrapper would turn a synchronous throw into a rejection and + * defer every hook body by a microtask, which is observable to anything not awaiting the hook. + */ +const instrument = (hookName: string, index: number, fn: HookFn): HookFn => { + if ((fn as unknown as Record)[INSTRUMENTED]) { + return fn + } + + // ConfigParser binds every handler it folds in, so `fn.name` reads as `bound ` for the + // user's hooks and other services' alike; the index is what identifies one across its pair. + const label = `${hookName}#${index}${fn.name ? ` (${fn.name})` : ''}` + const recordFailure = (error: unknown) => { + hookFailures[hookName] = (error as Error)?.message || String(error) + } + const leave = () => { + const at = activeHooks.lastIndexOf(hookName) + if (at !== -1) { + activeHooks.splice(at, 1) + } + } + + const wrapped = function (this: unknown, ...args: unknown[]) { + const startedAt = Date.now() + const finish = (outcome: string) => quietly(() => + BStackLogger.debug(`${HOOK_WINDOW_LOG_PREFIX} ${label} ${outcome} in ${Date.now() - startedAt}ms`)) + + quietly(() => { + BStackLogger.debug(`${HOOK_WINDOW_LOG_PREFIX} ${label} started`) + activeHooks.push(hookName) + }) + + let result: unknown + try { + result = fn.apply(this, args) + } catch (error) { + quietly(() => { + recordFailure(error) + leave() + }) + finish(`threw: ${(error as Error)?.message}`) + throw error + } + + if (isThenable(result)) { + return (result as Promise).then( + (value) => { + quietly(leave) + finish('finished') + return value + }, + (error) => { + quietly(() => { + recordFailure(error) + leave() + }) + finish(`rejected: ${(error as Error)?.message}`) + throw error + } + ) + } + + quietly(leave) + 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 on a hook — ConfigParser folds the user's + * and every service's into one array per hook name, fired together via `Promise.all` — so patching + * the array in place is the only way to see a handler that is not ours start and finish. + * + * Logging only. Idempotent, and never throws: instrumentation must not stop a suite starting. + */ +export function instrumentBrowserContextHooks(config?: Options.Testrunner): void { + if (!config) { + return + } + + try { + const record = config as unknown as Record + activeHooks.length = 0 + for (const hookName of BROWSER_CONTEXT_HOOKS) { + // fresh record per config: the maps live as long as the worker + delete hookFailures[hookName] + + const handlers = record[hookName] + if (!Array.isArray(handlers) || handlers.length === 0) { + continue + } + quietly(() => BStackLogger.debug(`${HOOK_WINDOW_LOG_PREFIX} instrumenting ${hookName}: ${handlers.length} handler(s)`)) + record[hookName] = handlers.map((handler, index) => { + if (typeof handler !== 'function') { + return handler + } + try { + return instrument(hookName, index, handler as HookFn) + } catch { + // hand back the original rather than losing the hook, or the ones after it + return handler + } + }) + } + } catch (error) { + // including this one: a throwing logger here would escape the function entirely + quietly(() => BStackLogger.debug(`Could not instrument config hooks: ${error}`)) + } +} diff --git a/packages/browserstack-service/src/insights-handler.ts b/packages/browserstack-service/src/insights-handler.ts index e64a312..69d1a97 100644 --- a/packages/browserstack-service/src/insights-handler.ts +++ b/packages/browserstack-service/src/insights-handler.ts @@ -41,7 +41,7 @@ import type { import { BStackLogger } from './bstackLogger.js' import type { Capabilities } from '@wdio/types' import Listener from './testOps/listener.js' -import { TESTOPS_SCREENSHOT_ENV } from './constants.js' +import { TESTOPS_SCREENSHOT_ENV, PRE_TEST_HOOK_ID } from './constants.js' import { BrowserstackCLI } from './cli/index.js' import { TestFrameworkState } from './cli/states/testFrameworkState.js' import { HookState } from './cli/states/hookState.js' @@ -409,13 +409,64 @@ class _InsightsHandler { } } + /** + * Report a hook run for the pre-test window — the stretch between the driver existing and the + * first scenario, where WDIO's config-level hooks run and emit no framework event of their own. + * + * Cucumber only: it is the framework whose Direct-flow hook reporting this mirrors, and the + * one that always takes that flow. Returns the uuid to close with, or undefined if nothing + * was reported. + */ + async reportPreTestHookStarted(name: string): Promise { + try { + if (this._framework !== 'cucumber') { + return undefined + } + const hookMetaData: TestMeta = { + uuid: uuidv4(), + startedAt: (new Date()).toISOString(), + // absent before the first scenario; the hook uuid is what the scan is joined on + testRunId: InsightsHandler.currentTest?.uuid, + hookType: 'BEFORE_ALL', + kind: 'hook', + name, + scopes: [this._cucumberData.feature?.name || ''], + fileName: this._cucumberData.uri + } + this._tests[PRE_TEST_HOOK_ID] = hookMetaData + this.setCurrentHook({ uuid: hookMetaData.uuid }) + this.listener.hookStarted(this.getHookRunDataForCucumber(hookMetaData, 'HookRunStarted')) + return hookMetaData.uuid + } catch (error) { + BStackLogger.debug(`Could not report the pre-test hook run: ${error}`) + return undefined + } + } + + async reportPreTestHookFinished(uuid: string, passed: boolean, message?: string): Promise { + try { + const hookMetaData = this._tests[PRE_TEST_HOOK_ID] + if (!hookMetaData || hookMetaData.uuid !== uuid) { + return + } + hookMetaData.finishedAt = (new Date()).toISOString() + this.setCurrentHook({ uuid, finished: true }) + const result = { passed, error: message ? { message } : undefined } as Frameworks.TestResult + this.listener.hookFinished(this.getHookRunDataForCucumber(hookMetaData, 'HookRunFinished', result)) + delete this._tests[PRE_TEST_HOOK_ID] + } catch (error) { + BStackLogger.debug(`Could not close the pre-test hook run: ${error}`) + } + } + public getHookRunDataForCucumber(hookData: TestMeta, eventType: string, result?: Frameworks.TestResult) { const { uri, feature } = this._cucumberData const testData: TestData = { uuid: hookData.uuid, type: 'hook', - name: this.getCucumberHookName(hookData.hookType), + // stored name wins: callers that synthesise a hook carry their own label + name: hookData.name || this.getCucumberHookName(hookData.hookType), body: { lang: 'webdriverio', code: null diff --git a/packages/browserstack-service/src/service.ts b/packages/browserstack-service/src/service.ts index b996be4..855842f 100644 --- a/packages/browserstack-service/src/service.ts +++ b/packages/browserstack-service/src/service.ts @@ -11,19 +11,21 @@ import { isFalse, getUniqueIdentifier, getHookType, - isBrowserstackExecutorScript + isBrowserstackExecutorScript, + isMultiRemoteCaps } from './util.js' import type { BrowserstackConfig, BrowserstackOptions, MultiRemoteAction } from './types.js' import type { Pickle, Feature, ITestCaseHookParameter, CucumberHook } from './cucumber-types.js' import InsightsHandler from './insights-handler.js' import TestReporter from './reporter.js' -import { DEFAULT_OPTIONS, NOT_ALLOWED_KEYS_IN_CAPS, PERF_MEASUREMENT_ENV } from './constants.js' +import { DEFAULT_OPTIONS, NOT_ALLOWED_KEYS_IN_CAPS, PERF_MEASUREMENT_ENV, PRE_TEST_SCAN_FRAMEWORKS } from './constants.js' import CrashReporter from './crash-reporter.js' import AccessibilityHandler from './accessibility-handler.js' 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 +121,15 @@ 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. + // Scoped to the frameworks the pre-test window work applies to, so a jasmine or multiremote + // run has its handlers left exactly as WDIO registered them. + if (PRE_TEST_SCAN_FRAMEWORKS.includes(this._config?.framework as typeof PRE_TEST_SCAN_FRAMEWORKS[number]) && + !isMultiRemoteCaps(this._caps as Capabilities.TestrunnerCapabilities)) { + instrumentBrowserContextHooks(this._config) + } + PerformanceTester.startMonitoring('performance-report-service.csv') if (shouldProcessEventForTesthub('')) { this._config.reporters?.push(TestReporter) @@ -289,6 +300,22 @@ export default class BrowserstackService implements Services.ServiceInstance { this._options.accessibilityOptions ) + // Installed before before(), and reading _insightsHandler at CALL time rather + // than capturing it: WDIO runs the user's config-level before() concurrently + // with ours, so its first command can land while this method is still setting + // up. Installing late meant the first scan of the window found no reporter. + // Cucumber only — jasmine and multiremote report hooks on no path this reuses. + if (this._config.framework === 'cucumber') { + this._accessibilityHandler.setPreTestHookReporter({ + open: (name: string) => this._insightsHandler + ? this._insightsHandler.reportPreTestHookStarted(name) + : Promise.resolve(undefined), + close: (uuid: string, passed: boolean, message?: string) => this._insightsHandler + ? this._insightsHandler.reportPreTestHookFinished(uuid, passed, message) + : Promise.resolve() + }) + } + if (this._isCliAccessibilityFlow()){ BStackLogger.info(`CLI is running, tracking accessibility event for before: ${sessionId}`) // BrowserstackCLI.getInstance().getTestFramework()!.trackEvent(AutomationFrameworkState.CREATE, HookState.POST, { sessionId }) @@ -318,6 +345,7 @@ export default class BrowserstackService implements Services.ServiceInstance { return } await this._insightsHandler.before() + } /** diff --git a/packages/browserstack-service/src/util.ts b/packages/browserstack-service/src/util.ts index 8c381e9..600e562 100644 --- a/packages/browserstack-service/src/util.ts +++ b/packages/browserstack-service/src/util.ts @@ -595,9 +595,15 @@ export const formatString = (template: (string | null), ...values: (string | nul } // eslint-disable-next-line @typescript-eslint/no-explicit-any -export const _getParamsForAppAccessibility = ( commandName?: string, testName?: string, hookRunUuid?: string | null ): { thTestRunUuid: any, thHookRunUuid: any, thBuildUuid: any, thJwtToken: any, authHeader: any, scanTimestamp: number, method: string | undefined, testName: string | undefined } => { +/** + * `isGlobalHook` marks a scan fired from a WDIO CONFIG-level hook — before()/beforeSuite()/ + * beforeFeature() — which belongs to no test. TEST_ANALYTICS_ID there holds the uuid of a test + * that has not started (mocha mints it at instance creation), so sending it would attribute the + * scan to a test it did not come from. The hook run uuid is the parent in that case. + */ +export const _getParamsForAppAccessibility = ( commandName?: string, testName?: string, hookRunUuid?: string | null, isGlobalHook?: boolean ): { thTestRunUuid: any, thHookRunUuid: any, thBuildUuid: any, thJwtToken: any, authHeader: any, scanTimestamp: number, method: string | undefined, testName: string | undefined } => { return { - 'thTestRunUuid': process.env.TEST_ANALYTICS_ID, + 'thTestRunUuid': isGlobalHook ? undefined : process.env.TEST_ANALYTICS_ID, // Present only when the scan fires inside a hook (dropped by JSON.stringify when undefined, // so in-test scans are unchanged). SeleniumHub appAllyHandler relays this as `hook_run_uuid`. 'thHookRunUuid': hookRunUuid || undefined, @@ -611,7 +617,7 @@ export const _getParamsForAppAccessibility = ( commandName?: string, testName?: } /* eslint-disable @typescript-eslint/no-explicit-any */ -export const performA11yScan = async (isAppAutomate: boolean, browser: WebdriverIO.Browser | WebdriverIO.MultiRemoteBrowser, isBrowserStackSession?: boolean, isAccessibility?: boolean | string, commandName?: string, testName?: string, hookRunUuid?: string | null,) : Promise<{ [key: string]: any; } | undefined> => { +export const performA11yScan = async (isAppAutomate: boolean, browser: WebdriverIO.Browser | WebdriverIO.MultiRemoteBrowser, isBrowserStackSession?: boolean, isAccessibility?: boolean | string, commandName?: string, testName?: string, hookRunUuid?: string | null, isGlobalHook?: boolean) : Promise<{ [key: string]: any; } | undefined> => { if (!isAccessibilityAutomationSession(isAccessibility)) { BStackLogger.warn('Not an Accessibility Automation session, cannot perform Accessibility scan.') @@ -620,7 +626,7 @@ export const performA11yScan = async (isAppAutomate: boolean, browser: Webdriver try { if (isAppAccessibilityAutomationSession(isAccessibility, isAppAutomate)) { - const results: unknown = await (browser as WebdriverIO.Browser).execute(formatString(AccessibilityScripts.performScan, JSON.stringify(_getParamsForAppAccessibility(commandName, testName, hookRunUuid))) as string, {}) + const results: unknown = await (browser as WebdriverIO.Browser).execute(formatString(AccessibilityScripts.performScan, JSON.stringify(_getParamsForAppAccessibility(commandName, testName, hookRunUuid, isGlobalHook))) as string, {}) BStackLogger.debug(util.format(results as string)) return ( results as { [key: string]: any; } | undefined ) } diff --git a/packages/browserstack-service/tests/accessibility-handler.test.ts b/packages/browserstack-service/tests/accessibility-handler.test.ts index 1d75f54..03aac27 100644 --- a/packages/browserstack-service/tests/accessibility-handler.test.ts +++ b/packages/browserstack-service/tests/accessibility-handler.test.ts @@ -483,8 +483,10 @@ describe('beforeHook / afterHook (hook scans)', () => { await accessibilityHandler['commandWrapper']({ name: 'click', class: 'Element' } as any, undefined as any, orig, 'arg') expect(scanSpy).toHaveBeenCalled() const lastCall = scanSpy.mock.calls[scanSpy.mock.calls.length - 1] - // performA11yScan(isAppAutomate, browser, isBS, isA11y, commandName, testName, hookRunUuid) - expect(lastCall[lastCall.length - 1]).toBe('hook-uuid-42') + // performA11yScan(isAppAutomate, browser, isBS, isA11y, commandName, testName, hookRunUuid, + // isGlobalHook) — asserted by POSITION, not "last": the signature grew an + // argument and a last-element assertion silently followed it. + expect(lastCall[6]).toBe('hook-uuid-42') }) it('does NOT stamp a hook uuid on a test-body scan (afterHook cleared it)', async () => { @@ -500,7 +502,26 @@ describe('beforeHook / afterHook (hook scans)', () => { await accessibilityHandler['commandWrapper']({ name: 'click', class: 'Element' } as any, undefined as any, orig, 'arg') expect(scanSpy).toHaveBeenCalled() const lastCall = scanSpy.mock.calls[scanSpy.mock.calls.length - 1] - expect(lastCall[lastCall.length - 1]).toBeNull() + expect(lastCall[6]).toBeNull() + }) + + it('omits thTestRunUuid for a global-hook scan, keeps it for a test-body scan', () => { + // TEST_ANALYTICS_ID holds a uuid minted at instance creation, i.e. a test that has not + // started. Sending it from a config-level hook would attribute the scan to a test it did + // not come from; the hook run uuid is the parent there. + process.env.TEST_ANALYTICS_ID = 'test-run-uuid-1' + + const fromGlobalHook = utils._getParamsForAppAccessibility('back', undefined, 'hook-uuid-1', true) + expect(fromGlobalHook.thTestRunUuid).toBeUndefined() + expect(fromGlobalHook.thHookRunUuid).toBe('hook-uuid-1') + + const fromTestBody = utils._getParamsForAppAccessibility('back', 'a test', null, false) + expect(fromTestBody.thTestRunUuid).toBe('test-run-uuid-1') + + // default (flag omitted) must stay as it was for every existing caller + expect(utils._getParamsForAppAccessibility('back', 'a test').thTestRunUuid).toBe('test-run-uuid-1') + + delete process.env.TEST_ANALYTICS_ID }) it('_getParamsForAppAccessibility puts the hook uuid on the scan payload as thHookRunUuid', () => { @@ -689,6 +710,144 @@ describe('afterTest', () => { }) }) +describe('frameworks the pre-test window does NOT apply to', () => { + beforeEach(() => { + vi.spyOn(utils, 'isBrowserstackSession').mockReturnValue(true) + vi.spyOn(utils, 'isAccessibilityAutomationSession').mockReturnValue(true) + vi.spyOn(utils, 'validateCapsWithA11y').mockReturnValue(true) + }) + + it('leaves jasmine exactly as it was — no pre-test gate', async () => { + // jasmine IS in TEST_HOOK_FRAMEWORKS, so it already scans per test. Opening the window + // for it would add scans where it never had them, on a framework App-A11y does not + // support. Its per-test behaviour is untouched either way. + const handler = new AccessibilityHandler(browser, caps, options, false, config, 'jasmine', true, false, accessibilityOpts) + + await handler.before('session-jasmine') + + expect(AccessibilityHandler['_a11yScanSessionMap']['session-jasmine']).toBeUndefined() + }) + + it('leaves multiremote alone', async () => { + const multiremote = { ...browser, isMultiremote: true } as never + const handler = new AccessibilityHandler(multiremote, caps, options, false, config, 'mocha', true, false, accessibilityOpts) + + await handler.before('session-multiremote') + + expect(AccessibilityHandler['_a11yScanSessionMap']['session-multiremote']).toBeUndefined() + }) + + it('does not key the map on "undefined" when multiremote hands over no session id', async () => { + // A multiremote browser has no sessionId — MultiRemoteDriver defines none and the element + // path deletes it, which is why wdio-runner reads ids per instance. The service passes + // `browser.sessionId` straight through, so the handler receives undefined. + const multiremote = { ...browser, isMultiremote: true } as never + const handler = new AccessibilityHandler(multiremote, caps, options, false, config, 'mocha', true, false, accessibilityOpts) + + await handler.before(undefined as unknown as string) + + expect(AccessibilityHandler['_a11yScanSessionMap']['undefined']).toBeUndefined() + expect(handler['_sessionId']).toBeUndefined() + }) + + it('does not open a hook run for jasmine even if a reporter is installed', async () => { + const handler = new AccessibilityHandler(browser, caps, options, false, config, 'jasmine', true, false, accessibilityOpts) + const reporter = { open: vi.fn().mockResolvedValue('uuid'), close: vi.fn() } + handler.setPreTestHookReporter(reporter) + + await handler['ensurePreTestHookRun']() + + expect(reporter.open).not.toHaveBeenCalled() + }) +}) + +describe('pre-test hook run (cucumber, Direct flow)', () => { + let reporter: { open: ReturnType, close: ReturnType } + + beforeEach(() => { + accessibilityHandler = new AccessibilityHandler(browser, caps, options, false, config, 'cucumber', true, false, accessibilityOpts) + reporter = { open: vi.fn().mockResolvedValue('hook-uuid'), close: vi.fn().mockResolvedValue(undefined) } + accessibilityHandler.setPreTestHookReporter(reporter) + }) + + it('stamps the hook uuid onto scans, and clears it when the window closes', async () => { + // commandWrapper passes _currentHookRunUuid to performA11yScan as thHookRunUuid; without + // this the hook run exists but the window's scans still arrive with no parent. + await accessibilityHandler['ensurePreTestHookRun']() + expect(accessibilityHandler['_currentHookRunUuid']).toBe('hook-uuid') + + await accessibilityHandler['closePreTestHookRun']() + expect(accessibilityHandler['_currentHookRunUuid']).toBeNull() + }) + + it('opens once on demand and closes when the first scenario starts', async () => { + await accessibilityHandler['ensurePreTestHookRun']() + await accessibilityHandler['ensurePreTestHookRun']() + expect(reporter.open).toHaveBeenCalledTimes(1) + + await accessibilityHandler['closePreTestHookRun']() + + expect(reporter.close).toHaveBeenCalledWith('hook-uuid', true, undefined) + }) + + it('never closes a run it did not open', async () => { + await accessibilityHandler['closePreTestHookRun']() + + expect(reporter.close).not.toHaveBeenCalled() + }) + + it('reports nothing when no reporter is installed', async () => { + const bare = new AccessibilityHandler(browser, caps, options, false, config, 'cucumber', true, false, accessibilityOpts) + + await bare['ensurePreTestHookRun']() + + expect(bare['_preTestHookState']).toBe('idle') + }) + + it('stays closed-but-unreported when open() yields no uuid', async () => { + reporter.open.mockResolvedValue(undefined) + + await accessibilityHandler['ensurePreTestHookRun']() + await accessibilityHandler['closePreTestHookRun']() + + expect(reporter.close).not.toHaveBeenCalled() + }) +}) + +describe('pre-test scan gate (non-CLI flow)', () => { + beforeEach(() => { + accessibilityHandler = new AccessibilityHandler(browser, caps, options, false, config, 'mocha', true, false, accessibilityOpts) + vi.spyOn(utils, 'isBrowserstackSession').mockReturnValue(true) + vi.spyOn(utils, 'isAccessibilityAutomationSession').mockReturnValue(true) + vi.spyOn(utils, 'validateCapsWithA11y').mockReturnValue(true) + }) + + it('opens the gate in before(), so config-level hook commands scan', async () => { + await accessibilityHandler.before('session-direct') + + expect(AccessibilityHandler['_a11yScanSessionMap']['session-direct']).toBe(true) + }) + + it('leaves it closed when autoScanning is off', async () => { + const handler = new AccessibilityHandler(browser, caps, options, false, config, 'mocha', true, false, + { ...accessibilityOpts, autoScanning: false } as never) + + await handler.before('session-noauto') + + expect(AccessibilityHandler['_a11yScanSessionMap']['session-noauto']).toBeUndefined() + }) + + it('is recomputed per test, so a filtered test still closes it', async () => { + await accessibilityHandler.before('session-direct-2') + expect(AccessibilityHandler['_a11yScanSessionMap']['session-direct-2']).toBe(true) + + vi.spyOn(utils, 'shouldScanTestForAccessibility').mockReturnValue(false) + await accessibilityHandler.beforeTest('suite', { title: 'excluded', parent: 'suite' } as never) + + expect(AccessibilityHandler['_a11yScanSessionMap']['session-direct-2']).toBe(false) + }) +}) + describe('onSessionReload', () => { beforeEach(() => { accessibilityHandler = new AccessibilityHandler(browser, caps, options, false, config, 'framework', true, false, accessibilityOpts) diff --git a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts index 0c77511..23ced77 100644 --- a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts +++ b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts @@ -40,6 +40,28 @@ vi.mock('../../../src/util.js', () => ({ isBrowserstackSession: vi.fn().mockReturnValue(true) })) +// 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 }) + }) + } +})) + +const { mockGetHookFailure, mockGetActiveHookName } = vi.hoisted(() => ({ + mockGetHookFailure: vi.fn(), + mockGetActiveHookName: vi.fn() +})) +vi.mock('../../../src/hookInstrumentation.js', () => ({ + getPreTestWindowFailure: mockGetHookFailure, + getActiveHookName: mockGetActiveHookName +})) + vi.mock('../../../src/cli/grpcClient.js', () => ({ GrpcClient: { getInstance: vi.fn().mockReturnValue({ @@ -110,6 +132,7 @@ describe('AccessibilityModule', () => { afterEach(() => { vi.resetAllMocks() + mockTrackEvent.mockReset() }) describe('constructor', () => { @@ -200,6 +223,108 @@ describe('AccessibilityModule', () => { }) }) + describe('pre-test scan gate and its hook run', () => { + const withA11yOn = () => { + vi.mocked(validateCapsWithA11y).mockReturnValue(true) + vi.mocked(AutomationFramework.getState).mockImplementation((instance, key) => { + if (key.includes('CAPABILITIES')) { + return { browserName: 'chrome' } + } + return 'live-session' + }) + } + + 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. + withA11yOn() + + await accessibilityModule.onBeforeExecute() + + expect(accessibilityModule.accessibilityMap.get('live-session')).toBe(true) + }) + + it('leaves the gate closed when autoScanning is off', async () => { + withA11yOn() + accessibilityModule.autoScanning = false + + await accessibilityModule.onBeforeExecute() + + expect(accessibilityModule.accessibilityMap.has('live-session')).toBe(false) + }) + + 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('hook') + }) + + it('names the hook run after the hook that was executing, not a fixed label', async () => { + // beforeSuite runs in the same window as before(), so a fixed 'before' label would + // name the wrong hook whenever the first scan lands in beforeSuite. + mockGetActiveHookName.mockReturnValue('beforeSuite') + + await accessibilityModule['ensurePreTestHookRun']() + + const open = mockTrackEvent.mock.calls.find((c) => c[1] === HookState.PRE) + expect((open![2] as { test: { title: string } }).test.title).toBe('wdio "beforeSuite" hook') + }) + + it('falls back to a neutral name when no hook is identifiable', async () => { + mockGetActiveHookName.mockReturnValue(undefined) + + await accessibilityModule['ensurePreTestHookRun']() + + const open = mockTrackEvent.mock.calls.find((c) => c[1] === HookState.PRE) + expect((open![2] as { test: { title: string } }).test.title).toBe('wdio config-level hook') + }) + + it('reports the hook run as FAILED when the config-level before() threw', async () => { + // WDIO swallows a throwing config hook (executeHooksWithArgs resolves with the error), + // so the run stays exit-0 and every reporter is green. Reporting `passed` here would + // make the dashboard assert something false rather than merely omit it. + mockGetHookFailure.mockReturnValue('before: BOOM: config-level before hook failed') + await accessibilityModule['ensurePreTestHookRun']() + mockTrackEvent.mockClear() + + await accessibilityModule.onBeforeTest({ suiteTitle: 'S', test: { title: 't' } }) + + const close = mockTrackEvent.mock.calls.find((c) => c[1] === HookState.POST) + expect(close).toBeDefined() + const result = (close![2] as { result: { passed: boolean, error?: { message: string } } }).result + expect(result.passed).toBe(false) + expect(result.error?.message).toContain('BOOM') + }) + + 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, and never closes one it did not open', 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) + expect(accessibilityModule.currentHookRunUuid).toBeNull() + + mockTrackEvent.mockClear() + await accessibilityModule.onBeforeTest({ suiteTitle: 'S', test: { title: 't2' } }) + 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', true) @@ -570,7 +695,21 @@ describe('AccessibilityModule', () => { await (accessibilityModule as any).performScanCli(mockBrowser, 'click', 'hook-uuid-99') - expect(_getParamsForAppAccessibility).toHaveBeenCalledWith('click', undefined, 'hook-uuid-99') + // 4th arg is the global-hook flag; a direct call passes none, so it forwards undefined + expect(_getParamsForAppAccessibility).toHaveBeenCalledWith('click', undefined, 'hook-uuid-99', undefined) + }) + + it('marks a scan from the pre-test window as a global-hook scan', async () => { + // the flag is what strips thTestRunUuid: in that window the env var holds a uuid for a + // test that has not started, so sending it would attribute the scan to the wrong test + accessibilityModule.accessibility = true + accessibilityModule.isAppAccessibility = true + mockBrowser.execute.mockResolvedValue({ scanned: true }) + + await (accessibilityModule as never as { performScanCli: (b: unknown, c?: string, h?: string | null, g?: boolean) => Promise }) + .performScanCli(mockBrowser, 'back', 'hook-uuid-1', true) + + expect(_getParamsForAppAccessibility).toHaveBeenCalledWith('back', undefined, 'hook-uuid-1', true) }) it('passes no hook uuid for an ordinary (non-hook) app scan', async () => { @@ -580,7 +719,7 @@ describe('AccessibilityModule', () => { await (accessibilityModule as any).performScanCli(mockBrowser, 'click') - expect(_getParamsForAppAccessibility).toHaveBeenCalledWith('click', undefined, undefined) + expect(_getParamsForAppAccessibility).toHaveBeenCalledWith('click', undefined, undefined, undefined) }) }) }) \ No newline at end of file diff --git a/packages/browserstack-service/tests/hookInstrumentation.test.ts b/packages/browserstack-service/tests/hookInstrumentation.test.ts new file mode 100644 index 0000000..a659e90 --- /dev/null +++ b/packages/browserstack-service/tests/hookInstrumentation.test.ts @@ -0,0 +1,164 @@ +import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest' + +vi.mock('../src/bstackLogger.js', () => ({ BStackLogger: { debug: vi.fn() } })) + +import { BStackLogger } from '../src/bstackLogger.js' +import { instrumentBrowserContextHooks, getPreTestWindowFailure, getActiveHookName } from '../src/hookInstrumentation.js' +import { BROWSER_CONTEXT_HOOKS } from '../src/constants.js' + +describe('pre-test window failures', () => { + beforeEach(() => { + // the recorder is module state; clear it by re-running the hooks that write to it + instrumentBrowserContextHooks({ before: [], beforeSuite: [] } as never) + }) + + it('records a rejection from the config-level before hook', async () => { + const config = { before: [async () => { throw new Error('BOOM from before') }] } as never + + instrumentBrowserContextHooks(config) + await expect((config as { before: Array<() => Promise> }).before[0]()).rejects.toThrow('BOOM from before') + + expect(getPreTestWindowFailure()).toBe('before: BOOM from before') + }) + + it('also catches beforeSuite, which the Mocha adapter runs inside the same window', async () => { + // registered as a root before-all by @wdio/mocha-framework, so it runs after the + // config-level before and still ahead of the first test + const config = { beforeSuite: [async () => { throw new Error('BOOM from beforeSuite') }] } as never + + instrumentBrowserContextHooks(config) + await expect((config as { beforeSuite: Array<() => Promise> }).beforeSuite[0]()).rejects.toThrow('BOOM from beforeSuite') + + expect(getPreTestWindowFailure()).toBe('beforeSuite: BOOM from beforeSuite') + }) + + it('keeps a synchronous handler synchronous, and still records its throw', () => { + const config = { before: [() => { throw new Error('sync BOOM') }] } as never + + instrumentBrowserContextHooks(config) + const handler = (config as { before: Array<() => unknown> }).before[0] + + // not a promise: an async wrapper would turn this throw into an unhandled rejection + expect(() => handler()).toThrow('sync BOOM') + expect(getPreTestWindowFailure()).toBe('before: sync BOOM') + }) + + it('reports nothing when the window ran clean', async () => { + const config = { before: [async () => 'fine'] } as never + + instrumentBrowserContextHooks(config) + await (config as { before: Array<() => Promise> }).before[0]() + + expect(getPreTestWindowFailure()).toBeUndefined() + }) +}) + +describe('active hook name', () => { + it('reports which hook is executing, and nothing once it has finished', async () => { + const seen: Array = [] + const config = { + before: [async () => { seen.push(getActiveHookName()) }], + beforeSuite: [async () => { seen.push(getActiveHookName()) }] + } as never + + instrumentBrowserContextHooks(config) + const c = config as unknown as { before: Array<() => Promise>, beforeSuite: Array<() => Promise> } + await c.before[0]() + await c.beforeSuite[0]() + + expect(seen).toEqual(['before', 'beforeSuite']) + expect(getActiveHookName()).toBeUndefined() + }) + + it('unwinds even when the hook throws', async () => { + const config = { before: [async () => { throw new Error('BOOM') }] } as never + + instrumentBrowserContextHooks(config) + await expect((config as { before: Array<() => Promise> }).before[0]()).rejects.toThrow('BOOM') + + expect(getActiveHookName()).toBeUndefined() + }) +}) + +describe('coverage of the patched set', () => { + it('logs a start and a finish for every hook in BROWSER_CONTEXT_HOOKS', async () => { + // Guards the list against drift: adding a hook name without it actually being wrapped + // would otherwise show up only as a silence in a device log. + const config = Object.fromEntries( + BROWSER_CONTEXT_HOOKS.map((name) => [name, [async () => undefined]]) + ) as unknown as never + + instrumentBrowserContextHooks(config) + vi.mocked(BStackLogger.debug).mockClear() + + for (const name of BROWSER_CONTEXT_HOOKS) { + await (config as unknown as Record Promise>>)[name][0]() + } + + const logged = vi.mocked(BStackLogger.debug).mock.calls.map((c) => String(c[0])) + for (const name of BROWSER_CONTEXT_HOOKS) { + expect(logged.some((l) => l.includes(`[hook-window] ${name}#0`) && l.includes('started'))).toBe(true) + expect(logged.some((l) => l.includes(`[hook-window] ${name}#0`) && l.includes('finished'))).toBe(true) + } + }) +}) + +describe('instrumentation cannot break the hook it wraps', () => { + afterEach(() => { + vi.mocked(BStackLogger.debug).mockReset() + }) + + it('runs the handler even when our own logging throws', async () => { + vi.mocked(BStackLogger.debug).mockImplementation(() => { throw new Error('logger is gone') }) + let ran = false + const config = { before: [async () => { ran = true; return 'value' }] } as never + + instrumentBrowserContextHooks(config) + const handler = (config as unknown as { before: Array<() => Promise> }).before[0] + + await expect(handler()).resolves.toBe('value') + expect(ran).toBe(true) + }) + + it('still propagates the handler\'s own error, unchanged', async () => { + const boom = new Error('user hook failed') + const config = { before: [async () => { throw boom }] } as never + + instrumentBrowserContextHooks(config) + const handler = (config as unknown as { before: Array<() => Promise> }).before[0] + + await expect(handler()).rejects.toBe(boom) + }) + + it('does not choke on a return value with a hostile then getter', () => { + const hostile = { get then() { throw new Error('no touching') } } + const config = { before: [() => hostile] } as never + + instrumentBrowserContextHooks(config) + const handler = (config as unknown as { before: Array<() => unknown> }).before[0] + + expect(handler()).toBe(hostile) + }) + + it('keeps instrumenting the remaining handlers when one cannot be wrapped', () => { + // reading `.name` is enough to break wrapping for this one + const hostile = new Proxy(() => 'hostile', { + get(target, key) { + if (key === 'name') { + throw new Error('no name for you') + } + return Reflect.get(target, key) + } + }) + const normal = () => 'normal' + const config = { before: [hostile, normal] } as never + + instrumentBrowserContextHooks(config) + const handlers = (config as unknown as { before: Array<() => string> }).before + + expect(handlers[0]).toBe(hostile) // handed back untouched + expect(handlers[1]).not.toBe(normal) // the one after it still got wrapped + expect(handlers[0]()).toBe('hostile') + expect(handlers[1]()).toBe('normal') + }) +})