Skip to content

Commit 4b6fd36

Browse files
Fix vitest watch with durable objects (#11771)
Co-authored-by: Pete Bacon Darwin <pbacondarwin@cloudflare.com>
1 parent 1a9eddd commit 4b6fd36

File tree

12 files changed

+282
-24
lines changed

12 files changed

+282
-24
lines changed

.changeset/fix-do-storage-reset.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@cloudflare/vitest-pool-workers": patch
3+
---
4+
5+
Fix Durable Object storage causing SQLITE_CANTOPEN errors on repeated test runs
6+
7+
When running `vitest` multiple times in watch mode, Durable Object storage would fail with `SQLITE_CANTOPEN` errors. This happened because the storage reset function was deleting directories that workerd still had file handles to.
8+
9+
The fix preserves directory structure during storage reset, deleting only files while
10+
keeping directories intact. This allows workerd to maintain valid handles to SQLite
11+
database directories across test runs.

packages/create-cloudflare/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080
"recast": "^0.22.0",
8181
"semver": "^7.7.1",
8282
"smol-toml": "catalog:default",
83+
"tree-kill": "catalog:default",
8384
"typescript": "catalog:default",
8485
"undici": "catalog:default",
8586
"vite": "catalog:default",

packages/mock-npm-registry/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
"@verdaccio/types": "^10.8.0",
2020
"eslint": "catalog:default",
2121
"get-port": "^7.0.0",
22+
"tree-kill": "catalog:default",
2223
"ts-dedent": "^2.2.0",
2324
"typescript": "catalog:default",
2425
"verdaccio": "^6.0.5"

packages/vite-plugin-cloudflare/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@
6767
"picocolors": "^1.1.1",
6868
"semver": "^7.7.1",
6969
"tinyglobby": "^0.2.12",
70-
"tree-kill": "^1.2.2",
70+
"tree-kill": "catalog:default",
7171
"tsdown": "0.16.3",
7272
"typescript": "catalog:default",
7373
"vite": "catalog:vite-plugin",

packages/vitest-pool-workers/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
"eslint": "catalog:default",
7575
"get-port": "^7.1.0",
7676
"semver": "^7.7.1",
77+
"tree-kill": "catalog:default",
7778
"ts-dedent": "^2.2.0",
7879
"typescript": "catalog:default",
7980
"undici": "catalog:default",

packages/vitest-pool-workers/src/pool/loopback.ts

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import assert from "node:assert";
2+
import { opendirSync, rmSync } from "node:fs";
23
import fs from "node:fs/promises";
34
import path from "node:path";
45
import {
@@ -61,27 +62,35 @@ async function handleSnapshotRequest(
6162
return new Response(null, { status: 405 });
6263
}
6364

64-
async function emptyDir(dirPath: string) {
65-
let names: string[];
65+
function emptyDir(dirPath: string) {
66+
let dir;
6667
try {
67-
names = await fs.readdir(dirPath);
68+
dir = opendirSync(dirPath);
6869
} catch (e) {
6970
if (isFileNotFoundError(e)) {
7071
return;
7172
}
7273
throw e;
7374
}
74-
for (const name of names) {
75-
try {
76-
await fs.rm(path.join(dirPath, name), { recursive: true, force: true });
77-
} catch (e) {
78-
if (isEbusyError(e)) {
79-
// Sometimes workerd holds on to file handles in Windows, preventing us from cleaning up these.
80-
console.warn(
81-
`vitest-pool-worker: Unable to remove temporary directory: ${e}`
82-
);
75+
try {
76+
let entry;
77+
while ((entry = dir.readSync()) !== null) {
78+
const fullPath = path.join(dirPath, entry.name);
79+
80+
if (entry.isDirectory()) {
81+
emptyDir(fullPath);
82+
} else {
83+
try {
84+
rmSync(fullPath, { force: true });
85+
} catch (e) {
86+
if (isEbusyError(e)) {
87+
console.warn(`vitest-pool-worker: Unable to remove file: ${e}`);
88+
}
89+
}
8390
}
8491
}
92+
} finally {
93+
dir.closeSync();
8594
}
8695
}
8796

@@ -225,7 +234,7 @@ export function scheduleStorageReset(mf: Miniflare) {
225234
await abortAllWorker.fetch("http://placeholder", { method: "DELETE" });
226235
for (const persistPath of state.persistPaths) {
227236
// Clear directory rather than removing it so `workerd` can retain handle
228-
await emptyDir(persistPath);
237+
emptyDir(persistPath);
229238
}
230239
state.depth = 0;
231240
// If any of the code in this callback throws, the `storageResetPromise`

packages/vitest-pool-workers/test/global-setup.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import fs from "node:fs/promises";
44
import os from "node:os";
55
import path from "node:path";
66
import { startMockNpmRegistry } from "@cloudflare/mock-npm-registry";
7+
import { version } from "../package.json";
78
import type { GlobalSetupContext } from "vitest/node";
89

910
const repoRoot = path.resolve(__dirname, "../../..");
@@ -48,7 +49,8 @@ async function createTestProject() {
4849
private: true,
4950
type: "module",
5051
devDependencies: {
51-
"@cloudflare/vitest-pool-workers": "*",
52+
// Ensure we use the local version of vitest-pool-workers
53+
"@cloudflare/vitest-pool-workers": version,
5254
vitest: await getVitestPeerDep(),
5355
},
5456
};

packages/vitest-pool-workers/test/helpers.ts

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import fs from "node:fs/promises";
55
import path from "node:path";
66
import util from "node:util";
77
import { stripAnsi } from "miniflare";
8+
import treeKill from "tree-kill";
89
import { test as baseTest, inject, vi } from "vitest";
910

1011
const debuglog = util.debuglog("vitest-pool-workers:test");
@@ -122,7 +123,20 @@ export const test = baseTest.extend<{
122123
// Fixture for a starting long-running `vitest dev` process
123124
async vitestDev({ tmpPath }, use) {
124125
const tmpPoolInstallationPath = inject("tmpPoolInstallationPath");
125-
const processes: childProcess.ChildProcess[] = [];
126+
127+
// In case this process stops unexpectedly, kill all child processes
128+
const processes = new Set<childProcess.ChildProcess>();
129+
const killAllProcesses = () => {
130+
for (const proc of processes) {
131+
if (proc.pid) {
132+
proc.stdout?.destroy();
133+
proc.stderr?.destroy();
134+
proc.stdin?.destroy();
135+
treeKill(proc.pid, "SIGKILL");
136+
}
137+
}
138+
};
139+
process.on("exit", killAllProcesses);
126140

127141
await use(({ flags = [], maxBuffer } = {}) => {
128142
const proc = childProcess.exec(
@@ -133,14 +147,17 @@ export const test = baseTest.extend<{
133147
maxBuffer,
134148
}
135149
);
136-
processes.push(proc);
150+
processes.add(proc);
151+
proc.on("exit", () => {
152+
if (proc) {
153+
processes.delete(proc);
154+
}
155+
});
137156
return wrap(proc);
138157
});
139158

140-
// Kill all processes after the test
141-
for (const proc of processes) {
142-
proc.kill();
143-
}
159+
killAllProcesses();
160+
process.off("exit", killAllProcesses);
144161
},
145162
});
146163

packages/vitest-pool-workers/test/watch.test.ts

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,205 @@
11
import dedent from "ts-dedent";
22
import { minimalVitestConfig, test, waitFor } from "./helpers";
33

4+
const durableObjectWorker = dedent`
5+
export class Counter {
6+
count = 0;
7+
8+
constructor(readonly state: DurableObjectState) {
9+
void state.blockConcurrencyWhile(async () => {
10+
this.count = (await state.storage.get("count")) ?? 0;
11+
});
12+
}
13+
14+
fetch(request: Request) {
15+
this.count++;
16+
void this.state.storage.put("count", this.count);
17+
return new Response(this.count.toString());
18+
}
19+
}
20+
21+
export default {
22+
fetch(request: Request, env: any) {
23+
const { pathname } = new URL(request.url);
24+
const id = env.COUNTER.idFromName(pathname);
25+
const stub = env.COUNTER.get(id);
26+
return stub.fetch(request);
27+
}
28+
}
29+
`;
30+
31+
function makeDurableObjectConfig(isolatedStorage: boolean) {
32+
return dedent`
33+
import { defineWorkersConfig } from "@cloudflare/vitest-pool-workers/config";
34+
export default defineWorkersConfig({
35+
test: {
36+
poolOptions: {
37+
workers: {
38+
main: "./index.ts",
39+
singleWorker: true,
40+
isolatedStorage: ${isolatedStorage},
41+
miniflare: {
42+
compatibilityDate: "2024-01-01",
43+
compatibilityFlags: ["nodejs_compat"],
44+
durableObjects: {
45+
COUNTER: "Counter",
46+
},
47+
},
48+
},
49+
},
50+
}
51+
});
52+
`;
53+
}
54+
55+
// when isolatedStorage: true storage be reset at every test
56+
const isolatedStorageTests = dedent`
57+
import { SELF } from "cloudflare:test";
58+
import { it, expect } from "vitest";
59+
60+
it("first test: verifies incrementing works", async () => {
61+
// First fetch returns 1
62+
let response = await SELF.fetch("https://example.com/test-path");
63+
expect(await response.text()).toBe("1");
64+
// Second fetch returns 2 (counter increments)
65+
response = await SELF.fetch("https://example.com/test-path");
66+
expect(await response.text()).toBe("2");
67+
// Third fetch returns 3
68+
response = await SELF.fetch("https://example.com/test-path");
69+
expect(await response.text()).toBe("3");
70+
});
71+
72+
it("second test: storage is reset, starts at 1 again", async () => {
73+
// With isolatedStorage: true, storage is reset between tests
74+
const response = await SELF.fetch("https://example.com/test-path");
75+
expect(await response.text()).toBe("1");
76+
});
77+
78+
it("third test: storage is reset, starts at 1 again", async () => {
79+
// With isolatedStorage: true, storage is reset between tests
80+
const response = await SELF.fetch("https://example.com/test-path");
81+
expect(await response.text()).toBe("1");
82+
});
83+
`;
84+
85+
// when isolatedStorage: false storage should leak in between tests
86+
const sharedStorageTests = dedent`
87+
import { SELF } from "cloudflare:test";
88+
import { it, expect } from "vitest";
89+
90+
it("first increment", async () => {
91+
const response = await SELF.fetch("https://example.com/test-path");
92+
// First test in a run should always see "1" (storage reset between runs)
93+
expect(await response.text()).toBe("1");
94+
});
95+
96+
it("second increment", async () => {
97+
const response = await SELF.fetch("https://example.com/test-path");
98+
// With isolatedStorage: false, storage leaks between tests
99+
expect(await response.text()).toBe("2");
100+
});
101+
102+
it("third increment", async () => {
103+
const response = await SELF.fetch("https://example.com/test-path");
104+
// With isolatedStorage: false, storage leaks between tests
105+
expect(await response.text()).toBe("3");
106+
});
107+
`;
108+
109+
test(
110+
"DO storage is reset between vitest runs (isolatedStorage: true)",
111+
{ timeout: 60_000 },
112+
async ({ expect, seed, vitestRun }) => {
113+
await seed({
114+
"vitest.config.mts": makeDurableObjectConfig(true),
115+
"index.ts": durableObjectWorker,
116+
"index.test.ts": isolatedStorageTests,
117+
});
118+
119+
let result = await vitestRun();
120+
expect(result.stdout).toMatch(/Tests.*3 passed/s);
121+
expect(await result.exitCode).toBe(0);
122+
123+
result = await vitestRun();
124+
expect(result.stdout).toMatch(/Tests.*3 passed/s);
125+
expect(await result.exitCode).toBe(0);
126+
}
127+
);
128+
129+
test(
130+
"DO storage is reset between vitest runs (isolatedStorage: false)",
131+
{ timeout: 60_000 },
132+
async ({ expect, seed, vitestRun }) => {
133+
await seed({
134+
"vitest.config.mts": makeDurableObjectConfig(false),
135+
"index.ts": durableObjectWorker,
136+
"index.test.ts": sharedStorageTests,
137+
});
138+
139+
let result = await vitestRun();
140+
expect(result.stdout).toMatch(/Tests.*3 passed/s);
141+
expect(await result.exitCode).toBe(0);
142+
143+
result = await vitestRun();
144+
expect(result.stdout).toMatch(/Tests.*3 passed/s);
145+
expect(await result.exitCode).toBe(0);
146+
}
147+
);
148+
149+
test(
150+
"DO storage is reset in watch mode (isolatedStorage: true)",
151+
{ timeout: 50000 },
152+
async ({ expect, seed, vitestDev }) => {
153+
await seed({
154+
"vitest.config.mts": makeDurableObjectConfig(true),
155+
"index.ts": durableObjectWorker,
156+
"index.test.ts": isolatedStorageTests,
157+
});
158+
159+
const result = vitestDev();
160+
161+
await waitFor(() => {
162+
expect(result.stdout).toMatch(/Tests.*3 passed/s);
163+
});
164+
165+
await seed({
166+
"index.test.ts": isolatedStorageTests + "\n// trigger re-run",
167+
});
168+
169+
await waitFor(() => {
170+
const matches = result.stdout.match(/Tests\s+3 passed/g);
171+
expect(matches?.length).toBeGreaterThanOrEqual(2);
172+
});
173+
}
174+
);
175+
176+
test(
177+
"DO storage is reset in watch mode (isolatedStorage: false)",
178+
{ timeout: 50000 },
179+
async ({ expect, seed, vitestDev }) => {
180+
await seed({
181+
"vitest.config.mts": makeDurableObjectConfig(false),
182+
"index.ts": durableObjectWorker,
183+
"index.test.ts": sharedStorageTests,
184+
});
185+
186+
const result = vitestDev();
187+
188+
await waitFor(() => {
189+
expect(result.stdout).toMatch(/Tests.*3 passed/s);
190+
});
191+
192+
await seed({
193+
"index.test.ts": sharedStorageTests + "\n// trigger re-run",
194+
});
195+
196+
await waitFor(() => {
197+
const matches = result.stdout.match(/Tests\s+3 passed/g);
198+
expect(matches?.length).toBeGreaterThanOrEqual(2);
199+
});
200+
}
201+
);
202+
4203
test("automatically re-runs unit tests", async ({
5204
expect,
6205
seed,

packages/wrangler/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@
158158
"source-map": "^0.6.1",
159159
"supports-color": "^9.2.2",
160160
"timeago.js": "^4.0.2",
161+
"tree-kill": "catalog:default",
161162
"ts-dedent": "^2.2.0",
162163
"ts-json-schema-generator": "^1.5.0",
163164
"tsup": "8.3.0",

0 commit comments

Comments
 (0)