From 72c18a0ce57eb054583be645fd495f0223287229 Mon Sep 17 00:00:00 2001
From: dominikg <dominik.goepel@gmx.de>
Date: Thu, 23 Sep 2021 21:09:14 +0200
Subject: [PATCH 01/17] test(cli): add playground with tests for cli

---
 packages/playground/cli/__tests__/cli.spec.ts |  18 +++
 packages/playground/cli/__tests__/serve.js    | 148 ++++++++++++++++++
 packages/playground/cli/index.html            |   3 +
 packages/playground/cli/index.js              |   1 +
 packages/playground/cli/package.json          |  11 ++
 packages/playground/cli/vite.config.js        |   9 ++
 pnpm-lock.yaml                                |   3 +
 7 files changed, 193 insertions(+)
 create mode 100644 packages/playground/cli/__tests__/cli.spec.ts
 create mode 100644 packages/playground/cli/__tests__/serve.js
 create mode 100644 packages/playground/cli/index.html
 create mode 100644 packages/playground/cli/index.js
 create mode 100644 packages/playground/cli/package.json
 create mode 100644 packages/playground/cli/vite.config.js

diff --git a/packages/playground/cli/__tests__/cli.spec.ts b/packages/playground/cli/__tests__/cli.spec.ts
new file mode 100644
index 00000000000000..3b735faef95ddf
--- /dev/null
+++ b/packages/playground/cli/__tests__/cli.spec.ts
@@ -0,0 +1,18 @@
+import { port } from './serve'
+
+test('cli should work', async () => {
+  // this test uses a custom serve implementation, so regular helpers for browserLogs and goto don't work
+  // do the same thing manually
+  const logs = []
+  const onConsole = (msg) => {
+    logs.push(msg.text())
+  }
+  try {
+    page.on('console', onConsole)
+    await page.goto(`http://localhost:${port}/`)
+    expect(await page.textContent('.app')).toBe('vite cli works!')
+    expect(logs.some((msg) => msg.match('vite cli works!'))).toBe(true)
+  } finally {
+    page.off('console', onConsole)
+  }
+})
diff --git a/packages/playground/cli/__tests__/serve.js b/packages/playground/cli/__tests__/serve.js
new file mode 100644
index 00000000000000..f0b0d68f6aed50
--- /dev/null
+++ b/packages/playground/cli/__tests__/serve.js
@@ -0,0 +1,148 @@
+// @ts-check
+// this is automtically detected by scripts/jestPerTestSetup.ts and will replace
+// the default e2e test serve behavior
+
+// eslint-disable-next-line node/no-restricted-require
+const execa = require('execa')
+
+// make sure this port is unique
+const port = (exports.port = 9510)
+
+/**
+ * @param {string} root
+ * @param {boolean} isProd
+ */
+exports.serve = async function serve(root, isProd) {
+  // collect stdout and stderr streams from child processes here to avoid interfering with regular jest output
+  const streams = {
+    build: { out: [], err: [] },
+    server: { out: [], err: [] }
+  }
+  // helpers to collect streams
+  const collectStreams = (name, process) => {
+    process.stdout.on('data', (d) => streams[name].out.push(d.toString()))
+    process.stderr.on('data', (d) => streams[name].err.push(d.toString()))
+  }
+  const collectErrorStreams = (name, e) => {
+    e.stdout && streams[name].out.push(e.stdout)
+    e.stderr && streams[name].err.push(e.stderr)
+  }
+
+  // helper to output stream content on error
+  const printStreamsToConsole = async (name) => {
+    const std = streams[name]
+    if (std.out && std.out.length > 0) {
+      console.log(`stdout of ${name}\n${std.out.join('\n')}\n`)
+    }
+    if (std.err && std.err.length > 0) {
+      console.log(`stderr of ${name}\n${std.err.join('\n')}\n`)
+    }
+  }
+
+  // only run `vite build` when needed
+  if (isProd) {
+    try {
+      const buildProcess = execa('vite', ['build'], {
+        preferLocal: true,
+        cwd: root,
+        stdio: 'pipe'
+      })
+      collectStreams('build', buildProcess)
+      await buildProcess
+    } catch (e) {
+      collectErrorStreams('build', e)
+      await printStreamsToConsole('build')
+      throw e
+    }
+  }
+
+  // run `vite --port x` or `vite preview --port x` to start server
+  const serverProcess = execa(
+    'vite',
+    [isProd ? 'preview' : '', '--port', `${port}`],
+    {
+      preferLocal: true,
+      cwd: root,
+      stdio: 'pipe'
+    }
+  )
+  collectStreams('server', serverProcess)
+
+  // close server helper, send SIGTERM followed by SIGKILL if needed, give up after 3sec
+  const close = async () => {
+    if (serverProcess) {
+      let timer
+      const timerPromise = new Promise(
+        (_, reject) =>
+          (timer = setTimeout(() => {
+            reject(`server process still alive after 3s`)
+          }, 3000))
+      )
+
+      serverProcess.kill('SIGTERM', { forceKillAfterTimeout: 2000 })
+
+      try {
+        await Promise.race([serverProcess, timerPromise]).finally(() => {
+          clearTimeout(timer)
+        })
+      } catch (e) {
+        if (!e.killed) {
+          collectErrorStreams('server', e)
+          console.error('failed to end vite cli process', e)
+          await printStreamsToConsole('server')
+        }
+      }
+    }
+  }
+
+  try {
+    await startedOnPort(serverProcess, port, 3000)
+    return {
+      close
+    }
+  } catch (e) {
+    console.error('failed to start server', e)
+    try {
+      await close()
+    } catch (e1) {
+      console.error('failed to close server process', e1)
+    }
+  }
+}
+
+// helper to validate that server was started on the correct port
+async function startedOnPort(serverProcess, port, timeout) {
+  let id
+  let checkPort
+  const timerPromise = new Promise(
+    (_, reject) =>
+      (id = setTimeout(() => {
+        reject(`timeout for server start after ${timeout}`)
+      }, timeout))
+  )
+  const startedPromise = new Promise((resolve, reject) => {
+    checkPort = (data) => {
+      const str = data.toString()
+      // hack, console output may contain color code gibberish
+      // skip gibberish between localhost: and port number
+      const match = str.match(/(http:\/\/localhost:)(?:.*)(\d{4})/)
+      if (match) {
+        const startedPort = parseInt(match[2], 10)
+        if (startedPort === port) {
+          resolve()
+        } else {
+          const msg = `test server started on ${startedPort} instead of ${port}`
+          console.log(msg)
+          reject(msg)
+        }
+      }
+    }
+
+    serverProcess.stdout.on('data', checkPort)
+  })
+
+  return Promise.race([timerPromise, startedPromise]).finally(() => {
+    serverProcess.stdout.off('data', checkPort)
+    clearTimeout(id)
+  })
+}
diff --git a/packages/playground/cli/index.html b/packages/playground/cli/index.html
new file mode 100644
index 00000000000000..9dabaa9ddeb84b
--- /dev/null
+++ b/packages/playground/cli/index.html
@@ -0,0 +1,3 @@
+<script type="module" src="./index.js"></script>
+
+<div class="app">vite cli works!</div>
diff --git a/packages/playground/cli/index.js b/packages/playground/cli/index.js
new file mode 100644
index 00000000000000..6b6158ca27aaf8
--- /dev/null
+++ b/packages/playground/cli/index.js
@@ -0,0 +1 @@
+console.log('vite cli works!')
diff --git a/packages/playground/cli/package.json b/packages/playground/cli/package.json
new file mode 100644
index 00000000000000..0518f1fd930210
--- /dev/null
+++ b/packages/playground/cli/package.json
@@ -0,0 +1,11 @@
+{
+  "name": "test-cli",
+  "private": true,
+  "version": "0.0.0",
+  "scripts": {
+    "dev": "vite",
+    "build": "vite build",
+    "debug": "node --inspect-brk ../../vite/bin/vite",
+    "serve": "vite preview"
+  }
+}
diff --git a/packages/playground/cli/vite.config.js b/packages/playground/cli/vite.config.js
new file mode 100644
index 00000000000000..d91f796bb369d8
--- /dev/null
+++ b/packages/playground/cli/vite.config.js
@@ -0,0 +1,9 @@
+const { defineConfig } = require('vite')
+
+module.exports = defineConfig({
+  build: {
+    //speed up build
+    minify: false,
+    target: 'esnext'
+  }
+})
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
index 325d637a43700f..5a3488b3a4033a 100644
--- a/pnpm-lock.yaml
+++ b/pnpm-lock.yaml
@@ -120,6 +120,9 @@ importers:
     dependencies:
       tailwindcss: 2.2.15_ts-node@10.2.1
 
+  packages/playground/cli:
+    specifiers: {}
+
   packages/playground/css:
     specifiers:
       css-dep: link:./css-dep

From 5cf68defc19cc62fcc1f568e33b1382fd6a5d8d9 Mon Sep 17 00:00:00 2001
From: dominikg <dominik.goepel@gmx.de>
Date: Sat, 25 Sep 2021 16:48:04 +0200
Subject: [PATCH 02/17] fix: improve check condition if cli process kill was
 successful

---
 packages/playground/cli/__tests__/serve.js | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/packages/playground/cli/__tests__/serve.js b/packages/playground/cli/__tests__/serve.js
index f0b0d68f6aed50..9530b07fa46eef 100644
--- a/packages/playground/cli/__tests__/serve.js
+++ b/packages/playground/cli/__tests__/serve.js
@@ -68,27 +68,31 @@ exports.serve = async function serve(root, isProd) {
   )
   collectStreams('server', serverProcess)
 
-  // close server helper, send SIGTERM followed by SIGKILL if needed, give up after 3sec
+  // close server helper, send SIGTERM followed by SIGKILL if needed, give up after a timeout of 5 seconds
   const close = async () => {
+    const killTimeoutSeconds = 5
+    const killTimeoutMsg = `server process still alive ${killTimeoutSeconds}s after killing it`
     if (serverProcess) {
       let timer
       const timerPromise = new Promise(
         (_, reject) =>
           (timer = setTimeout(() => {
-            reject(`server process still alive after 3s`)
-          }, 3000))
+            reject(killTimeoutMsg)
+          }, 1000 * killTimeoutSeconds))
       )
 
-      serverProcess.kill('SIGTERM', { forceKillAfterTimeout: 2000 })
+      serverProcess.kill('SIGTERM', {
+        forceKillAfterTimeout: (killTimeoutSeconds - 2) * 1000
+      })
 
       try {
         await Promise.race([serverProcess, timerPromise]).finally(() => {
           clearTimeout(timer)
         })
       } catch (e) {
-        if (!e.killed) {
+        if (!serverProcess.killed || e === killTimeoutMsg) {
           collectErrorStreams('server', e)
-          console.error('failed to end vite cli process', e)
+          console.error('failed to end vite cli process:', e)
           await printStreamsToConsole('server')
         }
       }

From d59d700eaa24133595c6d2d0c71642d61cffd937 Mon Sep 17 00:00:00 2001
From: dominikg <dominik.goepel@gmx.de>
Date: Sat, 25 Sep 2021 23:38:32 +0200
Subject: [PATCH 03/17] fix: use detached process for cli server and kill it
 with native commands; add promise race helper

---
 packages/playground/cli/__tests__/serve.js | 72 +++++++++++-----------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/packages/playground/cli/__tests__/serve.js b/packages/playground/cli/__tests__/serve.js
index 9530b07fa46eef..03b25fe1c92eff 100644
--- a/packages/playground/cli/__tests__/serve.js
+++ b/packages/playground/cli/__tests__/serve.js
@@ -61,6 +61,7 @@ exports.serve = async function serve(root, isProd) {
     'vite',
     [isProd ? 'preview' : '', '--port', `${port}`],
     {
+      detached: true, // force a new process group so we can kill it with all subprocesses later
       preferLocal: true,
       cwd: root,
       stdio: 'pipe'
@@ -68,29 +69,15 @@ exports.serve = async function serve(root, isProd) {
   )
   collectStreams('server', serverProcess)
 
-  // close server helper, send SIGTERM followed by SIGKILL if needed, give up after a timeout of 5 seconds
+  // close server helper, send SIGKILL to process group. give up after a timeout of 3 seconds
   const close = async () => {
-    const killTimeoutSeconds = 5
-    const killTimeoutMsg = `server process still alive ${killTimeoutSeconds}s after killing it`
     if (serverProcess) {
-      let timer
-      const timerPromise = new Promise(
-        (_, reject) =>
-          (timer = setTimeout(() => {
-            reject(killTimeoutMsg)
-          }, 1000 * killTimeoutSeconds))
-      )
-
-      serverProcess.kill('SIGTERM', {
-        forceKillAfterTimeout: (killTimeoutSeconds - 2) * 1000
-      })
-
+      const killTimeoutMsg = `server process still alive 3s after killing it`
+      killProcessTree(serverProcess.pid)
       try {
-        await Promise.race([serverProcess, timerPromise]).finally(() => {
-          clearTimeout(timer)
-        })
+        await resolvedOrTimoutError(serverProcess, 3000, killTimeoutMsg)
       } catch (e) {
-        if (!serverProcess.killed || e === killTimeoutMsg) {
+        if (e.signal !== 'SIGKILL' || e === killTimeoutMsg) {
           collectErrorStreams('server', e)
           console.error('failed to end vite cli process:', e)
           await printStreamsToConsole('server')
@@ -101,29 +88,22 @@ exports.serve = async function serve(root, isProd) {
 
   try {
     await startedOnPort(serverProcess, port, 3000)
-    return {
-      close
-    }
+    return { close }
   } catch (e) {
-    console.error('failed to start server', e)
+    console.error('failed to start server:', e)
+    // server might have started on a different port, try to close it
     try {
       await close()
     } catch (e1) {
-      console.error('failed to close server process', e1)
+      console.error('failed to close server process:', e1)
     }
+    throw e
   }
 }
 
 // helper to validate that server was started on the correct port
 async function startedOnPort(serverProcess, port, timeout) {
-  let id
   let checkPort
-  const timerPromise = new Promise(
-    (_, reject) =>
-      (id = setTimeout(() => {
-        reject(`timeout for server start after ${timeout}`)
-      }, timeout))
-  )
   const startedPromise = new Promise((resolve, reject) => {
     checkPort = (data) => {
       const str = data.toString()
@@ -136,17 +116,37 @@ async function startedOnPort(serverProcess, port, timeout) {
           resolve()
         } else {
           const msg = `test server started on ${startedPort} instead of ${port}`
-          console.log(msg)
           reject(msg)
         }
       }
     }
-
     serverProcess.stdout.on('data', checkPort)
   })
-
-  return Promise.race([timerPromise, startedPromise]).finally(() => {
+  return resolvedOrTimoutError(
+    startedPromise,
+    3000,
+    'test server failed to start within 3s'
+  ).finally(() => {
     serverProcess.stdout.off('data', checkPort)
-    clearTimeout(id)
+  })
+}
+
+function killProcessTree(pid) {
+  const isWin = process.platform === 'win32'
+  const killServerProcess = isWin
+    ? `taskkill /pid ${pid} /T /F`
+    : `kill -9 -$(ps -o pgid= ${pid} | grep -o '[0-9]*')`
+  execa.commandSync(killServerProcess, { shell: !isWin }) // unix needs shell for $() expansion
+}
+
+async function resolvedOrTimoutError(promise, ms, errorMessage) {
+  let timer
+  return Promise.race([
+    promise,
+    new Promise((_, reject) => {
+      timer = setTimeout(() => reject(errorMessage), ms)
+    })
+  ]).finally(() => {
+    clearTimeout(timer)
   })
 }

From 998128645649ab766ee0723ad3f915e421bb460e Mon Sep 17 00:00:00 2001
From: dominikg <dominik.goepel@gmx.de>
Date: Sat, 25 Sep 2021 23:55:41 +0200
Subject: [PATCH 04/17] fix: remove unused timeout param and increase start
 timeout to 5s

---
 packages/playground/cli/__tests__/serve.js | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/packages/playground/cli/__tests__/serve.js b/packages/playground/cli/__tests__/serve.js
index 03b25fe1c92eff..2857961775e16b 100644
--- a/packages/playground/cli/__tests__/serve.js
+++ b/packages/playground/cli/__tests__/serve.js
@@ -87,7 +87,7 @@ exports.serve = async function serve(root, isProd) {
   }
 
   try {
-    await startedOnPort(serverProcess, port, 3000)
+    await startedOnPort(serverProcess, port)
     return { close }
   } catch (e) {
     console.error('failed to start server:', e)
@@ -102,7 +102,7 @@ exports.serve = async function serve(root, isProd) {
 }
 
 // helper to validate that server was started on the correct port
-async function startedOnPort(serverProcess, port, timeout) {
+async function startedOnPort(serverProcess, port) {
   let checkPort
   const startedPromise = new Promise((resolve, reject) => {
     checkPort = (data) => {
@@ -124,8 +124,8 @@ async function startedOnPort(serverProcess, port, timeout) {
   })
   return resolvedOrTimoutError(
     startedPromise,
-    3000,
-    'test server failed to start within 3s'
+    5000,
+    `test server failed to start within 5s`
   ).finally(() => {
     serverProcess.stdout.off('data', checkPort)
   })

From 4b43e294decfe97b4fc5fbf9a74112c7ed562fe0 Mon Sep 17 00:00:00 2001
From: dominikg <dominik.goepel@gmx.de>
Date: Sun, 26 Sep 2021 01:01:28 +0200
Subject: [PATCH 05/17] wip: see if ipc works

---
 packages/playground/cli/__tests__/serve.js | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/packages/playground/cli/__tests__/serve.js b/packages/playground/cli/__tests__/serve.js
index 2857961775e16b..0b44a93c6cef89 100644
--- a/packages/playground/cli/__tests__/serve.js
+++ b/packages/playground/cli/__tests__/serve.js
@@ -64,7 +64,7 @@ exports.serve = async function serve(root, isProd) {
       detached: true, // force a new process group so we can kill it with all subprocesses later
       preferLocal: true,
       cwd: root,
-      stdio: 'pipe'
+      stdio: ['pipe', 'pipe', 'pipe', 'ipc']
     }
   )
   collectStreams('server', serverProcess)
@@ -82,6 +82,9 @@ exports.serve = async function serve(root, isProd) {
           console.error('failed to end vite cli process:', e)
           await printStreamsToConsole('server')
         }
+      } finally {
+        serverProcess.disconnect()
+        serverProcess.unref()
       }
     }
   }

From fc29624685a0837c2e2dedbddefee9c68716dd58 Mon Sep 17 00:00:00 2001
From: dominikg <dominik.goepel@gmx.de>
Date: Sun, 26 Sep 2021 01:16:44 +0200
Subject: [PATCH 06/17] wip: remove detached: true. only send additional
 taskkill command on windows

---
 packages/playground/cli/__tests__/serve.js | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/packages/playground/cli/__tests__/serve.js b/packages/playground/cli/__tests__/serve.js
index 0b44a93c6cef89..30a16148b538f5 100644
--- a/packages/playground/cli/__tests__/serve.js
+++ b/packages/playground/cli/__tests__/serve.js
@@ -61,10 +61,10 @@ exports.serve = async function serve(root, isProd) {
     'vite',
     [isProd ? 'preview' : '', '--port', `${port}`],
     {
-      detached: true, // force a new process group so we can kill it with all subprocesses later
       preferLocal: true,
       cwd: root,
-      stdio: ['pipe', 'pipe', 'pipe', 'ipc']
+      stdio: 'pipe',
+      cleanup: true
     }
   )
   collectStreams('server', serverProcess)
@@ -73,8 +73,8 @@ exports.serve = async function serve(root, isProd) {
   const close = async () => {
     if (serverProcess) {
       const killTimeoutMsg = `server process still alive 3s after killing it`
-      killProcessTree(serverProcess.pid)
       try {
+        killProcess(serverProcess)
         await resolvedOrTimoutError(serverProcess, 3000, killTimeoutMsg)
       } catch (e) {
         if (e.signal !== 'SIGKILL' || e === killTimeoutMsg) {
@@ -82,9 +82,6 @@ exports.serve = async function serve(root, isProd) {
           console.error('failed to end vite cli process:', e)
           await printStreamsToConsole('server')
         }
-      } finally {
-        serverProcess.disconnect()
-        serverProcess.unref()
       }
     }
   }
@@ -134,12 +131,11 @@ async function startedOnPort(serverProcess, port) {
   })
 }
 
-function killProcessTree(pid) {
-  const isWin = process.platform === 'win32'
-  const killServerProcess = isWin
-    ? `taskkill /pid ${pid} /T /F`
-    : `kill -9 -$(ps -o pgid= ${pid} | grep -o '[0-9]*')`
-  execa.commandSync(killServerProcess, { shell: !isWin }) // unix needs shell for $() expansion
+function killProcess(childProcess) {
+  childProcess.kill('SIGTERM', { forceKillAfterTimeout: 2000 })
+  if (process.platform === 'win32') {
+    execa.commandSync(`taskkill /pid ${childProcess.pid} /T /F`)
+  }
 }
 
 async function resolvedOrTimoutError(promise, ms, errorMessage) {

From 9f2f178aa9f4cc3931e1ef3a6b80512774932e12 Mon Sep 17 00:00:00 2001
From: dominikg <dominik.goepel@gmx.de>
Date: Sun, 26 Sep 2021 02:07:39 +0200
Subject: [PATCH 07/17] wip: add --strict-port

---
 packages/playground/cli/__tests__/serve.js | 33 ++++++++--------------
 1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/packages/playground/cli/__tests__/serve.js b/packages/playground/cli/__tests__/serve.js
index 30a16148b538f5..3dcbe2e0d94b7e 100644
--- a/packages/playground/cli/__tests__/serve.js
+++ b/packages/playground/cli/__tests__/serve.js
@@ -8,6 +8,8 @@ const execa = require('execa')
 // make sure this port is unique
 const port = (exports.port = 9510)
 
+const is_windows = process.platform === 'win32'
+
 /**
  * @param {string} root
  * @param {boolean} isProd
@@ -29,7 +31,7 @@ exports.serve = async function serve(root, isProd) {
   }
 
   // helper to output stream content on error
-  const printStreamsToConsole = async (name) => {
+  const printStreamsToConsole = (name) => {
     const std = streams[name]
     if (std.out && std.out.length > 0) {
       console.log(`stdout of ${name}\n${std.out.join('\n')}\n`)
@@ -51,7 +53,7 @@ exports.serve = async function serve(root, isProd) {
       await buildProcess
     } catch (e) {
       collectErrorStreams('build', e)
-      await printStreamsToConsole('build')
+      printStreamsToConsole('build')
       throw e
     }
   }
@@ -59,12 +61,11 @@ exports.serve = async function serve(root, isProd) {
   // run `vite --port x` or `vite preview --port x` to start server
   const serverProcess = execa(
     'vite',
-    [isProd ? 'preview' : '', '--port', `${port}`],
+    [isProd ? 'preview' : '', '--port', `${port}`, '--strict-port'],
     {
       preferLocal: true,
       cwd: root,
-      stdio: 'pipe',
-      cleanup: true
+      stdio: 'pipe'
     }
   )
   collectStreams('server', serverProcess)
@@ -77,28 +78,16 @@ exports.serve = async function serve(root, isProd) {
         killProcess(serverProcess)
         await resolvedOrTimoutError(serverProcess, 3000, killTimeoutMsg)
       } catch (e) {
-        if (e.signal !== 'SIGKILL' || e === killTimeoutMsg) {
+        if ((!is_windows && !e.killed) || e === killTimeoutMsg) {
           collectErrorStreams('server', e)
+          printStreamsToConsole('server')
           console.error('failed to end vite cli process:', e)
-          await printStreamsToConsole('server')
         }
       }
     }
   }
-
-  try {
-    await startedOnPort(serverProcess, port)
-    return { close }
-  } catch (e) {
-    console.error('failed to start server:', e)
-    // server might have started on a different port, try to close it
-    try {
-      await close()
-    } catch (e1) {
-      console.error('failed to close server process:', e1)
-    }
-    throw e
-  }
+  await startedOnPort(serverProcess, port)
+  return { close }
 }
 
 // helper to validate that server was started on the correct port
@@ -133,7 +122,7 @@ async function startedOnPort(serverProcess, port) {
 
 function killProcess(childProcess) {
   childProcess.kill('SIGTERM', { forceKillAfterTimeout: 2000 })
-  if (process.platform === 'win32') {
+  if (is_windows) {
     execa.commandSync(`taskkill /pid ${childProcess.pid} /T /F`)
   }
 }

From 11c21835b1b7511f4fcef90402cb5d2dff2106cb Mon Sep 17 00:00:00 2001
From: dominikg <dominik.goepel@gmx.de>
Date: Sun, 26 Sep 2021 02:24:31 +0200
Subject: [PATCH 08/17] refactor: remove nested finally and add back error log
 for failed server start

---
 packages/playground/cli/__tests__/serve.js | 24 ++++++++++++++--------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/packages/playground/cli/__tests__/serve.js b/packages/playground/cli/__tests__/serve.js
index 3dcbe2e0d94b7e..791cee8a3acf64 100644
--- a/packages/playground/cli/__tests__/serve.js
+++ b/packages/playground/cli/__tests__/serve.js
@@ -86,14 +86,25 @@ exports.serve = async function serve(root, isProd) {
       }
     }
   }
-  await startedOnPort(serverProcess, port)
+  try {
+    await resolvedOrTimoutError(
+      startedOnPort(serverProcess, port),
+      5000,
+      `test server failed to start within 5s`
+    )
+  } catch (e) {
+    collectErrorStreams('server', e)
+    printStreamsToConsole('server')
+    throw e
+  }
+
   return { close }
 }
 
 // helper to validate that server was started on the correct port
 async function startedOnPort(serverProcess, port) {
   let checkPort
-  const startedPromise = new Promise((resolve, reject) => {
+  return new Promise((resolve, reject) => {
     checkPort = (data) => {
       const str = data.toString()
       // hack, console output may contain color code gibberish
@@ -102,8 +113,10 @@ async function startedOnPort(serverProcess, port) {
       if (match) {
         const startedPort = parseInt(match[2], 10)
         if (startedPort === port) {
+          serverProcess.stdout.off('data', checkPort)
           resolve()
         } else {
+          serverProcess.stdout.off('data', checkPort)
           const msg = `test server started on ${startedPort} instead of ${port}`
           reject(msg)
         }
@@ -111,13 +124,6 @@ async function startedOnPort(serverProcess, port) {
     }
     serverProcess.stdout.on('data', checkPort)
   })
-  return resolvedOrTimoutError(
-    startedPromise,
-    5000,
-    `test server failed to start within 5s`
-  ).finally(() => {
-    serverProcess.stdout.off('data', checkPort)
-  })
 }
 
 function killProcess(childProcess) {

From e58be2a69448fc83001aaf93564b7eeb7f8e5abf Mon Sep 17 00:00:00 2001
From: dominikg <dominik.goepel@gmx.de>
Date: Sun, 26 Sep 2021 08:22:36 +0200
Subject: [PATCH 09/17] wip: log commands used on error

---
 packages/playground/cli/__tests__/serve.js | 29 ++++++++++++----------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/packages/playground/cli/__tests__/serve.js b/packages/playground/cli/__tests__/serve.js
index 791cee8a3acf64..506fc523a8555e 100644
--- a/packages/playground/cli/__tests__/serve.js
+++ b/packages/playground/cli/__tests__/serve.js
@@ -43,8 +43,9 @@ exports.serve = async function serve(root, isProd) {
 
   // only run `vite build` when needed
   if (isProd) {
+    const buildCommand = `vite build`
     try {
-      const buildProcess = execa('vite', ['build'], {
+      const buildProcess = execa.command(buildCommand, {
         preferLocal: true,
         cwd: root,
         stdio: 'pipe'
@@ -52,25 +53,23 @@ exports.serve = async function serve(root, isProd) {
       collectStreams('build', buildProcess)
       await buildProcess
     } catch (e) {
+      console.error(`failed to run vite cli process : "${buildCommand}"`)
       collectErrorStreams('build', e)
       printStreamsToConsole('build')
       throw e
     }
   }
 
-  // run `vite --port x` or `vite preview --port x` to start server
-  const serverProcess = execa(
-    'vite',
-    [isProd ? 'preview' : '', '--port', `${port}`, '--strict-port'],
-    {
-      preferLocal: true,
-      cwd: root,
-      stdio: 'pipe'
-    }
-  )
+  const serverCommand = isProd
+    ? `vite preview --port ${port} --strict-port`
+    : `vite --port ${port} --strict-port`
+  const serverProcess = execa.command(serverCommand, {
+    preferLocal: true,
+    cwd: root,
+    stdio: 'pipe'
+  })
   collectStreams('server', serverProcess)
 
-  // close server helper, send SIGKILL to process group. give up after a timeout of 3 seconds
   const close = async () => {
     if (serverProcess) {
       const killTimeoutMsg = `server process still alive 3s after killing it`
@@ -81,7 +80,10 @@ exports.serve = async function serve(root, isProd) {
         if ((!is_windows && !e.killed) || e === killTimeoutMsg) {
           collectErrorStreams('server', e)
           printStreamsToConsole('server')
-          console.error('failed to end vite cli process:', e)
+          console.error(
+            `failed to end vite cli process : "${serverCommand}"`,
+            e
+          )
         }
       }
     }
@@ -93,6 +95,7 @@ exports.serve = async function serve(root, isProd) {
       `test server failed to start within 5s`
     )
   } catch (e) {
+    console.error(`failed to start vite cli process : "${serverCommand}"`)
     collectErrorStreams('server', e)
     printStreamsToConsole('server')
     throw e

From fecf5da68cc1b1541e2cf3aab76e65fa6de0082c Mon Sep 17 00:00:00 2001
From: dominikg <dominik.goepel@gmx.de>
Date: Sun, 26 Sep 2021 08:42:37 +0200
Subject: [PATCH 10/17] wip: revert to state that ran successfully on
 linux/mac, add extra taskkill for windows only

---
 packages/playground/cli/__tests__/serve.js | 114 +++++++++++----------
 1 file changed, 59 insertions(+), 55 deletions(-)

diff --git a/packages/playground/cli/__tests__/serve.js b/packages/playground/cli/__tests__/serve.js
index 506fc523a8555e..1e6b1f84171897 100644
--- a/packages/playground/cli/__tests__/serve.js
+++ b/packages/playground/cli/__tests__/serve.js
@@ -8,7 +8,7 @@ const execa = require('execa')
 // make sure this port is unique
 const port = (exports.port = 9510)
 
-const is_windows = process.platform === 'win32'
+const isWindows = process.platform === 'win32'
 
 /**
  * @param {string} root
@@ -31,7 +31,7 @@ exports.serve = async function serve(root, isProd) {
   }
 
   // helper to output stream content on error
-  const printStreamsToConsole = (name) => {
+  const printStreamsToConsole = async (name) => {
     const std = streams[name]
     if (std.out && std.out.length > 0) {
       console.log(`stdout of ${name}\n${std.out.join('\n')}\n`)
@@ -43,9 +43,8 @@ exports.serve = async function serve(root, isProd) {
 
   // only run `vite build` when needed
   if (isProd) {
-    const buildCommand = `vite build`
     try {
-      const buildProcess = execa.command(buildCommand, {
+      const buildProcess = execa('vite', ['build'], {
         preferLocal: true,
         cwd: root,
         stdio: 'pipe'
@@ -53,61 +52,80 @@ exports.serve = async function serve(root, isProd) {
       collectStreams('build', buildProcess)
       await buildProcess
     } catch (e) {
-      console.error(`failed to run vite cli process : "${buildCommand}"`)
       collectErrorStreams('build', e)
-      printStreamsToConsole('build')
+      await printStreamsToConsole('build')
       throw e
     }
   }
 
-  const serverCommand = isProd
-    ? `vite preview --port ${port} --strict-port`
-    : `vite --port ${port} --strict-port`
-  const serverProcess = execa.command(serverCommand, {
-    preferLocal: true,
-    cwd: root,
-    stdio: 'pipe'
-  })
+  // run `vite --port x` or `vite preview --port x` to start server
+  const serverProcess = execa(
+    'vite',
+    [isProd ? 'preview' : '', '--port', `${port}`],
+    {
+      preferLocal: true,
+      cwd: root,
+      stdio: 'pipe'
+    }
+  )
   collectStreams('server', serverProcess)
 
+  // close server helper, send SIGTERM followed by SIGKILL if needed, give up after 3sec
   const close = async () => {
     if (serverProcess) {
-      const killTimeoutMsg = `server process still alive 3s after killing it`
+      let timer
+      const timerPromise = new Promise(
+        (_, reject) =>
+          (timer = setTimeout(() => {
+            reject(`server process still alive after 3s`)
+          }, 3000))
+      )
+
+      serverProcess.kill('SIGTERM', { forceKillAfterTimeout: 2000 })
+      if (isWindows) {
+        execa.commandSync(`taskkill /pid ${serverProcess.pid} /T /F`)
+      }
+
       try {
-        killProcess(serverProcess)
-        await resolvedOrTimoutError(serverProcess, 3000, killTimeoutMsg)
+        await Promise.race([serverProcess, timerPromise]).finally(() => {
+          clearTimeout(timer)
+        })
       } catch (e) {
-        if ((!is_windows && !e.killed) || e === killTimeoutMsg) {
+        if (!e.killed && !isWindows) {
           collectErrorStreams('server', e)
-          printStreamsToConsole('server')
-          console.error(
-            `failed to end vite cli process : "${serverCommand}"`,
-            e
-          )
+          console.error('failed to end vite cli process', e)
+          await printStreamsToConsole('server')
         }
       }
     }
   }
+
   try {
-    await resolvedOrTimoutError(
-      startedOnPort(serverProcess, port),
-      5000,
-      `test server failed to start within 5s`
-    )
+    await startedOnPort(serverProcess, port, 3000)
+    return {
+      close
+    }
   } catch (e) {
-    console.error(`failed to start vite cli process : "${serverCommand}"`)
-    collectErrorStreams('server', e)
-    printStreamsToConsole('server')
-    throw e
+    console.error('failed to start server', e)
+    try {
+      await close()
+    } catch (e1) {
+      console.error('failed to close server process', e1)
+    }
   }
-
-  return { close }
 }
 
 // helper to validate that server was started on the correct port
-async function startedOnPort(serverProcess, port) {
+async function startedOnPort(serverProcess, port, timeout) {
+  let id
   let checkPort
-  return new Promise((resolve, reject) => {
+  const timerPromise = new Promise(
+    (_, reject) =>
+      (id = setTimeout(() => {
+        reject(`timeout for server start after ${timeout}`)
+      }, timeout))
+  )
+  const startedPromise = new Promise((resolve, reject) => {
     checkPort = (data) => {
       const str = data.toString()
       // hack, console output may contain color code gibberish
@@ -116,34 +134,20 @@ async function startedOnPort(serverProcess, port) {
       if (match) {
         const startedPort = parseInt(match[2], 10)
         if (startedPort === port) {
-          serverProcess.stdout.off('data', checkPort)
           resolve()
         } else {
-          serverProcess.stdout.off('data', checkPort)
           const msg = `test server started on ${startedPort} instead of ${port}`
+          console.log(msg)
           reject(msg)
         }
       }
     }
+
     serverProcess.stdout.on('data', checkPort)
   })
-}
-
-function killProcess(childProcess) {
-  childProcess.kill('SIGTERM', { forceKillAfterTimeout: 2000 })
-  if (is_windows) {
-    execa.commandSync(`taskkill /pid ${childProcess.pid} /T /F`)
-  }
-}
 
-async function resolvedOrTimoutError(promise, ms, errorMessage) {
-  let timer
-  return Promise.race([
-    promise,
-    new Promise((_, reject) => {
-      timer = setTimeout(() => reject(errorMessage), ms)
-    })
-  ]).finally(() => {
-    clearTimeout(timer)
+  return Promise.race([timerPromise, startedPromise]).finally(() => {
+    serverProcess.stdout.off('data', checkPort)
+    clearTimeout(id)
   })
 }

From 0a3c3a3aef4df088311755808e7acb98ff7844e3 Mon Sep 17 00:00:00 2001
From: dominikg <dominik.goepel@gmx.de>
Date: Sun, 26 Sep 2021 09:17:39 +0200
Subject: [PATCH 11/17] wip: don't rely on execa preferLocal, call vite bin
 with relative path

---
 packages/playground/cli/__tests__/serve.js | 28 +++++++++++++---------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/packages/playground/cli/__tests__/serve.js b/packages/playground/cli/__tests__/serve.js
index 1e6b1f84171897..a3af4878f1674b 100644
--- a/packages/playground/cli/__tests__/serve.js
+++ b/packages/playground/cli/__tests__/serve.js
@@ -2,6 +2,7 @@
 // this is automtically detected by scripts/jestPerTestSetup.ts and will replace
 // the default e2e test serve behavior
 
+const path = require('path')
 // eslint-disable-next-line node/no-restricted-require
 const execa = require('execa')
 
@@ -15,6 +16,13 @@ const isWindows = process.platform === 'win32'
  * @param {boolean} isProd
  */
 exports.serve = async function serve(root, isProd) {
+  const viteBin = path.join(
+    path.relative(root, process.cwd()),
+    'packages',
+    'vite',
+    'bin',
+    'vite.js'
+  )
   // collect stdout and stderr streams from child processes here to avoid interfering with regular jest output
   const streams = {
     build: { out: [], err: [] },
@@ -44,8 +52,7 @@ exports.serve = async function serve(root, isProd) {
   // only run `vite build` when needed
   if (isProd) {
     try {
-      const buildProcess = execa('vite', ['build'], {
-        preferLocal: true,
+      const buildProcess = execa(viteBin, ['build'], {
         cwd: root,
         stdio: 'pipe'
       })
@@ -59,15 +66,14 @@ exports.serve = async function serve(root, isProd) {
   }
 
   // run `vite --port x` or `vite preview --port x` to start server
-  const serverProcess = execa(
-    'vite',
-    [isProd ? 'preview' : '', '--port', `${port}`],
-    {
-      preferLocal: true,
-      cwd: root,
-      stdio: 'pipe'
-    }
-  )
+  const viteServerArgs = ['--port', `${port}`, '--strict-port']
+  if (isProd) {
+    viteServerArgs.unshift('preview')
+  }
+  const serverProcess = execa('vite', viteServerArgs, {
+    cwd: root,
+    stdio: 'pipe'
+  })
   collectStreams('server', serverProcess)
 
   // close server helper, send SIGTERM followed by SIGKILL if needed, give up after 3sec

From 0c5218d42e1f0e41c96a7c83216c376788c74603 Mon Sep 17 00:00:00 2001
From: dominikg <dominik.goepel@gmx.de>
Date: Sun, 26 Sep 2021 09:25:39 +0200
Subject: [PATCH 12/17] fix: use viteBin for server process too

---
 packages/playground/cli/__tests__/serve.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/playground/cli/__tests__/serve.js b/packages/playground/cli/__tests__/serve.js
index a3af4878f1674b..dc0cdc12c53e50 100644
--- a/packages/playground/cli/__tests__/serve.js
+++ b/packages/playground/cli/__tests__/serve.js
@@ -70,7 +70,7 @@ exports.serve = async function serve(root, isProd) {
   if (isProd) {
     viteServerArgs.unshift('preview')
   }
-  const serverProcess = execa('vite', viteServerArgs, {
+  const serverProcess = execa(viteBin, viteServerArgs, {
     cwd: root,
     stdio: 'pipe'
   })

From 9a02e48bdaf25e9262940edc693cbf68b9e77c4e Mon Sep 17 00:00:00 2001
From: dominikg <dominik.goepel@gmx.de>
Date: Sun, 26 Sep 2021 11:31:37 +0200
Subject: [PATCH 13/17] fix: refactor postcss-caching test to not call
 process.chdir and silence server logs

---
 .../css/postcss-caching/css.spec.ts           | 61 +++++++++++++------
 1 file changed, 43 insertions(+), 18 deletions(-)

diff --git a/packages/playground/css/postcss-caching/css.spec.ts b/packages/playground/css/postcss-caching/css.spec.ts
index 835538666db7bb..6c85d127003680 100644
--- a/packages/playground/css/postcss-caching/css.spec.ts
+++ b/packages/playground/css/postcss-caching/css.spec.ts
@@ -4,26 +4,51 @@ import path from 'path'
 
 test('postcss config', async () => {
   const port = 5005
+  const startServer = async (root) => {
+    const server = await createServer({
+      root,
+      logLevel: 'silent',
+      server: {
+        port,
+        strictPort: true
+      },
+      build: {
+        // skip transpilation during tests to make it faster
+        target: 'esnext'
+      }
+    })
+    await server.listen()
+    return server
+  }
   const blueAppDir = path.join(__dirname, 'blue-app')
   const greenAppDir = path.join(__dirname, 'green-app')
+  let blueApp
+  let greenApp
+  try {
+    blueApp = await startServer(blueAppDir)
 
-  process.chdir(blueAppDir)
-  const blueApp = await createServer()
-  await blueApp.listen(port)
-  await page.goto(`http://localhost:${port}`)
-  const blueA = await page.$('.postcss-a')
-  expect(await getColor(blueA)).toBe('blue')
-  const blueB = await page.$('.postcss-b')
-  expect(await getColor(blueB)).toBe('black')
-  await blueApp.close()
+    await page.goto(`http://localhost:${port}`)
+    const blueA = await page.$('.postcss-a')
+    expect(await getColor(blueA)).toBe('blue')
+    const blueB = await page.$('.postcss-b')
+    expect(await getColor(blueB)).toBe('black')
+    await blueApp.close()
+    blueApp = null
 
-  process.chdir(greenAppDir)
-  const greenApp = await createServer()
-  await greenApp.listen(port)
-  await page.goto(`http://localhost:${port}`)
-  const greenA = await page.$('.postcss-a')
-  expect(await getColor(greenA)).toBe('black')
-  const greenB = await page.$('.postcss-b')
-  expect(await getColor(greenB)).toBe('green')
-  await greenApp.close()
+    greenApp = await startServer(greenAppDir)
+    await page.goto(`http://localhost:${port}`)
+    const greenA = await page.$('.postcss-a')
+    expect(await getColor(greenA)).toBe('black')
+    const greenB = await page.$('.postcss-b')
+    expect(await getColor(greenB)).toBe('green')
+    await greenApp.close()
+    greenApp = null
+  } finally {
+    if (blueApp) {
+      await blueApp.close()
+    }
+    if (greenApp) {
+      await greenApp.close()
+    }
+  }
 })

From 6b54ce7b6948d2012a64d4a40b3b82a3c8e4a673 Mon Sep 17 00:00:00 2001
From: dominikg <dominik.goepel@gmx.de>
Date: Sun, 26 Sep 2021 12:14:42 +0200
Subject: [PATCH 14/17] fix: enforce localhost for cli test and avoid logger
 error when trying to log network details

---
 packages/playground/cli/vite.config.js | 3 +++
 packages/vite/src/node/logger.ts       | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/packages/playground/cli/vite.config.js b/packages/playground/cli/vite.config.js
index d91f796bb369d8..014aa6d378934b 100644
--- a/packages/playground/cli/vite.config.js
+++ b/packages/playground/cli/vite.config.js
@@ -1,6 +1,9 @@
 const { defineConfig } = require('vite')
 
 module.exports = defineConfig({
+  server: {
+    host: 'localhost'
+  },
   build: {
     //speed up build
     minify: false,
diff --git a/packages/vite/src/node/logger.ts b/packages/vite/src/node/logger.ts
index a6a6ae279f71e2..7720766041f03d 100644
--- a/packages/vite/src/node/logger.ts
+++ b/packages/vite/src/node/logger.ts
@@ -176,7 +176,7 @@ export function printServerUrls(
   } else {
     Object.values(os.networkInterfaces())
       .flatMap((nInterface) => nInterface ?? [])
-      .filter((detail) => detail.family === 'IPv4')
+      .filter((detail) => detail && detail.address && detail.family === 'IPv4')
       .map((detail) => {
         const type = detail.address.includes('127.0.0.1')
           ? 'Local:   '

From 29dd8d16276cccf3542ac586065332b604d26ada Mon Sep 17 00:00:00 2001
From: dominikg <dominik.goepel@gmx.de>
Date: Sun, 26 Sep 2021 14:22:11 +0200
Subject: [PATCH 15/17] fix: call server.listen() before accessing
 server.address() to get the actual address; restore original for startup
 duration

---
 packages/vite/src/node/cli.ts    | 13 +++++--------
 packages/vite/src/node/logger.ts |  2 +-
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/packages/vite/src/node/cli.ts b/packages/vite/src/node/cli.ts
index 605c274cba2e58..1166b48f35f857 100644
--- a/packages/vite/src/node/cli.ts
+++ b/packages/vite/src/node/cli.ts
@@ -106,19 +106,16 @@ cli
         throw new Error('HTTP server not available')
       }
 
+      await server.listen()
+
       printHttpServerUrls(server.httpServer, server.config, options)
 
       // @ts-ignore
       if (global.__vite_start_time) {
-        info(
-          chalk.cyan(
-            // @ts-ignore
-            performance.now() - global.__vite_start_time
-          )
-        )
+        // @ts-ignore
+        const startupDuration = performance.now() - global.__vite_start_time
+        info(`\n  ${chalk.cyan(`ready in ${Math.ceil(startupDuration)}ms.`)}\n`)
       }
-
-      await server.listen()
     } catch (e) {
       createLogger(options.logLevel).error(
         chalk.red(`error when starting dev server:\n${e.stack}`),
diff --git a/packages/vite/src/node/logger.ts b/packages/vite/src/node/logger.ts
index 7720766041f03d..5ba5adf7c63054 100644
--- a/packages/vite/src/node/logger.ts
+++ b/packages/vite/src/node/logger.ts
@@ -160,7 +160,7 @@ export function printHttpServerUrls(
   }
 }
 
-export function printServerUrls(
+function printServerUrls(
   hostname: Hostname,
   protocol: string,
   port: number,

From bdd36bd648de2ccb6ffe953c03a5df68e987f0ea Mon Sep 17 00:00:00 2001
From: dominikg <dominik.goepel@gmx.de>
Date: Sun, 26 Sep 2021 15:07:10 +0200
Subject: [PATCH 16/17] wip: and another round of windows improvement

---
 packages/playground/cli/__tests__/serve.js | 30 ++++++++++++++++------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/packages/playground/cli/__tests__/serve.js b/packages/playground/cli/__tests__/serve.js
index dc0cdc12c53e50..964b1ee4c59e93 100644
--- a/packages/playground/cli/__tests__/serve.js
+++ b/packages/playground/cli/__tests__/serve.js
@@ -80,24 +80,25 @@ exports.serve = async function serve(root, isProd) {
   const close = async () => {
     if (serverProcess) {
       let timer
+      const timeoutError = `server process still alive after 3s`
       const timerPromise = new Promise(
         (_, reject) =>
           (timer = setTimeout(() => {
-            reject(`server process still alive after 3s`)
+            reject(timeoutError)
           }, 3000))
       )
 
-      serverProcess.kill('SIGTERM', { forceKillAfterTimeout: 2000 })
-      if (isWindows) {
-        execa.commandSync(`taskkill /pid ${serverProcess.pid} /T /F`)
-      }
-
       try {
-        await Promise.race([serverProcess, timerPromise]).finally(() => {
+        const closeTimerRace = Promise.race([
+          serverProcess,
+          timerPromise
+        ]).finally(() => {
           clearTimeout(timer)
         })
+        killProcess(serverProcess)
+        await closeTimerRace
       } catch (e) {
-        if (!e.killed && !isWindows) {
+        if (e === timeoutError || (!serverProcess.killed && !isWindows)) {
           collectErrorStreams('server', e)
           console.error('failed to end vite cli process', e)
           await printStreamsToConsole('server')
@@ -157,3 +158,16 @@ async function startedOnPort(serverProcess, port, timeout) {
     clearTimeout(id)
   })
 }
+
+// helper function to kill process, uses taskkill on windows to ensure child process is killed too
+function killProcess(serverProcess) {
+  if (isWindows) {
+    try {
+      execa.commandSync(`taskkill /pid ${serverProcess.pid} /T /F`)
+    } catch (e) {
+      console.error('failed to taskkill:', e)
+    }
+  } else {
+    serverProcess.kill('SIGTERM', { forceKillAfterTimeout: 2000 })
+  }
+}

From 76b59b92c00cf2358c6384a463facd16ccb2eae7 Mon Sep 17 00:00:00 2001
From: dominikg <dominik.goepel@gmx.de>
Date: Sun, 26 Sep 2021 15:47:06 +0200
Subject: [PATCH 17/17] refactor: cleanup serve implementation in cli
 playground, improve error messages

---
 packages/playground/cli/__tests__/serve.js | 89 ++++++++++------------
 packages/playground/testUtils.ts           |  1 +
 2 files changed, 42 insertions(+), 48 deletions(-)

diff --git a/packages/playground/cli/__tests__/serve.js b/packages/playground/cli/__tests__/serve.js
index 964b1ee4c59e93..5dd058f4e1a83c 100644
--- a/packages/playground/cli/__tests__/serve.js
+++ b/packages/playground/cli/__tests__/serve.js
@@ -5,24 +5,17 @@
 const path = require('path')
 // eslint-disable-next-line node/no-restricted-require
 const execa = require('execa')
-
-// make sure this port is unique
-const port = (exports.port = 9510)
+const { workspaceRoot } = require('../../testUtils')
 
 const isWindows = process.platform === 'win32'
+const port = (exports.port = 9510) // make sure this port is unique across tests with custom servers
+const viteBin = path.join(workspaceRoot, 'packages', 'vite', 'bin', 'vite.js')
 
 /**
  * @param {string} root
  * @param {boolean} isProd
  */
 exports.serve = async function serve(root, isProd) {
-  const viteBin = path.join(
-    path.relative(root, process.cwd()),
-    'packages',
-    'vite',
-    'bin',
-    'vite.js'
-  )
   // collect stdout and stderr streams from child processes here to avoid interfering with regular jest output
   const streams = {
     build: { out: [], err: [] },
@@ -51,14 +44,16 @@ exports.serve = async function serve(root, isProd) {
 
   // only run `vite build` when needed
   if (isProd) {
+    const buildCommand = `${viteBin} build`
     try {
-      const buildProcess = execa(viteBin, ['build'], {
+      const buildProcess = execa.command(buildCommand, {
         cwd: root,
         stdio: 'pipe'
       })
       collectStreams('build', buildProcess)
       await buildProcess
     } catch (e) {
+      console.error(`error while executing cli command "${buildCommand}":`, e)
       collectErrorStreams('build', e)
       await printStreamsToConsole('build')
       throw e
@@ -70,7 +65,8 @@ exports.serve = async function serve(root, isProd) {
   if (isProd) {
     viteServerArgs.unshift('preview')
   }
-  const serverProcess = execa(viteBin, viteServerArgs, {
+  const serverCommand = `${viteBin} ${viteServerArgs.join(' ')}`
+  const serverProcess = execa.command(serverCommand, {
     cwd: root,
     stdio: 'pipe'
   })
@@ -79,28 +75,17 @@ exports.serve = async function serve(root, isProd) {
   // close server helper, send SIGTERM followed by SIGKILL if needed, give up after 3sec
   const close = async () => {
     if (serverProcess) {
-      let timer
       const timeoutError = `server process still alive after 3s`
-      const timerPromise = new Promise(
-        (_, reject) =>
-          (timer = setTimeout(() => {
-            reject(timeoutError)
-          }, 3000))
-      )
-
       try {
-        const closeTimerRace = Promise.race([
-          serverProcess,
-          timerPromise
-        ]).finally(() => {
-          clearTimeout(timer)
-        })
         killProcess(serverProcess)
-        await closeTimerRace
+        await resolvedOrTimeout(serverProcess, 3000, timeoutError)
       } catch (e) {
         if (e === timeoutError || (!serverProcess.killed && !isWindows)) {
           collectErrorStreams('server', e)
-          console.error('failed to end vite cli process', e)
+          console.error(
+            `error while killing cli command "${serverCommand}":`,
+            e
+          )
           await printStreamsToConsole('server')
         }
       }
@@ -109,29 +94,25 @@ exports.serve = async function serve(root, isProd) {
 
   try {
     await startedOnPort(serverProcess, port, 3000)
-    return {
-      close
-    }
+    return { close }
   } catch (e) {
-    console.error('failed to start server', e)
+    collectErrorStreams('server', e)
+    console.error(`error while executing cli command "${serverCommand}":`, e)
+    await printStreamsToConsole('server')
     try {
       await close()
     } catch (e1) {
-      console.error('failed to close server process', e1)
+      console.error(
+        `error while killing cli command after failed execute "${serverCommand}":`,
+        e1
+      )
     }
   }
 }
 
 // helper to validate that server was started on the correct port
 async function startedOnPort(serverProcess, port, timeout) {
-  let id
   let checkPort
-  const timerPromise = new Promise(
-    (_, reject) =>
-      (id = setTimeout(() => {
-        reject(`timeout for server start after ${timeout}`)
-      }, timeout))
-  )
   const startedPromise = new Promise((resolve, reject) => {
     checkPort = (data) => {
       const str = data.toString()
@@ -143,20 +124,18 @@ async function startedOnPort(serverProcess, port, timeout) {
         if (startedPort === port) {
           resolve()
         } else {
-          const msg = `test server started on ${startedPort} instead of ${port}`
-          console.log(msg)
+          const msg = `server listens on port ${startedPort} instead of ${port}`
           reject(msg)
         }
       }
     }
-
     serverProcess.stdout.on('data', checkPort)
   })
-
-  return Promise.race([timerPromise, startedPromise]).finally(() => {
-    serverProcess.stdout.off('data', checkPort)
-    clearTimeout(id)
-  })
+  return resolvedOrTimeout(
+    startedPromise,
+    timeout,
+    `failed to start within ${timeout}ms`
+  ).finally(() => serverProcess.stdout.off('data', checkPort))
 }
 
 // helper function to kill process, uses taskkill on windows to ensure child process is killed too
@@ -171,3 +150,17 @@ function killProcess(serverProcess) {
     serverProcess.kill('SIGTERM', { forceKillAfterTimeout: 2000 })
   }
 }
+
+// helper function that rejects with errorMessage if promise isn't settled within ms
+async function resolvedOrTimeout(promise, ms, errorMessage) {
+  let timer
+  return Promise.race([
+    promise,
+    new Promise((_, reject) => {
+      timer = setTimeout(() => reject(errorMessage), ms)
+    })
+  ]).finally(() => {
+    clearTimeout(timer)
+    timer = null
+  })
+}
diff --git a/packages/playground/testUtils.ts b/packages/playground/testUtils.ts
index f0b54f1e5c972d..2be13b1aee8cc4 100644
--- a/packages/playground/testUtils.ts
+++ b/packages/playground/testUtils.ts
@@ -17,6 +17,7 @@ export const isBuild = !!process.env.VITE_TEST_BUILD
 const testPath = expect.getState().testPath
 const testName = slash(testPath).match(/playground\/([\w-]+)\//)?.[1]
 export const testDir = path.resolve(__dirname, '../../packages/temp', testName)
+export const workspaceRoot = path.resolve(__dirname, '../../')
 
 const hexToNameMap: Record<string, string> = {}
 Object.keys(colors).forEach((color) => {