Skip to content

Commit 5eb3570

Browse files
authored
fix(cache): improve fsModuleCache resiliency during external modifications (#10869)
1 parent 86d4a9d commit 5eb3570

3 files changed

Lines changed: 185 additions & 14 deletions

File tree

packages/vitest/src/node/cache/fsModuleCache.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -324,16 +324,23 @@ export class FileSystemModuleCache {
324324

325325
// no metadata found, just store a new one, don't reset the cache
326326
if (!metadata) {
327-
if (!existsSync(this.rootCache)) {
328-
mkdirSync(this.rootCache, { recursive: true })
329-
}
330327
debugFs?.(`fs metadata file was created with hash ${currentLockfileHash}`)
331328

332-
await writeFile(
333-
this.metadataFilePath,
334-
JSON.stringify({ lockfileHash: currentLockfileHash }, null, 2),
335-
'utf-8',
336-
)
329+
try {
330+
if (!existsSync(this.rootCache)) {
331+
mkdirSync(this.rootCache, { recursive: true })
332+
}
333+
await writeFile(
334+
this.metadataFilePath,
335+
JSON.stringify({ lockfileHash: currentLockfileHash }, null, 2),
336+
'utf-8',
337+
)
338+
}
339+
catch (error) {
340+
// Recording the metadata is best-effort and losing the file shouldn't
341+
// abort the entire execution
342+
debugFs?.(`failed to write fs cache metadata: ${error}`)
343+
}
337344
return
338345
}
339346

packages/vitest/src/node/environments/fetchModule.ts

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,17 @@ import { readFile } from 'node:fs/promises'
1111
import { isExternalUrl, unwrapId } from '@vitest/utils/helpers'
1212
import { join } from 'pathe'
1313
import { fetchModule } from 'vite'
14+
import { createDebugger } from '../../utils/debugger'
1415
import { hash } from '../hash'
1516
import { detectModuleType } from '../resolver'
1617
import { normalizeResolvedIdToUrl } from './normalizeUrl'
1718

18-
const saveCachePromises = new Map<string, Promise<VitestFetchResult>>()
19+
const debugFs = createDebugger('vitest:cache:fs')
20+
21+
const saveCachePromises = new Map<
22+
string,
23+
Promise<VitestFetchResult | FetchCachedFileSystemResult>
24+
>()
1925
const readFilePromises = new Map<string, Promise<string | null>>()
2026

2127
/**
@@ -252,6 +258,13 @@ class ModuleFetcher {
252258
importer: string | undefined,
253259
): Promise<FetchResult | FetchCachedFileSystemResult | undefined> {
254260
if (moduleGraphModule.transformResult?.__vitestTmp) {
261+
if (!existsSync(moduleGraphModule.transformResult.__vitestTmp)) {
262+
debugFs?.(
263+
`cached file ${moduleGraphModule.transformResult.__vitestTmp} disappeared, re-transforming`,
264+
)
265+
moduleGraphModule.transformResult.__vitestTmp = undefined
266+
return undefined
267+
}
255268
return {
256269
cached: true as const,
257270
file: moduleGraphModule.file,
@@ -388,21 +401,23 @@ class ModuleFetcher {
388401
: result
389402

390403
if (saveCachePromises.has(cachePath)) {
391-
await saveCachePromises.get(cachePath)
392-
return returnResult
404+
return saveCachePromises.get(cachePath)!
393405
}
394406

395407
const savePromise = this.fsCache
396408
.saveCachedModule(cachePath, result, importedUrls, mappings)
397-
.then(() => result)
409+
.then(() => returnResult)
410+
.catch((error) => {
411+
debugFs?.(`failed to cache ${cachePath}, serving it inline: ${error}`)
412+
return result
413+
})
398414
.finally(() => {
399415
saveCachePromises.delete(cachePath)
400416
})
401417

402418
saveCachePromises.set(cachePath, savePromise)
403-
await savePromise
404419

405-
return returnResult
420+
return savePromise
406421
}
407422

408423
private readFileConcurrently(file: string): Promise<string | null> {
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
import { chmodSync, existsSync, mkdirSync, readdirSync, rmSync } from 'node:fs'
2+
import { join } from 'pathe'
3+
import { afterEach, expect, test } from 'vitest'
4+
import { runInlineTests } from '#test-utils'
5+
6+
// The on-disk module cache is an optimisation. It lives in a directory nobody
7+
// owns exclusively — CI images sweep it mid-run, disks fill up, mounts are
8+
// mounted read-only — so every failure to write or read it has to degrade to
9+
// serving the module inline instead of failing the run.
10+
11+
const restore: Array<() => void> = []
12+
13+
afterEach(() => {
14+
restore.splice(0).forEach(fn => fn())
15+
})
16+
17+
// Windows ignores the POSIX mode bits `chmodSync` sets on a directory, so there
18+
// the cache stays writable and this scenario cannot be staged at all.
19+
test.skipIf(process.platform === 'win32')('a cache directory that cannot be written to does not fail the run', async () => {
20+
const cachePath = join(
21+
import.meta.dirname,
22+
'../fixtures/.tmp-readonly-module-cache',
23+
)
24+
rmSync(cachePath, { force: true, recursive: true })
25+
mkdirSync(cachePath, { recursive: true })
26+
chmodSync(cachePath, 0o555)
27+
restore.push(() => {
28+
chmodSync(cachePath, 0o755)
29+
rmSync(cachePath, { force: true, recursive: true })
30+
})
31+
32+
const { stderr, testTree } = await runInlineTests(
33+
{
34+
'sum.js': `export const sum = (a, b) => a + b`,
35+
'basic.test.js': /* js */ `
36+
import { expect, test } from "vitest"
37+
import { sum } from "./sum.js"
38+
test("still runs without a writable cache", () => {
39+
expect(sum(1, 2)).toBe(3)
40+
})
41+
`,
42+
},
43+
{
44+
fsModuleCache: true,
45+
fsModuleCachePath: cachePath,
46+
},
47+
)
48+
49+
expect(stderr).toBe('')
50+
expect(testTree()).toMatchObject({
51+
'basic.test.js': {
52+
'still runs without a writable cache': 'passed',
53+
},
54+
})
55+
// nothing could be written, and that is fine
56+
expect(readdirSync(cachePath)).toEqual([])
57+
})
58+
59+
test('a cached file removed mid-run is re-transformed instead of failing', async () => {
60+
const cachePath = join(
61+
import.meta.dirname,
62+
'../fixtures/.tmp-swept-module-cache',
63+
)
64+
rmSync(cachePath, { force: true, recursive: true })
65+
restore.push(() => rmSync(cachePath, { force: true, recursive: true }))
66+
67+
// `a` imports the shared module (populating the cache and the server's
68+
// in-memory pointer to it), then deletes the cache from under the run. `b`
69+
// imports the same module afterwards, so the server has to notice its
70+
// pointer is dangling and re-transform rather than hand back a dead path.
71+
const { stderr, testTree } = await runInlineTests(
72+
{
73+
'shared.js': `export const shared = "shared-value"`,
74+
'a.test.js': /* js */ `
75+
import { rmSync } from "node:fs"
76+
import { expect, test } from "vitest"
77+
import { shared } from "./shared.js"
78+
test("populates the cache, then sweeps it", () => {
79+
expect(shared).toBe("shared-value")
80+
rmSync(${JSON.stringify(cachePath)}, { force: true, recursive: true })
81+
})
82+
`,
83+
'b.test.js': /* js */ `
84+
import { expect, test } from "vitest"
85+
import { shared } from "./shared.js"
86+
test("still resolves the swept module", () => {
87+
expect(shared).toBe("shared-value")
88+
})
89+
`,
90+
},
91+
{
92+
fsModuleCache: true,
93+
fsModuleCachePath: cachePath,
94+
// run the files in order in one process so `a` reliably sweeps the cache
95+
// before `b` asks the server for the same module
96+
fileParallelism: false,
97+
maxWorkers: 1,
98+
minWorkers: 1,
99+
sequence: { shuffle: false },
100+
},
101+
)
102+
103+
expect(stderr).toBe('')
104+
expect(testTree()).toMatchObject({
105+
'a.test.js': {
106+
'populates the cache, then sweeps it': 'passed',
107+
},
108+
'b.test.js': {
109+
'still resolves the swept module': 'passed',
110+
},
111+
})
112+
})
113+
114+
test('the cache is still populated and reused when nothing interferes', async () => {
115+
const cachePath = join(
116+
import.meta.dirname,
117+
'../fixtures/.tmp-healthy-module-cache',
118+
)
119+
rmSync(cachePath, { force: true, recursive: true })
120+
restore.push(() => rmSync(cachePath, { force: true, recursive: true }))
121+
122+
const structure = {
123+
'sum.js': `export const sum = (a, b) => a + b`,
124+
'basic.test.js': /* js */ `
125+
import { expect, test } from "vitest"
126+
import { sum } from "./sum.js"
127+
test("adds", () => {
128+
expect(sum(1, 2)).toBe(3)
129+
})
130+
`,
131+
}
132+
const config = {
133+
fsModuleCache: true,
134+
fsModuleCachePath: cachePath,
135+
}
136+
137+
const cold = await runInlineTests(structure, config)
138+
expect(cold.stderr).toBe('')
139+
expect(existsSync(cachePath)).toBe(true)
140+
const written = readdirSync(cachePath).length
141+
// the degradation paths must not have turned caching off altogether
142+
expect(written).toBeGreaterThan(0)
143+
144+
const warm = await runInlineTests(structure, config)
145+
expect(warm.stderr).toBe('')
146+
expect(warm.testTree()).toMatchObject({
147+
'basic.test.js': { adds: 'passed' },
148+
})
149+
})

0 commit comments

Comments
 (0)
Sponsor
SponsoredKunjungi sekarang
Promo