[cli] Fix "now-dev-next" and "now-dev-static-build-routing" unit tests (#7824)

These two tests have been problematic on MacOS for a long time,
but now that we've re-introduced `@vercel/next` into this repo,
they've started to fail on Linux/Windows as well.

So the idea is to use the `devCommand` property so that
`@vercel/next` Builder doesn't get invoked at all.

Since the `devCommand` property was not yet being unit tested,
some logic in `vc build` needed to be adjusted in order to properly
shut down the dev command (via `tree-kill` module) when the
process was being stopped in order to cleanly shut down.

packages/cli/src/util/dev/server.ts

@@ -89,6 +89,7 @@ import {
 } from './types';
 import { ProjectEnvVariable, ProjectSettings } from '../../types';
 import exposeSystemEnvs from './expose-system-envs';
+import { treeKill } from '../tree-kill';
 
 const frontendRuntimeSet = new Set(
   frameworkList.map(f => f.useRuntime?.use || '@vercel/static-build')
@@ -998,20 +999,7 @@ export default class DevServer {
     }
 
     if (devProcess) {
-      ops.push(
+      ops.push(treeKill(devProcess.pid));
-        new Promise<void>((resolve, reject) => {
-          devProcess.once('exit', () => resolve());
-          try {
-            process.kill(devProcess.pid);
-          } catch (err) {
-            if (err.code === 'ESRCH') {
-              // Process already exited
-              return resolve();
-            }
-            reject(err);
-          }
-        })
-      );
     }
 
     ops.push(close(this.server));
@@ -1052,12 +1040,7 @@ export default class DevServer {
     const { debug } = this.output;
     debug(`Killing builder dev server with PID ${pid}`);
     this.devServerPids.delete(pid);
-    try {
+    await treeKill(pid);
-      process.kill(pid, 'SIGTERM');
-      debug(`Killed builder dev server with PID ${pid}`);
-    } catch (err) {
-      debug(`Failed to kill builder dev server with PID ${pid}: ${err}`);
-    }
   }
 
   async send404(
@@ -2120,7 +2103,7 @@ export default class DevServer {
       .replace(/%PORT%/g, `${port}`);
 
     this.output.debug(
-      `Starting dev command with parameters : ${JSON.stringify({
+      `Starting dev command with parameters: ${JSON.stringify({
         cwd,
         command,
         port,
@@ -2152,6 +2135,7 @@ export default class DevServer {
       cwd,
       env,
     });
+    this.devProcess = p;
 
     if (!p.stdout || !p.stderr) {
       throw new Error('Expected child process to have stdout and stderr');
@@ -2164,17 +2148,14 @@ export default class DevServer {
       process.stdout.write(data.replace(proxyPort, devPort));
     });
 
-    p.on('exit', (code: number) => {
+    p.on('exit', (code, signal) => {
-      if (code > 0) {
+      this.output.debug(`Dev command exited with "${signal || code}"`);
-        process.exit(code);
-      }
       this.devProcessPort = undefined;
     });
 
     await checkForPort(port, 1000 * 60 * 5);
 
     this.devProcessPort = port;
-    this.devProcess = p;
   }
 }
 

packages/cli/test/fixtures/unit/now-dev-next/.gitignore

@@ -1,2 +1,3 @@
 .next
 yarn.lock
+.vercel

packages/cli/test/fixtures/unit/now-dev-next/next.config.js

@@ -1,3 +0,0 @@
-module.exports = {
-  target: 'serverless'
-}

packages/cli/test/fixtures/unit/now-dev-next/package.json

@@ -5,8 +5,8 @@
     "now-build": "next build"
   },
   "dependencies": {
-    "next": "^8.0.0",
+    "next": "12.1.6",
-    "react": "^16.7.0",
+    "react": "18.1.0",
-    "react-dom": "^16.7.0"
+    "react-dom": "18.1.0"
   }
 }

packages/cli/test/fixtures/unit/now-dev-next/pages/index.js

@@ -1,11 +1,11 @@
-import { withRouter } from 'next/router';
+export async function getServerSideProps({ req }) {
-
+  return {
-function Index({ router }) {
+    props: {
-  const data = {
+      url: req.url,
-    pathname: router.pathname,
+    },
-    query: router.query
   };
-  return <div>{ JSON.stringify(data) }</div>;
 }
 
-export default withRouter(Index);
+export default function Index(props) {
+  return <pre>{JSON.stringify(props)}</pre>;
+}

packages/cli/test/fixtures/unit/now-dev-next/vercel.json

@@ -1,11 +1,9 @@
 {
   "version": 2,
-  "name": "now-dev-next",
-  "builds": [{ "src": "package.json", "use": "@vercel/next@canary" }],
   "routes": [
     {
-      "src": "/(.*)",
+      "src": "/test",
-      "dest": "/index?route-param=b"
+      "dest": "/?route-param=b"
     }
   ]
 }

packages/cli/test/util/dev/server.test.ts

@@ -8,8 +8,8 @@ import listen from 'async-listen';
 import { createServer } from 'http';
 import { client } from '../../mocks/client';
 import DevServer from '../../../src/util/dev/server';
+import { DevServerOptions } from '../../../src/util/dev/types';
 
-const IS_MAC_OS = process.platform === 'darwin';
 const IS_WINDOWS = process.platform === 'win32';
 
 async function runNpmInstall(fixturePath: string) {
@@ -21,29 +21,34 @@ async function runNpmInstall(fixturePath: string) {
   }
 }
 
+interface TestFixtureOptions extends Omit<DevServerOptions, 'output'> {
+  skip?: boolean;
+}
+
 const testFixture =
   (
     name: string,
     fn: (server: DevServer) => Promise<void>,
-    options: { skip: boolean } = { skip: false }
+    options?: TestFixtureOptions
   ) =>
   async () => {
-    if (options.skip) {
+    if (options?.skip) {
       console.log('Skipping this test for this platform.');
       return;
     }
 
-    let server: DevServer | null = null;
+    let server: DevServer | undefined;
     const fixturePath = path.join(__dirname, '../../fixtures/unit', name);
     await runNpmInstall(fixturePath);
     try {
-      server = new DevServer(fixturePath, { output: client.output });
+      server = new DevServer(fixturePath, {
+        ...options,
+        output: client.output,
+      });
       await server.start(0);
       await fn(server);
     } finally {
-      if (server) {
+      await server?.stop();
-        await server.stop();
-      }
     }
   };
 
@@ -141,7 +146,7 @@ describe('DevServer', () => {
     testFixture(
       'now-dev-next',
       async server => {
-        const res = await fetch(`${server.address}/something?url-param=a`);
+        const res = await fetch(`${server.address}/test?url-param=a`);
         validateResponseHeaders(res);
 
         const text = await res.text();
@@ -149,16 +154,19 @@ describe('DevServer', () => {
         // Hacky way of getting the page payload from the response
         // HTML since we don't have a HTML parser handy.
         const json = text
-          .match(/<div>(.*)<\/div>/)![1]
+          .match(/<pre>(.*)<\/pre>/)![1]
-          .replace('</div>', '')
+          .replace('</pre>', '')
+          .replace('<!-- -->', '')
+          .replace(/&amp;/g, '&')
           .replace(/&quot;/g, '"');
         const parsed = JSON.parse(json);
+        const query = url.parse(parsed.url, true).query;
 
-        expect(parsed.query['url-param']).toEqual('a');
+        expect(query['url-param']).toEqual('a');
-        expect(parsed.query['route-param']).toEqual('b');
+        expect(query['route-param']).toEqual('b');
       },
       {
-        skip: IS_MAC_OS,
+        devCommand: 'next dev --port $PORT',
       }
     )
   );
@@ -215,8 +223,7 @@ describe('DevServer', () => {
         expect(body).toEqual('hello and welcome');
       },
       {
-        // this test very often times out in the testFixture startup logic on Mac
+        devCommand: 'next dev --port $PORT',
-        skip: IS_MAC_OS,
       }
     )
   );