refactor: move vscode tests to e2e (#5911)

* wip: migrate vscode tests to e2e

* feat: add codeWorkspace to global setup

* refactor: only use dir in spawn when we should

* wip: migrate more tests

* refactor: move all vscode tests to e2e

* refactor(ci): move unit to own job

* fixup: add codecov to unit test step

* Update test/e2e/models/CodeServer.ts

* Update test/e2e/models/CodeServer.ts

* docs: add note about intercept requests

* refactor: rm unused clean() calls

* refactor: delete duplicate test

* refactor: update 'should not redirect' test

.github/workflows/build.yaml

@@ -139,6 +139,55 @@ jobs:
         if: steps.changed-files.outputs.any_changed == 'true'
         run: yarn lint:ts
 
+  test-unit:
+    name: Run unit tests
+    runs-on: ubuntu-20.04
+    timeout-minutes: 5
+    steps:
+      - name: Checkout repo
+        uses: actions/checkout@v3
+        with:
+          fetch-depth: 2
+
+      - name: Get changed files
+        id: changed-files
+        uses: tj-actions/changed-files@v26.1
+        with:
+          files: |
+            **/*.ts
+          files_ignore: |
+            lib/vscode/**
+
+      - name: Install Node.js v16
+        if: steps.changed-files.outputs.any_changed == 'true'
+        uses: actions/setup-node@v3
+        with:
+          node-version: "16"
+
+      - name: Fetch dependencies from cache
+        if: steps.changed-files.outputs.any_changed == 'true'
+        id: cache-node-modules
+        uses: actions/cache@v3
+        with:
+          path: "**/node_modules"
+          key: yarn-build-${{ hashFiles('**/yarn.lock') }}
+          restore-keys: |
+            yarn-build-
+
+      - name: Install dependencies
+        if: steps.changed-files.outputs.any_changed == 'true' && steps.cache-node-modules.outputs.cache-hit != 'true'
+        run: SKIP_SUBMODULE_DEPS=1 yarn --frozen-lockfile
+
+      - name: Run unit tests
+        if: steps.changed-files.outputs.any_changed == 'true'
+        run: yarn test:unit
+
+      - name: Upload coverage report to Codecov
+        uses: codecov/codecov-action@v3
+        with:
+          token: ${{ secrets.CODECOV_TOKEN }}
+        if: success()
+
   build:
     name: Build code-server
     runs-on: ubuntu-20.04
@@ -206,19 +255,6 @@ jobs:
         if: steps.cache-vscode.outputs.cache-hit != 'true'
         run: yarn build:vscode
 
-      # Our code imports code from VS Code's `out` directory meaning VS Code
-      # must be built before running these tests.
-      # TODO: Move to its own step?
-      - name: Run code-server unit tests
-        run: yarn test:unit
-        if: success()
-
-      - name: Upload coverage report to Codecov
-        uses: codecov/codecov-action@v3
-        with:
-          token: ${{ secrets.CODECOV_TOKEN }}
-        if: success()
-
       # The release package does not contain any native modules
       # and is neutral to architecture/os/libc version.
       - name: Create release package

ci/dev/test-unit.sh

@@ -11,22 +11,6 @@ main() {
   make -s out/index.js
   popd
 
-  # Our code imports from `out` in order to work during development but if you
-  # have only built for production you will have not have this directory.  In
-  # that case symlink `out` to a production build directory.
-  if [[ ! -e lib/vscode/out ]]; then
-    pushd lib
-    local out=(vscode-reh-web-*)
-    if [[ -d "${out[0]}" ]]; then
-      ln -s "../${out[0]}/out" ./vscode/out
-    else
-      echo "Could not find lib/vscode/out or lib/vscode-reh-web-*"
-      echo "Code must be built before running unit tests"
-      exit 1
-    fi
-    popd
-  fi
-
   # We must keep jest in a sub-directory. See ../../test/package.json for more
   # information. We must also run it from the root otherwise coverage will not
   # include our source files.

test/e2e/models/CodeServer.ts

@@ -128,6 +128,8 @@ export class CodeServer {
         path.join(dir, "extensions"),
         "--auth",
         "none",
+        // The workspace to open.
+        ...(this.args.includes("--ignore-last-opened") ? [] : [dir]),
         ...this.args,
         // Using port zero will spawn on a random port.
         "--bind-addr",
@@ -139,8 +141,6 @@ export class CodeServer {
         path.join(dir, "config.yaml"),
         "--user-data-dir",
         dir,
-        // The last argument is the workspace to open.
-        dir,
       ]
       this.logger.debug("spawning `node " + args.join(" ") + "`")
       const proc = cp.spawn("node", args, {

test/e2e/routes.test.ts

@@ -0,0 +1,118 @@
+import { describe, test, expect } from "./baseFixture"
+import { clean, tmpdir } from "../utils/helpers"
+import * as path from "path"
+import { promises as fs } from "fs"
+
+const routes = ["/", "/vscode", "/vscode/"]
+
+describe("VS Code Routes", ["--disable-workspace-trust"], {}, async () => {
+  const testName = "vscode-routes-default"
+  test.beforeAll(async () => {
+    await clean(testName)
+  })
+
+  test("should load all route variations", async ({ codeServerPage }) => {
+    for (const route of routes) {
+      await codeServerPage.navigate(route)
+
+      // Check there were no redirections
+      const url = new URL(codeServerPage.page.url())
+      expect(url.pathname).toBe(route)
+
+      // TODO@jsjoeio 
+      // now that we are in a proper browser instead of scraping the HTML we
+      // could possibly intercept requests to make sure assets are loading from
+      // the right spot.
+      //
+      // Check that page loaded from correct route
+      const html = await codeServerPage.page.innerHTML("html")
+      switch (route) {
+        case "/":
+        case "/vscode/":
+          expect(html).toMatch(/src="\.\/[a-z]+-[0-9a-z]+\/static\//)
+          break
+        case "/vscode":
+          expect(html).toMatch(/src="\.\/vscode\/[a-z]+-[0-9a-z]+\/static\//)
+          break
+      }
+    }
+  })
+})
+
+const CODE_WORKSPACE_DIR = process.env.CODE_WORKSPACE_DIR || ""
+describe("VS Code Routes with code-workspace", ["--disable-workspace-trust", CODE_WORKSPACE_DIR], {}, async () => {
+  test("should redirect to the passed in workspace using human-readable query", async ({ codeServerPage }) => {
+    const url = new URL(codeServerPage.page.url())
+    expect(url.pathname).toBe("/")
+    expect(url.search).toBe(`?workspace=${CODE_WORKSPACE_DIR}`)
+  })
+})
+
+const CODE_FOLDER_DIR = process.env.CODE_FOLDER_DIR || ""
+describe("VS Code Routes with code-workspace", ["--disable-workspace-trust", CODE_FOLDER_DIR], {}, async () => {
+  test("should redirect to the passed in folder using human-readable query", async ({ codeServerPage }) => {
+    const url = new URL(codeServerPage.page.url())
+    expect(url.pathname).toBe("/")
+    expect(url.search).toBe(`?folder=${CODE_FOLDER_DIR}`)
+  })
+})
+
+describe(
+  "VS Code Routes with ignore-last-opened",
+  ["--disable-workspace-trust", "--ignore-last-opened"],
+  {},
+  async () => {
+    test("should not redirect", async ({ codeServerPage }) => {
+      const folder = process.env.CODE_FOLDER_DIR
+
+      await codeServerPage.navigate(`/`)
+      await codeServerPage.navigate(`/?folder=${folder}`)
+      await codeServerPage.navigate(`/`)
+
+      const url = new URL(codeServerPage.page.url())
+      expect(url.pathname).toBe("/")
+      expect(url.search).toBe("")
+    })
+  },
+)
+
+describe(
+  "VS Code Routes with no workspace or folder",
+  ["--disable-workspace-trust"],
+  {},
+  async () => {
+    test("should redirect to last query folder/workspace", async ({ codeServerPage }) => {
+      const folder = process.env.CODE_FOLDER_DIR
+      const workspace = process.env.CODE_WORKSPACE_DIR
+      await codeServerPage.navigate(`/?folder=${folder}&workspace=${workspace}`)
+
+      // If you visit again without query parameters it will re-attach them by
+      // redirecting.  It should always redirect to the same route.
+      for (const route of routes) {
+        await codeServerPage.navigate(route)
+        const url = new URL(codeServerPage.page.url())
+        expect(url.pathname).toBe(route)
+        expect(url.search).toBe(`?folder=${folder}&workspace=${workspace}`)
+      }
+    })
+  },
+)
+
+describe(
+  "VS Code Routes with no workspace or folder",
+  ["--disable-workspace-trust"],
+  {},
+  async () => {
+    test("should not redirect if ew passed in", async ({ codeServerPage }) => {
+      const folder = process.env.CODE_FOLDER_DIR
+      const workspace = process.env.CODE_WORKSPACE_DIR
+      await codeServerPage.navigate(`/?folder=${folder}&workspace=${workspace}`)
+
+      // Closing the folder should stop the redirecting.
+      await codeServerPage.navigate("/?ew=true")
+      let url = new URL(codeServerPage.page.url())
+      expect(url.pathname).toBe("/")
+      expect(url.search).toBe("?ew=true")
+    })
+  },
+)

test/unit/node/routes/vscode.test.ts

@@ -1,137 +0,0 @@
-import { promises as fs } from "fs"
-import * as path from "path"
-import { clean, tmpdir } from "../../../utils/helpers"
-import * as httpserver from "../../../utils/httpserver"
-import * as integration from "../../../utils/integration"
-
-// TODO@jsjoeio - move these to integration tests since they rely on Code
-// to be built
-describe.skip("vscode", () => {
-  let codeServer: httpserver.HttpServer | undefined
-
-  const testName = "vscode"
-  beforeAll(async () => {
-    await clean(testName)
-  })
-
-  afterEach(async () => {
-    if (codeServer) {
-      await codeServer.dispose()
-      codeServer = undefined
-    }
-  })
-
-  const routes = ["/", "/vscode", "/vscode/"]
-
-  it("should load all route variations", async () => {
-    codeServer = await integration.setup(["--auth=none"], "")
-
-    for (const route of routes) {
-      const resp = await codeServer.fetch(route)
-      expect(resp.status).toBe(200)
-      const html = await resp.text()
-      const url = new URL(resp.url) // Check there were no redirections.
-      expect(url.pathname + url.search).toBe(route)
-      switch (route) {
-        case "/":
-        case "/vscode/":
-          expect(html).toMatch(/src="\.\/[a-z]+-[0-9a-z]+\/static\//)
-          break
-        case "/vscode":
-          expect(html).toMatch(/src="\.\/vscode\/[a-z]+-[0-9a-z]+\/static\//)
-          break
-      }
-    }
-  })
-
-  it("should redirect to the passed in workspace using human-readable query", async () => {
-    const workspace = path.join(await tmpdir(testName), "test.code-workspace")
-    await fs.writeFile(workspace, "")
-    codeServer = await integration.setup(["--auth=none", workspace], "")
-
-    const resp = await codeServer.fetch("/")
-    const url = new URL(resp.url)
-    expect(url.pathname).toBe("/")
-    expect(url.search).toBe(`?workspace=${workspace}`)
-  })
-
-  it("should redirect to the passed in folder using human-readable query", async () => {
-    const folder = await tmpdir(testName)
-    codeServer = await integration.setup(["--auth=none", folder], "")
-
-    const resp = await codeServer.fetch("/")
-    const url = new URL(resp.url)
-    expect(url.pathname).toBe("/")
-    expect(url.search).toBe(`?folder=${folder}`)
-  })
-
-  it("should redirect to last query folder/workspace", async () => {
-    codeServer = await integration.setup(["--auth=none"], "")
-
-    const folder = await tmpdir(testName)
-    const workspace = path.join(await tmpdir(testName), "test.code-workspace")
-    await fs.writeFile(workspace, "")
-    let resp = await codeServer.fetch("/", undefined, {
-      folder,
-      workspace,
-    })
-    expect(resp.status).toBe(200)
-    await resp.text()
-
-    // If you visit again without query parameters it will re-attach them by
-    // redirecting.  It should always redirect to the same route.
-    for (const route of routes) {
-      resp = await codeServer.fetch(route)
-      const url = new URL(resp.url)
-      expect(url.pathname).toBe(route)
-      expect(url.search).toBe(`?folder=${folder}&workspace=${workspace}`)
-      await resp.text()
-    }
-
-    // Closing the folder should stop the redirecting.
-    resp = await codeServer.fetch("/", undefined, { ew: "true" })
-    let url = new URL(resp.url)
-    expect(url.pathname).toBe("/")
-    expect(url.search).toBe("?ew=true")
-    await resp.text()
-
-    resp = await codeServer.fetch("/")
-    url = new URL(resp.url)
-    expect(url.pathname).toBe("/")
-    expect(url.search).toBe("")
-    await resp.text()
-  })
-
-  it("should do nothing when nothing is passed in", async () => {
-    codeServer = await integration.setup(["--auth=none"], "")
-
-    const resp = await codeServer.fetch("/", undefined)
-
-    expect(resp.status).toBe(200)
-    const url = new URL(resp.url)
-    expect(url.search).toBe("")
-    await resp.text()
-  })
-
-  it("should not redirect when last opened is ignored", async () => {
-    codeServer = await integration.setup(["--auth=none", "--ignore-last-opened"], "")
-
-    const folder = await tmpdir(testName)
-    const workspace = path.join(await tmpdir(testName), "test.code-workspace")
-    await fs.writeFile(workspace, "")
-
-    let resp = await codeServer.fetch("/", undefined, {
-      folder,
-      workspace,
-    })
-    expect(resp.status).toBe(200)
-    await resp.text()
-
-    // No redirections.
-    resp = await codeServer.fetch("/")
-    const url = new URL(resp.url)
-    expect(url.pathname).toBe("/")
-    expect(url.search).toBe("")
-    await resp.text()
-  })
-})

test/utils/globalE2eSetup.ts

@@ -1,6 +1,8 @@
 import { workspaceDir } from "./constants"
-import { clean } from "./helpers"
+import { clean, tmpdir } from "./helpers"
 import * as wtfnode from "./wtfnode"
+import * as path from "path"
+import { promises as fs } from "fs"
 
 /**
  * Perform workspace cleanup and authenticate. This should be ran before e2e
@@ -17,5 +19,14 @@ export default async function () {
     wtfnode.setup()
   }
 
+  // Create dummy code-workspace for routes.test.ts
+  const codeWorkspace = path.join(await tmpdir(workspaceDir), "test.code-workspace")
+  await fs.writeFile(codeWorkspace, "")
+  process.env.CODE_WORKSPACE_DIR = codeWorkspace
+
+  // Create dummy folder for routes.test.ts
+  const folder = await tmpdir(workspaceDir)
+  process.env.CODE_FOLDER_DIR = folder
+
   console.log("✅ Global Setup for Playwright End-to-End Tests is now complete.")
 }