From 7588800415452fba06f49dd0fdea04fdb6df1498 Mon Sep 17 00:00:00 2001 From: Rahul Sethi <5822355+RamIdeas@users.noreply.github.com> Date: Mon, 29 Jul 2024 20:44:29 +0100 Subject: [PATCH] try improve fixture flakiness due to unstable_dev (#6367) * try address fixture flakiness * add changeset --- .changeset/flat-bears-scream.md | 5 + .../tests/get-platform-proxy.env.test.ts | 1 + .../local-mode-tests/tests/module.test.ts | 133 +++++++++--------- packages/wrangler/src/dev/start-server.ts | 56 ++++---- 4 files changed, 104 insertions(+), 91 deletions(-) create mode 100644 .changeset/flat-bears-scream.md diff --git a/.changeset/flat-bears-scream.md b/.changeset/flat-bears-scream.md new file mode 100644 index 000000000000..78e958c45c8a --- /dev/null +++ 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 index b52c948f0b6b..3c3b48bb4919 100644 --- 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 { 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 index e72707ffa253..ce3faa96fd0e 100644 --- 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 index c410fb273223..831ecc523f1e 100644 --- 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