Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/pr-122.md
Original file line number Diff line number Diff line change
@@ -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.
8 changes: 8 additions & 0 deletions packages/browserstack-service/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
67 changes: 67 additions & 0 deletions packages/browserstack-service/src/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
isBrowserstackSession,
patchConsoleLogs,
isTrue,
isFalse,
getUniqueIdentifier,
getHookType,
isBrowserstackExecutorScript
Expand Down Expand Up @@ -88,6 +89,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,
Expand All @@ -105,6 +113,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('')) {
Expand Down Expand Up @@ -560,6 +572,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
}

Expand All @@ -568,6 +581,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 {
Expand Down
154 changes: 154 additions & 0 deletions packages/browserstack-service/tests/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2558,3 +2558,157 @@ describe('_isAppAutomate honors skipAppOverride', () => {
expect(svc._isAppAutomate()).toBe(false)
})
})

describe('afterTest bail skip cascade (SDK-7063)', () => {
let getInstanceSpy: ReturnType<typeof vi.spyOn>

// 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<string, unknown>) => new BrowserstackService(
{ testObservability: false } as any,
[] as any,
{ user: 'foo', key: 'bar', ...config } as any
)

const runAfterTest = async (svc: BrowserstackService, failing: any, results: Record<string, unknown>) => {
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<typeof vi.fn>) => [...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([])
})
})
Loading