diff --git a/.github/workflows/pr-unit.yml b/.github/workflows/pr-unit.yml index 74e375c11..de7db4f1f 100644 --- a/.github/workflows/pr-unit.yml +++ b/.github/workflows/pr-unit.yml @@ -12,24 +12,54 @@ on: - reopened jobs: + determine-affected: + name: "Determine Affected Packages" + if: ${{ !contains(github.event.pull_request.labels.*.name, 'ci/skip-unit') }} + runs-on: ubuntu-latest + outputs: + packages: ${{ steps.affected.outputs.packages }} + has_packages: ${{ steps.affected.outputs.has_packages }} + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + fetch-depth: 0 + - name: Setup Node.js + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 + with: + node-version: "18" + - name: Install dependencies + run: yarn install --frozen-lockfile --prefer-offline + - name: Determine affected packages + id: affected + run: | + if [[ "${{ contains(github.event.pull_request.labels.*.name, 'ci/run-all-unit') }}" == "true" ]]; then + echo "Label ci/run-all-unit detected, running all packages" + PACKAGES='["cdktn","cdktn-cli","@cdktn/hcl2cdk","@cdktn/hcl2json","@cdktn/provider-schema","@cdktn/provider-generator","@cdktn/commons","@cdktn/cli-core"]' + else + if [[ "${{ github.event_name }}" == "pull_request" ]]; then + BASE_SHA="${{ github.event.pull_request.base.sha }}" + else + BASE_SHA="${{ github.event.merge_group.base_sha }}" + fi + PACKAGES=$(bash tools/affected-packages.sh "$BASE_SHA") + fi + echo "packages=$PACKAGES" >> "$GITHUB_OUTPUT" + if [[ "$PACKAGES" == "[]" ]]; then + echo "has_packages=false" >> "$GITHUB_OUTPUT" + else + echo "has_packages=true" >> "$GITHUB_OUTPUT" + fi + echo "Affected packages: $PACKAGES" + all_unit_tests: name: "All Unit Tests" - if: ${{ !contains(github.event.pull_request.labels.*.name, 'ci/skip-unit') }} + needs: determine-affected + if: ${{ needs.determine-affected.outputs.has_packages == 'true' }} uses: ./.github/workflows/unit.yml strategy: fail-fast: false matrix: - package: - [ - cdktn, - cdktn-cli, - "@cdktn/hcl2cdk", - "@cdktn/hcl2json", - "@cdktn/provider-schema", - "@cdktn/provider-generator", - "@cdktn/commons", - "@cdktn/cli-core", - ] + package: ${{ fromJSON(needs.determine-affected.outputs.packages) }} terraform_version: ["1.6.5", "1.5.5"] with: concurrency_group_prefix: pr-all @@ -141,6 +171,7 @@ jobs: name: Unit Tests - PR - Result needs: [ + determine-affected, all_unit_tests, cdktn, cdktn_cli, diff --git a/RFCs/CI-IMPROVEMENTS.md b/RFCs/CI-IMPROVEMENTS.md new file mode 100644 index 000000000..f2e47d6f9 --- /dev/null +++ b/RFCs/CI-IMPROVEMENTS.md @@ -0,0 +1,90 @@ +# Plan: Affected-Only Unit Tests in CI + +## Context + +Every PR currently runs **16 unit test jobs** (8 packages × 2 Terraform versions) regardless of what changed. A PR that only touches `CHANGELOG.md` or `.github/` still triggers all 250+ tests. This wastes CI minutes and slows down reviews. + +The repo already has **Nx 20.8.2** installed with a project graph that correctly resolves workspace dependencies. `nx show projects --affected` works out of the box — verified locally that it correctly identifies `@cdktn/cli-core` + `cdktn-cli` + downstream examples when only `cli-core` was changed, and returns empty for non-package file changes. + +## Approach: Nx `affected` with dynamic matrix + +Use `nx show projects --affected` in a pre-job to build a dynamic matrix for `pr-unit.yml`. No new dependencies needed. + +**Why Nx over alternatives:** + +- Already installed and working (v20.8.2) +- Walks the dependency graph transitively (Lerna `--since` does not) +- No migration needed (Turborepo would require replacing Lerna) + +## Files to Modify + +1. **`.github/workflows/pr-unit.yml`** — add `determine-affected` job, make `all_unit_tests` matrix dynamic +2. **`tools/affected-packages.sh`** (new) — shell script to compute affected testable packages with fallback + +## Implementation + +### Step 1: Create `tools/affected-packages.sh` + +Script that: + +- Runs `npx nx show projects --affected --base=$BASE_REF --head=HEAD` +- Filters output to only the 8 testable packages (excludes examples, hcl-tools, etc.) +- Outputs a JSON array for GitHub Actions matrix consumption +- **Fallback**: if Nx fails or errors, outputs all 8 packages (safe default) +- **Empty case**: if no packages affected, outputs `[]` + +### Step 2: Modify `pr-unit.yml` + +Add a new `determine-affected` job before `all_unit_tests`: + +``` +determine-affected (ubuntu-latest, ~30s) + ├─ checkout with fetch-depth: 0 + ├─ setup node + yarn install + ├─ if label 'ci/run-all-unit' → output all 8 packages + ├─ else → run tools/affected-packages.sh with PR base SHA + └─ outputs: packages (JSON array), has_packages (bool) +``` + +Modify `all_unit_tests`: + +- Add `needs: determine-affected` +- Add condition: `has_packages == 'true'` +- Replace hardcoded `package` matrix with `fromJSON(needs.determine-affected.outputs.packages)` +- Keep `terraform_version: ["1.6.5", "1.5.5"]` hardcoded + +Handle `merge_group` vs `pull_request`: + +- PR: `base_sha = github.event.pull_request.base.sha` +- merge_group: `base_sha = github.event.merge_group.base_sha` + +Add `determine-affected` to the `results` job `needs` list. + +### Step 3: Add `ci/run-all-unit` label override + +In the `determine-affected` job, check for label and short-circuit to all packages. This preserves manual control for cases where you want to force full test coverage. + +## What stays unchanged + +- **`unit.yml`** (reusable workflow) — no changes +- **All per-package label-based jobs** (`cdktn`, `cdktn_cli`, etc.) — kept as-is +- **Integration/provider/example workflows** — out of scope for this change (can add path filtering later) +- **Build caching, concurrency groups** — unchanged + +## Expected Impact + +| PR touches | Jobs before | Jobs after | +| ------------------------------- | ----------- | ------------------------ | +| Only docs/CI/changelog | 16 | 0 | +| Single leaf (`cdktn-cli`) | 16 | 2 (1 pkg × 2 TF) | +| `@cdktn/hcl2json` | 16 | 10 (5 pkgs × 2 TF) | +| Core `cdktn` lib | 16 | 16 (all affected) | +| Root `package.json`/`yarn.lock` | 16 | 16 (Nx treats as global) | + +## Verification + +1. Run `bash tools/affected-packages.sh origin/main` locally on various branches to verify output +2. Open a docs-only PR → confirm 0 unit test jobs run +3. Open a PR touching `@cdktn/commons` → confirm only `commons` + dependents are tested +4. Apply `ci/run-all-unit` label → confirm all 16 jobs run +5. Verify `results` job still correctly reports pass/fail diff --git a/tools/affected-packages.sh b/tools/affected-packages.sh new file mode 100755 index 000000000..153a86374 --- /dev/null +++ b/tools/affected-packages.sh @@ -0,0 +1,65 @@ +#!/usr/bin/env bash +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: MPL-2.0 + +# Determine which testable packages are affected by changes between BASE and HEAD. +# Outputs a JSON array suitable for GitHub Actions matrix consumption. +# +# Usage: +# bash tools/affected-packages.sh +# bash tools/affected-packages.sh origin/main +# +# Output examples: +# ["cdktn","cdktn-cli","@cdktn/commons"] # some packages affected +# [] # nothing affected + +set -euo pipefail + +BASE_REF="${1:?Usage: affected-packages.sh }" + +# The 8 packages that have unit tests +TESTABLE_PACKAGES=( + "cdktn" + "cdktn-cli" + "@cdktn/hcl2cdk" + "@cdktn/hcl2json" + "@cdktn/provider-schema" + "@cdktn/provider-generator" + "@cdktn/commons" + "@cdktn/cli-core" +) + +all_packages_json() { + printf '%s\n' "${TESTABLE_PACKAGES[@]}" | jq -Rsc 'split("\n") | map(select(length > 0))' +} + +# Fallback: return all packages on any error +fallback() { + echo "::warning::Nx affected detection failed, falling back to all packages" >&2 + all_packages_json + exit 0 +} +trap fallback ERR + +# Get affected projects from Nx +AFFECTED=$(npx nx show projects --affected --base="$BASE_REF" --head=HEAD 2>/dev/null) + +if [[ -z "$AFFECTED" ]]; then + # No projects affected at all + echo "[]" + exit 0 +fi + +# Filter to only testable packages +MATCHED=() +for pkg in "${TESTABLE_PACKAGES[@]}"; do + if echo "$AFFECTED" | grep -qx "$pkg"; then + MATCHED+=("$pkg") + fi +done + +if [[ ${#MATCHED[@]} -eq 0 ]]; then + echo "[]" +else + printf '%s\n' "${MATCHED[@]}" | jq -Rsc 'split("\n") | map(select(length > 0))' +fi