Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): test change should trigger failed test rerun #3773

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

fix(core): test change should trigger failed test rerun #3773

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
43d9d1c
fix(core): test change should trigger failed test rerun
btea Jul 15, 2023
1596d97
fix: lint
btea Jul 15, 2023
67617fd
fix: add isFailedModel & test
btea Jul 17, 2023
0d017c1
chore: fix lint
btea Jul 17, 2023
9419398
Merge branch 'main' into fix/test-change-trigger-rerun-fail
btea Jul 2, 2024
f15d3b4
chore: fix lint
btea Jul 2, 2024
df0d771
chore: fix lint
btea Jul 2, 2024
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
12 changes: 10 additions & 2 deletions packages/vitest/src/node/core.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export class Vitest {
coverageProvider: CoverageProvider | null | undefined
logger: Logger
pool: ProcessPool | undefined
isFailedModel = false

vitenode: ViteNodeServer = undefined!

Expand Down Expand Up @@ -823,16 +824,23 @@ export class Vitest {

this.snapshot.clear()
let files = Array.from(this.changedTests)

const failedTest = this.state.getFailedFilepaths()
if (this.filenamePattern) {
const filteredFiles = await this.globTestFiles([this.filenamePattern])
files = files.filter(file => filteredFiles.some(f => f[1] === file))

if (failedTest.length && this.isFailedModel) {
files = [...new Set(files.concat(failedTest))]
}
// A file that does not match the current filename pattern was changed
if (files.length === 0) {
return
}
}
else {
if (failedTest.length && this.isFailedModel) {
files = [...new Set(files.concat(failedTest))]
}
}

this.changedTests.clear()

Expand Down
7 changes: 7 additions & 0 deletions packages/vitest/src/node/stdin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ export function registerConsoleShortcuts(
}
// rerun all tests
if (name === 'a' || name === 'return') {
ctx.isFailedModel = false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should set properties of ctx outside it like this. Could this filter be toggled on/off inside the core.ts instead?

But your approach is right in the way that we indeed need to save the failure filter in state.

return ctx.changeNamePattern('')
}
// rerun current pattern tests
if (name === 'r') {
ctx.isFailedModel = false
const files = await ctx.getTestFilepaths()
return ctx.changeNamePattern('', files, 'rerun all tests')
}
Expand All @@ -104,6 +110,7 @@ export function registerConsoleShortcuts(
}
// rerun only failed tests
if (name === 'f') {
ctx.isFailedModel = true
return ctx.rerunFailed()
}
// change project filter
Expand Down
31 changes: 31 additions & 0 deletions test/watch/test/stdin.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,37 @@ test('2 - test that is cancelled', async () => {
})
})

test('rerun failed tests', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need a test case that fails in main, and is fixed by these changes. This test case does not fail on main branch.

Maybe something like below?

test('rerun failed tests', async () => {
  const testPath = 'fixtures/error.test.ts'
  const failAssertion = 'expect(1).toBe(2)'
  const testCase = `// Dynamic test case
  import { test, expect } from 'vitest'

  // Failure re-run should run this
  test('failing test', () => {
    ${failAssertion}
  })

  // Failure re-run should NOT run this
  test('passing test', () => {
    expect(1).toBe(1)
  })
  `

  writeFileSync(testPath, testCase, 'utf8')
  cleanups.push(() => rmSync(testPath))

  const vitest = await runVitestCli(...cliArgs)
  await vitest.waitForStdout('Test Files  1 failed | 2 passed (3)')
  await vitest.waitForStdout('Tests  1 failed | 3 passed (4)')

  // Re-run failed tests only
  vitest.write('f')

  // Only a single test should have run
  await vitest.waitForStdout('Test Files  1 failed (1)')

  // Only a single test case should have run
  await vitest.waitForStdout('Tests  1 failed (1)')

  // Fix failing test case
  writeFileSync(testPath, testCase.replace(failAssertion, ''), 'utf8')

  // Only the previously failed test file should run
  await vitest.waitForStdout('Test Files  1 pass (1)')

  // Only the previously failed test case should run
  await vitest.waitForStdout('Tests  1 pass (1)')
})

const { vitest } = await runVitest({ ..._options })
const testPath = 'fixtures/error.test.ts'
const testCase = `
import { test, expect } from 'vitest'
test('fail test', () => {
expect(1).toBe(2)
})
`
const testTwoPath = 'fixtures/normal.test.ts'
const testTwoCase = `
import { test, expect } from 'vitest'
test('normal test', () => {
expect(1).toBe(1)
})
`
writeFileSync(testPath, testCase, 'utf8')
writeFileSync(testTwoPath, testTwoCase, 'utf8')

await vitest.waitForStdout('1 failed')

writeFileSync(testTwoPath, testCase.replace(/1/g, '2'), 'utf8')
await vitest.waitForStdout('1 pass')

vitest.write('f')
await vitest.waitForStdout('1 failed')

writeFileSync(testTwoPath, testCase.replace(/2/g, '3'), 'utf8')
await vitest.waitForStdout('1 failed')
})

test('rerun current pattern tests', async () => {
const { vitest } = await runVitest({ ..._options, testNamePattern: 'sum' })

Expand Down
Loading