From c2f0c6ca6fe72f374ff3ef1fb2212a14b465eaf8 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Wed, 23 Mar 2022 14:15:24 +0530 Subject: [PATCH] fix: cleanup after `pages dev` tests We weren't killing the process started by wrangler whenever its parent was killed. This fix is to listen on SIGINT/SIGTERM and kill that process. I also did some minor configuration cleanups. Fixes https://github.com/cloudflare/wrangler2/issues/397 and https://github.com/cloudflare/wrangler2/issues/618 --- .changeset/young-boats-dance.md | 9 ++++++++ .../example-pages-functions-app/package.json | 6 ++--- .../tests/index.test.ts | 17 ++++++++++---- .../example-pages-functions-app/tsconfig.json | 3 ++- packages/example-remix-pages-app/package.json | 3 ++- .../tests/index.test.ts | 23 +++++++++++++------ packages/wrangler/bin/wrangler.js | 11 ++++++++- 7 files changed, 55 insertions(+), 17 deletions(-) create mode 100644 .changeset/young-boats-dance.md diff --git a/.changeset/young-boats-dance.md b/.changeset/young-boats-dance.md new file mode 100644 index 000000000000..2746226d3da8 --- /dev/null +++ b/.changeset/young-boats-dance.md @@ -0,0 +1,9 @@ +--- +"wrangler": patch +--- + +fix: cleanup after `pages dev` tests + +We weren't killing the process started by wrangler whenever its parent was killed. This fix is to listen on SIGINT/SIGTERM and kill that process. I also did some minor configuration cleanups. + +Fixes https://github.com/cloudflare/wrangler2/issues/397 and https://github.com/cloudflare/wrangler2/issues/618 diff --git a/packages/example-pages-functions-app/package.json b/packages/example-pages-functions-app/package.json index 362b7661503d..b67c40523f83 100644 --- a/packages/example-pages-functions-app/package.json +++ b/packages/example-pages-functions-app/package.json @@ -1,6 +1,7 @@ { - "private": true, "name": "example-pages-functions-app", + "version": "0.0.0", + "private": true, "scripts": { "dev": "npx wrangler pages dev public --port 8789", "test": "npx jest --forceExit" @@ -29,6 +30,5 @@ } ] } - }, - "version": null + } } diff --git a/packages/example-pages-functions-app/tests/index.test.ts b/packages/example-pages-functions-app/tests/index.test.ts index c36760f7072b..e2b62ae04ac9 100644 --- a/packages/example-pages-functions-app/tests/index.test.ts +++ b/packages/example-pages-functions-app/tests/index.test.ts @@ -1,5 +1,5 @@ import { spawn } from "child_process"; -import { resolve } from "path"; +import * as path from "path"; import { fetch } from "undici"; import type { ChildProcess } from "child_process"; import type { Response } from "undici"; @@ -26,7 +26,7 @@ describe("Pages Functions", () => { beforeAll(async () => { wranglerProcess = spawn("npm", ["run", "dev"], { shell: isWindows, - cwd: resolve(__dirname, "../"), + cwd: path.resolve(__dirname, "../"), env: { BROWSER: "none", ...process.env }, }); wranglerProcess.stdout.on("data", (chunk) => { @@ -37,8 +37,17 @@ describe("Pages Functions", () => { }); }); - afterAll(() => { - wranglerProcess.kill(); + afterAll(async () => { + await new Promise((resolve, reject) => { + wranglerProcess.once("exit", (code) => { + if (!code) { + resolve(code); + } else { + reject(code); + } + }); + wranglerProcess.kill(); + }); }); it("renders static pages", async () => { diff --git a/packages/example-pages-functions-app/tsconfig.json b/packages/example-pages-functions-app/tsconfig.json index 554bd93e6b0a..64ff8608ae43 100644 --- a/packages/example-pages-functions-app/tsconfig.json +++ b/packages/example-pages-functions-app/tsconfig.json @@ -4,6 +4,7 @@ "target": "ES2020", "module": "CommonJS", "lib": ["ES2020"], - "types": ["@cloudflare/workers-types"] + "types": ["@cloudflare/workers-types"], + "moduleResolution": "node" } } diff --git a/packages/example-remix-pages-app/package.json b/packages/example-remix-pages-app/package.json index 26dc2379e63d..75320d0f7e21 100644 --- a/packages/example-remix-pages-app/package.json +++ b/packages/example-remix-pages-app/package.json @@ -1,6 +1,7 @@ { - "private": true, "name": "example-remix-pages-app", + "version": "0.0.0", + "private": true, "description": "", "license": "", "scripts": { diff --git a/packages/example-remix-pages-app/tests/index.test.ts b/packages/example-remix-pages-app/tests/index.test.ts index eb97987e41f5..66f2fb0cae4a 100644 --- a/packages/example-remix-pages-app/tests/index.test.ts +++ b/packages/example-remix-pages-app/tests/index.test.ts @@ -1,5 +1,5 @@ import { spawn, spawnSync } from "child_process"; -import { resolve } from "path"; +import * as path from "path"; import { fetch } from "undici"; import type { ChildProcess } from "child_process"; import type { Response } from "undici"; @@ -26,23 +26,32 @@ describe("Remix", () => { beforeAll(async () => { spawnSync("npm", ["run", "build"], { shell: isWindows, - cwd: resolve(__dirname, "../"), + cwd: path.resolve(__dirname, "../"), }); wranglerProcess = spawn("npm", ["run", "dev:wrangler"], { shell: isWindows, - cwd: resolve(__dirname, "../"), + cwd: path.resolve(__dirname, "../"), env: { BROWSER: "none", ...process.env }, }); - wranglerProcess.stdout.on("data", (chunk) => { + wranglerProcess.stdout?.on("data", (chunk) => { console.log(chunk.toString()); }); - wranglerProcess.stderr.on("data", (chunk) => { + wranglerProcess.stderr?.on("data", (chunk) => { console.log(chunk.toString()); }); }); - afterAll(() => { - wranglerProcess.kill(); + afterAll(async () => { + await new Promise((resolve, reject) => { + wranglerProcess.once("exit", (code) => { + if (!code) { + resolve(code); + } else { + reject(code); + } + }); + wranglerProcess.kill(); + }); }); it("renders", async () => { diff --git a/packages/wrangler/bin/wrangler.js b/packages/wrangler/bin/wrangler.js index d02521eebf6a..e05330cb9231 100755 --- a/packages/wrangler/bin/wrangler.js +++ b/packages/wrangler/bin/wrangler.js @@ -5,6 +5,8 @@ const semiver = require("semiver"); const MIN_NODE_VERSION = "16.7.0"; +let wranglerProcess; + async function main() { if (semiver(process.versions.node, MIN_NODE_VERSION) < 0) { // Note Volta and nvm are also recommended in the official docs: @@ -18,7 +20,7 @@ Consider using a Node.js version manager such as https://volta.sh/ or https://gi return; } - spawn( + wranglerProcess = spawn( process.execPath, [ "--no-warnings", @@ -33,4 +35,11 @@ Consider using a Node.js version manager such as https://volta.sh/ or https://gi ); } +process.on("SIGINT", () => { + wranglerProcess.kill(); +}); +process.on("SIGTERM", () => { + wranglerProcess.kill(); +}); + void main();