fix(v8): report skipped, hook-aborted tests and hook results in the CLI flow

packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts

@@ -44,7 +44,10 @@ export default class WdioMochaTestFramework extends TestFramework {
         }
 
         try {
-            if (CLIUtils.matchHookRegex(testFrameworkState.toString()) && hookState === HookState.PRE) {
+            // HOOK_REGEX is anchored (^BEFORE_|^AFTER_) — match the short state name; the
+            // fully-qualified `TestFrameworkState.BEFORE_ALL` never matches, so hook ids were
+            // never minted and hooks reached TRA with an empty uuid (dropped at ingestion).
+            if (CLIUtils.matchHookRegex(testFrameworkState.toString().split('.')[1]) && hookState === HookState.PRE) {
                 instance.updateMultipleEntries({
                     [TestFrameworkConstants.KEY_HOOK_ID]: uuidv4(),
                 })
@@ -93,13 +96,49 @@ export default class WdioMochaTestFramework extends TestFramework {
    * @returns {TestFrameworkInstance}
    */
     resolveInstance(testFrameworkState: State, hookState: State, args: Record<string, unknown> = {}): TestFrameworkInstance|null {
-        let instance = null
         logger.info(`resolveInstance: resolving instance for testFrameworkState=${testFrameworkState} hookState=${hookState}`)
-        if (testFrameworkState === TestFrameworkState.INIT_TEST || testFrameworkState === TestFrameworkState.NONE) {
+        const shortState = testFrameworkState.toString().split('.')[1]
+        const isHook = CLIUtils.matchHookRegex(shortState)
+        let instance = TestFramework.getTrackedInstance()
+
+        // Whether the current tracked instance has already run its test body.
+        const hasRunTest = !!(instance && TestFramework.getState(instance, TestFrameworkConstants.KEY_TEST_ID))
+
+        // New-test boundary (ports Junit5Framework.resolveInstance): the previous method was a
+        // test / after-hook (POST) and the current is a before-hook / init (PRE) — the next test
+        // is starting. At entry, getCurrentTestState()/getCurrentHookState() still hold the PREVIOUS
+        // method's state (updateInstanceState has not run yet).
+        const prevState = instance ? instance.getCurrentTestState().toString() : ''
+        const prevWasTerminal = instance ? (instance.getCurrentTestState() === TestFrameworkState.TEST || prevState.includes('AFTER')) : false
+        const isNewTestBoundary = instance ? (prevWasTerminal && instance.getCurrentHookState() === HookState.POST && hookState === HookState.PRE) : false
+
+        if (testFrameworkState === TestFrameworkState.NONE) {
             this.trackWdioMochaInstance(testFrameworkState, args)
+        } else if (testFrameworkState === TestFrameworkState.INIT_TEST) {
+            // WDIO fires `before each` BEFORE `beforeTest` (INIT_TEST). Reuse the instance a
+            // preceding before-hook already opened for this same upcoming test; only start a new
+            // one if the current instance has already run a test (or none exists).
+            if (!instance || hasRunTest) {
+                this.trackWdioMochaInstance(testFrameworkState, args)
+            }
+        } else if (isHook && hookState === HookState.PRE) {
+            // Suite-level (`before all`) and the first `before each` fire before any INIT_TEST, so
+            // no instance exists yet — create one. Also, a BEFORE hook right after a completed test
+            // (new-test boundary) starts a new test. The boundary restriction must be limited to
+            // BEFORE hooks: `after each`/`after all` also fire at a POST->PRE boundary (afterTest
+            // emits TEST POST just before `after each`), but they belong to the just-finished test.
+            // Creating a fresh (uuid-less) instance for them would drop test_run_id and orphan the
+            // hook from TestRunFinished.hooks — so let after-hooks reuse the finished test's instance.
+            if (!instance || (isNewTestBoundary && shortState.startsWith('BEFORE_'))) {
+                this.trackWdioMochaInstance(testFrameworkState, args)
+            }
         }
 
         instance = TestFramework.getTrackedInstance()
+        if (!instance) {
+            logger.error(`resolveInstance: unable to resolve/create instance for testFrameworkState=${testFrameworkState} hookState=${hookState}`)
+            return null
+        }
         this.updateInstanceState(instance, testFrameworkState, hookState)
 
         return instance
@@ -167,14 +206,18 @@ export default class WdioMochaTestFramework extends TestFramework {
     }
 
     loadTestResult(instance: TestFrameworkInstance, args: Record<string, unknown>) {
-        const results = args.result as Frameworks.TestResult
-        const { error, passed } = results
+        const results = args.result as Frameworks.TestResult & { skipped?: boolean }
+        const { error, passed, skipped } = results
         let result = 'passed'
         let failure: Array<unknown>|null = null
         let failureReason: string|null = null
         let failureType: string|null = null
         if (!passed) {
-            result = (error && error.message && error.message.includes('sync skip; aborting execution')) ? 'ignore' : 'failed'
+            if (skipped) {
+                result = 'skipped'
+            } else {
+                result = (error && error.message && error.message.includes('sync skip; aborting execution')) ? 'ignore' : 'failed'
+            }
             if (error && result !== 'skipped') {
                 failure = [{ backtrace: [removeAnsiColors(error.message), removeAnsiColors(error.stack || '')] }] // add all errors here
                 failureReason = removeAnsiColors(error.message)
@@ -319,7 +362,11 @@ export default class WdioMochaTestFramework extends TestFramework {
     ) {
         const testResult = args.result as Frameworks.TestResult
         const test = args.test as Frameworks.Test
-        const key = testFrameworkState.toString()
+        // Key hooks by the short state name (e.g. BEFORE_ALL), matching how the binary looks
+        // them up via `event.test_hooks_started[request.testFrameworkState]`. `toString()` yields
+        // the fully-qualified `TestFrameworkState.BEFORE_ALL`, which never matches — hook finishes
+        // were dropped with "unable to determine hook-finished".
+        const key = testFrameworkState.toString().split('.')[1]
 
         const hooksStarted = TestFramework.getState(instance, TestFrameworkConstants.KEY_HOOKS_STARTED) as Map<string, unknown[]>
         if (!hooksStarted.has(key)) {
@@ -358,7 +405,19 @@ export default class WdioMochaTestFramework extends TestFramework {
 
             if (hooksList.length > 0) {
                 const hook = hooksList.pop() as Record<string, unknown>
-                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.
+                // wdio v8's TestResult type does not declare `skipped`, but the runtime carries it
+                // wdio v8 does not set `skipped` for a this.skip() before-all — it surfaces as an
+                // error carrying mocha's sync-skip marker; treat both shapes as a deliberate skip
+                const skippedHook = (testResult as Frameworks.TestResult & { skipped?: boolean })?.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
                 }

packages/browserstack-service/src/cli/modules/testHubModule.ts

@@ -113,7 +113,10 @@ export default class TestHubModule extends BaseModule {
             this.logger.debug(`sendTestFrameworkEvent for testState: ${testFrameworkState} hookState: ${testHookState}`)
             const platformIndex = process.env.WDIO_WORKER_ID ? parseInt(process.env.WDIO_WORKER_ID.split('-')[0]) : 0
             const uuid =  TestFramework.getState(instance, TestFrameworkConstants.KEY_TEST_UUID) || instance.getRef()
-            const eventJson = Buffer.from(JSON.stringify(Object.fromEntries(testData)))
+            // Nested values such as test_hooks_started/test_hooks_finished are JS Maps, which
+            // JSON.stringify would serialise to `{}` and strip the hook data. Convert any Map to
+            // a plain object so the binary receives populated hook maps.
+            const eventJson = Buffer.from(JSON.stringify(Object.fromEntries(testData), (_key, value) => value instanceof Map ? Object.fromEntries(value) : value))
             const executionContext = { hash: trackedContext.getId(), threadId: trackedContext.getThreadId().toString(), processId: trackedContext.getProcessId().toString() }
             const payload: Omit<TestFrameworkEventRequest, 'binSessionId'> = {
                 platformIndex,

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<string>()
+const reportedSkips = new Set<string>()
+// 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<void> = Promise.resolve()
+
+export function markTestStarted(identifier: string) {
+    startedTests.add(identifier)
+}
+
+export function reportSkippedTest(framework: TestFramework, identifier: string, test: Frameworks.Test, suiteTitle?: string): Promise<void> {
+    if (startedTests.has(identifier) || reportedSkips.has(identifier)) {
+        return reportChain
+    }
+    reportedSkips.add(identifier)
+    const result = { passed: false, skipped: true } as unknown 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<void> {
+    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
+}

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'
@@ -35,9 +37,11 @@ class _TestReporter extends WDIOReporter {
     public static currentTest: CurrentRunInfo = {}
     private _userCaps?: Capabilities.RemoteCapability = {}
     private listener = Listener.getInstance()
+    private _runnerSpec?: string
 
     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
@@ -211,6 +215,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()
         this.listener.testFinished(await this.getRunData(testStats, 'TestRunSkipped'))