Skip to content

Commit

Permalink
Include lint and test jobs for node-ci-push workflow (using reusable …
Browse files Browse the repository at this point in the history
…workflow) (#2348)

## Summary:
- Lint and test jobs from [node-ci-pr.yml](https://github.com/Khan/wonder-blocks/blob/a32b0779acc9ed8dc742955160cc7748052296e8/.github/workflows/node-ci-pr.yml#L65-L160) are also run in the `node-ci-push.yml` workflow
  - this is done so that feature branches can be updated with the latest changes in main (there are currently branch protection rules on feature/* branches that require these status checks to pass)
- Lint and test jobs are refactored into [reusable workflows](https://docs.github.com/en/actions/sharing-automations/reusing-workflows) so that both `node-ci-push` and `node-ci-pr` can use the same config

See #2346 for an alternative where we use composite actions instead to share configuration

Update: We decided to use reusable workflows so that the logs are easier to read for the steps and we can use the `secrets` config (which isn't supported by composite actions). The branch rules have been updated to look for `Lint / Lint` and `Test / Test` checks.

Pros:
- shared configuration for `node-ci-push` and `node-ci-pr`
- easy to see the [logs](https://github.com/Khan/wonder-blocks/actions/runs/11393035774/job/31700470290?pr=2348) for the different steps:
![Screenshot 2024-10-17 at 3 21 04 PM](https://github.com/user-attachments/assets/effeca62-4326-4626-bee0-02f43c984d19)


Cons:
- since we are using another workflow, the names of the jobs are nested (ie. `Node CI (PR) / Lint / Lint` instead of just `Node CI (PR) / Lint` ) and the github checks will need to be updated (let me know though if there's a way around this!)
![Screenshot 2024-10-17 at 3 16 53 PM](https://github.com/user-attachments/assets/baee2f1b-78a8-479c-a416-d07cc76972c9)

Issue: WB-1778

## Test plan:
- Confirm linting and tests run in CI
- Confirm steps properly fail when there are linting errors, typescript errors, and failing tests
![image](https://github.com/user-attachments/assets/e6749cc3-5b85-4e6d-933b-8217e8acf7cf)

Author: beaesguerra

Reviewers: beaesguerra, jandrade

Required Reviewers:

Approved By: jandrade

Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ⏭️  Chromatic - Skip on Release PR (changesets), 🚫 Chromatic - Get results on regular PRs, ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ gerald, ✅ Publish npm snapshot (ubuntu-latest, 20.x), 🚫 Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏭️  Chromatic - Skip on Release PR (changesets), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ gerald, ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ⌛ undefined, ⌛ undefined

Pull Request URL: #2348
  • Loading branch information
beaesguerra authored Oct 18, 2024
1 parent 0ac8cbd commit bb7f7cf
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 123 deletions.
73 changes: 73 additions & 0 deletions .github/workflows/node-ci-lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# This reusable workflow is executed to run linting checks for the project
name: Run lint for the project

on:
workflow_call:

jobs:
lint:
name: Lint
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest]
node-version: [20.x]
steps:
- uses: actions/checkout@v4
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}

- name: Install & cache node_modules
uses: Khan/actions@shared-node-cache-v2
with:
node-version: ${{ matrix.node-version }}

- name: Get All Changed Files
uses: Khan/actions@get-changed-files-v2
id: changed

- id: js-ts-files
name: Find .js, .ts changed files
uses: Khan/actions@filter-files-v1
with:
changed-files: ${{ steps.changed.outputs.files }}
extensions: '.js,.jsx,.ts,.tsx'

- id: eslint-reset
uses: Khan/actions@filter-files-v1
name: Files that would trigger a full eslint run
with:
changed-files: ${{ steps.changed.outputs.files }}
files: '.eslintrc.js,yarn.lock,.eslintignore'

- id: typecheck-reset
uses: Khan/actions@filter-files-v1
name: Files that would trigger a typecheck run
with:
changed-files: ${{ steps.changed.outputs.files }}
files: '.yarn.lock'

# Linting / type checking
- name: Eslint
uses: Khan/actions@full-or-limited-v0
with:
full-trigger: ${{ steps.eslint-reset.outputs.filtered }}
full: yarn lint:ci .
limited-trigger: ${{ steps.js-ts-files.outputs.filtered }}
limited: yarn lint:ci {}

- name: Typecheck
if: always() # always run this check until we update the eslint config
# if: steps.js-ts-files.outputs.filtered != '[]' || steps.typecheck-reset.outputs.filtered != '[]'
run: yarn typecheck

- name: Build .js bundles
run: yarn build

- name: Build .d.ts types
run: yarn build:types

- name: Check package.json files
run: node utils/publish/pre-publish-check-ci.js
93 changes: 4 additions & 89 deletions .github/workflows/node-ci-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,99 +65,14 @@ jobs:
lint:
name: Lint
needs: prime_cache_primary
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest]
node-version: [20.x]
steps:
- uses: actions/checkout@v4
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}

- name: Install & cache node_modules
uses: Khan/actions@shared-node-cache-v2
with:
node-version: ${{ matrix.node-version }}

- name: Get All Changed Files
uses: Khan/actions@get-changed-files-v2
id: changed

- id: js-ts-files
name: Find .js, .ts changed files
uses: Khan/actions@filter-files-v1
with:
changed-files: ${{ steps.changed.outputs.files }}
extensions: '.js,.jsx,.ts,.tsx'

- id: eslint-reset
uses: Khan/actions@filter-files-v1
name: Files that would trigger a full eslint run
with:
changed-files: ${{ steps.changed.outputs.files }}
files: '.eslintrc.js,yarn.lock,.eslintignore'

- id: typecheck-reset
uses: Khan/actions@filter-files-v1
name: Files that would trigger a typecheck run
with:
changed-files: ${{ steps.changed.outputs.files }}
files: '.yarn.lock'

# Linting / type checking
- name: Eslint
uses: Khan/actions@full-or-limited-v0
with:
full-trigger: ${{ steps.eslint-reset.outputs.filtered }}
full: yarn lint:ci .
limited-trigger: ${{ steps.js-ts-files.outputs.filtered }}
limited: yarn lint:ci {}

- name: Typecheck
if: always() # always run this check until we update the eslint config
# if: steps.js-ts-files.outputs.filtered != '[]' || steps.typecheck-reset.outputs.filtered != '[]'
run: yarn typecheck

- name: Build .js bundles
run: yarn build

- name: Build .d.ts types
run: yarn build:types

- name: Check package.json files
run: node utils/publish/pre-publish-check-ci.js
uses: ./.github/workflows/node-ci-lint.yml

test:
name: Test
needs: prime_cache_primary
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest]
node-version: [20.x]
shard: ["1/2", "2/2"]
steps:
- uses: actions/checkout@v4
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}

- name: Install & cache node_modules
uses: Khan/actions@shared-node-cache-v2
with:
node-version: ${{ matrix.node-version }}
# Testing and coverage
- name: Run jest tests with coverage
run: yarn coverage:ci --shard ${{ matrix.shard }}
- name: Upload Coverage
uses: codecov/codecov-action@v4
with:
token: ${{ secrets.CODECOV_TOKEN }}
files: ./coverage/coverage-final.json
uses: ./.github/workflows/node-ci-test.yml
secrets:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}

check_builds:
name: Check build sizes
Expand Down
47 changes: 13 additions & 34 deletions .github/workflows/node-ci-push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ on:
# 1. Prime caches for primary configuration (ubuntu on node 14).
# This way the next two jobs can run in parallel but rely on this primed
# cache.
# 2. Coverage
# 3. Chromatic autoApprove on squash commits
# 2. Lint
# 3. Test (with coverage)
# 4. Chromatic autoApprove on squash commits
#
# For pushes directly to a branch, we assume a PR has been used with wider
# checks, this just makes sure our coverage data is up-to-date.
Expand All @@ -35,36 +36,14 @@ jobs:
with:
node-version: ${{ matrix.node-version }}

lint:
name: Lint
needs: prime_cache_primary
uses: ./.github/workflows/node-ci-lint.yml

coverage:
needs: [prime_cache_primary]
name: Gather coverage
env:
CI: true
runs-on: ${{ matrix.os }}
strategy:
matrix:
# Use a matrix as it means we get the version info in the job name
# which is very helpful.
os: [ubuntu-latest]
node-version: [20.x]
steps:
- uses: actions/checkout@v4
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}

# Cache and install dependencies
- name: Install & cache node_modules
uses: Khan/actions@shared-node-cache-v2
with:
node-version: ${{ matrix.node-version }}

- name: Run Jest with coverage
run: yarn coverage:ci
- name: Upload Coverage
uses: codecov/codecov-action@v4
with:
token: ${{ secrets.CODECOV_TOKEN }}
files: ./coverage/coverage-final.json
test:
name: Test
needs: prime_cache_primary
uses: ./.github/workflows/node-ci-test.yml
secrets:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
37 changes: 37 additions & 0 deletions .github/workflows/node-ci-test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# This reusable workflow is executed to run test checks for the project
name: Run tests for the project

on:
workflow_call:
secrets:
CODECOV_TOKEN:
required: true

jobs:
test:
name: Test
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest]
node-version: [20.x]
shard: ["1/2", "2/2"]
steps:
- uses: actions/checkout@v4
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}

- name: Install & cache node_modules
uses: Khan/actions@shared-node-cache-v2
with:
node-version: ${{ matrix.node-version }}
# Testing and coverage
- name: Run jest tests with coverage
run: yarn coverage:ci --shard ${{ matrix.shard }}
- name: Upload Coverage
uses: codecov/codecov-action@v4
with:
token: ${{ secrets.CODECOV_TOKEN }}
files: ./coverage/coverage-final.json

0 comments on commit bb7f7cf

Please sign in to comment.