try improve fixture flakiness due to unstable_dev (#6367)

* try address fixture flakiness * add changeset
cloudflare · Jul 29, 2024 · 7588800 · 7588800
1 parent 4998c54
commit 7588800
Show file tree

Hide file tree

Showing 4 changed files with 104 additions and 91 deletions.
diff --git a/.changeset/flat-bears-scream.md b/.changeset/flat-bears-scream.md
@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+fix: implicitly cleanup (call `stop()`) in `unstable_dev` if the returned Promise rejected and the `stop()` function was not returned
diff --git a/fixtures/get-platform-proxy/tests/get-platform-proxy.env.test.ts b/fixtures/get-platform-proxy/tests/get-platform-proxy.env.test.ts
@@ -374,6 +374,7 @@ async function startWorkers(): Promise<UnstableDevWorker[]> {
 				ip: "127.0.0.1",
 				experimental: {
 					devEnv: true,
+					fileBasedRegistry: true,
 				},
 			});
 		})

diff --git a/fixtures/local-mode-tests/tests/module.test.ts b/fixtures/local-mode-tests/tests/module.test.ts
@@ -20,6 +20,7 @@ describe("module worker", () => {
 				config: path.resolve(__dirname, "..", "wrangler.module.toml"),
 				vars: { VAR4: "https://google.com" },
 				ip: "127.0.0.1",
+				port: 0,
 				experimental: {
 					disableExperimentalWarning: true,
 					disableDevRegistry: true,
@@ -57,74 +58,72 @@ describe("module worker", () => {
 			}"
 		`);
 	});
-	describe("header parsing", () => {
-		it.concurrent("should return Hi by default", async () => {
-			const resp = await worker.fetch("/");
-			expect(resp).not.toBe(undefined);
+
+	it("should return Hi by default", async () => {
+		const resp = await worker.fetch("/");
+		expect(resp).not.toBe(undefined);
+		const respJson = await resp.text();
+		expect(respJson).toBe(JSON.stringify({ greeting: "Hi!" }));
+	});
+	it("should return Bonjour when French", async () => {
+		const resp = await worker.fetch("/", { headers: { lang: "fr-FR" } });
+		expect(resp).not.toBe(undefined);
+		if (resp) {
 			const respJson = await resp.text();
-			expect(respJson).toBe(JSON.stringify({ greeting: "Hi!" }));
-		});
-		it.concurrent("should return Bonjour when French", async () => {
-			const resp = await worker.fetch("/", { headers: { lang: "fr-FR" } });
-			expect(resp).not.toBe(undefined);
-			if (resp) {
-				const respJson = await resp.text();
-				expect(respJson).toBe(JSON.stringify({ greeting: "Bonjour!" }));
-			}
-		});
-
-		it.concurrent("should return G'day when Australian", async () => {
-			const resp = await worker.fetch("/", { headers: { lang: "en-AU" } });
-			expect(resp).not.toBe(undefined);
-			if (resp) {
-				const respJson = await resp.text();
-				expect(respJson).toBe(JSON.stringify({ greeting: "G'day!" }));
-			}
-		});
-
-		it.concurrent("should return Good day when British", async () => {
-			const resp = await worker.fetch("/", { headers: { lang: "en-GB" } });
-			expect(resp).not.toBe(undefined);
-			if (resp) {
-				const respJson = await resp.text();
-				expect(respJson).toBe(JSON.stringify({ greeting: "Good day!" }));
-			}
-		});
-
-		it.concurrent("should return Howdy when Texan", async () => {
-			const resp = await worker.fetch("/", { headers: { lang: "en-TX" } });
-			expect(resp).not.toBe(undefined);
-			if (resp) {
-				const respJson = await resp.text();
-				expect(respJson).toBe(JSON.stringify({ greeting: "Howdy!" }));
-			}
-		});
-
-		it.concurrent("should return Hello when American", async () => {
-			const resp = await worker.fetch("/", { headers: { lang: "en-US" } });
-			expect(resp).not.toBe(undefined);
-			if (resp) {
-				const respJson = await resp.text();
-				expect(respJson).toBe(JSON.stringify({ greeting: "Hello!" }));
-			}
-		});
-
-		it.concurrent("should return Hola when Spanish", async () => {
-			const resp = await worker.fetch("/", { headers: { lang: "es-ES" } });
-			expect(resp).not.toBe(undefined);
-			if (resp) {
-				const respJson = await resp.text();
-				expect(respJson).toBe(JSON.stringify({ greeting: "Hola!" }));
-			}
-		});
+			expect(respJson).toBe(JSON.stringify({ greeting: "Bonjour!" }));
+		}
+	});
+
+	it("should return G'day when Australian", async () => {
+		const resp = await worker.fetch("/", { headers: { lang: "en-AU" } });
+		expect(resp).not.toBe(undefined);
+		if (resp) {
+			const respJson = await resp.text();
+			expect(respJson).toBe(JSON.stringify({ greeting: "G'day!" }));
+		}
+	});
+
+	it("should return Good day when British", async () => {
+		const resp = await worker.fetch("/", { headers: { lang: "en-GB" } });
+		expect(resp).not.toBe(undefined);
+		if (resp) {
+			const respJson = await resp.text();
+			expect(respJson).toBe(JSON.stringify({ greeting: "Good day!" }));
+		}
+	});
+
+	it("should return Howdy when Texan", async () => {
+		const resp = await worker.fetch("/", { headers: { lang: "en-TX" } });
+		expect(resp).not.toBe(undefined);
+		if (resp) {
+			const respJson = await resp.text();
+			expect(respJson).toBe(JSON.stringify({ greeting: "Howdy!" }));
+		}
+	});
+
+	it("should return Hello when American", async () => {
+		const resp = await worker.fetch("/", { headers: { lang: "en-US" } });
+		expect(resp).not.toBe(undefined);
+		if (resp) {
+			const respJson = await resp.text();
+			expect(respJson).toBe(JSON.stringify({ greeting: "Hello!" }));
+		}
+	});
+
+	it("should return Hola when Spanish", async () => {
+		const resp = await worker.fetch("/", { headers: { lang: "es-ES" } });
+		expect(resp).not.toBe(undefined);
+		if (resp) {
+			const respJson = await resp.text();
+			expect(respJson).toBe(JSON.stringify({ greeting: "Hola!" }));
+		}
 	});
-	describe("buffer import", () => {
-		it.concurrent("returns hex string", async () => {
-			const resp = await worker.fetch("/buffer");
-			expect(resp).not.toBe(undefined);
-
-			const text = await resp.text();
-			expect(text).toMatch("68656c6c6f");
-		});
+
+	it("returns hex string", async () => {
+		const resp = await worker.fetch("/buffer");
+		expect(resp).not.toBe(undefined);
+
+		const text = await resp.text();
+		expect(text).toMatch("68656c6c6f");
 	});
 });
diff --git a/packages/wrangler/src/dev/start-server.ts b/packages/wrangler/src/dev/start-server.ts
@@ -175,31 +175,39 @@ export async function startDevServer(
 
 		await devEnv.config.set(startDevWorkerOptions);
 
-		await Promise.all([
-			// adhere to unstable_dev contract:
-			//   - only resolve when UserWorker is ready
-			//   - reject if UserWorker fails to start
-			Promise.race(
-				devEnv.runtimes.flatMap((runtime) => [
-					once(runtime, "reloadComplete"),
-					once(runtime, "error").then((err) => Promise.reject(err)),
-				])
-			),
-			// adhere to unstable_dev contract:
-			//   - only resolve when _perceived_ UserWorker is ready
-			//   - throw if _perceived_ UserWorker fails to start
-			// to the eyeball, the ProxyWorker is the _perceived_ UserWorker
-			Promise.race([
-				devEnv.proxy.ready.promise,
-				once(devEnv.proxy, "error").then((err) => Promise.reject(err)),
-			]),
-		]);
-
-		return {
-			stop: async () => {
-				await Promise.allSettled([stopWorkerRegistry(), devEnv.teardown()]);
-			},
+		const stop = async () => {
+			await Promise.allSettled([stopWorkerRegistry(), devEnv.teardown()]);
 		};
+
+		try {
+			await Promise.all([
+				// adhere to unstable_dev contract:
+				//   - only resolve when UserWorker is ready
+				//   - reject if UserWorker fails to start
+				Promise.race(
+					devEnv.runtimes.flatMap((runtime) => [
+						once(runtime, "reloadComplete"),
+						once(runtime, "error").then((err) => Promise.reject(err)),
+					])
+				),
+				// adhere to unstable_dev contract:
+				//   - only resolve when _perceived_ UserWorker is ready
+				//   - throw if _perceived_ UserWorker fails to start
+				// to the eyeball, the ProxyWorker is the _perceived_ UserWorker
+				Promise.race([
+					devEnv.proxy.ready.promise,
+					once(devEnv.proxy, "error").then((err) => Promise.reject(err)),
+				]),
+			]);
+		} catch (err) {
+			// unstable_dev's api only returns the stop function when the promise resolves
+			// so call stop for the user if the promise rejects
+			await stop();
+
+			throw err;
+		}
+
+		return { stop };
 	}
 
 	// temp: fake these events by calling the handler directly