Bail option to skip all subsequent tests

[jgoux]

Clear and concise description of the problem

Depending on the kind of tests we run, it can be interesting to have a fail-fast feature and exit as soon as a test is failing.

I'm not the only one needing such feature apparently: #964

Jest has bail for this purpose.

Suggested solution

As Vitest tries to smooth the transition from Jest, I think the bail option is a sensible name.

But Jest implementation is incomplete/weird, it's only skipping the next test suites (describe blocks), but it continues to run the other tests within the same test suite.

I think the default should be to bail by test case and not by test suite.

# Skip all the subsequent tests after n test failures (default 1)
--bail <n>

I'm not sure what could be the API if we want to deal with both test cases and test suites, maybe adding a --bail-scope <test|suite> (default test)? 🤔

Alternative

No response

Additional context

Here are related issues about it:

• Ability to skip rest test cases if one fails jestjs/jest#8387
• Jest should fail fast and exit early (change request for --bail) jestjs/jest#6527
• Bug: --bail flag does not bail jestjs/jest#2867

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that request the same feature to avoid creating a duplicate.

[sheremet-va]

Can you explain how you see it? For example, what should happen in those situations:

1. test failed without describe

test('test 1', () => {/* code that fails */})
test('test 2', () => {/* other test */})

2. test failed inside describe

describe('describe 1', () => {
  test('test 1', () => {/* code that fails */})
  test('test 2', () => {/* other test */})
})

describe('describe 2', () => {
  test('test 1', () => {/* other test */})
  test('test 2', () => {/* other test */})
})

3. test failed inside nested describe

describe('describe 1', () => {
  describe('describe 2', () => {
    test('test 1', () => {/* code that fails */})
    test('test 2', () => {/* other test */})
  })

  test('test 1', () => {/* other test */})
  test('test 2', () => {/* other test */})
})

4. test fails in one spec

// spec1.spec.ts
test('test 1', () => {/* code that fails */})
// spec2.spec.ts
test('test 2', () => {/* other test */})

[jgoux]

1. test failed without describe

test('test 1', () => {/* code that fails */})
test('test 2', () => {/* other test */})

skip test 2 in both test and suite scopes.

2. test failed inside describe

describe('describe 1', () => {
  test('test 1', () => {/* code that fails */})
  test('test 2', () => {/* other test */})
})

describe('describe 2', () => {
  test('test 1', () => {/* other test */})
  test('test 2', () => {/* other test */})
})

skip all the tests in test scope
skip only "describe 1"."test 2" in suite scope

3. test failed inside nested describe

describe('describe 1', () => {
  describe('describe 2', () => {
    test('test 1', () => {/* code that fails */})
    test('test 2', () => {/* other test */})
  })

  test('test 1', () => {/* other test */})
  test('test 2', () => {/* other test */})
})

skip all the tests in test scope
skip only "describe 2"."test 2" in suite scope

4. test fails in one spec

// spec1.spec.ts
test('test 1', () => {/* code that fails */})
// spec2.spec.ts
test('test 2', () => {/* other test */})

skip all the tests in test scope
skip only other tests in spec1.spec.ts in suite scope

I think the rule should be:

• using --bail-scope test, you see the whole tests as a flatten structure, if one test fail, no matter where it fails (in another file, in a describe, whatever) we skip everything else and display the report.
• using --bail-scope suite, you skip the subsequent tests in the same "unit" as the failed test. Keep in mind that this "unit" can be logical, if you have 2 describe blocks named the same in two different test files, they are the same "unit". Also, it's more predictable to stop at the closest "unit", if you have nested describe, I don't think you want to skip the tests in the parent describe.

[sheremet-va]

Thank you for your clarification. I have some notes tho.

if you have 2 describe blocks named the same in two different test files, they are the same "unit"

This wouldn't be possible. One spec file knows nothing about the other, they are isolated (as they should be). And it doesn't really makes sense that test in one file can affect test in another.

I also don't like the name "test" for the scope. I personally get confused associating scope test with all tests, maybe we can choose a better name?

Also, should we have some escape hatch for specifying scope suite for some files, like describe.bail? Or that would be an overkill?

[jgoux]

This wouldn't be possible. One spec file knows nothing about the other, they are isolated (as they should be). And it doesn't really makes sense that test in one file can affect test in another.

I thought it was something possible with Jest, as you can nest describe blocks, it can make sense to have big groups to organize your tests (they don't really affect each other, it's more of an organization thing).

I also don't like the name "test" for the scope. I personally get confused associating scope test with all tests, maybe we can choose a better name?

Yes maybe test|suite is too vague, how about single|group?

Also, should we have some escape hatch for specifying scope suite for some files, like describe.bail? Or that would be an overkill?

It's a good idea but I would wait a bit for the initial implementation first and have more feedbacks about it.

[sheremet-va]

I thought it was something possible with Jest, as you can nest describe blocks, it can make sense to have big groups to organize your tests (they don't really affect each other, it's more of an organization thing).

As long as they are in the same file, it should work. Spec files don't communicate with each other.

Yes maybe test|suite is too vague, how about single|group?

single|group is more vague :D

I am ok with suite, but not sure about test, since in my head test is a smallest block of this hierarchy.

As far as I understand, test scope fails the whole tests run?

[jtuchel]

May I ask if there is any news?

As already mentioned Jest provides a --bail flag
And Playwright provides a --max-failures -x flag

It would save a lot of time in CI pipelines :)

[vladcosorg]

I'm integrating vitest into the lint-staged pipeline and replacing jest. Would be cool to bail early if there is any failed test.

[prafed]

Any update on this? I am quite happy with vitest but it can add up a lot of cost for CI pipelines if you get an error in the first minute of execution but subsequent tests keep on running for another 15 minutes.

[sheremet-va]

Any update on this? I am quite happy with vitest but it can add up a lot of cost for CI pipelines if you get an error in the first minute of execution but subsequent tests keep on running for another 15 minutes.

PR welcome.

[trivikr]

Is there any specific place folks can look at to introduce --bail option?

For example, here is a function which marks failure for a task in a runner:

vitest/packages/runner/src/run.ts

Lines 198 to 204 in 895103a

	function failTask(result: TaskResult, err: unknown) {
	result.state = 'fail'
	const error = processError(err)
	result.error = error
	result.errors ??= []
	result.errors.push(error)
	}

[trivikr]

I think vitest runner needs to check value of suite.result.state after it's run, and break the for loop with failure if bail option is set.

Feel free to take up this task, once maintainers confirm the solution posted above is the right way to go ahead or provide alternative suggestions.

I'll revisit this issue after a month or so, and can take up if it's not implemented by then.

[trivikr]

I think the check needs to be done in runTest function:

• Track number of failed tests in a global variable (say numberOfFailedTests), if bail option is set.
• Increment numberOfFailedTests whenever a test fails. It can be done whenever failTask is called on test.result. A better alternative would be to create a new function which does this work - say failTest which does this work and calls failTask

  • vitest/packages/runner/src/run.ts

    Lines 152 to 162 in 895103a

    	catch (e) {
    	failTask(test.result, e)
    	}
    	try {
    	await callSuiteHook(test.suite, test, 'afterEach', runner, [test.context, test.suite])
    	await callCleanupHooks(beforeEachCleanups)
    	}
    	catch (e) {
    	failTask(test.result, e)
    	}

• Skip running rest of the tests/suites if numberOfFailedTests is equal to or greater than one allowed by bail configuration.
  • return from runTest

    vitest/packages/runner/src/run.ts

    Line 107 in 895103a

    export async function runTest(test: Test, runner: VitestRunner) {

  • break from runFiles

    vitest/packages/runner/src/run.ts

    Lines 316 to 317 in 895103a

    	for (const file of files) {
    	if (!file.tasks.length && !runner.config.passWithNoTests) {

If you're interested to take up this work, do wait for a comment from maintainers to check if there are better alternatives.

[sheremet-va]

If you're interested to take up this work, do wait for a comment from maintainers to check if there are better alternatives.

I don't know. Probably right. Be aware that files will only have one file if it's running with --threads:

vitest/packages/runner/src/run.ts

Lines 316 to 317 in 895103a

	for (const file of files) {
	if (!file.tasks.length && !runner.config.passWithNoTests) {

The hardest part here is to tell Vitest to stop running all other workers from within the worker without breaking Tinypool.

[trivikr]

The hardest part here is to tell Vitest to stop running all other workers from within the worker without breaking Tinypool.

Can we use SharedArrayBuffer to store numberOfFailedTests so that it can be read and updated across workers?

The blog post Using Shared Array Buffer in Node.js provides an example of how SharedArrayBuffer can be shared between workers.

I see that birpc is used for communication between workers?

vitest/packages/vitest/src/node/pools/threads.ts

Lines 23 to 33 in 895103a

	createBirpc<{}, RuntimeRPC>(
	createMethodsRPC(ctx),
	{
	post(v) {
	port.postMessage(v)
	},
	on(fn) {
	port.on('message', fn)
	},
	},
	)

Can this store numberOfFailedTests value, or some support needs to be added in tinypool?

cc Tinypool author @Aslemammad for comments.

[sheremet-va]

Can we use SharedArrayBuffer to store numberOfFailedTests so that it can be read and updated across workers?

I think it's for use in worker_threads, but we also support child_process.

[trivikr]

but we also support child_process.

The numberOfFailedTests can be tracked by sending messages between parent and child using subprocess.send.

[trivikr]

It looks like the messages can be exchanged between worker threads or child processes using RuntimeRPC interfaced passed in createBirpc

• Interface

  vitest/packages/vitest/src/types/rpc.ts

  Line 9 in 895103a

  export interface RuntimeRPC {

• child.ts

  vitest/packages/vitest/src/node/pools/child.ts

  Line 19 in 895103a

  createBirpc<{}, RuntimeRPC>(

• vitest/packages/vitest/src/node/pools/threads.ts

  Line 23 in 895103a

  createBirpc<{}, RuntimeRPC>(

[trivikr]

After diving deep into the source code, I think the numberOfFailedTests can be tracked in Vitest core.

High level suggestion:

• The variable can be similar to restartsCount (say failedTestsCount)

  vitest/packages/vitest/src/node/core.ts

  Line 47 in bce5a9f

  restartsCount = 0

• A function can be added (say onTestFailed) which tracks the failed tests count, and calls .exit(true) or equivalent when the number goes over what's configured by bail.
• The interface RuntimeRPC provides a function to call core function.
• The RuntimeRPC function is called from failTest in runner.

If you're interested to take up this work, do wait for a comment from maintainers to check if there are better alternatives.

[AriPerkkio]

I think this and #3128 could share the mechanic of how test running is stopped. Maybe there should be a way to tell all pools (and their test runners) to stop their execution gracefully. And when a test fails with --bail option, or in #3128's case user manually attempts to pause the execution in watch mode, this signal would be sent to pools.

• When a pool receives this signal it should not start a new test file. It should let the current test file finish properly so that all afterEach cleanups etc are run.
• When a runner receives this signal it should not start a new test case. It should let the current test case finish. Maybe every test/it should check whether an abort signal has been sent earlier before starting up.