Merge pull request #3664 from code-asher/parallel-tests

Author: code-asher

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

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

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

test/e2e/baseFixture.ts

@@ -1,12 +1,71 @@
+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`.
+ *
+ * If `includeCredentials` is `true` page requests will be authenticated.
+ */
+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)
+
+    // 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()
+    })
+
+    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
+}
+
+/**
+ * Create a test that spawns code-server if necessary and ensures the page is
+ * ready.
+ */
+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)
   },
 })
 
+/** 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", true, () => {
   test("browser should display correct userAgent", async ({ codeServerPage, browserName }) => {
     const displayNames = {
       chromium: "Chrome",

test/e2e/codeServer.test.ts

@@ -1,20 +1,15 @@
-import { CODE_SERVER_ADDRESS, storageState } from "../utils/constants"
-import { test, expect } from "./baseFixture"
+import { describe, test, expect } from "./baseFixture"
 
-test.describe("CodeServer", () => {
-  test.use({
-    storageState,
-  })
-
-  test(`should navigate to ${CODE_SERVER_ADDRESS}`, async ({ codeServerPage }) => {
+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
     // which means we should be logged in
     // so it should be on the address
     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,13 +1,8 @@
-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", () => {
-  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)

test/e2e/login.test.ts

@@ -1,13 +1,7 @@
 import { PASSWORD } from "../utils/constants"
-import { test, expect } from "./baseFixture"
-
-test.describe("login", () => {
-  // Reset the browser so no cookies are persisted
-  // by emptying the storageState
-  test.use({
-    storageState: {},
-  })
+import { describe, test, expect } from "./baseFixture"
 
+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")

test/e2e/logout.test.ts

@@ -1,25 +1,7 @@
-import { CODE_SERVER_ADDRESS, PASSWORD } from "../utils/constants"
-import { test, expect } from "./baseFixture"
-
-test.describe("logout", () => {
-  // Reset the browser so no cookies are persisted
-  // by emptying the storageState
-  test.use({
-    storageState: {},
-  })
-
-  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)
+import { describe, test, expect } from "./baseFixture"
 
+describe("logout", true, () => {
+  test("should be able logout", async ({ codeServerPage }) => {
     // Click the Application menu
     await codeServerPage.page.click("[aria-label='Application Menu']")
 
@@ -28,17 +10,11 @@ test.describe("logout", () => {
     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
     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`)
   })
 })