diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f6061b08..38f06ec8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -53,6 +53,9 @@ jobs: - name: Run unit tests run: make test-unit + - name: Self-check CI scripts + run: ./scripts/ci/conformance-scope_test.sh + lint: runs-on: ubuntu-latest steps: @@ -104,36 +107,38 @@ jobs: - name: Verify schema run: make verify-schema - # Discover test cases from testdata/ directory for matrix strategy + # Resolve which test cases this run exercises: the whole testdata/ matrix on + # push, dispatch or a `full-conformance` label; only the cases a pull request + # touched otherwise. A PR touching no fixture resolves to zero and skips the + # conformance jobs and their cleanup entirely. discover-tests: needs: [build, lint, pkl-validate, verify-schema, test-unit] runs-on: ubuntu-latest - # Conformance does not run on pull requests - see debug-conformance.yml - # for the targeted per-branch run. PRs are gated on the fast checks above. - if: github.event_name != 'pull_request' outputs: test-cases: ${{ steps.discover.outputs.test-cases }} + count: ${{ steps.discover.outputs.count }} steps: - name: Checkout uses: actions/checkout@v7 + with: + # The pull_request path diffs against the base commit, which needs the + # full history. Not a `github.event_name` ternary: `... && 0 || 1` + # yields 1 on a pull request, because 0 is falsy in a GitHub + # expression and `||` then takes the right branch. + fetch-depth: 0 - - name: Discover test cases + - name: Resolve conformance scope id: discover - run: | - # Find all *.pkl files in testdata/, exclude -update.pkl and -replace.pkl variants, - # and extract the base test case name - TEST_CASES=$(ls testdata/*.pkl 2>/dev/null \ - | xargs -n1 basename \ - | grep -v '\-update\.pkl$' \ - | grep -v '\-replace\.pkl$' \ - | sed 's/\.pkl$//' \ - | jq -R -s -c 'split("\n") | map(select(length > 0))') - echo "Discovered test cases: ${TEST_CASES}" - echo "test-cases=${TEST_CASES}" >> "$GITHUB_OUTPUT" + env: + EVENT_NAME: ${{ github.event_name }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + FULL_LABEL: ${{ contains(github.event.pull_request.labels.*.name, 'full-conformance') }} + run: ./scripts/ci/conformance-scope.sh # Clean up test resources before running any conformance tests pre-cleanup: needs: [discover-tests] + if: needs.discover-tests.outputs.count != '0' runs-on: ubuntu-latest permissions: id-token: write @@ -156,12 +161,14 @@ jobs: - name: Clean test resources run: ./scripts/ci/clean-environment.sh - # Conformance tests run the full CRUD lifecycle against real GCP resources, - # on push to main and on manual dispatch - not on pull requests. + # Conformance tests run the full CRUD lifecycle against real GCP resources. # Each test case runs in its own isolated job via matrix strategy. # Cleanup is handled by separate pre/post-cleanup jobs, NOT per test. + # An empty matrix vector is a hard error in Actions, so the count gate here is + # what makes "this PR touched no fixture" a skip rather than a red run. conformance-tests: needs: [discover-tests, pre-cleanup] + if: needs.discover-tests.outputs.count != '0' runs-on: ubuntu-latest # Raised to accommodate Cloud SQL: the CRUD + discovery lifecycle # provisions three instances sequentially (5-15 min each). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6ae35264..cd07d38a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -102,12 +102,47 @@ Workload Identity Federation (see [docs/gcp-github-actions-setup.md](docs/gcp-github-actions-setup.md) and the secrets below): -- **`ci.yml`** runs the whole matrix on push to `main` and on manual dispatch. - It does **not** run on pull requests: a full matrix takes 80-100 minutes, and - every conformance workflow here shares the `gcp-conformance-tests` - serialization group, so a run queued behind another is usually evicted before - it starts. Pull requests are gated on the fast checks (build, lint, unit - tests, manifest, schema). +- **`ci.yml`** resolves its conformance scope from the event: + + | Event | Scope | + |---|---| + | push to `main`, `workflow_dispatch` | every case in `testdata/` | + | pull request | only the cases the PR touched | + | pull request + `full-conformance` label | every case in `testdata/` | + + A pull request that touches no fixture resolves to zero cases and skips + `conformance-tests` and both cleanup jobs entirely, so a plumbing or refactor + PR does not pay for ~117 live GCP lifecycles. + + Scope is computed by `scripts/ci/conformance-scope.sh`: + + - Added, copied, modified or renamed `testdata/*.pkl` are in scope. + **Deletions are ignored** - a removed fixture has nothing left to run. + - An `-update` / `-replace` companion maps back to its case, since one matrix + entry drives the whole lifecycle. Editing only `bucket-update.pkl` still + runs `bucket`. + - `testdata/config/` is shared Pkl, not fixtures, and never becomes a case. + - A changed companion with no base fixture **fails the run** rather than + filtering to nothing and passing green. + - Names in `.github/conformance-pr-skip.txt` (optional; absent today) are + subtracted from a PR's scope. Use it for cases that cannot pass in CI + regardless of the diff. It does not affect push or nightly. A name in it + that matches no fixture fails the run, so the two cannot drift. + + **Known gap:** a change under `pkg/` or `testdata/config/vars.pkl` affects + every resource but touches no fixture, so it resolves to an empty scope. Two + backstops: label the PR `full-conformance`, or rely on push-to-`main`, which + still runs everything. Auto-escalating a one-line `vars.pkl` edit to 117 live + cases costs more than it catches. + + **Rough edge:** every conformance workflow here shares the + `gcp-conformance-tests` serialization group because they target one project, + and GitHub keeps only *one* run queued per group. A third concurrent run + evicts the waiting one, which then reports `cancelled` rather than failed. + PR-triggered conformance makes that more likely. + + `scripts/ci/conformance-scope_test.sh` covers the mapping rules against + throwaway git repos - no cloud credentials - and runs in the `test-unit` job. - **`debug-conformance.yml`** runs only the test cases you name, against the ref you dispatch it on. This is how you validate a resource change on your branch before opening the pull request: diff --git a/scripts/ci/conformance-scope.sh b/scripts/ci/conformance-scope.sh new file mode 100755 index 00000000..6c9f9213 --- /dev/null +++ b/scripts/ci/conformance-scope.sh @@ -0,0 +1,162 @@ +#!/usr/bin/env bash +# © 2025 Platform Engineering Labs Inc. +# SPDX-License-Identifier: FSL-1.1-ALv2 +# Decide which conformance test cases a run should exercise. +# +# push / workflow_dispatch / a PR labelled `full-conformance` +# -> every case discovered in testdata/ +# any other pull request +# -> only the cases the PR actually touched +# +# A pull request ran no conformance at all before this: a new or edited fixture +# was unverified until it reached main, and the only early check was dispatching +# debug-conformance.yml by hand with the case names typed out. Scoping to the +# touched cases gives a PR the same evidence the full matrix asks for - CRUD +# *and* discovery green - without paying for the other ~110 live GCP lifecycles. +# +# Deletions are ignored on purpose: a removed fixture has nothing left to run. +# A case's `-update` / `-replace` companion maps back to the case itself, since +# one matrix entry drives the whole lifecycle - the same rule ci.yml already +# uses to keep companions out of the matrix. +# +# Writes `test-cases` (a JSON array) and `count` to $GITHUB_OUTPUT. +# +# Inputs (environment): +# EVENT_NAME github.event_name +# BASE_SHA github.event.pull_request.base.sha (pull_request only) +# FULL_LABEL "true" when the PR carries the full-conformance label +# GITHUB_OUTPUT / GITHUB_STEP_SUMMARY optional; stdout-only when unset +set -euo pipefail + +EVENT_NAME="${EVENT_NAME:-push}" +BASE_SHA="${BASE_SHA:-}" +FULL_LABEL="${FULL_LABEL:-false}" + +SKIP_FILE=".github/conformance-pr-skip.txt" + +emit() { + local cases="$1" count="$2" reason="$3" skipped="${4:-}" + if [ -n "${GITHUB_OUTPUT:-}" ]; then + printf 'test-cases=%s\n' "$cases" >> "$GITHUB_OUTPUT" + printf 'count=%s\n' "$count" >> "$GITHUB_OUTPUT" + fi + if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then + { + printf '### Conformance scope\n\n%s\n\n' "$reason" + if [ -n "$skipped" ]; then + printf 'Skipped per `%s`:\n\n' "$SKIP_FILE" + printf '%s\n' "$skipped" | sed 's/^/- `/;s/$/`/' + printf '\n' + fi + if [ "$count" -eq 0 ]; then + printf 'No test cases to run.\n' + else + printf '%s test case(s):\n\n' "$count" + echo "$cases" | jq -r '.[] | "- `\(.)`"' + fi + } >> "$GITHUB_STEP_SUMMARY" + fi + printf '%s\n' "$reason" + if [ -n "$skipped" ]; then + printf 'skipped per %s: %s\n' "$SKIP_FILE" "$(printf '%s' "$skipped" | tr '\n' ' ')" + fi + printf 'count=%s\n' "$count" + echo "$cases" | jq -r 'if length == 0 then "(none)" else join(", ") end' +} + +entries() { + # One name per line, '#' comments and blank lines dropped. + [ -f "$1" ] || return 0 + grep -vE '^[[:space:]]*(#|$)' "$1" | sed 's/[[:space:]]*$//' | sed '/^$/d' +} + +# Every case name ci.yml would run: a top-level testdata/*.pkl that is not an +# -update / -replace companion. testdata/config/ holds shared Pkl, not fixtures. +discovered() { + find testdata -maxdepth 1 -name '*.pkl' -type f 2>/dev/null \ + | sed 's|^testdata/||;s|\.pkl$||' \ + | grep -vE -- '-(update|replace)$' \ + | sort -u +} + +# A skip entry that names no case does nothing and looks like it does. Fail +# rather than let the two drift apart. +assert_skip_names_exist() { + local unknown + unknown=$(comm -23 <(entries "$SKIP_FILE" | sort -u) <(discovered)) + if [ -n "$unknown" ]; then + echo "::error::$SKIP_FILE names cases that do not exist in testdata/: $(printf '%s' "$unknown" | tr '\n' ' ')" >&2 + exit 1 + fi +} + +assert_skip_names_exist + +# --- full matrix ------------------------------------------------------------ +if [ "$EVENT_NAME" != "pull_request" ] || [ "$FULL_LABEL" = "true" ]; then + CASES=$(discovered | jq -R . | jq -sc .) + COUNT=$(echo "$CASES" | jq 'length') + if [ "$COUNT" -eq 0 ]; then + echo "::error::no test cases discovered in testdata/" >&2 + exit 1 + fi + if [ "$FULL_LABEL" = "true" ]; then + REASON="Full matrix (\`full-conformance\` label)." + else + REASON="Full matrix (\`$EVENT_NAME\`)." + fi + emit "$CASES" "$COUNT" "$REASON" + exit 0 +fi + +# --- pull request: only what changed --------------------------------------- +if [ -z "$BASE_SHA" ]; then + echo "::error::BASE_SHA is required on a pull_request" >&2 + exit 1 +fi + +# Three-dot: changes on this branch since it diverged from the base, so an +# unrelated commit landing on the base does not pull extra cases into scope. +# Needs the full history that actions/checkout fetch-depth: 0 provides. +if ! git cat-file -e "${BASE_SHA}^{commit}" 2>/dev/null; then + echo "::error::base commit $BASE_SHA is not in this checkout - is fetch-depth: 0 set?" >&2 + exit 1 +fi + +CHANGED=$(git diff --name-only --diff-filter=ACMR "${BASE_SHA}...HEAD" -- 'testdata/*.pkl' || true) + +# Git's pathspec glob matches '/', so 'testdata/*.pkl' also catches +# testdata/config/vars.pkl. Only top-level files are fixtures. +NAMES=$(printf '%s\n' "$CHANGED" \ + | sed -n 's|^testdata/\([^/]*\)\.pkl$|\1|p' \ + | sed -E 's/-(update|replace)$//' \ + | sort -u \ + | sed '/^$/d') + +if [ -z "$NAMES" ]; then + emit '[]' 0 'No `testdata/*.pkl` fixtures added or modified - conformance skipped.' + exit 0 +fi + +# A name with no fixture behind it would put a matrix entry on the board that +# tests nothing and passes green. Same guard debug-conformance.yml wants for its +# hand-typed input. +MISSING=$(comm -23 <(printf '%s\n' "$NAMES") <(discovered)) +if [ -n "$MISSING" ]; then + echo "::error::changed companion fixture with no base fixture: $(printf '%s' "$MISSING" | tr '\n' ' ') - expected testdata/.pkl" >&2 + exit 1 +fi + +# Subtract the never-auto-run list. After the existence check, so a typo in the +# skip file cannot mask a genuinely broken fixture name. +SKIPPED=$(comm -12 <(printf '%s\n' "$NAMES") <(entries "$SKIP_FILE" | sort -u)) +KEPT=$(comm -23 <(printf '%s\n' "$NAMES") <(entries "$SKIP_FILE" | sort -u)) + +if [ -z "$KEPT" ]; then + emit '[]' 0 'Every case this pull request touched is on the never-auto-run list.' "$SKIPPED" + exit 0 +fi + +CASES=$(printf '%s\n' "$KEPT" | jq -R . | jq -sc .) +COUNT=$(echo "$CASES" | jq 'length') +emit "$CASES" "$COUNT" 'Test cases added or modified by this pull request.' "$SKIPPED" diff --git a/scripts/ci/conformance-scope_test.sh b/scripts/ci/conformance-scope_test.sh new file mode 100755 index 00000000..5762b6ae --- /dev/null +++ b/scripts/ci/conformance-scope_test.sh @@ -0,0 +1,140 @@ +#!/usr/bin/env bash +# © 2025 Platform Engineering Labs Inc. +# SPDX-License-Identifier: FSL-1.1-ALv2 +# Self-check for conformance-scope.sh. Builds a throwaway git repo per case, so +# it exercises the real diff and the real name mapping. No network, no GCP. +# +# ./scripts/ci/conformance-scope_test.sh +set -uo pipefail + +SCRIPT="$(cd "$(dirname "$0")" && pwd)/conformance-scope.sh" +PASS=0 FAIL=0 + +# run_case +run_case() { + local name="$1" want_count="$2" want_json="$3"; shift 3 + local dir; dir=$(mktemp -d) + ( + set -e + cd "$dir" + git init -q . + git config user.email t@t; git config user.name t + mkdir -p testdata/config .github scripts/ci + cp "$SCRIPT" scripts/ci/conformance-scope.sh + printf 'x\n' > testdata/pre-existing.pkl + printf 'shared\n' > testdata/config/vars.pkl + git add -A; git commit -q -m base + BASE=$(git rev-parse HEAD) + eval "$@" + git add -A; git commit -q -m change --allow-empty + out=$(EVENT_NAME=pull_request BASE_SHA="$BASE" FULL_LABEL=false \ + bash scripts/ci/conformance-scope.sh 2>&1) + got_count=$(printf '%s' "$out" | sed -n 's/^count=//p') + got_json=$(printf '%s' "$out" | tail -1) + if [ "$got_count" = "$want_count" ] && [ "$got_json" = "$want_json" ]; then + exit 0 + fi + echo " want count=$want_count json=$want_json" + echo " got count=$got_count json=$got_json" + exit 1 + ) + if [ $? -eq 0 ]; then + echo "ok $name"; PASS=$((PASS+1)) + else + echo "FAIL $name"; FAIL=$((FAIL+1)) + fi + rm -rf "$dir" +} + +run_case "added fixture is in scope" 1 "bucket" \ + 'printf "x\n" > testdata/bucket.pkl' + +run_case "-update companion maps back to its case" 1 "bucket" \ + 'printf "x\n" > testdata/bucket.pkl; printf "y\n" > testdata/bucket-update.pkl' + +run_case "-replace companion maps back too" 1 "disk" \ + 'printf "x\n" > testdata/disk.pkl; printf "y\n" > testdata/disk-replace.pkl' + +run_case "editing only the companion still runs the case" 1 "pre-existing" \ + 'printf "y\n" > testdata/pre-existing-update.pkl' + +run_case "deletion alone is out of scope" 0 "(none)" \ + 'git rm -q testdata/pre-existing.pkl' + +run_case "non-fixture change is out of scope" 0 "(none)" \ + 'printf "x\n" > README.md' + +# Git's pathspec glob matches '/', so testdata/config/vars.pkl is caught by the +# diff filter; it is shared Pkl, not a fixture, and must not become a case. +run_case "shared testdata/config Pkl is not a case" 0 "(none)" \ + 'printf "changed\n" > testdata/config/vars.pkl' + +run_case "several cases are deduped and sorted" 2 "a-thing, b-thing" \ + 'printf "x\n" > testdata/b-thing.pkl; printf "x\n" > testdata/b-thing-update.pkl; + printf "x\n" > testdata/a-thing.pkl' + +run_case "skip-listed case is subtracted" 1 "bucket" \ + 'printf "x\n" > testdata/bucket.pkl; printf "x\n" > testdata/alloydb-cluster.pkl; + printf "# r\nalloydb-cluster\n" > .github/conformance-pr-skip.txt' + +run_case "every touched case skip-listed leaves an empty scope" 0 "(none)" \ + 'printf "x\n" > testdata/alloydb-cluster.pkl; + printf "alloydb-cluster\n" > .github/conformance-pr-skip.txt' + +# A skip entry naming no fixture silently does nothing - fail instead. +dir=$(mktemp -d) +( + cd "$dir"; git init -q .; git config user.email t@t; git config user.name t + mkdir -p testdata .github scripts/ci + cp "$SCRIPT" scripts/ci/conformance-scope.sh + printf 'x\n' > testdata/bucket.pkl + printf 'buckett\n' > .github/conformance-pr-skip.txt + git add -A; git commit -q -m base + EVENT_NAME=push bash scripts/ci/conformance-scope.sh >/dev/null 2>&1 +) +if [ $? -ne 0 ]; then echo "ok unknown skip-list name fails loudly"; PASS=$((PASS+1)) +else echo "FAIL unknown skip-list name fails loudly"; FAIL=$((FAIL+1)); fi +rm -rf "$dir" + +# A companion whose base fixture does not exist must fail loudly rather than +# filter to nothing and pass green. +dir=$(mktemp -d) +( + cd "$dir"; git init -q .; git config user.email t@t; git config user.name t + mkdir -p testdata .github scripts/ci + cp "$SCRIPT" scripts/ci/conformance-scope.sh + printf 'x\n' > testdata/keep.pkl + git add -A; git commit -q -m base + BASE=$(git rev-parse HEAD) + printf 'y\n' > testdata/orphan-update.pkl + git add -A; git commit -q -m change + EVENT_NAME=pull_request BASE_SHA="$BASE" bash scripts/ci/conformance-scope.sh >/dev/null 2>&1 +) +if [ $? -ne 0 ]; then echo "ok orphan companion fails loudly"; PASS=$((PASS+1)) +else echo "FAIL orphan companion fails loudly"; FAIL=$((FAIL+1)); fi +rm -rf "$dir" + +# push and the full-conformance label both take the whole discovered matrix, +# and the skip list does not apply to either. +dir=$(mktemp -d) +( + cd "$dir"; git init -q .; git config user.email t@t; git config user.name t + mkdir -p testdata .github scripts/ci + cp "$SCRIPT" scripts/ci/conformance-scope.sh + printf 'x\n' > testdata/bucket.pkl + printf 'x\n' > testdata/bucket-update.pkl + printf 'x\n' > testdata/disk.pkl + printf 'disk\n' > .github/conformance-pr-skip.txt + git add -A; git commit -q -m base + push_out=$(EVENT_NAME=push bash scripts/ci/conformance-scope.sh 2>&1 | tail -1) + label_out=$(EVENT_NAME=pull_request FULL_LABEL=true BASE_SHA=$(git rev-parse HEAD) \ + bash scripts/ci/conformance-scope.sh 2>&1 | tail -1) + [ "$push_out" = "bucket, disk" ] && [ "$label_out" = "bucket, disk" ] +) +if [ $? -eq 0 ]; then echo "ok push and full-conformance label use the whole matrix"; PASS=$((PASS+1)) +else echo "FAIL push and full-conformance label use the whole matrix"; FAIL=$((FAIL+1)); fi +rm -rf "$dir" + +echo +echo "$PASS passed, $FAIL failed" +[ "$FAIL" -eq 0 ] diff --git a/testdata/bucket-update.pkl b/testdata/bucket-update.pkl index 5ea5ae0e..8bec0acf 100644 --- a/testdata/bucket-update.pkl +++ b/testdata/bucket-update.pkl @@ -22,7 +22,7 @@ forma { location = v.gcpRegion storageClass = "STANDARD" versioning = new bucket.Versioning { - enabled = false // This is the mutable field being tested (changed from false to true) + enabled = true // This is the mutable field being tested (changed from false to true) } labels = new Mapping { ["environment"] = "test"