From b30aefcfb162e80fe1080e561b5a5d8a4d83b57a Mon Sep 17 00:00:00 2001 From: Asher Date: Tue, 4 Feb 2020 15:00:44 -0600 Subject: [PATCH] Add linting steps and improve testing steps --- .drone.yml | 145 +++++++++++++++++++++++++++----------------- package.json | 3 +- scripts/cacher.sh | 13 +++- scripts/ci.bash | 5 +- scripts/test.sh | 19 ++++++ src/node/entry.ts | 18 +++++- src/node/wrapper.ts | 59 ++++++++++-------- 7 files changed, 175 insertions(+), 87 deletions(-) create mode 100755 scripts/test.sh diff --git a/.drone.yml b/.drone.yml index 82693edd..904ff5f7 100644 --- a/.drone.yml +++ b/.drone.yml @@ -6,22 +6,30 @@ platform: arch: amd64 steps: -- name: submodules - image: alpine/git - commands: - - git submodule update --init - - name: cache:restore + image: codercom/nbin:centos + commands: + - yum install -y libxkbfile-devel libsecret-devel + - . /opt/rh/devtoolset-6/enable + - ./scripts/cacher.sh + +- name: lint image: node:12 commands: - - ./scripts/cacher.sh + - yarn lint + +- name: test + image: codercom/nbin:centos + commands: + - yum install -y openssl + - yarn test - name: build image: codercom/nbin:centos commands: - yum install -y libxkbfile-devel libsecret-devel - . /opt/rh/devtoolset-6/enable - - timeout 50m ./scripts/ci.bash || echo 'Timed out or failed; continuing so we can preserve cache for the next run' + - timeout 50m ./scripts/ci.bash || echo 'Timed out or failed; continuing to preserve cache for the next run' - name: cache:package image: node:12 @@ -40,10 +48,10 @@ steps: when: event: push -- name: test - image: node:12 +- name: test:build + image: codercom/nbin:centos commands: - - yarn test + - yarn test:build - name: publish:github image: plugins/github-release @@ -90,21 +98,28 @@ platform: arch: amd64 steps: -- name: submodules - image: alpine/git - commands: - - git submodule update --init - - name: cache:restore image: node:12-alpine commands: + - apk add libxkbfile-dev libsecret-dev build-base git bash python - ./scripts/cacher.sh +- name: lint + image: node:12-alpine + commands: + - yarn lint + +- name: test + image: node:12-alpine + commands: + - apk add openssl + - yarn test + - name: build image: node:12-alpine commands: - apk add libxkbfile-dev libsecret-dev build-base git bash python - - timeout 50m ./scripts/ci.bash || echo 'Timed out or failed; continuing so we can preserve cache for the next run' + - timeout 50m ./scripts/ci.bash || echo 'Timed out or failed; continuing to preserve cache for the next run' - name: cache:package image: node:12-alpine @@ -123,10 +138,10 @@ steps: when: event: push -- name: test +- name: test:build image: node:12-alpine commands: - - yarn test + - yarn test:build - name: publish:github image: plugins/github-release @@ -159,22 +174,27 @@ platform: arch: arm64 steps: -- name: submodules - image: alpine - commands: - - apk add git - - git submodule update --init - - name: cache:restore image: node:12 commands: + - apt update && apt install -y build-essential git libsecret-1-dev libx11-dev libxkbfile-dev - ./scripts/cacher.sh +- name: lint + image: node:12 + commands: + - yarn lint + +- name: test + image: node:12 + commands: + - yarn test + - name: build image: node:12 commands: - apt update && apt install -y build-essential git libsecret-1-dev libx11-dev libxkbfile-dev - - timeout 50m ./scripts/ci.bash || echo 'Timed out or failed; continuing so we can preserve cache for the next run' + - timeout 50m ./scripts/ci.bash || echo 'Timed out or failed; continuing to preserve cache for the next run' - name: cache:package image: node:12 @@ -193,10 +213,10 @@ steps: when: event: push -- name: test +- name: test:build image: node:12 commands: - - yarn test + - yarn test:build - name: publish:github image: plugins/github-release @@ -243,22 +263,28 @@ platform: arch: arm64 steps: -- name: submodules - image: alpine - commands: - - apk add git - - git submodule update --init - - name: cache:restore image: node:12-alpine commands: + - apk add libxkbfile-dev libsecret-dev build-base git bash python - ./scripts/cacher.sh +- name: lint + image: node:12-alpine + commands: + - yarn lint + +- name: test + image: node:12-alpine + commands: + - apk add openssl + - yarn test + - name: build image: node:12-alpine commands: - apk add libxkbfile-dev libsecret-dev build-base git bash python - - timeout 50m ./scripts/ci.bash || echo 'Timed out or failed; continuing so we can preserve cache for the next run' + - timeout 50m ./scripts/ci.bash || echo 'Timed out or failed; continuing to preserve cache for the next run' - name: cache:package image: node:12-alpine @@ -277,10 +303,10 @@ steps: when: event: push -- name: test +- name: test:build image: node:12-alpine commands: - - yarn test + - yarn test:build - name: publish:github image: plugins/github-release @@ -313,22 +339,27 @@ platform: arch: arm steps: -- name: submodules - image: alpine - commands: - - apk add git - - git submodule update --init - - name: cache:restore image: node:12 commands: + - apt update && apt install -y build-essential git libsecret-1-dev libx11-dev libxkbfile-dev - ./scripts/cacher.sh +- name: lint + image: node:12 + commands: + - yarn lint + +- name: test + image: node:12 + commands: + - yarn test + - name: build image: node:12 commands: - apt update && apt install -y build-essential git libsecret-1-dev libx11-dev libxkbfile-dev - - timeout 50m ./scripts/ci.bash || echo 'Timed out or failed; continuing so we can preserve cache for the next run' + - timeout 50m ./scripts/ci.bash || echo 'Timed out or failed; continuing to preserve cache for the next run' - name: cache:package image: node:12 @@ -347,11 +378,11 @@ steps: when: event: push -- name: test +- name: test:build image: node:12 failure: ignore commands: - - yarn test + - yarn test:build # - name: publish:github # image: plugins/github-release @@ -388,22 +419,28 @@ platform: arch: arm steps: -- name: submodules - image: alpine - commands: - - apk add git - - git submodule update --init - - name: cache:restore image: node:12-alpine commands: + - apk add libxkbfile-dev libsecret-dev build-base git bash python - ./scripts/cacher.sh +- name: lint + image: node:12-alpine + commands: + - yarn lint + +- name: test + image: node:12-alpine + commands: + - apk add openssl + - yarn test + - name: build image: node:12-alpine commands: - apk add libxkbfile-dev libsecret-dev build-base git bash python - - timeout 50m ./scripts/ci.bash || echo 'Timed out or failed; continuing so we can preserve cache for the next run' + - timeout 50m ./scripts/ci.bash || echo 'Timed out or failed; continuing to preserve cache for the next run' - name: cache:package image: node:12-alpine @@ -422,11 +459,11 @@ steps: when: event: push -- name: test +- name: test:build image: node:12-alpine failure: ignore commands: - - yarn test + - yarn test:build # - name: publish:github # image: plugins/github-release diff --git a/package.json b/package.json index 43e79f7d..2b61885f 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,8 @@ "patch:generate": "cd ./lib/vscode && git diff HEAD > ../../scripts/vscode.patch", "patch:apply": "cd ./lib/vscode && git apply ../../scripts/vscode.patch", "test": "mocha -r ts-node/register ./test/*.test.ts", - "lint:js": "eslint {src,test,scripts} --ext .ts,.tsx", + "test:build": "./scripts/test.sh", + "lint:js": "eslint src test scripts --ext .ts,.tsx", "lint:css": "stylelint 'src/**/*.css'", "lint": "./scripts/lint.sh", "watch": "yarn runner watch", diff --git a/scripts/cacher.sh b/scripts/cacher.sh index df076f1a..23912f62 100755 --- a/scripts/cacher.sh +++ b/scripts/cacher.sh @@ -46,7 +46,18 @@ main() { # The action is determined by the name of the step. case $DRONE_STEP_NAME in - *restore*) restore "$branch" "$DRONE_REPO_BRANCH" ;; + *restore*) + # Sub-modules must be pulled first since extracting the cache directories + # will prevent git from cloning into them. + git submodule update --init + + restore "$branch" "$DRONE_REPO_BRANCH" + + # Now make sure the pulled Node modules are up to date. + YARN_CACHE_FOLDER="$(pwd)/yarn-cache" + export YARN_CACHE_FOLDER + yarn + ;; *rebuild*|*package*) package "$branch" ;; *) exit 1 ;; esac diff --git a/scripts/ci.bash b/scripts/ci.bash index 6ccada80..c288a10a 100755 --- a/scripts/ci.bash +++ b/scripts/ci.bash @@ -3,6 +3,7 @@ set -euo pipefail +# This script assumes that yarn has already ran. function main() { cd "$(dirname "${0}")/.." @@ -21,10 +22,6 @@ function main() { export PACKAGE="true" fi - if [[ -z ${SKIP_YARN:-} ]] ; then - yarn - fi - yarn build yarn binary if [[ -n ${PACKAGE:-} ]] ; then diff --git a/scripts/test.sh b/scripts/test.sh new file mode 100755 index 00000000..c6dd7e5d --- /dev/null +++ b/scripts/test.sh @@ -0,0 +1,19 @@ +#!/usr/bin/env sh +# test.sh -- Simple build test. + +set -eu + +main() { + cd "$(dirname "$0")/.." + + # The main goal here is to ensure that the build fully completed and the + # result looks usable. + version=$(node ./build/out/node/entry.js --version --json) + echo "Got '$version' for the version" + case $version in + "{ codeServer":*) exit 0 ;; + *) exit 1 ;; + esac +} + +main "$@" diff --git a/src/node/entry.ts b/src/node/entry.ts index a2e2f60f..4a77e98e 100644 --- a/src/node/entry.ts +++ b/src/node/entry.ts @@ -49,7 +49,7 @@ const main = async (args: Args = {}): Promise => { new VscodeHttpProvider([], { base: "/vscode-embed", auth, password }) ) - ipcMain.onDispose(() => httpServer.dispose()) + ipcMain().onDispose(() => httpServer.dispose()) const serverAddress = await httpServer.listen() logger.info(`Server listening on ${serverAddress}`) @@ -84,4 +84,18 @@ const main = async (args: Args = {}): Promise => { } } -wrap(main) +// TODO: Implement CLI parser. +if (process.argv.includes("--version")) { + const version = require("../../package.json").version + if (process.argv.includes("--json")) { + console.log({ + codeServer: version, + vscode: require("../../lib/vscode/package.json").version, + }) + } else { + console.log(version) + } + process.exit(0) +} else { + wrap(main) +} diff --git a/src/node/wrapper.ts b/src/node/wrapper.ts index 69978b96..293e9d39 100644 --- a/src/node/wrapper.ts +++ b/src/node/wrapper.ts @@ -21,14 +21,6 @@ export class ProcessError extends Error { } } -/** - * Ensure we control when the process exits. - */ -const exit = process.exit -process.exit = function(code?: number) { - logger.warn(`process.exit() was prevented: ${code || "unknown code"}.`) -} as (code?: number) => never - /** * Allows the wrapper and inner processes to communicate. */ @@ -37,19 +29,26 @@ export class IpcMain { public readonly onMessage = this._onMessage.event private readonly _onDispose = new Emitter() public readonly onDispose = this._onDispose.event + public readonly exit: (code?: number) => never public constructor(public readonly parentPid?: number) { process.on("SIGINT", () => this._onDispose.emit("SIGINT")) process.on("SIGTERM", () => this._onDispose.emit("SIGTERM")) process.on("exit", () => this._onDispose.emit(undefined)) + // Ensure we control when the process exits. + this.exit = process.exit + process.exit = function(code?: number) { + logger.warn(`process.exit() was prevented: ${code || "unknown code"}.`) + } as (code?: number) => never + this.onDispose((signal) => { // Remove listeners to avoid possibly triggering disposal again. process.removeAllListeners() // Let any other handlers run first then exit. logger.debug(`${parentPid ? "inner process" : "wrapper"} ${process.pid} disposing`, field("code", signal)) - setTimeout(() => exit(0), 0) + setTimeout(() => this.exit(0), 0) }) // Kill the inner process if the parent dies. This is for the case where the @@ -117,9 +116,17 @@ export class IpcMain { } } -export const ipcMain = new IpcMain( - typeof process.env.CODE_SERVER_PARENT_PID !== "undefined" ? parseInt(process.env.CODE_SERVER_PARENT_PID) : undefined -) +let _ipcMain: IpcMain +export const ipcMain = (): IpcMain => { + if (!_ipcMain) { + _ipcMain = new IpcMain( + typeof process.env.CODE_SERVER_PARENT_PID !== "undefined" + ? parseInt(process.env.CODE_SERVER_PARENT_PID) + : undefined + ) + } + return _ipcMain +} export interface WrapperOptions { maxMemory?: number @@ -135,14 +142,14 @@ export class WrapperProcess { private started?: Promise public constructor(private currentVersion: string, private readonly options?: WrapperOptions) { - ipcMain.onDispose(() => { + ipcMain().onDispose(() => { if (this.process) { this.process.removeAllListeners() this.process.kill() } }) - ipcMain.onMessage(async (message) => { + ipcMain().onMessage(async (message) => { switch (message.type) { case "relaunch": logger.info(`Relaunching: ${this.currentVersion} -> ${message.version}`) @@ -156,7 +163,7 @@ export class WrapperProcess { await this.start() } catch (error) { logger.error(error.message) - exit(typeof error.code === "number" ? error.code : 1) + ipcMain().exit(typeof error.code === "number" ? error.code : 1) } break default: @@ -170,12 +177,14 @@ export class WrapperProcess { if (!this.started) { const child = this.spawn() logger.debug(`spawned inner process ${child.pid}`) - this.started = ipcMain.handshake(child).then(() => { - child.once("exit", (code) => { - logger.debug(`inner process ${child.pid} exited unexpectedly`) - exit(code || 0) + this.started = ipcMain() + .handshake(child) + .then(() => { + child.once("exit", (code) => { + logger.debug(`inner process ${child.pid} exited unexpectedly`) + ipcMain().exit(code || 0) + }) }) - }) this.process = child } return this.started @@ -201,23 +210,23 @@ export class WrapperProcess { // // It's possible that the pipe has closed (for example if you run code-server // // --version | head -1). Assume that means we're done. if (!process.stdout.isTTY) { - process.stdout.on("error", () => exit()) + process.stdout.on("error", () => ipcMain().exit()) } export const wrap = (fn: () => Promise): void => { - if (ipcMain.parentPid) { - ipcMain + if (ipcMain().parentPid) { + ipcMain() .handshake() .then(() => fn()) .catch((error: ProcessError): void => { logger.error(error.message) - exit(typeof error.code === "number" ? error.code : 1) + ipcMain().exit(typeof error.code === "number" ? error.code : 1) }) } else { const wrapper = new WrapperProcess(require("../../package.json").version) wrapper.start().catch((error) => { logger.error(error.message) - exit(typeof error.code === "number" ? error.code : 1) + ipcMain().exit(typeof error.code === "number" ? error.code : 1) }) } }