Coverage cleanOnRerun: false causes crash

[asportnoy]

Describe the bug

When setting coverage.cleanOnRerun to false, saving a file will cause the CLI to crash with Error: ENOENT: no such file or directory, open '/path/to/repo/coverage/.tmp/coverage-[num].json'.

Reproduction

https://stackblitz.com/edit/vitest-dev-vitest-7cu7ag?file=vite.config.ts

1. Press "Fork" so you can edit
2. Wait for the coverage to be outputted in the CLI
3. Make any change to src/basic.ts. The CLI will crash with the error above.

If it doesn't crash, just wait for the coverage output again and try making another change.

System Info

System:
    OS: macOS 14.4.1
    CPU: (8) arm64 Apple M1 Pro
    Memory: 177.73 MB / 16.00 GB
    Shell: 3.7.1 - /opt/homebrew/bin/fish
  Binaries:
    Node: 20.11.1 - ~/.local/share/nvm/v20.11.1/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 10.2.4 - ~/.local/share/nvm/v20.11.1/bin/npm
  Browsers:
    Chrome: 123.0.6312.122
    Safari: 17.4.1
  npmPackages:
    @vitejs/plugin-react: ^4.2.1 => 4.2.1 
    @vitest/coverage-istanbul: ^1.4.0 => 1.4.0 
    @vitest/ui: ^1.4.0 => 1.4.0 
    vitest: ^1.4.0 => 1.4.0

Used Package Manager

npm

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[AriPerkkio]

The coverageFilesDirectory is always removed after tests finished but is not re-created when cleanOnRerun is disabled:

vitest/packages/coverage-v8/src/provider.ts

Lines 119 to 122 in 04b234d

	if (existsSync(this.coverageFilesDirectory))
	await fs.rm(this.coverageFilesDirectory, { recursive: true, force: true, maxRetries: 10 })
	await fs.mkdir(this.coverageFilesDirectory, { recursive: true })

[AriPerkkio]

For now we can add a fix that prevents the crash. But I'm starting to think that we should remove whole cleanOnRerun option as it doesn't make much sense. If results of previous run are preserved should we merge the next results with them? That would cause incorrect reports.

[asportnoy]

That was what I was trying to do. In some cases, the coverage outputted only shows the file you most recently changed. I wanted it to preserve the coverage of the other files too.

[AriPerkkio]

Showing coverage results from earlier test runs will be difficult and Vitest is unable to do that reliably. The reports from earlier runs can be completely off if some of those files or their test files were changed in between.

• If new lines were added in the source file, the old results will now point to incorrect lines.
• If a test file is removed, the source files that it was earlier covering are still incorrectly covered.

The cleanOnRerun option will cause bugs and shouldn't be used.

[asportnoy]

If the source file or test is changed, shouldn't it be able to just re-generate the relevant coverage files? It should be able to just leave the coverage records for files that are unrelated to the change.

[AriPerkkio]

Yep, that's exactly how it works. It re-generates the relevant coverage files and leaves the other ones as is. The re-generated parts will be merged with old ones. Lines that should show 1x hits will show 2x and so on.

[asportnoy]

Is there another option that regenerates relevant files but replaces the old results? So it'll still leave results for unchanged files but won't have duplicate results for the changed ones.

[sheremet-va]

I think the only way to achieve that would be for us to store coverage per test file, and then generate the new report by merging all of them every time the test reruns. But this is extremely memory-consuming, I would imagine.

[asportnoy]

Got it. That is what I was expecting to happen with cleanOnRerun: false, so if you were to implement an option for that, that would be a good replacement. I agree that, at least with how it's implemented now, this option should probably be deprecated in the future.

[AriPerkkio]

to store coverage per test file

I guess we could use the files from here

vitest/packages/vitest/src/runtime/runners/index.ts

Lines 70 to 77 in c84113f

	testRunner.onAfterRunFiles = async (files) => {
	const state = getWorkerState()
	const coverage = await takeCoverageInsideWorker(config.coverage, executor)
	rpc().onAfterSuiteRun({
	coverage,
	transformMode: state.environment.transformMode,
	projectName: state.ctx.projectName,
	})

... as cache keys in here

vitest/packages/coverage-v8/src/provider.ts

Lines 133 to 137 in c84113f

	onAfterSuiteRun({ coverage, transformMode, projectName }: AfterSuiteRunMeta) {
	if (transformMode !== 'web' && transformMode !== 'ssr')
	throw new Error(`Invalid transform mode: ${transformMode}`)
	let entry = this.coverageFiles.get(projectName || DEFAULT_PROJECT)

and use those to detect which results to invalidate from memory (this.coverageFiles). 🤔

Without invalidating previous results we'll see this kind of issues. Uncovered branches are shown as covered as previous runs covered them:

coverage-clean-on-rerun.mp4