From b24585f08f2ea267746a2d6ca0e43edcbb29726f Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Tue, 29 Sep 2026 16:20:58 +0900 Subject: [PATCH] fix: don't retry when `test.fails` expectedly failed (#11219) Co-authored-by: Hiroshi Ogawa <4232207+hi-ogawa@users.noreply.github.com> Co-authored-by: Codex (GPT-6) --- packages/vitest/src/runtime/runner/run.ts | 30 ++-- test/e2e/test/repeats.test.ts | 33 +++++ test/e2e/test/retry.test.ts | 168 +++++++++++++++++++++- test/unit/test/on-finished.test.ts | 12 +- test/unit/test/repeats.test.ts | 2 +- test/unit/test/retry.test.ts | 2 +- 6 files changed, 223 insertions(+), 24 deletions(-) diff --git a/packages/vitest/src/runtime/runner/run.ts b/packages/vitest/src/runtime/runner/run.ts index 3fcf0910f..6e8299535 100644 --- a/packages/vitest/src/runtime/runner/run.ts +++ b/packages/vitest/src/runtime/runner/run.ts @@ -602,6 +602,7 @@ async function runTest(test: Test, runner: VitestRunner): Promise { test.result.state = 'run' as TaskState const retry = getRetryCount(test.retry) for (let retryCount = 0; retryCount <= retry; retryCount++) { + const attemptErrorsStart = test.result.errors?.length ?? 0 let beforeEachCleanups: unknown[] = [] // fixtureCheckpoint is passed by callAroundEachHooks - it represents the count // of fixture cleanup functions AFTER all aroundEach fixtures have been resolved @@ -706,6 +707,23 @@ async function runTest(test: Test, runner: VitestRunner): Promise { return } + // if test is marked to be failed, flip the result unless `TestSyntaxError` is present + if (test.fails) { + if (test.result.state === 'pass') { + const error = processError(new Error('Expect test to fail')) + test.result.state = 'fail' + test.result.errors ??= [] + test.result.errors.push(error) + } else if ( + !test.result.errors?.slice(attemptErrorsStart).some((e) => e.__vitest_test_syntax_error__) + ) { + test.result.state = 'pass' + test.result.errors?.splice(attemptErrorsStart) + if (!test.result.errors?.length) { + test.result.errors = undefined + } + } + } if (test.result.state === 'pass') { break } @@ -739,18 +757,6 @@ async function runTest(test: Test, runner: VitestRunner): Promise { test.result.state = 'fail' } - // if test is marked to be failed, flip the result unless `TestSyntaxError` is present - if (test.fails) { - if (test.result.state === 'pass') { - const error = processError(new Error('Expect test to fail')) - test.result.state = 'fail' - test.result.errors = [error] - } else if (!test.result.errors?.some((e) => e.__vitest_test_syntax_error__)) { - test.result.state = 'pass' - test.result.errors = undefined - } - } - cleanupRunningTest() setCurrentTest(undefined) diff --git a/test/e2e/test/repeats.test.ts b/test/e2e/test/repeats.test.ts index 297916168..ab2593bb3 100644 --- a/test/e2e/test/repeats.test.ts +++ b/test/e2e/test/repeats.test.ts @@ -134,3 +134,36 @@ test('onTestFailed runs only for failed repeats', async () => { } `) }) +test('expected failures are evaluated for each repeat', async () => { + const { errorTree } = await runInlineTests({ + 'repeats.test.js': ` + import { afterAll, expect, it } from 'vitest' + + const runs = [0, 0, 0, 0, 0] + + it.fails('alternates passing and failing repeats', { repeats: 4 }, ({ task }) => { + const repeatCount = task.result.repeatCount + runs[repeatCount]++ + if (repeatCount % 2 === 1) { + throw new Error('repeat ' + repeatCount + ' failed') + } + }) + + afterAll(() => { + expect(runs).toEqual([1, 1, 1, 1, 1]) + }) + `, + }) + + expect(errorTree()).toMatchInlineSnapshot(` + { + "repeats.test.js": { + "alternates passing and failing repeats": [ + "Expect test to fail", + "Expect test to fail", + "Expect test to fail", + ], + }, + } + `) +}) diff --git a/test/e2e/test/retry.test.ts b/test/e2e/test/retry.test.ts index 7fbfc8274..17a29fc44 100644 --- a/test/e2e/test/retry.test.ts +++ b/test/e2e/test/retry.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from 'vitest' -import { runVitest } from '../../test-utils' +import { runInlineTests, runVitest } from '../../test-utils' function run(testNamePattern: string) { return runVitest({ @@ -25,3 +25,169 @@ describe('retry', () => { expect(stdout).toContain('1 failed') }) }) + +test('expected failures stop retrying after a failed assertion', async () => { + const { stderr, errorTree } = await runInlineTests({ + 'fails.test.js': ` + import { afterAll, expect, it } from 'vitest' + + const runs = { + immediate: 0, + passesThenFails: 0, + repeats: [0, 0, 0], + } + + it.fails('fails immediately', { retry: 2 }, () => { + runs.immediate++ + expect(1).toBe(2) + }) + + it.fails('passes then fails', { retry: 2 }, () => { + runs.passesThenFails++ + expect(runs.passesThenFails).toBe(1) + }) + + it.fails('repeats', { retry: 2, repeats: 2 }, ({ task }) => { + runs.repeats[task.result.repeatCount]++ + expect(1).toBe(2) + }) + + afterAll(() => { + expect(runs).toEqual({ + immediate: 1, + passesThenFails: 2, + repeats: [1, 1, 1], + }) + }) + `, + }) + + expect(stderr).toBe('') + expect(errorTree()).toMatchInlineSnapshot(` + { + "fails.test.js": { + "fails immediately": "passed", + "passes then fails": "passed", + "repeats": "passed", + }, + } + `) +}) + +test('expected failures exhaust retries in every repeat when assertions pass', async () => { + const { errorTree } = await runInlineTests({ + 'fails.test.js': ` + import { afterAll, expect, it } from 'vitest' + + const runs = [[0], [0, 0], [0, 0, 0]] + + for (const repeats of [0, 1, 2]) { + it.fails('unexpected pass with ' + repeats + ' repeats', { retry: 2, repeats }, ({ task }) => { + runs[repeats][task.result.repeatCount]++ + expect(1).toBe(1) + }) + } + + afterAll(() => { + expect(runs).toEqual([ + [3], + [3, 3], + [3, 3, 3], + ]) + }) + `, + }) + + expect(errorTree()).toMatchInlineSnapshot(` + { + "fails.test.js": { + "unexpected pass with 0 repeats": [ + "Expect test to fail", + "Expect test to fail", + "Expect test to fail", + ], + "unexpected pass with 1 repeats": [ + "Expect test to fail", + "Expect test to fail", + "Expect test to fail", + "Expect test to fail", + "Expect test to fail", + "Expect test to fail", + ], + "unexpected pass with 2 repeats": [ + "Expect test to fail", + "Expect test to fail", + "Expect test to fail", + "Expect test to fail", + "Expect test to fail", + "Expect test to fail", + "Expect test to fail", + "Expect test to fail", + "Expect test to fail", + ], + }, + } + `) +}) + +test('expected failures can recover through a retry in every repeat', async () => { + const { stderr, errorTree } = await runInlineTests({ + 'repeats.test.js': ` + import { afterAll, expect, it } from 'vitest' + + const attempts = [0, 0, 0] + it.fails('recovers', { repeats: 2, retry: 2 }, ({ task }) => { + const attempt = attempts[task.result.repeatCount]++ + if (attempt > 0) { + throw new Error('attempt failed') + } + }) + + afterAll(() => { + expect(attempts).toEqual([2, 2, 2]) + }) + `, + }) + + expect(stderr).toBe('') + expect(errorTree()).toMatchInlineSnapshot(` + { + "repeats.test.js": { + "recovers": "passed", + }, + } + `) +}) + +test('syntax errors remain failures after successful repeats', async () => { + const { errorTree } = await runInlineTests({ + 'repeats.test.js': ` + import { afterAll, expect, it } from 'vitest' + + const runs = [0, 0] + + it.fails('syntax error', { repeats: 1, retry: 1 }, ({ task }) => { + runs[task.result.repeatCount]++ + if (task.result.repeatCount === 0) { + expect(1).toMatchInlineSnapshot('1') + } + expect(1).toBe(2) + }) + + afterAll(() => { + expect(runs).toEqual([2, 1]) + }) + `, + }) + + expect(errorTree()).toMatchInlineSnapshot(` + { + "repeats.test.js": { + "syntax error": [ + "'toMatchInlineSnapshot' cannot be used with 'test.fails'", + "'toMatchInlineSnapshot' cannot be used with 'test.fails'", + ], + }, + } + `) +}) diff --git a/test/unit/test/on-finished.test.ts b/test/unit/test/on-finished.test.ts index 99c1e88cc..f105cf8a0 100644 --- a/test/unit/test/on-finished.test.ts +++ b/test/unit/test/on-finished.test.ts @@ -105,9 +105,7 @@ describe('repeats fail', () => { state.push(`${tag}fail`) }) - if (t.task.result?.repeatCount === 1) { - throw new Error('fail') - } + throw new Error('fail') }) it('assert', () => { @@ -115,11 +113,13 @@ describe('repeats fail', () => { [ "(0, 0) run", "(0, 0) finish", + "(0, 0) fail", "(0, 1) run", "(0, 1) finish", "(0, 1) fail", "(0, 2) run", "(0, 2) finish", + "(0, 2) fail", ] `) }) @@ -186,12 +186,6 @@ describe('retry fail', () => { "(0, 0) run", "(0, 0) finish", "(0, 0) fail", - "(1, 0) run", - "(1, 0) finish", - "(1, 0) fail", - "(2, 0) run", - "(2, 0) finish", - "(2, 0) fail", ] `) }) diff --git a/test/unit/test/repeats.test.ts b/test/unit/test/repeats.test.ts index 14f44e6af..16a9ef497 100644 --- a/test/unit/test/repeats.test.ts +++ b/test/unit/test/repeats.test.ts @@ -40,7 +40,7 @@ const retryNumbers: number[] = [] describe('testing repeats with retry', () => { describe('normal test', () => { - const result = [1, 1, 1, 1, 1, 1, 1, 1, 1, 1] + const result = [1, 1, 1, 1, 1] test.fails('test 1', { repeats: 4, retry: 1 }, () => { retryNumbers.push(1) expect(1).toBe(2) diff --git a/test/unit/test/retry.test.ts b/test/unit/test/retry.test.ts index dde98ac82..e5b0165c2 100644 --- a/test/unit/test/retry.test.ts +++ b/test/unit/test/retry.test.ts @@ -20,7 +20,7 @@ it('retry test fails', { retry: 10 }, () => { it('result', () => { expect(count1).toEqual(3) - expect(count2).toEqual(2) + expect(count2).toEqual(1) expect(count3).toEqual(3) }) -- 2.51.2