From ba0364a5228a8a4f3c7ddb37af461428c3a66fc8 Mon Sep 17 00:00:00 2001
From: Asher <ash@coder.com>
Date: Tue, 22 Jun 2021 16:34:11 -0500
Subject: [PATCH 1/8] Run each e2e test in a new workspace

The workspaces also have settings to prevent the welcome page from
appearing.
---
 test/e2e/models/CodeServer.ts | 35 +++++++++++++++++++++++++++--------
 test/playwright.config.ts     |  8 +-------
 test/utils/constants.ts       |  1 +
 test/utils/globalSetup.ts     | 13 +++++++++----
 test/utils/helpers.ts         | 21 ++++++++++++++-------
 5 files changed, 52 insertions(+), 26 deletions(-)

diff --git a/test/e2e/models/CodeServer.ts b/test/e2e/models/CodeServer.ts
index 9ec5151d3..662c82d40 100644
--- a/test/e2e/models/CodeServer.ts
+++ b/test/e2e/models/CodeServer.ts
@@ -1,21 +1,24 @@
+import { promises as fs } from "fs"
+import * as path from "path"
 import { Page } from "playwright"
-import { CODE_SERVER_ADDRESS } from "../../utils/constants"
+import { CODE_SERVER_ADDRESS, workspaceDir } from "../../utils/constants"
+import { tmpdir } from "../../utils/helpers"
+
 // This is a Page Object Model
 // We use these to simplify e2e test authoring
 // See Playwright docs: https://playwright.dev/docs/pom/
 export class CodeServer {
-  page: Page
-  editorSelector = "div.monaco-workbench"
+  private readonly editorSelector = "div.monaco-workbench"
 
-  constructor(page: Page) {
-    this.page = page
-  }
+  constructor(public readonly page: Page) {}
 
   /**
-   * Navigates to CODE_SERVER_ADDRESS
+   * Navigates to CODE_SERVER_ADDRESS. It will open a newly created random
+   * directory.
    */
   async navigate() {
-    await this.page.goto(CODE_SERVER_ADDRESS, { waitUntil: "networkidle" })
+    const dir = await this.createWorkspace()
+    await this.page.goto(`${CODE_SERVER_ADDRESS}?folder=${dir}`, { waitUntil: "networkidle" })
   }
 
   /**
@@ -113,4 +116,20 @@ export class CodeServer {
     await this.navigate()
     await this.reloadUntilEditorIsReady()
   }
+
+  /**
+   * Create a random workspace and seed it with settings.
+   */
+  private async createWorkspace(): Promise<string> {
+    const dir = await tmpdir(workspaceDir)
+    await fs.mkdir(path.join(dir, ".vscode"))
+    await fs.writeFile(
+      path.join(dir, ".vscode/settings.json"),
+      JSON.stringify({
+        "workbench.startupEditor": "none",
+      }),
+      "utf8",
+    )
+    return dir
+  }
 }
diff --git a/test/playwright.config.ts b/test/playwright.config.ts
index c36355871..cb8eae186 100644
--- a/test/playwright.config.ts
+++ b/test/playwright.config.ts
@@ -6,7 +6,7 @@ import path from "path"
 const config: PlaywrightTestConfig = {
   testDir: path.join(__dirname, "e2e"), // Search for tests in this directory.
   timeout: 60000, // Each test is given 60 seconds.
-  retries: 3, // Retry failing tests 2 times
+  retries: process.env.CI ? 2 : 1, // Retry twice in CI due to flakiness.
   workers: 1,
   globalSetup: require.resolve("./utils/globalSetup.ts"),
   reporter: "list",
@@ -34,10 +34,4 @@ const config: PlaywrightTestConfig = {
   ],
 }
 
-if (process.env.CI) {
-  // In CI, retry failing tests 2 times
-  // in the event of flakiness
-  config.retries = 2
-}
-
 export default config
diff --git a/test/utils/constants.ts b/test/utils/constants.ts
index e927d6595..e656145e4 100644
--- a/test/utils/constants.ts
+++ b/test/utils/constants.ts
@@ -1,3 +1,4 @@
 export const CODE_SERVER_ADDRESS = process.env.CODE_SERVER_ADDRESS || "http://localhost:8080"
 export const PASSWORD = process.env.PASSWORD || "e45432jklfdsab"
 export const storageState = JSON.parse(process.env.STORAGE || "{}")
+export const workspaceDir = "workspaces"
diff --git a/test/utils/globalSetup.ts b/test/utils/globalSetup.ts
index d5fd46608..8d94c7438 100644
--- a/test/utils/globalSetup.ts
+++ b/test/utils/globalSetup.ts
@@ -1,15 +1,20 @@
-// This setup runs before our e2e tests
-// so that it authenticates us into code-server
-// ensuring that we're logged in before we run any tests
 import { chromium } from "playwright"
 import { hash } from "../../src/node/util"
-import { PASSWORD } from "./constants"
+import { PASSWORD, workspaceDir } from "./constants"
+import { clean } from "./helpers"
 import * as wtfnode from "./wtfnode"
 
+/**
+ * Perform workspace cleanup and authenticate. This should be set up to run
+ * before our tests execute.
+ */
 export default async function () {
   console.log("\n🚨 Running Global Setup for Playwright End-to-End Tests")
   console.log("   Please hang tight...")
 
+  // Cleanup workspaces from previous tests.
+  await clean(workspaceDir)
+
   const cookieToStore = {
     sameSite: "Lax" as const,
     name: "key",
diff --git a/test/utils/helpers.ts b/test/utils/helpers.ts
index f31752b83..f299fc130 100644
--- a/test/utils/helpers.ts
+++ b/test/utils/helpers.ts
@@ -1,4 +1,4 @@
-import * as fs from "fs"
+import { promises as fs } from "fs"
 import * as os from "os"
 import * as path from "path"
 
@@ -20,13 +20,20 @@ export function createLoggerMock() {
 }
 
 /**
- * Create a uniquely named temporary directory.
- *
- * These directories are placed under a single temporary code-server directory.
+ * Clean up directories left by a test. It is recommended to do this when a test
+ * starts to avoid potentially accumulating infinite test directories.
+ */
+export async function clean(testName: string): Promise<void> {
+  const dir = path.join(os.tmpdir(), `code-server/tests/${testName}`)
+  await fs.rm(dir, { recursive: true })
+}
+
+/**
+ * Create a uniquely named temporary directory for a test.
  */
 export async function tmpdir(testName: string): Promise<string> {
-  const dir = path.join(os.tmpdir(), "code-server/tests")
-  await fs.promises.mkdir(dir, { recursive: true })
+  const dir = path.join(os.tmpdir(), `code-server/tests/${testName}`)
+  await fs.mkdir(dir, { recursive: true })
 
-  return await fs.promises.mkdtemp(path.join(dir, `${testName}-`), { encoding: "utf8" })
+  return await fs.mkdtemp(path.join(dir, `${testName}-`), { encoding: "utf8" })
 }

From add55ecd62725d71b44c9957b4b604bd0ec24984 Mon Sep 17 00:00:00 2001
From: Asher <ash@coder.com>
Date: Fri, 25 Jun 2021 12:15:12 -0500
Subject: [PATCH 2/8] Import utils as a group in tests

This should simplify testing new utils a bit.
---
 test/unit/node/util.test.ts | 94 +++++++++++++++++--------------------
 test/unit/util.test.ts      | 75 +++++++++++++----------------
 2 files changed, 74 insertions(+), 95 deletions(-)

diff --git a/test/unit/node/util.test.ts b/test/unit/node/util.test.ts
index a06791776..6ea6c818c 100644
--- a/test/unit/node/util.test.ts
+++ b/test/unit/node/util.test.ts
@@ -1,14 +1,4 @@
-import {
-  hash,
-  isHashMatch,
-  handlePasswordValidation,
-  PasswordMethod,
-  getPasswordMethod,
-  hashLegacy,
-  isHashLegacyMatch,
-  isCookieValid,
-  sanitizeString,
-} from "../../../src/node/util"
+import * as util from "../../../src/node/util"
 
 describe("getEnvPaths", () => {
   describe("on darwin", () => {
@@ -161,7 +151,7 @@ describe("getEnvPaths", () => {
 describe("hash", () => {
   it("should return a hash of the string passed in", async () => {
     const plainTextPassword = "mySecretPassword123"
-    const hashed = await hash(plainTextPassword)
+    const hashed = await util.hash(plainTextPassword)
     expect(hashed).not.toBe(plainTextPassword)
   })
 })
@@ -169,32 +159,32 @@ describe("hash", () => {
 describe("isHashMatch", () => {
   it("should return true if the password matches the hash", async () => {
     const password = "codeserver1234"
-    const _hash = await hash(password)
-    const actual = await isHashMatch(password, _hash)
+    const _hash = await util.hash(password)
+    const actual = await util.isHashMatch(password, _hash)
     expect(actual).toBe(true)
   })
   it("should return false if the password does not match the hash", async () => {
     const password = "password123"
-    const _hash = await hash(password)
-    const actual = await isHashMatch("otherPassword123", _hash)
+    const _hash = await util.hash(password)
+    const actual = await util.isHashMatch("otherPassword123", _hash)
     expect(actual).toBe(false)
   })
   it("should return true with actual hash", async () => {
     const password = "password123"
     const _hash = "$argon2i$v=19$m=4096,t=3,p=1$EAoczTxVki21JDfIZpTUxg$rkXgyrW4RDGoDYrxBFD4H2DlSMEhP4h+Api1hXnGnFY"
-    const actual = await isHashMatch(password, _hash)
+    const actual = await util.isHashMatch(password, _hash)
     expect(actual).toBe(true)
   })
   it("should return false if the password is empty", async () => {
     const password = ""
     const _hash = "$argon2i$v=19$m=4096,t=3,p=1$EAoczTxVki21JDfIZpTUxg$rkXgyrW4RDGoDYrxBFD4H2DlSMEhP4h+Api1hXnGnFY"
-    const actual = await isHashMatch(password, _hash)
+    const actual = await util.isHashMatch(password, _hash)
     expect(actual).toBe(false)
   })
   it("should return false if the hash is empty", async () => {
     const password = "hellowpasssword"
     const _hash = ""
-    const actual = await isHashMatch(password, _hash)
+    const actual = await util.isHashMatch(password, _hash)
     expect(actual).toBe(false)
   })
 })
@@ -202,7 +192,7 @@ describe("isHashMatch", () => {
 describe("hashLegacy", () => {
   it("should return a hash of the string passed in", () => {
     const plainTextPassword = "mySecretPassword123"
-    const hashed = hashLegacy(plainTextPassword)
+    const hashed = util.hashLegacy(plainTextPassword)
     expect(hashed).not.toBe(plainTextPassword)
   })
 })
@@ -210,40 +200,40 @@ describe("hashLegacy", () => {
 describe("isHashLegacyMatch", () => {
   it("should return true if is match", () => {
     const password = "password123"
-    const _hash = hashLegacy(password)
-    expect(isHashLegacyMatch(password, _hash)).toBe(true)
+    const _hash = util.hashLegacy(password)
+    expect(util.isHashLegacyMatch(password, _hash)).toBe(true)
   })
   it("should return false if is match", () => {
     const password = "password123"
-    const _hash = hashLegacy(password)
-    expect(isHashLegacyMatch("otherPassword123", _hash)).toBe(false)
+    const _hash = util.hashLegacy(password)
+    expect(util.isHashLegacyMatch("otherPassword123", _hash)).toBe(false)
   })
   it("should return true if hashed from command line", () => {
     const password = "password123"
     // Hashed using printf "password123" | sha256sum | cut -d' ' -f1
     const _hash = "ef92b778bafe771e89245b89ecbc08a44a4e166c06659911881f383d4473e94f"
-    expect(isHashLegacyMatch(password, _hash)).toBe(true)
+    expect(util.isHashLegacyMatch(password, _hash)).toBe(true)
   })
 })
 
 describe("getPasswordMethod", () => {
   it("should return PLAIN_TEXT for no hashed password", () => {
     const hashedPassword = undefined
-    const passwordMethod = getPasswordMethod(hashedPassword)
-    const expected: PasswordMethod = "PLAIN_TEXT"
+    const passwordMethod = util.getPasswordMethod(hashedPassword)
+    const expected: util.PasswordMethod = "PLAIN_TEXT"
     expect(passwordMethod).toEqual(expected)
   })
   it("should return ARGON2 for password with 'argon2'", () => {
     const hashedPassword =
       "$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY"
-    const passwordMethod = getPasswordMethod(hashedPassword)
-    const expected: PasswordMethod = "ARGON2"
+    const passwordMethod = util.getPasswordMethod(hashedPassword)
+    const expected: util.PasswordMethod = "ARGON2"
     expect(passwordMethod).toEqual(expected)
   })
   it("should return SHA256 for password with legacy hash", () => {
     const hashedPassword = "936a185caaa266bb9cbe981e9e05cb78cd732b0b3280eb944412bb6f8f8f07af"
-    const passwordMethod = getPasswordMethod(hashedPassword)
-    const expected: PasswordMethod = "SHA256"
+    const passwordMethod = util.getPasswordMethod(hashedPassword)
+    const expected: util.PasswordMethod = "SHA256"
     expect(passwordMethod).toEqual(expected)
   })
 })
@@ -251,63 +241,63 @@ describe("getPasswordMethod", () => {
 describe("handlePasswordValidation", () => {
   it("should return true with a hashedPassword for a PLAIN_TEXT password", async () => {
     const p = "password"
-    const passwordValidation = await handlePasswordValidation({
+    const passwordValidation = await util.handlePasswordValidation({
       passwordMethod: "PLAIN_TEXT",
       passwordFromRequestBody: p,
       passwordFromArgs: p,
       hashedPasswordFromArgs: undefined,
     })
 
-    const matchesHash = await isHashMatch(p, passwordValidation.hashedPassword)
+    const matchesHash = await util.isHashMatch(p, passwordValidation.hashedPassword)
 
     expect(passwordValidation.isPasswordValid).toBe(true)
     expect(matchesHash).toBe(true)
   })
   it("should return false when PLAIN_TEXT password doesn't match args", async () => {
     const p = "password"
-    const passwordValidation = await handlePasswordValidation({
+    const passwordValidation = await util.handlePasswordValidation({
       passwordMethod: "PLAIN_TEXT",
       passwordFromRequestBody: "password1",
       passwordFromArgs: p,
       hashedPasswordFromArgs: undefined,
     })
 
-    const matchesHash = await isHashMatch(p, passwordValidation.hashedPassword)
+    const matchesHash = await util.isHashMatch(p, passwordValidation.hashedPassword)
 
     expect(passwordValidation.isPasswordValid).toBe(false)
     expect(matchesHash).toBe(false)
   })
   it("should return true with a hashedPassword for a SHA256 password", async () => {
     const p = "helloworld"
-    const passwordValidation = await handlePasswordValidation({
+    const passwordValidation = await util.handlePasswordValidation({
       passwordMethod: "SHA256",
       passwordFromRequestBody: p,
       passwordFromArgs: undefined,
       hashedPasswordFromArgs: "936a185caaa266bb9cbe981e9e05cb78cd732b0b3280eb944412bb6f8f8f07af",
     })
 
-    const matchesHash = isHashLegacyMatch(p, passwordValidation.hashedPassword)
+    const matchesHash = util.isHashLegacyMatch(p, passwordValidation.hashedPassword)
 
     expect(passwordValidation.isPasswordValid).toBe(true)
     expect(matchesHash).toBe(true)
   })
   it("should return false when SHA256 password doesn't match hash", async () => {
     const p = "helloworld1"
-    const passwordValidation = await handlePasswordValidation({
+    const passwordValidation = await util.handlePasswordValidation({
       passwordMethod: "SHA256",
       passwordFromRequestBody: p,
       passwordFromArgs: undefined,
       hashedPasswordFromArgs: "936a185caaa266bb9cbe981e9e05cb78cd732b0b3280eb944412bb6f8f8f07af",
     })
 
-    const matchesHash = isHashLegacyMatch(p, passwordValidation.hashedPassword)
+    const matchesHash = util.isHashLegacyMatch(p, passwordValidation.hashedPassword)
 
     expect(passwordValidation.isPasswordValid).toBe(false)
     expect(matchesHash).toBe(false)
   })
   it("should return true with a hashedPassword for a ARGON2 password", async () => {
     const p = "password"
-    const passwordValidation = await handlePasswordValidation({
+    const passwordValidation = await util.handlePasswordValidation({
       passwordMethod: "ARGON2",
       passwordFromRequestBody: p,
       passwordFromArgs: undefined,
@@ -315,14 +305,14 @@ describe("handlePasswordValidation", () => {
         "$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY",
     })
 
-    const matchesHash = await isHashMatch(p, passwordValidation.hashedPassword)
+    const matchesHash = await util.isHashMatch(p, passwordValidation.hashedPassword)
 
     expect(passwordValidation.isPasswordValid).toBe(true)
     expect(matchesHash).toBe(true)
   })
   it("should return false when ARGON2 password doesn't match hash", async () => {
     const p = "password1"
-    const passwordValidation = await handlePasswordValidation({
+    const passwordValidation = await util.handlePasswordValidation({
       passwordMethod: "ARGON2",
       passwordFromRequestBody: p,
       passwordFromArgs: undefined,
@@ -330,7 +320,7 @@ describe("handlePasswordValidation", () => {
         "$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY",
     })
 
-    const matchesHash = await isHashMatch(p, passwordValidation.hashedPassword)
+    const matchesHash = await util.isHashMatch(p, passwordValidation.hashedPassword)
 
     expect(passwordValidation.isPasswordValid).toBe(false)
     expect(matchesHash).toBe(false)
@@ -339,7 +329,7 @@ describe("handlePasswordValidation", () => {
 
 describe("isCookieValid", () => {
   it("should be valid if hashed-password for SHA256 matches cookie.key", async () => {
-    const isValid = await isCookieValid({
+    const isValid = await util.isCookieValid({
       passwordMethod: "SHA256",
       cookieKey: "936a185caaa266bb9cbe981e9e05cb78cd732b0b3280eb944412bb6f8f8f07af",
       hashedPasswordFromArgs: "936a185caaa266bb9cbe981e9e05cb78cd732b0b3280eb944412bb6f8f8f07af",
@@ -348,7 +338,7 @@ describe("isCookieValid", () => {
     expect(isValid).toBe(true)
   })
   it("should be invalid if hashed-password for SHA256 does not match cookie.key", async () => {
-    const isValid = await isCookieValid({
+    const isValid = await util.isCookieValid({
       passwordMethod: "SHA256",
       cookieKey: "936a185caaa266bb9cbe981e9e05cb78cd732b0b3280eb9442bb6f8f8f07af",
       hashedPasswordFromArgs: "936a185caaa266bb9cbe981e9e05cb78cd732b0b3280eb944412bb6f8f8f07af",
@@ -357,7 +347,7 @@ describe("isCookieValid", () => {
     expect(isValid).toBe(false)
   })
   it("should be valid if hashed-password for ARGON2 matches cookie.key", async () => {
-    const isValid = await isCookieValid({
+    const isValid = await util.isCookieValid({
       passwordMethod: "ARGON2",
       cookieKey: "$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY",
       hashedPasswordFromArgs:
@@ -367,7 +357,7 @@ describe("isCookieValid", () => {
     expect(isValid).toBe(true)
   })
   it("should be invalid if hashed-password for ARGON2 does not match cookie.key", async () => {
-    const isValid = await isCookieValid({
+    const isValid = await util.isCookieValid({
       passwordMethod: "ARGON2",
       cookieKey: "$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9H",
       hashedPasswordFromArgs:
@@ -377,7 +367,7 @@ describe("isCookieValid", () => {
     expect(isValid).toBe(false)
   })
   it("should be valid if password for PLAIN_TEXT matches cookie.key", async () => {
-    const isValid = await isCookieValid({
+    const isValid = await util.isCookieValid({
       passwordMethod: "PLAIN_TEXT",
       cookieKey: "$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY",
       passwordFromArgs: "password",
@@ -386,7 +376,7 @@ describe("isCookieValid", () => {
     expect(isValid).toBe(true)
   })
   it("should be invalid if hashed-password for PLAIN_TEXT does not match cookie.key", async () => {
-    const isValid = await isCookieValid({
+    const isValid = await util.isCookieValid({
       passwordMethod: "PLAIN_TEXT",
       cookieKey: "$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9H",
       passwordFromArgs: "password1234",
@@ -398,12 +388,12 @@ describe("isCookieValid", () => {
 
 describe("sanitizeString", () => {
   it("should return an empty string if passed a type other than a string", () => {
-    expect(sanitizeString({} as string)).toBe("")
+    expect(util.sanitizeString({} as string)).toBe("")
   })
   it("should trim whitespace", () => {
-    expect(sanitizeString(" hello   ")).toBe("hello")
+    expect(util.sanitizeString(" hello   ")).toBe("hello")
   })
   it("should always return an empty string", () => {
-    expect(sanitizeString("   ")).toBe("")
+    expect(util.sanitizeString("   ")).toBe("")
   })
 })
diff --git a/test/unit/util.test.ts b/test/unit/util.test.ts
index e63fcde57..6a60e1533 100644
--- a/test/unit/util.test.ts
+++ b/test/unit/util.test.ts
@@ -1,16 +1,5 @@
 import { JSDOM } from "jsdom"
-import {
-  arrayify,
-  generateUuid,
-  getFirstString,
-  getOptions,
-  logError,
-  plural,
-  resolveBase,
-  split,
-  trimSlashes,
-  normalize,
-} from "../../src/common/util"
+import * as util from "../../src/common/util"
 import { createLoggerMock } from "../utils/helpers"
 
 const dom = new JSDOM()
@@ -21,67 +10,67 @@ export type LocationLike = Pick<Location, "pathname" | "origin">
 describe("util", () => {
   describe("normalize", () => {
     it("should remove multiple slashes", () => {
-      expect(normalize("//foo//bar//baz///mumble")).toBe("/foo/bar/baz/mumble")
+      expect(util.normalize("//foo//bar//baz///mumble")).toBe("/foo/bar/baz/mumble")
     })
 
     it("should remove trailing slashes", () => {
-      expect(normalize("qux///")).toBe("qux")
+      expect(util.normalize("qux///")).toBe("qux")
     })
 
     it("should preserve trailing slash if it exists", () => {
-      expect(normalize("qux///", true)).toBe("qux/")
-      expect(normalize("qux", true)).toBe("qux")
+      expect(util.normalize("qux///", true)).toBe("qux/")
+      expect(util.normalize("qux", true)).toBe("qux")
     })
   })
 
   describe("split", () => {
     it("should split at a comma", () => {
-      expect(split("Hello,world", ",")).toStrictEqual(["Hello", "world"])
+      expect(util.split("Hello,world", ",")).toStrictEqual(["Hello", "world"])
     })
 
     it("shouldn't split if the delimiter doesn't exist", () => {
-      expect(split("Hello world", ",")).toStrictEqual(["Hello world", ""])
+      expect(util.split("Hello world", ",")).toStrictEqual(["Hello world", ""])
     })
   })
 
   describe("plural", () => {
     it("should add an s if count is greater than 1", () => {
-      expect(plural(2, "dog")).toBe("dogs")
+      expect(util.plural(2, "dog")).toBe("dogs")
     })
     it("should NOT add an s if the count is 1", () => {
-      expect(plural(1, "dog")).toBe("dog")
+      expect(util.plural(1, "dog")).toBe("dog")
     })
   })
 
   describe("generateUuid", () => {
     it("should generate a unique uuid", () => {
-      const uuid = generateUuid()
-      const uuid2 = generateUuid()
+      const uuid = util.generateUuid()
+      const uuid2 = util.generateUuid()
       expect(uuid).toHaveLength(24)
       expect(typeof uuid).toBe("string")
       expect(uuid).not.toBe(uuid2)
     })
     it("should generate a uuid of a specific length", () => {
-      const uuid = generateUuid(10)
+      const uuid = util.generateUuid(10)
       expect(uuid).toHaveLength(10)
     })
   })
 
   describe("trimSlashes", () => {
     it("should remove leading slashes", () => {
-      expect(trimSlashes("/hello-world")).toBe("hello-world")
+      expect(util.trimSlashes("/hello-world")).toBe("hello-world")
     })
 
     it("should remove trailing slashes", () => {
-      expect(trimSlashes("hello-world/")).toBe("hello-world")
+      expect(util.trimSlashes("hello-world/")).toBe("hello-world")
     })
 
     it("should remove both leading and trailing slashes", () => {
-      expect(trimSlashes("/hello-world/")).toBe("hello-world")
+      expect(util.trimSlashes("/hello-world/")).toBe("hello-world")
     })
 
     it("should remove multiple leading and trailing slashes", () => {
-      expect(trimSlashes("///hello-world////")).toBe("hello-world")
+      expect(util.trimSlashes("///hello-world////")).toBe("hello-world")
     })
   })
 
@@ -101,23 +90,23 @@ describe("util", () => {
     })
 
     it("should resolve a base", () => {
-      expect(resolveBase("localhost:8080")).toBe("/localhost:8080")
+      expect(util.resolveBase("localhost:8080")).toBe("/localhost:8080")
     })
 
     it("should resolve a base with a forward slash at the beginning", () => {
-      expect(resolveBase("/localhost:8080")).toBe("/localhost:8080")
+      expect(util.resolveBase("/localhost:8080")).toBe("/localhost:8080")
     })
 
     it("should resolve a base with query params", () => {
-      expect(resolveBase("localhost:8080?folder=hello-world")).toBe("/localhost:8080")
+      expect(util.resolveBase("localhost:8080?folder=hello-world")).toBe("/localhost:8080")
     })
 
     it("should resolve a base with a path", () => {
-      expect(resolveBase("localhost:8080/hello/world")).toBe("/localhost:8080/hello/world")
+      expect(util.resolveBase("localhost:8080/hello/world")).toBe("/localhost:8080/hello/world")
     })
 
     it("should resolve a base to an empty string when not provided", () => {
-      expect(resolveBase()).toBe("")
+      expect(util.resolveBase()).toBe("")
     })
   })
 
@@ -142,7 +131,7 @@ describe("util", () => {
     })
 
     it("should return options with base and cssStaticBase even if it doesn't exist", () => {
-      expect(getOptions()).toStrictEqual({
+      expect(util.getOptions()).toStrictEqual({
         base: "",
         csStaticBase: "",
       })
@@ -162,7 +151,7 @@ describe("util", () => {
       // it returns the element
       spy.mockImplementation(() => mockElement)
 
-      expect(getOptions()).toStrictEqual({
+      expect(util.getOptions()).toStrictEqual({
         base: "",
         csStaticBase: "/static/development/Users/jp/Dev/code-server",
         disableTelemetry: false,
@@ -179,7 +168,7 @@ describe("util", () => {
       // spreads the original options
       // then parses the queryOpts
       location.search = '?options={"logLevel":2}'
-      expect(getOptions()).toStrictEqual({
+      expect(util.getOptions()).toStrictEqual({
         base: "",
         csStaticBase: "",
         logLevel: 2,
@@ -189,12 +178,12 @@ describe("util", () => {
 
   describe("arrayify", () => {
     it("should return value it's already an array", () => {
-      expect(arrayify(["hello", "world"])).toStrictEqual(["hello", "world"])
+      expect(util.arrayify(["hello", "world"])).toStrictEqual(["hello", "world"])
     })
 
     it("should wrap the value in an array if not an array", () => {
       expect(
-        arrayify({
+        util.arrayify({
           name: "Coder",
           version: "3.8",
         }),
@@ -202,21 +191,21 @@ describe("util", () => {
     })
 
     it("should return an empty array if the value is undefined", () => {
-      expect(arrayify(undefined)).toStrictEqual([])
+      expect(util.arrayify(undefined)).toStrictEqual([])
     })
   })
 
   describe("getFirstString", () => {
     it("should return the string if passed a string", () => {
-      expect(getFirstString("Hello world!")).toBe("Hello world!")
+      expect(util.getFirstString("Hello world!")).toBe("Hello world!")
     })
 
     it("should get the first string from an array", () => {
-      expect(getFirstString(["Hello", "World"])).toBe("Hello")
+      expect(util.getFirstString(["Hello", "World"])).toBe("Hello")
     })
 
     it("should return undefined if the value isn't an array or a string", () => {
-      expect(getFirstString({ name: "Coder" })).toBe(undefined)
+      expect(util.getFirstString({ name: "Coder" })).toBe(undefined)
     })
   })
 
@@ -235,14 +224,14 @@ describe("util", () => {
       const message = "You don't have access to that folder."
       const error = new Error(message)
 
-      logError(loggerModule.logger, "ui", error)
+      util.logError(loggerModule.logger, "ui", error)
 
       expect(loggerModule.logger.error).toHaveBeenCalled()
       expect(loggerModule.logger.error).toHaveBeenCalledWith(`ui: ${error.message} ${error.stack}`)
     })
 
     it("should log an error, even if not an instance of error", () => {
-      logError(loggerModule.logger, "api", "oh no")
+      util.logError(loggerModule.logger, "api", "oh no")
 
       expect(loggerModule.logger.error).toHaveBeenCalled()
       expect(loggerModule.logger.error).toHaveBeenCalledWith("api: oh no")

From 49c44818d926e6c339e008809c4ecac7bb636997 Mon Sep 17 00:00:00 2001
From: Asher <ash@coder.com>
Date: Wed, 23 Jun 2021 15:57:16 -0500
Subject: [PATCH 3/8] Move onLine to utilities

This way it can be used by the tests when spawning code-server on a
random port to look for the address.
---
 ci/dev/watch.ts             | 33 +--------------------------------
 src/node/util.ts            | 32 ++++++++++++++++++++++++++++++++
 test/unit/node/util.test.ts | 37 +++++++++++++++++++++++++++++++++++++
 3 files changed, 70 insertions(+), 32 deletions(-)

diff --git a/ci/dev/watch.ts b/ci/dev/watch.ts
index 714a12f7e..1503256c0 100644
--- a/ci/dev/watch.ts
+++ b/ci/dev/watch.ts
@@ -2,6 +2,7 @@ import browserify from "browserify"
 import * as cp from "child_process"
 import * as fs from "fs"
 import * as path from "path"
+import { onLine } from "../../src/node/util"
 
 async function main(): Promise<void> {
   try {
@@ -97,38 +98,6 @@ class Watcher {
       path.join(this.rootPath, "out/browser/pages/vscode.js"),
     ]
 
-    // From https://github.com/chalk/ansi-regex
-    const pattern = [
-      "[\\u001B\\u009B][[\\]()#;?]*(?:(?:(?:[a-zA-Z\\d]*(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)",
-      "(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-ntqry=><~]))",
-    ].join("|")
-    const re = new RegExp(pattern, "g")
-
-    /**
-     * Split stdout on newlines and strip ANSI codes.
-     */
-    const onLine = (proc: cp.ChildProcess, callback: (strippedLine: string, originalLine: string) => void): void => {
-      let buffer = ""
-      if (!proc.stdout) {
-        throw new Error("no stdout")
-      }
-      proc.stdout.setEncoding("utf8")
-      proc.stdout.on("data", (d) => {
-        const data = buffer + d
-        const split = data.split("\n")
-        const last = split.length - 1
-
-        for (let i = 0; i < last; ++i) {
-          callback(split[i].replace(re, ""), split[i])
-        }
-
-        // The last item will either be an empty string (the data ended with a
-        // newline) or a partial line (did not end with a newline) and we must
-        // wait to parse it until we get a full line.
-        buffer = split[last]
-      })
-    }
-
     let startingVscode = false
     let startedVscode = false
     onLine(vscode, (line, original) => {
diff --git a/src/node/util.ts b/src/node/util.ts
index 053f2df51..9129c7e84 100644
--- a/src/node/util.ts
+++ b/src/node/util.ts
@@ -17,6 +17,38 @@ export interface Paths {
   runtime: string
 }
 
+// From https://github.com/chalk/ansi-regex
+const pattern = [
+  "[\\u001B\\u009B][[\\]()#;?]*(?:(?:(?:[a-zA-Z\\d]*(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)",
+  "(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-ntqry=><~]))",
+].join("|")
+const re = new RegExp(pattern, "g")
+
+/**
+ * Split stdout on newlines and strip ANSI codes.
+ */
+export const onLine = (proc: cp.ChildProcess, callback: (strippedLine: string, originalLine: string) => void): void => {
+  let buffer = ""
+  if (!proc.stdout) {
+    throw new Error("no stdout")
+  }
+  proc.stdout.setEncoding("utf8")
+  proc.stdout.on("data", (d) => {
+    const data = buffer + d
+    const split = data.split("\n")
+    const last = split.length - 1
+
+    for (let i = 0; i < last; ++i) {
+      callback(split[i].replace(re, ""), split[i])
+    }
+
+    // The last item will either be an empty string (the data ended with a
+    // newline) or a partial line (did not end with a newline) and we must
+    // wait to parse it until we get a full line.
+    buffer = split[last]
+  })
+}
+
 export const paths = getEnvPaths()
 
 /**
diff --git a/test/unit/node/util.test.ts b/test/unit/node/util.test.ts
index 6ea6c818c..38534c228 100644
--- a/test/unit/node/util.test.ts
+++ b/test/unit/node/util.test.ts
@@ -1,3 +1,5 @@
+import * as cp from "child_process"
+import { generateUuid } from "../../../src/common/util"
 import * as util from "../../../src/node/util"
 
 describe("getEnvPaths", () => {
@@ -397,3 +399,38 @@ describe("sanitizeString", () => {
     expect(util.sanitizeString("   ")).toBe("")
   })
 })
+
+describe("onLine", () => {
+  // Spawn a process that outputs anything given on stdin.
+  let proc: cp.ChildProcess | undefined
+
+  beforeAll(() => {
+    proc = cp.spawn("node", ["-e", 'process.stdin.setEncoding("utf8");process.stdin.on("data", console.log)'])
+  })
+
+  afterAll(() => {
+    proc?.kill()
+  })
+
+  it("should call with individual lines", async () => {
+    const size = 100
+    const received = new Promise<string[]>((resolve) => {
+      const lines: string[] = []
+      util.onLine(proc!, (line) => {
+        lines.push(line)
+        if (lines.length === size) {
+          resolve(lines)
+        }
+      })
+    })
+
+    const expected: string[] = []
+    for (let i = 0; i < size; ++i) {
+      expected.push(generateUuid(i))
+    }
+
+    proc?.stdin?.write(expected.join("\n"))
+
+    expect(await received).toEqual(expected)
+  })
+})

From da4de439e0d4fd2bf5bc6fd843691987e741871c Mon Sep 17 00:00:00 2001
From: Asher <ash@coder.com>
Date: Tue, 22 Jun 2021 16:34:44 -0500
Subject: [PATCH 4/8] Spawn a code-server instance for each test suite

This uses the current dev build by default but can be overidden with
CODE_SERVER_TEST_ENTRY (for example to test a release or some other
version).

Each instance has a separate state directory. This should make
parallelization work.

This also means you are no longer required to specify the password and
address yourself (or the extension directory once we add a test
extension). `yarn test:e2e` should just work as-is.

Lastly, it means the tests are no longer subject to yarn watch randomly
restarting.
---
 .github/workflows/ci.yaml      |  15 +--
 ci/dev/test-e2e.sh             |  31 +++++-
 test/e2e/baseFixture.ts        |  47 +++++++--
 test/e2e/browser.test.ts       |   4 +-
 test/e2e/codeServer.test.ts    |  10 +-
 test/e2e/globalSetup.test.ts   |   4 +-
 test/e2e/login.test.ts         |   4 +-
 test/e2e/logout.test.ts        |   8 +-
 test/e2e/models/CodeServer.ts  | 186 +++++++++++++++++++++++++++------
 test/e2e/openHelpAbout.test.ts |   4 +-
 test/e2e/terminal.test.ts      |   4 +-
 test/playwright.config.ts      |   3 +-
 test/utils/constants.ts        |   3 +-
 test/utils/helpers.ts          |   2 +-
 14 files changed, 253 insertions(+), 72 deletions(-)

diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index 8fcda93a4..f433f6413 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -334,8 +334,9 @@ jobs:
     needs: package-linux-amd64
     runs-on: ubuntu-latest
     env:
-      PASSWORD: e45432jklfdsab
-      CODE_SERVER_ADDRESS: http://localhost:8080
+      # Since we build code-server we might as well run tests from the release
+      # since VS Code will load faster due to the bundling.
+      CODE_SERVER_TEST_ENTRY: "./release-packages/code-server-linux-amd64"
     steps:
       - uses: actions/checkout@v2
 
@@ -362,9 +363,11 @@ jobs:
           name: release-packages
           path: ./release-packages
 
-      - name: Untar code-server file
+      - name: Untar code-server release
         run: |
-          cd release-packages && tar -xzf code-server*-linux-amd64.tar.gz
+          cd release-packages
+          tar -xzf code-server*-linux-amd64.tar.gz
+          mv code-server*-linux-amd64 code-server-linux-amd64
 
       - name: Install dependencies
         if: steps.cache-yarn.outputs.cache-hit != 'true'
@@ -380,9 +383,7 @@ jobs:
           yarn install --check-files
 
       - name: Run end-to-end tests
-        run: |
-          ./release-packages/code-server*-linux-amd64/bin/code-server --log trace &
-          yarn test:e2e
+        run: yarn test:e2e
 
       - name: Upload test artifacts
         if: always()
diff --git a/ci/dev/test-e2e.sh b/ci/dev/test-e2e.sh
index 1e6dcfa59..47c01cda6 100755
--- a/ci/dev/test-e2e.sh
+++ b/ci/dev/test-e2e.sh
@@ -3,10 +3,35 @@ set -euo pipefail
 
 main() {
   cd "$(dirname "$0")/../.."
+  source ./ci/lib.sh
+
+  local dir="$PWD"
+  if [[ ! ${CODE_SERVER_TEST_ENTRY-} ]]; then
+    echo "Set CODE_SERVER_TEST_ENTRY to test another build of code-server"
+  else
+    pushd "$CODE_SERVER_TEST_ENTRY"
+    dir="$PWD"
+    popd
+  fi
+
+  echo "Testing build in '$dir'"
+
+  # Simple sanity checks to see that we've built. There could still be things
+  # wrong (native modules version issues, incomplete build, etc).
+  if [[ ! -d $dir/out ]]; then
+    echo >&2 "No code-server build detected"
+    echo >&2 "You can build it with 'yarn build' or 'yarn watch'"
+    exit 1
+  fi
+
+  if [[ ! -d $dir/lib/vscode/out ]]; then
+    echo >&2 "No VS Code build detected"
+    echo >&2 "You can build it with 'yarn build:vscode' or 'yarn watch'"
+    exit 1
+  fi
+
   cd test
-  # We set these environment variables because they're used in the e2e tests
-  # they don't have to be these values, but these are the defaults
-  PASSWORD=e45432jklfdsab CODE_SERVER_ADDRESS=http://localhost:8080 yarn playwright test "$@"
+  yarn playwright test "$@"
 }
 
 main "$@"
diff --git a/test/e2e/baseFixture.ts b/test/e2e/baseFixture.ts
index a5fc15511..b4105c227 100644
--- a/test/e2e/baseFixture.ts
+++ b/test/e2e/baseFixture.ts
@@ -1,12 +1,47 @@
+import { field, logger } from "@coder/logger"
 import { test as base } from "@playwright/test"
-import { CodeServer } from "./models/CodeServer"
+import { CodeServer, CodeServerPage } from "./models/CodeServer"
 
-export const test = base.extend<{ codeServerPage: CodeServer }>({
-  codeServerPage: async ({ page }, use) => {
-    const codeServer = new CodeServer(page)
-    await codeServer.navigate()
-    await use(codeServer)
+/**
+ * Wraps `test.describe` to create and manage an instance of code-server. If you
+ * don't use this you will need to create your own code-server instance and pass
+ * it to `test.use`.
+ */
+export const describe = (name: string, fn: (codeServer: CodeServer) => void) => {
+  test.describe(name, () => {
+    // This will spawn on demand so nothing is necessary on before.
+    const codeServer = new CodeServer(name)
+
+    // Kill code-server after the suite has ended. This may happen even without
+    // doing it explicitly but it seems prudent to be sure.
+    test.afterAll(async () => {
+      await codeServer.close()
+    })
+
+    // This makes `codeServer` available to the extend call below.
+    test.use({ codeServer })
+
+    fn(codeServer)
+  })
+}
+
+interface TestFixtures {
+  codeServer: CodeServer
+  codeServerPage: CodeServerPage
+}
+
+/**
+ * Create a test that spawns code-server if necessary and ensures the page is
+ * ready.
+ */
+export const test = base.extend<TestFixtures>({
+  codeServer: undefined, // No default; should be provided through `test.use`.
+  codeServerPage: async ({ codeServer, page }, use) => {
+    const codeServerPage = new CodeServerPage(codeServer, page)
+    await codeServerPage.navigate()
+    await use(codeServerPage)
   },
 })
 
+/** Shorthand for test.expect. */
 export const expect = test.expect
diff --git a/test/e2e/browser.test.ts b/test/e2e/browser.test.ts
index 9e3e01545..078b4e309 100644
--- a/test/e2e/browser.test.ts
+++ b/test/e2e/browser.test.ts
@@ -1,7 +1,7 @@
-import { test, expect } from "./baseFixture"
+import { describe, test, expect } from "./baseFixture"
 
 // This is a "gut-check" test to make sure playwright is working as expected
-test.describe("browser", () => {
+describe("browser", () => {
   test("browser should display correct userAgent", async ({ codeServerPage, browserName }) => {
     const displayNames = {
       chromium: "Chrome",
diff --git a/test/e2e/codeServer.test.ts b/test/e2e/codeServer.test.ts
index f2a95121a..a40e30d89 100644
--- a/test/e2e/codeServer.test.ts
+++ b/test/e2e/codeServer.test.ts
@@ -1,12 +1,12 @@
-import { CODE_SERVER_ADDRESS, storageState } from "../utils/constants"
-import { test, expect } from "./baseFixture"
+import { storageState } from "../utils/constants"
+import { describe, test, expect } from "./baseFixture"
 
-test.describe("CodeServer", () => {
+describe("CodeServer", () => {
   test.use({
     storageState,
   })
 
-  test(`should navigate to ${CODE_SERVER_ADDRESS}`, async ({ codeServerPage }) => {
+  test("should navigate to home page", async ({ codeServerPage }) => {
     // We navigate codeServer before each test
     // and we start the test with a storage state
     // which means we should be logged in
@@ -14,7 +14,7 @@ test.describe("CodeServer", () => {
     const url = codeServerPage.page.url()
     // We use match because there may be a / at the end
     // so we don't want it to fail if we expect http://localhost:8080 to match http://localhost:8080/
-    expect(url).toMatch(CODE_SERVER_ADDRESS)
+    expect(url).toMatch(await codeServerPage.address())
   })
 
   test("should always see the code-server editor", async ({ codeServerPage }) => {
diff --git a/test/e2e/globalSetup.test.ts b/test/e2e/globalSetup.test.ts
index e1c54e139..ed3506ea9 100644
--- a/test/e2e/globalSetup.test.ts
+++ b/test/e2e/globalSetup.test.ts
@@ -1,9 +1,9 @@
 import { storageState } from "../utils/constants"
-import { test, expect } from "./baseFixture"
+import { describe, test, expect } from "./baseFixture"
 
 // This test is to make sure the globalSetup works as expected
 // meaning globalSetup ran and stored the storageState
-test.describe("globalSetup", () => {
+describe("globalSetup", () => {
   test.use({
     storageState,
   })
diff --git a/test/e2e/login.test.ts b/test/e2e/login.test.ts
index 7975bfcac..b2289740a 100644
--- a/test/e2e/login.test.ts
+++ b/test/e2e/login.test.ts
@@ -1,7 +1,7 @@
 import { PASSWORD } from "../utils/constants"
-import { test, expect } from "./baseFixture"
+import { describe, test, expect } from "./baseFixture"
 
-test.describe("login", () => {
+describe("login", () => {
   // Reset the browser so no cookies are persisted
   // by emptying the storageState
   test.use({
diff --git a/test/e2e/logout.test.ts b/test/e2e/logout.test.ts
index bec0f323a..ec480c4f1 100644
--- a/test/e2e/logout.test.ts
+++ b/test/e2e/logout.test.ts
@@ -1,7 +1,7 @@
-import { CODE_SERVER_ADDRESS, PASSWORD } from "../utils/constants"
-import { test, expect } from "./baseFixture"
+import { PASSWORD } from "../utils/constants"
+import { describe, test, expect } from "./baseFixture"
 
-test.describe("logout", () => {
+describe("logout", () => {
   // Reset the browser so no cookies are persisted
   // by emptying the storageState
   test.use({
@@ -39,6 +39,6 @@ test.describe("logout", () => {
     // https://github.com/microsoft/playwright/issues/1987#issuecomment-620182151
     await Promise.all([codeServerPage.page.waitForNavigation(), codeServerPage.page.click(logoutButton)])
     const currentUrl = codeServerPage.page.url()
-    expect(currentUrl).toBe(`${CODE_SERVER_ADDRESS}/login`)
+    expect(currentUrl).toBe(`${await codeServerPage.address()}/login`)
   })
 })
diff --git a/test/e2e/models/CodeServer.ts b/test/e2e/models/CodeServer.ts
index 662c82d40..b3e2bdafa 100644
--- a/test/e2e/models/CodeServer.ts
+++ b/test/e2e/models/CodeServer.ts
@@ -1,24 +1,163 @@
+import { Logger, logger } from "@coder/logger"
+import * as cp from "child_process"
 import { promises as fs } from "fs"
 import * as path from "path"
 import { Page } from "playwright"
-import { CODE_SERVER_ADDRESS, workspaceDir } from "../../utils/constants"
+import { onLine } from "../../../src/node/util"
+import { PASSWORD, workspaceDir } from "../../utils/constants"
 import { tmpdir } from "../../utils/helpers"
 
-// This is a Page Object Model
-// We use these to simplify e2e test authoring
-// See Playwright docs: https://playwright.dev/docs/pom/
-export class CodeServer {
-  private readonly editorSelector = "div.monaco-workbench"
+interface CodeServerProcess {
+  process: cp.ChildProcess
+  address: string
+}
 
-  constructor(public readonly page: Page) {}
+/**
+ * Class for spawning and managing code-server.
+ */
+export class CodeServer {
+  private process: Promise<CodeServerProcess> | undefined
+  private readonly logger: Logger
+  private closed = false
+
+  constructor(name: string) {
+    this.logger = logger.named(name)
+  }
 
   /**
-   * Navigates to CODE_SERVER_ADDRESS. It will open a newly created random
-   * directory.
+   * The address at which code-server can be accessed. Spawns code-server if it
+   * has not yet been spawned.
+   */
+  async address(): Promise<string> {
+    if (!this.process) {
+      this.process = this.spawn()
+    }
+    const { address } = await this.process
+    return address
+  }
+
+  /**
+   * Create a random workspace and seed it with settings.
+   */
+  private async createWorkspace(): Promise<string> {
+    const dir = await tmpdir(workspaceDir)
+    await fs.mkdir(path.join(dir, ".vscode"))
+    await fs.writeFile(
+      path.join(dir, ".vscode/settings.json"),
+      JSON.stringify({
+        "workbench.startupEditor": "none",
+      }),
+      "utf8",
+    )
+    return dir
+  }
+
+  /**
+   * Spawn a new code-server process with its own workspace and data
+   * directories.
+   */
+  private async spawn(): Promise<CodeServerProcess> {
+    // This will be used both as the workspace and data directory to ensure
+    // instances don't bleed into each other.
+    const dir = await this.createWorkspace()
+
+    return new Promise((resolve, reject) => {
+      this.logger.debug("spawning")
+      const proc = cp.spawn(
+        "node",
+        [
+          process.env.CODE_SERVER_TEST_ENTRY || ".",
+          // Using port zero will spawn on a random port.
+          "--bind-addr",
+          "127.0.0.1:0",
+          // Setting the XDG variables would be easier and more thorough but the
+          // modules we import ignores those variables for non-Linux operating
+          // systems so use these flags instead.
+          "--config",
+          path.join(dir, "config.yaml"),
+          "--user-data-dir",
+          dir,
+          // The last argument is the workspace to open.
+          dir,
+        ],
+        {
+          cwd: path.join(__dirname, "../../.."),
+          env: {
+            ...process.env,
+            PASSWORD,
+          },
+        },
+      )
+
+      proc.on("error", (error) => {
+        this.logger.error(error.message)
+        reject(error)
+      })
+
+      proc.on("close", () => {
+        const error = new Error("closed unexpectedly")
+        if (!this.closed) {
+          this.logger.error(error.message)
+        }
+        reject(error)
+      })
+
+      let resolved = false
+      proc.stdout.setEncoding("utf8")
+      onLine(proc, (line) => {
+        // Log the line without the timestamp.
+        this.logger.trace(line.replace(/\[.+\]/, ""))
+        if (resolved) {
+          return
+        }
+        const match = line.trim().match(/HTTP server listening on (https?:\/\/[.:\d]+)$/)
+        if (match) {
+          // Cookies don't seem to work on IP address so swap to localhost.
+          // TODO: Investigate whether this is a bug with code-server.
+          const address = match[1].replace("127.0.0.1", "localhost")
+          this.logger.debug(`spawned on ${address}`)
+          resolved = true
+          resolve({ process: proc, address })
+        }
+      })
+    })
+  }
+
+  /**
+   * Close the code-server process.
+   */
+  async close(): Promise<void> {
+    logger.debug("closing")
+    if (this.process) {
+      const proc = (await this.process).process
+      this.closed = true // To prevent the close handler from erroring.
+      proc.kill()
+    }
+  }
+}
+
+/**
+ * This is a "Page Object Model" (https://playwright.dev/docs/pom/) meant to
+ * wrap over a page and represent actions on that page in a more readable way.
+ * This targets a specific code-server instance which must be passed in.
+ * Navigation and setup performed by this model will cause the code-server
+ * process to spawn if it hasn't yet.
+ */
+export class CodeServerPage {
+  private readonly editorSelector = "div.monaco-workbench"
+
+  constructor(private readonly codeServer: CodeServer, public readonly page: Page) {}
+
+  address() {
+    return this.codeServer.address()
+  }
+
+  /**
+   * Navigate to code-server.
    */
   async navigate() {
-    const dir = await this.createWorkspace()
-    await this.page.goto(`${CODE_SERVER_ADDRESS}?folder=${dir}`, { waitUntil: "networkidle" })
+    const address = await this.codeServer.address()
+    await this.page.goto(address, { waitUntil: "networkidle" })
   }
 
   /**
@@ -45,7 +184,7 @@ export class CodeServer {
       await this.page.waitForTimeout(1000)
       reloadCount += 1
       if ((await this.isEditorVisible()) && (await this.isConnected())) {
-        console.log(`    Editor became ready after ${reloadCount} reloads`)
+        logger.debug(`editor became ready after ${reloadCount} reloads`)
         break
       }
       await this.page.reload()
@@ -67,7 +206,7 @@ export class CodeServer {
   async isConnected() {
     await this.page.waitForLoadState("networkidle")
 
-    const host = new URL(CODE_SERVER_ADDRESS).host
+    const host = new URL(await this.codeServer.address()).host
     const hostSelector = `[title="Editing on ${host}"]`
     await this.page.waitForSelector(hostSelector)
 
@@ -107,29 +246,12 @@ export class CodeServer {
   }
 
   /**
-   * Navigates to CODE_SERVER_ADDRESS
-   * and reloads until the editor is ready
+   * Navigates to code-server then reloads until the editor is ready.
    *
-   * Helpful for running before tests
+   * It is recommended to run setup before using this model in any tests.
    */
   async setup() {
     await this.navigate()
     await this.reloadUntilEditorIsReady()
   }
-
-  /**
-   * Create a random workspace and seed it with settings.
-   */
-  private async createWorkspace(): Promise<string> {
-    const dir = await tmpdir(workspaceDir)
-    await fs.mkdir(path.join(dir, ".vscode"))
-    await fs.writeFile(
-      path.join(dir, ".vscode/settings.json"),
-      JSON.stringify({
-        "workbench.startupEditor": "none",
-      }),
-      "utf8",
-    )
-    return dir
-  }
 }
diff --git a/test/e2e/openHelpAbout.test.ts b/test/e2e/openHelpAbout.test.ts
index 5c48346ed..1cb030540 100644
--- a/test/e2e/openHelpAbout.test.ts
+++ b/test/e2e/openHelpAbout.test.ts
@@ -1,7 +1,7 @@
 import { storageState } from "../utils/constants"
-import { test, expect } from "./baseFixture"
+import { describe, test, expect } from "./baseFixture"
 
-test.describe("Open Help > About", () => {
+describe("Open Help > About", () => {
   test.use({
     storageState,
   })
diff --git a/test/e2e/terminal.test.ts b/test/e2e/terminal.test.ts
index e682f464e..b2b284c74 100644
--- a/test/e2e/terminal.test.ts
+++ b/test/e2e/terminal.test.ts
@@ -4,9 +4,9 @@ import * as path from "path"
 import util from "util"
 import { storageState } from "../utils/constants"
 import { tmpdir } from "../utils/helpers"
-import { expect, test } from "./baseFixture"
+import { describe, expect, test } from "./baseFixture"
 
-test.describe("Integrated Terminal", () => {
+describe("Integrated Terminal", () => {
   // Create a new context with the saved storage state
   // so we don't have to logged in
   const testFileName = "pipe"
diff --git a/test/playwright.config.ts b/test/playwright.config.ts
index cb8eae186..9063a04b9 100644
--- a/test/playwright.config.ts
+++ b/test/playwright.config.ts
@@ -6,8 +6,7 @@ import path from "path"
 const config: PlaywrightTestConfig = {
   testDir: path.join(__dirname, "e2e"), // Search for tests in this directory.
   timeout: 60000, // Each test is given 60 seconds.
-  retries: process.env.CI ? 2 : 1, // Retry twice in CI due to flakiness.
-  workers: 1,
+  retries: process.env.CI ? 2 : 1, // Retry in CI due to flakiness.
   globalSetup: require.resolve("./utils/globalSetup.ts"),
   reporter: "list",
   // Put any shared options on the top level.
diff --git a/test/utils/constants.ts b/test/utils/constants.ts
index e656145e4..08acb9788 100644
--- a/test/utils/constants.ts
+++ b/test/utils/constants.ts
@@ -1,4 +1,3 @@
-export const CODE_SERVER_ADDRESS = process.env.CODE_SERVER_ADDRESS || "http://localhost:8080"
-export const PASSWORD = process.env.PASSWORD || "e45432jklfdsab"
+export const PASSWORD = "e45432jklfdsab"
 export const storageState = JSON.parse(process.env.STORAGE || "{}")
 export const workspaceDir = "workspaces"
diff --git a/test/utils/helpers.ts b/test/utils/helpers.ts
index f299fc130..a6a8d4513 100644
--- a/test/utils/helpers.ts
+++ b/test/utils/helpers.ts
@@ -25,7 +25,7 @@ export function createLoggerMock() {
  */
 export async function clean(testName: string): Promise<void> {
   const dir = path.join(os.tmpdir(), `code-server/tests/${testName}`)
-  await fs.rm(dir, { recursive: true })
+  await fs.rmdir(dir, { recursive: true })
 }
 
 /**

From f2fa7701a965e95a11be2a22676629d8f0f3615c Mon Sep 17 00:00:00 2001
From: Asher <ash@coder.com>
Date: Wed, 23 Jun 2021 17:41:36 -0500
Subject: [PATCH 5/8] Centralize credential handling

My thinking is that this may reduce the cognitive overhead for
developers writing new test suites.

This also allows us to perform different setup steps (like ensuring the
editor is visible when authenticated).
---
 test/e2e/baseFixture.ts        | 29 ++++++++++++++++++++++-----
 test/e2e/browser.test.ts       |  2 +-
 test/e2e/codeServer.test.ts    |  7 +------
 test/e2e/globalSetup.test.ts   |  7 +------
 test/e2e/login.test.ts         |  8 +-------
 test/e2e/logout.test.ts        |  8 +-------
 test/e2e/models/CodeServer.ts  |  8 ++++++--
 test/e2e/openHelpAbout.test.ts |  7 +------
 test/e2e/terminal.test.ts      |  7 +------
 test/utils/constants.ts        |  1 -
 test/utils/globalSetup.ts      | 36 ++++++++++++++++------------------
 11 files changed, 54 insertions(+), 66 deletions(-)

diff --git a/test/e2e/baseFixture.ts b/test/e2e/baseFixture.ts
index b4105c227..e67d09b8d 100644
--- a/test/e2e/baseFixture.ts
+++ b/test/e2e/baseFixture.ts
@@ -6,8 +6,10 @@ import { CodeServer, CodeServerPage } from "./models/CodeServer"
  * Wraps `test.describe` to create and manage an instance of code-server. If you
  * don't use this you will need to create your own code-server instance and pass
  * it to `test.use`.
+ *
+ * If `includeCredentials` is `true` page requests will be authenticated.
  */
-export const describe = (name: string, fn: (codeServer: CodeServer) => void) => {
+export const describe = (name: string, includeCredentials: boolean, fn: (codeServer: CodeServer) => void) => {
   test.describe(name, () => {
     // This will spawn on demand so nothing is necessary on before.
     const codeServer = new CodeServer(name)
@@ -18,14 +20,30 @@ export const describe = (name: string, fn: (codeServer: CodeServer) => void) =>
       await codeServer.close()
     })
 
-    // This makes `codeServer` available to the extend call below.
-    test.use({ codeServer })
+    const storageState = JSON.parse(process.env.STORAGE || "{}")
+
+    // Sanity check to ensure the cookie is set.
+    const cookies = storageState?.cookies
+    if (includeCredentials && (!cookies || cookies.length !== 1 || !!cookies[0].key)) {
+      logger.error("no cookies", field("storage", JSON.stringify(cookies)))
+      throw new Error("no credentials to include")
+    }
+
+    test.use({
+      // Makes `codeServer` and `authenticated` available to the extend call
+      // below.
+      codeServer,
+      authenticated: includeCredentials,
+      // This provides a cookie that authenticates with code-server.
+      storageState: includeCredentials ? storageState : {},
+    })
 
     fn(codeServer)
   })
 }
 
 interface TestFixtures {
+  authenticated: boolean
   codeServer: CodeServer
   codeServerPage: CodeServerPage
 }
@@ -35,10 +53,11 @@ interface TestFixtures {
  * ready.
  */
 export const test = base.extend<TestFixtures>({
+  authenticated: false,
   codeServer: undefined, // No default; should be provided through `test.use`.
-  codeServerPage: async ({ codeServer, page }, use) => {
+  codeServerPage: async ({ authenticated, codeServer, page }, use) => {
     const codeServerPage = new CodeServerPage(codeServer, page)
-    await codeServerPage.navigate()
+    await codeServerPage.setup(authenticated)
     await use(codeServerPage)
   },
 })
diff --git a/test/e2e/browser.test.ts b/test/e2e/browser.test.ts
index 078b4e309..3c3b2411c 100644
--- a/test/e2e/browser.test.ts
+++ b/test/e2e/browser.test.ts
@@ -1,7 +1,7 @@
 import { describe, test, expect } from "./baseFixture"
 
 // This is a "gut-check" test to make sure playwright is working as expected
-describe("browser", () => {
+describe("browser", true, () => {
   test("browser should display correct userAgent", async ({ codeServerPage, browserName }) => {
     const displayNames = {
       chromium: "Chrome",
diff --git a/test/e2e/codeServer.test.ts b/test/e2e/codeServer.test.ts
index a40e30d89..cfac6f74c 100644
--- a/test/e2e/codeServer.test.ts
+++ b/test/e2e/codeServer.test.ts
@@ -1,11 +1,6 @@
-import { storageState } from "../utils/constants"
 import { describe, test, expect } from "./baseFixture"
 
-describe("CodeServer", () => {
-  test.use({
-    storageState,
-  })
-
+describe("CodeServer", true, () => {
   test("should navigate to home page", async ({ codeServerPage }) => {
     // We navigate codeServer before each test
     // and we start the test with a storage state
diff --git a/test/e2e/globalSetup.test.ts b/test/e2e/globalSetup.test.ts
index ed3506ea9..8b4589b15 100644
--- a/test/e2e/globalSetup.test.ts
+++ b/test/e2e/globalSetup.test.ts
@@ -1,13 +1,8 @@
-import { storageState } from "../utils/constants"
 import { describe, test, expect } from "./baseFixture"
 
 // This test is to make sure the globalSetup works as expected
 // meaning globalSetup ran and stored the storageState
-describe("globalSetup", () => {
-  test.use({
-    storageState,
-  })
-
+describe("globalSetup", true, () => {
   test("should keep us logged in using the storageState", async ({ codeServerPage }) => {
     // Make sure the editor actually loaded
     expect(await codeServerPage.isEditorVisible()).toBe(true)
diff --git a/test/e2e/login.test.ts b/test/e2e/login.test.ts
index b2289740a..bc9d5e8e9 100644
--- a/test/e2e/login.test.ts
+++ b/test/e2e/login.test.ts
@@ -1,13 +1,7 @@
 import { PASSWORD } from "../utils/constants"
 import { describe, test, expect } from "./baseFixture"
 
-describe("login", () => {
-  // Reset the browser so no cookies are persisted
-  // by emptying the storageState
-  test.use({
-    storageState: {},
-  })
-
+describe("login", false, () => {
   test("should see the login page", async ({ codeServerPage }) => {
     // It should send us to the login page
     expect(await codeServerPage.page.title()).toBe("code-server login")
diff --git a/test/e2e/logout.test.ts b/test/e2e/logout.test.ts
index ec480c4f1..ac932fa47 100644
--- a/test/e2e/logout.test.ts
+++ b/test/e2e/logout.test.ts
@@ -1,13 +1,7 @@
 import { PASSWORD } from "../utils/constants"
 import { describe, test, expect } from "./baseFixture"
 
-describe("logout", () => {
-  // Reset the browser so no cookies are persisted
-  // by emptying the storageState
-  test.use({
-    storageState: {},
-  })
-
+describe("logout", false, () => {
   test("should be able login and logout", async ({ codeServerPage }) => {
     // Type in password
     await codeServerPage.page.fill(".password", PASSWORD)
diff --git a/test/e2e/models/CodeServer.ts b/test/e2e/models/CodeServer.ts
index b3e2bdafa..42600df3d 100644
--- a/test/e2e/models/CodeServer.ts
+++ b/test/e2e/models/CodeServer.ts
@@ -250,8 +250,12 @@ export class CodeServerPage {
    *
    * It is recommended to run setup before using this model in any tests.
    */
-  async setup() {
+  async setup(authenticated: boolean) {
     await this.navigate()
-    await this.reloadUntilEditorIsReady()
+    // If we aren't authenticated we'll see a login page so we can't wait until
+    // the editor is ready.
+    if (authenticated) {
+      await this.reloadUntilEditorIsReady()
+    }
   }
 }
diff --git a/test/e2e/openHelpAbout.test.ts b/test/e2e/openHelpAbout.test.ts
index 1cb030540..47daaaf14 100644
--- a/test/e2e/openHelpAbout.test.ts
+++ b/test/e2e/openHelpAbout.test.ts
@@ -1,11 +1,6 @@
-import { storageState } from "../utils/constants"
 import { describe, test, expect } from "./baseFixture"
 
-describe("Open Help > About", () => {
-  test.use({
-    storageState,
-  })
-
+describe("Open Help > About", true, () => {
   test("should see a 'Help' then 'About' button in the Application Menu that opens a dialog", async ({
     codeServerPage,
   }) => {
diff --git a/test/e2e/terminal.test.ts b/test/e2e/terminal.test.ts
index b2b284c74..836583a8d 100644
--- a/test/e2e/terminal.test.ts
+++ b/test/e2e/terminal.test.ts
@@ -2,11 +2,10 @@ import * as cp from "child_process"
 import * as fs from "fs"
 import * as path from "path"
 import util from "util"
-import { storageState } from "../utils/constants"
 import { tmpdir } from "../utils/helpers"
 import { describe, expect, test } from "./baseFixture"
 
-describe("Integrated Terminal", () => {
+describe("Integrated Terminal", true, () => {
   // Create a new context with the saved storage state
   // so we don't have to logged in
   const testFileName = "pipe"
@@ -14,10 +13,6 @@ describe("Integrated Terminal", () => {
   let tmpFolderPath = ""
   let tmpFile = ""
 
-  test.use({
-    storageState,
-  })
-
   test.beforeAll(async () => {
     tmpFolderPath = await tmpdir("integrated-terminal")
     tmpFile = path.join(tmpFolderPath, testFileName)
diff --git a/test/utils/constants.ts b/test/utils/constants.ts
index 08acb9788..9b5c2b6e8 100644
--- a/test/utils/constants.ts
+++ b/test/utils/constants.ts
@@ -1,3 +1,2 @@
 export const PASSWORD = "e45432jklfdsab"
-export const storageState = JSON.parse(process.env.STORAGE || "{}")
 export const workspaceDir = "workspaces"
diff --git a/test/utils/globalSetup.ts b/test/utils/globalSetup.ts
index 8d94c7438..eace7f9b2 100644
--- a/test/utils/globalSetup.ts
+++ b/test/utils/globalSetup.ts
@@ -1,4 +1,4 @@
-import { chromium } from "playwright"
+import { Cookie } from "playwright"
 import { hash } from "../../src/node/util"
 import { PASSWORD, workspaceDir } from "./constants"
 import { clean } from "./helpers"
@@ -15,31 +15,29 @@ export default async function () {
   // Cleanup workspaces from previous tests.
   await clean(workspaceDir)
 
-  const cookieToStore = {
-    sameSite: "Lax" as const,
-    name: "key",
-    value: await hash(PASSWORD),
-    domain: "localhost",
-    path: "/",
-    expires: -1,
-    httpOnly: false,
-    secure: false,
-  }
-
-  const browser = await chromium.launch()
-  const page = await browser.newPage()
-  const storage = await page.context().storageState()
-
   if (process.env.WTF_NODE) {
     wtfnode.setup()
   }
 
-  storage.cookies = [cookieToStore]
+  // TODO: Replace this with a call to code-server to get the cookie. To avoid
+  // too much overhead we can do an http POST request and avoid spawning a
+  // browser for it.
+  const cookies: Cookie[] = [
+    {
+      domain: "localhost",
+      expires: -1,
+      httpOnly: false,
+      name: "key",
+      path: "/",
+      sameSite: "Lax",
+      secure: false,
+      value: await hash(PASSWORD),
+    },
+  ]
 
   // Save storage state and store as an env variable
   // More info: https://playwright.dev/docs/auth/#reuse-authentication-state
-  process.env.STORAGE = JSON.stringify(storage)
-  await browser.close()
+  process.env.STORAGE = JSON.stringify({ cookies })
 
   console.log("✅ Global Setup for Playwright End-to-End Tests is now complete.")
 }

From 49c7cc6e8a11bcfb6e462d1e4c898952c0e0b637 Mon Sep 17 00:00:00 2001
From: Asher <ash@coder.com>
Date: Thu, 24 Jun 2021 15:08:41 -0500
Subject: [PATCH 6/8] Retain failed e2e videos only

---
 test/playwright.config.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/playwright.config.ts b/test/playwright.config.ts
index 9063a04b9..679dd33f9 100644
--- a/test/playwright.config.ts
+++ b/test/playwright.config.ts
@@ -12,7 +12,7 @@ const config: PlaywrightTestConfig = {
   // Put any shared options on the top level.
   use: {
     headless: true, // Run tests in headless browsers.
-    video: "on",
+    video: "retain-on-failure",
   },
 
   projects: [

From 43c6ffcb8f18719bcb5b36c831a918387acf84db Mon Sep 17 00:00:00 2001
From: Asher <ash@coder.com>
Date: Fri, 25 Jun 2021 11:20:44 -0500
Subject: [PATCH 7/8] Remove login steps from logout test

I figure login is already tested so we can skip this and just use the
cookie.
---
 test/e2e/logout.test.ts | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/test/e2e/logout.test.ts b/test/e2e/logout.test.ts
index ac932fa47..61a0615ac 100644
--- a/test/e2e/logout.test.ts
+++ b/test/e2e/logout.test.ts
@@ -1,19 +1,7 @@
-import { PASSWORD } from "../utils/constants"
 import { describe, test, expect } from "./baseFixture"
 
-describe("logout", false, () => {
-  test("should be able login and logout", async ({ codeServerPage }) => {
-    // Type in password
-    await codeServerPage.page.fill(".password", PASSWORD)
-    // Click the submit button and login
-    await codeServerPage.page.click(".submit")
-    await codeServerPage.page.waitForLoadState("networkidle")
-    // We do this because occassionally code-server doesn't load on Firefox
-    // but loads if you reload once or twice
-    await codeServerPage.reloadUntilEditorIsReady()
-    // Make sure the editor actually loaded
-    expect(await codeServerPage.isEditorVisible()).toBe(true)
-
+describe("logout", true, () => {
+  test("should be able logout", async ({ codeServerPage }) => {
     // Click the Application menu
     await codeServerPage.page.click("[aria-label='Application Menu']")
 

From 2238d7391e73cf08d6dad1ce33372ddb7f4d55ee Mon Sep 17 00:00:00 2001
From: Asher <ash@coder.com>
Date: Fri, 25 Jun 2021 12:05:13 -0500
Subject: [PATCH 8/8] Fix occasional logout failure

It seems a dialog sometimes appears asking if you want to lose
changes (even though we have no changes; it seems based on timers in
some way). Playwright defaults to dismissing them (so quickly you might
not even see them) so accepting instead fixes navigation to the logout
page getting canceled.
---
 test/e2e/baseFixture.ts | 5 +++++
 test/e2e/logout.test.ts | 6 ------
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/test/e2e/baseFixture.ts b/test/e2e/baseFixture.ts
index e67d09b8d..a18722ed4 100644
--- a/test/e2e/baseFixture.ts
+++ b/test/e2e/baseFixture.ts
@@ -56,6 +56,11 @@ export const test = base.extend<TestFixtures>({
   authenticated: false,
   codeServer: undefined, // No default; should be provided through `test.use`.
   codeServerPage: async ({ authenticated, codeServer, page }, use) => {
+    // It's possible code-server might prevent navigation because of unsaved
+    // changes (seems to happen based on timing even if no changes have been
+    // made too). In these cases just accept.
+    page.on("dialog", (d) => d.accept())
+
     const codeServerPage = new CodeServerPage(codeServer, page)
     await codeServerPage.setup(authenticated)
     await use(codeServerPage)
diff --git a/test/e2e/logout.test.ts b/test/e2e/logout.test.ts
index 61a0615ac..052e438d0 100644
--- a/test/e2e/logout.test.ts
+++ b/test/e2e/logout.test.ts
@@ -10,12 +10,6 @@ describe("logout", true, () => {
     expect(await codeServerPage.page.isVisible(logoutButton)).toBe(true)
 
     await codeServerPage.page.hover(logoutButton)
-    // TODO(@jsjoeio)
-    // Look into how we're attaching the handlers for the logout feature
-    // We need to see how it's done upstream and add logging to the
-    // handlers themselves.
-    // They may be attached too slowly, hence why we need this timeout
-    await codeServerPage.page.waitForTimeout(2000)
 
     // Recommended by Playwright for async navigation
     // https://github.com/microsoft/playwright/issues/1987#issuecomment-620182151