fix(coverage): remove empty coverage folder on test failure too (#6547)

Author: AriPerkkio

Diff for: packages/coverage-istanbul/src/provider.ts

@@ -307,13 +307,23 @@ export class IstanbulCoverageProvider extends BaseCoverageProvider implements Co
     const keepResults = !this.options.cleanOnRerun && this.ctx.config.watch
 
     if (!keepResults) {
-      this.coverageFiles = new Map()
-      await fs.rm(this.coverageFilesDirectory, { recursive: true })
+      await this.cleanAfterRun()
+    }
+  }
 
-      // Remove empty reports directory, e.g. when only text-reporter is used
-      if (readdirSync(this.options.reportsDirectory).length === 0) {
-        await fs.rm(this.options.reportsDirectory, { recursive: true })
-      }
+  private async cleanAfterRun() {
+    this.coverageFiles = new Map()
+    await fs.rm(this.coverageFilesDirectory, { recursive: true })
+
+    // Remove empty reports directory, e.g. when only text-reporter is used
+    if (readdirSync(this.options.reportsDirectory).length === 0) {
+      await fs.rm(this.options.reportsDirectory, { recursive: true })
+    }
+  }
+
+  async onTestFailure() {
+    if (!this.options.reportOnFailure) {
+      await this.cleanAfterRun()
     }
   }
 

Diff for: packages/coverage-v8/src/provider.ts

@@ -279,13 +279,23 @@ export class V8CoverageProvider extends BaseCoverageProvider implements Coverage
     const keepResults = !this.options.cleanOnRerun && this.ctx.config.watch
 
     if (!keepResults) {
-      this.coverageFiles = new Map()
-      await fs.rm(this.coverageFilesDirectory, { recursive: true })
+      await this.cleanAfterRun()
+    }
+  }
 
-      // Remove empty reports directory, e.g. when only text-reporter is used
-      if (readdirSync(this.options.reportsDirectory).length === 0) {
-        await fs.rm(this.options.reportsDirectory, { recursive: true })
-      }
+  private async cleanAfterRun() {
+    this.coverageFiles = new Map()
+    await fs.rm(this.coverageFilesDirectory, { recursive: true })
+
+    // Remove empty reports directory, e.g. when only text-reporter is used
+    if (readdirSync(this.options.reportsDirectory).length === 0) {
+      await fs.rm(this.options.reportsDirectory, { recursive: true })
+    }
+  }
+
+  async onTestFailure() {
+    if (!this.options.reportOnFailure) {
+      await this.cleanAfterRun()
     }
   }
 

Diff for: packages/vitest/src/node/core.ts

@@ -974,8 +974,12 @@ export class Vitest {
   }
 
   private async reportCoverage(coverage: unknown, allTestsRun: boolean) {
-    if (!this.config.coverage.reportOnFailure && this.state.getCountOfFailedTests() > 0) {
-      return
+    if (this.state.getCountOfFailedTests() > 0) {
+      await this.coverageProvider?.onTestFailure?.()
+
+      if (!this.config.coverage.reportOnFailure) {
+        return
+      }
     }
 
     if (this.coverageProvider) {

Diff for: packages/vitest/src/node/types/coverage.ts

@@ -26,6 +26,9 @@ export interface CoverageProvider {
   /** Called with coverage results after a single test file has been run */
   onAfterSuiteRun: (meta: AfterSuiteRunMeta) => void | Promise<void>
 
+  /** Callback called when test run fails */
+  onTestFailure?: () => void | Promise<void>
+
   /** Callback to generate final coverage results */
   generateCoverage: (
     reportContext: ReportContext

Diff for: test/coverage-test/test/empty-coverage-directory.test.ts

@@ -1,23 +1,37 @@
-import { existsSync, readdirSync } from 'node:fs'
-import { expect } from 'vitest'
-import { coverageTest, normalizeURL, runVitest, test } from '../utils'
+import { existsSync } from 'node:fs'
+import { beforeEach, expect } from 'vitest'
+import { captureStdout, coverageTest, normalizeURL, runVitest, test } from '../utils'
 import { sum } from '../fixtures/src/math'
 
+beforeEach(() => {
+  return captureStdout()
+})
+
 test('empty coverage directory is cleaned after tests', async () => {
   await runVitest({
     include: [normalizeURL(import.meta.url)],
+    testNamePattern: 'passing test',
     coverage: { reporter: 'text', all: false },
   })
 
-  if (existsSync('./coverage')) {
-    if (readdirSync('./coverage').length !== 0) {
-      throw new Error('Test case expected coverage directory to be empty')
-    }
+  expect(existsSync('./coverage')).toBe(false)
+})
+
+test('empty coverage directory is cleaned after failing test run', async () => {
+  const { exitCode } = await runVitest({
+    include: [normalizeURL(import.meta.url)],
+    testNamePattern: 'failing test',
+    coverage: { reporter: 'text', all: false },
+  }, { throwOnError: false })
 
-    throw new Error('Empty coverage directory was not cleaned')
-  }
+  expect(existsSync('./coverage')).toBe(false)
+  expect(exitCode).toBe(1)
 })
 
-coverageTest('cover some lines', () => {
+coverageTest('passing test', () => {
   expect(sum(2, 3)).toBe(5)
 })
+
+coverageTest('failing test', () => {
+  expect(sum(2, 3)).toBe(6)
+})

Diff for: test/coverage-test/test/on-failure.test.ts

@@ -0,0 +1,62 @@
+import { expect } from 'vitest'
+import { captureStdout, coverageTest, isV8Provider, normalizeURL, runVitest, test } from '../utils'
+import { sum } from '../fixtures/src/math'
+
+test('report is not generated when tests fail', async () => {
+  const stdout = captureStdout()
+
+  const { exitCode } = await runVitest({
+    include: [normalizeURL(import.meta.url)],
+    coverage: {
+      all: false,
+      include: ['**/fixtures/src/math.ts'],
+      reporter: 'text',
+    },
+  }, { throwOnError: false })
+
+  expect(stdout()).toBe('')
+  expect(exitCode).toBe(1)
+})
+
+test('report is generated when tests fail and { reportOnFailure: true }', async () => {
+  const stdout = captureStdout()
+
+  const { exitCode } = await runVitest({
+    include: [normalizeURL(import.meta.url)],
+    coverage: {
+      all: false,
+      include: ['**/fixtures/src/math.ts'],
+      reporter: 'text',
+      reportOnFailure: true,
+    },
+  }, { throwOnError: false })
+
+  if (isV8Provider()) {
+    expect(stdout()).toMatchInlineSnapshot(`
+      "----------|---------|----------|---------|---------|-------------------
+      File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
+      ----------|---------|----------|---------|---------|-------------------
+      All files |      50 |      100 |      25 |      50 |                   
+       math.ts  |      50 |      100 |      25 |      50 | 6-7,10-11,14-15   
+      ----------|---------|----------|---------|---------|-------------------
+      "
+    `)
+  }
+  else {
+    expect(stdout()).toMatchInlineSnapshot(`
+      "----------|---------|----------|---------|---------|-------------------
+      File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
+      ----------|---------|----------|---------|---------|-------------------
+      All files |      25 |      100 |      25 |      25 |                   
+       math.ts  |      25 |      100 |      25 |      25 | 6-14              
+      ----------|---------|----------|---------|---------|-------------------
+      "
+    `)
+  }
+
+  expect(exitCode).toBe(1)
+})
+
+coverageTest('failing test', () => {
+  expect(sum(1, 2)).toBe(4)
+})

Diff for: test/coverage-test/utils.ts

@@ -5,7 +5,8 @@ import { normalize } from 'pathe'
 import libCoverage from 'istanbul-lib-coverage'
 import type { FileCoverageData } from 'istanbul-lib-coverage'
 import type { TestFunction, UserConfig } from 'vitest'
-import { describe as vitestDescribe, test as vitestTest } from 'vitest'
+import { vi, describe as vitestDescribe, test as vitestTest } from 'vitest'
+import stripAnsi from 'strip-ansi'
 import * as testUtils from '../test-utils'
 
 export function test(name: string, fn: TestFunction, skip = false) {
@@ -103,3 +104,14 @@ export function isBrowser() {
 export function normalizeURL(importMetaURL: string) {
   return normalize(fileURLToPath(importMetaURL))
 }
+
+export function captureStdout() {
+  const spy = vi.fn()
+  const original = process.stdout.write
+  process.stdout.write = spy
+
+  return function collect() {
+    process.stdout.write = original
+    return stripAnsi(spy.mock.calls.map(call => call[0]).join(''))
+  }
+}