diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6084d20c7f..bfea2f6a02 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,27 +2,27 @@ # PRs test what they touch; main and nightly test everything. On a pull request the "Select # checks" job maps the changed files to workspace packages (scripts/ci/affected.py), adds the # packages that depend on them, and Clippy, the test archive and the test shards cover only -# those. Changes to the lockfile, root manifest, toolchain/cargo/nextest config, .github/, -# vendor/, tests/, scripts/ or any file no package owns select the whole workspace, as do -# pushes to main, the nightly run and Run workflow. +# those. Test data under tests/, vendored crates under vendor/ and root files a crate embeds +# select the packages that read them. Changes to the lockfile, root manifest, +# toolchain/cargo/nextest config, this workflow, scripts/ or any file no package owns select the +# whole workspace, as do pushes to main that change code, the nightly run and Run workflow. +# Documentation, packaging, the other workflows and the GitHub bots' scripts select nothing: +# no native job runs for them, on pull requests or on main. # Tests are built once and run in shards. A few heavy tests (see .config/nextest.toml) are # skipped on pull requests and run on pushes to main and nightly. The Windows jobs (D3D12 shaders # and app startup), the reference tests and the doc tests run on main and nightly only, not on # pull requests (the fast lane, in Select checks); a break there is fixed forward on main. Only -# main saves the build caches; pull requests restore them. The test build, the shards and the doc -# tests start only after the format and repository checks pass. -# The merge queue runs the full suite on exactly the commit it then puts on main, so a push to -# main that a passing merge-queue run already tested runs only the repository checks. A push that -# skipped the queue runs everything when it changes code; one that changes only documentation, -# packaging, other workflows or the GitHub bots' scripts (the same rule as pull requests, -# `touches_code` in Select checks) runs only the repository checks. Test data under tests/ and -# vendored crates under vendor/ select the packages that read them (scripts/ci/affected.py). +# main saves the build caches; pull requests restore them. On pull requests the test build and the +# shards start only after the format and repository checks pass. +# A push to main runs only the repository checks when a passing merge-queue run tested that very +# commit, or when nothing a build reads changed since the last push to main whose run passed. +# Otherwise it runs everything. # Heavy checks wait for review: a pull request without the `approved-for-ci` label runs only # Select checks, the format and repository checks and a quick compile. Adding the label (after # review) runs everything above, and every later push to the PR does too, except a push that only # merges main into a change that already passed (scripts/ci/main_merge.py): it carries that -# result and runs the repository checks. Pushes to main, merge -# groups, the nightly run and Run workflow are not pull requests and always run the full suite. +# result and runs the repository checks. Merge groups, the nightly run and Run workflow are not +# pull requests and always run the full suite. # GitHub cannot filter `labeled` events by label name, so any other label starts a run whose jobs # are all skipped; it gets its own concurrency group (below) so it never cancels the real run. # Cross-compilation is opt-in through Run workflow; release builds remain in release.yml. @@ -130,7 +130,6 @@ jobs: BASE_SHA: ${{ github.event.pull_request.base.sha }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} PUSH_SHA: ${{ github.sha }} - BEFORE_SHA: ${{ github.event.before }} REPO: ${{ github.repository }} APPROVED: ${{ contains(github.event.pull_request.labels.*.name, 'approved-for-ci') }} GH_TOKEN: ${{ github.token }} @@ -141,42 +140,38 @@ jobs: windows=false full=true packages= - # Whether any of the NUL-separated paths on stdin is something a build or test reads. - # Pull requests and pushes to main share it, so a docs-only change runs no native job - # on either. - touches_code() { - local path - while IFS= read -r -d '' path; do - case "$path" in - .github/workflows/ci.yml) return 0 ;; - *.rs|Cargo.toml|Cargo.lock|*/Cargo.toml|.cargo/*|rust-toolchain*) return 0 ;; - docs/*|*.md|.github/*|packaging/*|LICENSE*|NOTICE|THIRD-PARTY-LICENSES.txt) ;; - # The GitHub bots' scripts: no Rust, tested in this job (Claim and triage bot tests). - scripts/claim/*|scripts/triage/*) ;; - *) return 0 ;; - esac - done - return 1 + # Whether the NUL-separated paths in $1 reach any code: scripts/ci/affected.py selects + # something or the whole workspace. Documentation, packaging, other workflows and the bots' + # scripts select nothing. Pull requests and pushes to main share it. + select_packages() { + python3 -I scripts/ci/affected.py --paths "$1" > "$RUNNER_TEMP/ci-affected" + cat "$RUNNER_TEMP/ci-affected" + full=$(sed -n 's/^full=//p' "$RUNNER_TEMP/ci-affected") + packages=$(sed -n 's/^packages=//p' "$RUNNER_TEMP/ci-affected") } if [ "$EVENT_NAME" = push ]; then # The queue lands the very commit its merge_group run tested. If that run passed, # testing the push again proves nothing new; otherwise (a merge that skipped the # queue, or the API cannot answer) test everything. - # curl and grep only: Select checks of a push runs on the self-hosted runner. + # The API through curl and grep: Select checks of a push runs on the self-hosted runner. tested=$(curl -fsS -H "Authorization: Bearer $GH_TOKEN" -H "Accept: application/vnd.github+json" \ "https://api.github.com/repos/$REPO/actions/workflows/ci.yml/runs?event=merge_group&head_sha=$PUSH_SHA&status=success&per_page=1" \ | grep -o '"total_count": *[0-9]*' | grep -o '[0-9]*$' || echo 0) if [ "${tested:-0}" -gt 0 ]; then echo "The merge queue already tested $PUSH_SHA: repository checks only." - elif [ -n "$BEFORE_SHA" ] && [ "${BEFORE_SHA#0000000}" = "$BEFORE_SHA" ] \ - && git cat-file -e "$BEFORE_SHA^{commit}" 2>/dev/null \ - && git diff --no-renames --name-only -z "$BEFORE_SHA" "$PUSH_SHA" > "$RUNNER_TEMP/ci-paths" \ - && ! touches_code < "$RUNNER_TEMP/ci-paths"; then - # Nothing a build reads changed (documentation, packaging, other workflows, the bots' - # scripts): the code on main is what the last push already tested. - echo "$BEFORE_SHA..$PUSH_SHA changes no code: repository checks only." + elif tested=$(curl -fsS -H "Authorization: Bearer $GH_TOKEN" -H "Accept: application/vnd.github+json" \ + "https://api.github.com/repos/$REPO/actions/workflows/ci.yml/runs?event=push&branch=main&status=success&per_page=1" \ + | grep -o '"head_sha": *"[0-9a-f]*"' | head -n 1 | grep -o '[0-9a-f]\{40\}') \ + && [ -n "$tested" ] && git merge-base --is-ancestor "$tested" "$PUSH_SHA" \ + && git diff --no-renames --name-only -z "$tested" "$PUSH_SHA" > "$RUNNER_TEMP/ci-paths" \ + && select_packages "$RUNNER_TEMP/ci-paths" && [ "$full" != true ] && [ -z "$packages" ]; then + # Nothing a build reads changed since the last push to main whose run passed (the + # push before may never have run: a newer push cancels a waiting one). + echo "$tested..$PUSH_SHA changes no code: repository checks only." else # Code changed (or the push can't be compared): main runs everything. + full=true + packages= native=true windows=true fi @@ -190,7 +185,7 @@ jobs: case "$path" in crates/kernel/*|vendor/truck-*) kernel=true ;; esac # What can change a robustness golden: the kernel and the crates a design is built with. case "$path" in - crates/kernel/*|crates/engine/*|crates/doc/*|crates/sketch/*|crates/geom/*|vendor/truck-*) goldens=true ;; + crates/kernel/*|crates/engine/*|crates/doc/*|crates/sketch/*|crates/geom/*|vendor/truck-*|tests/robustness/*) goldens=true ;; esac # What the Windows D3D12 jobs exercise: the GPU crate and its shaders, the app and # its startup, the viewport's wgpu glue, the vendored egui-wgpu, their scripts. @@ -199,20 +194,16 @@ jobs: tools/windows-*.ps1|.github/workflows/ci.yml|rust-toolchain*|.cargo/*) windows=true ;; esac done < "$RUNNER_TEMP/ci-paths" - if touches_code < "$RUNNER_TEMP/ci-paths"; then native=true; fi # A lockfile change that moves the graphics stack (wgpu, naga, egui, winit, windows). if [ "$windows" = false ] && grep -qxz 'Cargo.lock' "$RUNNER_TEMP/ci-paths" \ && git diff "$BASE_SHA...$HEAD_SHA" -- Cargo.lock \ | grep -Eq '^[ +-]name = "(wgpu[a-z_-]*|naga|egui[a-z_-]*|eframe|winit|windows[a-z_-]*|d3d12)"'; then windows=true fi - # Pull requests build and test the touched packages and their dependents. - python3 -I scripts/ci/affected.py --paths "$RUNNER_TEMP/ci-paths" > "$RUNNER_TEMP/ci-affected" - cat "$RUNNER_TEMP/ci-affected" - full=$(sed -n 's/^full=//p' "$RUNNER_TEMP/ci-affected") - packages=$(sed -n 's/^packages=//p' "$RUNNER_TEMP/ci-affected") - # Code changed but nothing was selected: be safe and test everything. - if [ "$native" = true ] && [ "$full" != true ] && [ -z "$packages" ]; then full=true; fi + # Pull requests build and test the touched packages and their dependents; nothing + # selected means no code changed. + select_packages "$RUNNER_TEMP/ci-paths" + if [ "$full" = true ] || [ -n "$packages" ]; then native=true; fi # The goldens are solidcraft-engine tests: its tests must be in the archive. if [ "$goldens" = true ] && [ "$full" != true ]; then case " $packages " in *" solidcraft-engine "*) ;; *) packages="$packages solidcraft-engine" ;; esac @@ -361,12 +352,14 @@ jobs: build-tests: name: Build tests - # Fail fast: the test archive (and so the shards) waits for the repository checks, the most - # common cheap failure; heavy jobs used to run on to the end beside a failed format check. + # Fail fast on pull requests: the test archive (and so the shards) waits for the repository + # checks, the most common cheap failure; heavy jobs used to run on to the end beside a failed + # format check. Main, nightly and Run workflow build regardless, so the action matrix and the + # property tests that hang off this job keep their signal. needs: [changes, checks] # The self-hosted shards (main and the merge queue) build from their warm build directories # instead: downloading the 3 GB archive took them longer than building. - if: needs.changes.outputs.native == 'true' && needs.changes.outputs.local != 'true' + if: ${{ !cancelled() && needs.changes.result == 'success' && needs.changes.outputs.native == 'true' && needs.changes.outputs.local != 'true' && (github.event_name != 'pull_request' || needs.checks.result == 'success') }} runs-on: ${{ (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && (endsWith(format('{0}', github.event.pull_request.number), '7') || endsWith(format('{0}', github.event.pull_request.number), '8') || endsWith(format('{0}', github.event.pull_request.number), '9'))) && 'self-hosted-untrusted' || 'ubuntu-24.04' }} # Fork builds compile inside gVisor on shared boxes; give them room to finish and upload. timeout-minutes: ${{ (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && (endsWith(format('{0}', github.event.pull_request.number), '7') || endsWith(format('{0}', github.event.pull_request.number), '8') || endsWith(format('{0}', github.event.pull_request.number), '9'))) && 90 || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository) && 60 || 45 }} @@ -413,7 +406,7 @@ jobs: tests: name: Native Linux tests (${{ matrix.shard }}/4) needs: [changes, checks, build-tests] - if: ${{ !cancelled() && needs.changes.outputs.native == 'true' && needs.checks.result == 'success' && (needs.changes.outputs.local == 'true' || needs.build-tests.result == 'success') }} + if: ${{ !cancelled() && needs.changes.outputs.native == 'true' && (github.event_name != 'pull_request' || needs.checks.result == 'success') && (needs.changes.outputs.local == 'true' || needs.build-tests.result == 'success') }} runs-on: ${{ needs.changes.outputs.local == 'true' && 'self-hosted-fast' || ((github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && (endsWith(format('{0}', github.event.pull_request.number), '7') || endsWith(format('{0}', github.event.pull_request.number), '8') || endsWith(format('{0}', github.event.pull_request.number), '9'))) && 'self-hosted-untrusted' || 'ubuntu-24.04') }} timeout-minutes: 60 strategy: @@ -599,8 +592,7 @@ jobs: doc-tests: name: Doc tests - # After the repository checks pass (see Build tests). - needs: [changes, checks] + needs: changes if: needs.changes.outputs.doctests == 'true' runs-on: ${{ needs.changes.outputs.local == 'true' && 'self-hosted-fast' || 'ubuntu-24.04' }} timeout-minutes: 45 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index aeb2b314f7..b490d3bbc8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -183,10 +183,12 @@ depend on wall-clock timing. **Ready pull requests test what they touch.** CI maps changed files to workspace packages and runs Clippy, the test build and tests for those and their dependents. The [package selector](scripts/ci/affected.py) and [CI workflow](.github/workflows/ci.yml) define -which changes require native checks; documentation, packaging and bot-script-only PRs can run -repository checks only. Workspace-wide changes, merge-queue groups, nightly runs and Run -workflow select the full workspace. A push to `main` reuses a successful merge-queue run for -the same commit; otherwise it runs the full suite. +which changes require native checks; documentation, packaging, other-workflow and bot-script-only +changes run the repository checks only, on pull requests and on `main`. Workspace-wide changes, +merge-queue groups, nightly runs and Run workflow select the full workspace. A push to `main` +reuses a successful merge-queue run for the same commit, and runs only the repository checks +when no code changed since the last push to `main` that passed; otherwise it runs the full +suite. Doc tests, reference tests and the Windows jobs run on `main` and nightly only. **CI shards the tests.** One job builds them into a cargo-nextest archive; two shards run on pull requests and four during full validation. Every test runs in its own process: don't rely diff --git a/scripts/ci/affected.py b/scripts/ci/affected.py index b826086c6f..513db3c85b 100644 --- a/scripts/ci/affected.py +++ b/scripts/ci/affected.py @@ -13,24 +13,37 @@ Prints and optionally appends to $GITHUB_OUTPUT: A changed file maps to the package whose directory holds it. The packages that depend on those (directly or not, dev-dependencies included) are added. Test data under tests/ maps to the -packages that read it, and a vendored crate (vendor/, a [patch] of a crates.io dependency) to the -packages that use it. Anything that can change every package, or that no package owns, selects -the full workspace. Only documentation and packaging files are ignored. +packages that read it, a vendored crate (vendor/, a [patch] of a crates.io dependency) to the +packages that use it, and a root file or folder a crate embeds to that crate. Anything that can +change every package, or that no package owns, selects the full workspace. Documentation, +packaging, the other workflows and the GitHub bots' scripts are ignored: when only those change, +nothing is selected and full is false, which CI reads as "no code changed" (on pull requests and +on pushes to main alike). """ import argparse import json import subprocess import sys -# Changes here can affect every package. -FULL_PREFIXES = (".cargo/", ".config/", ".github/", "scripts/") -FULL_FILES = ("Cargo.toml", "Cargo.lock") -# Changes here affect no code: documentation and packaging. -IGNORED_PREFIXES = ("docs/", "packaging/") +# Changes here can affect every package. The CI workflow itself is one; other workflows are not. +FULL_PREFIXES = (".cargo/", ".config/", "scripts/") +FULL_FILES = ("Cargo.toml", "Cargo.lock", ".github/workflows/ci.yml") +# Changes here affect no code: documentation, packaging, the other workflows and GitHub settings, +# and the GitHub bots' scripts (tested in the repository checks). Root *.md files too, unless +# embedded (below). +IGNORED_PREFIXES = ("docs/", "packaging/", ".github/", "scripts/claim/", "scripts/triage/") +IGNORED_FILES = ("vendor/README.md",) # Root files that a crate embeds (include_str!), so they are not ignorable documentation. -EMBEDDED = {"ATTRIBUTION.md": "solidcraft-ui-egui"} -# Asset folders that only some packages read: the UI embeds the catalogs, xtask checks them. -EMBEDDED_PREFIXES = {"assets/locales/": ("solidcraft-ui-egui", "xtask")} +EMBEDDED = { + "ATTRIBUTION.md": "solidcraft-ui-egui", + "LICENSE-MIT": "solidcraft-engine", + "LICENSE-APACHE": "solidcraft-engine", + "NOTICE": "solidcraft-engine", + "THIRD-PARTY-LICENSES.txt": "solidcraft-engine", +} +# Folders that only some packages read: the UI embeds the catalogs and xtask checks them; the MCP +# server's tests read the agent skills. +EMBEDDED_PREFIXES = {"assets/locales/": ("solidcraft-ui-egui", "xtask"), "skills/": ("solidcraft-mcp",)} # Test data and the package directories whose tests (or xtask) read it. Any other folder under # tests/ selects the full workspace. TEST_DATA = { @@ -38,7 +51,7 @@ TEST_DATA = { "tests/hole_matrix/": ("crates/engine/",), "tests/projection/": ("crates/engine/",), "tests/taper/": ("crates/engine/",), - "tests/oracle/": ("apps/solidcraft-cli/", "crates/kernel/", "xtask/"), + "tests/oracle/": ("apps/solidcraft-cli/", "xtask/"), "tests/action_matrix/": ("apps/solidcraft-cli/", "crates/engine/", "xtask/"), } # Vendored crates (vendor//, a [patch] of a crates.io dependency) and the dependency names @@ -102,12 +115,14 @@ def select(meta, paths): hit = set() for path in paths: - if path in FULL_FILES or path.startswith(FULL_PREFIXES) or path.startswith("rust-toolchain"): + if path in FULL_FILES or path.startswith("rust-toolchain"): + return True, [] + if path in IGNORED_FILES or path.startswith(IGNORED_PREFIXES): + continue + if path.startswith(FULL_PREFIXES): return True, [] owners = [d for d in by_dir if path.startswith(d)] if path.startswith("vendor/"): - if path == "vendor/README.md": - continue users = users_of(path.split("/")[1]) if not users: return True, [] @@ -124,7 +139,7 @@ def select(meta, paths): hit.add(EMBEDDED[path]) elif any(path.startswith(p) for p in EMBEDDED_PREFIXES): hit.update(next(v for p, v in EMBEDDED_PREFIXES.items() if path.startswith(p))) - elif path.startswith(IGNORED_PREFIXES) or ("/" not in path and path.endswith(".md")): + elif "/" not in path and path.endswith(".md"): continue else: return True, [] # not owned by any package diff --git a/scripts/ci/test_affected.py b/scripts/ci/test_affected.py index 6afb880665..34f6ded3ef 100644 --- a/scripts/ci/test_affected.py +++ b/scripts/ci/test_affected.py @@ -52,6 +52,12 @@ class Synthetic(unittest.TestCase): def test_docs_only_selects_nothing(self): self.assertEqual(affected.select(SYNTH, ["docs/book/a.md", "README.md", "packaging/x.yml"]), (False, [])) + def test_no_code_paths_select_nothing(self): + # What CI reads as "no code changed", on pull requests and pushes to main alike. + for p in [".github/workflows/docker.yml", ".github/ISSUE_TEMPLATE/bug.yml", "scripts/claim/claim.js", + "scripts/triage/triage.test.js", "vendor/README.md", "SECURITY.md"]: + self.assertEqual(affected.select(SYNTH, [p]), (False, []), p) + class RealWorkspace(unittest.TestCase): @classmethod @@ -78,11 +84,21 @@ class RealWorkspace(unittest.TestCase): self.assertNotIn("solidcraft-kernel", pk) self.assertNotIn("solidcraft-render", pk) - def test_oracle_cases_select_the_cli_and_xtask(self): + def test_oracle_cases_select_the_cli_and_xtask_not_the_kernel(self): full, pk = affected.select(self.meta, ["tests/oracle/box/recipe.json"]) self.assertFalse(full) self.assertIn("solidcraft-cli", pk) self.assertIn("xtask", pk) + self.assertNotIn("solidcraft-kernel", pk) + + def test_embedded_root_files_select_their_crate(self): + for path, pkg in [("ATTRIBUTION.md", "solidcraft-ui-egui"), ("LICENSE-MIT", "solidcraft-engine"), + ("NOTICE", "solidcraft-engine"), ("THIRD-PARTY-LICENSES.txt", "solidcraft-engine"), + ("skills/solidcraft-modeling/SKILL.md", "solidcraft-mcp")]: + full, pk = affected.select(self.meta, [path]) + self.assertFalse(full, path) + self.assertIn(pkg, pk, path) + self.assertNotIn("solidcraft-kernel", pk, path) def test_unmapped_test_data_selects_full(self): self.assertEqual(affected.select(self.meta, ["tests/tools/x.json"]), (True, []))