fix: cleanup after pages dev tests (#678)

cloudflare · Mar 23, 2022 · 82e4143 · 82e4143
1 parent 990316f
commit 82e4143
Show file tree

Hide file tree

Showing 8 changed files with 57 additions and 17 deletions.
diff --git a/.changeset/young-boats-dance.md b/.changeset/young-boats-dance.md
@@ -0,0 +1,10 @@
+---
+"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
+Fixes 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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();
diff --git a/packages/wrangler/src/pages.tsx b/packages/wrangler/src/pages.tsx
@@ -27,6 +27,7 @@ const EXIT = (message?: string, code?: number) => {
   if (message) console.log(message);
   if (code) process.exitCode = code;
   EXIT_CALLBACKS.forEach((callback) => callback());
+  RUNNING_BUILDERS.forEach((builder) => builder.stop?.());
   process.exit(code);
 };