Skip to content

dev(testing): separate unit and e2e tests #2852

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Mar 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,15 @@ jobs:
with:
args: ./ci/steps/lint.sh

test:
test-unit:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- name: Run unit tests
uses: ./ci/images/debian10
with:
args: ./ci/steps/test-unit.sh
test-e2e:
needs: linux-amd64
runs-on: ubuntu-latest
env:
Expand All @@ -44,19 +52,19 @@ jobs:
run: |
cd release-packages && tar -xzf code-server*-linux-amd64.tar.gz
- uses: microsoft/playwright-github-action@v1
- name: Install dependencies and run tests
- name: Install dependencies and run end-to-end tests
run: |
./release-packages/code-server*-linux-amd64/bin/code-server --home $CODE_SERVER_ADDRESS/healthz &
yarn --frozen-lockfile
yarn test
yarn test:e2e
- name: Upload test artifacts
if: always()
uses: actions/upload-artifact@v2
with:
name: test-videos
path: ./test/videos
path: ./test/e2e/videos
- name: Remove release packages and test artifacts
run: rm -rf ./release-packages ./test/videos
run: rm -rf ./release-packages ./test/e2e/videos

release:
runs-on: ubuntu-latest
Expand Down
4 changes: 2 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ node-*
.home
coverage
**/.DS_Store
test/videos
test/screenshots
test/e2e/videos
test/e2e/screenshots
18 changes: 11 additions & 7 deletions ci/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ Make sure you have `$GITHUB_TOKEN` set and [hub](https://github.com/github/hub)

Currently, we run a command to manually generate the code coverage shield. Follow these steps:

1. Run `yarn test` and make sure all the tests are passing
1. Run `yarn test:unit` and make sure all the tests are passing
2. Run `yarn badges`
3. Go into the README and change the color from `red` to `green` in this line:

Expand All @@ -72,8 +72,10 @@ This directory contains scripts used for the development of code-server.
- Runs formatters.
- [./ci/dev/lint.sh](./dev/lint.sh) (`yarn lint`)
- Runs linters.
- [./ci/dev/test.sh](./dev/test.sh) (`yarn test`)
- Runs tests.
- [./ci/dev/test-unit.sh](./dev/test-unit.sh) (`yarn test:unit`)
- Runs unit tests.
- [./ci/dev/test-e2e.sh](./dev/test-e2e.sh) (`yarn test:e2e`)
- Runs end-to-end tests.
- [./ci/dev/ci.sh](./dev/ci.sh) (`yarn ci`)
- Runs `yarn fmt`, `yarn lint` and `yarn test`.
- [./ci/dev/watch.ts](./dev/watch.ts) (`yarn watch`)
Expand Down Expand Up @@ -142,11 +144,13 @@ This directory contains the scripts used in CI.
Helps avoid clobbering the CI configuration.

- [./steps/fmt.sh](./steps/fmt.sh)
- Runs `yarn fmt` after ensuring VS Code is patched.
- Runs `yarn fmt`.
- [./steps/lint.sh](./steps/lint.sh)
- Runs `yarn lint` after ensuring VS Code is patched.
- [./steps/test.sh](./steps/test.sh)
- Runs `yarn test` after ensuring VS Code is patched.
- Runs `yarn lint`.
- [./steps/test-unit.sh](./steps/test-unit.sh)
- Runs `yarn test:unit`.
- [./steps/test-e2e.sh](./steps/test-e2e.sh)
- Runs `yarn test:e2e`.
- [./steps/release.sh](./steps/release.sh)
- Runs the release process.
- Generates the npm package at `./release`.
Expand Down
2 changes: 1 addition & 1 deletion ci/dev/ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ main() {

yarn fmt
yarn lint
yarn test
yarn test:unit
}

main "$@"
5 changes: 1 addition & 4 deletions ci/dev/test.sh → ci/dev/test-e2e.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,9 @@ set -euo pipefail

main() {
cd "$(dirname "$0")/../.."
cd test/test-plugin
make -s out/index.js
# 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.
cd "$OLDPWD"
if [[ -z ${PASSWORD-} ]] || [[ -z ${CODE_SERVER_ADDRESS-} ]]; then
echo "The end-to-end testing suites rely on your local environment"
echo -e "\n"
Expand All @@ -21,7 +18,7 @@ main() {
echo -e "\n"
exit 1
fi
CS_DISABLE_PLUGINS=true ./test/node_modules/.bin/jest "$@"
CS_DISABLE_PLUGINS=true ./test/node_modules/.bin/jest "$@" --config ./test/jest.e2e.config.ts
}

main "$@"
15 changes: 15 additions & 0 deletions ci/dev/test-unit.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/usr/bin/env bash
set -euo pipefail

main() {
cd "$(dirname "$0")/../.."
cd test/unit/test-plugin
make -s out/index.js
# 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.
cd "$OLDPWD"
CS_DISABLE_PLUGINS=true ./test/node_modules/.bin/jest "$@"
}

main "$@"
12 changes: 12 additions & 0 deletions ci/steps/test-e2e.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#!/usr/bin/env bash
set -euo pipefail

main() {
cd "$(dirname "$0")/../.."

"./release-packages/code-server*-linux-amd64/bin/code-server" --home "$CODE_SERVER_ADDRESS"/healthz &
yarn --frozen-lockfile
yarn test:e2e
}

main "$@"
2 changes: 1 addition & 1 deletion ci/steps/test.sh → ci/steps/test-unit.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ main() {

yarn --frozen-lockfile

yarn test
yarn test:unit
}

main "$@"
9 changes: 5 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
"release:standalone": "./ci/build/build-standalone-release.sh",
"release:github-draft": "./ci/build/release-github-draft.sh",
"release:github-assets": "./ci/build/release-github-assets.sh",
"test:e2e": "./ci/dev/test-e2e.sh",
"test:standalone-release": "./ci/build/test-standalone-release.sh",
"test:unit": "./ci/dev/test-unit.sh",
"package": "./ci/build/build-packages.sh",
"postinstall": "./ci/dev/postinstall.sh",
"update:vscode": "./ci/dev/update-vscode.sh",
Expand Down Expand Up @@ -124,7 +126,8 @@
"testPathIgnorePatterns": [
"node_modules",
"lib",
"out"
"out",
"test/e2e"
],
"collectCoverage": true,
"collectCoverageFrom": [
Expand All @@ -144,8 +147,6 @@
"lines": 40
}
},
"testTimeout": 30000,
"globalSetup": "<rootDir>/test/globalSetup.ts",
"modulePathIgnorePatterns": [
"<rootDir>/lib/vscode",
"<rootDir>/release-packages",
Expand All @@ -156,7 +157,7 @@
"<rootDir>/release-images"
],
"moduleNameMapper": {
"^.+\\.(css|less)$": "<rootDir>/test/cssStub.ts"
"^.+\\.(css|less)$": "<rootDir>/test/utils/cssStub.ts"
}
}
}
2 changes: 1 addition & 1 deletion test/e2e.test.ts → test/e2e/e2e.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { chromium, Page, Browser } from "playwright"
import { CODE_SERVER_ADDRESS } from "./constants"
import { CODE_SERVER_ADDRESS } from "../utils/constants"

let browser: Browser
let page: Page
Expand Down
8 changes: 4 additions & 4 deletions test/goHome.test.ts → test/e2e/goHome.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { chromium, Page, Browser, BrowserContext, Cookie } from "playwright"
import { hash } from "../src/node/util"
import { CODE_SERVER_ADDRESS, PASSWORD, STORAGE } from "./constants"
import { createCookieIfDoesntExist } from "./helpers"
import { hash } from "../../src/node/util"
import { CODE_SERVER_ADDRESS, PASSWORD, STORAGE, E2E_VIDEO_DIR } from "../utils/constants"
import { createCookieIfDoesntExist } from "../utils/helpers"

describe("go home", () => {
let browser: Browser
Expand Down Expand Up @@ -45,7 +45,7 @@ describe("go home", () => {

context = await browser.newContext({
storageState: { cookies: maybeUpdatedCookies },
recordVideo: { dir: "./test/videos/" },
recordVideo: { dir: E2E_VIDEO_DIR },
})
})

Expand Down
2 changes: 1 addition & 1 deletion test/login.test.ts → test/e2e/login.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { chromium, Page, Browser, BrowserContext } from "playwright"
import { CODE_SERVER_ADDRESS, PASSWORD } from "./constants"
import { CODE_SERVER_ADDRESS, PASSWORD } from "../utils/constants"

describe("login", () => {
let browser: Browser
Expand Down
22 changes: 22 additions & 0 deletions test/jest.e2e.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// jest.config.ts
import type { Config } from "@jest/types"

const config: Config.InitialOptions = {
transform: {
"^.+\\.ts$": "<rootDir>/node_modules/ts-jest",
},
globalSetup: "<rootDir>/utils/globalSetup.ts",
testEnvironment: "node",
testPathIgnorePatterns: ["node_modules", "lib", "out", "test/unit"],
testTimeout: 30000,
modulePathIgnorePatterns: [
"<rootDir>/../lib/vscode",
"<rootDir>/../release-packages",
"<rootDir>/../release",
"<rootDir>/../release-standalone",
"<rootDir>/../release-npm-package",
"<rootDir>/../release-gcp",
"<rootDir>/../release-images",
],
}
export default config
4 changes: 2 additions & 2 deletions test/cli.test.ts → test/unit/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import * as fs from "fs-extra"
import * as net from "net"
import * as os from "os"
import * as path from "path"
import { Args, parse, setDefaults, shouldOpenInExistingInstance } from "../src/node/cli"
import { paths, tmpdir } from "../src/node/util"
import { Args, parse, setDefaults, shouldOpenInExistingInstance } from "../../src/node/cli"
import { paths, tmpdir } from "../../src/node/util"

type Mutable<T> = {
-readonly [P in keyof T]: T[P]
Expand Down
6 changes: 3 additions & 3 deletions test/constants.test.ts → test/unit/constants.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { commit, getPackageJson, version } from "../src/node/constants"
import { loggerModule } from "./helpers"
import { commit, getPackageJson, version } from "../../src/node/constants"
import { loggerModule } from "../utils/helpers"

// jest.mock is hoisted above the imports so we must use `require` here.
jest.mock("@coder/logger", () => require("./helpers").loggerModule)
jest.mock("@coder/logger", () => require("../utils/helpers").loggerModule)

describe("constants", () => {
describe("getPackageJson", () => {
Expand Down
4 changes: 2 additions & 2 deletions test/emitter.test.ts → test/unit/emitter.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// Note: we need to import logger from the root
// because this is the logger used in logError in ../src/common/util
import { logger } from "../node_modules/@coder/logger"
import { logger } from "../../node_modules/@coder/logger"

import { Emitter } from "../src/common/emitter"
import { Emitter } from "../../src/common/emitter"

describe("emitter", () => {
let spy: jest.SpyInstance
Expand Down
4 changes: 2 additions & 2 deletions test/health.test.ts → test/unit/health.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as httpserver from "./httpserver"
import * as integration from "./integration"
import * as httpserver from "../utils/httpserver"
import * as integration from "../utils/integration"

describe("health", () => {
let codeServer: httpserver.HttpServer | undefined
Expand Down
2 changes: 1 addition & 1 deletion test/http.test.ts → test/unit/http.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { HttpCode, HttpError } from "../src/common/http"
import { HttpCode, HttpError } from "../../src/common/http"

describe("http", () => {
describe("HttpCode", () => {
Expand Down
10 changes: 5 additions & 5 deletions test/plugin.test.ts → test/unit/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ import { logger } from "@coder/logger"
import * as express from "express"
import * as fs from "fs"
import * as path from "path"
import { HttpCode } from "../src/common/http"
import { AuthType } from "../src/node/cli"
import { codeServer, PluginAPI } from "../src/node/plugin"
import * as apps from "../src/node/routes/apps"
import * as httpserver from "./httpserver"
import { HttpCode } from "../../src/common/http"
import { AuthType } from "../../src/node/cli"
import { codeServer, PluginAPI } from "../../src/node/plugin"
import * as apps from "../../src/node/routes/apps"
import * as httpserver from "../utils/httpserver"
const fsp = fs.promises

// Jest overrides `require` so our usual override doesn't work.
Expand Down
4 changes: 2 additions & 2 deletions test/proxy.test.ts → test/unit/proxy.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import bodyParser from "body-parser"
import * as express from "express"
import * as httpserver from "./httpserver"
import * as integration from "./integration"
import * as httpserver from "../utils/httpserver"
import * as integration from "../utils/integration"

describe("proxy", () => {
const nhooyrDevServer = new httpserver.HttpServer()
Expand Down
8 changes: 4 additions & 4 deletions test/register.test.ts → test/unit/register.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { JSDOM } from "jsdom"
import { loggerModule } from "./helpers"
import { loggerModule } from "../utils/helpers"

describe("register", () => {
describe("when navigator and serviceWorker are defined", () => {
Expand Down Expand Up @@ -40,7 +40,7 @@ describe("register", () => {

it("should register a ServiceWorker", () => {
// Load service worker like you would in the browser
require("../src/browser/register")
require("../../src/browser/register")
expect(mockRegisterFn).toHaveBeenCalled()
expect(mockRegisterFn).toHaveBeenCalledTimes(1)
})
Expand All @@ -54,7 +54,7 @@ describe("register", () => {
})

// Load service worker like you would in the browser
require("../src/browser/register")
require("../../src/browser/register")

expect(mockRegisterFn).toHaveBeenCalled()
expect(loggerModule.logger.error).toHaveBeenCalled()
Expand All @@ -78,7 +78,7 @@ describe("register", () => {

it("should log an error to the console", () => {
// Load service worker like you would in the browser
require("../src/browser/register")
require("../../src/browser/register")
expect(spy).toHaveBeenCalled()
expect(spy).toHaveBeenCalledTimes(1)
expect(spy).toHaveBeenCalledWith("[Service Worker] navigator is undefined")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ describe("serviceWorker", () => {
})

it("should add 3 listeners: install, activate and fetch", () => {
require("../src/browser/serviceWorker.ts")
require("../../src/browser/serviceWorker.ts")
const listenerEventNames = listeners.map((listener) => listener.event)

expect(listeners).toHaveLength(3)
Expand All @@ -68,20 +68,20 @@ describe("serviceWorker", () => {
})

it("should call the proper callbacks for 'install'", async () => {
require("../src/browser/serviceWorker.ts")
require("../../src/browser/serviceWorker.ts")
emit("install")
expect(spy).toHaveBeenCalledWith("[Service Worker] installed")
expect(spy).toHaveBeenCalledTimes(1)
})

it("should do nothing when 'fetch' is called", async () => {
require("../src/browser/serviceWorker.ts")
require("../../src/browser/serviceWorker.ts")
emit("fetch")
expect(spy).not.toHaveBeenCalled()
})

it("should call the proper callbacks for 'activate'", async () => {
require("../src/browser/serviceWorker.ts")
require("../../src/browser/serviceWorker.ts")
emit("activate")

// Activate serviceWorker
Expand Down
Loading