Skip to content

Commit

Permalink
ci: ensure tests use build output and simplify CI/CD workflow
Browse files Browse the repository at this point in the history
Thanks, @takaokouji, for making me take a closer look at the caching
behavior in this workflow. Subtle and dangerous!

In the previous version, the `build/` directory was cached with this
key:
```yaml
key: ${{ runner.os }}-build-${{ hashFiles('package-lock.json') }}
```

As @takaokouji observed, this means that if a commit or PR doesn't
change `package-lock.json`, the cache key will not change. Two unrelated
builds, possibly with very different content in `src/` or elsewhere,
could share the same build cache. In this case, when the later testing
steps restore the cache, they'll both be testing the same build output.
One of them won't be testing what you expect!

Luckily, the deploy steps also used the same cache keys, so they also
didn't deploy what you expect. I say "luckily" because the alternative
is to release something that didn't run any tests.

All the caching and retrieval steps also took a lot of time, enabled
only a tiny amount of extra parallelism (in the deploy steps), and added
a lot of complexity to the workflow. Removing all of that means a
faster, easier-to-understand workflow with no sneaky gremlins making us
test or deploy the wrong thing.
  • Loading branch information
cwillisf committed Aug 26, 2024
1 parent 8aec6f4 commit 650464c
Showing 1 changed file with 8 additions and 133 deletions.
141 changes: 8 additions & 133 deletions .github/workflows/ci-cd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ permissions:
pull-requests: write # comment on released pull requests

jobs:
setup:
ci-cd:
runs-on: ubuntu-latest
env:
DETECT_CHROMEDRIVER_VERSION: "true"
Expand All @@ -38,93 +38,21 @@ jobs:
run: npm ci
- name: Lint
run: npm run test:lint
- name: Cache node_modules
id: cache-nodemodules
uses: actions/cache@e12d46a63a90f2fae62d114769bbf2a179198b5c # v3
with:
path:
node_modules
key: ${{ runner.os }}-node-${{ hashFiles('package-lock.json') }}
- name: Cache src/generated
id: cache-generated
uses: actions/cache@e12d46a63a90f2fae62d114769bbf2a179198b5c # v3
with:
path:
src/generated
key: ${{ runner.os }}-generated-${{ hashFiles('package-lock.json') }}
- name: Cache static/microbit
id: cache-static
uses: actions/cache@e12d46a63a90f2fae62d114769bbf2a179198b5c # v3
with:
path:
static/microbit
key: ${{ runner.os }}-microbit-${{ hashFiles('package-lock.json') }}
test-unit:
needs: setup
runs-on: ubuntu-latest
env:
JEST_JUNIT_OUTPUT_NAME: unit-results.xml
JEST_JUNIT_OUTPUT_DIR: test-results/unit
steps:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4
- name: Cache NPM dependencies
uses: actions/cache@e12d46a63a90f2fae62d114769bbf2a179198b5c # v3
with:
path:
node_modules
key: ${{ runner.os }}-node-${{ hashFiles('package-lock.json') }}
- name: Run Unit Tests
env:
JEST_JUNIT_OUTPUT_NAME: unit-results.xml
JEST_JUNIT_OUTPUT_DIR: test-results/unit
run: npm run test:unit -- --reporters="default" --reporters="jest-junit" --coverage --coverageReporters=text --coverageReporters=lcov --maxWorkers="2"
- name: Store Unit Test Results
uses: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32 # v3
with:
name: unit-test-output
path: ./test-results/*
build:
needs: [setup, test-unit]
env:
NODE_OPTIONS: --max-old-space-size=4000
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4
- uses: actions/setup-node@26961cf329f22f6837d5f54c3efd76b480300ace # v4
with:
cache: "npm"
node-version-file: ".nvmrc"
- name: Retrieve node_modules
uses: actions/cache@e12d46a63a90f2fae62d114769bbf2a179198b5c # v3
with:
path:
node_modules
key: ${{ runner.os }}-node-${{ hashFiles('package-lock.json') }}
- name: Retrieve src/generated
uses: actions/cache@e12d46a63a90f2fae62d114769bbf2a179198b5c # v3
with:
path:
src/generated
key: ${{ runner.os }}-generated-${{ hashFiles('package-lock.json') }}
- name: Retireve static/microbit
uses: actions/cache@e12d46a63a90f2fae62d114769bbf2a179198b5c # v3
with:
path:
static/microbit
key: ${{ runner.os }}-microbit-${{ hashFiles('package-lock.json') }}
- name: Run Build
env:
NODE_OPTIONS: --max-old-space-size=4000
NODE_ENV: production
run: npm run build
- name: Cache Build Directory
uses: actions/cache@e12d46a63a90f2fae62d114769bbf2a179198b5c # v3
with:
path:
./build
key: ${{ runner.os }}-build-${{ hashFiles('package-lock.json') }}
- name: Cache Dist Directory
uses: actions/cache@e12d46a63a90f2fae62d114769bbf2a179198b5c # v3
with:
path:
./dist
key: ${{ runner.os }}-dist-${{ hashFiles('package-lock.json') }}
- name: Store Build Output
uses: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32 # v3
with:
Expand All @@ -135,55 +63,20 @@ jobs:
with:
name: dist-output
path: ./dist
test-integration:
needs: build
runs-on: ubuntu-latest
env:
JEST_JUNIT_OUTPUT_NAME: results.txt
JEST_JUNIT_OUTPUT_DIR: test-results/integration
steps:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4
- name: Retrieve npm dependencies
uses: actions/cache@e12d46a63a90f2fae62d114769bbf2a179198b5c # v3
with:
path:
node_modules
key: ${{ runner.os }}-node-${{ hashFiles('package-lock.json') }}
- name: Retrieve Build
uses: actions/cache@e12d46a63a90f2fae62d114769bbf2a179198b5c # v3
with:
path:
./build
key: ${{ runner.os }}-build-${{ hashFiles('package-lock.json') }}
- run: |
for F in chrome chromium chromedriver; do
which $F && $F --version || echo Not found: $F
done
- name: Run Integration Tests
env:
JEST_JUNIT_OUTPUT_NAME: results.txt
JEST_JUNIT_OUTPUT_DIR: test-results/integration
run: npm run test:integration -- --reporters="default" --reporters="jest-junit"
- name: Store Integration Test Results
uses: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32 # v3
with:
name: integration-test-output
path: ./test-results/*
deploy-npm:
needs: [test-integration, test-unit]
if: (github.ref == 'refs/heads/master') || (github.ref == 'refs/heads/develop') || (github.ref == 'refs/heads/beta') || startsWith(github.ref, 'refs/heads/hotfix')
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4
- name: Retrieve npm dependencies
uses: actions/cache@e12d46a63a90f2fae62d114769bbf2a179198b5c # v3
with:
path:
node_modules
key: ${{ runner.os }}-node-${{ hashFiles('package-lock.json') }}
- name: Retrieve Dist Directory
uses: actions/cache@e12d46a63a90f2fae62d114769bbf2a179198b5c # v3
with:
path:
./dist
key: ${{ runner.os }}-dist-${{ hashFiles('package-lock.json') }}
- run: |
if [[ ${{contains(github.ref, 'hotfix')}} ]]; then
sed -e "s|hotfix/REPLACE|${{ github.ref_name }}|" --in-place release.config.js
Expand All @@ -193,24 +86,6 @@ jobs:
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: npx --no -- semantic-release
deploy-gh-pages:
needs: [test-integration, test-unit]
if: (!(startsWith(github.ref, 'refs/heads/dependabot/') || startsWith(github.ref, 'refs/heads/renovate/')))
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4
- name: Retrieve npm dependencies
uses: actions/cache@e12d46a63a90f2fae62d114769bbf2a179198b5c # v3
with:
path:
node_modules
key: ${{ runner.os }}-node-${{ hashFiles('package-lock.json') }}
- name: Retrieve Build Directory
uses: actions/cache@e12d46a63a90f2fae62d114769bbf2a179198b5c # v3
with:
path:
./build
key: ${{ runner.os }}-build-${{ hashFiles('package-lock.json') }}
- name: Deploy playground to GitHub Pages
uses: peaceiris/actions-gh-pages@373f7f263a76c20808c831209c920827a82a2847 # v3
with:
Expand Down

0 comments on commit 650464c

Please sign in to comment.