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

@@ -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()

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 "$@"

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

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",

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 }) => {

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,
   })

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({

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`)
   })
 })

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"
-
-  constructor(public readonly page: Page) {}
+interface CodeServerProcess {
+  process: cp.ChildProcess
+  address: string
+}
 
 /**
-   * Navigates to CODE_SERVER_ADDRESS. It will open a newly created random
-   * directory.
+ * 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)
+  }
+
+  /**
+   * 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
-  }
 }

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,
   })

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"

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.

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"

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 })
 }
 
 /**