From b3f7a209043e01ef469e1cb69e7b8d9024b36ddf Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 21 Dec 2022 12:04:59 -0700 Subject: [PATCH] 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 | 62 +++++++++--- ci/dev/test-unit.sh | 16 ---- test/e2e/models/CodeServer.ts | 4 +- test/e2e/routes.test.ts | 118 +++++++++++++++++++++++ test/unit/node/routes/vscode.test.ts | 137 --------------------------- test/utils/globalE2eSetup.ts | 13 ++- 6 files changed, 181 insertions(+), 169 deletions(-) create mode 100644 test/e2e/routes.test.ts delete mode 100644 test/unit/node/routes/vscode.test.ts diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 2eacba567..101b7577f 100644 --- a/.github/workflows/build.yaml +++ b/.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 diff --git a/ci/dev/test-unit.sh b/ci/dev/test-unit.sh index b3e0b14c9..e312c073e 100755 --- a/ci/dev/test-unit.sh +++ b/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. diff --git a/test/e2e/models/CodeServer.ts b/test/e2e/models/CodeServer.ts index f2c2e0cc4..2fb625627 100644 --- a/test/e2e/models/CodeServer.ts +++ b/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, { diff --git a/test/e2e/routes.test.ts b/test/e2e/routes.test.ts new file mode 100644 index 000000000..70e2632ba --- /dev/null +++ b/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") + }) + }, +) \ No newline at end of file diff --git a/test/unit/node/routes/vscode.test.ts b/test/unit/node/routes/vscode.test.ts deleted file mode 100644 index 5d3dbb63d..000000000 --- a/test/unit/node/routes/vscode.test.ts +++ /dev/null @@ -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() - }) -}) diff --git a/test/utils/globalE2eSetup.ts b/test/utils/globalE2eSetup.ts index 3596ed33f..107dc85fa 100644 --- a/test/utils/globalE2eSetup.ts +++ b/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.") }