refactor: make e2e work with reverse proxy

This refactors the e2e test in a couple ways:
- remove setting cookie in localStorage (instead we pass --auth none)
- refactor address() method to account for reverse proxy logic

Author: jsjoeio

test/e2e/baseFixture.ts

@@ -1,4 +1,3 @@
-import { field, logger } from "@coder/logger"
 import { test as base } from "@playwright/test"
 import { CodeServer, CodeServerPage } from "./models/CodeServer"
 
@@ -11,37 +10,24 @@ import { CodeServer, CodeServerPage } from "./models/CodeServer"
  */
 export const describe = (
   name: string,
-  includeCredentials: boolean,
   codeServerArgs: string[],
   codeServerEnv: NodeJS.ProcessEnv,
   fn: (codeServer: CodeServer) => void,
 ) => {
   test.describe(name, () => {
     // This will spawn on demand so nothing is necessary on before.
-    const codeServer = new CodeServer(name, codeServerArgs, codeServerEnv)
+    const codeServer = new CodeServer(name, codeServerArgs, codeServerEnv, undefined)
 
     // 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 : {},
       // NOTE@jsjoeio some tests use --cert which uses a self-signed certificate
       // without this option, those tests will fail.
       ignoreHTTPSErrors: true,
@@ -52,7 +38,6 @@ export const describe = (
 }
 
 interface TestFixtures {
-  authenticated: boolean
   codeServer: CodeServer
   codeServerPage: CodeServerPage
 }
@@ -62,15 +47,14 @@ interface TestFixtures {
  * 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) => {
+  codeServerPage: async ({ 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, authenticated)
+    const codeServerPage = new CodeServerPage(codeServer, page)
     await codeServerPage.navigate()
     await use(codeServerPage)
   },

test/e2e/codeServer.test.ts

@@ -3,10 +3,11 @@ import { promises as fs } from "fs"
 import * as os from "os"
 import * as path from "path"
 import * as util from "util"
+import { getMaybeProxiedCodeServer } from "../utils/helpers"
 import { describe, test, expect } from "./baseFixture"
 import { CodeServer } from "./models/CodeServer"
 
-describe("code-server", true, [], {}, () => {
+describe("code-server", [], {}, () => {
   // TODO@asher: Generalize this?  Could be nice if we were to ever need
   // multiple migration tests in other suites.
   const instances = new Map<string, CodeServer>()
@@ -48,7 +49,8 @@ describe("code-server", true, [], {}, () => {
     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(await codeServerPage.address())
+    const address = await getMaybeProxiedCodeServer(codeServerPage)
+    expect(url).toMatch(address)
   })
 
   test("should always see the code-server editor", async ({ codeServerPage }) => {
@@ -70,7 +72,9 @@ describe("code-server", true, [], {}, () => {
   test("should migrate state to avoid collisions", async ({ codeServerPage }) => {
     // This can take a very long time in development because of how long pages
     // take to load and we are doing a lot of that here.
-    test.slow()
+    if (process.env.VSCODE_DEV === "1") {
+      test.slow()
+    }
 
     const dir = await codeServerPage.workspaceDir
     const files = [path.join(dir, "foo"), path.join(dir, "bar")]
@@ -90,6 +94,7 @@ describe("code-server", true, [], {}, () => {
     // domain and can write to the same database.
     const cs = await spawn("4.0.2", dir)
     const address = new URL(await cs.address())
+
     await codeServerPage.navigate("/proxy/" + address.port + "/")
     await codeServerPage.openFile(files[1])
     expect(await codeServerPage.tabIsVisible(files[0])).toBe(false)

test/e2e/downloads.test.ts

@@ -3,7 +3,7 @@ import * as path from "path"
 import { clean } from "../utils/helpers"
 import { describe, test, expect } from "./baseFixture"
 
-describe("Downloads (enabled)", true, [], {}, async () => {
+describe("Downloads (enabled)", [], {}, async () => {
   const testName = "downloads-enabled"
   test.beforeAll(async () => {
     await clean(testName)
@@ -25,7 +25,7 @@ describe("Downloads (enabled)", true, [], {}, async () => {
   })
 })
 
-describe("Downloads (disabled)", true, ["--disable-file-downloads"], {}, async () => {
+describe("Downloads (disabled)", ["--disable-file-downloads"], {}, async () => {
   const testName = "downloads-disabled"
   test.beforeAll(async () => {
     await clean(testName)

test/e2e/extensions.test.ts

@@ -1,23 +1,36 @@
 import * as path from "path"
-import { describe, test } from "./baseFixture"
+import { test as base } from "@playwright/test"
+import { describe, test, expect } from "./baseFixture"
+import { getMaybeProxiedCodeServer } from "../utils/helpers"
 
 function runTestExtensionTests() {
   // This will only work if the test extension is loaded into code-server.
   test("should have access to VSCODE_PROXY_URI", async ({ codeServerPage }) => {
-    const address = await codeServerPage.address()
+    const address = await getMaybeProxiedCodeServer(codeServerPage)
 
     await codeServerPage.executeCommandViaMenus("code-server: Get proxy URI")
 
-    await codeServerPage.page.waitForSelector(`text=${address}/proxy/{{port}}`)
+    const text = await codeServerPage.page.locator(".notification-list-item-message").textContent()
+    // Remove end slash in address
+    const normalizedAddress = address.replace(/\/+$/, "")
+    expect(text).toBe(`${normalizedAddress}/proxy/{{port}}`)
   })
 }
 
 const flags = ["--extensions-dir", path.join(__dirname, "./extensions")]
 
-describe("Extensions", true, flags, {}, () => {
+describe("Extensions", flags, {}, () => {
   runTestExtensionTests()
 })
 
-describe("Extensions with --cert", true, [...flags, "--cert"], {}, () => {
-  runTestExtensionTests()
-})
+if (process.env.USE_PROXY !== "1") {
+  describe("Extensions with --cert", [...flags, "--cert"], {}, () => {
+    runTestExtensionTests()
+  })
+} else {
+  base.describe("Extensions with --cert", () => {
+    base.skip("skipped because USE_PROXY is set", () => {
+      // Playwright will not show this without a function.
+    })
+  })
+}

test/e2e/github.test.ts

@@ -2,7 +2,7 @@ import { test as base } from "@playwright/test"
 import { describe, expect, test } from "./baseFixture"
 
 if (process.env.GITHUB_TOKEN) {
-  describe("GitHub token", true, [], {}, () => {
+  describe("GitHub token", [], {}, () => {
     test("should be logged in to pull requests extension", async ({ codeServerPage }) => {
       await codeServerPage.exec("git init")
       await codeServerPage.exec("git remote add origin https://github.com/coder/code-server")
@@ -16,7 +16,7 @@ if (process.env.GITHUB_TOKEN) {
     })
   })
 
-  describe("No GitHub token", true, [], { GITHUB_TOKEN: "" }, () => {
+  describe("No GitHub token", [], { GITHUB_TOKEN: "" }, () => {
     test("should not be logged in to pull requests extension", async ({ codeServerPage }) => {
       await codeServerPage.exec("git init")
       await codeServerPage.exec("git remote add origin https://github.com/coder/code-server")

test/e2e/globalSetup.test.ts

@@ -1,7 +1,7 @@
 import { PASSWORD } from "../utils/constants"
 import { describe, test, expect } from "./baseFixture"
 
-describe("login", false, [], {}, () => {
+describe("login", ["--auth", "password"], {}, () => {
   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/login.test.ts

@@ -4,10 +4,10 @@ import { promises as fs } from "fs"
 import * as path from "path"
 import { Page } from "playwright"
 import * as util from "util"
-import { logError, plural } from "../../../src/common/util"
+import { logError, normalize, plural } from "../../../src/common/util"
 import { onLine } from "../../../src/node/util"
 import { PASSWORD, workspaceDir } from "../../utils/constants"
-import { idleTimer, tmpdir } from "../../utils/helpers"
+import { getMaybeProxiedCodeServer, idleTimer, tmpdir } from "../../utils/helpers"
 
 interface CodeServerProcess {
   process: cp.ChildProcess
@@ -58,6 +58,15 @@ export class CodeServer {
       this.process = this.spawn()
     }
     const { address } = await this.process
+
+    // NOTE@jsjoeio - when enabled, we assume code-server is running
+    // via a reverse proxy with something like Caddy
+    // and being accessed at host/port i.e. localhost:8000/1337
+    // if (process.env.USE_PROXY && process.env.USE_PROXY === "1") {
+    // const uri = new URL(address)
+    // return `http://${uri.hostname}:8000/${uri.port}/ide/`
+    // }
+
     return address
   }
 
@@ -104,6 +113,8 @@ export class CodeServer {
         this.entry,
         "--extensions-dir",
         path.join(dir, "extensions"),
+        "--auth",
+        "none",
         ...this.args,
         // Using port zero will spawn on a random port.
         "--bind-addr",
@@ -124,6 +135,10 @@ export class CodeServer {
         env: {
           ...process.env,
           ...this.env,
+          // Set to empty string to prevent code-server from
+          // using the existing instance when running the e2e tests
+          // from an integrated terminal.
+          VSCODE_IPC_HOOK_CLI: "",
           PASSWORD,
         },
       })
@@ -183,6 +198,13 @@ export class CodeServer {
       proc.kill()
     }
   }
+
+  /**
+   * Whether or not authentication is enabled.
+   */
+  authEnabled(): boolean {
+    return this.args.includes("password")
+  }
 }
 
 /**
@@ -195,11 +217,7 @@ export class CodeServer {
 export class CodeServerPage {
   private readonly editorSelector = "div.monaco-workbench"
 
-  constructor(
-    private readonly codeServer: CodeServer,
-    public readonly page: Page,
-    private readonly authenticated: boolean,
-  ) {
+  constructor(private readonly codeServer: CodeServer, public readonly page: Page) {
     this.page.on("console", (message) => {
       this.codeServer.logger.debug(message.text())
     })
@@ -224,12 +242,16 @@ export class CodeServerPage {
    * editor to become available.
    */
   async navigate(endpoint = "/") {
-    const to = new URL(endpoint, await this.codeServer.address())
+    const address = await getMaybeProxiedCodeServer(this.codeServer)
+    const noramlizedUrl = normalize(address + endpoint, true)
+    const to = new URL(noramlizedUrl)
+
+    this.codeServer.logger.info(`navigating to ${to}`)
     await this.page.goto(to.toString(), { waitUntil: "networkidle" })
 
-    // Only reload editor if authenticated. Otherwise we'll get stuck
+    // Only reload editor if auth is not enabled. Otherwise we'll get stuck
     // reloading the login page.
-    if (this.authenticated) {
+    if (!this.codeServer.authEnabled()) {
       await this.reloadUntilEditorIsReady()
     }
   }

test/e2e/models/CodeServer.ts

@@ -1,7 +1,7 @@
 import { version } from "../../src/node/constants"
 import { describe, test, expect } from "./baseFixture"
 
-describe("Open Help > About", true, [], {}, () => {
+describe("Open Help > About", [], {}, () => {
   test("should see code-server version in about dialog", async ({ codeServerPage }) => {
     // Open using the menu.
     await codeServerPage.navigateMenus(["Help", "About"])

test/e2e/openHelpAbout.test.ts

@@ -2,10 +2,10 @@ import * as cp from "child_process"
 import { promises as fs } from "fs"
 import * as path from "path"
 import util from "util"
-import { clean, tmpdir } from "../utils/helpers"
+import { clean, getMaybeProxiedCodeServer, tmpdir } from "../utils/helpers"
 import { describe, expect, test } from "./baseFixture"
 
-describe("Integrated Terminal", true, [], {}, () => {
+describe("Integrated Terminal", [], {}, () => {
   const testName = "integrated-terminal"
   test.beforeAll(async () => {
     await clean(testName)
@@ -26,7 +26,8 @@ describe("Integrated Terminal", true, [], {}, () => {
     await codeServerPage.page.keyboard.press("Enter")
 
     const { stdout } = await output
-    expect(stdout).toMatch(await codeServerPage.address())
+    const address = await getMaybeProxiedCodeServer(codeServerPage)
+    expect(stdout).toMatch(address)
   })
 
   test("should be able to invoke `code-server` to open a file", async ({ codeServerPage }) => {

test/e2e/terminal.test.ts

@@ -1,2 +1,4 @@
 export const PASSWORD = "e45432jklfdsab"
 export const workspaceDir = "workspaces"
+export const REVERSE_PROXY_BASE_PATH = "ide"
+export const REVERSE_PROXY_PORT = "8000"

test/utils/constants.ts

@@ -1,7 +1,4 @@
-import { Cookie } from "playwright"
-import { CookieKeys } from "../../src/common/http"
-import { hash } from "../../src/node/util"
-import { PASSWORD, workspaceDir } from "./constants"
+import { workspaceDir } from "./constants"
 import { clean } from "./helpers"
 import * as wtfnode from "./wtfnode"
 
@@ -20,25 +17,5 @@ export default async function () {
     wtfnode.setup()
   }
 
-  // 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: CookieKeys.Session,
-      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({ cookies })
-
   console.log("✅ Global Setup for Playwright End-to-End Tests is now complete.")
 }