From 1371ca6a6fba068b98484ec554200e558c5283ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ari=20Perkki=C3=B6?= Date: Wed, 25 Sep 2024 08:32:43 +0300 Subject: [PATCH] fix(coverage): remove empty coverage folder on test failure too (#6547) --- packages/coverage-istanbul/src/provider.ts | 22 +++++-- packages/coverage-v8/src/provider.ts | 22 +++++-- packages/vitest/src/node/core.ts | 8 ++- packages/vitest/src/node/types/coverage.ts | 3 + .../test/empty-coverage-directory.test.ts | 34 +++++++--- test/coverage-test/test/on-failure.test.ts | 62 +++++++++++++++++++ test/coverage-test/utils.ts | 14 ++++- 7 files changed, 140 insertions(+), 25 deletions(-) create mode 100644 test/coverage-test/test/on-failure.test.ts diff --git a/packages/coverage-istanbul/src/provider.ts b/packages/coverage-istanbul/src/provider.ts index d3a8bd207727..34769995522b 100644 --- a/packages/coverage-istanbul/src/provider.ts +++ b/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 --git a/packages/coverage-v8/src/provider.ts b/packages/coverage-v8/src/provider.ts index 92de2b7f7779..2921c26b6014 100644 --- a/packages/coverage-v8/src/provider.ts +++ b/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 --git a/packages/vitest/src/node/core.ts b/packages/vitest/src/node/core.ts index 88ebd36e0c20..93d0c885e5ce 100644 --- a/packages/vitest/src/node/core.ts +++ b/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 --git a/packages/vitest/src/node/types/coverage.ts b/packages/vitest/src/node/types/coverage.ts index 993ed20cee92..02adf6ad648b 100644 --- a/packages/vitest/src/node/types/coverage.ts +++ b/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 + /** Callback called when test run fails */ + onTestFailure?: () => void | Promise + /** Callback to generate final coverage results */ generateCoverage: ( reportContext: ReportContext diff --git a/test/coverage-test/test/empty-coverage-directory.test.ts b/test/coverage-test/test/empty-coverage-directory.test.ts index 54770047dd1d..61c1ff866f88 100644 --- a/test/coverage-test/test/empty-coverage-directory.test.ts +++ b/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 --git a/test/coverage-test/test/on-failure.test.ts b/test/coverage-test/test/on-failure.test.ts new file mode 100644 index 000000000000..4c6d36f467ae --- /dev/null +++ b/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 --git a/test/coverage-test/utils.ts b/test/coverage-test/utils.ts index 865b7d8d7cb1..63cfbb42f582 100644 --- a/test/coverage-test/utils.ts +++ b/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('')) + } +}