Skip to content

Commit 33ebaee

Browse files
petebacondarwinpetebacondarwin
committed
[fixtures] De-flake the dev-registry tests on Windows by making tail teardown safe
The `Tests (Windows, fixtures)` job failed roughly half the time on `fixtures/dev-registry`, almost always as a 50s timeout in one of the `vite dev <-> vite dev` tests. The cause is a workerd abort during test teardown: when a dev session is killed while another running session is forwarding tail events to it, the surviving session's workerd calls `std::terminate`. Miniflare then starts a replacement runtime, the Vite plugin restarts the dev server to rebuild its module-runner sockets, and the ~30s of churn lands on whichever test is still running. The fixture made that shape easy to hit. `tail_consumers` formed two cycles (`exported-handler` <-> `worker-entrypoint-with-assets` and `worker-entrypoint` <-> `exported-handler-with-assets`), so most tests carried a live tail edge whether or not they tested tail handlers, and no shutdown order could keep every producer shorter-lived than its consumer. Make the tail relationships a one-directional chain (`worker-entrypoint` -> `exported-handler-with-assets` -> `exported-handler`) and drop the incidental edges, which leaves tail edges only in the tests that assert on them. Split the three bidirectional tail tests into one test per direction so each can start its consumer first and its producer second; Vitest tears sessions down in LIFO order, so the producer is always killed while its consumer is still alive. All existing tail assertions are preserved, including the worker-name log prefixes from multi-worker Wrangler sessions. Two supporting fixes, both of which let a session observe a peer disappearing mid-teardown: - The `devRegistryPath` fixture deleted the registry directory in its teardown, which Vitest runs *before* `onTestFinished` callbacks — i.e. while every dev session was still running. Register the removal as an `onTestFinished` callback during fixture setup so LIFO ordering runs it last instead. - `runWranglerDev`'s `stop()` resolved when the kill signal had been delivered (on Windows, when `taskkill` exited), not when the process was gone. Wait for the actual exit, bounded, so sequential teardown really is sequential.
1 parent 511635c commit 33ebaee

7 files changed

Lines changed: 215 additions & 118 deletions

‎fixtures/dev-registry/tests/dev-registry.test.ts‎

Lines changed: 180 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,18 @@ const it = test.extend<{
2727
// Fixture for creating a temporary directory
2828
async devRegistryPath({}, use) {
2929
const tmpPath = await fs.realpath(await fs.mkdtemp(tmpPathBase));
30+
31+
// Fixture teardown runs *before* `onTestFinished` callbacks, so removing
32+
// the directory here would pull the registry out from under dev sessions
33+
// that are still running. Registering the cleanup as an
34+
// `onTestFinished` callback during fixture setup instead makes it the
35+
// first one registered, and therefore the last one to run under Vitest's
36+
// LIFO ordering — after every dev session has exited.
37+
onTestFinished(async () => {
38+
await fs.rm(tmpPath, { recursive: true, maxRetries: 10 });
39+
});
40+
3041
await use(tmpPath);
31-
await fs.rm(tmpPath, { recursive: true, maxRetries: 10 });
3242
},
3343
});
3444

@@ -89,6 +99,27 @@ async function runWranglerDev(
8999
return url;
90100
}
91101

102+
/**
103+
* Starts a tail consumer, then its producer, returning both URLs.
104+
*
105+
* Vitest runs `onTestFinished` callbacks in LIFO order, so the session started
106+
* last is torn down first. Starting the producer last therefore guarantees it
107+
* is killed while its consumer is still running.
108+
*
109+
* The reverse order is not safe: killing a dev session that another running
110+
* session is forwarding tail events to aborts workerd on the surviving side on
111+
* Windows, which restarts the dev server mid-teardown and times the test out.
112+
* The `tail_consumers` in this fixture are one-directional for the same reason
113+
* — with a cycle there is no order that keeps every producer shorter-lived than
114+
* its consumer.
115+
*/
116+
async function startTailPair(
117+
startConsumer: () => Promise<string>,
118+
startProducer: () => Promise<string>
119+
): Promise<[consumer: string, producer: string]> {
120+
return [await startConsumer(), await startProducer()];
121+
}
122+
92123
async function setupPlatformProxy(config: string, devRegistryPath?: string) {
93124
vi.stubEnv("WRANGLER_REGISTRY_PATH", devRegistryPath);
94125

@@ -395,47 +426,33 @@ describe("Dev Registry: wrangler dev <-> wrangler dev", () => {
395426
}, waitForTimeout);
396427
});
397428

398-
it("supports tail handler", async ({ devRegistryPath }) => {
399-
const exportedHandlerWithAssets = await runWranglerDev(
400-
"wrangler.exported-handler-with-assets.jsonc",
401-
devRegistryPath
402-
);
403-
const workerEntrypoint = await runWranglerDev(
404-
[
405-
"wrangler.worker-entrypoint.jsonc",
406-
"wrangler.internal-durable-object.jsonc",
407-
],
408-
devRegistryPath
429+
it("supports tail handler when the consumer has assets", async ({
430+
devRegistryPath,
431+
}) => {
432+
// The producer runs alongside a second worker so that its logs are
433+
// prefixed with the worker name, exercising multi-worker sessions too
434+
const [exportedHandlerWithAssets, workerEntrypoint] = await startTailPair(
435+
() =>
436+
runWranglerDev(
437+
"wrangler.exported-handler-with-assets.jsonc",
438+
devRegistryPath
439+
),
440+
() =>
441+
runWranglerDev(
442+
[
443+
"wrangler.worker-entrypoint.jsonc",
444+
"wrangler.internal-durable-object.jsonc",
445+
],
446+
devRegistryPath
447+
)
409448
);
410449

411450
const searchParams = new URLSearchParams({
412451
"test-method": "tail",
413452
});
414453

415454
await vi.waitFor(async () => {
416-
// Trigger tail handler of worker-entrypoint via exported handler
417-
await fetch(`${exportedHandlerWithAssets}?${searchParams}`, {
418-
method: "POST",
419-
body: JSON.stringify(["hello world", "this is the 2nd log"]),
420-
});
421-
await fetch(`${exportedHandlerWithAssets}?${searchParams}`, {
422-
method: "POST",
423-
body: JSON.stringify(["some other log"]),
424-
});
425-
426-
const response = await fetch(`${workerEntrypoint}?${searchParams}`);
427-
428-
expect(await response.json()).toEqual({
429-
worker: "Worker Entrypoint",
430-
tailEvents: expect.arrayContaining([
431-
[["[exported-handler]"], ["hello world", "this is the 2nd log"]],
432-
[["[exported-handler]"], ["some other log"]],
433-
]),
434-
});
435-
}, waitForTimeout);
436-
437-
await vi.waitFor(async () => {
438-
// Trigger tail handler of exported-handler via worker-entrypoint
455+
// Trigger tail handler of exported-handler-with-assets via worker-entrypoint
439456
await fetch(`${workerEntrypoint}?${searchParams}`, {
440457
method: "POST",
441458
body: JSON.stringify(["hello from test"]),
@@ -464,6 +481,45 @@ describe("Dev Registry: wrangler dev <-> wrangler dev", () => {
464481
}, waitForTimeout);
465482
});
466483

484+
it("supports tail handler when the producer has assets", async ({
485+
devRegistryPath,
486+
}) => {
487+
const [exportedHandler, exportedHandlerWithAssets] = await startTailPair(
488+
() => runWranglerDev("wrangler.exported-handler.jsonc", devRegistryPath),
489+
() =>
490+
runWranglerDev(
491+
"wrangler.exported-handler-with-assets.jsonc",
492+
devRegistryPath
493+
)
494+
);
495+
496+
const searchParams = new URLSearchParams({
497+
"test-method": "tail",
498+
});
499+
500+
await vi.waitFor(async () => {
501+
// Trigger tail handler of exported-handler via exported-handler-with-assets
502+
await fetch(`${exportedHandlerWithAssets}?${searchParams}`, {
503+
method: "POST",
504+
body: JSON.stringify(["hello world", "this is the 2nd log"]),
505+
});
506+
await fetch(`${exportedHandlerWithAssets}?${searchParams}`, {
507+
method: "POST",
508+
body: JSON.stringify(["some other log"]),
509+
});
510+
511+
const response = await fetch(`${exportedHandler}?${searchParams}`);
512+
513+
expect(await response.json()).toEqual({
514+
worker: "exported-handler",
515+
tailEvents: expect.arrayContaining([
516+
[["[exported-handler]"], ["hello world", "this is the 2nd log"]],
517+
[["[exported-handler]"], ["some other log"]],
518+
]),
519+
});
520+
}, waitForTimeout);
521+
});
522+
467523
it("supports queues across dev sessions", async ({ devRegistryPath }) => {
468524
const exportedHandler = await runWranglerDev(
469525
"wrangler.exported-handler.jsonc",
@@ -710,62 +766,81 @@ describe("Dev Registry: vite dev <-> vite dev", () => {
710766
}, waitForTimeout);
711767
});
712768

713-
it("supports tail handler", async ({ devRegistryPath }) => {
714-
const exportedHandler = await runViteDev(
715-
"vite.exported-handler.config.ts",
716-
devRegistryPath
717-
);
718-
const workerEntrypointWithAssets = await runViteDev(
719-
"vite.worker-entrypoint-with-assets.config.ts",
720-
devRegistryPath
769+
it("supports tail handler when the consumer has assets", async ({
770+
devRegistryPath,
771+
}) => {
772+
const [exportedHandlerWithAssets, workerEntrypoint] = await startTailPair(
773+
() =>
774+
runViteDev(
775+
"vite.exported-handler-with-assets.config.ts",
776+
devRegistryPath
777+
),
778+
() => runViteDev("vite.worker-entrypoint.config.ts", devRegistryPath)
721779
);
722780

723781
const searchParams = new URLSearchParams({
724782
"test-method": "tail",
725783
});
726784

727785
await vi.waitFor(async () => {
728-
// Trigger tail handler of worker-entrypoint via exported-handler
729-
await fetch(`${exportedHandler}?${searchParams}`, {
786+
// Trigger tail handler of exported-handler-with-assets via worker-entrypoint
787+
await fetch(`${workerEntrypoint}?${searchParams}`, {
730788
method: "POST",
731-
body: JSON.stringify(["hello world", "this is the 2nd log"]),
789+
body: JSON.stringify(["hello from test"]),
732790
});
733-
await fetch(`${exportedHandler}?${searchParams}`, {
791+
await fetch(`${workerEntrypoint}?${searchParams}`, {
734792
method: "POST",
735-
body: JSON.stringify(["some other log"]),
793+
body: JSON.stringify(["yet another log", "and another one"]),
736794
});
737795

738796
const response = await fetch(
739-
`${workerEntrypointWithAssets}?${searchParams}`
797+
`${exportedHandlerWithAssets}?${searchParams}`
740798
);
741799

742800
expect(await response.json()).toEqual({
743-
worker: "Worker Entrypoint",
801+
worker: "exported-handler",
744802
tailEvents: expect.arrayContaining([
745-
[["[exported-handler]"], ["hello world", "this is the 2nd log"]],
746-
[["[exported-handler]"], ["some other log"]],
803+
[["[Worker Entrypoint]"], ["hello from test"]],
804+
[["[Worker Entrypoint]"], ["yet another log", "and another one"]],
747805
]),
748806
});
749807
}, waitForTimeout);
808+
});
809+
810+
it("supports tail handler when the producer has assets", async ({
811+
devRegistryPath,
812+
}) => {
813+
const [exportedHandler, exportedHandlerWithAssets] = await startTailPair(
814+
() => runViteDev("vite.exported-handler.config.ts", devRegistryPath),
815+
() =>
816+
runViteDev(
817+
"vite.exported-handler-with-assets.config.ts",
818+
devRegistryPath
819+
)
820+
);
821+
822+
const searchParams = new URLSearchParams({
823+
"test-method": "tail",
824+
});
750825

751826
await vi.waitFor(async () => {
752-
// Trigger tail handler of exported-handler via worker-entrypoint
753-
await fetch(`${workerEntrypointWithAssets}?${searchParams}`, {
827+
// Trigger tail handler of exported-handler via exported-handler-with-assets
828+
await fetch(`${exportedHandlerWithAssets}?${searchParams}`, {
754829
method: "POST",
755-
body: JSON.stringify(["hello from test"]),
830+
body: JSON.stringify(["hello world", "this is the 2nd log"]),
756831
});
757-
await fetch(`${workerEntrypointWithAssets}?${searchParams}`, {
832+
await fetch(`${exportedHandlerWithAssets}?${searchParams}`, {
758833
method: "POST",
759-
body: JSON.stringify(["yet another log", "and another one"]),
834+
body: JSON.stringify(["some other log"]),
760835
});
761836

762837
const response = await fetch(`${exportedHandler}?${searchParams}`);
763838

764839
expect(await response.json()).toEqual({
765840
worker: "exported-handler",
766841
tailEvents: expect.arrayContaining([
767-
[["[Worker Entrypoint]"], ["hello from test"]],
768-
[["[Worker Entrypoint]"], ["yet another log", "and another one"]],
842+
[["[exported-handler]"], ["hello world", "this is the 2nd log"]],
843+
[["[exported-handler]"], ["some other log"]],
769844
]),
770845
});
771846
}, waitForTimeout);
@@ -972,62 +1047,81 @@ describe("Dev Registry: vite dev <-> wrangler dev", () => {
9721047
}, waitForTimeout);
9731048
});
9741049

975-
it("supports tail handler", async ({ devRegistryPath }) => {
976-
const exportedHandlerWithStaticAssets = await runViteDev(
977-
"vite.exported-handler-with-assets.config.ts",
978-
devRegistryPath
979-
);
980-
const workerEntrypoint = await runWranglerDev(
981-
"wrangler.worker-entrypoint.jsonc",
982-
devRegistryPath
1050+
it("supports tail handler from wrangler dev to vite dev", async ({
1051+
devRegistryPath,
1052+
}) => {
1053+
const [exportedHandlerWithAssets, workerEntrypoint] = await startTailPair(
1054+
() =>
1055+
runViteDev(
1056+
"vite.exported-handler-with-assets.config.ts",
1057+
devRegistryPath
1058+
),
1059+
() => runWranglerDev("wrangler.worker-entrypoint.jsonc", devRegistryPath)
9831060
);
9841061

9851062
const searchParams = new URLSearchParams({
9861063
"test-method": "tail",
9871064
});
9881065

9891066
await vi.waitFor(async () => {
990-
// Trigger tail handler of worker-entrypoint via exported-handler
991-
await fetch(`${exportedHandlerWithStaticAssets}?${searchParams}`, {
1067+
// Trigger tail handler of exported-handler-with-assets via worker-entrypoint
1068+
await fetch(`${workerEntrypoint}?${searchParams}`, {
9921069
method: "POST",
993-
body: JSON.stringify(["hello world", "this is the 2nd log"]),
1070+
body: JSON.stringify(["hello from test"]),
9941071
});
995-
await fetch(`${exportedHandlerWithStaticAssets}?${searchParams}`, {
1072+
await fetch(`${workerEntrypoint}?${searchParams}`, {
9961073
method: "POST",
997-
body: JSON.stringify(["some other log"]),
1074+
body: JSON.stringify(["yet another log", "and another one"]),
9981075
});
9991076

1000-
const response = await fetch(`${workerEntrypoint}?${searchParams}`);
1077+
const response = await fetch(
1078+
`${exportedHandlerWithAssets}?${searchParams}`
1079+
);
10011080

10021081
expect(await response.json()).toEqual({
1003-
worker: "Worker Entrypoint",
1082+
worker: "exported-handler",
10041083
tailEvents: expect.arrayContaining([
1005-
[["[exported-handler]"], ["hello world", "this is the 2nd log"]],
1006-
[["[exported-handler]"], ["some other log"]],
1084+
[["[Worker Entrypoint]"], ["hello from test"]],
1085+
[["[Worker Entrypoint]"], ["yet another log", "and another one"]],
10071086
]),
10081087
});
10091088
}, waitForTimeout);
1089+
});
1090+
1091+
it("supports tail handler from vite dev to wrangler dev", async ({
1092+
devRegistryPath,
1093+
}) => {
1094+
const [exportedHandler, exportedHandlerWithAssets] = await startTailPair(
1095+
() => runWranglerDev("wrangler.exported-handler.jsonc", devRegistryPath),
1096+
() =>
1097+
runViteDev(
1098+
"vite.exported-handler-with-assets.config.ts",
1099+
devRegistryPath
1100+
)
1101+
);
1102+
1103+
const searchParams = new URLSearchParams({
1104+
"test-method": "tail",
1105+
});
10101106

10111107
await vi.waitFor(async () => {
1012-
// Trigger tail handler of exported-handler via worker-entrypoint
1013-
await fetch(`${workerEntrypoint}?${searchParams}`, {
1108+
// Trigger tail handler of exported-handler via exported-handler-with-assets
1109+
await fetch(`${exportedHandlerWithAssets}?${searchParams}`, {
10141110
method: "POST",
1015-
body: JSON.stringify(["hello from test"]),
1111+
body: JSON.stringify(["hello world", "this is the 2nd log"]),
10161112
});
1017-
await fetch(`${workerEntrypoint}?${searchParams}`, {
1113+
await fetch(`${exportedHandlerWithAssets}?${searchParams}`, {
10181114
method: "POST",
1019-
body: JSON.stringify(["yet another log", "and another one"]),
1115+
body: JSON.stringify(["some other log"]),
10201116
});
10211117

1022-
const response = await fetch(
1023-
`${exportedHandlerWithStaticAssets}?${searchParams}`
1024-
);
1118+
const response = await fetch(`${exportedHandler}?${searchParams}`);
10251119

10261120
expect(await response.json()).toEqual({
10271121
worker: "exported-handler",
10281122
tailEvents: expect.arrayContaining([
1029-
[["[Worker Entrypoint]"], ["hello from test"]],
1030-
[["[Worker Entrypoint]"], ["yet another log", "and another one"]],
1123+
[["[exported-handler]"], ["hello world", "this is the 2nd log"]],
1124+
[["[exported-handler]"], ["some other log"]],
10311125
]),
10321126
});
10331127
}, waitForTimeout);

‎fixtures/dev-registry/wrangler.exported-handler-with-assets.jsonc‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,12 @@
3434
"entrypoint": "NamedEntrypoint",
3535
},
3636
],
37+
// Middle link of the one-directional tail chain described in
38+
// wrangler.worker-entrypoint.jsonc. Pointing this back at worker-entrypoint
39+
// would close the cycle.
3740
"tail_consumers": [
3841
{
39-
"service": "worker-entrypoint",
42+
"service": "exported-handler",
4043
},
4144
],
4245
}

0 commit comments

Comments
 (0)