diff --git a/.changeset/pr-122.md b/.changeset/pr-122.md new file mode 100644 index 0000000..9d6aad0 --- /dev/null +++ b/.changeset/pr-122.md @@ -0,0 +1,5 @@ +--- +"@wdio/browserstack-service": minor +--- + +- Tests that never run because Mocha's `bail` halted a spec are now reported as `skipped`, instead of being left out of the report entirely — so a Test Run accounts for every test in that spec. diff --git a/packages/browserstack-service/README.md b/packages/browserstack-service/README.md index cd0127d..17f3216 100644 --- a/packages/browserstack-service/README.md +++ b/packages/browserstack-service/README.md @@ -76,6 +76,14 @@ export const config = { You can explore all the features of Test Reporting and Analytics in [this sandbox](https://automation.browserstack.com/) or read more about it [here](https://www.browserstack.com/docs/test-reporting-and-analytics/overview/what-is-test-observability). +#### Reporting of tests skipped by `bail` + +With `mochaOpts: { bail: true }`, Mocha halts a spec on its first failure and the remaining tests in that file never run. Those tests are now reported with the status `skipped`, so the report accounts for every test in the spec rather than silently omitting them. Tests in sibling `describe` blocks in the same file are covered too. + +Requires `mocha` as the framework and Test Reporting enabled. + +Note that WebdriverIO's own top-level `bail` option is a different setting: it counts failed *spec files* and stops scheduling further ones, rather than stopping tests within a spec. Tests in spec files that never start are not currently reported. + ### browserstackLocal Set this to true to enable routing connections from BrowserStack cloud through your computer. diff --git a/packages/browserstack-service/src/service.ts b/packages/browserstack-service/src/service.ts index b9e2bbe..8a91e08 100644 --- a/packages/browserstack-service/src/service.ts +++ b/packages/browserstack-service/src/service.ts @@ -8,6 +8,7 @@ import { isBrowserstackSession, patchConsoleLogs, isTrue, + isFalse, getUniqueIdentifier, getHookType } from './util.js' @@ -87,6 +88,13 @@ export default class BrowserstackService implements Services.ServiceInstance { private _percyCaptureMode: string | undefined = undefined private _percyHandler?: PercyHandler private _turboScale + /** + * Only mocha's own bail drops the rest of a spec. wdio's top-level `bail` is a launcher + * spec-scheduling filter (it stops queueing further specs once N runners have failed) and + * never reaches mocha, so under it every test in the current spec still runs — enumerating + * on it would report tests as skipped that are about to execute. + */ + private _mochaBail: boolean = false constructor ( options: BrowserstackConfig & Options.Testrunner, @@ -104,6 +112,10 @@ export default class BrowserstackService implements Services.ServiceInstance { this._percy = isTrue(process.env.BROWSERSTACK_PERCY) this._percyCaptureMode = process.env.BROWSERSTACK_PERCY_CAPTURE_MODE this._turboScale = this._options.turboScale + // mirror mocha's own truthiness check on the option rather than isTrue(), which is a + // strict 'true' string compare and would miss a numeric `bail: 1` + const bailOpt: unknown = this._config?.mochaOpts?.bail + this._mochaBail = this._config?.framework === 'mocha' && Boolean(bailOpt) && !isFalse(bailOpt) PerformanceTester.startMonitoring('performance-report-service.csv') if (shouldProcessEventForTesthub('')) { @@ -542,6 +554,7 @@ export default class BrowserstackService implements Services.ServiceInstance { } await BrowserstackCLI.getInstance().getTestFramework()!.trackEvent(TestFrameworkState.LOG_REPORT, HookState.POST, { test, result: results }) await BrowserstackCLI.getInstance().getTestFramework()!.trackEvent(TestFrameworkState.TEST, HookState.POST, { test, result: results, suiteTitle: this._suiteTitle }) + await this.reportBailSkippedTests(test, results) return } @@ -550,6 +563,60 @@ export default class BrowserstackService implements Services.ServiceInstance { await this._percyHandler?.afterTest() } + /** + * Whether this failure will be retried, in which case mocha has not dropped anything yet and + * the tests after it are still going to run. + * + * `results.retries` only tracks wdio's spec-file retries — `@wdio/utils` builds it as + * `{ attempts: 0, limit: repeatTest }` and `@wdio/mocha-framework` never feeds `mochaOpts.retries` + * into it, so under mocha-level retries it stays `{0, 0}` and tells us nothing. Read mocha's own + * runnable state for that case, otherwise the cascade fires on the first attempt and reports + * tests as skipped that the retry then actually runs. + */ + private hasRetryPending(test: Frameworks.Test, results: Frameworks.TestResult): boolean { + const mochaTest = test.ctx?.test as { currentRetry?: () => number, retries?: () => number } | undefined + if (typeof mochaTest?.currentRetry === 'function' && typeof mochaTest.retries === 'function') { + if (mochaTest.currentRetry() < mochaTest.retries()) { + return true + } + } + return Boolean(results.retries && results.retries.attempts < results.retries.limit) + } + + /** + * mocha's `bail` aborts the run on the first failure, so every test the spec had not reached + * yet is dropped without emitting any event and never appears on the dashboard. Report them + * as skipped — same cascade the failed-hook path uses, from the spec's root suite so sibling + * describes are covered too (bail kills the whole spec, not just the failing describe). + * + * The root can span more than one file when specs are grouped — `MochaAdapter` adds every spec + * it is handed to one mocha instance. Cascading across them is still correct: bail aborts that + * whole runner, so those tests do not run either. + */ + private async reportBailSkippedTests(test: Frameworks.Test, results: Frameworks.TestResult) { + if (!this._mochaBail || results.passed || results.skipped) { + return + } + try { + // inside the boundary: hasRetryPending reaches into mocha's own runnable, which this + // SDK does not own + if (this.hasRetryPending(test, results)) { + return + } + const framework = BrowserstackCLI.getInstance().getTestFramework() + let suite = test.ctx?.test?.parent + if (!framework || !suite) { + return + } + while (suite.parent) { + suite = suite.parent + } + await reportSuiteSkipped(framework, suite) + } catch (err) { + BStackLogger.debug(`Failed reporting bail-skipped tests: ${err}`) + } + } + @PerformanceTester.Measure(PERFORMANCE_SDK_EVENTS.EVENTS.SDK_HOOK, { hookType: 'after' }) async after (result: number) { try { diff --git a/packages/browserstack-service/tests/service.test.ts b/packages/browserstack-service/tests/service.test.ts index 95a2e32..32bed6e 100644 --- a/packages/browserstack-service/tests/service.test.ts +++ b/packages/browserstack-service/tests/service.test.ts @@ -2427,3 +2427,157 @@ describe('_isAppAutomate honors skipAppOverride', () => { expect(svc._isAppAutomate()).toBe(false) }) }) + +describe('afterTest bail skip cascade (SDK-7063)', () => { + let getInstanceSpy: ReturnType + + // reportSkippedTest de-dupes on `${parent} - ${title}` in a module-scope Set that outlives + // each test, so every case here needs its own titles. + const buildTree = (tag: string) => { + const root: any = { title: '', tests: [], suites: [], parent: undefined } + const suiteA: any = { title: `${tag} Suite A`, tests: [], suites: [], parent: root } + const suiteB: any = { title: `${tag} Suite B`, tests: [], suites: [], parent: root } + root.suites.push(suiteA, suiteB) + + const ran: any = { title: `${tag} A1`, state: 'passed', parent: suiteA, file: '/spec/a.js' } + const failing: any = { title: `${tag} A2`, state: 'failed', parent: suiteA, file: '/spec/a.js' } + const dropped: any = { title: `${tag} A3`, parent: suiteA, file: '/spec/a.js' } + suiteA.tests.push(ran, failing, dropped) + // sibling top-level describe — only reachable because the cascade walks up to root + suiteB.tests.push({ title: `${tag} B1`, parent: suiteB, file: '/spec/a.js' }) + + failing.ctx = { test: { parent: suiteA } } + return { failing, root } + } + + const makeService = (config: Record) => new BrowserstackService( + { testObservability: false } as any, + [] as any, + { user: 'foo', key: 'bar', ...config } as any + ) + + const runAfterTest = async (svc: BrowserstackService, failing: any, results: Record) => { + const trackEvent = vi.fn().mockResolvedValue(undefined) + getInstanceSpy = vi.spyOn(BrowserstackCLI, 'getInstance').mockReturnValue({ + isRunning: () => true, + getTestFramework: () => ({ trackEvent }) + } as any) + await svc.afterTest(failing, undefined as never, results as any) + return trackEvent + } + + // WHICH tests got reported, not just how many events fired — a cascade that swept the wrong + // tests still produces the same call count. Pairs with the count assertions, which catch the + // opposite failure (a test emitted twice). + const skippedTitles = (trackEvent: ReturnType) => [...new Set( + trackEvent.mock.calls + .filter(([, , payload]: any[]) => payload?.result?.skipped === true) + .map(([, , payload]: any[]) => payload.test.title as string) + )].sort() + + afterEach(() => { + getInstanceSpy?.mockRestore() + }) + + it('reports un-run tests across sibling describes when mocha bail is on', async () => { + const { failing } = buildTree('bail1') + const svc = makeService({ framework: 'mocha', mochaOpts: { bail: true } }) + const trackEvent = await runAfterTest(svc, failing, { passed: false }) + + // 2 events close the failing test (LOG_REPORT/POST + TEST/POST), then 4 per skipped test. + // A3 (same describe) and B1 (SIBLING describe) => 2 skipped => 8. + expect(trackEvent).toHaveBeenCalledTimes(2 + 8) + // exactly the un-run tests: A1 already passed and A2 is the failure being reported, + // so sweeping either of them in would be a defect the count alone cannot see + expect(skippedTitles(trackEvent)).toEqual(['bail1 A3', 'bail1 B1']) + }) + + it('does not cascade when only wdio-level bail is set', async () => { + // wdio's `bail` never halts a spec, so those tests still run — reporting them + // as skipped here would double-report them. + const { failing } = buildTree('bail2') + const svc = makeService({ framework: 'mocha', bail: 1 }) + const trackEvent = await runAfterTest(svc, failing, { passed: false }) + + expect(trackEvent).toHaveBeenCalledTimes(2) + expect(skippedTitles(trackEvent)).toEqual([]) + }) + + it('does not cascade while a wdio spec-file retry is still queued', async () => { + const { failing } = buildTree('bail3') + const svc = makeService({ framework: 'mocha', mochaOpts: { bail: true } }) + const trackEvent = await runAfterTest(svc, failing, { + passed: false, + retries: { attempts: 0, limit: 2 } + }) + + expect(trackEvent).toHaveBeenCalledTimes(2) + expect(skippedTitles(trackEvent)).toEqual([]) + }) + + it('does not cascade while a MOCHA-level retry is still queued', async () => { + // wdio's `results.retries` only tracks spec-file retries — @wdio/mocha-framework never + // feeds mochaOpts.retries into it, so it reads {0,0} here and cannot be relied on. + // Without reading mocha's own runnable state the cascade fires on attempt 1 and reports + // tests as skipped that the retry then actually runs. + const { failing } = buildTree('bail5') + failing.ctx.test.currentRetry = () => 0 + failing.ctx.test.retries = () => 1 + const svc = makeService({ framework: 'mocha', mochaOpts: { bail: true, retries: 1 } }) + const trackEvent = await runAfterTest(svc, failing, { + passed: false, + retries: { attempts: 0, limit: 0 } + }) + + expect(trackEvent).toHaveBeenCalledTimes(2) + expect(skippedTitles(trackEvent)).toEqual([]) + }) + + it('cascades once the final mocha retry has been used', async () => { + const { failing } = buildTree('bail6') + failing.ctx.test.currentRetry = () => 1 + failing.ctx.test.retries = () => 1 + const svc = makeService({ framework: 'mocha', mochaOpts: { bail: true, retries: 1 } }) + const trackEvent = await runAfterTest(svc, failing, { + passed: false, + retries: { attempts: 0, limit: 0 } + }) + + expect(trackEvent).toHaveBeenCalledTimes(2 + 8) + expect(skippedTitles(trackEvent)).toEqual(['bail6 A3', 'bail6 B1']) + }) + + it('does not cascade when the test passed', async () => { + const { failing } = buildTree('bail4') + const svc = makeService({ framework: 'mocha', mochaOpts: { bail: true } }) + const trackEvent = await runAfterTest(svc, failing, { passed: true }) + + expect(trackEvent).toHaveBeenCalledTimes(2) + expect(skippedTitles(trackEvent)).toEqual([]) + }) + + it('does not cascade when the test was itself skipped', async () => { + // a skipped test does not abort the spec, and the pre-existing skip paths already + // report it — cascading here would double-report the rest of the suite + const { failing } = buildTree('bail7') + const svc = makeService({ framework: 'mocha', mochaOpts: { bail: true } }) + const trackEvent = await runAfterTest(svc, failing, { passed: false, skipped: true }) + + // A2 is the test being reported and its own result is legitimately `skipped`; what must + // NOT appear is A3/B1, which the cascade would have added. + expect(trackEvent).toHaveBeenCalledTimes(2) + expect(skippedTitles(trackEvent)).toEqual(['bail7 A2']) + }) + + it('never throws out of afterTest when mocha state is hostile', async () => { + // afterTest is awaited by wdio; anything escaping this cascade would surface as a + // framework-level error in the user's run + const { failing } = buildTree('bail8') + failing.ctx.test.currentRetry = () => { throw new Error('mocha exploded') } + failing.ctx.test.retries = () => 1 + const svc = makeService({ framework: 'mocha', mochaOpts: { bail: true } }) + + const trackEvent = await runAfterTest(svc, failing, { passed: false }) + expect(skippedTitles(trackEvent)).toEqual([]) + }) +})