fix(coverage): fail fast when concurrent runs share coverage.reportsDirectory

packages/vitest/src/node/coverage.ts

@@ -5,8 +5,10 @@ import type { CoverageModuleLoader, CoverageOptions, CoverageProvider, ReportCon
 import type { SerializedCoverageConfig } from '../runtime/config'
 import type { AfterSuiteRunMeta } from '../types/general'
 import type { TestProject } from './project'
+import { createHash } from 'node:crypto'
 import { existsSync, promises as fs, readdirSync, writeFileSync } from 'node:fs'
 import module from 'node:module'
+import { tmpdir } from 'node:os'
 import path from 'node:path'
 import { fileURLToPath } from 'node:url'
 import { cleanUrl, slash } from '@vitest/utils/helpers'
@@ -74,6 +76,17 @@ export async function getCoverageProvider(
   return null
 }
 
+function isProcessAlive(pid: number): boolean {
+  try {
+    // Sending signal 0 checks if process exists without actually killing it: https://nodejs.org/api/process.html#processkillpid-signal
+    process.kill(pid, 0)
+    return true
+  }
+  catch (error) {
+    return (error as NodeJS.ErrnoException).code === 'EPERM'
+  }
+}
+
 export class BaseCoverageProvider {
   ctx!: Vitest
   readonly name!: 'v8' | 'istanbul'
@@ -240,6 +253,8 @@ export class BaseCoverageProvider {
   }
 
   async clean(clean = true): Promise<void> {
+    await this.acquireReportsDirectoryLock()
+
     if (clean && existsSync(this.options.reportsDirectory)) {
       await fs.rm(this.options.reportsDirectory, {
         recursive: true,
@@ -262,6 +277,105 @@ export class BaseCoverageProvider {
     this.pendingPromises = []
   }
 
+  private get reportsDirectoryLockFile(): string {
+    const hash = createHash('sha256')
+      .update(resolve(this.options.reportsDirectory))
+      .digest('hex')
+      .slice(0, 16)
+
+    return resolve(tmpdir(), `vitest-coverage-${hash}.lock`)
+  }
+
+  private async acquireReportsDirectoryLock(): Promise<void> {
+    const lockFile = this.reportsDirectoryLockFile
+    const payload = JSON.stringify({
+      pid: process.pid,
+      reportsDirectory: resolve(this.options.reportsDirectory),
+      timestamp: Date.now(),
+    })
+
+    // One initial attempt, plus a single retry that is only reached after
+    // reclaiming a stale lock left behind by a process that no longer exists.
+    for (let attempt = 0; attempt < 2; attempt++) {
+      try {
+        await fs.writeFile(lockFile, payload, { flag: 'wx' })
+        return
+      }
+      catch (error) {
+        if ((error as NodeJS.ErrnoException).code !== 'EEXIST') {
+          throw error
+        }
+      }
+
+      const owner = await this.readReportsDirectoryLockOwner(lockFile)
+
+      // This same process already holds the lock (e.g. watch-mode reruns).
+      if (owner?.pid === process.pid) {
+        return
+      }
+
+      // The lock is held by another running process, so fail fast.
+      if (owner != null && isProcessAlive(owner.pid)) {
+        throw this.reportsDirectoryInUseError(owner.pid)
+      }
+
+      // The lock is stale (its owner is gone or the file is unreadable).
+      // Steal it with an atomic rename so that two processes racing to reclaim
+      // the same stale lock can't both succeed: only the one whose rename wins
+      // removes the file, the loser's rename fails with ENOENT and it retries
+      // the exclusive create on the next iteration.
+      try {
+        await fs.rename(lockFile, `${lockFile}.${process.pid}.stale`)
+        await fs.rm(`${lockFile}.${process.pid}.stale`, { force: true })
+      }
+      catch (error) {
+        if ((error as NodeJS.ErrnoException).code !== 'ENOENT') {
+          throw error
+        }
+      }
+    }
+
+    // The reclaimed stale lock was taken by another process before we could grab it.
+    throw this.reportsDirectoryInUseError()
+  }
+
+  private reportsDirectoryInUseError(pid?: number): Error {
+    return new Error(
+      `The coverage report directory "${this.options.reportsDirectory}" is already in use by `
+      + `another Vitest process${pid ? ` (pid ${pid})` : ''}. Running coverage for multiple Vitest `
+      + `processes in the same directory at the same time is not supported, because they would delete `
+      + `each other's reports.\nGive each run its own "coverage.reportsDirectory" `
+      + `(e.g. --coverage.reportsDirectory=coverage-${process.pid}) or run them sequentially.`,
+    )
+  }
+
+  private async readReportsDirectoryLockOwner(
+    lockFile: string,
+  ): Promise<{ pid: number } | null> {
+    try {
+      const owner = JSON.parse(await fs.readFile(lockFile, 'utf-8'))
+
+      if (typeof owner?.pid === 'number') {
+        return owner
+      }
+    }
+    catch {}
+
+    return null
+  }
+
+  private async releaseReportsDirectoryLock(): Promise<void> {
+    const lockFile = this.reportsDirectoryLockFile
+    const owner = await this.readReportsDirectoryLockOwner(lockFile)
+
+    // Only remove the lock when we can confirm we own it. A transient unreadable
+    // read (e.g. another process mid-write) must not authorize deleting a lock
+    // that belongs to a different process.
+    if (owner?.pid === process.pid) {
+      await fs.rm(lockFile, { force: true })
+    }
+  }
+
   private normalizeCoverageFileError(error: unknown): unknown {
     if (
       error instanceof Error
@@ -349,12 +463,17 @@ export class BaseCoverageProvider {
   }
 
   async cleanAfterRun(): Promise<void> {
-    this.coverageFiles = new Map()
-    await fs.rm(this.coverageFilesDirectory, { recursive: true })
+    try {
+      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 })
+      // 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 })
+      }
+    }
+    finally {
+      await this.releaseReportsDirectoryLock()
     }
   }
 

test/coverage-test/test/temporary-files.unit.test.ts

@@ -1,7 +1,26 @@
-import { resolve } from 'node:path'
-import { expect, test } from 'vitest'
+import { spawn } from 'node:child_process'
+import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join, resolve } from 'node:path'
+import { expect, onTestFinished, test } from 'vitest'
 import { BaseCoverageProvider } from 'vitest/node'
 
+function createProvider() {
+  const reportsDirectory = mkdtempSync(join(tmpdir(), 'vitest-coverage-reports-'))
+  const provider = new BaseCoverageProvider()
+  provider.coverageFilesDirectory = join(reportsDirectory, '.tmp')
+  provider.options = { reportsDirectory } as any
+
+  const lockFile = (provider as any).reportsDirectoryLockFile as string
+
+  onTestFinished(() => {
+    rmSync(reportsDirectory, { recursive: true, force: true })
+    rmSync(lockFile, { force: true })
+  })
+
+  return { provider, reportsDirectory, lockFile }
+}
+
 test('missing coverage temp directory throws an actionable error', async () => {
   const provider = new BaseCoverageProvider()
   provider.coverageFilesDirectory = resolve('missing-coverage-directory', '.tmp')
@@ -17,3 +36,59 @@ test('missing coverage temp directory throws an actionable error', async () => {
     `Something removed the coverage directory "${provider.coverageFilesDirectory}" Vitest created earlier. Make sure you are not running multiple Vitests with the same "coverage.reportsDirectory" at the same time.`,
   )
 })
+
+test('clean() acquires the reportsDirectory lock and cleanAfterRun() releases it', async () => {
+  const { provider, lockFile } = createProvider()
+
+  await provider.clean(true)
+
+  expect(existsSync(provider.coverageFilesDirectory)).toBe(true)
+  expect(existsSync(lockFile)).toBe(true)
+  expect(JSON.parse(readFileSync(lockFile, 'utf-8')).pid).toBe(process.pid)
+
+  await provider.cleanAfterRun()
+
+  expect(existsSync(lockFile)).toBe(false)
+})
+
+test('clean() is re-entrant for the same process (e.g. watch mode reruns)', async () => {
+  const { provider } = createProvider()
+
+  await provider.clean(true)
+
+  await expect(provider.clean(true)).resolves.toBeUndefined()
+
+  await provider.cleanAfterRun()
+})
+
+test('clean() throws an actionable error when another live process holds the lock', async () => {
+  const { provider, reportsDirectory, lockFile } = createProvider()
+
+  const child = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { stdio: 'ignore' })
+  await new Promise(resolve => child.once('spawn', resolve))
+  onTestFinished(() => {
+    child.kill()
+  })
+
+  writeFileSync(lockFile, JSON.stringify({ pid: child.pid, reportsDirectory, timestamp: Date.now() }))
+
+  await expect(provider.clean(true)).rejects.toThrow('is already in use by another Vitest process')
+
+  expect(JSON.parse(readFileSync(lockFile, 'utf-8')).pid).toBe(child.pid)
+})
+
+test('clean() reclaims a stale lock left by a process that no longer exists', async () => {
+  const { provider, reportsDirectory, lockFile } = createProvider()
+
+  const child = spawn(process.execPath, ['-e', ''], { stdio: 'ignore' })
+  await new Promise(resolve => child.once('exit', resolve))
+  const deadPid = child.pid!
+
+  writeFileSync(lockFile, JSON.stringify({ pid: deadPid, reportsDirectory, timestamp: Date.now() }))
+
+  await expect(provider.clean(true)).resolves.toBeUndefined()
+
+  expect(JSON.parse(readFileSync(lockFile, 'utf-8')).pid).toBe(process.pid)
+
+  await provider.cleanAfterRun()
+})