diff --git a/.github/scripts/detect-code-changes.sh b/.github/scripts/detect-code-changes.sh index b27fcae6b..ac3e56974 100644 --- a/.github/scripts/detect-code-changes.sh +++ b/.github/scripts/detect-code-changes.sh @@ -5,13 +5,20 @@ event_name=${1:?usage: detect-code-changes.sh [pull-re repository=${2:?usage: detect-code-changes.sh [pull-request-number]} pull_request_number=${3:-} -set_code_output() { - printf 'code=%s\n' "$1" >>"$GITHUB_OUTPUT" - printf 'Non-docs changes: %s\n' "$1" +# code: anything outside docs, so the Unit legs and static checks run. +# e2e: anything the Playwright suite can observe. The suite boots the web +# dev server (packages/web, which imports only packages/core) against +# stubbed routes; packages/backend, packages/sync, and packages/scripts +# are never loaded, so a PR that touches only those skips the e2e +# shards. merge_group and push runs always run everything, so nothing +# reaches main untested. +set_outputs() { + printf 'code=%s\ne2e=%s\n' "$1" "$2" >>"$GITHUB_OUTPUT" + printf 'Non-docs changes: %s\nE2E-reachable changes: %s\n' "$1" "$2" } if [ "$event_name" != "pull_request" ]; then - set_code_output true + set_outputs true true exit 0 fi @@ -21,7 +28,7 @@ if ! files=$(gh api --paginate \ "repos/${repository}/pulls/${pull_request_number}/files" \ --jq '.[].filename'); then echo "Could not read pull request files; running checks." >&2 - set_code_output true + set_outputs true true exit 0 fi @@ -29,10 +36,17 @@ fi # printf's SIGPIPE as a failure on large pull requests. code_files=$(printf '%s\n' "$files" | grep -vE '(\.md$|^docs/|^\.gitignore$)' || true) +e2e_files=$(printf '%s\n' "$code_files" | + grep -vE '^packages/(backend|sync|scripts)/' || true) # An empty response is unverified, so run the checks. -if [ -z "$files" ] || [ -n "$code_files" ]; then - set_code_output true -else - set_code_output false +if [ -z "$files" ]; then + set_outputs true true + exit 0 fi + +code=false +e2e=false +[ -n "$code_files" ] && code=true +[ -n "$e2e_files" ] && e2e=true +set_outputs "$code" "$e2e" diff --git a/.github/workflows/test-e2e.yml b/.github/workflows/test-e2e.yml index 9475c80e1..c7f797dbc 100644 --- a/.github/workflows/test-e2e.yml +++ b/.github/workflows/test-e2e.yml @@ -35,11 +35,16 @@ jobs: # does report, and reports Success. So for pull_request the filter lives # here, as a job gate, not in `on:`. The push trigger keeps its # paths-ignore: push runs block nothing. + # The `e2e` output is false when a PR touches only packages/backend, + # packages/sync, or packages/scripts (plus docs): the suite boots the web + # dev server against stubbed routes and never loads those packages. 22% of + # PR e2e runs in the 2026-09 audit were such PRs. merge_group and push runs + # always report true, so main is still tested end to end. changes: runs-on: ubuntu-latest timeout-minutes: 2 outputs: - code: ${{ steps.filter.outputs.code }} + e2e: ${{ steps.filter.outputs.e2e }} steps: - name: Check out repository uses: actions/checkout@v7 @@ -64,7 +69,7 @@ jobs: # would show the unexpanded ${{ matrix.shard }} expression. e2e-shard: needs: changes - if: needs.changes.outputs.code == 'true' + if: needs.changes.outputs.e2e == 'true' runs-on: ubuntu-latest timeout-minutes: 12 strategy: @@ -117,7 +122,7 @@ jobs: run: bunx playwright test --shard=${{ matrix.shard }}/4 # The one required status check. Reports success when every shard passed - # or when the docs-only gate skipped them all, and fails on a shard + # or when the `changes` gate skipped them all, and fails on a shard # failure. A cancelled run (a newer push superseded it) is not a failure; # `!cancelled()` leaves it cancelled instead of painting the old head red. e2e: diff --git a/docs/CI-CD/workflows.md b/docs/CI-CD/workflows.md index 6b3ac69ea..4a87c8855 100644 --- a/docs/CI-CD/workflows.md +++ b/docs/CI-CD/workflows.md @@ -5,7 +5,7 @@ Compass uses GitHub Actions for continuous integration, Docker Hub for image dis | Workflow | Trigger | Purpose | |---|---|---| | Unit (`test-unit.yml`) | Push / PR / merge_group to `main` | Runs `static` (lint, knip, type-check) and unit tests | -| E2E (`test-e2e.yml`) | Push / PR / merge_group to `main` | Playwright e2e in four shards behind one required `e2e` gate (docs-only diffs skipped) | +| E2E (`test-e2e.yml`) | Push / PR / merge_group to `main` | Playwright e2e in four shards behind one required `e2e` gate (PRs that touch only docs, or only `packages/backend`, `packages/sync`, `packages/scripts`, skip the shards; merge queue and `main` always run them) | | CodeQL | Push / PR to `main` | Static security analysis | | Performance budget | Push to `main` (web/core/lock/budget), nightly schedule, `workflow_dispatch`; PR only when `.github/perf/**` or the workflow file changes | Lighthouse budget (not a required merge check) | | Error autofix (`error-autofix.yml`) | `posthog[bot]` issue / `workflow_dispatch` | Governed Routine: triage or fix PostHog error issues | diff --git a/packages/scripts/src/testing/detect-code-changes.test.ts b/packages/scripts/src/testing/detect-code-changes.test.ts index 9e1041240..be49fd7ad 100644 --- a/packages/scripts/src/testing/detect-code-changes.test.ts +++ b/packages/scripts/src/testing/detect-code-changes.test.ts @@ -74,6 +74,17 @@ describe("detect-code-changes", () => { } }); + it("gates the e2e shards on the e2e output and the unit legs on code", () => { + const unit = readFileSync(".github/workflows/test-unit.yml", "utf8"); + const e2e = readFileSync(".github/workflows/test-e2e.yml", "utf8"); + + expect(e2e).toContain("e2e: ${{ steps.filter.outputs.e2e }}"); + expect(e2e).toContain("if: needs.changes.outputs.e2e == 'true'"); + expect(e2e).not.toContain("needs.changes.outputs.code"); + expect(unit).toContain("code: ${{ steps.filter.outputs.code }}"); + expect(unit).not.toContain("outputs.e2e"); + }); + it("runs Unit once per PR push and names Unit vs E2E", () => { const unit = readFileSync(".github/workflows/test-unit.yml", "utf8"); const e2e = readFileSync(".github/workflows/test-e2e.yml", "utf8"); @@ -105,7 +116,7 @@ describe("detect-code-changes", () => { const result = runDetector(eventName); expect(result.status, result.stderr).toBe(0); - expect(result.output).toBe("code=true\n"); + expect(result.output).toBe("code=true\ne2e=true\n"); expect(result.command).toBe(""); } }); @@ -117,7 +128,36 @@ describe("detect-code-changes", () => { ); expect(result.status, result.stderr).toBe(0); - expect(result.output).toBe("code=false\n"); + expect(result.output).toBe("code=false\ne2e=false\n"); + }); + + it("skips only e2e for backend, sync, and scripts pull requests", () => { + const result = runDetector( + "pull_request", + "packages/backend/src/app.ts\npackages/sync/src/jobs.ts\npackages/scripts/src/cli.ts\ndocs/sync.md", + ); + + expect(result.status, result.stderr).toBe(0); + expect(result.output).toBe("code=true\ne2e=false\n"); + }); + + it("runs e2e when a backend pull request also touches anything else", () => { + for (const other of [ + "packages/core/src/types.ts", + "packages/web/src/app.tsx", + "e2e/timed/event-smoke.spec.ts", + "playwright.config.ts", + "bun.lock", + ".github/workflows/test-e2e.yml", + ]) { + const result = runDetector( + "pull_request", + `packages/backend/src/app.ts\n${other}`, + ); + + expect(result.status, result.stderr).toBe(0); + expect(result.output, other).toBe("code=true\ne2e=true\n"); + } }); it("runs checks for pull requests containing code", () => { @@ -127,7 +167,7 @@ describe("detect-code-changes", () => { ); expect(result.status, result.stderr).toBe(0); - expect(result.output).toBe("code=true\n"); + expect(result.output).toBe("code=true\ne2e=true\n"); expect(result.command).toContain( "api --paginate repos/example/compass/pulls/42/files --jq .[].filename", ); @@ -141,7 +181,7 @@ describe("detect-code-changes", () => { const result = runDetector("pull_request", files, ghStatus); expect(result.status, result.stderr).toBe(0); - expect(result.output).toBe("code=true\n"); + expect(result.output).toBe("code=true\ne2e=true\n"); } }); });