diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index a98c83fe1..acf9c1bb4 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -115,6 +115,9 @@ jobs: e2e-capture-overhead: ${{ steps.e2e.outputs.e2e-capture-overhead }} e2e-bot-matrix: ${{ steps.e2e.outputs.bot-matrix }} e2e-bots-any: ${{ steps.e2e.outputs.bots-any }} + go-packages: ${{ steps.units.outputs.go-packages }} + web-tests: ${{ steps.units.outputs.web-tests }} + ci-jobs: ${{ steps.jobs.outputs.ci-jobs }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -139,6 +142,24 @@ jobs: HEAD_SHA: ${{ github.sha }} CI_FULL_RUN: "${{ contains(github.event.pull_request.labels.*.name, 'ci: full-run') }}" run: python3 hack/ci_scope.py e2e --base "$BASE_SHA" --head "$HEAD_SHA" + - name: select the unit test packages and web tests the change affects + id: units + # Package-level Go and `vitest related` selection (spec 023 US2). Any + # doubt emits ./... for every module and "all" for web. + env: + BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.before }} + HEAD_SHA: ${{ github.sha }} + CI_FULL_RUN: "${{ contains(github.event.pull_request.labels.*.name, 'ci: full-run') }}" + run: python3 hack/ci_scope.py units --base "$BASE_SHA" --head "$HEAD_SHA" + - name: map edited ci.yaml lines to their jobs + id: jobs + # A ci.yaml edit runs only the jobs it sits in, plus the jobs they need; + # an edit outside any job or in changes/report sets `all` (spec 023 US3, OD-3). + env: + BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.before }} + HEAD_SHA: ${{ github.sha }} + CI_FULL_RUN: "${{ contains(github.event.pull_request.labels.*.name, 'ci: full-run') }}" + run: python3 hack/ci_scope.py jobs --base "$BASE_SHA" --head "$HEAD_SHA" - uses: dorny/paths-filter@ceb8a2b8f2d89434be7ff52d3de7ec3738c5cc9d # v4.0.3 id: filter with: @@ -206,11 +227,11 @@ jobs: # smoke-tests deploy/kind/**'s pinned dev ingress-nginx manifest # on a real Kind cluster, and must NOT run on every e2e-affecting # change (a Go or chart edit does not touch that manifest pin). - # On PRs there is no `ci` fail-safe either — the workflow file - # itself is listed directly below. Master pushes always run it. + # On PRs there is no `ci` fail-safe either: an edit to this job's own + # ci.yaml block selects it through `ci-jobs`, and a full-scope run + # turns it on. Master pushes always run it. kindsmoke: - 'deploy/kind/**' - - '.github/workflows/ci.yaml' gamecoverage: - 'modules' - 'modules/**' @@ -243,20 +264,17 @@ jobs: - 'charts/gameplane/crd-manifests/**' - 'hack/check-codegen-drift_test.sh' ci: - - '.github/workflows/ci.yaml' + # ci.yaml and .golangci.yml map to their own jobs (step `jobs`, OD-3). - '.github/actions/**' - 'hack/ci_scope.py' - 'hack/test_ci_scope.py' + - 'hack/check_changed_coverage.py' + - 'hack/test_check_changed_coverage.py' - 'Makefile' - 'go.work' - 'go.work.sum' - 'test/e2e/buckets.sh' - # golangci-lint walks up from `working-directory` to find this - # config, so every `lint (…)` leg picks up an edit here even - # though none of them declare it as an input. Force the lint - # job on so a config change is tested against real code - # instead of silently taking effect on the next unrelated PR. - - '.golangci.yml' + - 'test/e2e/suite-components.json' github: - '.github/workflows/**' - '.github/actions/**' @@ -264,6 +282,8 @@ jobs: - '.github/dependabot.yml' - 'hack/ci_scope.py' - 'hack/test_ci_scope.py' + - 'hack/check_changed_coverage.py' + - 'hack/test_check_changed_coverage.py' - 'hack/check-ci-report-coverage.sh' - 'hack/test-check-ci-report-coverage.sh' - 'hack/testdata/check-ci-report-coverage/**' @@ -351,14 +371,14 @@ jobs: E2ETREE: ${{ steps.filter.outputs.e2etree }} GAMECOVERAGE: ${{ steps.filter.outputs.gamecoverage }} UNIVERSAL: ${{ steps.e2e.outputs.universal }} - CI: ${{ steps.filter.outputs.ci == 'true' || steps.scope.outputs.all == 'true' || steps.e2e.outputs.all == 'true' }} + CI: ${{ steps.filter.outputs.ci == 'true' || steps.scope.outputs.all == 'true' || steps.e2e.outputs.all == 'true' || steps.units.outputs.all == 'true' || steps.jobs.outputs.all == 'true' }} CAPTURE: ${{ steps.filter.outputs.capture_overhead }} SUBMODULES: ${{ steps.filter.outputs.submodules }} CODEGEN: ${{ steps.filter.outputs.codegen }} GITHUB: ${{ steps.filter.outputs.github }} SPECS: ${{ steps.filter.outputs.specs }} DOCS: ${{ steps.filter.outputs.docs }} - KINDSMOKE: "${{ steps.filter.outputs.kindsmoke == 'true' || (github.event_name == 'push' && github.ref == 'refs/heads/master') || github.event_name == 'workflow_dispatch' || contains(github.event.pull_request.labels.*.name, 'ci: full-run') }}" + KINDSMOKE: "${{ steps.filter.outputs.kindsmoke == 'true' || steps.scope.outputs.all == 'true' || steps.e2e.outputs.all == 'true' || steps.units.outputs.all == 'true' || steps.jobs.outputs.all == 'true' || (github.event_name == 'push' && github.ref == 'refs/heads/master') || github.event_name == 'workflow_dispatch' || contains(github.event.pull_request.labels.*.name, 'ci: full-run') }}" run: | gov=false; { [ "$GO" = true ] || [ "$CI" = true ]; } && gov=true webv=false; { [ "$WEB" = true ] || [ "$CI" = true ]; } && webv=true @@ -406,6 +426,8 @@ jobs: GATE_OUTPUTS: ${{ toJSON(steps.combine.outputs) }} BOT_MATRIX: ${{ steps.bots.outputs.matrix }} E2E_OUTPUTS: ${{ toJSON(steps.e2e.outputs) }} + UNITS_OUTPUTS: ${{ toJSON(steps.units.outputs) }} + CI_JOBS: ${{ steps.jobs.outputs.ci-jobs }} run: python3 hack/ci_scope.py summary --base "$BASE_SHA" --head "$HEAD_SHA" # Build the operator/api/agent (+ gameprobe) images ONCE and publish them as @@ -415,7 +437,7 @@ jobs: build-images: name: build e2e images needs: [changes] - if: needs.changes.outputs.images == 'true' + if: (needs.changes.outputs.images == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'build-images') runs-on: ubuntu-latest timeout-minutes: 15 permissions: @@ -429,7 +451,7 @@ jobs: build-images-arm64: name: build e2e images (arm64) needs: [changes] - if: needs.changes.outputs.images == 'true' + if: (needs.changes.outputs.images == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'build-images-arm64') runs-on: ubuntu-24.04-arm timeout-minutes: 20 permissions: @@ -471,7 +493,7 @@ jobs: capture-sidecar-setcap-proof: name: capture-sidecar CAP_NET_RAW survival needs: [changes] - if: needs.changes.outputs.go == 'true' + if: (needs.changes.outputs.go == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'capture-sidecar-setcap-proof') runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -579,7 +601,7 @@ jobs: vuln-images: name: vuln (images) needs: [changes, build-images] - if: needs.changes.outputs.images == 'true' + if: (needs.changes.outputs.images == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'vuln-images') runs-on: ubuntu-latest timeout-minutes: 20 permissions: @@ -611,7 +633,7 @@ jobs: docs: name: docs and repository checks needs: [changes] - if: needs.changes.outputs.lint == 'true' || needs.changes.outputs.specs == 'true' || needs.changes.outputs.docs == 'true' + if: (needs.changes.outputs.lint == 'true' || needs.changes.outputs.specs == 'true' || needs.changes.outputs.docs == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'docs') runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -649,7 +671,7 @@ jobs: codegen-drift: name: codegen drift needs: [changes] - if: needs.changes.outputs.codegen == 'true' || github.event_name == 'push' + if: (needs.changes.outputs.codegen == 'true' || github.event_name == 'push') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'codegen-drift') runs-on: ubuntu-latest timeout-minutes: 20 permissions: @@ -675,7 +697,7 @@ jobs: submodule-freshness: name: submodule freshness needs: [changes] - if: needs.changes.outputs.submodules == 'true' || github.event_name == 'push' + if: (needs.changes.outputs.submodules == 'true' || github.event_name == 'push') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'submodule-freshness') runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -718,7 +740,7 @@ jobs: vuln-go: name: vuln (go) needs: [changes] - if: needs.changes.outputs.go == 'true' || github.event_name == 'push' + if: (needs.changes.outputs.go == 'true' || github.event_name == 'push') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'vuln-go') runs-on: ubuntu-latest timeout-minutes: 30 permissions: @@ -738,7 +760,7 @@ jobs: lint: name: lint (${{ matrix.module }}) needs: [changes] - if: needs.changes.outputs.lint == 'true' + if: (needs.changes.outputs.lint == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'lint') runs-on: ubuntu-latest timeout-minutes: 15 permissions: @@ -814,7 +836,7 @@ jobs: go: name: go (${{ matrix.module }} / ${{ matrix.arch }}) needs: [changes] - if: needs.changes.outputs.go == 'true' + if: (needs.changes.outputs.go == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'go') # amd64 on the standard hosted runner; arm64 on the free public-repo # arm runner. Unit + envtest run on both arches every PR — this is where # real arch bugs (build tags, sqlite codepaths, struct alignment) surface. @@ -822,6 +844,12 @@ jobs: timeout-minutes: 20 permissions: contents: read + env: + # Space-separated package patterns from the changes job (spec 023 US2). + # Empty means no package is affected and only vet runs; a module the + # selection does not list runs ./... . + GO_PACKAGES: ${{ join(fromJSON(needs.changes.outputs.go-packages || '{}')[matrix.module] || fromJSON('["./..."]'), ' ') }} + BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.before }} strategy: fail-fast: false matrix: @@ -866,11 +894,13 @@ jobs: # uses -covermode=atomic, which is valid without -race. `make cover` # still runs -race for local/manual use. - name: unit test + coverage + if: env.GO_PACKAGES != '' working-directory: ${{ matrix.module }} run: | mkdir -p coverage + read -r -a packages <<<"$GO_PACKAGES" go test -covermode=atomic -coverpkg=./... \ - -coverprofile=coverage/unit.out ./... + -coverprofile=coverage/unit.out "${packages[@]}" - name: agent file access across real UIDs if: matrix.module == 'agent' && matrix.arch == 'amd64' working-directory: agent @@ -880,28 +910,30 @@ jobs: go test -c -o "$RUNNER_TEMP/agent-files.test" ./internal/files sudo "$RUNNER_TEMP/agent-files.test" -test.run='^TestFileAccessAcrossUIDs$' -test.v -test.timeout=2m - name: setup-envtest binary cache - if: matrix.module == 'operator' || matrix.module == 'api' + if: (matrix.module == 'operator' || matrix.module == 'api') && env.GO_PACKAGES != '' uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: ~/go/bin/setup-envtest key: setup-envtest-bin-release-0.24-${{ runner.os }}-${{ runner.arch }} - name: envtest assets cache - if: matrix.module == 'operator' || matrix.module == 'api' + if: (matrix.module == 'operator' || matrix.module == 'api') && env.GO_PACKAGES != '' uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: ~/.local/share/kubebuilder-envtest key: envtest-1.36.2-${{ runner.os }}-${{ runner.arch }} - name: integration (envtest) test + coverage - if: matrix.module == 'operator' || matrix.module == 'api' + if: (matrix.module == 'operator' || matrix.module == 'api') && env.GO_PACKAGES != '' working-directory: ${{ matrix.module }} run: | [ -x "$HOME/go/bin/setup-envtest" ] || go install sigs.k8s.io/controller-runtime/tools/setup-envtest@release-0.24 KUBEBUILDER_ASSETS=$("$HOME"/go/bin/setup-envtest use 1.36.2 -p path) export KUBEBUILDER_ASSETS + read -r -a packages <<<"$GO_PACKAGES" go test -tags=envtest -timeout 10m \ -covermode=atomic -coverpkg=./... \ - -coverprofile=coverage/envtest.out ./... + -coverprofile=coverage/envtest.out "${packages[@]}" - name: warm coverage tool cache + if: env.GO_PACKAGES != '' # cover-go-merge and cover-go-check `go run` pinned tools, so they hit # the module mirror as well and fail the same transient way. Fetch them # here, with retries, so the gate itself runs from the module cache and @@ -918,12 +950,25 @@ jobs: done fetch - name: merge profiles + threshold gate + # A whole-module run keeps the module total gate (spec 023 OD-1). + if: env.GO_PACKAGES == './...' run: make cover-go-merge && make cover-go-check + - name: merge profiles + changed-lines coverage gate + # A package-level run gates the added lines at the module's own + # threshold instead (spec 023 OD-1, OD-5). + if: env.GO_PACKAGES != '' && env.GO_PACKAGES != './...' + env: + MODULE: ${{ matrix.module }} + run: | + git fetch --no-tags --depth=1 origin "$BASE_SHA" + make cover-go-merge + python3 hack/check_changed_coverage.py --base "$BASE_SHA" --head "$GITHUB_SHA" \ + --go-profile "$MODULE/coverage/merged.out" --module "$MODULE" - name: per-file coverage of secrets_managed.go (spec 013 G-19) # The threshold gate checks package totals only. Print this one file's # statement coverage from the merged profile so coverage-gaps.md can # cite a measured figure. - if: matrix.module == 'api' && matrix.arch == 'amd64' + if: matrix.module == 'api' && matrix.arch == 'amd64' && env.GO_PACKAGES != '' run: | awk '$1 ~ /\/internal\/handlers\/secrets_managed\.go:/ { total += $2; if ($3 > 0) covered += $2; else { split($1, pos, ":"); missed = missed " " pos[2] } } END { if (total == 0) { print "secrets_managed.go not found in profile"; exit 1 } @@ -935,7 +980,7 @@ jobs: web: name: web needs: [changes] - if: needs.changes.outputs.web == 'true' + if: (needs.changes.outputs.web == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'web') runs-on: ubuntu-latest timeout-minutes: 15 permissions: @@ -1016,7 +1061,34 @@ jobs: - run: npm run build - name: unit tests + coverage id: cover - run: npm run test:cover + # Partial runs test only what the changed web/src files reach and gate + # the added lines; "all" keeps the full suite and thresholds (spec 023 OD-9). + env: + WEB_TESTS: ${{ needs.changes.outputs.web-tests }} + BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.before }} + run: | + set -euo pipefail + if [ -z "$WEB_TESTS" ] || [ "$WEB_TESTS" = '"all"' ]; then + npm run test:cover + exit 0 + fi + mapfile -t files < <(jq -r '.[] | sub("^web/"; "")' <<<"$WEB_TESTS") + if [ "${#files[@]}" -eq 0 ]; then + echo "No dashboard source changed; Vitest skipped, lint and build ran." + exit 0 + fi + npx vitest related --run --passWithNoTests --coverage --coverage.reporter=json \ + --coverage.thresholds.lines=0 --coverage.thresholds.functions=0 \ + --coverage.thresholds.branches=0 --coverage.thresholds.statements=0 \ + "${files[@]}" + if [ ! -f coverage/coverage-final.json ]; then + echo "No coverage report from vitest related; running the full suite." + npm run test:cover + exit 0 + fi + git fetch --no-tags --depth=1 origin "$BASE_SHA" + cd .. && python3 hack/check_changed_coverage.py --base "$BASE_SHA" --head "$GITHUB_SHA" \ + --vitest-json web/coverage/coverage-final.json - name: report coverage (commit status) if: always() continue-on-error: true @@ -1055,7 +1127,7 @@ jobs: web-e2e-mock: name: web e2e (mock) needs: [changes] - if: needs.changes.outputs.web == 'true' + if: (needs.changes.outputs.web == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'web-e2e-mock') runs-on: ubuntu-latest permissions: contents: read @@ -1088,7 +1160,7 @@ jobs: helm: name: helm lint needs: [changes] - if: needs.changes.outputs.charts == 'true' + if: (needs.changes.outputs.charts == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'helm') runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -1101,7 +1173,7 @@ jobs: standalone-compose: name: standalone Compose smoke needs: [changes] - if: needs.changes.outputs.standalone == 'true' + if: (needs.changes.outputs.standalone == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'standalone-compose') runs-on: ubuntu-latest timeout-minutes: 30 permissions: @@ -1147,7 +1219,7 @@ jobs: chart-template: name: chart render needs: [changes] - if: needs.changes.outputs.charts == 'true' + if: (needs.changes.outputs.charts == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'chart-template') runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -1510,7 +1582,7 @@ jobs: workflow-lint: name: workflow lint needs: [changes] - if: needs.changes.outputs.github == 'true' + if: (needs.changes.outputs.github == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'workflow-lint') runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -1580,7 +1652,7 @@ jobs: go-e2e-unit: name: go (test/e2e) needs: [changes] - if: needs.changes.outputs.e2e-unit == 'true' + if: (needs.changes.outputs.e2e-unit == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'go-e2e-unit') runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -1609,7 +1681,7 @@ jobs: api-postgres: name: api (postgres) needs: [changes] - if: contains(fromJSON(needs.changes.outputs.modules), 'api') + if: (contains(fromJSON(needs.changes.outputs.modules), 'api')) || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'api-postgres') runs-on: ubuntu-latest timeout-minutes: 15 permissions: @@ -1669,7 +1741,7 @@ jobs: e2e-buckets: name: e2e bucket coverage needs: [changes] - if: needs.changes.outputs.e2e == 'true' + if: (needs.changes.outputs.e2e == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'e2e-buckets') runs-on: ubuntu-latest timeout-minutes: 5 permissions: @@ -1707,7 +1779,7 @@ jobs: e2e-go: name: e2e ${{ matrix.bucket }} / ${{ matrix.arch }} (kind) needs: [changes, build-images] - if: needs.changes.outputs.e2e == 'true' + if: (needs.changes.outputs.e2e == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'e2e-go') runs-on: ${{ matrix.arch == 'arm64' && 'ubuntu-24.04-arm' || 'ubuntu-latest' }} timeout-minutes: ${{ matrix.job_timeout }} permissions: @@ -1814,7 +1886,7 @@ jobs: e2e-go-arm64: name: e2e ${{ matrix.bucket }} / ${{ matrix.arch }} (kind) needs: [changes, build-images-arm64] - if: needs.changes.outputs.e2e == 'true' + if: (needs.changes.outputs.e2e == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'e2e-go-arm64') runs-on: ${{ matrix.arch == 'arm64' && 'ubuntu-24.04-arm' || 'ubuntu-latest' }} timeout-minutes: ${{ matrix.job_timeout }} permissions: @@ -1880,7 +1952,7 @@ jobs: e2e-multicluster: name: e2e multicluster / ${{ matrix.arch }} (kind x2) needs: [changes, build-images] - if: needs.changes.outputs.e2e == 'true' && needs.changes.outputs.e2e-multicluster == 'true' + if: (needs.changes.outputs.e2e == 'true' && needs.changes.outputs.e2e-multicluster == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'e2e-multicluster') runs-on: ${{ matrix.arch == 'arm64' && 'ubuntu-24.04-arm' || 'ubuntu-latest' }} timeout-minutes: 60 # Same, across two clusters. permissions: @@ -1945,7 +2017,7 @@ jobs: e2e-multicluster-arm64: name: e2e multicluster / ${{ matrix.arch }} (kind x2) needs: [changes, build-images-arm64] - if: needs.changes.outputs.e2e == 'true' && needs.changes.outputs.e2e-multicluster == 'true' + if: (needs.changes.outputs.e2e == 'true' && needs.changes.outputs.e2e-multicluster == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'e2e-multicluster-arm64') runs-on: ${{ matrix.arch == 'arm64' && 'ubuntu-24.04-arm' || 'ubuntu-latest' }} timeout-minutes: 60 # Same, across two clusters. permissions: @@ -1972,7 +2044,7 @@ jobs: e2e-upgrade: name: e2e upgrade / ${{ matrix.arch }} (kind) needs: [changes, build-images] - if: needs.changes.outputs.e2e == 'true' && needs.changes.outputs.e2e-upgrade == 'true' + if: (needs.changes.outputs.e2e == 'true' && needs.changes.outputs.e2e-upgrade == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'e2e-upgrade') runs-on: ${{ matrix.arch == 'arm64' && 'ubuntu-24.04-arm' || 'ubuntu-latest' }} timeout-minutes: 60 # Same, plus a chart upgrade across versions. permissions: @@ -2022,7 +2094,7 @@ jobs: e2e-upgrade-arm64: name: e2e upgrade / ${{ matrix.arch }} (kind) needs: [changes, build-images-arm64] - if: needs.changes.outputs.e2e == 'true' && needs.changes.outputs.e2e-upgrade == 'true' + if: (needs.changes.outputs.e2e == 'true' && needs.changes.outputs.e2e-upgrade == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'e2e-upgrade-arm64') runs-on: ${{ matrix.arch == 'arm64' && 'ubuntu-24.04-arm' || 'ubuntu-latest' }} timeout-minutes: 60 # Same, plus a chart upgrade across versions. permissions: @@ -2042,7 +2114,7 @@ jobs: e2e-web-live: name: e2e web live / ${{ matrix.arch }} (kind) needs: [changes, build-images] - if: needs.changes.outputs.weblive == 'true' && needs.changes.outputs.e2e-web-live == 'true' + if: (needs.changes.outputs.weblive == 'true' && needs.changes.outputs.e2e-web-live == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'e2e-web-live') runs-on: ${{ matrix.arch == 'arm64' && 'ubuntu-24.04-arm' || 'ubuntu-latest' }} timeout-minutes: 60 # Same, plus a live dashboard drive. permissions: @@ -2122,7 +2194,7 @@ jobs: e2e-web-live-arm64: name: e2e web live / ${{ matrix.arch }} (kind) needs: [changes, build-images-arm64] - if: needs.changes.outputs.weblive == 'true' && needs.changes.outputs.e2e-web-live == 'true' + if: (needs.changes.outputs.weblive == 'true' && needs.changes.outputs.e2e-web-live == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'e2e-web-live-arm64') runs-on: ${{ matrix.arch == 'arm64' && 'ubuntu-24.04-arm' || 'ubuntu-latest' }} timeout-minutes: 60 # Same, plus a live dashboard drive. permissions: @@ -2139,7 +2211,7 @@ jobs: e2e-game-bot: name: e2e game bot / ${{ matrix.test }} (kind) needs: [changes, build-images] - if: needs.changes.outputs.e2e == 'true' && needs.changes.outputs.e2e-bots-any == 'true' + if: (needs.changes.outputs.e2e == 'true' && needs.changes.outputs.e2e-bots-any == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'e2e-game-bot') # amd64 only: this boots the external itzg/minecraft-server (multi-arch) AND # passivelemon/terraria-docker, whose arm64 support is unverified upstream — # so an arm64 leg would fail on the Terraria pull. Add an arch axis here once @@ -2207,7 +2279,8 @@ jobs: if: >- (github.event_name == 'push' && github.ref == 'refs/heads/master' && needs.changes.outputs.e2e == 'true') || (github.event_name == 'pull_request' && needs.changes.outputs.capture_overhead == 'true' && needs.changes.outputs.e2e-capture-overhead == 'true') || - (github.event_name == 'workflow_dispatch' && needs.changes.outputs.e2e == 'true') + (github.event_name == 'workflow_dispatch' && needs.changes.outputs.e2e == 'true') || + contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'e2e-capture-overhead') runs-on: ubuntu-latest timeout-minutes: 45 continue-on-error: true @@ -2268,7 +2341,7 @@ jobs: e2e-postgres: name: e2e postgres / amd64 (kind) needs: [changes, build-images] - if: needs.changes.outputs.e2e == 'true' + if: (needs.changes.outputs.e2e == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'e2e-postgres') runs-on: ubuntu-latest timeout-minutes: 75 # Boots a kind cluster with PostgreSQL, installs the chart, runs three buckets. permissions: @@ -2346,14 +2419,14 @@ jobs: # controller no longer reaches Available on the runner's Kind node image, # was previously only caught by a developer running `make dev-up` (#521). # - # Gated on `kindsmoke` (deploy/kind/** or this workflow file), not the + # Gated on `kindsmoke` (deploy/kind/** or a full-scope run), not the # broad `e2e` gate: a Go/chart/test-tree change doesn't touch the pinned # manifest or the cluster config this job exercises, so it stays out of # every other PR's critical path. e2e-kind-ingress-smoke: name: ingress-nginx smoke (kind) needs: [changes] - if: needs.changes.outputs.kindsmoke == 'true' + if: (needs.changes.outputs.kindsmoke == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'e2e-kind-ingress-smoke') runs-on: ubuntu-latest timeout-minutes: 15 permissions: diff --git a/Makefile b/Makefile index e2f585572..364339f79 100644 --- a/Makefile +++ b/Makefile @@ -493,3 +493,4 @@ clean: ## Remove build artifacts .PHONY: test-ci-scope test-ci-scope: ## Verify dependency-aware CI and image selection (CI only) python3 -m unittest discover -s hack -p 'test_ci_scope.py' -v + python3 -m unittest discover -s hack -p 'test_check_changed_coverage.py' -v diff --git a/docs/agent-architecture.md b/docs/agent-architecture.md index 02e2a47e5..3617b9cf6 100644 --- a/docs/agent-architecture.md +++ b/docs/agent-architecture.md @@ -47,6 +47,7 @@ This document is the **first stop** for AI agents modifying or extending Gamepla | Task | Read | | --- | --- | | Translations, language packs, localized server or module messages (i18n) | `docs/translating.md`, `specs/019-localization-language-packs/contracts/` (`pack-format.md`, `server-messages.md`, `i18n-api.md`, `module-locales.md`, `check-tool.md`), and the Internationalization section of `web/specs.md` | +| CI test selection: which jobs, packages and tests a PR runs (`hack/ci_scope.py`, `hack/check_changed_coverage.py`, `test/e2e/suite-components.json`, the `changes` job in `.github/workflows/ci.yaml`) | `docs/contributing.md` ("How CI picks tests"), `specs/023-change-based-test-selection/contracts/` (`ci-scope-cli.md`, `suite-components.md`, `changed-coverage.md`) | ## How to use this index 1. Identify the domain of your task (e.g., "Add an auth provider"). diff --git a/docs/contributing.md b/docs/contributing.md index affec7301..10d112309 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -72,6 +72,53 @@ These jobs block merging. Each gate with a fixture has a `hack/*_test.sh` self-t - **`codegen drift`** runs `make generate manifests` and `make tidy`, then fails if `git diff` is not empty. Run the same commands locally and commit what they change (CLAUDE.md rule 7). - **`submodule freshness`** runs `hack/check-submodule-freshness.sh`. It fails when the `modules` or `website` pointer names a commit that is not on that repo's `main`, when a gitlink is missing, or when the script cannot fetch that repo's `main`. For a pointer that is not on `main`, merge the submodule PR first, then bump the pointer to the merged commit. Otherwise restore the gitlink, or fix the fetch error, and re-run. +### How CI picks tests + +On a pull request, CI runs only the work your change can affect (spec 023). The +`detect changes` job decides, and its job summary has a **Test selection** table that +lists every suite as `run` or `skip`, with the changed files that chose it or the reason +everything ran. + +What a partial run narrows: + +- **Go unit tests** run only the packages that own a changed file, plus every package + that imports them, across modules. A module's own `go.mod` or `go.sum` reruns that + whole module plus every package that imports it, and its `.testcoverage.yml` reruns + the whole module. Deleting a package runs the packages that still import it. The + module coverage gate then becomes a + changed-lines gate: the lines you add must meet the module's own minimum + (`hack/check_changed_coverage.py`). +- **Web unit tests** run `vitest related` on the changed `web/src` files, with the same + changed-lines gate at the Vitest `lines` threshold. If `vitest related` writes no + coverage report, the job runs the full `npm run test:cover` instead. Lint and build + always run whole. + Shared inputs (`package.json`, the lockfile, the Vite/Vitest/TS/ESLint configs, + `src/test/**`, `i18n/locales/**`) run the full suite. +- **Go e2e tests** run only the tests that `test/e2e/suite-components.json` selects: + tests whose components or paths changed, tests that use a changed e2e test or helper + file, and every test when a universal path changes (charts, containers, CRD types, + the Kind setup, signing keys). Jobs with no selected test are skipped. +- **`ci.yaml` edits** run only the jobs whose lines changed, plus the jobs they need. + An edited e2e job also runs its other-architecture twin. + `.golangci.yml` runs only the lint legs and `workflow lint`. Other workflow files run + only `workflow lint`. + +Everything still runs on every push to `master`, and when a selection input changes +(`hack/ci_scope.py`, `hack/check_changed_coverage.py`, their tests, `Makefile`, +`go.work`, `go.work.sum`, `.github/actions/**`, `test/e2e/buckets.sh`, +`test/e2e/suite-components.json`). It also runs everything for a `ci.yaml` edit outside +any job or in `changes`/`report`, a path no rule maps, and any selection error. + +To force a full run on your PR, add the label `ci: full-run` (read on the next push), or +start the `ci` workflow by hand on your branch (`workflow_dispatch`). + +When you add an e2e test or a component, keep `test/e2e/suite-components.json` in step: +add the test name pattern under `suites` with the components and paths it exercises, or +list the component under `unexercised` with a reason. The `e2e bucket coverage` job +(`ci_scope.py verify-suites`) fails when a registered test matches no entry or a +component is missing. The full rules are in +`specs/023-change-based-test-selection/contracts/suite-components.md`. + ## Testing Run the whole suite: diff --git a/hack/check_changed_coverage.py b/hack/check_changed_coverage.py new file mode 100644 index 000000000..428433066 --- /dev/null +++ b/hack/check_changed_coverage.py @@ -0,0 +1,217 @@ +"""Changed-lines coverage gate for partial CI runs (spec 023 FR-009, OD-1, OD-5). + +Reads added lines from `git diff -U0`, intersects them with a Go cover profile or a +Vitest (Istanbul) coverage-final.json, and fails when the covered share of added +statement lines is below the module's own threshold. Python stdlib only. +""" + +import argparse +import fnmatch +import json +import os +import re +import subprocess +import sys +from pathlib import Path + + +ROOT = Path(__file__).resolve().parent.parent +PROFILE_LINE = re.compile(r"^(.+):(\d+)\.\d+,(\d+)\.\d+ (\d+) (\d+)$") +HUNK = re.compile(r"^@@ -\S+ \+(\d+)(?:,(\d+))? @@") + + +def added_lines(root, base, head, prefix): + """Return {repo path: set of added line numbers} for files under prefix.""" + diff = subprocess.run( + ["git", "diff", "-U0", "--no-color", "--no-renames", base, head, "--", prefix], + cwd=root, check=True, capture_output=True, text=True).stdout + lines, current = {}, None + for line in diff.splitlines(): + if line.startswith("+++ "): + target = line[4:] + current = target[2:] if target.startswith("b/") else None + elif current and line.startswith("@@"): + match = HUNK.match(line) + if match: + start, count = int(match[1]), int(match[2] or "1") + lines.setdefault(current, set()).update(range(start, start + count)) + return lines + + +def expand(glob): + """Expand {a,b} alternatives in a glob into plain fnmatch patterns.""" + match = re.search(r"\{([^{}]*)\}", glob) + if not match: + return [glob] + return [pattern for option in match[1].split(",") + for pattern in expand(glob[:match.start()] + option + glob[match.end():])] + + +def go_config(module_dir): + """Return (threshold.total, exclude.paths regexes) from a module's .testcoverage.yml.""" + text = (module_dir / ".testcoverage.yml").read_text(encoding="utf-8") + total, excludes, section = None, [], None + for raw in text.splitlines(): + line = raw.split("#", 1)[0].rstrip() + if not line.strip(): + continue + if not line.startswith((" ", "-")): + section = line.split(":", 1)[0].strip() + continue + item = line.strip() + if section == "threshold" and item.startswith("total:"): + total = float(item.split(":", 1)[1]) + elif section == "exclude" and item.startswith("- "): + excludes.append(item[2:].strip().strip("'\"")) + if total is None: + raise ValueError(f"{module_dir.name}/.testcoverage.yml has no threshold.total") + return total, excludes + + +def vitest_config(web_dir): + """Return (coverage.thresholds.lines, expanded coverage.exclude globs) from vitest.config.ts.""" + text = (web_dir / "vitest.config.ts").read_text(encoding="utf-8") + start = text.find("coverage:") + coverage = text[start:] if start >= 0 else "" + lines = re.search(r"thresholds:\s*\{[^}]*?\blines:\s*(\d+(?:\.\d+)?)", coverage) + if not lines: + raise ValueError("web/vitest.config.ts has no coverage.thresholds.lines") + block = re.search(r"\bexclude:\s*\[(.*?)\]", coverage, re.DOTALL) + globs = re.findall(r'"([^"]+)"', block[1]) if block else [] + return float(lines[1]), [pattern for glob in globs for pattern in expand(glob)] + + +def mark(lines, first, last, covered): + """Mark lines first..last; a line is covered when any block covering it ran.""" + for number in range(first, last + 1): + lines[number] = lines.get(number, False) or covered + + +def go_statements(profile, module_path, module): + """Return {repo path: {line: covered}} from a Go cover profile.""" + result = {} + for line in Path(profile).read_text(encoding="utf-8").splitlines(): + match = PROFILE_LINE.match(line) + if not match or not match[1].startswith(module_path + "/"): + continue + path = module + "/" + match[1][len(module_path) + 1:] + mark(result.setdefault(path, {}), int(match[2]), int(match[3]), int(match[5]) > 0) + return result + + +def vitest_statements(report, root): + """Return {repo path: {line: covered}} from an Istanbul coverage-final.json.""" + data = json.loads(Path(report).read_text(encoding="utf-8")) + result = {} + for name, entry in data.items(): + path = Path(entry.get("path", name)) + if not path.is_absolute(): + path = root / path + try: + relative = path.resolve().relative_to(root.resolve()).as_posix() + except ValueError: + continue + lines = result.setdefault(relative, {}) + counts = entry.get("s", {}) + for key, location in entry.get("statementMap", {}).items(): + mark(lines, location["start"]["line"], location["end"]["line"], + counts.get(key, 0) > 0) + return result + + +def check(added, statements, threshold, excluded): + """Return (exit code, one-line result, uncovered file:line list).""" + total, missing = 0, [] + for path in sorted(added): + if excluded(path): + continue + known = statements.get(path, {}) + for number in sorted(added[path]): + if number in known: + total += 1 + if not known[number]: + missing.append(f"{path}:{number}") + if total == 0: + return 0, "changed-lines coverage: no added statement lines, passes", [] + covered = total - len(missing) + share = 100 * covered / total + line = (f"changed-lines coverage: {covered}/{total} added statement lines covered " + f"({share:.1f}%, minimum {threshold:g}%)") + return (0 if share >= threshold else 1), line, missing + + +def go_inputs(root, module, profiles, base, head): + """Return (threshold, added lines, statements, excluded) for one Go module.""" + threshold, patterns = go_config(root / module) + text = (root / module / "go.mod").read_text(encoding="utf-8") + match = re.search(r"^module\s+(\S+)", text, re.MULTILINE) + if not match: + raise ValueError(f"{module}/go.mod has no module directive") + statements = {} + for profile in profiles: + for path, lines in go_statements(profile, match[1], module).items(): + merged = statements.setdefault(path, {}) + for number, covered in lines.items(): + mark(merged, number, number, covered) + + def excluded(path): + """Skip tests, non-Go files and the module's own coverage exclusions.""" + relative = path[len(module) + 1:] + directory = relative.rpartition("/")[0] + return (not relative.endswith(".go") or relative.endswith("_test.go") + or any(re.search(p, relative) or re.search(p, directory) for p in patterns)) + + return threshold, added_lines(root, base, head, module), statements, excluded + + +def web_inputs(root, report, base, head): + """Return (threshold, added lines, statements, excluded) for the dashboard.""" + threshold, globs = vitest_config(root / "web") + + def excluded(path): + """Skip tests, non-TypeScript files and vitest.config.ts coverage.exclude.""" + relative = path[len("web/"):] + return (not relative.endswith((".ts", ".tsx")) + or relative.endswith((".test.ts", ".test.tsx")) + or any(fnmatch.fnmatch(relative, glob) for glob in globs)) + + return (threshold, added_lines(root, base, head, "web/src"), + vitest_statements(report, root), excluded) + + +def main(argv=None): + """Parse arguments, run the gate and return the exit code.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--base", required=True) + parser.add_argument("--head", required=True) + parser.add_argument("--go-profile", action="append", default=[]) + parser.add_argument("--module") + parser.add_argument("--vitest-json") + parser.add_argument("--root", default=str(ROOT)) + args = parser.parse_args(argv) + root = Path(args.root) + if not args.vitest_json and not (args.go_profile and args.module): + parser.error("pass --vitest-json, or --go-profile with --module") + try: + if args.vitest_json: + inputs = web_inputs(root, args.vitest_json, args.base, args.head) + else: + inputs = go_inputs(root, args.module.strip("/"), args.go_profile, args.base, + args.head) + except (OSError, ValueError, KeyError, subprocess.CalledProcessError) as exc: + print(f"::error::changed-lines coverage: {exc}") + return 1 + threshold, added, statements, excluded = inputs + code, line, missing = check(added, statements, threshold, excluded) + print(line) + for item in missing: + print(f"uncovered: {item}") + summary = os.environ.get("GITHUB_STEP_SUMMARY") + if summary: + with open(summary, "a", encoding="utf-8") as output: + output.write(line + "\n") + return code + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/hack/ci_scope.py b/hack/ci_scope.py index eb4deba2c..91da39131 100644 --- a/hack/ci_scope.py +++ b/hack/ci_scope.py @@ -22,8 +22,9 @@ ] # Mirrors the paths-filter `ci` filter in .github/workflows/ci.yaml; keep both in sync. FORCE_ALL_FILES = { - ".github/workflows/ci.yaml", "hack/ci_scope.py", "hack/test_ci_scope.py", "Makefile", - "go.work", "go.work.sum", "test/e2e/buckets.sh", ".golangci.yml", + "hack/ci_scope.py", "hack/test_ci_scope.py", "Makefile", "go.work", "go.work.sum", + "test/e2e/buckets.sh", "test/e2e/suite-components.json", + "hack/check_changed_coverage.py", "hack/test_check_changed_coverage.py", } FORCE_ALL_PREFIXES = (".github/actions/",) # Mirrors the e2e-go matrix buckets in .github/workflows/ci.yaml. @@ -32,7 +33,7 @@ SUITES_FILE = "test/e2e/suite-components.json" # Paths no Go e2e test reads. Their own jobs keep their paths-filter gates. NO_E2E_FILES = { - "design.pen", "LICENSE", ".claudeignore", ".coderabbit.yaml", ".editorconfig", + "design.pen", "LICENSE", ".claudeignore", ".coderabbit.yaml", ".editorconfig", ".golangci.yml", ".gitattributes", ".gitignore", ".gitmodules", ".hadolint.yaml", "deploy/kind/up.sh", "deploy/kind/down.sh", "test/e2e/joincoverage.sh", "test/e2e/joincoverage_test.sh", @@ -46,6 +47,65 @@ EXTRA_COMPONENTS = ("web", "charts", "deploy-kind", "modules", "e2e-harness") # Top-level areas a selection rule knows; any other path is unmapped and forces full scope. KNOWN_DIRS = ("charts", "containers", "deploy/kind", "signing", "test/e2e", "web", "modules") +# A change to one of these files in a module root reruns that whole module (research R4). +GO_MODULE_WIDE = {"go.mod", "go.sum", ".testcoverage.yml"} +# Tests that read files outside their package directory. A changed path under a key also +# selects these package directories (research R4, the package form of file_consumers). +FILE_READERS = { + "operator/config/crd": ("operator/internal/controller", "api/internal/handlers"), + "operator/test/crds": ("operator/internal/controller",), + "modules": ("gp-module/internal/validator",), + "i18n/testdata": ("i18n/cmd/i18n-check",), + "api/internal": ("api/internal/messages",), # usage_test walks every api/internal file +} +# Directories that never hold a package the go job tests. +SKIP_PACKAGE_DIRS = {"testdata", "vendor", "node_modules"} +# Web files that change every Vitest run (research R6). +WEB_ALL_FILES = {"web/package.json", "web/package-lock.json", "web/vitest.config.ts", + "web/vite.config.ts", "web/eslint.config.js"} +CI_FILE = ".github/workflows/ci.yaml" +# ci.yaml jobs that compute or report the selection; an edit there forces full scope (R8). +CI_SELECTION_JOBS = {"changes", "report"} +CI_JOB_KEY = re.compile(r"^ ([A-Za-z0-9_-]+):\s*(?:#.*)?$") +CI_TOP_KEY = re.compile(r"^[A-Za-z_][\w-]*:") +# e2e jobs whose own switch output a ci.yaml edit inside them turns on (mode e2e). +E2E_JOB_SWITCHES = { + "e2e-multicluster": "e2e-multicluster", "e2e-multicluster-arm64": "e2e-multicluster", + "e2e-upgrade": "e2e-upgrade", "e2e-upgrade-arm64": "e2e-upgrade", + "e2e-web-live": "e2e-web-live", "e2e-web-live-arm64": "e2e-web-live", + "e2e-capture-overhead": "e2e-capture-overhead", +} +# Summary row prefixes and the ci.yaml jobs behind them; the first match wins. +ROW_JOBS = ( + ("lint (", ("lint",)), + ("go (test/e2e)", ("go-e2e-unit",)), + ("go (", ("go",)), + ("api (postgres)", ("api-postgres",)), + ("web e2e (mock)", ("web-e2e-mock",)), + ("web", ("web",)), + ("helm lint", ("helm",)), + ("chart render", ("chart-template",)), + ("standalone Compose smoke", ("standalone-compose",)), + ("workflow lint", ("workflow-lint",)), + ("docs and repository checks", ("docs",)), + ("codegen drift", ("codegen-drift",)), + ("submodule freshness", ("submodule-freshness",)), + ("vuln (go)", ("vuln-go",)), + ("capture-sidecar CAP_NET_RAW survival", ("capture-sidecar-setcap-proof",)), + ("dependency review", ("dependency-review",)), + ("telemetry default gate", ("telemetry-default-gate",)), + ("build e2e images", ("build-images", "build-images-arm64")), + ("vuln (images)", ("vuln-images",)), + ("e2e bucket coverage", ("e2e-buckets",)), + ("e2e multicluster", ("e2e-multicluster", "e2e-multicluster-arm64")), + ("e2e upgrade", ("e2e-upgrade", "e2e-upgrade-arm64")), + ("e2e postgres", ("e2e-postgres",)), + ("e2e game bot", ("e2e-game-bot",)), + ("e2e web live", ("e2e-web-live", "e2e-web-live-arm64")), + ("e2e capture overhead", ("e2e-capture-overhead",)), + ("ingress-nginx smoke", ("e2e-kind-ingress-smoke",)), + ("e2e ", ("e2e-go", "e2e-go-arm64")), +) def is_unmapped(path, modules): @@ -96,9 +156,12 @@ def changed_paths(root, base, head, *, history=False): def affected_modules(root, paths): """Select changed modules and the reverse dependency closure, including tests.""" modules = workspace_modules(root) - shared = {"Makefile", "go.work", "go.work.sum", ".golangci.yml", "test/e2e/buckets.sh"} + # ci.yaml and .golangci.yml map to their own jobs through ci_jobs() (OD-3). + shared = {"Makefile", "go.work", "go.work.sum", "test/e2e/buckets.sh"} if paths is None or any( - p in shared or p.startswith((".github/", "hack/ci_scope", "hack/test_ci_scope")) + p in shared or p.startswith((".github/actions/", "hack/ci_scope", "hack/test_ci_scope", + "hack/check_changed_coverage", + "hack/test_check_changed_coverage")) or Path(p).name in {"go.mod", "go.sum"} or ("Dockerfile" in Path(p).name and not any(p.startswith(m + "/") for m in modules)) for p in paths @@ -163,6 +226,7 @@ def image_inputs(dockerfile): def select_images(root, images, paths): + """Return the images to rebuild: all of them for a config change, else those with changed inputs.""" # Signing/configuration changes must rebuild all images even without COPYs. if paths is None or any( p in {"go.work", "go.work.sum", ".dockerignore", "signing/cosign.pub"} @@ -448,7 +512,7 @@ def describe(names, total): return text + (f"; {cite(files)}" if files else "") run, why = {}, {} - for bucket in E2E_BUCKETS + ["ratelimit", "multicluster", "upgrade", "capture-overhead"]: + for bucket in [*E2E_BUCKETS, "ratelimit", "multicluster", "upgrade", "capture-overhead"]: names = chosen(bucket) if names: why[bucket] = describe(names, len(buckets[bucket])) @@ -473,6 +537,248 @@ def describe(names, total): } +def go_imports(text): + """Return the import paths named in a Go file's import declarations.""" + found = re.findall(r'^import\s+(?:[\w.]+\s+)?"([^"]+)"', text, re.MULTILINE) + for block in re.findall(r"^import\s*\((.*?)\)", text, re.MULTILINE | re.DOTALL): + found += re.findall(r'"([^"]+)"', block) + return found + + +def package_index(root, modules): + """Return {package dir: imported paths} and {import path: package dir} for the workspace.""" + imports, dirs = {}, {} + for module, name in modules.items(): + base = root / module + for current, subdirs, files in os.walk(base): + here = Path(current) + subdirs[:] = sorted(d for d in subdirs if not d.startswith((".", "_")) + and d not in SKIP_PACKAGE_DIRS + and not (here / d / "go.mod").exists()) + sources = [f for f in files if f.endswith(".go")] + if not sources: + continue + directory = here.relative_to(root).as_posix() + inner = here.relative_to(base).as_posix() + dirs[name if inner == "." else f"{name}/{inner}"] = directory + imports[directory] = {path for f in sorted(sources) + for path in go_imports((here / f).read_text(encoding="utf-8"))} + return imports, dirs + + +def owning_module(path, modules): + """Return the workspace module directory that contains path, or None.""" + owners = [m for m in modules if under(path, m)] + return max(owners, key=len) if owners else None + + +def go_packages(root, paths, reason): + """Return {module: [go test package patterns]} for the go job (research R4, OD-8). + + A changed file selects its package and, unless it is a _test.go file, every package + that imports it, across modules. A module's own go.mod, go.sum or .testcoverage.yml, + or a file in no package, selects ./... for that module; go.mod and go.sum also + select every package that imports the module, and a deleted package selects the + packages that still import it. + """ + modules = workspace_modules(root) + targets = [m for m in sorted(modules) if m != "test/e2e"] + if reason or paths is None: + return {m: ["./..."] for m in targets} + imports, dirs = package_index(root, modules) + wide, seeds, direct = set(), set(), set() + for path in paths: + if path.endswith(".md"): + continue + readers = [d for key, found in FILE_READERS.items() if under(path, key) for d in found] + direct.update(readers) # a reader's tests read the files; its importers do not + module = owning_module(path, modules) + if module is None: + continue + if path in {f"{module}/{name}" for name in GO_MODULE_WIDE}: + wide.add(module) + if path.rpartition("/")[2] in {"go.mod", "go.sum"}: + # a dependency change can break every package that imports this module + seeds.update(d for d in imports if owning_module(d, modules) == module) + continue + directory = path.rpartition("/")[0] + if path.endswith(".go") and directory not in imports: + # the package lost its last Go file: its import path is still in its importers + gone = modules[module] + directory[len(module):] + seeds.update(d for d, found in imports.items() if gone in found) + while directory not in imports and directory != module: + directory = directory.rpartition("/")[0] + if directory in imports: + (direct if path.endswith("_test.go") else seeds).add(directory) + elif not readers: + wide.add(module) + importers = {} + for directory, imported in imports.items(): + for name in imported: + if name in dirs: + importers.setdefault(dirs[name], set()).add(directory) + selected, queue = set(), [d for d in seeds if d in imports] + while queue: + directory = queue.pop() + if directory not in selected: + selected.add(directory) + queue.extend(importers.get(directory, ())) + selected |= {d for d in direct if d in imports} + result = {} + for module in targets: + if module in wide: + result[module] = ["./..."] + else: + result[module] = sorted("." if d == module else "./" + d[len(module) + 1:] + for d in selected if owning_module(d, modules) == module) + return result + + +def web_tests(root, paths, reason): + """Return "all" or the changed web/src files for `vitest related` (research R6, OD-9).""" + if reason or paths is None: + return "all" + files = [] + for path in paths: + if (path in WEB_ALL_FILES or under(path, "web/src/test") or under(path, "i18n/locales") + or fnmatch.fnmatch(path, "web/tsconfig*.json")): + return "all" + if not under(path, "web") or path.endswith(".md") or under(path, "web/e2e"): + continue + if not under(path, "web/src") or not (root / path).exists(): + return "all" # vitest related cannot follow a deleted file or a non-source input + files.append(path) + return sorted(files) + + +def unit_outputs(root, paths, reason): + """Return the outputs of mode units: Go package patterns per module and web tests.""" + return { + "all": bool(reason), + "reason": reason or "", + "go-packages": go_packages(root, paths, reason), + "web-tests": web_tests(root, paths, reason), + } + + +def ci_blocks(text): + """Return ({line number: job id or None}, {job id: needed job ids}) for a workflow.""" + owner, needs = {}, {} + job, in_jobs, collecting = None, False, None + for number, line in enumerate(text.splitlines(), 1): + if CI_TOP_KEY.match(line): + in_jobs, job, collecting = line.split(":", 1)[0] == "jobs", None, None + elif in_jobs and CI_JOB_KEY.match(line): + job, collecting = CI_JOB_KEY.match(line)[1], None + needs.setdefault(job, set()) + elif job and line.startswith(" needs:"): + collecting = job + line = line[len(" needs:"):] + elif re.match(r"^ \S", line): + collecting = None + owner[number] = job + if collecting: + needs[collecting].update(re.findall(r"[A-Za-z0-9_][A-Za-z0-9_-]*", + line.split("#", 1)[0])) + return owner, needs + + +def changed_ci_lines(root, base, head): + """Return ([(old line, text)], [(new line, text)]) for ci.yaml lines changed base..head.""" + diff = subprocess.run(["git", "diff", "-U0", "--no-color", base, head, "--", CI_FILE], + cwd=root, check=True, capture_output=True, text=True).stdout + removed, added, old, new, started = [], [], 0, 0, False + for line in diff.splitlines(): + hunk = re.match(r"^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@", line) + if hunk: + old, new, started = int(hunk[1]), int(hunk[2]), True + elif started and line.startswith("-"): + removed.append((old, line[1:])) + old += 1 + elif started and line.startswith("+"): + added.append((new, line[1:])) + new += 1 + return removed, added + + +def git_text(root, revision, path): + """Return a file's text at revision, or "" when it does not exist there.""" + result = subprocess.run(["git", "show", f"{revision}:{path}"], cwd=root, + capture_output=True, text=True, check=False) + return result.stdout if result.returncode == 0 else "" + + +def ci_jobs(root, base, head, paths): + """Map shared CI file changes to ci.yaml job ids (research R8, OD-3). + + Returns (jobs, forced). jobs holds every job whose ci.yaml lines changed, `lint` for + .golangci.yml, `workflow-lint` for any .github/ or .golangci.yml change, and every job + they need. forced is True when a changed ci.yaml line that is not blank or a comment + sits outside any job or inside the changes or report job. + """ + jobs, forced, needs = set(), False, {} + if any(p.startswith(".github/") or p == ".golangci.yml" for p in paths): + jobs.add("workflow-lint") + if ".golangci.yml" in paths: + jobs.add("lint") + if CI_FILE in paths: + removed, added = changed_ci_lines(root, base, head) + for revision, changes in ((base, removed), (head, added)): + owner, found = ci_blocks(git_text(root, revision, CI_FILE)) + needs.update(found) + for number, text in changes: + if not text.strip() or text.strip().startswith("#"): + continue + job = owner.get(number) + if job is None or job in CI_SELECTION_JOBS: + forced = True + else: + jobs.add(job) + for name in list(jobs): # FR-010: an e2e suite runs on amd64 and arm64 together + twin = name[:-len("-arm64")] if name.endswith("-arm64") else name + "-arm64" + if name.startswith("e2e-") and twin in needs: + jobs.add(twin) + queue = list(jobs) + while queue: + for name in needs.get(queue.pop(), ()): + if name not in jobs: + jobs.add(name) + queue.append(name) + return sorted(jobs - CI_SELECTION_JOBS), forced + + +def scoped(env, base, head): + """Return (paths, reason, ci.yaml jobs): the diff, why everything runs, edited jobs.""" + event_reason = full_scope_reason(env, []) + paths = None if event_reason else changed_paths(ROOT, base, head) + reason = full_scope_reason(env, paths) + if reason: + return paths, reason, [] + jobs, forced = ci_jobs(ROOT, base, head, paths) + return paths, ("selection-rule change" if forced else None), jobs + + +def widen_e2e(values, full, jobs): + """Run every test of each e2e job whose ci.yaml block changed (research R8).""" + if {"e2e-go", "e2e-go-arm64"} & set(jobs): + values["e2e-run"] = full["e2e-run"] + for job, key in E2E_JOB_SWITCHES.items(): + if job in jobs: + values[key] = True + if "e2e-game-bot" in jobs: + values["bot-matrix"], values["bots-any"] = full["bot-matrix"], full["bots-any"] + return values + + +def widen_units(values, jobs): + """Run every package or web test of a go or web job whose ci.yaml block changed.""" + if "go" in jobs: + values["go-packages"] = {m: ["./..."] for m in values["go-packages"]} + if "web" in jobs: + values["web-tests"] = "all" + return values + + def verify_suites(suites, buckets, modules): """Return the violations of contracts/suite-components.md rules 1, 2 and 5.""" errors = [] @@ -515,6 +821,10 @@ def summarize(env, paths, reason): gates = load_json(env, "GATE_OUTPUTS") bots = load_json(env, "BOT_MATRIX") e2e = load_json(env, "E2E_OUTPUTS") + units = load_json(env, "UNITS_OUTPUTS") + edited_jobs = set(jlist(env, "CI_JOBS")) + go_selected = decoded(units, "go-packages", {}) + web_selected = decoded(units, "web-tests", "all") # Without the e2e step's outputs every e2e suite follows the coarse e2e gate. narrowed = bool(e2e) e2e_run = decoded(e2e, "e2e-run", {}) @@ -553,8 +863,25 @@ def e2e_feed(): rows = [] - def add(suite, run, feed=(), run_reason="", skip_reason="not affected"): + def go_note(module): + """Return the package count note for one go row, or "" for a whole-module run.""" + packages = go_selected.get(module, ["./..."]) if units else ["./..."] + if packages == ["./..."]: + return "" + return f"{len(packages)} packages; " if packages else "vet only, no affected package; " + + if not units or web_selected == "all": + web_note = "" + elif web_selected: + web_note = f"{len(web_selected)} files for vitest related; " + else: + web_note = "lint and build only; " + + def add(suite, run, feed=(), run_reason="", skip_reason="not affected", note=""): """Append one selection row with a run or skip decision and its reason.""" + jobs = next((found for prefix, found in ROW_JOBS if suite.startswith(prefix)), ()) + if not run and edited_jobs & set(jobs): + run, run_reason = True, "ci.yaml: lines in this job changed" if run and reason: why = f"full: {reason}" elif run and run_reason: @@ -567,16 +894,19 @@ def add(suite, run, feed=(), run_reason="", skip_reason="not affected"): why = "push event" if event == "push" else "changed: (see filters)" else: why = skip_reason + if run and not reason: + why = note + why rows.append({"suite": suite, "decision": "run" if run else "skip", "reason": why}) for m in modules: add(f"lint ({m})", m in selected, module_feed(m)) for m in modules: if m != "test/e2e": - add(f"go ({m}) (amd64, arm64)", gate("go") and m in selected, module_feed(m)) + add(f"go ({m}) (amd64, arm64)", gate("go") and m in selected, module_feed(m), + note=go_note(m)) add("go (test/e2e)", flag(scope, "e2e-unit"), module_feed("test/e2e")) add("api (postgres)", "api" in selected, module_feed("api")) - add("web", gate("web"), filtered("web")) + add("web", gate("web"), filtered("web"), note=web_note) add("web e2e (mock)", gate("web"), filtered("web")) add("helm lint", gate("charts"), filtered("charts")) add("chart render", gate("charts"), filtered("charts")) @@ -634,6 +964,7 @@ def e2e_row(suite, gate_on, picked, key, feed): def emit(values): + """Append each value to the GITHUB_OUTPUT file as one KEY=JSON line.""" lines = "".join(f"{key}={json.dumps(value, separators=(',', ':'))}\n" for key, value in values.items()) with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as output: @@ -641,6 +972,7 @@ def emit(values): def bot_matrix(names): + """Return the bot-fast shard matrix {"test": names}, rejecting duplicate or non-Test names.""" if not names or len(names) != len(set(names)) or any( not re.fullmatch(r"Test[A-Za-z0-9_]+", name) for name in names ): @@ -649,9 +981,10 @@ def bot_matrix(names): def main(): + """Run the CLI mode named on the command line and write its GitHub outputs.""" parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("mode", choices=("modules", "images", "dockerfiles", "bots", "summary", - "e2e", "verify-suites")) + "e2e", "verify-suites", "units", "jobs")) parser.add_argument("--base", default="") parser.add_argument("--head", default="HEAD") args = parser.parse_args() @@ -699,26 +1032,50 @@ def main(): if args.mode == "e2e": env = os.environ try: - event_reason = full_scope_reason(env, []) - paths = None if event_reason else changed_paths(ROOT, args.base, args.head) - values = e2e_outputs(ROOT, paths, full_scope_reason(env, paths)) + paths, reason, jobs = scoped(env, args.base, args.head) + values = e2e_outputs(ROOT, paths, reason) + if jobs: + values = widen_e2e(values, e2e_outputs(ROOT, None, "ci.yaml job edit"), jobs) except Exception as exc: # any doubt selects every e2e test (FR-002) print(f"::warning::e2e selection failed, running every e2e test: {exc}") values = e2e_outputs(ROOT, None, f"error: {exc}") emit(values) return + if args.mode == "jobs": + try: + _, reason, jobs = scoped(os.environ, args.base, args.head) + except Exception as exc: # any doubt selects the full suite (FR-002) + print(f"::warning::ci.yaml job mapping failed, running everything: {exc}") + reason, jobs = f"error: {exc}", [] + emit({"all": bool(reason), "reason": reason or "", "ci-jobs": jobs}) + return + if args.mode == "units": + env = os.environ + try: + paths, reason, jobs = scoped(env, args.base, args.head) + modules = workspace_modules(ROOT) + unmapped = [p for p in paths or [] if is_unmapped(p, modules)] + if reason is None and unmapped: + reason = f"unmapped path {unmapped[0]}" + values = widen_units(unit_outputs(ROOT, paths, reason), jobs) + except Exception as exc: # any doubt runs every unit test (FR-002) + print(f"::warning::unit selection failed, running every unit test: {exc}") + values = unit_outputs(ROOT, None, f"error: {exc}") + emit(values) + return # PRs use affected work; every master push, manual run and ci: full-run # label is the full-suite backstop. env = os.environ try: - event_reason = full_scope_reason(env, []) - paths = None if event_reason else changed_paths(ROOT, args.base, args.head) + paths, reason, jobs = scoped(env, args.base, args.head) + if paths is not None and reason and full_scope_reason(env, paths) is None: + paths = None # a ci.yaml edit outside any job or in changes/report (R8) if paths is not None and any(is_unmapped(p, workspace_modules(ROOT)) for p in paths): paths = None # mode e2e reports the path; every module runs (FR-002) selected = affected_modules(ROOT, paths) except Exception as exc: # any doubt selects the full suite (FR-002) print(f"::warning::test selection failed, running everything: {exc}") - paths = None + paths, jobs = None, [] selected = sorted(workspace_modules(ROOT)) modules = sorted(workspace_modules(ROOT)) emit({ @@ -727,8 +1084,11 @@ def main(): "lint": bool(selected), "go": bool(set(selected) - {"test/e2e"}), "e2e-unit": "test/e2e" in selected, - "lint-exclude": [{"module": m} for m in modules if m not in selected], - "go-exclude": [{"module": m} for m in modules if m not in selected and m != "test/e2e"], + # An edit inside the lint or go job of ci.yaml runs that job on every module. + "lint-exclude": [{"module": m} for m in modules + if m not in selected and "lint" not in jobs], + "go-exclude": [{"module": m} for m in modules + if m not in selected and m != "test/e2e" and "go" not in jobs], }) diff --git a/hack/test_check_changed_coverage.py b/hack/test_check_changed_coverage.py new file mode 100644 index 000000000..7cb50d2d0 --- /dev/null +++ b/hack/test_check_changed_coverage.py @@ -0,0 +1,176 @@ +"""Behavioral coverage for the changed-lines coverage gate (run in CI).""" + +import contextlib +import io +import json +import os +import subprocess +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +from check_changed_coverage import check, expand, go_config, main, vitest_config + + +GIT = ["git", "-c", "user.name=test", "-c", "user.email=test@example.com", + "-c", "commit.gpgsign=false", "-c", "core.hooksPath=/dev/null", + # background gc could still be writing .git when the temp dir is removed + "-c", "gc.auto=0", "-c", "maintenance.auto=false"] +COVERAGE_YML = """threshold: + file: 0 + total: 80 + +exclude: + paths: + - ^cmd/ # wiring +""" +VITEST_CONFIG = """export default defineConfig({ + test: { + exclude: ["e2e/**"], + coverage: { + exclude: ["src/main.tsx", "**/*.test.{ts,tsx}"], + thresholds: { + lines: 92, + functions: 76, + }, + }, + }, +}); +""" +GO_FUNC = "package mod\n// added comment\nfunc F() int {\n\treturn 1\n}\n" + + +class ChangedCoverageTests(unittest.TestCase): + """Check added-line coverage against Go profiles and Istanbul JSON.""" + + def setUp(self): + """Create a git repository with one Go module and a web tree.""" + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + self.git("init", "-q") + self.write("mod/go.mod", "module example.com/mod\n") + self.write("mod/.testcoverage.yml", COVERAGE_YML) + self.write("mod/a.go", "package mod\n") + self.write("mod/cmd/main.go", "package main\n") + self.write("web/vitest.config.ts", VITEST_CONFIG) + self.write("web/src/a.ts", "export const a = 1;\n") + self.base = self.commit() + self.head = self.base + + def git(self, *args): + """Run git in the fixture repository and return its trimmed output.""" + return subprocess.run([*GIT, *args], cwd=self.root, check=True, capture_output=True, + text=True).stdout.strip() + + def write(self, name, content): + """Write a fixture file and return its path.""" + path = self.root / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(content, encoding="utf-8") + return path + + def commit(self): + """Commit every fixture change and return the new commit id.""" + self.git("add", "-A") + self.git("commit", "-q", "-m", "change") + return self.git("rev-parse", "HEAD") + + def run_main(self, *args): + """Run the gate and return (exit code, stdout, step summary).""" + summary = self.root / "summary.md" + out = io.StringIO() + with patch.dict(os.environ, {"GITHUB_STEP_SUMMARY": str(summary)}), \ + contextlib.redirect_stdout(out): + code = main(["--root", str(self.root), "--base", self.base, "--head", self.head, + *args]) + text = summary.read_text(encoding="utf-8") if summary.exists() else "" + return code, out.getvalue(), text + + def go_change(self, count): + """Add a comment and a three-line function block; return a profile path.""" + self.write("mod/a.go", GO_FUNC) + self.head = self.commit() + profile = self.write("profile.out", + f"mode: atomic\nexample.com/mod/a.go:3.14,5.2 1 {count}\n") + return str(profile) + + def test_covered_go_lines_pass(self): + code, out, summary = self.run_main("--go-profile", self.go_change(1), "--module", "mod") + self.assertEqual(code, 0) + self.assertIn("3/3 added statement lines covered", out) + self.assertIn("changed-lines coverage", summary) + + def test_uncovered_go_lines_fail_and_are_listed(self): + code, out, _ = self.run_main("--go-profile", self.go_change(0), "--module", "mod") + self.assertEqual(code, 1) + self.assertIn("0/3 added statement lines covered", out) + for number in (3, 4, 5): + self.assertIn(f"uncovered: mod/a.go:{number}", out) + + def test_zero_statement_lines_pass(self): + self.write("mod/a.go", "package mod\n// only a comment\n") + self.head = self.commit() + profile = self.write("profile.out", "mode: atomic\n") + code, out, _ = self.run_main("--go-profile", str(profile), "--module", "mod") + self.assertEqual(code, 0) + self.assertIn("no added statement lines", out) + + def test_go_tests_and_excluded_paths_are_ignored(self): + self.write("mod/cmd/main.go", "package main\nfunc main() {\n}\n") + self.write("mod/a_test.go", "package mod\nfunc helper() {\n}\n") + self.head = self.commit() + profile = self.write("profile.out", "mode: atomic\n" + "example.com/mod/cmd/main.go:2.13,3.2 0 0\n" + "example.com/mod/a_test.go:2.15,3.2 0 0\n") + code, out, _ = self.run_main("--go-profile", str(profile), "--module", "mod") + self.assertEqual(code, 0) + self.assertIn("no added statement lines", out) + + def test_istanbul_json_for_web(self): + self.write("web/src/a.ts", "export const a = 1;\nexport const b = 2;\n") + self.head = self.commit() + source = str(self.root / "web/src/a.ts") + for hits, want in ((0, 1), (1, 0)): + with self.subTest(hits=hits): + report = self.write("web/coverage/coverage-final.json", json.dumps({source: { + "path": source, + "statementMap": {"0": {"start": {"line": 2, "column": 0}, + "end": {"line": 2, "column": 20}}}, + "s": {"0": hits}, + }})) + code, out, _ = self.run_main("--vitest-json", str(report)) + self.assertEqual(code, want, out) + + def test_threshold_comes_from_testcoverage_yml(self): + self.assertEqual(go_config(self.root / "mod"), (80.0, ["^cmd/"])) + + def test_threshold_comes_from_vitest_config(self): + self.assertEqual(vitest_config(self.root / "web"), + (92.0, ["src/main.tsx", "**/*.test.ts", "**/*.test.tsx"])) + + def test_missing_threshold_fails(self): + self.write("mod/.testcoverage.yml", "threshold:\n file: 0\n") + with self.assertRaises(ValueError): + go_config(self.root / "mod") + code, out, _ = self.run_main("--go-profile", self.go_change(1), "--module", "mod") + self.assertEqual(code, 1) + self.assertIn("::error::", out) + self.write("web/vitest.config.ts", "export default defineConfig({ test: {} });\n") + with self.assertRaises(ValueError): + vitest_config(self.root / "web") + + def test_share_at_threshold_passes(self): + added = {"f.go": {1, 2}} + statements = {"f.go": {1: True, 2: False}} + self.assertEqual(check(added, statements, 50, lambda _: False)[0], 0) + self.assertEqual(check(added, statements, 51, lambda _: False)[0], 1) + + def test_brace_expansion(self): + self.assertEqual(expand("**/*.test.{ts,tsx}"), ["**/*.test.ts", "**/*.test.tsx"]) + self.assertEqual(expand("src/main.tsx"), ["src/main.tsx"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/hack/test_ci_scope.py b/hack/test_ci_scope.py index 015257971..c8dd10dc1 100644 --- a/hack/test_ci_scope.py +++ b/hack/test_ci_scope.py @@ -5,13 +5,15 @@ import tempfile import unittest from pathlib import Path +from typing import ClassVar from unittest.mock import patch from subprocess import CompletedProcess from ci_scope import (E2E_BUCKETS, FORCE_ALL_FILES, ROOT, affected_modules, bot_matrix, bucket_lists, changed_paths, e2e_outputs, full_scope_reason, image_inputs, is_unmapped, load_suites, main, publish_paths, select_images, summarize, - verify_suites, + verify_suites, go_packages, web_tests, CI_FILE, ci_blocks, ci_jobs, + widen_e2e, widen_units, workspace_modules) @@ -65,7 +67,7 @@ def test_unrelated_module_stays_isolated(self): def test_shared_inputs_and_dependency_changes_select_every_module(self): for path in ("go.work", "go.work.sum", "api/go.mod", "api/go.sum", - "Makefile", ".golangci.yml", ".github/workflows/ci.yaml", + "Makefile", ".github/actions/go-cache/action.yml", "hack/ci_scope.py", "test/e2e/buckets.sh", "web/Dockerfile"): with self.subTest(path=path): @@ -461,7 +463,7 @@ def test_component_change_leaves_the_universal_gate_off(self): class VerifySuitesTests(unittest.TestCase): """verify-suites rules 1, 2 and 5 from contracts/suite-components.md (T015).""" - MODULES = {"api": "example.com/api", "agent": "example.com/agent"} + MODULES: ClassVar[dict[str, str]] = {"api": "example.com/api", "agent": "example.com/agent"} def suites(self, **overrides): """Return a valid minimal suite map, with top-level keys replaced by overrides.""" @@ -505,7 +507,7 @@ class E2ERunTests(unittest.TestCase): def test_full_scope_regex_equals_buckets_sh_regex(self): values = e2e_outputs(ROOT, None, "master push") self.assertEqual(values["reason"], "master push") - for bucket in E2E_BUCKETS + ["ratelimit"]: + for bucket in [*E2E_BUCKETS, "ratelimit"]: expected = subprocess.run(["bash", "test/e2e/buckets.sh", "regex", bucket], cwd=ROOT, check=True, capture_output=True, text=True).stdout.strip() self.assertEqual(values["e2e-run"][bucket], expected) @@ -555,5 +557,392 @@ def test_modules_mode_selects_every_module_for_an_unmapped_path(self): self.assertEqual(values, FULL_VALUES) +class GoPackageTests(unittest.TestCase): + """Package-level Go selection from import scanning (spec 023 T031, research R4).""" + + def setUp(self): + """Create the shared workspace plus three api packages and one lib package.""" + make_go_workspace(self) + write_file(self.root, "lib/helpers/h.go", "package helpers\n") + write_file(self.root, "api/internal/a/a.go", + 'package a\n\nimport "example.com/lib/helpers"\n\nvar _ = helpers.X\n') + write_file(self.root, "api/internal/a/a_test.go", "package a\n") + write_file(self.root, "api/internal/b/b.go", + 'package b\n\nimport (\n\t"fmt"\n\tx "example.com/api/internal/a"\n)\n') + write_file(self.root, "api/internal/c/c.go", "package c\n") + write_file(self.root, "api/internal/c/testdata/in.txt", "fixture\n") + + def packages(self, *paths): + """Return the go-packages selection for the changed paths.""" + return go_packages(self.root, list(paths), None) + + def test_leaf_change_selects_only_its_package(self): + self.assertEqual(self.packages("api/internal/b/b.go"), + {"api": ["./internal/b"], "lib": [], "other": []}) + + def test_importer_chain_crosses_modules(self): + self.assertEqual(self.packages("lib/helpers/h.go"), + {"api": ["./internal/a", "./internal/b"], "lib": ["./helpers"], + "other": []}) + + def test_test_file_change_selects_only_its_package(self): + self.assertEqual(self.packages("api/internal/a/a_test.go")["api"], ["./internal/a"]) + + def test_testdata_and_embedded_files_select_their_package(self): + self.assertEqual(self.packages("api/internal/c/testdata/in.txt")["api"], + ["./internal/c"]) + self.assertEqual(self.packages("api/internal/c/schema.json")["api"], ["./internal/c"]) + + def test_module_files_select_the_whole_module(self): + for name in ("go.mod", "go.sum", ".testcoverage.yml"): + with self.subTest(name=name): + self.assertEqual(self.packages(f"api/{name}"), + {"api": ["./..."], "lib": [], "other": []}) + + def test_dependency_manifest_selects_importers_across_modules(self): + for name in ("go.mod", "go.sum"): + with self.subTest(name=name): + self.assertEqual(self.packages(f"lib/{name}"), + {"api": ["./internal/a", "./internal/b"], "lib": ["./..."], + "other": []}) + self.assertEqual(self.packages("lib/.testcoverage.yml"), + {"api": [], "lib": ["./..."], "other": []}) + + def test_deleted_package_selects_its_former_importers(self): + (self.root / "lib/helpers/h.go").unlink() + self.assertEqual(self.packages("lib/helpers/h.go"), + {"api": ["./internal/a", "./internal/b"], "lib": ["./..."], + "other": []}) + + def test_file_outside_any_package_selects_the_whole_module(self): + self.assertEqual(self.packages("api/config/x.yaml")["api"], ["./..."]) + + def test_markdown_selects_nothing(self): + self.assertEqual(self.packages("api/README.md")["api"], []) + + def test_full_scope_runs_every_module(self): + self.assertEqual(go_packages(self.root, None, "master push"), + {"api": ["./..."], "lib": ["./..."], "other": ["./..."]}) + + +class WebTestsTests(unittest.TestCase): + """Web unit selection for `vitest related` (spec 023 T032, research R6).""" + + def setUp(self): + """Create a temporary web tree with one route.""" + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + write_file(self.root, "web/src/routes/servers.tsx", "export {};\n") + + def test_route_file_gives_its_source_path(self): + self.assertEqual(web_tests(self.root, ["web/src/routes/servers.tsx", "api/x.go"], None), + ["web/src/routes/servers.tsx"]) + + def test_shared_web_inputs_give_all(self): + for path in ("web/src/test/setup.ts", "web/package.json", "web/package-lock.json", + "web/vitest.config.ts", "web/vite.config.ts", "web/tsconfig.app.json", + "web/tsconfig.json", "web/eslint.config.js", "i18n/locales/en/app.json"): + with self.subTest(path=path): + self.assertEqual(web_tests(self.root, [path], None), "all") + + def test_deleted_source_gives_all(self): + self.assertEqual(web_tests(self.root, ["web/src/routes/gone.tsx"], None), "all") + + def test_playwright_and_unrelated_paths_select_nothing(self): + self.assertEqual(web_tests(self.root, ["web/e2e/servers.spec.ts", "web/README.md", + "docs/a.md"], None), []) + + def test_full_scope_gives_all(self): + self.assertEqual(web_tests(self.root, None, "master push"), "all") + self.assertEqual(web_tests(self.root, ["web/src/routes/servers.tsx"], "manual run"), + "all") + + +class UnitSummaryNoteTests(unittest.TestCase): + """The summary rows name package and file counts from mode units (spec 023 T039).""" + + def setUp(self): + """Create the shared Go workspace fixture for each test.""" + make_go_workspace(self) + + def rows(self, units): + """Return the summary rows of a partial run with go and web gates on.""" + env = { + "GITHUB_EVENT_NAME": "pull_request", + "SCOPE_OUTPUTS": json.dumps({"modules": json.dumps(["api", "lib"])}), + "FILTER_OUTPUTS": json.dumps({}), + "GATE_OUTPUTS": json.dumps({"go": "true", "web": "true"}), + "BOT_MATRIX": json.dumps({"test": []}), + "UNITS_OUTPUTS": json.dumps({key: json.dumps(value) for key, value in units.items()}), + } + with patch("ci_scope.ROOT", self.root): + rows, _ = summarize(env, ["api/internal/a/a.go"], None) + return {row["suite"]: row["reason"] for row in rows} + + def test_notes_name_package_and_file_counts(self): + rows = self.rows({"go-packages": {"api": ["./internal/a", "./internal/b"], "lib": []}, + "web-tests": ["web/src/a.tsx"]}) + self.assertTrue(rows["go (api) (amd64, arm64)"].startswith("2 packages; ")) + self.assertTrue(rows["go (lib) (amd64, arm64)"].startswith("vet only, no affected package; ")) + self.assertTrue(rows["web"].startswith("1 files for vitest related; ")) + + def test_whole_runs_add_no_note(self): + rows = self.rows({"go-packages": {"api": ["./..."], "lib": ["./..."]}, "web-tests": "all"}) + self.assertFalse(rows["go (api) (amd64, arm64)"].startswith(("2 packages", "vet only"))) + self.assertFalse(rows["web"].startswith(("1 files", "lint and build"))) + rows = self.rows({"go-packages": {}, "web-tests": []}) + self.assertTrue(rows["web"].startswith("lint and build only; ")) + + +GIT = ["git", "-c", "user.name=test", "-c", "user.email=test@example.com", + "-c", "commit.gpgsign=false", "-c", "core.hooksPath=/dev/null", + # background gc could still be writing .git when the temp dir is removed + "-c", "gc.auto=0", "-c", "maintenance.auto=false"] +CI_FIXTURE = """name: ci +on: + push: +permissions: + contents: read +concurrency: + group: ci +env: + A: "1" +jobs: + changes: + runs-on: ubuntu-latest + steps: + - run: echo changes + web: + needs: [changes] + steps: + - run: echo web + build-images: + needs: + - changes + steps: + - run: echo build + e2e-go: + needs: [changes, build-images] + steps: + - run: echo e2e + report: + needs: + [ + changes, + web, + ] + steps: + - run: echo report +""" + + +class CiJobsTests(unittest.TestCase): + """ci.yaml lines map to the jobs they sit in (spec 023 T042, research R8, OD-3).""" + + def setUp(self): + """Create a git repository holding the fixture workflow.""" + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + self.git("init", "-q") + write_file(self.root, CI_FILE, CI_FIXTURE) + self.base = self.commit() + + def git(self, *args): + """Run git in the fixture repository and return its trimmed output.""" + return subprocess.run([*GIT, *args], cwd=self.root, check=True, capture_output=True, + text=True).stdout.strip() + + def commit(self): + """Commit every change and return the new commit id.""" + self.git("add", "-A") + self.git("commit", "-q", "--allow-empty", "-m", "change") + return self.git("rev-parse", "HEAD") + + def edit(self, old, new): + """Apply one edit to the base workflow and return ci_jobs for it.""" + self.git("reset", "-q", "--hard", self.base) + text = (self.root / CI_FILE).read_text(encoding="utf-8") + self.assertIn(old, text) + write_file(self.root, CI_FILE, text.replace(old, new, 1)) + return ci_jobs(self.root, self.base, self.commit(), [CI_FILE]) + + def test_needs_are_parsed_in_every_form(self): + self.assertEqual(ci_blocks(CI_FIXTURE)[1], { + "changes": set(), "web": {"changes"}, "build-images": {"changes"}, + "e2e-go": {"changes", "build-images"}, "report": {"changes", "web"}}) + + def test_comment_or_blank_line_selects_only_workflow_lint(self): + for new in (" # note\n - run: echo web\n", "\n - run: echo web\n"): + with self.subTest(new=new): + self.assertEqual(self.edit(" - run: echo web\n", new), + (["workflow-lint"], False)) + + def test_step_change_selects_its_job(self): + self.assertEqual(self.edit("echo web\n", "echo web2\n"), (["web", "workflow-lint"], False)) + + def test_selected_job_brings_the_jobs_it_needs(self): + self.assertEqual(self.edit("echo e2e\n", "echo e2e2\n"), + (["build-images", "e2e-go", "workflow-lint"], False)) + + def test_removed_line_maps_against_the_base_file(self): + self.assertEqual(self.edit(" - run: echo build\n", ""), + (["build-images", "workflow-lint"], False)) + + def test_top_level_and_selection_job_edits_force_full_scope(self): + for old, new in ((" push:\n", " push:\n workflow_dispatch:\n"), + ("contents: read", "contents: write"), ("group: ci", "group: ci2"), + ('A: "1"', 'A: "2"'), ("echo changes", "echo changes2"), + ("echo report", "echo report2")): + with self.subTest(old=old): + self.assertTrue(self.edit(old, new)[1]) + + def test_golangci_selects_lint_legs_only(self): + self.assertEqual(ci_jobs(self.root, self.base, self.base, [".golangci.yml"]), + (["lint", "workflow-lint"], False)) + self.assertIsNone(full_scope_reason({}, [".golangci.yml"])) + self.assertIsNone(full_scope_reason({}, [CI_FILE])) + + def test_other_workflow_files_select_workflow_lint_only(self): + for path in (".github/workflows/publish-edge.yaml", ".github/zizmor.yml", + ".github/dependabot.yml"): + with self.subTest(path=path): + self.assertEqual(ci_jobs(self.root, self.base, self.base, [path]), + (["workflow-lint"], False)) + + def test_selection_inputs_still_force_full_scope(self): + for path in ("hack/ci_scope.py", "hack/test_ci_scope.py", "Makefile", "go.work", + "go.work.sum", ".github/actions/go-cache/action.yml", + "test/e2e/buckets.sh", "test/e2e/suite-components.json", + "hack/check_changed_coverage.py"): + with self.subTest(path=path): + self.assertEqual(full_scope_reason({}, [path]), "selection-rule change") + + +CI_ARCH_FIXTURE = """name: ci +jobs: + changes: + steps: + - run: echo changes + build-images: + needs: [changes] + steps: + - run: echo build + build-images-arm64: + needs: [changes] + steps: + - run: echo build arm64 + e2e-go: + needs: [changes, build-images] + steps: + - run: echo e2e + e2e-go-arm64: + needs: [changes, build-images-arm64] + steps: + - run: echo e2e arm64 +""" + + +class CiJobsArchitectureTests(unittest.TestCase): + """An edited e2e job brings its other-architecture twin (spec 023 FR-010).""" + + def setUp(self): + """Create a git repository holding a workflow with paired e2e jobs.""" + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + self.git("init", "-q") + write_file(self.root, CI_FILE, CI_ARCH_FIXTURE) + self.base = self.commit() + + def git(self, *args): + """Run git in the fixture repository and return its trimmed output.""" + return subprocess.run([*GIT, *args], cwd=self.root, check=True, capture_output=True, + text=True).stdout.strip() + + def commit(self): + """Commit every change and return the new commit id.""" + self.git("add", "-A") + self.git("commit", "-q", "--allow-empty", "-m", "change") + return self.git("rev-parse", "HEAD") + + def edit(self, old, new): + """Apply one edit to the base workflow and return ci_jobs for it.""" + self.git("reset", "-q", "--hard", self.base) + text = (self.root / CI_FILE).read_text(encoding="utf-8") + self.assertIn(old, text) + write_file(self.root, CI_FILE, text.replace(old, new, 1)) + return ci_jobs(self.root, self.base, self.commit(), [CI_FILE]) + + def test_either_architecture_edit_selects_both(self): + both = (["build-images", "build-images-arm64", "e2e-go", "e2e-go-arm64", + "workflow-lint"], False) + self.assertEqual(self.edit("echo e2e\n", "echo e2e now\n"), both) + self.assertEqual(self.edit("echo e2e arm64\n", "echo e2e arm64 now\n"), both) + + def test_job_without_twin_stays_alone(self): + self.assertEqual(self.edit("echo build\n", "echo build now\n"), + (["build-images", "workflow-lint"], False)) + + +class CiJobsModuleTests(unittest.TestCase): + """Shared CI config no longer selects every Go module (spec 023 T044).""" + + def setUp(self): + """Create the shared Go workspace fixture for each test.""" + make_go_workspace(self) + + def test_ci_yaml_and_golangci_select_no_module(self): + for path in (CI_FILE, ".golangci.yml", ".github/workflows/publish-edge.yaml"): + with self.subTest(path=path): + self.assertEqual(affected_modules(self.root, [path]), []) + + +class WidenTests(unittest.TestCase): + """An edited job widens its own selection (spec 023 T045).""" + + def test_widen_e2e(self): + values = {"e2e-run": {"operator": "^(TestA)$"}, "e2e-upgrade": False, + "e2e-web-live": False, "bot-matrix": {"test": []}, "bots-any": False} + full = {"e2e-run": {"operator": "^(TestA|TestB)$", "api-auth": "^(TestC)$"}, + "bot-matrix": {"test": ["TestBot"]}, "bots-any": True} + widened = widen_e2e(dict(values), full, ["e2e-go-arm64", "e2e-upgrade", "e2e-game-bot"]) + self.assertEqual(widened["e2e-run"], full["e2e-run"]) + self.assertTrue(widened["e2e-upgrade"]) + self.assertFalse(widened["e2e-web-live"]) + self.assertEqual((widened["bot-matrix"], widened["bots-any"]), ({"test": ["TestBot"]}, True)) + self.assertEqual(widen_e2e(dict(values), full, ["web"]), values) + + def test_widen_units(self): + values = {"go-packages": {"api": ["./internal/a"], "lib": []}, "web-tests": []} + self.assertEqual(widen_units(dict(values), ["go", "web"]), + {"go-packages": {"api": ["./..."], "lib": ["./..."]}, "web-tests": "all"}) + self.assertEqual(widen_units(dict(values), ["helm"]), values) + + +class EditedJobSummaryTests(unittest.TestCase): + """The summary marks rows run by an edited ci.yaml job (spec 023 T045).""" + + def setUp(self): + """Create the shared Go workspace fixture for each test.""" + make_go_workspace(self) + + def test_edited_job_row_runs(self): + env = { + "GITHUB_EVENT_NAME": "pull_request", + "SCOPE_OUTPUTS": json.dumps({}), "FILTER_OUTPUTS": json.dumps({}), + "GATE_OUTPUTS": json.dumps({}), "BOT_MATRIX": json.dumps({"test": []}), + "CI_JOBS": json.dumps(["web", "workflow-lint"]), + } + with patch("ci_scope.ROOT", self.root): + rows, _ = summarize(env, [CI_FILE], None) + rows = {row["suite"]: row for row in rows} + self.assertEqual(rows["web"]["decision"], "run") + self.assertEqual(rows["web"]["reason"], "ci.yaml: lines in this job changed") + self.assertEqual(rows["workflow lint"]["decision"], "run") + self.assertEqual(rows["web e2e (mock)"]["decision"], "skip") + + if __name__ == "__main__": unittest.main() diff --git a/specs/023-change-based-test-selection/briefs/ci-line-mapping.md b/specs/023-change-based-test-selection/briefs/ci-line-mapping.md new file mode 100644 index 000000000..1e851b188 --- /dev/null +++ b/specs/023-change-based-test-selection/briefs/ci-line-mapping.md @@ -0,0 +1,728 @@ +# Brief: PR 4, shared CI configuration mapped per job (T042–T046) + +Branch `claude/project-thread-63yei6-pr4`, stacked on PR 3. Facts below were checked +once by the scout; fix agents apply them blind. Every edit is exact: find the quoted +"old" text and replace it with "new". Do not touch any other line. Never edit an +existing test, except section 4b, which needs valgul's sign-off first. + +Scout facts: + +- In `ci.yaml`, job keys are the two-space keys under `jobs:`. `needs:` appears as + ` needs: [a, b]`, as a `- a` list, or as a multi-line `[ … ]` flow list (job + `report`). +- `ci.yaml` and `.golangci.yml` force everything today in three places: + `FORCE_ALL_FILES`; the `shared` set plus the `.github/` prefix in `affected_modules`; + and the paths-filter `ci` list. +- The `lint` job reads `lint-exclude`, `go` reads `go-exclude` and `GO_PACKAGES`, + `web` reads `web-tests`, the `e2e-go*` jobs read `e2e-run`, and `e2e-game-bot` reads + `e2e-bot-matrix`. A ci.yaml edit inside one of those jobs must therefore widen that + job's own selection. A job-level `if:` is not enough on its own. + +## 1. `hack/ci_scope.py` + +### 1a. `FORCE_ALL_FILES` + +old: +```python +FORCE_ALL_FILES = { + ".github/workflows/ci.yaml", "hack/ci_scope.py", "hack/test_ci_scope.py", "Makefile", + "go.work", "go.work.sum", "test/e2e/buckets.sh", ".golangci.yml", + "hack/check_changed_coverage.py", "hack/test_check_changed_coverage.py", +} +``` +new: +```python +FORCE_ALL_FILES = { + "hack/ci_scope.py", "hack/test_ci_scope.py", "Makefile", "go.work", "go.work.sum", + "test/e2e/buckets.sh", "test/e2e/suite-components.json", + "hack/check_changed_coverage.py", "hack/test_check_changed_coverage.py", +} +``` + +### 1b. `NO_E2E_FILES`: add `.golangci.yml` (otherwise it is unmapped) + +old: +```python + "design.pen", "LICENSE", ".claudeignore", ".coderabbit.yaml", ".editorconfig", +``` +new: +```python + "design.pen", "LICENSE", ".claudeignore", ".coderabbit.yaml", ".editorconfig", ".golangci.yml", +``` + +### 1c. Constants: insert after the `WEB_ALL_FILES = {...}` statement (two lines) + +```python +CI_FILE = ".github/workflows/ci.yaml" +# ci.yaml jobs that compute or report the selection; an edit there forces full scope (R8). +CI_SELECTION_JOBS = {"changes", "report"} +CI_JOB_KEY = re.compile(r"^ ([A-Za-z0-9_-]+):\s*(?:#.*)?$") +CI_TOP_KEY = re.compile(r"^[A-Za-z_][\w-]*:") +# e2e jobs whose own switch output a ci.yaml edit inside them turns on (mode e2e). +E2E_JOB_SWITCHES = { + "e2e-multicluster": "e2e-multicluster", "e2e-multicluster-arm64": "e2e-multicluster", + "e2e-upgrade": "e2e-upgrade", "e2e-upgrade-arm64": "e2e-upgrade", + "e2e-web-live": "e2e-web-live", "e2e-web-live-arm64": "e2e-web-live", + "e2e-capture-overhead": "e2e-capture-overhead", +} +# Summary row prefixes and the ci.yaml jobs behind them; the first match wins. +ROW_JOBS = ( + ("lint (", ("lint",)), + ("go (test/e2e)", ("go-e2e-unit",)), + ("go (", ("go",)), + ("api (postgres)", ("api-postgres",)), + ("web e2e (mock)", ("web-e2e-mock",)), + ("web", ("web",)), + ("helm lint", ("helm",)), + ("chart render", ("chart-template",)), + ("standalone Compose smoke", ("standalone-compose",)), + ("workflow lint", ("workflow-lint",)), + ("docs and repository checks", ("docs",)), + ("codegen drift", ("codegen-drift",)), + ("submodule freshness", ("submodule-freshness",)), + ("vuln (go)", ("vuln-go",)), + ("capture-sidecar CAP_NET_RAW survival", ("capture-sidecar-setcap-proof",)), + ("dependency review", ("dependency-review",)), + ("telemetry default gate", ("telemetry-default-gate",)), + ("build e2e images", ("build-images", "build-images-arm64")), + ("vuln (images)", ("vuln-images",)), + ("e2e bucket coverage", ("e2e-buckets",)), + ("e2e multicluster", ("e2e-multicluster", "e2e-multicluster-arm64")), + ("e2e upgrade", ("e2e-upgrade", "e2e-upgrade-arm64")), + ("e2e postgres", ("e2e-postgres",)), + ("e2e game bot", ("e2e-game-bot",)), + ("e2e web live", ("e2e-web-live", "e2e-web-live-arm64")), + ("e2e capture overhead", ("e2e-capture-overhead",)), + ("ingress-nginx smoke", ("e2e-kind-ingress-smoke",)), + ("e2e ", ("e2e-go", "e2e-go-arm64")), +) +``` + +### 1d. `affected_modules` + +old: +```python + shared = {"Makefile", "go.work", "go.work.sum", ".golangci.yml", "test/e2e/buckets.sh"} + if paths is None or any( + p in shared or p.startswith((".github/", "hack/ci_scope", "hack/test_ci_scope", +``` +new: +```python + # ci.yaml and .golangci.yml map to their own jobs through ci_jobs() (OD-3). + shared = {"Makefile", "go.work", "go.work.sum", "test/e2e/buckets.sh"} + if paths is None or any( + p in shared or p.startswith((".github/actions/", "hack/ci_scope", "hack/test_ci_scope", +``` + +### 1e. New functions: insert right before `def verify_suites(suites, buckets, modules):` + +```python +def ci_blocks(text): + """Return ({line number: job id or None}, {job id: needed job ids}) for a workflow.""" + owner, needs = {}, {} + job, in_jobs, collecting = None, False, None + for number, line in enumerate(text.splitlines(), 1): + if CI_TOP_KEY.match(line): + in_jobs, job, collecting = line.split(":", 1)[0] == "jobs", None, None + elif in_jobs and CI_JOB_KEY.match(line): + job, collecting = CI_JOB_KEY.match(line)[1], None + needs.setdefault(job, set()) + elif job and line.startswith(" needs:"): + collecting = job + line = line[len(" needs:"):] + elif re.match(r"^ \S", line): + collecting = None + owner[number] = job + if collecting: + needs[collecting].update(re.findall(r"[A-Za-z0-9_-]+", line.split("#", 1)[0])) + return owner, needs + + +def changed_ci_lines(root, base, head): + """Return ([(old line, text)], [(new line, text)]) for ci.yaml lines changed base..head.""" + diff = subprocess.run(["git", "diff", "-U0", "--no-color", base, head, "--", CI_FILE], + cwd=root, check=True, capture_output=True, text=True).stdout + removed, added, old, new, started = [], [], 0, 0, False + for line in diff.splitlines(): + hunk = re.match(r"^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@", line) + if hunk: + old, new, started = int(hunk[1]), int(hunk[2]), True + elif started and line.startswith("-"): + removed.append((old, line[1:])) + old += 1 + elif started and line.startswith("+"): + added.append((new, line[1:])) + new += 1 + return removed, added + + +def git_text(root, revision, path): + """Return a file's text at revision, or "" when it does not exist there.""" + result = subprocess.run(["git", "show", f"{revision}:{path}"], cwd=root, + capture_output=True, text=True, check=False) + return result.stdout if result.returncode == 0 else "" + + +def ci_jobs(root, base, head, paths): + """Map shared CI file changes to ci.yaml job ids (research R8, OD-3). + + Returns (jobs, forced). jobs holds every job whose ci.yaml lines changed, `lint` for + .golangci.yml, `workflow-lint` for any .github/ or .golangci.yml change, and every job + they need. forced is True when a changed ci.yaml line that is not blank or a comment + sits outside any job or inside the changes or report job. + """ + jobs, forced, needs = set(), False, {} + if any(p.startswith(".github/") or p == ".golangci.yml" for p in paths): + jobs.add("workflow-lint") + if ".golangci.yml" in paths: + jobs.add("lint") + if CI_FILE in paths: + removed, added = changed_ci_lines(root, base, head) + for revision, changes in ((base, removed), (head, added)): + owner, found = ci_blocks(git_text(root, revision, CI_FILE)) + needs.update(found) + for number, text in changes: + if not text.strip() or text.strip().startswith("#"): + continue + job = owner.get(number) + if job is None or job in CI_SELECTION_JOBS: + forced = True + else: + jobs.add(job) + queue = list(jobs) + while queue: + for name in needs.get(queue.pop(), ()): + if name not in jobs: + jobs.add(name) + queue.append(name) + return sorted(jobs - CI_SELECTION_JOBS), forced + + +def scoped(env, base, head): + """Return (paths, reason, ci.yaml jobs): the diff, why everything runs, edited jobs.""" + event_reason = full_scope_reason(env, []) + paths = None if event_reason else changed_paths(ROOT, base, head) + reason = full_scope_reason(env, paths) + if reason: + return paths, reason, [] + jobs, forced = ci_jobs(ROOT, base, head, paths) + return paths, ("selection-rule change" if forced else None), jobs + + +def widen_e2e(values, full, jobs): + """Run every test of each e2e job whose ci.yaml block changed (research R8).""" + if {"e2e-go", "e2e-go-arm64"} & set(jobs): + values["e2e-run"] = full["e2e-run"] + for job, key in E2E_JOB_SWITCHES.items(): + if job in jobs: + values[key] = True + if "e2e-game-bot" in jobs: + values["bot-matrix"], values["bots-any"] = full["bot-matrix"], full["bots-any"] + return values + + +def widen_units(values, jobs): + """Run every package or web test of a go or web job whose ci.yaml block changed.""" + if "go" in jobs: + values["go-packages"] = {m: ["./..."] for m in values["go-packages"]} + if "web" in jobs: + values["web-tests"] = "all" + return values +``` + +### 1f. `summarize`: rows turned on by an edited ci.yaml job + +old: +```python + units = load_json(env, "UNITS_OUTPUTS") +``` +new: +```python + units = load_json(env, "UNITS_OUTPUTS") + edited_jobs = set(jlist(env, "CI_JOBS")) +``` + +old: +```python + def add(suite, run, feed=(), run_reason="", skip_reason="not affected", note=""): + """Append one selection row with a run or skip decision and its reason.""" +``` +new: +```python + def add(suite, run, feed=(), run_reason="", skip_reason="not affected", note=""): + """Append one selection row with a run or skip decision and its reason.""" + jobs = next((found for prefix, found in ROW_JOBS if suite.startswith(prefix)), ()) + if not run and edited_jobs & set(jobs): + run, run_reason = True, "ci.yaml: lines in this job changed" +``` + +### 1g. `main` + +Mode choices. old: +```python + "e2e", "verify-suites", "units")) +``` +new: +```python + "e2e", "verify-suites", "units", "jobs")) +``` + +Mode e2e. old: +```python + event_reason = full_scope_reason(env, []) + paths = None if event_reason else changed_paths(ROOT, args.base, args.head) + values = e2e_outputs(ROOT, paths, full_scope_reason(env, paths)) +``` +new: +```python + paths, reason, jobs = scoped(env, args.base, args.head) + values = e2e_outputs(ROOT, paths, reason) + if jobs: + values = widen_e2e(values, e2e_outputs(ROOT, None, "ci.yaml job edit"), jobs) +``` + +Mode units. old: +```python + event_reason = full_scope_reason(env, []) + paths = None if event_reason else changed_paths(ROOT, args.base, args.head) + reason = full_scope_reason(env, paths) + modules = workspace_modules(ROOT) + unmapped = [p for p in paths or [] if is_unmapped(p, modules)] + if reason is None and unmapped: + reason = f"unmapped path {unmapped[0]}" + values = unit_outputs(ROOT, paths, reason) +``` +new: +```python + paths, reason, jobs = scoped(env, args.base, args.head) + modules = workspace_modules(ROOT) + unmapped = [p for p in paths or [] if is_unmapped(p, modules)] + if reason is None and unmapped: + reason = f"unmapped path {unmapped[0]}" + values = widen_units(unit_outputs(ROOT, paths, reason), jobs) +``` + +New mode `jobs`: insert right before the line ` if args.mode == "units":` +```python + if args.mode == "jobs": + try: + _, reason, jobs = scoped(os.environ, args.base, args.head) + except Exception as exc: # any doubt selects the full suite (FR-002) + print(f"::warning::ci.yaml job mapping failed, running everything: {exc}") + reason, jobs = f"error: {exc}", [] + emit({"all": bool(reason), "reason": reason or "", "ci-jobs": jobs}) + return +``` + +Mode modules. old: +```python + event_reason = full_scope_reason(env, []) + paths = None if event_reason else changed_paths(ROOT, args.base, args.head) + if paths is not None and any(is_unmapped(p, workspace_modules(ROOT)) for p in paths): + paths = None # mode e2e reports the path; every module runs (FR-002) + selected = affected_modules(ROOT, paths) + except Exception as exc: # any doubt selects the full suite (FR-002) + print(f"::warning::test selection failed, running everything: {exc}") + paths = None + selected = sorted(workspace_modules(ROOT)) +``` +new: +```python + paths, reason, jobs = scoped(env, args.base, args.head) + if paths is not None and reason and full_scope_reason(env, paths) is None: + paths = None # a ci.yaml edit outside any job or in changes/report (R8) + if paths is not None and any(is_unmapped(p, workspace_modules(ROOT)) for p in paths): + paths = None # mode e2e reports the path; every module runs (FR-002) + selected = affected_modules(ROOT, paths) + except Exception as exc: # any doubt selects the full suite (FR-002) + print(f"::warning::test selection failed, running everything: {exc}") + paths, jobs = None, [] + selected = sorted(workspace_modules(ROOT)) +``` +old: +```python + "lint-exclude": [{"module": m} for m in modules if m not in selected], + "go-exclude": [{"module": m} for m in modules if m not in selected and m != "test/e2e"], +``` +new: +```python + # An edit inside the lint or go job of ci.yaml runs that job on every module. + "lint-exclude": [{"module": m} for m in modules + if m not in selected and "lint" not in jobs], + "go-exclude": [{"module": m} for m in modules + if m not in selected and m != "test/e2e" and "go" not in jobs], +``` + +## 2. `.github/workflows/ci.yaml` + +2a. `changes` job outputs. old: +```yaml + web-tests: ${{ steps.units.outputs.web-tests }} +``` +new: +```yaml + web-tests: ${{ steps.units.outputs.web-tests }} + ci-jobs: ${{ steps.jobs.outputs.ci-jobs }} +``` + +2b. New step. Insert before ` - uses: dorny/paths-filter@` (right after the `units` step): +```yaml + - name: map edited ci.yaml lines to their jobs + id: jobs + # A ci.yaml edit runs only the jobs it sits in, plus the jobs they need; + # an edit outside any job or in changes/report sets `all` (spec 023 US3, OD-3). + env: + BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.before }} + HEAD_SHA: ${{ github.sha }} + CI_FULL_RUN: "${{ contains(github.event.pull_request.labels.*.name, 'ci: full-run') }}" + run: python3 hack/ci_scope.py jobs --base "$BASE_SHA" --head "$HEAD_SHA" +``` + +2c. paths-filter `ci` list. old: +```yaml + ci: + - '.github/workflows/ci.yaml' + - '.github/actions/**' +``` +new: +```yaml + ci: + # ci.yaml and .golangci.yml map to their own jobs (step `jobs`, OD-3). + - '.github/actions/**' +``` +old: +```yaml + - 'test/e2e/buckets.sh' + # golangci-lint walks up from `working-directory` to find this + # config, so every `lint (…)` leg picks up an edit here even + # though none of them declare it as an input. Force the lint + # job on so a config change is tested against real code + # instead of silently taking effect on the next unrelated PR. + - '.golangci.yml' + github: +``` +new: +```yaml + - 'test/e2e/buckets.sh' + - 'test/e2e/suite-components.json' + github: +``` + +2d. combine. old: +```yaml + CI: ${{ steps.filter.outputs.ci == 'true' || steps.scope.outputs.all == 'true' || steps.e2e.outputs.all == 'true' }} +``` +new: +```yaml + CI: ${{ steps.filter.outputs.ci == 'true' || steps.scope.outputs.all == 'true' || steps.e2e.outputs.all == 'true' || steps.jobs.outputs.all == 'true' }} +``` + +2e. Summary env. old: +```yaml + UNITS_OUTPUTS: ${{ toJSON(steps.units.outputs) }} +``` +new: +```yaml + UNITS_OUTPUTS: ${{ toJSON(steps.units.outputs) }} + CI_JOBS: ${{ steps.jobs.outputs.ci-jobs }} +``` + +2f. Job gates (T045): run this script once from the repo root. It is a +deterministic rewrite: one `if:` line per listed job. +```sh +python3 - <<'EOF' +import re +from pathlib import Path +path = Path(".github/workflows/ci.yaml") +lines = path.read_text(encoding="utf-8").split("\n") +jobs = ["build-images", "build-images-arm64", "capture-sidecar-setcap-proof", "vuln-images", + "docs", "codegen-drift", "submodule-freshness", "vuln-go", "lint", "go", "web", + "web-e2e-mock", "helm", "standalone-compose", "chart-template", "workflow-lint", + "go-e2e-unit", "api-postgres", "e2e-buckets", "e2e-go", "e2e-go-arm64", + "e2e-multicluster", "e2e-multicluster-arm64", "e2e-upgrade", "e2e-upgrade-arm64", + "e2e-web-live", "e2e-web-live-arm64", "e2e-game-bot", "e2e-postgres", + "e2e-kind-ingress-smoke"] +done, job, in_jobs = set(), None, False +for i, line in enumerate(lines): + if re.match(r"^[A-Za-z_][\w-]*:", line): + in_jobs, job = line.startswith("jobs:"), None + elif in_jobs and re.match(r"^ ([A-Za-z0-9_-]+):\s*$", line): + job = line.strip()[:-1] + elif job in jobs and job not in done and line.startswith(" if: "): + expr = line[len(" if: "):] + assert not expr.startswith(">"), job + lines[i] = (f" if: ({expr}) || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), " + f"'{job}')") + done.add(job) +missing = set(jobs) - done +assert not missing, missing +path.write_text("\n".join(lines), encoding="utf-8") +print("gated", len(done)) +EOF +``` +Expected output: `gated 30`. + +2g. `e2e-capture-overhead` (folded `if: >-`, edited by hand). old: +```yaml + (github.event_name == 'workflow_dispatch' && needs.changes.outputs.e2e == 'true') + runs-on: ubuntu-latest + timeout-minutes: 45 +``` +new: +```yaml + (github.event_name == 'workflow_dispatch' && needs.changes.outputs.e2e == 'true') || + contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'e2e-capture-overhead') + runs-on: ubuntu-latest + timeout-minutes: 45 +``` + +## 3. Specs + +3a. `specs/023-change-based-test-selection/contracts/ci-scope-cli.md`. old: +```markdown +| `ci-jobs` | JSON list of job ids selected by the `ci.yaml` line mapper | Folded into each job's gate | +``` +new: +```markdown +| `ci-jobs` | JSON list of job ids selected by the `ci.yaml` line mapper | Emitted by mode `jobs`. Folded into each job's gate | +``` +Insert before the line `## Mode \`bots\` — changed`: +```markdown +## Mode `jobs` (new) + +Maps shared CI file changes to `ci.yaml` job ids (research R8, settled OD-3). The `jobs` +step of the `changes` job runs right after `units`. Outputs: `all` (boolean), +`reason` (`selection-rule change` when a changed line sits outside any job or inside +`changes` or `report`, else empty), and `ci-jobs`. + +`ci-jobs` lists every job whose block holds a changed line that is neither blank nor a +comment, plus every job those jobs need. `.golangci.yml` adds `lint`, and any `.github/` +or `.golangci.yml` change adds `workflow-lint`. Lines are read from `git diff -U0`: +removed lines map against the base file and added lines against the head file. Each +listed job's gate is ORed with `contains(fromJSON(needs.changes.outputs.ci-jobs), '')`. +Modes `modules`, `units` and `e2e` widen the job's own selection, so the job runs all of +its legs, packages or tests. Still forcing everything (FR-014): `hack/ci_scope.py`, +`hack/test_ci_scope.py`, `hack/check_changed_coverage.py` and its test, `Makefile`, +`go.work`, `go.work.sum`, `.github/actions/**`, `test/e2e/buckets.sh`, +`test/e2e/suite-components.json`. + +``` + +3b. `specs/023-change-based-test-selection/tasks.md`: change `- [ ] T042` … `- [ ] T046` +(five lines) to `- [X]`. + +## 4. Tests: `hack/test_ci_scope.py` + +### 4a. New tests (no existing test changes) + +Import: old +```python + verify_suites, go_packages, web_tests, +``` +new +```python + verify_suites, go_packages, web_tests, CI_FILE, ci_blocks, ci_jobs, + widen_e2e, widen_units, +``` + +Insert right before `if __name__ == "__main__":`: + +```python +GIT = ["git", "-c", "user.name=test", "-c", "user.email=test@example.com", + "-c", "commit.gpgsign=false", "-c", "core.hooksPath=/dev/null"] +CI_FIXTURE = """name: ci +on: + push: +permissions: + contents: read +concurrency: + group: ci +env: + A: "1" +jobs: + changes: + runs-on: ubuntu-latest + steps: + - run: echo changes + web: + needs: [changes] + steps: + - run: echo web + build-images: + needs: + - changes + steps: + - run: echo build + e2e-go: + needs: [changes, build-images] + steps: + - run: echo e2e + report: + needs: + [ + changes, + web, + ] + steps: + - run: echo report +""" + + +class CiJobsTests(unittest.TestCase): + """ci.yaml lines map to the jobs they sit in (spec 023 T042, research R8, OD-3).""" + + def setUp(self): + """Create a git repository holding the fixture workflow.""" + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + self.git("init", "-q") + write_file(self.root, CI_FILE, CI_FIXTURE) + self.base = self.commit() + + def git(self, *args): + """Run git in the fixture repository and return its trimmed output.""" + return subprocess.run([*GIT, *args], cwd=self.root, check=True, capture_output=True, + text=True).stdout.strip() + + def commit(self): + """Commit every change and return the new commit id.""" + self.git("add", "-A") + self.git("commit", "-q", "--allow-empty", "-m", "change") + return self.git("rev-parse", "HEAD") + + def edit(self, old, new): + """Apply one edit to the base workflow and return ci_jobs for it.""" + self.git("reset", "-q", "--hard", self.base) + text = (self.root / CI_FILE).read_text(encoding="utf-8") + self.assertIn(old, text) + write_file(self.root, CI_FILE, text.replace(old, new, 1)) + return ci_jobs(self.root, self.base, self.commit(), [CI_FILE]) + + def test_needs_are_parsed_in_every_form(self): + self.assertEqual(ci_blocks(CI_FIXTURE)[1], { + "changes": set(), "web": {"changes"}, "build-images": {"changes"}, + "e2e-go": {"changes", "build-images"}, "report": {"changes", "web"}}) + + def test_comment_or_blank_line_selects_only_workflow_lint(self): + for new in (" # note\n - run: echo web\n", "\n - run: echo web\n"): + with self.subTest(new=new): + self.assertEqual(self.edit(" - run: echo web\n", new), + (["workflow-lint"], False)) + + def test_step_change_selects_its_job(self): + self.assertEqual(self.edit("echo web\n", "echo web2\n"), (["web", "workflow-lint"], False)) + + def test_selected_job_brings_the_jobs_it_needs(self): + self.assertEqual(self.edit("echo e2e\n", "echo e2e2\n"), + (["build-images", "e2e-go", "workflow-lint"], False)) + + def test_removed_line_maps_against_the_base_file(self): + self.assertEqual(self.edit(" - run: echo build\n", ""), + (["build-images", "workflow-lint"], False)) + + def test_top_level_and_selection_job_edits_force_full_scope(self): + for old, new in ((" push:\n", " push:\n workflow_dispatch:\n"), + ("contents: read", "contents: write"), ("group: ci", "group: ci2"), + ('A: "1"', 'A: "2"'), ("echo changes", "echo changes2"), + ("echo report", "echo report2")): + with self.subTest(old=old): + self.assertTrue(self.edit(old, new)[1]) + + def test_golangci_selects_lint_legs_only(self): + self.assertEqual(ci_jobs(self.root, self.base, self.base, [".golangci.yml"]), + (["lint", "workflow-lint"], False)) + self.assertIsNone(full_scope_reason({}, [".golangci.yml"])) + self.assertIsNone(full_scope_reason({}, [CI_FILE])) + + def test_other_workflow_files_select_workflow_lint_only(self): + for path in (".github/workflows/publish-edge.yaml", ".github/zizmor.yml", + ".github/dependabot.yml"): + with self.subTest(path=path): + self.assertEqual(ci_jobs(self.root, self.base, self.base, [path]), + (["workflow-lint"], False)) + + def test_selection_inputs_still_force_full_scope(self): + for path in ("hack/ci_scope.py", "hack/test_ci_scope.py", "Makefile", "go.work", + "go.work.sum", ".github/actions/go-cache/action.yml", + "test/e2e/buckets.sh", "test/e2e/suite-components.json", + "hack/check_changed_coverage.py"): + with self.subTest(path=path): + self.assertEqual(full_scope_reason({}, [path]), "selection-rule change") + + +class CiJobsModuleTests(unittest.TestCase): + """Shared CI config no longer selects every Go module (spec 023 T044).""" + + def setUp(self): + """Create the shared Go workspace fixture for each test.""" + make_go_workspace(self) + + def test_ci_yaml_and_golangci_select_no_module(self): + for path in (CI_FILE, ".golangci.yml", ".github/workflows/publish-edge.yaml"): + with self.subTest(path=path): + self.assertEqual(affected_modules(self.root, [path]), []) + + +class WidenTests(unittest.TestCase): + """An edited job widens its own selection (spec 023 T045).""" + + def test_widen_e2e(self): + values = {"e2e-run": {"operator": "^(TestA)$"}, "e2e-upgrade": False, + "e2e-web-live": False, "bot-matrix": {"test": []}, "bots-any": False} + full = {"e2e-run": {"operator": "^(TestA|TestB)$", "api-auth": "^(TestC)$"}, + "bot-matrix": {"test": ["TestBot"]}, "bots-any": True} + widened = widen_e2e(dict(values), full, ["e2e-go-arm64", "e2e-upgrade", "e2e-game-bot"]) + self.assertEqual(widened["e2e-run"], full["e2e-run"]) + self.assertTrue(widened["e2e-upgrade"]) + self.assertFalse(widened["e2e-web-live"]) + self.assertEqual((widened["bot-matrix"], widened["bots-any"]), ({"test": ["TestBot"]}, True)) + self.assertEqual(widen_e2e(dict(values), full, ["web"]), values) + + def test_widen_units(self): + values = {"go-packages": {"api": ["./internal/a"], "lib": []}, "web-tests": []} + self.assertEqual(widen_units(dict(values), ["go", "web"]), + {"go-packages": {"api": ["./..."], "lib": ["./..."]}, "web-tests": "all"}) + self.assertEqual(widen_units(dict(values), ["helm"]), values) + + +class EditedJobSummaryTests(unittest.TestCase): + """The summary marks rows run by an edited ci.yaml job (spec 023 T045).""" + + def setUp(self): + """Create the shared Go workspace fixture for each test.""" + make_go_workspace(self) + + def test_edited_job_row_runs(self): + env = { + "GITHUB_EVENT_NAME": "pull_request", + "SCOPE_OUTPUTS": json.dumps({}), "FILTER_OUTPUTS": json.dumps({}), + "GATE_OUTPUTS": json.dumps({}), "BOT_MATRIX": json.dumps({"test": []}), + "CI_JOBS": json.dumps(["web", "workflow-lint"]), + } + with patch("ci_scope.ROOT", self.root): + rows, _ = summarize(env, [CI_FILE], None) + rows = {row["suite"]: row for row in rows} + self.assertEqual(rows["web"]["decision"], "run") + self.assertEqual(rows["web"]["reason"], "ci.yaml: lines in this job changed") + self.assertEqual(rows["workflow lint"]["decision"], "run") + self.assertEqual(rows["web e2e (mock)"]["decision"], "skip") +``` + +### 4b. Existing assertion change: apply ONLY after valgul's sign-off (card, 2026-10-22) + +In `test_shared_inputs_and_dependency_changes_select_every_module`, old: +```python + "Makefile", ".golangci.yml", ".github/workflows/ci.yaml", +``` +new: +```python + "Makefile", +``` + +## 5. Verification (compile only, rule 8) + +```sh +python3 -c "import ast; [ast.parse(open(f).read()) for f in ('hack/ci_scope.py', 'hack/test_ci_scope.py')]" +python3 -c "import yaml; yaml.safe_load(open('.github/workflows/ci.yaml'))" +``` diff --git a/specs/023-change-based-test-selection/briefs/docs.md b/specs/023-change-based-test-selection/briefs/docs.md new file mode 100644 index 000000000..b1eeaafa0 --- /dev/null +++ b/specs/023-change-based-test-selection/briefs/docs.md @@ -0,0 +1,68 @@ +# Brief: PR 5, docs (T048–T050) + +Branch `claude/project-thread-63yei6-pr5`, stacked on PR 4. Apply exactly; touch nothing else. + +## 1. `docs/contributing.md`: new subsection + +Insert right before the line `## Testing` (it follows the `### Security and drift gates` +list), keeping one blank line before and after: + +```markdown +### How CI picks tests + +On a pull request, CI runs only the work your change can affect (spec 023). The +`detect changes` job decides, and its job summary has a **Test selection** table that +lists every suite as `run` or `skip`, with the changed files that chose it or the reason +everything ran. + +What a partial run narrows: + +- **Go unit tests** run only the packages that own a changed file, plus every package + that imports them, across modules. A module's own `go.mod`, `go.sum` or + `.testcoverage.yml` reruns that whole module. The module coverage gate then becomes a + changed-lines gate: the lines you add must meet the module's own minimum + (`hack/check_changed_coverage.py`). +- **Web unit tests** run `vitest related` on the changed `web/src` files, with the same + changed-lines gate at the Vitest `lines` threshold. Lint and build always run whole. + Shared inputs (`package.json`, the lockfile, the Vite/Vitest/TS/ESLint configs, + `src/test/**`, `i18n/locales/**`) run the full suite. +- **Go e2e tests** run only the tests whose components changed, per + `test/e2e/suite-components.json`. Jobs with no selected test are skipped. +- **`ci.yaml` edits** run only the jobs whose lines changed, plus the jobs they need. + `.golangci.yml` runs only the lint legs. Other workflow files run only `workflow lint`. + +Everything still runs on every push to `master`, and when a selection input changes +(`hack/ci_scope.py`, `hack/check_changed_coverage.py`, their tests, `Makefile`, +`go.work`, `go.work.sum`, `.github/actions/**`, `test/e2e/buckets.sh`, +`test/e2e/suite-components.json`). It also runs everything for a `ci.yaml` edit outside +any job or in `changes`/`report`, a path no rule maps, and any selection error. + +To force a full run on your PR, add the label `ci: full-run` (read on the next push), or +start the `ci` workflow by hand on your branch (`workflow_dispatch`). + +When you add an e2e test or a component, keep `test/e2e/suite-components.json` in step: +add the test name pattern under `suites` with the components and paths it exercises, or +list the component under `unexercised` with a reason. The `e2e bucket coverage` job +(`ci_scope.py verify-suites`) fails when a registered test matches no entry or a +component is missing. The full rules are in +`specs/023-change-based-test-selection/contracts/suite-components.md`. +``` + +## 2. `docs/agent-architecture.md`: task map row (T049) + +old: +```markdown +| Translations, language packs, localized server or module messages (i18n) | +``` +Keep that whole row unchanged and add this row directly below it: +```markdown +| CI test selection: which jobs, packages and tests a PR runs (`hack/ci_scope.py`, `hack/check_changed_coverage.py`, `test/e2e/suite-components.json`, the `changes` job in `.github/workflows/ci.yaml`) | `docs/contributing.md` ("How CI picks tests"), `specs/023-change-based-test-selection/contracts/` (`ci-scope-cli.md`, `suite-components.md`, `changed-coverage.md`) | +``` + +## 3. `specs/023-change-based-test-selection/tasks.md` + +Change `- [ ] T048`, `- [ ] T049` and `- [ ] T050` to `- [X]`. + +## 4. Checks + +Only check that the links and paths named above exist (`ls`). No link checker, no lint. diff --git a/specs/023-change-based-test-selection/briefs/unit-selection.md b/specs/023-change-based-test-selection/briefs/unit-selection.md new file mode 100644 index 000000000..f6aace037 --- /dev/null +++ b/specs/023-change-based-test-selection/briefs/unit-selection.md @@ -0,0 +1,1125 @@ +# Brief: PR 3, unit test selection and changed-lines coverage (T031–T040) + +Branch `claude/project-thread-63yei6-pr3`, stacked on PR 2. Facts below were checked +once by the scout; fix agents apply them blind. Every edit is exact: find the quoted +"old" text, replace it with "new". Do not touch any other line, and never edit an +existing test. + +Scout facts: + +- Tests that read files outside their package: `operator/internal/controller` + (`config/crd`, `test/crds/snapshot`), `api/internal/handlers` (`operator/config/crd`), + `gp-module/internal/validator` (`modules/cs2`), `i18n/cmd/i18n-check` + (`i18n/testdata/defective`), `api/internal/messages` (walks all of `api/internal`). +- `tunnel/frpc/go.mod` is a nested module outside `go.work`; the package index skips any + sub-directory that holds its own `go.mod`. +- Every `.testcoverage.yml` has `threshold.total`; some have `exclude.paths` regexes + that match a module-relative file path or a package directory (`^api/v1alpha1$`). +- `web/vitest.config.ts` has `coverage.exclude` (with `**/*.test.{ts,tsx}`) and + `coverage.thresholds.lines: 92`. Its reporters do not include `json`, so the partial + run passes `--coverage.reporter=json`. That also means no `coverage-summary.json`, so + the commit-status step exits 0 on partial runs. +- `make test-ci-scope` runs in the `workflow-lint` job, gated by the `github` filter. + +## 1. `hack/ci_scope.py` + +### 1a. Constants. Insert after the `KNOWN_DIRS = (...)` line (line 48) + +```python +# A change to one of these files in a module root reruns that whole module (research R4). +GO_MODULE_WIDE = {"go.mod", "go.sum", ".testcoverage.yml"} +# Tests that read files outside their package directory. A changed path under a key also +# selects these package directories (research R4, the package form of file_consumers). +FILE_READERS = { + "operator/config/crd": ("operator/internal/controller", "api/internal/handlers"), + "operator/test/crds": ("operator/internal/controller",), + "modules": ("gp-module/internal/validator",), + "i18n/testdata": ("i18n/cmd/i18n-check",), + "api/internal": ("api/internal/messages",), # usage_test walks every api/internal file +} +# Directories that never hold a package the go job tests. +SKIP_PACKAGE_DIRS = {"testdata", "vendor", "node_modules"} +# Web files that change every Vitest run (research R6). +WEB_ALL_FILES = {"web/package.json", "web/package-lock.json", "web/vitest.config.ts", + "web/vite.config.ts", "web/eslint.config.js"} +``` + +### 1b. `FORCE_ALL_FILES` + +old: +```python +FORCE_ALL_FILES = { + ".github/workflows/ci.yaml", "hack/ci_scope.py", "hack/test_ci_scope.py", "Makefile", + "go.work", "go.work.sum", "test/e2e/buckets.sh", ".golangci.yml", +} +``` +new: +```python +FORCE_ALL_FILES = { + ".github/workflows/ci.yaml", "hack/ci_scope.py", "hack/test_ci_scope.py", "Makefile", + "go.work", "go.work.sum", "test/e2e/buckets.sh", ".golangci.yml", + "hack/check_changed_coverage.py", "hack/test_check_changed_coverage.py", +} +``` + +### 1c. `affected_modules` shared prefixes + +old: +```python + p in shared or p.startswith((".github/", "hack/ci_scope", "hack/test_ci_scope")) + or Path(p).name in {"go.mod", "go.sum"} +``` +new: +```python + p in shared or p.startswith((".github/", "hack/ci_scope", "hack/test_ci_scope", + "hack/check_changed_coverage", + "hack/test_check_changed_coverage")) + or Path(p).name in {"go.mod", "go.sum"} +``` + +### 1d. New functions. Insert right before `def verify_suites(suites, buckets, modules):` + +```python +def go_imports(text): + """Return the import paths named in a Go file's import declarations.""" + found = re.findall(r'^import\s+(?:[\w.]+\s+)?"([^"]+)"', text, re.MULTILINE) + for block in re.findall(r"^import\s*\((.*?)\)", text, re.MULTILINE | re.DOTALL): + found += re.findall(r'"([^"]+)"', block) + return found + + +def package_index(root, modules): + """Return {package dir: imported paths} and {import path: package dir} for the workspace.""" + imports, dirs = {}, {} + for module, name in modules.items(): + base = root / module + for current, subdirs, files in os.walk(base): + here = Path(current) + subdirs[:] = sorted(d for d in subdirs if not d.startswith((".", "_")) + and d not in SKIP_PACKAGE_DIRS + and not (here / d / "go.mod").exists()) + sources = [f for f in files if f.endswith(".go")] + if not sources: + continue + directory = here.relative_to(root).as_posix() + inner = here.relative_to(base).as_posix() + dirs[name if inner == "." else f"{name}/{inner}"] = directory + imports[directory] = {path for f in sorted(sources) + for path in go_imports((here / f).read_text(encoding="utf-8"))} + return imports, dirs + + +def owning_module(path, modules): + """Return the workspace module directory that contains path, or None.""" + owners = [m for m in modules if under(path, m)] + return max(owners, key=len) if owners else None + + +def go_packages(root, paths, reason): + """Return {module: [go test package patterns]} for the go job (research R4, OD-8). + + A changed file selects its package and, unless it is a _test.go file, every package + that imports it, across modules. A module's own go.mod, go.sum or .testcoverage.yml, + or a file in no package, selects ./... for that module. + """ + modules = workspace_modules(root) + targets = [m for m in sorted(modules) if m != "test/e2e"] + if reason or paths is None: + return {m: ["./..."] for m in targets} + imports, dirs = package_index(root, modules) + wide, seeds, direct = set(), set(), set() + for path in paths: + if path.endswith(".md"): + continue + readers = [d for key, found in FILE_READERS.items() if under(path, key) for d in found] + direct.update(readers) # a reader's tests read the files; its importers do not + module = owning_module(path, modules) + if module is None: + continue + if path in {f"{module}/{name}" for name in GO_MODULE_WIDE}: + wide.add(module) + continue + directory = path.rpartition("/")[0] + while directory not in imports and directory != module: + directory = directory.rpartition("/")[0] + if directory in imports: + (direct if path.endswith("_test.go") else seeds).add(directory) + elif not readers: + wide.add(module) + importers = {} + for directory, imported in imports.items(): + for name in imported: + if name in dirs: + importers.setdefault(dirs[name], set()).add(directory) + selected, queue = set(), [d for d in seeds if d in imports] + while queue: + directory = queue.pop() + if directory not in selected: + selected.add(directory) + queue.extend(importers.get(directory, ())) + selected |= {d for d in direct if d in imports} + result = {} + for module in targets: + if module in wide: + result[module] = ["./..."] + else: + result[module] = sorted("." if d == module else "./" + d[len(module) + 1:] + for d in selected if owning_module(d, modules) == module) + return result + + +def web_tests(root, paths, reason): + """Return "all" or the changed web/src files for `vitest related` (research R6, OD-9).""" + if reason or paths is None: + return "all" + files = [] + for path in paths: + if (path in WEB_ALL_FILES or under(path, "web/src/test") or under(path, "i18n/locales") + or fnmatch.fnmatch(path, "web/tsconfig*.json")): + return "all" + if not under(path, "web") or path.endswith(".md") or under(path, "web/e2e"): + continue + if not under(path, "web/src") or not (root / path).exists(): + return "all" # vitest related cannot follow a deleted file or a non-source input + files.append(path) + return sorted(files) + + +def unit_outputs(root, paths, reason): + """Return the outputs of mode units: Go package patterns per module and web tests.""" + return { + "all": bool(reason), + "reason": reason or "", + "go-packages": go_packages(root, paths, reason), + "web-tests": web_tests(root, paths, reason), + } +``` + +### 1e. `summarize`: notes on the go and web rows (T039) + +old: +```python + e2e = load_json(env, "E2E_OUTPUTS") +``` +new: +```python + e2e = load_json(env, "E2E_OUTPUTS") + units = load_json(env, "UNITS_OUTPUTS") + go_selected = decoded(units, "go-packages", {}) + web_selected = decoded(units, "web-tests", "all") +``` + +old: +```python + def add(suite, run, feed=(), run_reason="", skip_reason="not affected"): + """Append one selection row with a run or skip decision and its reason.""" +``` +new: +```python + def go_note(module): + """Return the package count note for one go row, or "" for a whole-module run.""" + packages = go_selected.get(module, ["./..."]) if units else ["./..."] + if packages == ["./..."]: + return "" + return f"{len(packages)} packages; " if packages else "vet only, no affected package; " + + if not units or web_selected == "all": + web_note = "" + elif web_selected: + web_note = f"{len(web_selected)} files for vitest related; " + else: + web_note = "lint and build only; " + + def add(suite, run, feed=(), run_reason="", skip_reason="not affected", note=""): + """Append one selection row with a run or skip decision and its reason.""" +``` + +old: +```python + else: + why = skip_reason + rows.append({"suite": suite, "decision": "run" if run else "skip", "reason": why}) +``` +new: +```python + else: + why = skip_reason + if run and not reason: + why = note + why + rows.append({"suite": suite, "decision": "run" if run else "skip", "reason": why}) +``` + +old: +```python + add(f"go ({m}) (amd64, arm64)", gate("go") and m in selected, module_feed(m)) +``` +new: +```python + add(f"go ({m}) (amd64, arm64)", gate("go") and m in selected, module_feed(m), + note=go_note(m)) +``` + +old: +```python + add("web", gate("web"), filtered("web")) +``` +new: +```python + add("web", gate("web"), filtered("web"), note=web_note) +``` + +### 1f. `main`: mode `units` + +old: +```python + parser.add_argument("mode", choices=("modules", "images", "dockerfiles", "bots", "summary", + "e2e", "verify-suites")) +``` +new: +```python + parser.add_argument("mode", choices=("modules", "images", "dockerfiles", "bots", "summary", + "e2e", "verify-suites", "units")) +``` + +Insert right before the line ` # PRs use affected work; every master push, manual run and ci: full-run`: + +```python + if args.mode == "units": + env = os.environ + try: + event_reason = full_scope_reason(env, []) + paths = None if event_reason else changed_paths(ROOT, args.base, args.head) + reason = full_scope_reason(env, paths) + modules = workspace_modules(ROOT) + unmapped = [p for p in paths or [] if is_unmapped(p, modules)] + if reason is None and unmapped: + reason = f"unmapped path {unmapped[0]}" + values = unit_outputs(ROOT, paths, reason) + except Exception as exc: # any doubt runs every unit test (FR-002) + print(f"::warning::unit selection failed, running every unit test: {exc}") + values = unit_outputs(ROOT, None, f"error: {exc}") + emit(values) + return +``` + +Also in the summary-mode block, add `UNITS_OUTPUTS` to nothing: no change there. + +## 2. New file `hack/check_changed_coverage.py` + +```python +"""Changed-lines coverage gate for partial CI runs (spec 023 FR-009, OD-1, OD-5). + +Reads added lines from `git diff -U0`, intersects them with a Go cover profile or a +Vitest (Istanbul) coverage-final.json, and fails when the covered share of added +statement lines is below the module's own threshold. Python stdlib only. +""" + +import argparse +import fnmatch +import json +import os +import re +import subprocess +import sys +from pathlib import Path + + +ROOT = Path(__file__).resolve().parent.parent +PROFILE_LINE = re.compile(r"^(.+):(\d+)\.\d+,(\d+)\.\d+ (\d+) (\d+)$") +HUNK = re.compile(r"^@@ -\S+ \+(\d+)(?:,(\d+))? @@") + + +def added_lines(root, base, head, prefix): + """Return {repo path: set of added line numbers} for files under prefix.""" + diff = subprocess.run( + ["git", "diff", "-U0", "--no-color", "--no-renames", base, head, "--", prefix], + cwd=root, check=True, capture_output=True, text=True).stdout + lines, current = {}, None + for line in diff.splitlines(): + if line.startswith("+++ "): + target = line[4:] + current = target[2:] if target.startswith("b/") else None + elif current and line.startswith("@@"): + match = HUNK.match(line) + if match: + start, count = int(match[1]), int(match[2] or "1") + lines.setdefault(current, set()).update(range(start, start + count)) + return lines + + +def expand(glob): + """Expand {a,b} alternatives in a glob into plain fnmatch patterns.""" + match = re.search(r"\{([^{}]*)\}", glob) + if not match: + return [glob] + return [pattern for option in match[1].split(",") + for pattern in expand(glob[:match.start()] + option + glob[match.end():])] + + +def go_config(module_dir): + """Return (threshold.total, exclude.paths regexes) from a module's .testcoverage.yml.""" + text = (module_dir / ".testcoverage.yml").read_text(encoding="utf-8") + total, excludes, section = None, [], None + for raw in text.splitlines(): + line = raw.split("#", 1)[0].rstrip() + if not line.strip(): + continue + if not line.startswith((" ", "-")): + section = line.split(":", 1)[0].strip() + continue + item = line.strip() + if section == "threshold" and item.startswith("total:"): + total = float(item.split(":", 1)[1]) + elif section == "exclude" and item.startswith("- "): + excludes.append(item[2:].strip().strip("'\"")) + if total is None: + raise ValueError(f"{module_dir.name}/.testcoverage.yml has no threshold.total") + return total, excludes + + +def vitest_config(web_dir): + """Return (coverage.thresholds.lines, expanded coverage.exclude globs) from vitest.config.ts.""" + text = (web_dir / "vitest.config.ts").read_text(encoding="utf-8") + start = text.find("coverage:") + coverage = text[start:] if start >= 0 else "" + lines = re.search(r"thresholds:\s*\{[^}]*?\blines:\s*(\d+(?:\.\d+)?)", coverage) + if not lines: + raise ValueError("web/vitest.config.ts has no coverage.thresholds.lines") + block = re.search(r"\bexclude:\s*\[(.*?)\]", coverage, re.DOTALL) + globs = re.findall(r'"([^"]+)"', block[1]) if block else [] + return float(lines[1]), [pattern for glob in globs for pattern in expand(glob)] + + +def mark(lines, first, last, covered): + """Mark lines first..last; a line is covered when any block covering it ran.""" + for number in range(first, last + 1): + lines[number] = lines.get(number, False) or covered + + +def go_statements(profile, module_path, module): + """Return {repo path: {line: covered}} from a Go cover profile.""" + result = {} + for line in Path(profile).read_text(encoding="utf-8").splitlines(): + match = PROFILE_LINE.match(line) + if not match or not match[1].startswith(module_path + "/"): + continue + path = module + "/" + match[1][len(module_path) + 1:] + mark(result.setdefault(path, {}), int(match[2]), int(match[3]), int(match[5]) > 0) + return result + + +def vitest_statements(report, root): + """Return {repo path: {line: covered}} from an Istanbul coverage-final.json.""" + data = json.loads(Path(report).read_text(encoding="utf-8")) + result = {} + for name, entry in data.items(): + path = Path(entry.get("path", name)) + if not path.is_absolute(): + path = root / path + try: + relative = path.resolve().relative_to(root.resolve()).as_posix() + except ValueError: + continue + lines = result.setdefault(relative, {}) + counts = entry.get("s", {}) + for key, location in entry.get("statementMap", {}).items(): + mark(lines, location["start"]["line"], location["end"]["line"], + counts.get(key, 0) > 0) + return result + + +def check(added, statements, threshold, excluded): + """Return (exit code, one-line result, uncovered file:line list).""" + total, missing = 0, [] + for path in sorted(added): + if excluded(path): + continue + known = statements.get(path, {}) + for number in sorted(added[path]): + if number in known: + total += 1 + if not known[number]: + missing.append(f"{path}:{number}") + if total == 0: + return 0, "changed-lines coverage: no added statement lines, passes", [] + covered = total - len(missing) + share = 100 * covered / total + line = (f"changed-lines coverage: {covered}/{total} added statement lines covered " + f"({share:.1f}%, minimum {threshold:g}%)") + return (0 if share >= threshold else 1), line, missing + + +def go_inputs(root, module, profiles, base, head): + """Return (threshold, added lines, statements, excluded) for one Go module.""" + threshold, patterns = go_config(root / module) + text = (root / module / "go.mod").read_text(encoding="utf-8") + match = re.search(r"^module\s+(\S+)", text, re.MULTILINE) + if not match: + raise ValueError(f"{module}/go.mod has no module directive") + statements = {} + for profile in profiles: + for path, lines in go_statements(profile, match[1], module).items(): + merged = statements.setdefault(path, {}) + for number, covered in lines.items(): + mark(merged, number, number, covered) + + def excluded(path): + """Skip tests, non-Go files and the module's own coverage exclusions.""" + relative = path[len(module) + 1:] + directory = relative.rpartition("/")[0] + return (not relative.endswith(".go") or relative.endswith("_test.go") + or any(re.search(p, relative) or re.search(p, directory) for p in patterns)) + + return threshold, added_lines(root, base, head, module), statements, excluded + + +def web_inputs(root, report, base, head): + """Return (threshold, added lines, statements, excluded) for the dashboard.""" + threshold, globs = vitest_config(root / "web") + + def excluded(path): + """Skip tests, non-TypeScript files and vitest.config.ts coverage.exclude.""" + relative = path[len("web/"):] + return (not relative.endswith((".ts", ".tsx")) + or relative.endswith((".test.ts", ".test.tsx")) + or any(fnmatch.fnmatch(relative, glob) for glob in globs)) + + return (threshold, added_lines(root, base, head, "web/src"), + vitest_statements(report, root), excluded) + + +def main(argv=None): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--base", required=True) + parser.add_argument("--head", required=True) + parser.add_argument("--go-profile", action="append", default=[]) + parser.add_argument("--module") + parser.add_argument("--vitest-json") + parser.add_argument("--root", default=str(ROOT)) + args = parser.parse_args(argv) + root = Path(args.root) + if not args.vitest_json and not (args.go_profile and args.module): + parser.error("pass --vitest-json, or --go-profile with --module") + try: + if args.vitest_json: + inputs = web_inputs(root, args.vitest_json, args.base, args.head) + else: + inputs = go_inputs(root, args.module.strip("/"), args.go_profile, args.base, + args.head) + except (OSError, ValueError, KeyError, subprocess.CalledProcessError) as exc: + print(f"::error::changed-lines coverage: {exc}") + return 1 + threshold, added, statements, excluded = inputs + code, line, missing = check(added, statements, threshold, excluded) + print(line) + for item in missing: + print(f"uncovered: {item}") + summary = os.environ.get("GITHUB_STEP_SUMMARY") + if summary: + with open(summary, "a", encoding="utf-8") as output: + output.write(line + "\n") + return code + + +if __name__ == "__main__": + sys.exit(main()) +``` + +## 3. New file `hack/test_check_changed_coverage.py` (T033) + +```python +"""Behavioral coverage for the changed-lines coverage gate (run in CI).""" + +import contextlib +import io +import json +import os +import subprocess +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +from check_changed_coverage import check, expand, go_config, main, vitest_config + + +GIT = ["git", "-c", "user.name=test", "-c", "user.email=test@example.com", + "-c", "commit.gpgsign=false", "-c", "core.hooksPath=/dev/null"] +COVERAGE_YML = """threshold: + file: 0 + total: 80 + +exclude: + paths: + - ^cmd/ # wiring +""" +VITEST_CONFIG = """export default defineConfig({ + test: { + exclude: ["e2e/**"], + coverage: { + exclude: ["src/main.tsx", "**/*.test.{ts,tsx}"], + thresholds: { + lines: 92, + functions: 76, + }, + }, + }, +}); +""" +GO_FUNC = "package mod\n// added comment\nfunc F() int {\n\treturn 1\n}\n" + + +class ChangedCoverageTests(unittest.TestCase): + """Check added-line coverage against Go profiles and Istanbul JSON.""" + + def setUp(self): + """Create a git repository with one Go module and a web tree.""" + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + self.git("init", "-q") + self.write("mod/go.mod", "module example.com/mod\n") + self.write("mod/.testcoverage.yml", COVERAGE_YML) + self.write("mod/a.go", "package mod\n") + self.write("mod/cmd/main.go", "package main\n") + self.write("web/vitest.config.ts", VITEST_CONFIG) + self.write("web/src/a.ts", "export const a = 1;\n") + self.base = self.commit() + self.head = self.base + + def git(self, *args): + """Run git in the fixture repository and return its trimmed output.""" + return subprocess.run([*GIT, *args], cwd=self.root, check=True, capture_output=True, + text=True).stdout.strip() + + def write(self, name, content): + """Write a fixture file and return its path.""" + path = self.root / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(content, encoding="utf-8") + return path + + def commit(self): + """Commit every fixture change and return the new commit id.""" + self.git("add", "-A") + self.git("commit", "-q", "-m", "change") + return self.git("rev-parse", "HEAD") + + def run_main(self, *args): + """Run the gate and return (exit code, stdout, step summary).""" + summary = self.root / "summary.md" + out = io.StringIO() + with patch.dict(os.environ, {"GITHUB_STEP_SUMMARY": str(summary)}), \ + contextlib.redirect_stdout(out): + code = main(["--root", str(self.root), "--base", self.base, "--head", self.head, + *args]) + text = summary.read_text(encoding="utf-8") if summary.exists() else "" + return code, out.getvalue(), text + + def go_change(self, count): + """Add a comment and a three-line function block; return a profile path.""" + self.write("mod/a.go", GO_FUNC) + self.head = self.commit() + profile = self.write("profile.out", + f"mode: atomic\nexample.com/mod/a.go:3.14,5.2 1 {count}\n") + return str(profile) + + def test_covered_go_lines_pass(self): + code, out, summary = self.run_main("--go-profile", self.go_change(1), "--module", "mod") + self.assertEqual(code, 0) + self.assertIn("3/3 added statement lines covered", out) + self.assertIn("changed-lines coverage", summary) + + def test_uncovered_go_lines_fail_and_are_listed(self): + code, out, _ = self.run_main("--go-profile", self.go_change(0), "--module", "mod") + self.assertEqual(code, 1) + self.assertIn("0/3 added statement lines covered", out) + for number in (3, 4, 5): + self.assertIn(f"uncovered: mod/a.go:{number}", out) + + def test_zero_statement_lines_pass(self): + self.write("mod/a.go", "package mod\n// only a comment\n") + self.head = self.commit() + profile = self.write("profile.out", "mode: atomic\n") + code, out, _ = self.run_main("--go-profile", str(profile), "--module", "mod") + self.assertEqual(code, 0) + self.assertIn("no added statement lines", out) + + def test_go_tests_and_excluded_paths_are_ignored(self): + self.write("mod/cmd/main.go", "package main\nfunc main() {\n}\n") + self.write("mod/a_test.go", "package mod\nfunc helper() {\n}\n") + self.head = self.commit() + profile = self.write("profile.out", "mode: atomic\n" + "example.com/mod/cmd/main.go:2.13,3.2 0 0\n" + "example.com/mod/a_test.go:2.15,3.2 0 0\n") + code, out, _ = self.run_main("--go-profile", str(profile), "--module", "mod") + self.assertEqual(code, 0) + self.assertIn("no added statement lines", out) + + def test_istanbul_json_for_web(self): + self.write("web/src/a.ts", "export const a = 1;\nexport const b = 2;\n") + self.head = self.commit() + source = str(self.root / "web/src/a.ts") + for hits, want in ((0, 1), (1, 0)): + with self.subTest(hits=hits): + report = self.write("web/coverage/coverage-final.json", json.dumps({source: { + "path": source, + "statementMap": {"0": {"start": {"line": 2, "column": 0}, + "end": {"line": 2, "column": 20}}}, + "s": {"0": hits}, + }})) + code, out, _ = self.run_main("--vitest-json", str(report)) + self.assertEqual(code, want, out) + + def test_threshold_comes_from_testcoverage_yml(self): + self.assertEqual(go_config(self.root / "mod"), (80.0, ["^cmd/"])) + + def test_threshold_comes_from_vitest_config(self): + self.assertEqual(vitest_config(self.root / "web"), + (92.0, ["src/main.tsx", "**/*.test.ts", "**/*.test.tsx"])) + + def test_missing_threshold_fails(self): + self.write("mod/.testcoverage.yml", "threshold:\n file: 0\n") + with self.assertRaises(ValueError): + go_config(self.root / "mod") + code, out, _ = self.run_main("--go-profile", self.go_change(1), "--module", "mod") + self.assertEqual(code, 1) + self.assertIn("::error::", out) + self.write("web/vitest.config.ts", "export default defineConfig({ test: {} });\n") + with self.assertRaises(ValueError): + vitest_config(self.root / "web") + + def test_share_at_threshold_passes(self): + added = {"f.go": {1, 2}} + statements = {"f.go": {1: True, 2: False}} + self.assertEqual(check(added, statements, 50, lambda _: False)[0], 0) + self.assertEqual(check(added, statements, 51, lambda _: False)[0], 1) + + def test_brace_expansion(self): + self.assertEqual(expand("**/*.test.{ts,tsx}"), ["**/*.test.ts", "**/*.test.tsx"]) + self.assertEqual(expand("src/main.tsx"), ["src/main.tsx"]) + + +if __name__ == "__main__": + unittest.main() +``` + +## 4. `hack/test_ci_scope.py`: new test classes (T031, T032). Existing tests untouched. + +Import line: add `go_packages, web_tests` to the `from ci_scope import (...)` list (keep +the rest exactly). old: +```python + is_unmapped, load_suites, main, publish_paths, select_images, summarize, + verify_suites, + workspace_modules) +``` +new: +```python + is_unmapped, load_suites, main, publish_paths, select_images, summarize, + verify_suites, go_packages, web_tests, + workspace_modules) +``` + +Insert right before `if __name__ == "__main__":` at the end of the file: + +```python +class GoPackageTests(unittest.TestCase): + """Package-level Go selection from import scanning (spec 023 T031, research R4).""" + + def setUp(self): + """Create the shared workspace plus three api packages and one lib package.""" + make_go_workspace(self) + write_file(self.root, "lib/helpers/h.go", "package helpers\n") + write_file(self.root, "api/internal/a/a.go", + 'package a\n\nimport "example.com/lib/helpers"\n\nvar _ = helpers.X\n') + write_file(self.root, "api/internal/a/a_test.go", "package a\n") + write_file(self.root, "api/internal/b/b.go", + 'package b\n\nimport (\n\t"fmt"\n\tx "example.com/api/internal/a"\n)\n') + write_file(self.root, "api/internal/c/c.go", "package c\n") + write_file(self.root, "api/internal/c/testdata/in.txt", "fixture\n") + + def packages(self, *paths): + """Return the go-packages selection for the changed paths.""" + return go_packages(self.root, list(paths), None) + + def test_leaf_change_selects_only_its_package(self): + self.assertEqual(self.packages("api/internal/b/b.go"), + {"api": ["./internal/b"], "lib": [], "other": []}) + + def test_importer_chain_crosses_modules(self): + self.assertEqual(self.packages("lib/helpers/h.go"), + {"api": ["./internal/a", "./internal/b"], "lib": ["./helpers"], + "other": []}) + + def test_test_file_change_selects_only_its_package(self): + self.assertEqual(self.packages("api/internal/a/a_test.go")["api"], ["./internal/a"]) + + def test_testdata_and_embedded_files_select_their_package(self): + self.assertEqual(self.packages("api/internal/c/testdata/in.txt")["api"], + ["./internal/c"]) + self.assertEqual(self.packages("api/internal/c/schema.json")["api"], ["./internal/c"]) + + def test_module_files_select_the_whole_module(self): + for name in ("go.mod", "go.sum", ".testcoverage.yml"): + with self.subTest(name=name): + self.assertEqual(self.packages(f"api/{name}"), + {"api": ["./..."], "lib": [], "other": []}) + + def test_file_outside_any_package_selects_the_whole_module(self): + self.assertEqual(self.packages("api/config/x.yaml")["api"], ["./..."]) + + def test_markdown_selects_nothing(self): + self.assertEqual(self.packages("api/README.md")["api"], []) + + def test_full_scope_runs_every_module(self): + self.assertEqual(go_packages(self.root, None, "master push"), + {"api": ["./..."], "lib": ["./..."], "other": ["./..."]}) + + +class WebTestsTests(unittest.TestCase): + """Web unit selection for `vitest related` (spec 023 T032, research R6).""" + + def setUp(self): + """Create a temporary web tree with one route.""" + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + write_file(self.root, "web/src/routes/servers.tsx", "export {};\n") + + def test_route_file_gives_its_source_path(self): + self.assertEqual(web_tests(self.root, ["web/src/routes/servers.tsx", "api/x.go"], None), + ["web/src/routes/servers.tsx"]) + + def test_shared_web_inputs_give_all(self): + for path in ("web/src/test/setup.ts", "web/package.json", "web/package-lock.json", + "web/vitest.config.ts", "web/vite.config.ts", "web/tsconfig.app.json", + "web/tsconfig.json", "web/eslint.config.js", "i18n/locales/en/app.json"): + with self.subTest(path=path): + self.assertEqual(web_tests(self.root, [path], None), "all") + + def test_deleted_source_gives_all(self): + self.assertEqual(web_tests(self.root, ["web/src/routes/gone.tsx"], None), "all") + + def test_playwright_and_unrelated_paths_select_nothing(self): + self.assertEqual(web_tests(self.root, ["web/e2e/servers.spec.ts", "web/README.md", + "docs/a.md"], None), []) + + def test_full_scope_gives_all(self): + self.assertEqual(web_tests(self.root, None, "master push"), "all") + self.assertEqual(web_tests(self.root, ["web/src/routes/servers.tsx"], "manual run"), + "all") + + +class UnitSummaryNoteTests(unittest.TestCase): + """The summary rows name package and file counts from mode units (spec 023 T039).""" + + def setUp(self): + """Create the shared Go workspace fixture for each test.""" + make_go_workspace(self) + + def rows(self, units): + """Return the summary rows of a partial run with go and web gates on.""" + env = { + "GITHUB_EVENT_NAME": "pull_request", + "SCOPE_OUTPUTS": json.dumps({"modules": json.dumps(["api", "lib"])}), + "FILTER_OUTPUTS": json.dumps({}), + "GATE_OUTPUTS": json.dumps({"go": "true", "web": "true"}), + "BOT_MATRIX": json.dumps({"test": []}), + "UNITS_OUTPUTS": json.dumps({key: json.dumps(value) for key, value in units.items()}), + } + with patch("ci_scope.ROOT", self.root): + rows, _ = summarize(env, ["api/internal/a/a.go"], None) + return {row["suite"]: row["reason"] for row in rows} + + def test_notes_name_package_and_file_counts(self): + rows = self.rows({"go-packages": {"api": ["./internal/a", "./internal/b"], "lib": []}, + "web-tests": ["web/src/a.tsx"]}) + self.assertTrue(rows["go (api) (amd64, arm64)"].startswith("2 packages; ")) + self.assertTrue(rows["go (lib) (amd64, arm64)"].startswith("vet only, no affected package; ")) + self.assertTrue(rows["web"].startswith("1 files for vitest related; ")) + + def test_whole_runs_add_no_note(self): + rows = self.rows({"go-packages": {"api": ["./..."], "lib": ["./..."]}, "web-tests": "all"}) + self.assertFalse(rows["go (api) (amd64, arm64)"].startswith(("2 packages", "vet only"))) + self.assertFalse(rows["web"].startswith(("1 files", "lint and build"))) + rows = self.rows({"go-packages": {}, "web-tests": []}) + self.assertTrue(rows["web"].startswith("lint and build only; ")) +``` + +## 5. `Makefile` test-ci-scope target + +old: +```make + python3 -m unittest discover -s hack -p 'test_ci_scope.py' -v +``` +new: +```make + python3 -m unittest discover -s hack -p 'test_ci_scope.py' -v + python3 -m unittest discover -s hack -p 'test_check_changed_coverage.py' -v +``` +(Tab-indented, like the line above.) + +## 6. `.github/workflows/ci.yaml` + +6a. `changes` job outputs. old: +```yaml + e2e-bots-any: ${{ steps.e2e.outputs.bots-any }} +``` +new: +```yaml + e2e-bots-any: ${{ steps.e2e.outputs.bots-any }} + go-packages: ${{ steps.units.outputs.go-packages }} + web-tests: ${{ steps.units.outputs.web-tests }} +``` + +6b. New step right after the `e2e` step (that is, insert before +` - uses: dorny/paths-filter@`): +```yaml + - name: select the unit test packages and web tests the change affects + id: units + # Package-level Go and `vitest related` selection (spec 023 US2). Any + # doubt emits ./... for every module and "all" for web. + env: + BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.before }} + HEAD_SHA: ${{ github.sha }} + CI_FULL_RUN: "${{ contains(github.event.pull_request.labels.*.name, 'ci: full-run') }}" + run: python3 hack/ci_scope.py units --base "$BASE_SHA" --head "$HEAD_SHA" +``` + +6c. Both the `ci:` and the `github:` paths-filter lists contain the line +` - 'hack/test_ci_scope.py'`. After EACH of the two, add: +```yaml + - 'hack/check_changed_coverage.py' + - 'hack/test_check_changed_coverage.py' +``` + +6d. Summary step env. old: +```yaml + E2E_OUTPUTS: ${{ toJSON(steps.e2e.outputs) }} +``` +new: +```yaml + E2E_OUTPUTS: ${{ toJSON(steps.e2e.outputs) }} + UNITS_OUTPUTS: ${{ toJSON(steps.units.outputs) }} +``` + +6e. `go` job. old: +```yaml + timeout-minutes: 20 + permissions: + contents: read + strategy: + fail-fast: false + matrix: + module: [netguard, +``` +new (only the `env:` block is added; the `module:` line continues unchanged): +```yaml + timeout-minutes: 20 + permissions: + contents: read + env: + # Space-separated package patterns from the changes job (spec 023 US2). + # Empty means no package is affected and only vet runs; a module the + # selection does not list runs ./... . + GO_PACKAGES: ${{ join(fromJSON(needs.changes.outputs.go-packages || '{}')[matrix.module] || fromJSON('["./..."]'), ' ') }} + BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.before }} + strategy: + fail-fast: false + matrix: + module: [netguard, +``` + +6f. Unit test step. old: +```yaml + - name: unit test + coverage + working-directory: ${{ matrix.module }} + run: | + mkdir -p coverage + go test -covermode=atomic -coverpkg=./... \ + -coverprofile=coverage/unit.out ./... +``` +new: +```yaml + - name: unit test + coverage + if: env.GO_PACKAGES != '' + working-directory: ${{ matrix.module }} + run: | + mkdir -p coverage + read -r -a packages <<<"$GO_PACKAGES" + go test -covermode=atomic -coverpkg=./... \ + -coverprofile=coverage/unit.out "${packages[@]}" +``` + +6g. Envtest caches and step. In the three steps named `setup-envtest binary cache`, +`envtest assets cache` and `integration (envtest) test + coverage`, replace +```yaml + if: matrix.module == 'operator' || matrix.module == 'api' +``` +with +```yaml + if: (matrix.module == 'operator' || matrix.module == 'api') && env.GO_PACKAGES != '' +``` +And in the envtest run block, old: +```yaml + export KUBEBUILDER_ASSETS + go test -tags=envtest -timeout 10m \ + -covermode=atomic -coverpkg=./... \ + -coverprofile=coverage/envtest.out ./... +``` +new: +```yaml + export KUBEBUILDER_ASSETS + read -r -a packages <<<"$GO_PACKAGES" + go test -tags=envtest -timeout 10m \ + -covermode=atomic -coverpkg=./... \ + -coverprofile=coverage/envtest.out "${packages[@]}" +``` + +6h. Coverage steps. old: +```yaml + - name: warm coverage tool cache + # cover-go-merge and cover-go-check `go run` pinned tools, so they hit +``` +new: +```yaml + - name: warm coverage tool cache + if: env.GO_PACKAGES != '' + # cover-go-merge and cover-go-check `go run` pinned tools, so they hit +``` +old: +```yaml + - name: merge profiles + threshold gate + run: make cover-go-merge && make cover-go-check +``` +new: +```yaml + - name: merge profiles + threshold gate + # A whole-module run keeps the module total gate (spec 023 OD-1). + if: env.GO_PACKAGES == './...' + run: make cover-go-merge && make cover-go-check + - name: merge profiles + changed-lines coverage gate + # A package-level run gates the added lines at the module's own + # threshold instead (spec 023 OD-1, OD-5). + if: env.GO_PACKAGES != '' && env.GO_PACKAGES != './...' + env: + MODULE: ${{ matrix.module }} + run: | + git fetch --no-tags --depth=1 origin "$BASE_SHA" + make cover-go-merge + python3 hack/check_changed_coverage.py --base "$BASE_SHA" --head "$GITHUB_SHA" \ + --go-profile "$MODULE/coverage/merged.out" --module "$MODULE" +``` +old: +```yaml + if: matrix.module == 'api' && matrix.arch == 'amd64' + run: | + awk '$1 ~ /\/internal\/handlers\/secrets_managed\.go:/ +``` +new: +```yaml + if: matrix.module == 'api' && matrix.arch == 'amd64' && env.GO_PACKAGES != '' + run: | + awk '$1 ~ /\/internal\/handlers\/secrets_managed\.go:/ +``` + +6i. `web` job unit step. old: +```yaml + - name: unit tests + coverage + id: cover + run: npm run test:cover +``` +new: +```yaml + - name: unit tests + coverage + id: cover + # Partial runs test only what the changed web/src files reach and gate + # the added lines; "all" keeps the full suite and thresholds (spec 023 OD-9). + env: + WEB_TESTS: ${{ needs.changes.outputs.web-tests }} + BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.before }} + run: | + set -euo pipefail + if [ -z "$WEB_TESTS" ] || [ "$WEB_TESTS" = '"all"' ]; then + npm run test:cover + exit 0 + fi + mapfile -t files < <(jq -r '.[] | sub("^web/"; "")' <<<"$WEB_TESTS") + if [ "${#files[@]}" -eq 0 ]; then + echo "No dashboard source changed; Vitest skipped, lint and build ran." + exit 0 + fi + npx vitest related --run --passWithNoTests --coverage --coverage.reporter=json \ + --coverage.thresholds.lines=0 --coverage.thresholds.functions=0 \ + --coverage.thresholds.branches=0 --coverage.thresholds.statements=0 \ + "${files[@]}" + if [ ! -f coverage/coverage-final.json ]; then + echo "No coverage report from vitest related; running the full suite." + npm run test:cover + exit 0 + fi + git fetch --no-tags --depth=1 origin "$BASE_SHA" + cd .. && python3 hack/check_changed_coverage.py --base "$BASE_SHA" --head "$GITHUB_SHA" \ + --vitest-json web/coverage/coverage-final.json +``` + +## 7. Specs + +7a. `specs/023-change-based-test-selection/contracts/ci-scope-cli.md`, old: +```markdown +| `go-packages` | JSON object: module → list of package patterns | `["./..."]` on full scope or when a module's own `go.mod`/`go.sum` changed | +| `web-tests` | JSON: `"all"` or list of `web/src/...` source paths for `vitest related` | | +``` +new: +```markdown +| `go-packages` | JSON object: module → list of package patterns | Emitted by mode `units`. `["./..."]` on full scope or when a module's own `go.mod`/`go.sum`/`.testcoverage.yml` changed | +| `web-tests` | JSON: `"all"` or list of `web/src/...` source paths for `vitest related` | Emitted by mode `units` | +``` + +7b. Same file, insert before the line `## Mode \`bots\` — changed`: +```markdown +## Mode `units` (new) + +Emits package-level Go selection and the web unit selection (research R4, R6). The +`units` step of the `changes` job runs right after `e2e`. Outputs: + +| Key | Type | Meaning | +|---|---|---| +| `all` | boolean | True when `reason` is set | +| `reason` | string | A full-scope reason, `unmapped path

` or `error: `, or empty | +| `go-packages` | JSON object: module → list | Package patterns for `go test` (`./...`, `.`, `./internal/x`); an empty list means vet only. Every Go module except `test/e2e` is listed | +| `web-tests` | `"all"` or JSON list | `web/src` files for `vitest related`; an empty list skips Vitest (lint and build still run) | + +Rules: a changed file selects its package directory (walking up to the nearest one) and, +unless it is a `_test.go` file, every package that imports it, transitively and across +modules. `FILE_READERS` adds the packages whose tests read the changed directory. A +module's own `go.mod`, `go.sum` or `.testcoverage.yml`, or a file under no package, +gives `["./..."]`. For web, `package.json`, `package-lock.json`, `vitest.config.ts`, +`vite.config.ts`, `tsconfig*.json`, `eslint.config.js`, `src/test/**`, `i18n/locales/**`, +a deleted `web/src` file and any other `web/` file outside `src` and `e2e` give `"all"`. + +``` + +7c. `specs/023-change-based-test-selection/contracts/changed-coverage.md`, append at the +end of the `## Behaviour` list (after item 5, before `## Not changed`): +```markdown +6. A profile block marks every line from its start to its end line; a line counts as + covered when any block over it ran. Istanbul statements are read the same way. +7. Web: when `vitest related` writes no `coverage-final.json`, the job falls back to the + full `npm run test:cover` run instead of calling this script. +``` + +7d. `specs/023-change-based-test-selection/tasks.md`: change `- [ ] T031` … `- [ ] T040` +(ten lines) to `- [X]`. In the T033 line, replace +`extend the \`test-ci-scope\` target in \`Makefile\` to discover \`test_*.py\` in \`hack/\`` +with +`add a second \`unittest discover\` line for \`test_check_changed_coverage.py\` to the \`test-ci-scope\` target in \`Makefile\` (a \`test_*.py\` glob would also run \`test_auto_label.py\`, which has its own workflow)`. + +## 8. Verification (compile only, rule 8) + +```sh +python3 -m py_compile hack/ci_scope.py hack/check_changed_coverage.py \ + hack/test_ci_scope.py hack/test_check_changed_coverage.py +python3 -c "import yaml,sys; yaml.safe_load(open('.github/workflows/ci.yaml'))" +``` +No unittest, no make test-ci-scope, no lint. diff --git a/specs/023-change-based-test-selection/contracts/changed-coverage.md b/specs/023-change-based-test-selection/contracts/changed-coverage.md index 4e8ad371f..e3544db15 100644 --- a/specs/023-change-based-test-selection/contracts/changed-coverage.md +++ b/specs/023-change-based-test-selection/contracts/changed-coverage.md @@ -29,6 +29,10 @@ python3 hack/check_changed_coverage.py --base --head \ the check passes and says so. 4. Exit 1 when the share is below the threshold, listing each uncovered `file:line`. 5. Prints a one-line result to `$GITHUB_STEP_SUMMARY`. +6. A profile block marks every line from its start to its end line; a line counts as + covered when any block over it ran. Istanbul statements are read the same way. +7. Web: when `vitest related` writes no `coverage-final.json`, the job falls back to the + full `npm run test:cover` run instead of calling this script. ## Not changed diff --git a/specs/023-change-based-test-selection/contracts/ci-scope-cli.md b/specs/023-change-based-test-selection/contracts/ci-scope-cli.md index 2572d915f..ff2bc5b3e 100644 --- a/specs/023-change-based-test-selection/contracts/ci-scope-cli.md +++ b/specs/023-change-based-test-selection/contracts/ci-scope-cli.md @@ -21,12 +21,12 @@ path forces everything (research R8). | Key | Type | Meaning | |---|---|---| | `scope` | `"full"` \| `"partial"` | Run-level scope (FR-009 switch). Emitted by mode `summary`, not `modules` (see below) | -| `go-packages` | JSON object: module → list of package patterns | `["./..."]` on full scope or when a module's own `go.mod`/`go.sum` changed | -| `web-tests` | JSON: `"all"` or list of `web/src/...` source paths for `vitest related` | | +| `go-packages` | JSON object: module → list of package patterns | Emitted by mode `units`. `["./..."]` on full scope or when a module's own `go.mod`/`go.sum`/`.testcoverage.yml` changed | +| `web-tests` | JSON: `"all"` or list of `web/src/...` source paths for `vitest related` | Emitted by mode `units` | | `e2e-exclude` | Not emitted (decision 2). Unselected legs skip through `RUN_REGEX` instead | Would have been read by `e2e-go` and `e2e-go-arm64` | | `e2e-run` | JSON object: bucket → anchored `-run` regex of the selected tests | Emitted by mode `e2e`. Replaces `buckets.sh regex ` in the e2e jobs (OD-6); equals it on full scope | | `e2e-multicluster`, `e2e-upgrade`, `e2e-web-live`, `e2e-capture-overhead` | `"true"`/`"false"` | Emitted by mode `e2e`. Folded into the existing `combine` step booleans | -| `ci-jobs` | JSON list of job ids selected by the `ci.yaml` line mapper | Folded into each job's gate | +| `ci-jobs` | JSON list of job ids selected by the `ci.yaml` line mapper | Emitted by mode `jobs`. Folded into each job's gate | | `unmapped` | JSON list of paths | Emitted by mode `e2e`. Non-empty forces full scope and fails `selection-rules` | | `selection` | JSON list of SelectionResult | Emitted by mode `summary`; read by the summary table and `report` | @@ -72,6 +72,47 @@ Emits the e2e selection that `e2e_outputs()` computes from | `bots-any` | boolean | False skips `e2e-game-bot` | | `universal` | boolean | True when a universal path selected every test without a full-scope reason; `combine` folds it into the e2e and image gates, because universal paths such as `.dockerignore`, `containers/` and `signing/` match no e2e path filter | +## Mode `units` (new) + +Emits package-level Go selection and the web unit selection (research R4, R6). The +`units` step of the `changes` job runs right after `e2e`. Outputs: + +| Key | Type | Meaning | +|---|---|---| +| `all` | boolean | True when `reason` is set | +| `reason` | string | A full-scope reason, `unmapped path

` or `error: `, or empty | +| `go-packages` | JSON object: module → list | Package patterns for `go test` (`./...`, `.`, `./internal/x`); an empty list means vet only. Every Go module except `test/e2e` is listed | +| `web-tests` | `"all"` or JSON list | `web/src` files for `vitest related`; an empty list skips Vitest (lint and build still run) | + +Rules: a changed file selects its package directory (walking up to the nearest one) and, +unless it is a `_test.go` file, every package that imports it, transitively and across +modules. `FILE_READERS` adds the packages whose tests read the changed directory. A +module's own `go.mod`, `go.sum` or `.testcoverage.yml`, or a file under no package, +gives `["./..."]`; `go.mod` and `go.sum` also select every package that imports the +module, and a deleted `.go` file whose package is gone selects the packages that still +import that package. For web, `package.json`, `package-lock.json`, `vitest.config.ts`, +`vite.config.ts`, `tsconfig*.json`, `eslint.config.js`, `src/test/**`, `i18n/locales/**`, +a deleted `web/src` file and any other `web/` file outside `src` and `e2e` give `"all"`. + +## Mode `jobs` (new) + +Maps shared CI file changes to `ci.yaml` job ids (research R8, settled OD-3). The `jobs` +step of the `changes` job runs right after `units`. Outputs: `all` (boolean), +`reason` (`selection-rule change` when a changed line sits outside any job or inside +`changes` or `report`, else empty), and `ci-jobs`. + +`ci-jobs` lists every job whose block holds a changed line that is neither blank nor a +comment, plus every job those jobs need. `.golangci.yml` adds `lint`, and any `.github/` +or `.golangci.yml` change adds `workflow-lint`. Lines are read from `git diff -U0`: +removed lines map against the base file and added lines against the head file. Each +listed job's gate is ORed with `contains(fromJSON(needs.changes.outputs.ci-jobs), '')`. +Modes `modules`, `units` and `e2e` widen the job's own selection, so the job runs all of +its legs, packages or tests. An e2e job with an `-arm64` twin brings the twin too +(FR-010). These still force everything (FR-014): `hack/ci_scope.py`, +`hack/test_ci_scope.py`, `hack/check_changed_coverage.py` and its test, `Makefile`, +`go.work`, `go.work.sum`, `.github/actions/**`, `test/e2e/buckets.sh`, +`test/e2e/suite-components.json`. + ## Mode `bots` — changed Emits only the selected `bot-fast` tests. When none is selected it emits an empty diff --git a/specs/023-change-based-test-selection/tasks.md b/specs/023-change-based-test-selection/tasks.md index 191699657..595a6e5b5 100644 --- a/specs/023-change-based-test-selection/tasks.md +++ b/specs/023-change-based-test-selection/tasks.md @@ -114,20 +114,20 @@ Repository root of `GameplanePanel/Gameplane`. Main files: `hack/ci_scope.py`, ` ### Tests for User Story 2 -- [ ] T031 [P] [US2] Add `hack/test_ci_scope.py` cases for the package graph: leaf change, importer chain across modules, `_test.go`-only change, `testdata/` and embedded file change, module `go.mod` change giving `./...` -- [ ] T032 [P] [US2] Add `hack/test_ci_scope.py` cases for `web-tests`: a route file gives its source path, a change under `web/src/test/`, `web/package.json`, `package-lock.json`, `vitest.config.ts`, `vite.config.ts`, `tsconfig*.json` or `eslint.config.js` gives `all` -- [ ] T033 [P] [US2] Write `hack/test_check_changed_coverage.py` (unittest) with fixture profiles: Go atomic profile covering/missing added lines, Istanbul JSON for web, zero-statement diff passes, threshold read from `.testcoverage.yml` and `vitest.config.ts`, missing threshold fails; extend the `test-ci-scope` target in `Makefile` to discover `test_*.py` in `hack/` +- [X] T031 [P] [US2] Add `hack/test_ci_scope.py` cases for the package graph: leaf change, importer chain across modules, `_test.go`-only change, `testdata/` and embedded file change, module `go.mod` change giving `./...` +- [X] T032 [P] [US2] Add `hack/test_ci_scope.py` cases for `web-tests`: a route file gives its source path, a change under `web/src/test/`, `web/package.json`, `package-lock.json`, `vitest.config.ts`, `vite.config.ts`, `tsconfig*.json` or `eslint.config.js` gives `all` +- [X] T033 [P] [US2] Write `hack/test_check_changed_coverage.py` (unittest) with fixture profiles: Go atomic profile covering/missing added lines, Istanbul JSON for web, zero-statement diff passes, threshold read from `.testcoverage.yml` and `vitest.config.ts`, missing threshold fails; add a second `unittest discover` line for `test_check_changed_coverage.py` to the `test-ci-scope` target in `Makefile` (a `test_*.py` glob would also run `test_auto_label.py`, which has its own workflow) ### Implementation for User Story 2 -- [ ] T034 [US2] In `hack/ci_scope.py`, build the package import graph by scanning `import` blocks of every `.go` file (including `_test.go`) in the selected modules, map changed files (including `testdata/` and embedded files) to their package directory, compute reverse closure, extend `file_consumers` for cross-package file reads, and emit `go-packages` (research R4, OD-8) -- [ ] T035 [US2] In `hack/ci_scope.py`, emit `web-tests` per research R6 (OD-9) -- [ ] T036 [US2] Write `hack/check_changed_coverage.py` per contracts/changed-coverage.md (threshold from `.testcoverage.yml` `threshold.total` or `vitest.config.ts` `lines`, OD-5) -- [ ] T037 [US2] In the `go` job of `.github/workflows/ci.yaml` (lines ~621-674), replace `./...` in the unit and envtest `go test` commands with the module's `go-packages` list, keeping `-coverpkg=./...`; on `scope == full` keep `make cover-go-merge && make cover-go-check`, on `partial` merge profiles and run `hack/check_changed_coverage.py --go-profile ... --module

` instead -- [ ] T038 [US2] In the `web` job of `.github/workflows/ci.yaml` (lines ~676-733), keep `npm run lint` and `npm run build`; on partial scope with a file list run `npx vitest related --run --coverage --coverage.thresholds.lines=0 --coverage.thresholds.functions=0 --coverage.thresholds.branches=0 --coverage.thresholds.statements=0 ` then `hack/check_changed_coverage.py --vitest-json web/coverage/coverage-final.json`; otherwise run `npm run test:cover` unchanged. `web-e2e-mock` stays whole (OD-9) -- [ ] T039 [US2] Extend the `selection` records with per-module package counts and the web related-file count -- [ ] T040 [US2] Compile checks only; open PR 3 with labels `type: ci`, `area: shared`, `area: web` -- [ ] T041 [US2] Run quickstart scenarios 5–8 as draft PRs on PR 3's branch, record results in `validation.md`, close them and delete their branches +- [X] T034 [US2] In `hack/ci_scope.py`, build the package import graph by scanning `import` blocks of every `.go` file (including `_test.go`) in the selected modules, map changed files (including `testdata/` and embedded files) to their package directory, compute reverse closure, extend `file_consumers` for cross-package file reads, and emit `go-packages` (research R4, OD-8) +- [X] T035 [US2] In `hack/ci_scope.py`, emit `web-tests` per research R6 (OD-9) +- [X] T036 [US2] Write `hack/check_changed_coverage.py` per contracts/changed-coverage.md (threshold from `.testcoverage.yml` `threshold.total` or `vitest.config.ts` `lines`, OD-5) +- [X] T037 [US2] In the `go` job of `.github/workflows/ci.yaml` (lines ~621-674), replace `./...` in the unit and envtest `go test` commands with the module's `go-packages` list, keeping `-coverpkg=./...`; on `scope == full` keep `make cover-go-merge && make cover-go-check`, on `partial` merge profiles and run `hack/check_changed_coverage.py --go-profile ... --module ` instead +- [X] T038 [US2] In the `web` job of `.github/workflows/ci.yaml` (lines ~676-733), keep `npm run lint` and `npm run build`; on partial scope with a file list run `npx vitest related --run --coverage --coverage.thresholds.lines=0 --coverage.thresholds.functions=0 --coverage.thresholds.branches=0 --coverage.thresholds.statements=0 ` then `hack/check_changed_coverage.py --vitest-json web/coverage/coverage-final.json`; otherwise run `npm run test:cover` unchanged. `web-e2e-mock` stays whole (OD-9) +- [X] T039 [US2] Extend the `selection` records with per-module package counts and the web related-file count +- [X] T040 [US2] Compile checks only; open PR 3 with labels `type: ci`, `area: shared`, `area: web` +- [X] T041 [US2] Run quickstart scenarios 5–8 as draft PRs on PR 3's branch, record results in `validation.md`, close them and delete their branches **Checkpoint**: unit selection and changed-lines coverage work; full runs keep module gates. @@ -141,15 +141,15 @@ Repository root of `GameplanePanel/Gameplane`. Main files: `hack/ci_scope.py`, ` ### Tests for User Story 3 -- [ ] T042 [P] [US3] Add `hack/test_ci_scope.py` cases: comment-only `ci.yaml` change selects only `workflow-lint`; a step change inside job `web` selects `web` and `workflow-lint`; a change in `on:`, `env:`, `concurrency:`, `permissions:`, or inside the `changes` or `report` job forces full scope with reason `selection-rule change`; `.golangci.yml` selects only `lint` legs; `hack/ci_scope.py`, `Makefile`, `go.work`, `go.work.sum`, `.github/actions/**` still force full scope +- [X] T042 [P] [US3] Add `hack/test_ci_scope.py` cases: comment-only `ci.yaml` change selects only `workflow-lint`; a step change inside job `web` selects `web` and `workflow-lint`; a change in `on:`, `env:`, `concurrency:`, `permissions:`, or inside the `changes` or `report` job forces full scope with reason `selection-rule change`; `.golangci.yml` selects only `lint` legs; `hack/ci_scope.py`, `Makefile`, `go.work`, `go.work.sum`, `.github/actions/**` still force full scope ### Implementation for User Story 3 -- [ ] T043 [US3] In `hack/ci_scope.py`, add the `ci.yaml` line mapper from research R8 (job blocks by two-space keys under `jobs:`, blank/comment lines ignored) using `git diff -U0` line numbers, and emit `ci-jobs` -- [ ] T044 [US3] In `hack/ci_scope.py`, narrow the shared-file list per OD-3: remove `.github/` and `.golangci.yml` from the force-all set, map them per research R8, keep `hack/ci_scope*`, `hack/check_changed_coverage.py`, `Makefile`, `go.work*`, `.github/actions/**`, `test/e2e/buckets.sh`, `test/e2e/suite-components.json` as force-all -- [ ] T045 [US3] In `.github/workflows/ci.yaml`, OR each job's existing gate with `contains(fromJSON(needs.changes.outputs.ci-jobs), '')`, keeping `workflow-lint` selected for any `.github/` change -- [ ] T046 [US3] Compile checks only; open PR 4 with labels `type: ci`, `area: shared` -- [ ] T047 [US3] Run quickstart scenarios 9–11 as draft PRs on PR 4's branch, record runner-minutes for scenario 9 (SC-003) in `validation.md`, close them and delete their branches +- [X] T043 [US3] In `hack/ci_scope.py`, add the `ci.yaml` line mapper from research R8 (job blocks by two-space keys under `jobs:`, blank/comment lines ignored) using `git diff -U0` line numbers, and emit `ci-jobs` +- [X] T044 [US3] In `hack/ci_scope.py`, narrow the shared-file list per OD-3: remove `.github/` and `.golangci.yml` from the force-all set, map them per research R8, keep `hack/ci_scope*`, `hack/check_changed_coverage.py`, `Makefile`, `go.work*`, `.github/actions/**`, `test/e2e/buckets.sh`, `test/e2e/suite-components.json` as force-all +- [X] T045 [US3] In `.github/workflows/ci.yaml`, OR each job's existing gate with `contains(fromJSON(needs.changes.outputs.ci-jobs), '')`, keeping `workflow-lint` selected for any `.github/` change +- [X] T046 [US3] Compile checks only; open PR 4 with labels `type: ci`, `area: shared` +- [X] T047 [US3] Run quickstart scenarios 9–11 as draft PRs on PR 4's branch, record runner-minutes for scenario 9 (SC-003) in `validation.md`, close them and delete their branches **Checkpoint**: all four stories work independently. @@ -157,9 +157,9 @@ Repository root of `GameplanePanel/Gameplane`. Main files: `hack/ci_scope.py`, ` ## Phase 7: Polish & Docs — PR 5 -- [ ] T048 [P] Add a "How CI picks tests" section to `docs/contributing.md`: what a partial run skips, the `ci: full-run` label and manual run, how to add a `suite-components.json` entry, where to read the run summary -- [ ] T049 [P] Add the new selection files to the task map in `docs/agent-architecture.md` -- [ ] T050 Compile and link checks only; open PR 5 with labels `type: docs`, `area: shared` +- [X] T048 [P] Add a "How CI picks tests" section to `docs/contributing.md`: what a partial run skips, the `ci: full-run` label and manual run, how to add a `suite-components.json` entry, where to read the run summary +- [X] T049 [P] Add the new selection files to the task map in `docs/agent-architecture.md` +- [X] T050 Compile and link checks only; open PR 5 with labels `type: docs`, `area: shared` - [ ] T051 After the first `master` push following PR 5's merge, confirm every job ran (FR-006, SC-006) and record it in `validation.md` - [ ] T052 After 30 merged PRs, measure median runner-minutes against the 30 before PR 2 via the Actions API and record SC-002 in `validation.md`; for 60 days review each red `master` run for a selection miss (SC-004) diff --git a/specs/023-change-based-test-selection/validation.md b/specs/023-change-based-test-selection/validation.md index 04dfb5c5c..5725ffe81 100644 --- a/specs/023-change-based-test-selection/validation.md +++ b/specs/023-change-based-test-selection/validation.md @@ -75,3 +75,34 @@ the cause is the agreed `suite-components.json` mapping, not a code defect. Sett OD-12 in [OPEN-DECISIONS.md](./OPEN-DECISIONS.md): the mapping stays, and US1-1, US1-2 and quickstart rows 1-2 were reworded to match it. With the new wording, scenarios 1 and 2 pass. + +## T041 and T047: scenarios 5–11 (2026-10-22) + +Each scenario was a throwaway draft PR based on PR 3's branch at `4b90043` (PRs 3–5 +combined), allowed by OD-11. Results come from each run's `detect changes` outputs and +the jobs that ran. The drafts were closed once their selection was read; their queued +e2e jobs were cancelled because the selection, not the e2e result, is what each scenario +checks. + +| # | Draft PR | Change | Scope | Result | +|---|---|---|---|---| +| 5 | #787 | comment in `api/internal/steam/options.go` | partial | `go api` tested `./cmd ./internal/messages ./internal/steam` (the package and its importers); other modules empty; changed-lines gate line `changed-lines coverage: no added statement lines, passes` (US2-1, US2-2 met) | +| 6 | #788 | scenario 5 plus `var scenarioSixBreaksTheBuild int = "not an int"` in `api/cmd/main.go` | partial | same packages selected; `go (api)` failed in `go vet` with `cmd/main.go:916:37: cannot use "not an int"` (US2 independent test met) | +| 7 | #789 | new file `api/internal/steam/testdata/scenario7.txt` | partial | `go api` tested `./cmd ./internal/messages ./internal/steam`, so the owning package ran (US2-3 met) | +| 8 | #790 | comment in `web/src/routes/AdminLogs.tsx` | partial | `web-tests` = `["web/src/routes/AdminLogs.tsx"]`; Vitest `Affected 2 of 175 test files`, 36 tests passed; changed-lines gate passed; no Go job (US2-4 met) | +| 9 | #791 | comment line above `jobs:` in `ci.yaml` | partial | `ci-jobs` = `["workflow-lint"]`; jobs that ran: detect changes, workflow lint, ingress-nginx smoke, dependency review, telemetry default gate, ci report; about 3 runner-minutes (SC-003 met) | +| 10 | #792 | comment in `hack/ci_scope.py` | full: `selection-rule change` | every module `./...`, `web-tests` `"all"`, every e2e test selected, module coverage gates ran (`go api` 84.2% against 80%) (US3-2, FR-011 met) | +| 11 | #793 | scenario 1 (comment in `agent/internal/console/console.go`), then label `ci: full-run` and a second push | first push partial; second push full: `label ci: full-run` | second run: `all: true`, reason `label ci: full-run` in the e2e and units outputs, 69 jobs scheduled, every e2e suite selected (FR-007 met) | + +**Scenario 9.** The ingress-nginx smoke ran because the base still listed `ci.yaml` in +the `kindsmoke` path filter. Commit `d4ee6ac` removed that entry, so a comment-only +`ci.yaml` edit now runs only `workflow lint` plus the always-on jobs. The six +`TestCISelection_*` tests appeared in `e2e-run`, but the e2e gate was off, so no e2e job +ran. + +**Scenarios 5 and 7, e2e.** An `api` change selects most e2e tests (`operator` 64 of 78, +every API bucket whole), because nearly every e2e test goes through the API. This +follows the agreed `suite-components.json` mapping (OD-12). + +**Scenario 7.** A `testdata/` file is not a `_test.go` file, so its package's importers +are selected too. That is broader than US2-3 asks for, never narrower. diff --git a/test/e2e/ci_selection_e2e_test.go b/test/e2e/ci_selection_e2e_test.go index a62906016..d2d6edd00 100644 --- a/test/e2e/ci_selection_e2e_test.go +++ b/test/e2e/ci_selection_e2e_test.go @@ -34,10 +34,14 @@ type ciSelection struct { } // ciGit runs git in dir with a fixed identity and returns trimmed stdout. +// Background gc and maintenance are off: committing a whole checkout leaves +// enough loose objects to start them, and they would still be writing into +// .git when t.TempDir removes it. func ciGit(t *testing.T, dir string, args ...string) string { t.Helper() base := []string{"-C", dir, "-c", "user.name=ci-selection", - "-c", "user.email=ci-selection@example.invalid", "-c", "commit.gpgsign=false"} + "-c", "user.email=ci-selection@example.invalid", "-c", "commit.gpgsign=false", + "-c", "gc.auto=0", "-c", "maintenance.auto=false"} out, err := exec.CommandContext(t.Context(), "git", append(base, args...)...).CombinedOutput() if err != nil { t.Fatalf("git %s: %v\n%s", strings.Join(args, " "), err, out)