Skip to content

Commit

Permalink
Merge pull request #2852 from cdr/jsjoeio-2646-separate-testing
Browse files Browse the repository at this point in the history
dev(testing): separate unit and e2e tests
  • Loading branch information
jsjoeio committed Mar 15, 2021
2 parents 0506875 + 7e23575 commit ca56440
Show file tree
Hide file tree
Showing 41 changed files with 136 additions and 76 deletions.
18 changes: 13 additions & 5 deletions .github/workflows/ci.yaml
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
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
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
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
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
@@ -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
@@ -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
Expand Up @@ -6,7 +6,7 @@ main() {

yarn --frozen-lockfile

yarn test
yarn test:unit
}

main "$@"
9 changes: 5 additions & 4 deletions package.json
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
@@ -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
@@ -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
@@ -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
@@ -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
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
@@ -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
@@ -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
@@ -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
@@ -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
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
@@ -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
@@ -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
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

0 comments on commit ca56440

Please sign in to comment.