diff --git a/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts b/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts index 60c6ddb..76e4b88 100644 --- a/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts +++ b/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts @@ -410,7 +410,17 @@ export default class WdioMochaTestFramework extends TestFramework { if (hooksList.length > 0) { const hook = hooksList.pop() as Record - const result = testResult.status + // Frameworks.TestResult carries `passed`/`skipped`, not `status` — reading `.status` + // left hook_result at 'pending', which the binary coerces to 'passed' for any + // finished hook, so failed before-hooks showed green builds while CI exited 1. + // A this.skip() hook arrives as {passed: false, skipped: true} — a deliberate + // skip, not a failure. + // guard the sync-skip error shape too (wdio v8 delivers this.skip() hooks without + // a `skipped` flag; harmless on v9, keeps the lines aligned) + const skippedHook = testResult?.skipped || !!testResult?.error?.message?.includes('sync skip; aborting execution') + const result = testResult + ? (testResult.passed ? 'passed' : (skippedHook ? 'skipped' : 'failed')) + : TestFrameworkConstants.DEFAULT_HOOK_RESULT if (result !== TestFrameworkConstants.DEFAULT_HOOK_RESULT) { hook[TestFrameworkConstants.KEY_HOOK_RESULT] = result } diff --git a/packages/browserstack-service/src/cli/skipReporter.ts b/packages/browserstack-service/src/cli/skipReporter.ts new file mode 100644 index 0000000..5ada0da --- /dev/null +++ b/packages/browserstack-service/src/cli/skipReporter.ts @@ -0,0 +1,96 @@ +import path from 'node:path' + +import type { Frameworks } from '@wdio/types' + +import type TestFramework from './frameworks/testFramework.js' +import { TestFrameworkState } from './states/testFrameworkState.js' +import { HookState } from './states/hookState.js' +import { BStackLogger } from '../bstackLogger.js' + +/** + * Reports tests that never reach the beforeTest/afterTest lifecycle (static `it.skip`, + * `this.skip()` inside before hooks, suites aborted by a failed before hook) through the + * CLI gRPC tracker, so they land on the dashboard and attribute their Automate session. + * Without this, such tests emit no events at all in the CLI flow (the legacy + * Listener -> api/v1/batch path is dead here) and their sessions surface as + * `session_linking_issue_build` in TRA stability. + */ + +interface MochaRuntimeTest { + title: string + state?: string + body?: string + file?: string + parent?: { title?: string, parent?: unknown, tests?: unknown[], suites?: unknown[] } +} + +// tests that entered the CLI beforeTest lifecycle — those report their own finish +// (including runtime `this.skip()` inside a test body) and must not be re-reported +const startedTests = new Set() +const reportedSkips = new Set() +// wdio does not await reporter hooks, so back-to-back skips would interleave on the +// tracker's single mutable per-worker instance — serialize every report through one chain +let reportChain: Promise = Promise.resolve() + +export function markTestStarted(identifier: string) { + startedTests.add(identifier) +} + +export function reportSkippedTest(framework: TestFramework, identifier: string, test: Frameworks.Test, suiteTitle?: string): Promise { + if (startedTests.has(identifier) || reportedSkips.has(identifier)) { + return reportChain + } + reportedSkips.add(identifier) + const result = { passed: false, skipped: true } as Frameworks.TestResult + reportChain = reportChain.then(async () => { + // LOG_REPORT/POST is what loads the result into the instance (loadTestResult is + // gated on it, not on TEST/POST) — same sequence afterTest uses + await framework.trackEvent(TestFrameworkState.INIT_TEST, HookState.PRE, { test }) + await framework.trackEvent(TestFrameworkState.TEST, HookState.PRE, { test, suiteTitle }) + await framework.trackEvent(TestFrameworkState.LOG_REPORT, HookState.POST, { test, result }) + await framework.trackEvent(TestFrameworkState.TEST, HookState.POST, { test, result, suiteTitle }) + }).catch((err: unknown) => { + BStackLogger.debug(`Failed reporting skipped test '${identifier}': ${err}`) + }) + return reportChain +} + +/** + * Port of the legacy insights-handler skip propagation: when a BEFORE_ALL/BEFORE_EACH/ + * AFTER_EACH hook fails (or skips), mocha silently drops the remaining tests in the + * suite — report each state-undefined test as skipped, recursing into nested describes. + */ +export async function reportSuiteSkipped(framework: TestFramework, suite: { tests?: unknown[], suites?: unknown[] }): Promise { + for (const t of (suite.tests || []) as MochaRuntimeTest[]) { + if (t.state !== undefined) { + continue + } + const parentTitle = t.parent?.title ?? '' + const identifier = `${parentTitle} - ${t.title}` + // keep `parent` a string (the Automate session name interpolates it) and pass the + // real suite chain via ctx for scope/hierarchy extraction; `file` must resolve or + // the binary drops the event on path.relative(cwd, undefined) + const synthetic = { + title: t.title, + parent: parentTitle, + body: t.body || '', + file: t.file, + ctx: { test: { parent: t.parent } } + } as unknown as Frameworks.Test + await reportSkippedTest(framework, identifier, synthetic, parentTitle) + } + for (const sub of (suite.suites || []) as { tests?: unknown[], suites?: unknown[] }[]) { + await reportSuiteSkipped(framework, sub) + } +} + +/** Resolve a spec file path usable by the binary (it rejects events with no location). */ +export function resolveSpecFile(candidate: string | undefined, runnerSpec: string | undefined): string | undefined { + if (candidate) { + return candidate + } + if (runnerSpec) { + return runnerSpec.startsWith('file://') ? runnerSpec.replace(/^file:\/\//, '') : path.resolve(runnerSpec) + } + return undefined +} diff --git a/packages/browserstack-service/src/reporter.ts b/packages/browserstack-service/src/reporter.ts index 4fd6b73..1c48f50 100644 --- a/packages/browserstack-service/src/reporter.ts +++ b/packages/browserstack-service/src/reporter.ts @@ -2,7 +2,9 @@ import path from 'node:path' import type { SuiteStats, TestStats, RunnerStats, HookStats } from '@wdio/reporter' import WDIOReporter from '@wdio/reporter' -import type { Options } from '@wdio/types' +import type { Options, Frameworks } from '@wdio/types' +import { BrowserstackCLI } from './cli/index.js' +import { reportSkippedTest, resolveSpecFile } from './cli/skipReporter.js' import * as url from 'node:url' import { v4 as uuidv4 } from 'uuid' @@ -40,10 +42,12 @@ class _TestReporter extends WDIOReporter { public static currentTest: CurrentRunInfo = {} private _userCaps?: Capabilities.ResolvedTestrunnerCapabilities = {} private listener = Listener.getInstance() + private _runnerSpec?: string public static hashCodeToHandleTestSkip: Record = {} async onRunnerStart (runnerStats: RunnerStats) { this._capabilities = runnerStats.capabilities as WebdriverIO.Capabilities + this._runnerSpec = runnerStats.specs?.[0] this._userCaps = this.getUserCaps(runnerStats) this._config = runnerStats.config as BrowserstackConfig & Options.Testrunner this._sessionId = runnerStats.sessionId @@ -217,6 +221,30 @@ class _TestReporter extends WDIOReporter { return } + // CLI flow: the legacy Listener -> api/v1/batch path below is dead here, so route the + // skipped test through the gRPC tracker instead (static `it.skip` and hook-level skips + // never reach beforeTest/afterTest; without this they are invisible on the dashboard + // and their session is never linked to the build) + if (BrowserstackCLI.getInstance().isRunning()) { + const framework = BrowserstackCLI.getInstance().getTestFramework() + if (framework) { + const stats = testStats as TestStats & { parent?: string, file?: string, body?: string, ctx?: unknown } + const identifier = `${stats.parent} - ${stats.title}` + stats.file = resolveSpecFile(stats.file, this._runnerSpec) + // hierarchy travels via ctx (getMochaTestHierarchy); `parent` stays a string — + // the Automate session name interpolates it directly + let parentChain: { title: string, parent: unknown } | null = null + for (const s of this._suites) { + parentChain = { title: s.title, parent: parentChain } + } + if (parentChain) { + stats.ctx = { test: { parent: parentChain } } + } + await reportSkippedTest(framework, identifier, stats as unknown as Frameworks.Test, this._suites.at(-1)?.title) + } + return + } + testStats.start ||= new Date() testStats.end ||= new Date() const testData = await this.getRunData(testStats, 'TestRunSkipped') diff --git a/packages/browserstack-service/src/service.ts b/packages/browserstack-service/src/service.ts index c5f21d1..8a92cf0 100644 --- a/packages/browserstack-service/src/service.ts +++ b/packages/browserstack-service/src/service.ts @@ -30,6 +30,7 @@ import PerformanceTester from './instrumentation/performance/performance-tester. import * as PERFORMANCE_SDK_EVENTS from './instrumentation/performance/constants.js' import { EVENTS } from './instrumentation/performance/constants.js' import { BrowserstackCLI } from './cli/index.js' +import { markTestStarted, reportSuiteSkipped } from './cli/skipReporter.js' import { CLIUtils } from './cli/cliUtils.js' import { _fetch as fetch } from './fetchWrapper.js' @@ -435,6 +436,14 @@ export default class BrowserstackService implements Services.ServiceInstance { if (hookFrameworkState) { await framework.trackEvent(hookFrameworkState, HookState.POST, { test, result }) } + // a failed (or skipping) before/each hook silently drops the suite's remaining + // tests in mocha — report them as skipped so they surface on the dashboard and + // attribute their Automate session (port of the legacy insights-handler cascade) + const hookType = getHookType((test as Frameworks.Test).title) + const suite = (test as Frameworks.Test).ctx?.test?.parent + if (result && !result.passed && ['BEFORE_ALL', 'BEFORE_EACH', 'AFTER_EACH'].includes(hookType) && suite) { + await reportSuiteSkipped(framework, suite) + } } return } @@ -467,6 +476,9 @@ export default class BrowserstackService implements Services.ServiceInstance { if (this._config.framework === 'mocha' && uuid) { this._cliTestUuids.set(getUniqueIdentifier(test, this._config.framework), uuid as string) } + // this test reports its own finish (incl. runtime `this.skip()`), so the + // skip reporter must never re-report it from onTestSkip + markTestStarted(getUniqueIdentifier(test, this._config.framework)) this._insightsHandler?.setTestData(test, uuid) await BrowserstackCLI.getInstance().getTestFramework()!.trackEvent(TestFrameworkState.TEST, HookState.PRE, { test, suiteTitle }) return diff --git a/packages/browserstack-service/src/testOps/listener.ts b/packages/browserstack-service/src/testOps/listener.ts index 160a2ae..29ec61e 100644 --- a/packages/browserstack-service/src/testOps/listener.ts +++ b/packages/browserstack-service/src/testOps/listener.ts @@ -228,8 +228,8 @@ class Listener { await batchAndPostEvents(DATA_BATCH_ENDPOINT, 'BATCH_DATA', data) BStackLogger.debug('callback: marking events success ' + data.length) this.eventsSuccess(data) - } catch { - BStackLogger.debug('callback: marking events failed ' + data.length) + } catch (err) { + BStackLogger.debug(`callback: marking events failed ${data.length} reason: ${err}`) this.eventsFailed(data) } finally { this.pendingUploads -= 1 diff --git a/packages/browserstack-service/src/util.ts b/packages/browserstack-service/src/util.ts index 136eebd..ca200b4 100644 --- a/packages/browserstack-service/src/util.ts +++ b/packages/browserstack-service/src/util.ts @@ -1257,7 +1257,13 @@ export async function batchAndPostEvents (eventUrl: string, kind: string, data: }, body: JSON.stringify(data) }) - BStackLogger.debug(`[${kind}] Success response: ${JSON.stringify(await response.json())}`) + // read as text first: error responses (401/5xx) and empty bodies are not JSON, and a blind + // response.json() surfaced them as a misleading "Unexpected end of JSON input" + const responseBody = await response.text() + if (!response.ok) { + throw new Error(`HTTP ${response.status} ${response.statusText}${responseBody ? ` — ${responseBody.slice(0, 300)}` : ' (empty body)'}`) + } + BStackLogger.debug(`[${kind}] Success response: ${responseBody}`) } catch (error) { BStackLogger.debug(`[${kind}] EXCEPTION IN ${kind} REQUEST TO TEST REPORTING AND ANALYTICS : ${error}`) throw new Error('Exception in request ' + error) diff --git a/packages/browserstack-service/tests/cli/skipReporter.test.ts b/packages/browserstack-service/tests/cli/skipReporter.test.ts new file mode 100644 index 0000000..12aea7e --- /dev/null +++ b/packages/browserstack-service/tests/cli/skipReporter.test.ts @@ -0,0 +1,93 @@ +import path from 'node:path' + +import { describe, expect, it, vi, beforeEach } from 'vitest' +import type { Frameworks } from '@wdio/types' + +vi.mock('@wdio/logger', () => import(path.join(process.cwd(), '__mocks__', '@wdio/logger'))) + +import { markTestStarted, reportSkippedTest, reportSuiteSkipped, resolveSpecFile } from '../../src/cli/skipReporter.js' +import { TestFrameworkState } from '../../src/cli/states/testFrameworkState.js' +import { HookState } from '../../src/cli/states/hookState.js' +import type TestFramework from '../../src/cli/frameworks/testFramework.js' + +const makeFramework = () => ({ trackEvent: vi.fn().mockResolvedValue(undefined) }) as unknown as TestFramework + +const makeTest = (title: string, parent = 'suite') => ({ title, parent }) as unknown as Frameworks.Test + +describe('skipReporter', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('reports a skipped test through the INIT_TEST/TEST/LOG_REPORT sequence', async () => { + const framework = makeFramework() + await reportSkippedTest(framework, 'suite - reports once', makeTest('reports once'), 'suite') + + const calls = vi.mocked(framework.trackEvent).mock.calls + expect(calls.map(([state, hook]) => [state, hook])).toEqual([ + [TestFrameworkState.INIT_TEST, HookState.PRE], + [TestFrameworkState.TEST, HookState.PRE], + [TestFrameworkState.LOG_REPORT, HookState.POST], + [TestFrameworkState.TEST, HookState.POST], + ]) + expect(calls[2][2]).toMatchObject({ result: { passed: false, skipped: true } }) + }) + + it('does not re-report the same identifier', async () => { + const framework = makeFramework() + await reportSkippedTest(framework, 'suite - dedup', makeTest('dedup'), 'suite') + await reportSkippedTest(framework, 'suite - dedup', makeTest('dedup'), 'suite') + expect(framework.trackEvent).toHaveBeenCalledTimes(4) + }) + + it('does not report tests that entered the beforeTest lifecycle', async () => { + const framework = makeFramework() + markTestStarted('suite - runtime skip') + await reportSkippedTest(framework, 'suite - runtime skip', makeTest('runtime skip'), 'suite') + expect(framework.trackEvent).not.toHaveBeenCalled() + }) + + it('serializes concurrent reports through one chain', async () => { + const order: string[] = [] + const framework = { + trackEvent: vi.fn().mockImplementation(async (_s: unknown, _h: unknown, args: { test: { title: string } }) => { + order.push(args.test.title) + await new Promise(resolve => setTimeout(resolve, 1)) + }) + } as unknown as TestFramework + + await Promise.all([ + reportSkippedTest(framework, 'suite - first', makeTest('first'), 'suite'), + reportSkippedTest(framework, 'suite - second', makeTest('second'), 'suite'), + ]) + expect(order).toEqual(['first', 'first', 'first', 'first', 'second', 'second', 'second', 'second']) + }) + + it('walks nested suites and skips already-determined tests', async () => { + const framework = makeFramework() + const parent = { title: 'outer' } + const suite = { + tests: [ + { title: 'ran already', state: 'passed', parent }, + { title: 'undetermined', parent, file: '/spec/a.spec.js' }, + ], + suites: [{ + tests: [{ title: 'nested undetermined', parent: { title: 'inner' } }], + suites: [], + }], + } + await reportSuiteSkipped(framework, suite) + // 2 undetermined tests x 4 tracker events + expect(framework.trackEvent).toHaveBeenCalledTimes(8) + const reported = vi.mocked(framework.trackEvent).mock.calls + .filter(([state]) => state === TestFrameworkState.INIT_TEST) + .map(([, , args]) => (args as { test: { title: string } }).test.title) + expect(reported).toEqual(['undetermined', 'nested undetermined']) + }) + + it('resolves spec file from the runner spec when the test has none', () => { + expect(resolveSpecFile('/abs/spec.js', 'file:///runner/spec.js')).toBe('/abs/spec.js') + expect(resolveSpecFile(undefined, 'file:///runner/spec.js')).toBe('/runner/spec.js') + expect(resolveSpecFile(undefined, undefined)).toBeUndefined() + }) +})