feat(ci): refactor build workflow (#5572)

* refactor: remove stylelint

* refactor: move shellcheck to separate job

* refactor: add helm script and job

* refactor: add eslint job and yarn script

* fix(test/tsconfig): exclude test-plugin

* refactor: delete lint, add typecheck job

* refactor: remove prebuild

* wip: add notes about unit test refactor

* refactor: delete buggy socket test

This test was really added to in get cover specific lines but it's buggy
and only passes sometimes locally. I think it's okay to remove because:
- it's an implementation detail (not user facing)
- not preventing any specific regressions

* refactor: move test-plugin to integration suite

This seems more appropriate given this tests how a plugin might work
within code-server.

* wip

* wip: refactor vscode integration tests

* refactor: move unit tests to separate job

* fix: formatting

* Revert "wip: refactor vscode integration tests"

This reverts commit 13286bf.

* Revert "refactor: move unit tests to separate job"

This reverts commit 6c87b54.

* feat: collect codecov integration tests

* fixup! feat: collect codecov integration tests

* fixup! feat: collect codecov integration tests

* fixup!: move helm step

* fixup!: update ids for caching

* trigger ci

* trigger ci

* chore: clean up names in security.yaml

* fixup!: remove .tsx

* fixup!: change to src/**"

* fixup!: move helm cmd to yaml

* fixup!: always build test plugin

* fixup!: fix plugin typings

* fixup! add back flakey test

* fixup!: only install helm deps if changes

* fixup!: revert node mod caching

* dont keep, test for asher

* fixup!: add make to centos

* refactor: add test:native

This adds a new script to run native tests (i.e. --help which should run
in ci on all platforms).

* try updating glibc

* try 2.25

* Revert "refactor: move test-plugin to integration suite"

This reverts commit bc02005.

I couldn't get past some GLIBC errors in CI so moving back to unit
tests.

* Revert "try updating glibc"

This reverts commit 02ed560.

* fixup!

* asher: again

* try this for ts changes

* fixup

* refactor: scripts.yml -> scripts.yaml

* fixup!: move lint-sh to scripts.yaml

* fixup!: use apk for lint scripts

* fixup! fixup!: use apk for lint scripts

* fixup!: remove typecheck step

* fix: pattern for lint ts files

* test: lint should fail

* fixup! fixup!: use apk for lint scripts

* Revert "test: lint should fail"

This reverts commit 158c64d.

* fixup!: skip cancel workflow on forks

Looks like the cancel action workflow can't run on forks due to secrets.

See andymckay/cancel-action#4

* fixup: remove cancel-workflow

* fixup! fixup! fixup!: use apk for lint scripts

* fixup! fixup! fixup!: use apk for lint scripts

* fixup!: fix yarn key

* fixup!: add fetch-depth 0

.github/workflows/build.yaml

@@ -22,10 +22,10 @@ concurrency:
 # will skip running `yarn install` if it successfully fetched from cache
 
 jobs:
-  prebuild:
-    name: Pre-build checks
+  fmt:
+    name: Format with Prettier
     runs-on: ubuntu-latest
-    timeout-minutes: 20
+    timeout-minutes: 5
     steps:
       - name: Checkout repo
         uses: actions/checkout@v3
@@ -35,15 +35,81 @@ jobs:
         with:
           node-version: "16"
 
+      - name: Fetch dependencies from cache
+        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.cache-node-modules.outputs.cache-hit != 'true'
+        run: SKIP_SUBMODULE_DEPS=1 yarn --frozen-lockfile
+
+      - name: Format files with Prettier
+        run: yarn fmt
+
+  lint-helm:
+    name: Lint Helm chart
+    runs-on: ubuntu-latest
+    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/[email protected]
+        with:
+          files: |
+            ci/helm-chart/**
+
       - name: Install helm
+        if: steps.changed-files.outputs.any_changed == 'true'
         uses: azure/[email protected]
         with:
           token: ${{ secrets.GITHUB_TOKEN }}
 
       - name: Install helm kubeval plugin
+        if: steps.changed-files.outputs.any_changed == 'true'
         run: helm plugin install https://github.com/instrumenta/helm-kubeval
 
+      - name: Lint Helm chart
+        if: steps.changed-files.outputs.any_changed == 'true'
+        run: helm kubeval ci/helm-chart
+
+  lint-ts:
+    name: Lint TypeScript files
+    runs-on: ubuntu-latest
+    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/[email protected]
+        with:
+          files: |
+            **/*.ts
+            **/*.js
+          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:
@@ -53,20 +119,15 @@ jobs:
             yarn-build-
 
       - name: Install dependencies
-        if: steps.cache-node-modules.outputs.cache-hit != 'true'
+        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 yarn fmt
-        run: yarn fmt
-        if: success()
-
-      - name: Run yarn lint
-        run: yarn lint
-        if: success()
+      - name: Lint TypeScript files
+        if: steps.changed-files.outputs.any_changed == 'true'
+        run: yarn lint:ts
 
   build:
     name: Build
-    needs: prebuild
     runs-on: ubuntu-latest
     timeout-minutes: 30
     env:
@@ -222,6 +283,8 @@ jobs:
     runs-on: ubuntu-latest
     timeout-minutes: 15
     container: "centos:7"
+    env:
+      CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
 
     steps:
       - name: Checkout repo
@@ -234,7 +297,7 @@ jobs:
 
       - name: Install development tools
         run: |
-          yum install -y epel-release centos-release-scl
+          yum install -y epel-release centos-release-scl make
           yum install -y devtoolset-9-{make,gcc,gcc-c++} jq rsync python3
 
       - name: Install nfpm and envsubst
@@ -278,6 +341,12 @@ jobs:
       - name: Run integration tests on standalone release
         run: yarn test:integration
 
+      - name: Upload coverage report to Codecov
+        uses: codecov/codecov-action@v3
+        with:
+          token: ${{ secrets.CODECOV_TOKEN }}
+        if: success()
+
       - name: Build packages with nfpm
         run: yarn package
 
@@ -417,8 +486,8 @@ jobs:
         if: steps.cache-node-modules.outputs.cache-hit != 'true'
         run: SKIP_SUBMODULE_DEPS=1 yarn install
 
-      - name: Run integration tests on standalone release
-        run: yarn test:integration
+      - name: Run native module tests on standalone release
+        run: yarn test:native
 
       - name: Build packages with nfpm
         run: yarn package

.github/workflows/scripts.yml → .github/workflows/scripts.yaml

@@ -51,3 +51,17 @@ jobs:
 
       - name: Run script unit tests
         run: ./ci/dev/test-scripts.sh
+
+  lint:
+    name: Lint shell files
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    steps:
+      - name: Checkout repo
+        uses: actions/checkout@v3
+
+      - name: Install lint utilities
+        run: sudo apt install shellcheck
+
+      - name: Lint shell files
+        run: ./ci/dev/lint-scripts.sh

.github/workflows/security.yaml

@@ -1,4 +1,4 @@
-name: "Security Scanning"
+name: Security
 
 on:
   push:
@@ -19,7 +19,7 @@ concurrency:
 
 jobs:
   audit-ci:
-    name: Run audit-ci
+    name: Audit node modules
     runs-on: ubuntu-latest
     timeout-minutes: 15
     steps:
@@ -51,6 +51,7 @@ jobs:
         if: success()
 
   trivy-scan-repo:
+    name: Scan repo with Trivy
     permissions:
       contents: read # for actions/checkout to fetch code
       security-events: write # for github/codeql-action/upload-sarif to upload SARIF results
@@ -82,7 +83,7 @@ jobs:
       actions: read # for github/codeql-action/init to get workflow details
       contents: read # for actions/checkout to fetch code
       security-events: write # for github/codeql-action/autobuild to send a status report
-    name: Analyze
+    name: Analyze with CodeQL
     runs-on: ubuntu-20.04
 
     steps:

ci/dev/lint-scripts.sh

@@ -0,0 +1,9 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+main() {
+  cd "$(dirname "$0")/../.."
+  shellcheck -e SC2046,SC2164,SC2154,SC1091,SC1090,SC2002 $(git ls-files '*.sh' | grep -v 'lib/vscode')
+}
+
+main "$@"

ci/dev/test-native.sh

@@ -0,0 +1,39 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+help() {
+  echo >&2 "  You can build the standalone release with 'yarn release:standalone'"
+  echo >&2 "  Or you can pass in a custom path."
+  echo >&2 "  CODE_SERVER_PATH='/var/tmp/coder/code-server/bin/code-server' yarn test:integration"
+}
+
+# Make sure a code-server release works. You can pass in the path otherwise it
+# will look for release-standalone in the current directory.
+#
+# This is to make sure we don't have Node version errors or any other
+# compilation-related errors.
+main() {
+  cd "$(dirname "$0")/../.."
+
+  source ./ci/lib.sh
+
+  local path="$RELEASE_PATH-standalone/bin/code-server"
+  if [[ ! ${CODE_SERVER_PATH-} ]]; then
+    echo "Set CODE_SERVER_PATH to test another build of code-server"
+  else
+    path="$CODE_SERVER_PATH"
+  fi
+
+  echo "Running tests with code-server binary: '$path'"
+
+  if [[ ! -f $path ]]; then
+    echo >&2 "No code-server build detected"
+    echo >&2 "Looked in $path"
+    help
+    exit 1
+  fi
+
+  CODE_SERVER_PATH="$path" ./test/node_modules/.bin/jest "$@" --coverage=false --testRegex "./test/integration/help.test.ts"
+}
+
+main "$@"

package.json

@@ -19,14 +19,16 @@
     "test:e2e:proxy": "USE_PROXY=1 ./ci/dev/test-e2e.sh",
     "test:unit": "./ci/dev/test-unit.sh --forceExit --detectOpenHandles",
     "test:integration": "./ci/dev/test-integration.sh",
+    "test:native": "./ci/dev/test-native.sh",
     "test:scripts": "./ci/dev/test-scripts.sh",
     "package": "./ci/build/build-packages.sh",
     "postinstall": "./ci/dev/postinstall.sh",
     "publish:npm": "./ci/steps/publish-npm.sh",
     "publish:docker": "./ci/steps/docker-buildx-push.sh",
     "_audit": "./ci/dev/audit.sh",
     "fmt": "./ci/dev/fmt.sh",
-    "lint": "./ci/dev/lint.sh",
+    "lint:scripts": "./ci/dev/lint-scripts.sh",
+    "lint:ts": "eslint --max-warnings=0 --fix $(git ls-files '*.ts' '*.js' | grep -v 'lib/vscode')",
     "test": "echo 'Run yarn test:unit or yarn test:e2e' && exit 1",
     "ci": "./ci/dev/ci.sh",
     "watch": "VSCODE_DEV=1 VSCODE_IPC_HOOK_CLI= NODE_OPTIONS='--max_old_space_size=32384 --trace-warnings' ts-node ./ci/dev/watch.ts",
@@ -53,15 +55,12 @@
     "audit-ci": "^6.0.0",
     "doctoc": "^2.0.0",
     "eslint": "^7.7.0",
-    "eslint-config-prettier": "^8.1.0",
+    "eslint-config-prettier": "^8.5.0",
     "eslint-import-resolver-typescript": "^2.5.0",
     "eslint-plugin-import": "^2.18.2",
     "eslint-plugin-prettier": "^4.0.0",
     "prettier": "^2.2.1",
     "prettier-plugin-sh": "^0.12.0",
-    "shellcheck": "^1.0.0",
-    "stylelint": "^13.0.0",
-    "stylelint-config-recommended": "^5.0.0",
     "ts-node": "^10.0.0",
     "typescript": "^4.6.2"
   },

test/tsconfig.json

@@ -1,4 +1,5 @@
 {
   "extends": "../tsconfig.json",
-  "include": ["./**/*.ts"]
+  "include": ["./**/*.ts"],
+  "exclude": ["./unit/node/test-plugin"]
 }

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

@@ -4,6 +4,8 @@ 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("vscode", () => {
   let codeServer: httpserver.HttpServer | undefined