diff --git a/.github/workflows/build-wheels.yml b/.github/workflows/build-wheels.yml index 2731f5738..35bc078f7 100644 --- a/.github/workflows/build-wheels.yml +++ b/.github/workflows/build-wheels.yml @@ -25,7 +25,16 @@ jobs: # `~/.cache/uv` a second writer alongside ci.yml's tool cache for a job that # would save less time than the archive costs to move. build-pure-wheel: - runs-on: ubicloud-standard-2 + # Reached from ci.yml on every pull request, forks included, and from + # release.yml on a tag. A called workflow inherits its caller's `github` + # context, so this expression reads the caller's event: on the tag caller + # `github.event.pull_request` is null and the Ubicloud arm is selected, + # and on a fork's pull request the hosted arm is, because a fork cannot + # obtain an Ubicloud runner. Keep the continuation at this indent; a + # more-indented one would put a newline inside the expression. + runs-on: >- + ${{ github.event.pull_request.head.repo.fork + && 'ubuntu-latest' || 'ubicloud-standard-2' }} timeout-minutes: 20 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -37,7 +46,16 @@ jobs: artifact-name: wheels-pure build-native-wheels: + # Unchanged placement. GitHub spells matrix values into the check name, so + # both `ubuntu-latest` legs are named verbatim in the `main-required-checks` + # ruleset; moving either renames a required context, and nine steps here + # are guarded by `startsWith(matrix.os, 'ubuntu')`, which an expression + # would switch off. That is a decision for the repository owner, not a + # placement change. runs-on: ${{ matrix.os }} + # Measured at 29 to 107 s across the five legs; the Linux ones build in a + # container and the aarch64 leg emulates under QEMU. + timeout-minutes: 45 strategy: fail-fast: false matrix: @@ -156,7 +174,16 @@ jobs: # Also uncached: it installs published wheels into a throwaway virtual # environment and imports them. There is nothing expensive to keep. verify-wheel-install: - runs-on: ubicloud-standard-2 + # Reached from ci.yml on every pull request, forks included, and from + # release.yml on a tag. A called workflow inherits its caller's `github` + # context, so this expression reads the caller's event: on the tag caller + # `github.event.pull_request` is null and the Ubicloud arm is selected, + # and on a fork's pull request the hosted arm is, because a fork cannot + # obtain an Ubicloud runner. Keep the continuation at this indent; a + # more-indented one would put a newline inside the expression. + runs-on: >- + ${{ github.event.pull_request.head.repo.fork + && 'ubuntu-latest' || 'ubicloud-standard-2' }} timeout-minutes: 20 needs: - build-pure-wheel diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 557083e16..f340914c3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,7 +55,23 @@ env: jobs: lint-test: - runs-on: ubuntu-latest + # A pull request from a fork cannot obtain an Ubicloud runner, so the fork + # arm falls back to a GitHub-hosted label and only forks take it. Keep the + # continuation at this indent: a more-indented continuation in a folded + # scalar keeps its line break and puts a newline inside the expression. + # GitHub evaluates the broken value regardless, so a green run is not + # evidence; `test_placement_expressions_parse_to_one_line` is. + runs-on: >- + ${{ github.event.pull_request.head.repo.fork + && 'ubuntu-latest' || 'ubicloud-standard-2' }} + # Compile-bound on two toolchains plus a Windows cross-target check, + # measured at 198 to 292 s over six runs on four GitHub-hosted vCPU. + # Ubicloud vCPU are not GitHub vCPU, so the Ubicloud figure is expected to + # be higher and the arm is judged on the third run. The ceiling matches the + # interpreter-suite lanes: a regression fails visibly instead of quietly + # costing, and `ubicloud-standard-4` needs measured evidence this + # repository does not have yet. + timeout-minutes: 45 env: MBAKE_VERSION: '1.4.6' MDTABLEFIX_VERSION: '0.6.0' @@ -72,6 +88,12 @@ jobs: # token must not stay in .git/config where that code could reach it. persist-credentials: false + - name: Start resource sampler + uses: ./.github/actions/resource-sampler + with: + mode: start + vcpus: ${{ env.LINUX_RUNNER_VCPUS }} + - name: Compute cache keys uses: ./.github/actions/cache-keys with: @@ -80,10 +102,14 @@ jobs: ubuntu-release: ${{ env.UBUNTU_RELEASE }} compiler-shape: lint - # This job is GitHub-hosted, so these archives live in GitHub's cache - # service rather than Ubicloud's. Every key carries `runner.environment`, - # so the two lanes cannot restore each other's entries even though they - # share a family name, and each lane has its own single writer. + # Every `CARGO_CACHE_KEY` and `SCCACHE_CACHE_KEY` family carries + # `runner.environment`, so the owned arm and the fork arm cannot restore + # each other's entries even though they share a family name. Nothing + # writes the fork arm's scope of those two families: their saves require + # a push to `refs/heads/main`, which a fork can never produce, so a + # fork's pull request restores nothing from them and compiles cold. The + # plain `actions/cache` tool caches further down are not key families: + # they save on every event under GitHub's own branch scoping. - name: Restore the Cargo registry id: cargo-cache uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 @@ -328,26 +354,19 @@ jobs: printf '%s\n' "${fence}" } >> "${GITHUB_STEP_SUMMARY}" - # Single writer for the GitHub-hosted lane, mirroring `typecheck-test` - # on the Ubicloud lane. It is the only Linux gate here and the only one - # that compiles, so there is nothing to race with. - - name: Save the Cargo registry - if: >- - github.event_name == 'push' && github.ref == 'refs/heads/main' && - steps.cargo-cache.outputs.cache-hit != 'true' - uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - key: ${{ env.CARGO_CACHE_KEY }} - + # This job restores the Cargo registry and no longer saves it. On the + # owned arm it now runs on the Ubicloud lane, where `runner.environment` + # renders `self-hosted` and the key it would publish is the very family + # `extension-tests` writes. Two writers of one family is the collision + # `test_each_cache_family_has_exactly_one_writer` exists to refuse, and + # the registry holds the resolved dependency graph, which is the same + # content either job would archive. - name: Save the compiler cache - # The only GitHub-hosted job that compiles, so it owns the whole - # `github-hosted` compiler-cache scope; its Cranelift lint build is - # also a shape no other job produces. Its archive lands in GitHub's - # cache rather than Ubicloud's, which is the other reason the lane is - # named in the key. + # The `lint` build shape is produced by no other job, so this remains + # the sole writer of its compiler-cache family. Only the lane in the + # rendered key changed: the fork arm cannot reach this step, because + # the guard below requires a push to `refs/heads/main` and a fork's + # pull request is neither. # # The entry is a rolling generation named after the run, so there is # no cache-hit guard to skip: the key is new every time, and a run @@ -362,10 +381,25 @@ jobs: path: ~/.cache/sccache key: ${{ env.SCCACHE_CACHE_KEY }} + - name: Report peak resource use + if: always() + uses: ./.github/actions/resource-sampler + with: + mode: report + vcpus: ${{ env.LINUX_RUNNER_VCPUS }} + typecheck-test: name: Typecheck and test (Python ${{ matrix.python-label }}) continue-on-error: ${{ matrix.experimental }} - runs-on: ubicloud-standard-2 + # A pull request from a fork cannot obtain an Ubicloud runner, so the fork + # arm falls back to a GitHub-hosted label and only forks take it. Keep the + # continuation at this indent: a more-indented continuation in a folded + # scalar keeps its line break and puts a newline inside the expression. + # GitHub evaluates the broken value regardless, so a green run is not + # evidence; `test_placement_expressions_parse_to_one_line` is. + runs-on: >- + ${{ github.event.pull_request.head.repo.fork + && 'ubuntu-latest' || 'ubicloud-standard-2' }} timeout-minutes: 45 env: MDTABLEFIX_VERSION: '0.6.0' @@ -617,7 +651,15 @@ jobs: extension-tests: name: Extension-gated tests (Python/Rust boundary) - runs-on: ubicloud-standard-2 + # A pull request from a fork cannot obtain an Ubicloud runner, so the fork + # arm falls back to a GitHub-hosted label and only forks take it. Keep the + # continuation at this indent: a more-indented continuation in a folded + # scalar keeps its line break and puts a newline inside the expression. + # GitHub evaluates the broken value regardless, so a green run is not + # evidence; `test_placement_expressions_parse_to_one_line` is. + runs-on: >- + ${{ github.event.pull_request.head.repo.fork + && 'ubuntu-latest' || 'ubicloud-standard-2' }} timeout-minutes: 45 steps: - name: Check out repository @@ -822,6 +864,11 @@ jobs: extension-tests-windows: name: Extension-gated tests (Windows Python/Rust boundary) runs-on: windows-2022 + # Measured at 67 to 72 s. A ceiling belongs on every job that declares + # steps, not only on the paid ones: once a label is event-dependent, + # "an Ubicloud lane" is a property of the event, so a rule keyed on the + # label would stop applying on exactly the arm that hangs. + timeout-minutes: 20 steps: - name: Check out repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -873,7 +920,15 @@ jobs: # ratchet baseline, so this lane only compares its coverage locally # against the trusted baseline without generating coverage twice on main. if: github.event_name == 'pull_request' - runs-on: ubicloud-standard-2 + # A pull request from a fork cannot obtain an Ubicloud runner, so the fork + # arm falls back to a GitHub-hosted label and only forks take it. Keep the + # continuation at this indent: a more-indented continuation in a folded + # scalar keeps its line break and puts a newline inside the expression. + # GitHub evaluates the broken value regardless, so a green run is not + # evidence; `test_placement_expressions_parse_to_one_line` is. + runs-on: >- + ${{ github.event.pull_request.head.repo.fork + && 'ubuntu-latest' || 'ubicloud-standard-2' }} # Above the 2,700 s cargo watchdog set below plus the work either side # of the coverage step, which reached 43 s on run 34071469378. Fifty # minutes is the requirement, and the estate's rule is fifteen above @@ -1128,7 +1183,10 @@ jobs: # `needs` edge never leaves benchmark-ratchet skipped on pushes to # main, where the baseline artefact must stay fresh. changes: + # Stays GitHub-hosted: a five-second paths-filter call is administrative, + # API-bound work that gains nothing from a metered build slot. runs-on: ubuntu-latest + timeout-minutes: 10 permissions: contents: read # paths-filter reads pull-request changed files through GitHub's API. @@ -1426,7 +1484,15 @@ jobs: # ratio between the baseline and the candidate, so runner speed cancels out # of the comparison and the job does not need a fixed or larger shape. It # therefore starts at the estate default like every other Linux gate. - runs-on: ubicloud-standard-2 + # A pull request from a fork cannot obtain an Ubicloud runner, so the fork + # arm falls back to a GitHub-hosted label and only forks take it. Keep the + # continuation at this indent: a more-indented continuation in a folded + # scalar keeps its line break and puts a newline inside the expression. + # GitHub evaluates the broken value regardless, so a green run is not + # evidence; `test_placement_expressions_parse_to_one_line` is. + runs-on: >- + ${{ github.event.pull_request.head.repo.fork + && 'ubuntu-latest' || 'ubicloud-standard-2' }} timeout-minutes: 60 needs: - lint-test diff --git a/.github/workflows/delayed-pr-comment.yml b/.github/workflows/delayed-pr-comment.yml index c73c0642a..833bcab37 100644 --- a/.github/workflows/delayed-pr-comment.yml +++ b/.github/workflows/delayed-pr-comment.yml @@ -18,6 +18,10 @@ jobs: # Deliberately stays on GitHub-hosted Ubuntu: this sleeps while holding a # runner and must not consume a metered Ubicloud build slot. runs-on: ubuntu-latest + # A ceiling far above any plausible delay, so a runaway `delay_minutes` + # cannot hold a runner for GitHub's six-hour default. A delay longer than + # this belongs in a scheduled workflow, not in a sleeping job. + timeout-minutes: 180 permissions: pull-requests: write # needed to post comments steps: diff --git a/docs/ci-cache-ownership.md b/docs/ci-cache-ownership.md index 7881219d7..b429ec013 100644 --- a/docs/ci-cache-ownership.md +++ b/docs/ci-cache-ownership.md @@ -102,7 +102,6 @@ the family without saving it. | Family | Writer | | ------------------------------------- | ------------------------------------- | | Cargo registry, Ubicloud | `ci.yml` `extension-tests` | -| Cargo registry, GitHub-hosted | `ci.yml` `lint-test` | | Tools, per interpreter | `ci.yml` `typecheck-test`, that leg | | Compiler, 3.13 unoptimized | `ci.yml` `extension-tests` | | Compiler, 3.12/3.14/3.15a unoptimized | `ci.yml` `typecheck-test`, that leg | @@ -111,6 +110,17 @@ the family without saving it. | Compiler, Cranelift lint | `ci.yml` `lint-test` | | Compiler, Loom model | `loom.yml` `loom` | +`lint-test` moved to the Ubicloud lane with the fork fallback, taking its +Cranelift lint family with it, and stopped saving the Cargo registry: on its +owned arm it renders the same family `extension-tests` writes, and one writer +per family is the invariant. The Loom family stays on the GitHub-hosted lane, +because its writer is a scheduled job that stays there too. + +The fork arm renders `runner.environment` as `github-hosted`, so a fork's pull +request reads a scope whose only writer is that scheduled Loom job and runs +cold for everything else. Every save is guarded on a push to `refs/heads/main`, +which a fork can never produce, so the fork arm restores and never publishes. + _Table 2: The single job that publishes each family under the writer trigger described above._ diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 869fc76fe..57a54280c 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -43,24 +43,34 @@ gains nothing from a metered build slot. Table 1: GitHub Actions jobs, workflows, and runners -| Job | Workflow | Runner | -| ------------------------- | ---------------------------- | --------------------- | -| `typecheck-test` | `ci.yml` | `ubicloud-standard-2` | -| `extension-tests` | `ci.yml` | `ubicloud-standard-2` | -| `coverage` | `ci.yml` | `ubicloud-standard-2` | -| `benchmark-ratchet` | `ci.yml` | `ubicloud-standard-2` | -| `build-pure-wheel` | `build-wheels.yml` | `ubicloud-standard-2` | -| `verify-wheel-install` | `build-wheels.yml` | `ubicloud-standard-2` | -| `coverage-upload` | `coverage-main.yml` | `ubicloud-standard-2` | -| `lint-test` | `ci.yml` | `ubuntu-latest` | -| `changes` | `ci.yml` | `ubuntu-latest` | -| `workflow-harness` | `benchmark-gate-harness.yml` | `ubuntu-latest` | -| `loom-smoke` | `ci.yml` | `ubuntu-latest` | -| `loom` | `loom.yml` | `ubuntu-latest` | -| `extension-tests-windows` | `ci.yml` | `windows-2022` | -| `publish` | `release.yml` | `ubuntu-latest` | -| `delay_and_comment` | `delayed-pr-comment.yml` | `ubuntu-latest` | -| `build-native-wheels` | `build-wheels.yml` | `${{ matrix.os }}` | +| Job | Workflow | Owned arm | Fork arm | Ceiling | +| ------------------------- | ---------------------------- | --------------------- | --------------- | ------- | +| `lint-test` | `ci.yml` | `ubicloud-standard-2` | `ubuntu-latest` | 45 | +| `typecheck-test` | `ci.yml` | `ubicloud-standard-2` | `ubuntu-latest` | 45 | +| `extension-tests` | `ci.yml` | `ubicloud-standard-2` | `ubuntu-latest` | 45 | +| `coverage` | `ci.yml` | `ubicloud-standard-2` | `ubuntu-latest` | 65 | +| `benchmark-ratchet` | `ci.yml` | `ubicloud-standard-2` | `ubuntu-latest` | 60 | +| `build-pure-wheel` | `build-wheels.yml` | `ubicloud-standard-2` | `ubuntu-latest` | 20 | +| `verify-wheel-install` | `build-wheels.yml` | `ubicloud-standard-2` | `ubuntu-latest` | 20 | +| `coverage-upload` | `coverage-main.yml` | `ubicloud-standard-2` | none | 65 | +| `changes` | `ci.yml` | `ubuntu-latest` | none | 10 | +| `loom-smoke` | `ci.yml` | `ubuntu-latest` | none | 10 | +| `workflow-harness` | `benchmark-gate-harness.yml` | `ubuntu-latest` | none | 30 | +| `loom` | `loom.yml` | `ubuntu-latest` | none | 30 | +| `verus` | `rust-boundaries.yml` | `ubuntu-latest` | none | 20 | +| `extended` | `rust-boundaries.yml` | `ubuntu-latest` | none | 45 | +| `extension-tests-windows` | `ci.yml` | `windows-2022` | none | 20 | +| `publish` | `release.yml` | `ubuntu-latest` | none | 20 | +| `delay_and_comment` | `delayed-pr-comment.yml` | `ubuntu-latest` | none | 180 | +| `build-native-wheels` | `build-wheels.yml` | `${{ matrix.os }}` | none | 45 | +| `native` | `rust-boundaries.yml` | `${{ matrix.os }}` | none | 20 | + +The hosted rows are hosted for a reason, not by omission. `changes` and +`loom-smoke` are cheap gate jobs; `workflow-harness`, `loom`, and `extended` +run only on a schedule or a dispatch; and `rust-boundaries.yml`'s verifier +lanes stay on GitHub-hosted Linux by this repository's own decision. Placing +`verus` or `loom-smoke` on the paid lane is a separate question needing its own +measurements. `ubicloud-standard-2` (2 vCPU, 8 GB, Ubuntu 24.04 amd64) is the default shape and the only self-hosted label registered in `.github/actionlint.yaml`. @@ -76,17 +86,141 @@ scenario's within-run `rust_mean / python_mean` ratio between the baseline and the candidate, so runner speed cancels out of the comparison and the job needs neither a fixed nor a larger shape. -`lint-test` stays on `ubuntu-latest`. It installs Whitaker and keeps its own -npm and Whitaker archives on GitHub's cache service, which is a different store -from Ubicloud's. It owns its Cargo registry and compiler caches on that side -exactly as the Ubicloud jobs own theirs. +`lint-test` moved onto the paid lane. It is a developer-blocking gate that +compiles real work: Whitaker, clippy under two toolchains, a Windows +cross-target check, and Nixie. Six runs measured it at 198 to 292 seconds on +four GitHub-hosted vCPU, with a queue that was two to three seconds five times +and 154 seconds once; the same run queued every hosted lane 77 to 164 seconds +while its Ubicloud neighbours queued 18 to 37. The case is that tail. Ubicloud +vCPU are not GitHub vCPU, so the Ubicloud figure is expected to be higher than +the hosted range, and the arm is judged on its third warm run rather than its +first. Its required check context is the bare job id, which no runner label +spells, so the move renames nothing. The native-wheel matrix keeps its platform runners. Ubicloud has no Windows or macOS capacity, and its Linux legs build inside manylinux containers and under -QEMU emulation. +QEMU emulation. The two Linux legs are also the one place a placement change +would cost more than it looks: GitHub spells matrix values into the check name, +so `build-wheels / build-native-wheels (ubuntu-latest, …)` appears verbatim +twice in the `main-required-checks` ruleset, and nine steps in that job are +guarded on `startsWith(matrix.os, 'ubuntu')`, which an expression would switch +off. Moving either leg is a ruleset edit and a guard rewrite, not a label +change. -Every Ubicloud job declares `timeout-minutes` so a wedged runner cannot bill -for GitHub's six-hour default. +### The fork arm + +A pull request from a fork cannot obtain an Ubicloud runner, so every +fork-reachable lane selects its runner by event: + +```yaml +runs-on: >- + ${{ github.event.pull_request.head.repo.fork + && 'ubuntu-latest' || 'ubicloud-standard-2' }} +``` + +Keep the continuation at the same indent. A continuation indented one level +deeper keeps its line break inside the folded scalar and puts a newline in the +middle of the expression; GitHub evaluates the broken value anyway, so a green +run proves nothing and `test_placement_expressions_parse_to_one_line` is what +catches it. + +Reachability is a property of the triggers, not of the file. `build-wheels.yml` +declares only `workflow_call`, and its two Ubicloud jobs are as exposed as the +rest because `ci.yml` calls it on every pull request. A called workflow +inherits its caller's `github` context, so the expression above works unchanged +there: on the tag-push caller `github.event.pull_request` is null and the +Ubicloud arm is selected. `coverage-upload` carries no fallback because +`coverage-main.yml` triggers only on a push to `main` and a dispatch, neither +of which a fork can cause. + +The fork arm runs cold, and that is the accepted price. Every cache key carries +`runner.environment`, which renders `github-hosted` on the fork arm and +`self-hosted` on the owned one, so the two arms read different scopes. No +writer changes: every key-family save step is guarded on a push to +`refs/heads/main`, which is never a fork, so the fork arm restores and never +publishes. Since `lint-test` moved, the one `github-hosted` family still +written is the Loom model family, which `loom.yml` saves only on its scheduled +runs on `main`; `loom-smoke` restores it on a pull request, a fork's included, +and never saves it. + +`lint-test` therefore no longer saves the Cargo registry. On its owned arm it +renders the same `self-hosted` family `extension-tests` writes, and one writer +per family is the invariant `test_each_cache_family_has_exactly_one_writer` +holds. It still restores that archive; the registry holds the resolved +dependency graph, which is the same content either job would have published. + +### Ceilings + +Every job that declares `steps` declares `timeout-minutes`, so a wedged runner +cannot bill for GitHub's six-hour default. The rule is keyed on declaring steps +rather than on carrying an Ubicloud label, because once a label is an +expression "an Ubicloud lane" is a property of the event, and a rule keyed on +the label would stop applying on exactly the arm that hangs. + +Jobs that declare `uses:` are exempt and must declare no ceiling: GitHub rejects +`timeout-minutes` on a reusable-workflow caller, so placing a job and bounding +it are separate ideas and the bound lives in the callee. Cuprum's four callers +all call workflows whose own jobs are bounded by the rule above. + +### The actionlint registry + +`.github/actionlint.yaml` registers exactly the labels in use that are neither +GitHub-hosted nor already registered, asserted as an equality in both +directions. "In use" is derived from every job in the estate, reading both arms +of a conditional and every matrix runner entry, minus the named frozen set of +GitHub-hosted labels in `tests/helpers/ci_placement.py`. + +The frozen set is named rather than derived from a vendor prefix on purpose. A +`ubicloud-` prefix filter agrees with the named set over today's workflows and +differs on the case that matters: it would silently exempt a second paid +provider's labels from the registry question altogether. A subset assertion +would miss a stale registration, which is the other half of the equality. + +### The placement reader + +`tests/helpers/ci_placement.py` is the one reader of `runs-on` in the test +suite. Its scope is deliberately narrow: it models the three shapes this +repository declares, a literal label, the fork fallback, and a matrix key +resolved through its `include` entries, and refuses everything else by name. +Every contract that asks where a job runs, whether it runs, or what label it +resolves to goes through it, including the cache-family resolver in +`tests/helpers/ci_cache_families.py`, so there is one definition of a lane +rather than two that drift. + +Refusal is the design, not a gap. An expression recorded as one opaque label +carries no vendor prefix, so an Ubicloud classifier drops the lane and it sits +exempt from every placement, ceiling, and registry rule while still asking for +a paid runner. `tests/test_ci_placement_reader.py` drives the reader with +synthetic declarations, because a rule parametrized over this repository's own +correct workflows passes whether or not it discriminates. + +Reading is fallible, and the failure has one boundary. Every query reaches the +filesystem through `read_source` and `read_workflow` in +`tests/helpers/ci_workflows.py`, which translate an I/O error, invalid YAML, or +a document that is not a mapping into an `AssertionError` naming the file. +`tests/test_ci_workflow_loader.py` drives both against real files in a +temporary directory. The queries take workflow and job names rather than an +injected reader: their only input is this repository's committed workflows, and +the reader tests already substitute a synthetic declaration at the module seam, +so the placement logic is exercised over plain mappings without a reader +threaded through every contract. + +Three layers sit above it, each answering a question the layer below cannot: + +- `tests/test_ci_placement_guards.py` covers whether a job runs and which + context properties a declaration reads. +- `tests/test_ci_placement_properties.py` generates the input space rather than + enumerating it. Every reader defect found in review was an input nobody had + written down, and the properties have since found one more of their own: a + condition carrying a hyphen, such as `matrix.python-version`, was refused as + unmodellable. +- `tests/behaviour/test_ci_runner_selection.py` evaluates the composed + workflows against an event, following the call from `ci.yml` into + `build-wheels.yml`, and asserts the runner each event actually selects. That + is where the property that matters lives: a fork's pull request must never + select a runner a fork cannot obtain. `act` cannot stand in for it, because + it skips a job whose label it cannot map and exits zero, so the Ubicloud + lanes would go unexercised while the run reported success. ### Cache ownership @@ -103,11 +237,11 @@ key and its restore-key prefix as environment values. A restore and its save cannot disagree, and the rendered key is printed into the run summary so any miss can be explained from the run alone. -| Key family | Paths | Key inputs | Writer | -| ---------- | -------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------ | -| `cargo-` | `~/.cargo/registry`, `~/.cargo/git` | generation, OS, arch, runner environment, hash of `rust/Cargo.lock` and `rust/rust-toolchain.toml` | `extension-tests`, and `lint-test` on the GitHub-hosted lane | -| `tool-` | `~/.cargo/bin`, `~/.local/bin`, `~/.cache/uv`, `~/.local/share/uv`, `.uv-cache`, `.uv-tools` | the above plus Ubuntu release, Python version, nextest pin, hash of `uv.lock`, `pyproject.toml`, `Makefile`, and the sccache action | `typecheck-test` | -| `sccache-` | `~/.cache/sccache` | generation, OS, arch, runner environment, Ubuntu release, run identifier | `typecheck-test`, and `lint-test` on the GitHub-hosted lane | +| Key family | Paths | Key inputs | Writer | +| ---------- | -------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------- | +| `cargo-` | `~/.cargo/registry`, `~/.cargo/git` | generation, OS, arch, runner environment, hash of `rust/Cargo.lock` and `rust/rust-toolchain.toml` | `extension-tests` | +| `tool-` | `~/.cargo/bin`, `~/.local/bin`, `~/.cache/uv`, `~/.local/share/uv`, `.uv-cache`, `.uv-tools` | the above plus Ubuntu release, Python version, nextest pin, hash of `uv.lock`, `pyproject.toml`, `Makefile`, and the sccache action | `typecheck-test` | +| `sccache-` | `~/.cache/sccache` | generation, OS, arch, runner environment, Ubuntu release, run identifier | `typecheck-test`, `lint-test`, and the other compiling jobs | Four rules hold that table together, each with a contract test: @@ -221,9 +355,12 @@ measurement that forced the split. In outline, `extension-tests` writes the 3.13 unoptimized family, each `typecheck-test` leg that runs a suite writes its own interpreter's, `benchmark-ratchet` writes the 3.13 release family, `coverage-upload` writes the instrumented one, and `lint-test` writes the -GitHub-hosted lint family. The scheduled `loom.yml` job writes its separate -model family only on `refs/heads/main`, while manual dispatches and -`loom-smoke` restore it without saving. +Cranelift lint family, which moves to the Ubicloud lane with it. The scheduled +`loom.yml` job writes its separate model family only on `refs/heads/main`, and +stays on the GitHub-hosted lane because its writer does, while manual +dispatches and `loom-smoke` restore it without saving. Apart from that Loom +family, a fork's pull request reads a `github-hosted` scope that nothing +writes, so it compiles cold. The writer has to be a job that actually compiles, or the rolling generation freezes: it would restore the previous entry and republish it unchanged diff --git a/tests/behaviour/test_ci_runner_selection.py b/tests/behaviour/test_ci_runner_selection.py new file mode 100644 index 000000000..694a1dd46 --- /dev/null +++ b/tests/behaviour/test_ci_runner_selection.py @@ -0,0 +1,301 @@ +"""Behavioural tests for the runner each CI event selects. + +The contract tests in `tests/test_ci_runner_placement.py` read one job's +declaration at a time. These scenarios evaluate the composed workflows against +an event: they resolve every expression, follow the call from `ci.yml` into +`build-wheels.yml`, and assert the runner the event actually selects. + +That composition is where the interesting failures live. A lane can declare two +correct labels and still send a fork to a runner no fork can obtain, and a +called workflow's placement is invisible to any rule that reads its own +`workflow_call` trigger. `act` cannot stand in here: it skips a job whose label +it cannot map and exits zero, so the Ubicloud lanes would go unexercised while +the run reported success. +""" + +from __future__ import annotations + +import typing as typ + +import pytest +from pytest_bdd import given, parsers, scenario, then, when + +from tests.helpers.ci_events import ( + FORK_PULL_REQUEST, + OWNED_PULL_REQUEST, + PUSH_TO_MAIN, + TAG_PUSH, + schedule, +) +from tests.helpers.ci_runners import ( + FORK_REACHABLE_UBICLOUD_JOBS, + GITHUB_LABEL, + UBICLOUD_JOBS, + UBICLOUD_LABEL, + expand, +) + +if typ.TYPE_CHECKING: + from tests.helpers.ci_events import Scheduled + +FEATURE = "../features/ci_runner_selection.feature" +#: The wheel jobs that only a call can reach. `build-wheels.yml` declares +#: `workflow_call` alone, so its own trigger says nothing about who runs it. +CALLED_WHEEL_JOBS: typ.Final = ("build-pure-wheel", "verify-wheel-install") +PLATFORM_LABELS: typ.Final = frozenset({ + "macos-15-intel", + "macos-latest", + "ubuntu-latest", + "windows-2022", +}) +#: The fewest reviewed lanes any event asserting that step schedules. Named +#: rather than derived: a derivation from the same schedule the assertion +#: reads would move with the defect it is meant to catch. +MINIMUM_REVIEWED_LANES: typ.Final = 4 +#: The lane whose own guard admits one event only. Its presence and absence +#: are what make the event evaluation falsifiable: every other assertion here +#: reads a label, and a wrongly scheduled job declares the right one. +PULL_REQUEST_ONLY_LANE: typ.Final = ("ci.yml", "coverage") +EVENTS: typ.Final = { + "fork": FORK_PULL_REQUEST, + "owned": OWNED_PULL_REQUEST, + "push": PUSH_TO_MAIN, +} + + +@pytest.fixture +def selection() -> dict[str, object]: + """Carry the workflow under test and the schedule between steps.""" + return {} + + +@given("the continuous integration workflow") +def _ci_workflow(selection: dict[str, object]) -> None: + """Select the workflow every pull request and push triggers.""" + selection["workflow"] = "ci.yml" + + +@given("the release workflow") +def _release_workflow(selection: dict[str, object]) -> None: + """Select the workflow a tag push triggers.""" + selection["workflow"] = "release.yml" + + +@when("a pull request is opened from a branch of this repository") +def _owned_pull_request(selection: dict[str, object]) -> None: + """Resolve the schedule for a pull request from this repository.""" + selection["scheduled"] = schedule( + typ.cast("str", selection["workflow"]), OWNED_PULL_REQUEST + ) + + +@when("a pull request is opened from a fork") +def _fork_pull_request(selection: dict[str, object]) -> None: + """Resolve the schedule for a pull request from a fork.""" + selection["scheduled"] = schedule( + typ.cast("str", selection["workflow"]), FORK_PULL_REQUEST + ) + + +@when("a commit is pushed to the default branch") +def _push_to_main(selection: dict[str, object]) -> None: + """Resolve the schedule for a push to the default branch.""" + selection["scheduled"] = schedule( + typ.cast("str", selection["workflow"]), PUSH_TO_MAIN + ) + + +@when("a tag is pushed") +def _tag_push(selection: dict[str, object]) -> None: + """Resolve the schedule for a tag push.""" + selection["scheduled"] = schedule(typ.cast("str", selection["workflow"]), TAG_PUSH) + + +@when(parsers.parse("the {event} event is evaluated")) +def _named_event(selection: dict[str, object], event: str) -> None: + """Resolve the schedule for the event the outline names.""" + selection["scheduled"] = schedule( + typ.cast("str", selection["workflow"]), EVENTS[event] + ) + + +def _scheduled(selection: dict[str, object]) -> list[Scheduled]: + """Return the resolved schedule, refusing an empty one. + + An empty schedule would satisfy every assertion below by vacuity, which is + indistinguishable from a passing run until something is mutated. + + Returns + ------- + list[Scheduled] + Every job the event scheduled, guaranteed non-empty. + """ + resolved = typ.cast("list[Scheduled]", selection["scheduled"]) + assert resolved, "the event scheduled no jobs, so every assertion is vacuous" + return resolved + + +@then("every reviewed lane selects the Ubicloud runner") +def _reviewed_lanes_are_paid(selection: dict[str, object]) -> None: + """Assert each reviewed lane took the Ubicloud arm. + + The `continue` below is a vacuity hole on its own: a schedule containing no + reviewed lane at all would take it every time, check nothing, and read + exactly like a pass. The checked count closes it. + """ + resolved = { + (item.workflow, item.job): item.labels for item in _scheduled(selection) + } + checked = 0 + for workflow_name, job_name in expand(UBICLOUD_JOBS): + if (workflow_name, job_name) not in resolved: + continue + checked += 1 + assert resolved[workflow_name, job_name] == (UBICLOUD_LABEL,), ( + f"{workflow_name}:{job_name} must select {UBICLOUD_LABEL} on this " + f"event, got {resolved[workflow_name, job_name]}" + ) + assert checked >= MINIMUM_REVIEWED_LANES, ( + f"this event scheduled only {checked} reviewed lanes, fewer than the " + f"{MINIMUM_REVIEWED_LANES} every event asserting this step places; the " + "loop above would have checked nothing and still passed" + ) + + +@then("the wheel jobs reached through the called workflow select it too") +def _called_wheel_jobs_are_paid(selection: dict[str, object]) -> None: + """Assert the called workflow's wheel jobs were reached and placed.""" + resolved = {item.job: item.labels for item in _scheduled(selection)} + for job_name in CALLED_WHEEL_JOBS: + assert job_name in resolved, ( + f"{job_name} must be reached through the call into " + "build-wheels.yml; a caller that stopped calling it would leave " + "every rule about its placement true and unexercised" + ) + assert resolved[job_name] == (UBICLOUD_LABEL,), ( + f"{job_name} must select {UBICLOUD_LABEL}, got {resolved[job_name]}" + ) + + +@then("no lane selects a paid runner") +def _no_paid_runner_for_a_fork(selection: dict[str, object]) -> None: + """Assert no lane asked for a runner a fork cannot obtain.""" + offenders = [ + f"{item.workflow}:{item.job}={label}" + for item in _scheduled(selection) + for label in item.labels + if label.startswith("ubicloud-") + ] + assert not offenders, ( + "a fork's pull request cannot obtain an Ubicloud runner, so a lane " + f"selecting one would queue until it timed out: {offenders}" + ) + + +@then("every fork-reachable lane selects the hosted fallback") +def _fork_lanes_fall_back(selection: dict[str, object]) -> None: + """Assert every fork-reachable lane took the hosted arm.""" + resolved = { + (item.workflow, item.job): item.labels for item in _scheduled(selection) + } + for workflow_name, job_name in expand(FORK_REACHABLE_UBICLOUD_JOBS): + assert (workflow_name, job_name) in resolved, ( + f"{workflow_name}:{job_name} is declared fork-reachable but this " + "event did not schedule it" + ) + assert resolved[workflow_name, job_name] == (GITHUB_LABEL,), ( + f"{workflow_name}:{job_name} must fall back to {GITHUB_LABEL} for " + f"a fork, got {resolved[workflow_name, job_name]}" + ) + + +@then("the pull-request-only lane is absent") +def _pull_request_lane_absent(selection: dict[str, object]) -> None: + """Assert a push does not schedule the lane its own guard excludes. + + `ci.yml:coverage` declares `github.event_name == 'pull_request'`, so GitHub + skips it on a push. A schedule that listed it anyway would report the push + event placing a job the push event never runs, and every placement + assertion over that schedule would still pass, because the label it + declares is correct. Only its absence discriminates. + """ + scheduled = {(item.workflow, item.job) for item in _scheduled(selection)} + assert PULL_REQUEST_ONLY_LANE not in scheduled, ( + f"{PULL_REQUEST_ONLY_LANE} is guarded on the pull_request event, so a " + "push must not schedule it" + ) + + +@then("the pull-request-only lane is present") +def _pull_request_lane_present(selection: dict[str, object]) -> None: + """Prove the exclusion narrow: the event it is written for still runs it.""" + scheduled = {(item.workflow, item.job) for item in _scheduled(selection)} + assert PULL_REQUEST_ONLY_LANE in scheduled, ( + f"{PULL_REQUEST_ONLY_LANE} must run on a pull request; excluding it " + "here would make the absence assertion above hold for the wrong reason" + ) + + +@then("the native wheel matrix keeps its platform runners") +def _native_matrix_is_hosted(selection: dict[str, object]) -> None: + """Assert the native matrix kept exactly its platform runners.""" + resolved = {item.job: item.labels for item in _scheduled(selection)} + assert "build-native-wheels" in resolved, ( + "the native wheel matrix must be reached through the call" + ) + assert frozenset(resolved["build-native-wheels"]) == PLATFORM_LABELS, ( + "the native matrix must keep exactly its platform runners, got " + f"{sorted(resolved['build-native-wheels'])}" + ) + + +@then("every selected label is a concrete runner name") +def _labels_are_schedulable(selection: dict[str, object]) -> None: + """Assert every selected label is one GitHub could schedule.""" + for item in _scheduled(selection): + for label in item.labels: + assert "${{" not in label, ( + f"{item.workflow}:{item.job} selected an unevaluated " + f"expression: {label!r}" + ) + assert label, f"{item.workflow}:{item.job} selected an empty label" + assert label == label.strip(), ( + f"{item.workflow}:{item.job} selected {label!r}; GitHub keeps " + "whatever surrounds an interpolation, and a padded label " + "matches no runner" + ) + + +@scenario(FEATURE, "an owned pull request keeps every reviewed lane on the paid runner") +def test_an_owned_pull_request_keeps_the_paid_lane() -> None: + """An internal pull request uses the runners the repository pays for.""" + + +@scenario(FEATURE, "a fork's pull request selects only runners a fork can obtain") +def test_a_fork_pull_request_is_schedulable() -> None: + """A fork's pull request never waits on a runner it cannot be given.""" + + +@scenario(FEATURE, "a push to the default branch takes the owned arm") +def test_a_push_takes_the_owned_arm() -> None: + """With no pull-request payload the fork field is absent, so falsy.""" + + +@scenario(FEATURE, "a push does not schedule the pull-request-only lane") +def test_a_push_skips_the_pull_request_only_lane() -> None: + """A job's own event guard is honoured by the schedule.""" + + +@scenario(FEATURE, "a pull request does schedule the pull-request-only lane") +def test_a_pull_request_runs_the_pull_request_only_lane() -> None: + """The same guard admits the event it is written for.""" + + +@scenario(FEATURE, "a tag push reaches the wheel jobs on the paid runner") +def test_a_tag_push_reaches_the_wheel_jobs() -> None: + """The release path resolves through the same called workflow.""" + + +@scenario(FEATURE, "every selected runner is a schedulable label") +def test_every_selected_runner_is_schedulable() -> None: + """No event may select an unevaluated or padded label.""" diff --git a/tests/features/ci_runner_selection.feature b/tests/features/ci_runner_selection.feature new file mode 100644 index 000000000..144112910 --- /dev/null +++ b/tests/features/ci_runner_selection.feature @@ -0,0 +1,53 @@ +Feature: CI runner selection by event + The placement contracts read declarations one job at a time. These scenarios + exercise the composed workflows: they evaluate each event against the real + YAML, follow the call from ci.yml into build-wheels.yml, and assert the + runner each event actually selects. + + The property that matters is not that a lane names two labels. It is that a + fork's pull request never selects a runner a fork cannot obtain, and that an + owned event never gives away the paid lane. + + Scenario: an owned pull request keeps every reviewed lane on the paid runner + Given the continuous integration workflow + When a pull request is opened from a branch of this repository + Then every reviewed lane selects the Ubicloud runner + And the wheel jobs reached through the called workflow select it too + + Scenario: a fork's pull request selects only runners a fork can obtain + Given the continuous integration workflow + When a pull request is opened from a fork + Then no lane selects a paid runner + And every fork-reachable lane selects the hosted fallback + + Scenario: a push to the default branch takes the owned arm + Given the continuous integration workflow + When a commit is pushed to the default branch + Then every reviewed lane selects the Ubicloud runner + + Scenario: a push does not schedule the pull-request-only lane + Given the continuous integration workflow + When a commit is pushed to the default branch + Then the pull-request-only lane is absent + + Scenario: a pull request does schedule the pull-request-only lane + Given the continuous integration workflow + When a pull request is opened from a branch of this repository + Then the pull-request-only lane is present + + Scenario: a tag push reaches the wheel jobs on the paid runner + Given the release workflow + When a tag is pushed + Then the wheel jobs reached through the called workflow select it too + And the native wheel matrix keeps its platform runners + + Scenario Outline: every selected runner is a schedulable label + Given the continuous integration workflow + When the event is evaluated + Then every selected label is a concrete runner name + + Examples: + | event | + | fork | + | owned | + | push | diff --git a/tests/helpers/ci_cache_families.py b/tests/helpers/ci_cache_families.py index cda8079ca..00b43ca9c 100644 --- a/tests/helpers/ci_cache_families.py +++ b/tests/helpers/ci_cache_families.py @@ -19,6 +19,7 @@ import re import typing as typ +from tests.helpers.ci_placement import placement from tests.helpers.ci_workflows import job, save_steps, step_inputs, steps if typ.TYPE_CHECKING: @@ -43,6 +44,13 @@ #: ``runner.environment`` renders from the runner label, and the two lanes read #: different cache services, so an archive can never cross between them. +#: +#: A fork-fallback lane resolves to two labels and therefore to two lanes. The +#: lane an archive is *published* to is the owned one, because every save step +#: here is guarded on a push to ``refs/heads/main`` and a fork's pull request is +#: neither a push nor on that ref. That premise is not assumed: it is asserted +#: by ``test_saves_happen_only_on_trunk_and_only_after_a_miss``, and removing +#: the guard from a save fails it. LANE_OF_LABEL: typ.Final[cabc.Mapping[str, str]] = { "ubicloud-standard-2": "self-hosted", "ubicloud-standard-4": "self-hosted", @@ -84,8 +92,15 @@ def __str__(self) -> str: def _lane(workflow_name: str, job_name: str) -> str: - """Return the cache lane a job's runner label resolves to.""" - label = job(workflow_name, job_name).get("runs-on") + """Return the cache lane a job publishes its archives to.""" + label = placement(workflow_name, job_name).owned + _require( + condition=label is not None, + message=( + f"{workflow_name}:{job_name} has no single owned runner, so the " + "lane its archives land in is undefined" + ), + ) _require( condition=label in LANE_OF_LABEL, message=f"{workflow_name}:{job_name} runs on unmapped label {label!r}", diff --git a/tests/helpers/ci_events.py b/tests/helpers/ci_events.py new file mode 100644 index 000000000..2d35a809e --- /dev/null +++ b/tests/helpers/ci_events.py @@ -0,0 +1,190 @@ +"""Resolve the runner each workflow event actually selects. + +The placement contracts read declarations. This module evaluates them: given an +event, it resolves every job's `runs-on` to the concrete label GitHub would +schedule on, following a reusable-workflow call into its callee, so the +`ci.yml` to `build-wheels.yml` path is exercised rather than asserted a job at +a time. + +`act` is not the tool for this. It skips a job whose label it cannot map and +exits zero, so an Ubicloud lane would be silently unexercised and the run would +report success: a harness that cannot fail on the thing it exists to check. +Evaluating the expression against an event payload is the part that can be +modelled honestly without a runner, and it is the part that decides whether a +fork's pull request can be scheduled at all. +""" + +from __future__ import annotations + +import pathlib as pth +import re +import typing as typ + +from tests.helpers.ci_placement import declares_steps, placement +from tests.helpers.ci_workflows import job, jobs + +if typ.TYPE_CHECKING: + import collections.abc as cabc + +#: A pull request opened from a fork of this repository. +FORK_PULL_REQUEST: typ.Final = { + "event_name": "pull_request", + "fork": True, + "ref": "refs/pull/1/merge", +} +#: A pull request opened from a branch of this repository. +OWNED_PULL_REQUEST: typ.Final = { + "event_name": "pull_request", + "fork": False, + "ref": "refs/pull/1/merge", +} +#: A push to the default branch. There is no pull-request payload, so the fork +#: field is absent, which GitHub evaluates as falsy. +PUSH_TO_MAIN: typ.Final = { + "event_name": "push", + "fork": None, + "ref": "refs/heads/main", +} +#: A tag push, which is what reaches `release.yml`. +TAG_PUSH: typ.Final = {"event_name": "push", "fork": None, "ref": "refs/tags/v1.2.3"} + +#: A comparison this evaluator can decide: one context property this module +#: models, against a quoted literal. +_COMPARISON = re.compile( + r"\A\s*github\.(?Pevent_name|ref)\s*(?P==|!=)\s*" + r"'(?P[^']*)'\s*\Z" +) +#: Everything else in a condition is a run-time value this harness does not +#: model: `needs.*` outcomes, `inputs.*`, `github.actor`. Treating them as true +#: keeps the schedule a superset of what runs, which is the safe direction for +#: a placement assertion: it never claims a job is absent when it runs. +_UNKNOWN: typ.Final = True + + +def _decide(term: str, event: cabc.Mapping[str, object]) -> bool: + """Evaluate one `&&`-joined term against the event.""" + match = _COMPARISON.match(term) + if match is None: + return _UNKNOWN + actual = event.get(match.group("field")) + equal = actual == match.group("literal") + return equal if match.group("operator") == "==" else not equal + + +def runs_on_event(condition: object, event: cabc.Mapping[str, object]) -> bool: + """Report whether a job's own ``if:`` admits this event. + + GitHub skips `ci.yml:coverage` on a push, because it declares + `github.event_name == 'pull_request'`. A schedule that listed it anyway + would say the push event places a job the push event never runs. + + Only `github.event_name` and `github.ref` comparisons are decided. Every + other term is left to :data:`_UNKNOWN`, including any term a parenthesized + expression splits into, since none of those match the comparison pattern + either. An explicit parenthesis check stood here until a mutation showed it + could not change an outcome: the unknown-term path already covers it, and + an unreachable guard is worse than none because it reads as protection. + + This harness models the event, not `needs` outcomes or dispatch inputs, so + a schedule that is a superset of what runs never claims a job is absent + when it is present. + + Returns + ------- + bool + Whether the event admits the job. + """ + if not isinstance(condition, str): + return True + text = " ".join(condition.split()) + expression = re.fullmatch(r"\$\{\{(?P.*)\}\}", text, re.DOTALL) + if expression is not None: + text = " ".join(expression.group("body").split()) + return any( + all(_decide(term, event) for term in alternative.split("&&")) + for alternative in text.split("||") + ) + + +class Scheduled(typ.NamedTuple): + """One job as an event would schedule it. + + Attributes + ---------- + workflow: + The workflow the job is declared in, which is the callee for a job + reached through a reusable-workflow call. + job: + The job key within that workflow. + labels: + The label or labels the event selects. A matrix job selects one per + leg; every other job selects exactly one. + """ + + workflow: str + job: str + labels: tuple[str, ...] + + +def _select( + workflow_name: str, job_name: str, event: cabc.Mapping[str, object] +) -> tuple[str, ...]: + """Return the labels one event selects for one job.""" + placed = placement(workflow_name, job_name) + if placed.kind == "matrix": + return tuple(sorted(placed.labels)) + if placed.kind == "literal": + return (typ.cast("str", placed.owned),) + # The fork fallback. GitHub evaluates the condition against the event + # payload: absent on a push, false on an owned pull request, true on a + # fork's. Only a truthy value takes the hosted arm. + takes_fork_arm = bool(event.get("fork")) + return (typ.cast("str", placed.fork if takes_fork_arm else placed.owned),) + + +def schedule(workflow_name: str, event: cabc.Mapping[str, object]) -> list[Scheduled]: + """Resolve every job one event schedules, following reusable-workflow calls. + + Parameters + ---------- + workflow_name: + The workflow the event triggers. + event: + One of the event constants in this module. + + Returns + ------- + list[Scheduled] + Every job the event would schedule, with a called workflow's jobs + reported under the callee's name rather than the caller's, because that + is where their runners are declared. + + Notes + ----- + A called workflow inherits its caller's `github` context, so the same event + is passed through the call unchanged. That inheritance is the reason the + fork expression works inside `build-wheels.yml` without an extra input, and + modelling the call is what makes that claim testable. + """ + scheduled: list[Scheduled] = [] + for job_name in jobs(workflow_name): + declared = job(workflow_name, job_name) + if not runs_on_event(declared.get("if"), event): + continue + if declares_steps(workflow_name, job_name): + scheduled.append( + Scheduled( + workflow_name, job_name, _select(workflow_name, job_name, event) + ) + ) + continue + called = declared.get("uses") + if not isinstance(called, str) or not called.startswith("./.github/workflows/"): + # A caller of a workflow outside this repository declares no runner + # this harness can resolve, and its callee is not ours to model. + continue + # `PurePosixPath` rather than `rsplit`: a workflow reference is a POSIX + # path and the separator is part of its grammar, not a character to + # split on. + scheduled.extend(schedule(pth.PurePosixPath(called).name, event)) + return scheduled diff --git a/tests/helpers/ci_job_rules.py b/tests/helpers/ci_job_rules.py new file mode 100644 index 000000000..0333b3aba --- /dev/null +++ b/tests/helpers/ci_job_rules.py @@ -0,0 +1,132 @@ +"""Rules about a job that are not about which runner it selects. + +Split from `ci_placement`, which models `runs-on`. These three read the other +declarations a placement contract depends on: the ceiling that bounds a job, +whether its own guard lets it run, and which context properties a declaration +reads, which is what the job-name stability rule compares. + +Each is a predicate rather than an assertion inside a contract, so it can be +driven directly over the values this repository does not happen to declare. +That is not a stylistic preference: a rule parametrized over correct sources +passes whether or not it discriminates, and two of these survived their own +mutations until they were extracted and tested this way. +""" + +from __future__ import annotations + +import re +import typing as typ + +from tests.helpers.ci_placement import EXPRESSION, require +from tests.helpers.ci_workflows import job + +#: One `${{ ... }}` span. Non-greedy to the first `}}`, because `[^}]*` stops +#: at the `}` inside `format('{0}', matrix.os)` and reports no reference at all. +_EXPRESSION_SPAN = re.compile(r"\$\{\{(?P.*?)\}\}", re.DOTALL) +#: A quoted literal inside an expression body. Stripped before references are +#: read, so a label such as `'ubuntu-22.04'` is not mistaken for a property +#: path on the strength of its dot. +_QUOTED = re.compile(r"'[^']*'|\"[^\"]*\"") +#: A dotted context property such as `matrix.os` or +#: `github.event.pull_request.head.repo.fork`. A bare function name such as +#: `format` carries no dot and is correctly not a reference. +_PROPERTY = re.compile(r"[A-Za-z_][\w-]*(?:\.[\w-]+)+") +#: Expressions a job-level ``if:`` may not reduce to. A job that cannot run +#: satisfies every declaration-reading rule while gating nothing. +#: GitHub Actions evaluates `false`, numeric zero, the empty string and `null` +#: as falsy, and **a non-empty string as truthy**, so the quoted `'false'` +#: belongs on the other side of this line rather than in it. +_NEVER_RUNS: typ.Final = frozenset({"false", "0", "0.0", "''", '""', "null"}) + + +def ceiling(workflow_name: str, job_name: str) -> int: + """Return a job's declared ``timeout-minutes``, refusing an unusable value. + + GitHub requires a positive integer. Two spellings pass an ``isinstance`` + check and should not: YAML's ``true`` is an ``int`` to Python, and zero and + negatives are integers that bound nothing. + + Extracted from the contract so the rule can be driven directly. Asserted + against this repository's own correct workflows it would pass whether or + not it discriminated, and a mutation from ``type(...) is int`` back to + ``isinstance`` survived until this existed. + + Returns + ------- + int + The declared ceiling, guaranteed a positive, non-boolean integer. + + Notes + ----- + Fails the contract, through :func:`_require`, when the value is absent, + boolean, a non-integer, or not positive. + """ + declared = job(workflow_name, job_name).get("timeout-minutes") + where = f"{workflow_name}:{job_name}" + require( + condition=type(declared) is int, + message=( + f"{where} must declare an integer timeout-minutes, got {declared!r}; " + "note YAML's `true` is an int to `isinstance`" + ), + ) + value = typ.cast("int", declared) + require( + condition=value > 0, + message=f"{where} must declare a positive timeout-minutes, got {value!r}", + ) + return value + + +def references(value: object) -> frozenset[str]: + """Return every context property a declaration reads. + + Property paths rather than whole expression bodies. A name built with + ``${{ format('{0}', matrix.os) }}`` reads ``matrix.os`` and has to be + comparable with a ``runs-on`` reading the same key; returning the body made + that comparison miss entirely, so an unstable name passed the stability + rule. + + Returns + ------- + frozenset[str] + Every dotted context property the value reads, across every + ``${{ ... }}`` span, with quoted literals excluded. + """ + if not isinstance(value, str): + return frozenset() + found: set[str] = set() + for body in _EXPRESSION_SPAN.findall(value): + found.update(_PROPERTY.findall(_QUOTED.sub(" ", body))) + return frozenset(found) + + +def never_runs(workflow_name: str, job_name: str) -> bool: + """Report whether a job's own ``if:`` makes it unreachable. + + A constant-false guard leaves every declaration intact and runs nothing, + which is exactly the state a placement contract must refuse rather than + report as compliant. + + Returns + ------- + bool + Whether the job's own ``if:`` reduces to a constant false. + """ + condition = job(workflow_name, job_name).get("if") + # YAML types an unquoted `if: false` or `if: 0` before GitHub sees it, so + # the commonest way to switch a job off arrives as a bool or a number, + # not as text. Its truthiness is what GitHub evaluates. + if not isinstance(condition, str): + return isinstance(condition, (bool, int, float)) and not condition + text = " ".join(condition.split()) + expression = EXPRESSION.match(text) + if expression is not None: + text = " ".join(expression.group("body").split()) + lowered = text.lower() + if lowered in _NEVER_RUNS: + return True + # A containment test would accept `false && matrix.target == 'x'`, so the + # leading clause is matched explicitly, and for every falsy literal rather + # than only `false`: `null && ...` and `0 && ...` are equally unreachable. + return any(lowered.startswith(f"{literal} &&") for literal in _NEVER_RUNS) diff --git a/tests/helpers/ci_placement.py b/tests/helpers/ci_placement.py new file mode 100644 index 000000000..a58a9fb43 --- /dev/null +++ b/tests/helpers/ci_placement.py @@ -0,0 +1,334 @@ +"""Read where a Cuprum CI job can run, and whether it runs at all. + +A ``runs-on`` declaration stopped being a label the moment the fork fallback +landed. A pull request from a fork cannot obtain an Ubicloud runner, so every +fork-reachable lane selects its runner from an expression, and a reader that +narrows that value with ``str()`` sees one opaque label that carries no vendor +prefix. Axinite #372 recorded what that costs: the Ubicloud classifier said no, +the job was dropped before any assertion ran, and the lane sat exempt from the +placement, ceiling, and registry contracts while still asking for a paid runner. + +This module therefore fails towards refusal. It models the three shapes the +repository actually declares and rejects everything else by name, so an +unreadable declaration reddens a contract instead of quietly satisfying one: + +``literal`` + A bare label, such as ``windows-2022``. +``fork`` + The canonical fork fallback, read **by position**. Nile-valley #106's + contract asserted only that the expression named one hosted and one + Ubicloud label somewhere, so swapping the arms passed while sending forks + to a runner they cannot obtain. +``matrix`` + ``${{ matrix. }}``, resolved through the ``include`` entries it + selects. Rstest-bdd #788 showed why the resolution matters: a reader that + stops at ``matrix.os`` never sees the expression the matrix value holds. + +Whether a job *runs* is modelled here too. Lille #349's placement, budget and +cache rules all read a job's declared configuration and none read whether it +executes, so ``if: false`` on a build lane satisfied every one of them while +running nothing. +""" + +from __future__ import annotations + +import re +import typing as typ + +from tests.helpers.ci_workflows import job, jobs, workflow_sources + +if typ.TYPE_CHECKING: + import collections.abc as cabc + +#: The field the fork fallback branches on. A sibling such as +#: ``head.repo.private`` is the correct-variant mutation: an expression that +#: reads it is well-formed and wrong, and a contract that merely pattern-matches +#: a conditional accepts it. +FORK_FIELD = "github.event.pull_request.head.repo.fork" + +#: GitHub-hosted labels this repository is permitted to use. Named rather than +#: derived from a vendor prefix: a ``ubicloud-`` prefix filter would silently +#: exempt a second paid provider's labels from the registry question, which is +#: the substantive defect. The two agree over today's workflows and differ on +#: exactly the case that matters. +FROZEN_HOSTED_LABELS: typ.Final = ( + "macos-15-intel", + "macos-latest", + "ubuntu-20.04", + "ubuntu-22.04", + "ubuntu-latest", + "windows-2022", + "windows-latest", +) + +# Anchored with no tolerance for surrounding characters. GitHub interpolates +# into the surrounding string and keeps what is around the expression, so +# `runs-on: " ${{ ... }}"` resolves to a label with a leading space that no +# runner matches. Tolerating the padding would report such a lane as a correct +# fork placement, a false positive in the dangerous direction; anything padded +# falls to the interpolation refusal instead. +EXPRESSION = re.compile(r"^\$\{\{(?P.*)\}\}$", re.DOTALL) +#: An expression *fragment*, used to refuse a value that interpolates without +#: being wholly an expression, whether padded or concatenated. +_INTERPOLATION = re.compile(r"\$\{\{") +#: The canonical fork fallback, anchored end to end so the arms are read by +#: position rather than by membership. `\A`/`\Z` rather than a bare `fullmatch` +#: on a pattern that could otherwise be satisfied by a prefix. +_FORKEXPRESSION = re.compile( + # Hyphens belong in the character class: `matrix.python-version` is a + # legitimate condition, and excluding `-` refused it as unmodellable. Found + # by the generated property, not by any example anyone wrote down. + r"\A\s*(?P[A-Za-z_][\w.-]*)" + r"\s*&&\s*'(?P[^']+)'" + r"\s*\|\|\s*'(?P[^']+)'\s*\Z" +) +_MATRIX_EXPRESSION = re.compile(r"\A\s*matrix\.(?P[\w-]+)\s*\Z") + + +def require(*, condition: bool, message: str) -> None: + """Raise a contract failure when ``condition`` does not hold. + + Public because `ci_job_rules` shares it; the alternative was a third copy + of the same three lines. + + Raises + ------ + AssertionError + When ``condition`` is false, carrying ``message``. + """ + if not condition: + raise AssertionError(message) + + +class Placement(typ.NamedTuple): + """Where one job can run. + + Attributes + ---------- + kind: + ``literal``, ``fork`` or ``matrix``; the shape the declaration took. + owned: + The label selected when the fork arm is not taken, or ``None`` for a + matrix placement, which has no single owned label. Cache lanes resolve + through this, which is sound only because every save step in this + repository is guarded on a push to ``refs/heads/main``, an event a fork + can never produce. ``test_saves_happen_only_on_trunk_and_only_after_a_miss`` + holds that premise. + fork: + The label a fork's pull request takes, or ``None`` when the job + declares no fallback. + labels: + Every label the declaration can resolve to. + references: + Every expression reference the declaration reads, resolved through the + matrix values it selects. + """ + + kind: str + owned: str | None + fork: str | None + labels: frozenset[str] + references: frozenset[str] + + +def declares_steps(workflow_name: str, job_name: str) -> bool: + """Report whether a job runs its own steps rather than calling a workflow. + + A job that declares ``uses`` is a reusable-workflow caller: it declares no + runner of its own and GitHub rejects ``timeout-minutes`` on it, so placing + and bounding it are the callee's business. Keying the exemption on ``uses`` + rather than on a missing ``runs-on`` means a job with neither is still + refused (whitaker #438). + + Returns + ------- + bool + Whether the job declares its own steps and no ``uses`` key. + """ + declared = job(workflow_name, job_name) + return "uses" not in declared and isinstance(declared.get("steps"), list) + + +def all_jobs() -> list[tuple[str, str]]: + """Return every job in the estate as a ``(workflow, job)`` pair. + + Partitioning matters more than it looks: filtering to the jobs a manifest + recognizes sees nothing else, which is how an unclassified lane acquires a + runner unreviewed. + + Returns + ------- + list[tuple[str, str]] + Every job in the estate, in workflow and declaration order. + """ + return [ + (workflow_name, job_name) + for workflow_name, _ in workflow_sources() + for job_name in jobs(workflow_name) + ] + + +def _matrix_values(workflow_name: str, job_name: str, key: str) -> list[object]: + """Return the values one matrix key takes across the job's ``include``.""" + strategy = job(workflow_name, job_name).get("strategy") + require( + condition=isinstance(strategy, dict), + message=( + f"{workflow_name}:{job_name} selects its runner from matrix.{key} " + "but declares no strategy" + ), + ) + matrix = typ.cast("dict[str, object]", strategy).get("matrix") + require( + condition=isinstance(matrix, dict), + message=f"{workflow_name}:{job_name} strategy declares no matrix", + ) + declared = typ.cast("dict[str, object]", matrix) + # Two shapes reach here. A plain matrix lists the key's values directly + # (`os: [ubuntu-latest, windows-2022]`); an `include` matrix lists legs and + # the key sits inside each. Modelling only `include` refused + # `rust-boundaries.yml:native` outright, which fails safe but rejects a + # perfectly ordinary workflow. + direct = declared.get(key) + if isinstance(direct, list): + # `include` can still add a leg with a new value for the key, and that + # leg runs too. An entry without the key only extends existing legs. + extra = declared.get("include", []) + require( + condition=isinstance(extra, list) + and all(isinstance(leg, dict) for leg in extra), + message=f"{workflow_name}:{job_name} declares an unreadable include", + ) + return list(typ.cast("list[object]", direct)) + [ + leg[key] for leg in typ.cast("list[dict[str, object]]", extra) if key in leg + ] + include = declared.get("include") + require( + condition=isinstance(include, list), + message=( + f"{workflow_name}:{job_name} declares a matrix this reader cannot " + f"expand; the key {key!r} is neither a list nor present in an " + "`include` list" + ), + ) + legs = typ.cast("list[object]", include) + values: list[object] = [] + for index, leg in enumerate(legs): + require( + condition=isinstance(leg, dict) and key in leg, + message=( + f"{workflow_name}:{job_name} matrix leg {index} declares no " + f"{key!r}, so its runner is unreadable" + ), + ) + values.append(typ.cast("dict[str, object]", leg)[key]) + return values + + +def _literal_labels( + workflow_name: str, job_name: str, values: cabc.Iterable[object] +) -> frozenset[str]: + """Narrow matrix runner values to literal labels, refusing expressions.""" + labels: set[str] = set() + for value in values: + require( + # Any interpolation, not only a wholly-expression value: + # `ubuntu-${{ inputs.release }}` runs on a label nobody wrote down. + condition=isinstance(value, str) and _INTERPOLATION.search(value) is None, + message=( + f"{workflow_name}:{job_name} resolves its runner to {value!r}; " + "a matrix value holding an expression is a placement this " + "reader cannot model, and recording it as a label would exempt " + "the lane from every placement rule" + ), + ) + labels.add(str(value)) + return frozenset(labels) + + +def placement(workflow_name: str, job_name: str) -> Placement: + """Model one job's runner selection, refusing any shape it cannot read. + + Returns + ------- + Placement + The shape read, its arms, every label it can resolve to, and every + expression reference it reads. + + Raises + ------ + AssertionError + When the declaration is missing on a step-declaring job, is a list, is + an expression in neither the fork nor the matrix shape, or parses with + an embedded line break. + """ + where = f"{workflow_name}:{job_name}" + declared = job(workflow_name, job_name).get("runs-on") + require( + condition=not isinstance(declared, list), + message=( + f"{where} declares a list runs-on. Each entry would be recorded as " + "a label, so a runner group or an entry holding an expression would " + "read as a job placed on two runners, one named after the " + "expression (dev-env-rocky #216). Refused rather than modelled." + ), + ) + require( + condition=isinstance(declared, str) and bool(declared), + message=f"{where} must declare a non-empty string runs-on, got {declared!r}", + ) + text = str(declared) + # The folded-scalar hazard. A continuation indented one level deeper keeps + # its line break, and GitHub evaluates the broken value regardless, so a + # green run is not evidence that the expression is one line. + require( + condition="\n" not in text, + message=( + f"{where} runs-on parses with an embedded line break: {text!r}. " + "Keep a folded-scalar continuation at the same indent." + ), + ) + match = EXPRESSION.match(text) + if match is None: + # A value that interpolates without being wholly an expression, such as + # `ubuntu-${{ matrix.release }}`, is not a literal label. Recording it + # as one is the same defect this reader exists to prevent, wearing a + # different shape: the label it resolves to at run time is invisible to + # every placement and registry assertion. + require( + condition=_INTERPOLATION.search(text) is None, + message=( + f"{where} interpolates its runner label: {text!r}. The labels " + "this can resolve to are unreadable, so it is refused rather " + "than recorded as one literal." + ), + ) + return Placement("literal", text, None, frozenset({text}), frozenset()) + body = match.group("body") + fork = _FORKEXPRESSION.match(body) + if fork is not None: + return Placement( + "fork", + fork.group("owned_arm"), + fork.group("fork_arm"), + frozenset({fork.group("owned_arm"), fork.group("fork_arm")}), + frozenset({fork.group("condition")}), + ) + matrix = _MATRIX_EXPRESSION.match(body) + if matrix is not None: + key = matrix.group("key") + values = _matrix_values(workflow_name, job_name, key) + return Placement( + "matrix", + None, + None, + _literal_labels(workflow_name, job_name, values), + frozenset({f"matrix.{key}"}), + ) + message = ( + f"{where} selects its runner from an expression this reader cannot " + f"model: {text!r}. Recording it as one opaque label would carry no " + "vendor prefix, so the lane would be dropped by every Ubicloud " + "classifier while still asking for a paid runner (axinite #372)." + ) + raise AssertionError(message) diff --git a/tests/helpers/ci_runners.py b/tests/helpers/ci_runners.py index e22059abd..2402277e8 100644 --- a/tests/helpers/ci_runners.py +++ b/tests/helpers/ci_runners.py @@ -13,6 +13,15 @@ import typing as typ +from tests.helpers.ci_job_rules import ceiling, never_runs, references +from tests.helpers.ci_placement import ( + FORK_FIELD, + FROZEN_HOSTED_LABELS, + Placement, + all_jobs, + declares_steps, + placement, +) from tests.helpers.ci_workflows import ( CACHE_ACTION_PIN, CACHE_PLAIN, @@ -45,11 +54,37 @@ # workflows are meant to contain, `ci_workflows` reads what they do contain. # fmt: off __all__ = ( - "CACHE_ACTION_PIN", "CACHE_PLAIN", "CACHE_RESTORE", "CACHE_SAVE", - "ROOT", "WORKFLOW_DIR", "cache_paths", "cache_steps", "expand", "job", - "job_env", "jobs", "restore_steps", "save_steps", - "single_step_position_using", "single_step_using", "step_inputs", - "steps", "workflow_document", "workflow_env", "workflow_sources", + "CACHE_ACTION_PIN", + "CACHE_PLAIN", + "CACHE_RESTORE", + "CACHE_SAVE", + "FORK_FIELD", + "FROZEN_HOSTED_LABELS", + "ROOT", + "RUNNER_NAMED_JOBS", + "WORKFLOW_DIR", + "Placement", + "all_jobs", + "cache_paths", + "cache_steps", + "ceiling", + "declares_steps", + "expand", + "job", + "job_env", + "jobs", + "never_runs", + "placement", + "references", + "restore_steps", + "save_steps", + "single_step_position_using", + "single_step_using", + "step_inputs", + "steps", + "workflow_document", + "workflow_env", + "workflow_sources", ) # fmt: on @@ -98,14 +133,49 @@ #: and does real work, which is what buys it a paid runner. UBICLOUD_JOBS: typ.Final[cabc.Mapping[str, tuple[str, ...]]] = { "build-wheels.yml": ("build-pure-wheel", "verify-wheel-install"), - "ci.yml": ("typecheck-test", "extension-tests", "coverage", "benchmark-ratchet"), + "ci.yml": ( + "lint-test", + "typecheck-test", + "extension-tests", + "coverage", + "benchmark-ratchet", + ), "coverage-main.yml": ("coverage-upload",), } +#: Ubicloud lanes a pull request from a fork can reach, which must therefore +#: declare the fallback arm. Derived intent, not derived fact: the workflows +#: are read back against it, and `test_fork_reachability_matches_the_manifest` +#: holds it against the triggers so a lane cannot quietly leave the set. +#: +#: `coverage-upload` is absent because `coverage-main.yml` triggers only on a +#: push to `main` and a dispatch, neither of which a fork can cause. The two +#: `build-wheels.yml` jobs are present because `ci.yml` calls that workflow on +#: every pull request, so its own `workflow_call` trigger understates its +#: exposure (weaver: a called workflow's triggers are its callers'). +FORK_REACHABLE_UBICLOUD_JOBS: typ.Final[cabc.Mapping[str, tuple[str, ...]]] = { + "build-wheels.yml": ("build-pure-wheel", "verify-wheel-install"), + "ci.yml": ( + "lint-test", + "typecheck-test", + "extension-tests", + "coverage", + "benchmark-ratchet", + ), +} +#: The one job permitted to fail without failing the workflow, and the matrix +#: key that says so. The 3.15a leg tracks a pre-release interpreter and is not +#: a required context; every other leg gates a merge. +EXPERIMENTAL_LEG_KEY = "experimental" +CONTINUE_ON_ERROR_JOBS: typ.Final = (("ci.yml", "typecheck-test"),) #: Jobs that stay on GitHub-hosted runners, and why. Ubicloud offers Linux #: only, and a job that sleeps, calls an API, or publishes an artefact someone #: else built gains nothing from a metered build slot. GITHUB_HOSTED_JOBS: typ.Final[cabc.Mapping[str, tuple[str, ...]]] = { - "ci.yml": ("lint-test", "changes", "loom-smoke"), + # `lint-test` moved to the Ubicloud manifest: it is a developer-blocking + # Linux gate that compiles, which is what buys a paid runner. `loom-smoke` + # and `workflow-harness` stay here, and `rust-boundaries.yml`'s verifier + # lanes stay by this repository's own decision recorded below. + "ci.yml": ("changes", "loom-smoke"), "benchmark-gate-harness.yml": ("workflow-harness",), "delayed-pr-comment.yml": ("delay_and_comment",), "loom.yml": ("loom",), @@ -113,6 +183,15 @@ # Issue379 requires verifier schedules on GitHub-hosted Linux. "rust-boundaries.yml": ("verus", "extended"), } +#: Matrix jobs whose check context spells the runner label, reviewed and +#: accepted. A name reading a matrix key is stable per event, which is the +#: property the stability rule protects, but the context still carries a label +#: and a placement change would rename it. Two of `build-native-wheels`' +#: contexts are named verbatim in the `main-required-checks` ruleset, which is +#: why moving its Linux legs is the repository owner's decision rather than a +#: label change; `native` is not a required context today. +RUNNER_NAMED_JOBS: typ.Final = (("rust-boundaries.yml", "native"),) + #: Windows-native validation needs GitHub's hosted Windows image; Ubicloud #: offers Linux capacity only. WINDOWS_HOSTED_JOBS: typ.Final[cabc.Mapping[str, tuple[str, ...]]] = { @@ -178,7 +257,11 @@ #: the GitHub-hosted lane and the Ubicloud lane render different values and #: read different cache services; a key with two writers has one on each side. CACHE_WRITERS: typ.Final[cabc.Mapping[str, tuple[tuple[str, str], ...]]] = { - "CARGO_CACHE_KEY": (("ci.yml", "extension-tests"), ("ci.yml", "lint-test")), + # `lint-test` restores this key and no longer saves it. On its owned arm + # it renders the `self-hosted` lane, which is the family `extension-tests` + # writes, and the registry holds the resolved dependency graph either job + # would have archived. + "CARGO_CACHE_KEY": (("ci.yml", "extension-tests"),), # The compiler cache is written by whichever job actually compiles, and # each compile shape is its own family. See CACHE_FAMILY_WRITERS: this # mapping only says which jobs hold a save step, not which archive each @@ -214,12 +297,11 @@ cabc.Mapping[tuple[str, str, tuple[str, ...]], tuple[str, str]] ] = { ("CARGO_CACHE_KEY", "self-hosted", ()): ("ci.yml", "extension-tests"), - ("CARGO_CACHE_KEY", "github-hosted", ()): ("ci.yml", "lint-test"), ("TOOL_CACHE_KEY", "self-hosted", ("3.12",)): ("ci.yml", "typecheck-test"), ("TOOL_CACHE_KEY", "self-hosted", ("3.13",)): ("ci.yml", "typecheck-test"), ("TOOL_CACHE_KEY", "self-hosted", ("3.14",)): ("ci.yml", "typecheck-test"), ("TOOL_CACHE_KEY", "self-hosted", ("3.15",)): ("ci.yml", "typecheck-test"), - ("SCCACHE_CACHE_KEY", "github-hosted", ("3.13", "lint")): ( + ("SCCACHE_CACHE_KEY", "self-hosted", ("3.13", "lint")): ( "ci.yml", "lint-test", ), diff --git a/tests/helpers/ci_workflows.py b/tests/helpers/ci_workflows.py index 2d771ff55..5ebe04b95 100644 --- a/tests/helpers/ci_workflows.py +++ b/tests/helpers/ci_workflows.py @@ -4,6 +4,13 @@ ``tests/helpers/ci_runners.py``; this module only reads the workflows back. Every accessor validates the shape it narrows, so a malformed workflow fails with a named diagnostic rather than an opaque ``TypeError`` deep in a test. + +Reading and parsing are fallible too, and every query in the contract helpers +reaches the filesystem through the two readers here, ``read_workflow`` and +``read_source``. They translate an I/O or YAML failure into an +``AssertionError`` naming the file, so a contract over an unreadable workflow +fails as a contract, citing the file, rather than raising an unattributed +parser error from whichever query happened to touch it first. """ from __future__ import annotations @@ -48,22 +55,83 @@ def _mapping(value: object, message: str) -> dict[str, object]: return typ.cast("dict[str, object]", value) -def workflow_document(workflow_name: str) -> dict[object, object]: - """Parse one repository workflow, without narrowing its top-level keys.""" - path = WORKFLOW_DIR / workflow_name +def read_source(path: Path) -> str: + """Return one workflow's source text, naming the file if it cannot be read. + + Parameters + ---------- + path : Path + The workflow file to read. + + Returns + ------- + str + The file's UTF-8 text. + + Raises + ------ + AssertionError + If the file cannot be read, naming the file and the ``OSError``. + + Examples + -------- + >>> "jobs:" in read_source(WORKFLOW_DIR / "ci.yml") + True + """ + try: + return path.read_text(encoding="utf-8") + except OSError as error: + message = f"{path.name} could not be read: {error}" + raise AssertionError(message) from error + + +def read_workflow(path: Path) -> dict[object, object]: + """Parse one workflow file, without narrowing its top-level keys. + + Parameters + ---------- + path : Path + The workflow file to parse. + + Returns + ------- + dict[object, object] + The parsed document, whose trigger key YAML 1.1 reads as ``True``. + + Raises + ------ + AssertionError + If the file cannot be read, is not valid YAML, or does not parse to a + mapping, naming the file in each case. + + Examples + -------- + >>> "jobs" in read_workflow(WORKFLOW_DIR / "ci.yml") + True + """ + source = read_source(path) + try: + document = yaml.safe_load(source) + except yaml.YAMLError as error: + message = f"{path.name} is not valid YAML: {error}" + raise AssertionError(message) from error # YAML 1.1 reads the `on:` trigger key as the boolean `True`, so the # document is genuinely not string-keyed and the return type says so. # Claiming `dict[str, object]` here would be a false contract that hides # the one key a caller cannot reach by name. Callers that need a # string-keyed mapping narrow one of its values through `_mapping`. - document = yaml.safe_load(path.read_text(encoding="utf-8")) _require( condition=isinstance(document, dict), - message=f"{workflow_name} must parse to a mapping", + message=f"{path.name} must parse to a mapping", ) return typ.cast("dict[object, object]", document) +def workflow_document(workflow_name: str) -> dict[object, object]: + """Parse one repository workflow, without narrowing its top-level keys.""" + return read_workflow(WORKFLOW_DIR / workflow_name) + + def workflow_env(workflow_name: str) -> dict[str, object]: """Return the workflow-level ``env`` mapping.""" return _mapping( @@ -259,9 +327,37 @@ def expand(manifest: cabc.Mapping[str, tuple[str, ...]]) -> list[tuple[str, str] ] -def workflow_sources() -> list[tuple[str, str]]: - """Return every workflow's name and source text.""" - return [ - (path.name, path.read_text(encoding="utf-8")) - for path in sorted(WORKFLOW_DIR.glob("*.yml")) - ] +def workflow_sources(directory: Path = WORKFLOW_DIR) -> list[tuple[str, str]]: + """Return every workflow's name and source text. + + Parameters + ---------- + directory : Path + The directory to sweep. It defaults to this repository's workflows; + the loader tests pass a temporary one to show that an unreadable file + fails by name. + + Returns + ------- + list[tuple[str, str]] + Each ``*.yml`` file's name and text, sorted by path. Every read goes + through ``read_source``, so an unreadable file fails by name. + + Raises + ------ + AssertionError + If ``directory`` is not a readable directory or holds no workflow. + ``Path.glob`` returns nothing for a missing directory, and an empty + sweep would satisfy every "no workflow does X" contract vacuously. + """ + _require( + condition=directory.is_dir(), + message=f"{directory} is not a workflow directory", + ) + try: + paths = sorted(directory.glob("*.yml")) + except OSError as error: + message = f"{directory} could not be listed: {error}" + raise AssertionError(message) from error + _require(condition=bool(paths), message=f"{directory} holds no workflow") + return [(path.name, read_source(path)) for path in paths] diff --git a/tests/test_ci_actionlint_registry.py b/tests/test_ci_actionlint_registry.py new file mode 100644 index 000000000..96abea936 --- /dev/null +++ b/tests/test_ci_actionlint_registry.py @@ -0,0 +1,141 @@ +"""Contract tests for the actionlint runner-label registry. + +actionlint knows GitHub's own labels natively but not a self-hosted provider's, +so an unregistered label is not a lint error: the job simply queues for a +runner that never arrives. The registry is therefore asserted as an equality in +both directions against the labels the workflows actually resolve to, because a +subset assertion misses a stale registration and a vendor-prefix derivation +would exempt a second paid provider's labels from the question entirely. +""" + +from __future__ import annotations + +import re + +import yaml + +from tests.helpers.ci_placement import FROZEN_HOSTED_LABELS, declares_steps, placement +from tests.helpers.ci_runners import ROOT, all_jobs, workflow_sources + +ACTIONLINT_CONFIG = ROOT / ".github" / "actionlint.yaml" +#: A `vars.*` reference in a workflow. actionlint checks such a reference +#: against `config-variables`, so the declared list and the references read +#: back from the workflows have to agree in both directions: an undeclared +#: name is a lint error, and a declared name nothing reads is a stale +#: allowance that would let a deleted variable's typo pass. +CONFIGURATION_VARIABLE = re.compile(r"vars\.([A-Za-z_][A-Za-z0-9_]*)") +ALL_CASES = all_jobs() +#: The GitHub-hosted labels this repository actually runs on, maintained +#: independently of `FROZEN_HOSTED_LABELS`. The frozen set is an allowance +#: list, so a misspelling added to both it and a matrix leg would satisfy every +#: assertion that reads one against the other; this second list has to be +#: edited too, and it is an equality rather than a membership test. +HOSTED_LABELS_IN_USE = frozenset({ + "macos-15-intel", + "macos-latest", + "ubuntu-latest", + "windows-2022", +}) + + +def _labels_in_use() -> frozenset[str]: + """Return every runner label the estate's jobs can resolve to. + + Derived from every job, both arms of a conditional and every matrix runner + entry, because a registry assertion is only as complete as its derivation. + Netsuke's contract searched the concatenated workflow text for each + registered label, so a label named only in a comment explaining why a lane + no longer used it stayed registered for ever. + + Returns + ------- + frozenset[str] + Every label any step-declaring job in the estate can resolve to. + """ + labels: set[str] = set() + for workflow_name, job_name in ALL_CASES: + if not declares_steps(workflow_name, job_name): + # A reusable-workflow caller declares no runner. Exempt by `uses`, + # not by a missing `runs-on`: a job with neither is still refused + # by the reader (whitaker #438). + continue + labels |= placement(workflow_name, job_name).labels + return frozenset(labels) + + +def test_actionlint_registers_exactly_the_self_hosted_labels_in_use() -> None: + """Register intentional labels so a typo fails lint instead of queueing. + + Equality in both directions. A subset assertion misses a stale + registration, and deriving "in use" by a vendor prefix would silently + exempt a second paid provider's labels from the registry question + altogether, which is the substantive defect; the named frozen set and the + prefix filter agree over today's workflows and differ on exactly that case. + actionlint knows GitHub's own labels natively, so the frozen set is + subtracted rather than registered (whitaker #438). + """ + config = yaml.safe_load(ACTIONLINT_CONFIG.read_text(encoding="utf-8")) + declared = config["self-hosted-runner"]["labels"] + used = _labels_in_use() + unknown = used - frozenset(FROZEN_HOSTED_LABELS) + assert sorted(declared) == sorted(unknown), ( + "actionlint must register every label in use that is neither a " + f"GitHub-hosted label nor already registered; declared {sorted(declared)}, " + f"derived {sorted(unknown)}" + ) + # Carried forward from main, which replaced a hardcoded + # `["CODESCENE_CLI_SHA256"]` with a derivation after that variable's last + # reader was deleted. Keeping the old literal through this rebase would + # have silently reverted that fix and re-asserted a variable nothing reads. + # + # Compared as sorted lists: the config registers names for lint to + # resolve, and actionlint does not care what order they appear in. Sorting + # still fails on a duplicate, so the comparison loses nothing. + read = { + match.group(1) + for _, source in workflow_sources() + for match in CONFIGURATION_VARIABLE.finditer(source) + } + assert sorted(config["config-variables"]) == sorted(read), ( + "list only the configuration variables the workflows read, so a typo " + f"in a vars.* reference fails lint; declared " + f"{config['config-variables']}, read {sorted(read)}" + ) + + +def test_every_hosted_label_in_use_is_named_in_the_frozen_set() -> None: + """Keep the frozen set a closed list rather than an implicit tolerance. + + Without this the registry equality above could be satisfied by widening + `FROZEN_HOSTED_LABELS` to swallow a paid provider's label, which is the + failure the named set exists to prevent. + """ + hosted = _labels_in_use() & frozenset(FROZEN_HOSTED_LABELS) + assert hosted == HOSTED_LABELS_IN_USE, ( + "the GitHub-hosted labels in use must match the independently " + f"maintained list; in use {sorted(hosted)}, expected " + f"{sorted(HOSTED_LABELS_IN_USE)}. A misspelt label added to both a " + "matrix leg and the frozen set fails here, where it would otherwise " + "leave a job queued for a runner that does not exist" + ) + config = yaml.safe_load(ACTIONLINT_CONFIG.read_text(encoding="utf-8")) + registered = frozenset(config["self-hosted-runner"]["labels"]) + assert not (hosted & registered), ( + "a GitHub-hosted label must not also be registered as self-hosted; " + f"both claim {sorted(hosted & registered)}" + ) + assert hosted == _labels_in_use() - registered, ( + "every label in use must be either registered or named in the frozen " + f"hosted set; unaccounted: {sorted(_labels_in_use() - registered - hosted)}" + ) + + +def test_no_retired_runner_labels_remain() -> None: + """Leave no Namespace label or cache action behind after the migration.""" + for workflow_name, source in workflow_sources(): + assert "namespace-profile" not in source, ( + f"{workflow_name} still references a Namespace runner profile" + ) + assert "nscloud" not in source, ( + f"{workflow_name} still references the Namespace cache action" + ) diff --git a/tests/test_ci_cache_families_helper.py b/tests/test_ci_cache_families_helper.py index 71dad1a82..9cf608298 100644 --- a/tests/test_ci_cache_families_helper.py +++ b/tests/test_ci_cache_families_helper.py @@ -17,6 +17,7 @@ from hypothesis import strategies as st from tests.helpers import ci_cache_families as families +from tests.helpers.ci_placement import Placement if typ.TYPE_CHECKING: from tests.helpers.workflow_types import Step @@ -133,8 +134,40 @@ def test_an_unmapped_runner_label_is_a_contract_failure( "uses": families.CACHE_KEYS_ACTION, "with": {"python-version": "3.13", "compiler-shape": "debug"}, } - monkeypatch.setattr(families, "job", lambda *_: {"runs-on": "some-new-shape"}) + monkeypatch.setattr( + families, + "placement", + lambda *_: Placement( + "literal", "some-new-shape", None, frozenset(), frozenset() + ), + ) monkeypatch.setattr(families, "steps", lambda *_: [renderer]) monkeypatch.setattr(families, "save_steps", lambda *_: []) with pytest.raises(AssertionError, match="unmapped label"): families.writer_families("w.yml", "j") + + +def test_a_job_without_one_owned_runner_has_no_cache_lane( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Refuse to place an archive for a job whose runner the matrix decides. + + A matrix placement resolves to several labels and therefore to several + lanes. Choosing one would attribute an archive to a store it may never + reach, so the reader refuses rather than guesses. + """ + renderer = { + "uses": families.CACHE_KEYS_ACTION, + "with": {"python-version": "3.13", "compiler-shape": "debug"}, + } + monkeypatch.setattr( + families, + "placement", + lambda *_: Placement( + "matrix", None, None, frozenset({"ubuntu-latest"}), frozenset() + ), + ) + monkeypatch.setattr(families, "steps", lambda *_: [renderer]) + monkeypatch.setattr(families, "save_steps", lambda *_: []) + with pytest.raises(AssertionError, match="no single owned runner"): + families.writer_families("w.yml", "j") diff --git a/tests/test_ci_event_conditions.py b/tests/test_ci_event_conditions.py new file mode 100644 index 000000000..9b944f454 --- /dev/null +++ b/tests/test_ci_event_conditions.py @@ -0,0 +1,129 @@ +"""Tests for the event-condition evaluator behind the runner-selection scenarios. + +`tests/behaviour/test_ci_runner_selection.py` exercises the evaluator through +this repository's workflows, which between them use one `==` guard on an event +name and little else. That left the `!=` branch unexercised: inverting the +comparison passed every scenario, because no scheduled workflow depends on it. + +The predicate is the mechanism, so it is driven here directly, over both +operators, both modelled fields, the `${{ }}` wrapper, and the operands this +harness deliberately declines to model. +""" + +from __future__ import annotations + +import pytest + +from tests.helpers.ci_events import ( + OWNED_PULL_REQUEST, + PUSH_TO_MAIN, + TAG_PUSH, + runs_on_event, +) + + +@pytest.mark.parametrize( + ("condition", "event", "admitted"), + [ + pytest.param( + "github.event_name == 'pull_request'", + OWNED_PULL_REQUEST, + True, + id="equal-match", + ), + pytest.param( + "github.event_name == 'pull_request'", + PUSH_TO_MAIN, + False, + id="equal-mismatch", + ), + # The case the scenarios could not reach. `rust-boundaries.yml`'s + # `extended` job is guarded exactly this way, and inverting the + # operator passed every scenario until this table existed. + pytest.param( + "github.event_name != 'pull_request'", + PUSH_TO_MAIN, + True, + id="not-equal-match", + ), + pytest.param( + "github.event_name != 'pull_request'", + OWNED_PULL_REQUEST, + False, + id="not-equal-mismatch", + ), + pytest.param( + "github.ref == 'refs/heads/main'", PUSH_TO_MAIN, True, id="ref-match" + ), + pytest.param( + "github.ref == 'refs/heads/main'", TAG_PUSH, False, id="ref-mismatch" + ), + pytest.param( + "${{ github.event_name == 'push' }}", PUSH_TO_MAIN, True, id="wrapped" + ), + pytest.param( + "github.event_name == 'schedule' || github.event_name == 'push'", + PUSH_TO_MAIN, + True, + id="alternative-admits", + ), + pytest.param( + "github.event_name == 'schedule' || " + "github.event_name == 'workflow_dispatch'", + PUSH_TO_MAIN, + False, + id="no-alternative-admits", + ), + pytest.param( + "github.event_name == 'push' && github.ref == 'refs/heads/main'", + PUSH_TO_MAIN, + True, + id="conjunction-admits", + ), + pytest.param( + "github.event_name == 'push' && github.ref == 'refs/heads/main'", + TAG_PUSH, + False, + id="conjunction-refuses", + ), + ], +) +def test_event_conditions_are_evaluated( + condition: str, event: dict[str, object], *, admitted: bool +) -> None: + """Decide the comparisons this harness models, on both operators.""" + assert runs_on_event(condition, event) is admitted, ( + f"{condition!r} against {event['event_name']!r} must be " + f"{'admitted' if admitted else 'refused'}" + ) + + +@pytest.mark.parametrize( + "condition", + [ + pytest.param("needs.changes.outputs.bench == 'true'", id="needs-output"), + pytest.param("inputs.workflow-harness", id="dispatch-input"), + pytest.param("github.actor == 'dependabot[bot]'", id="actor"), + pytest.param( + "needs.changes.result == 'success' && (github.event_name != " + "'pull_request' || needs.changes.outputs.bench == 'true')", + id="parenthesized", + ), + pytest.param(None, id="no-condition"), + pytest.param(True, id="non-string"), + ], +) +def test_unmodelled_conditions_admit_the_job(condition: object) -> None: + """Admit a job whose guard this harness does not model. + + `needs` outcomes, dispatch inputs and the actor are run-time values. A + parenthesized expression reaches the same place by a different route: the + terms it splits into do not match the comparison pattern either, so no + special case is needed and none exists. Every one of them leaves the + schedule a superset of what actually runs, which is the safe direction + here: it can overstate what an event places, but it never claims a job is + absent when it runs, and absence is what the scenarios assert on. + """ + assert runs_on_event(condition, PUSH_TO_MAIN), ( + f"{condition!r} is not modelled, so the job must still be scheduled" + ) diff --git a/tests/test_ci_placement_guards.py b/tests/test_ci_placement_guards.py new file mode 100644 index 000000000..33c74cba8 --- /dev/null +++ b/tests/test_ci_placement_guards.py @@ -0,0 +1,253 @@ +"""Tests for the reader's guards: whether a job runs, and what it reads. + +Split from `test_ci_placement_reader.py`, which reads runner *shapes*. These +cover the two questions asked alongside the shape: whether a job's own guard +lets it run at all, and which context properties a declaration reads, which is +what the job-name stability rule compares. + +Both are driven with synthetic declarations rather than this repository's own +correct workflows, because a rule parametrized over correct sources passes +whether or not it discriminates. +""" + +from __future__ import annotations + +import pytest + +from tests.helpers import ci_job_rules as rules +from tests.helpers import ci_placement as reader + +OWNED = "ubicloud-standard-2" +HOSTED = "ubuntu-latest" +#: Every workflow the traversal must reach. Named rather than derived, so the +#: claim that `all_jobs` sees the whole estate is asserted rather than merely +#: true: a file the reader skipped would otherwise be invisible. +ESTATE_WORKFLOWS = frozenset({ + "benchmark-gate-harness.yml", + "build-wheels.yml", + "ci.yml", + "coverage-main.yml", + "delayed-pr-comment.yml", + "dependabot-automerge.yml", + "loom.yml", + "mutation-testing.yml", + "release.yml", + "rust-boundaries.yml", +}) + + +def _declare(monkeypatch: pytest.MonkeyPatch, declared: dict[str, object]) -> None: + """Make both readers see one synthetic job declaration. + + The shape reader and the job rules each import `job` into their own + namespace, so patching one would leave the other reading the repository's + real workflows and the test would silently assert nothing. + """ + monkeypatch.setattr(reader, "job", lambda *_: declared) + monkeypatch.setattr(rules, "job", lambda *_: declared) + + +@pytest.mark.parametrize( + "condition", + [ + "false", + "${{ false }}", + "0", + "0.0", + "''", + '""', + "null", + "false && matrix.target == 'x'", + "null && matrix.target == 'x'", + "0 && matrix.target == 'x'", + # YAML types these before GitHub sees them: an unquoted `if: false` + # arrives as a bool, and `if: 0` as a number. + False, + 0, + 0.0, + ], +) +def test_a_constant_false_guard_is_reported( + monkeypatch: pytest.MonkeyPatch, condition: object +) -> None: + """Catch a lane that satisfies every declaration rule and runs nothing.""" + _declare(monkeypatch, {"runs-on": HOSTED, "steps": [], "if": condition}) + assert rules.never_runs("w.yml", "j"), ( + f"{condition!r} can never be true, so the job gates nothing" + ) + + +@pytest.mark.parametrize( + "condition", + [ + "github.event_name == 'pull_request'", + "needs.changes.outputs.bench == 'true'", + "${{ !cancelled() }}", + "matrix.python-suite", + # GitHub treats a non-empty string as truthy, so a quoted `false` is a + # job that runs. Refusing it would be a false positive on a lane that + # executes, which is why it sits here rather than above. + "'false'", + '"false"', + "falsey", + "false_positive_guard", + True, + 1, + ], +) +def test_a_real_guard_is_not_reported_as_never_running( + monkeypatch: pytest.MonkeyPatch, condition: object +) -> None: + """Prove the rule narrow: a legitimate condition must still pass. + + Sufficiency alone is not enough. A guard rule that also refused the + conditions this repository genuinely uses would be switched off, and a + contract with a false positive gates nothing (femtologging #480). + """ + _declare(monkeypatch, {"runs-on": HOSTED, "steps": [], "if": condition}) + assert not rules.never_runs("w.yml", "j"), ( + f"{condition!r} is a legitimate guard and must not be refused" + ) + + +@pytest.mark.parametrize( + ("declared", "expected"), + [ + pytest.param({"steps": []}, True, id="steps"), + pytest.param({"uses": "./w.yml"}, False, id="caller"), + pytest.param({"uses": "./w.yml", "steps": []}, False, id="both"), + pytest.param({}, False, id="neither"), + ], +) +def test_the_caller_exemption_is_keyed_on_uses( + monkeypatch: pytest.MonkeyPatch, declared: dict[str, object], expected: bool +) -> None: + """Exempt a reusable-workflow caller, and refuse a job with neither key. + + A blanket tolerance of a missing `runs-on` would exempt a malformed job + too; keying on `uses` leaves that job refused (whitaker #438). + """ + _declare(monkeypatch, declared) + assert reader.declares_steps("w.yml", "j") is expected, ( + f"{sorted(declared)} must read as declares_steps={expected}" + ) + + +@pytest.mark.parametrize( + ("value", "expected"), + [ + pytest.param("plain", frozenset(), id="no-reference"), + pytest.param("${{ matrix.os }}", frozenset({"matrix.os"}), id="one"), + pytest.param( + "a ${{ matrix.a }} b ${{ matrix.b }}", + frozenset({"matrix.a", "matrix.b"}), + id="two", + ), + # The regression case. A body-level reader using `[^}]*` stops at the + # `}` inside `{0}` and reports no reference at all, so a name built + # this way shared nothing with a `runs-on` reading the same key and + # the stability rule passed on an unstable name. + pytest.param( + "${{ format('{0}', matrix.os) }}", + frozenset({"matrix.os"}), + id="nested-function", + ), + pytest.param( + "${{ format('{0}-{1}', matrix.os, matrix.arch) }}", + frozenset({"matrix.os", "matrix.arch"}), + id="nested-two-args", + ), + # A quoted label carrying a dot is a literal, not a property path. + pytest.param( + "${{ matrix.os == 'ubuntu-22.04' }}", + frozenset({"matrix.os"}), + id="quoted-dotted-literal", + ), + pytest.param( + f"${{{{ {reader.FORK_FIELD} && 'a' || 'b' }}}}", + frozenset({reader.FORK_FIELD}), + id="fork-expression", + ), + pytest.param(None, frozenset(), id="absent"), + pytest.param(True, frozenset(), id="non-string"), + ], +) +def test_references_are_read_from_any_declaration( + value: object, expected: frozenset[str] +) -> None: + """Read every reference, so a name and a runner can be compared.""" + assert rules.references(value) == expected, ( + f"{value!r} reads {sorted(rules.references(value))}" + ) + + +def test_a_matrix_leg_missing_the_runner_key_is_refused( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Refuse an incomplete matrix rather than silently dropping a leg.""" + _declare( + monkeypatch, + { + "runs-on": "${{ matrix.os }}", + "steps": [], + "strategy": {"matrix": {"include": [{"os": HOSTED}, {"arch": "x86_64"}]}}, + }, + ) + with pytest.raises(AssertionError, match="declares no 'os'"): + reader.placement("w.yml", "j") + + +def test_all_jobs_reaches_every_workflow() -> None: + """Name the traversal, or the claim that it is complete is unasserted. + + Falcon-pachinko #149: a `workflow_call` file with no caller is one line + from gaining one, and a caller can live in another repository, so the + registry counts its labels rather than excluding the file by rule. + """ + reached = {workflow_name for workflow_name, _ in reader.all_jobs()} + assert reached == ESTATE_WORKFLOWS, ( + f"the traversal must reach every workflow; reached {sorted(reached)}" + ) + + +def test_the_frozen_hosted_set_holds_no_paid_label() -> None: + """Keep the exemption list free of the labels it exists to expose.""" + assert all( + not label.startswith(("ubicloud-", "namespace-", "buildjet-", "warpbuild-")) + for label in reader.FROZEN_HOSTED_LABELS + ), "a paid provider's label must never be frozen out of the registry question" + + +@pytest.mark.parametrize( + ("declared", "expected"), + [ + pytest.param(True, "integer timeout-minutes", id="yaml-true"), + pytest.param(False, "integer timeout-minutes", id="yaml-false"), + pytest.param("45", "integer timeout-minutes", id="string"), + pytest.param(45.0, "integer timeout-minutes", id="float"), + pytest.param(None, "integer timeout-minutes", id="absent"), + pytest.param(0, "positive timeout-minutes", id="zero"), + pytest.param(-5, "positive timeout-minutes", id="negative"), + ], +) +def test_an_unusable_ceiling_is_refused( + monkeypatch: pytest.MonkeyPatch, declared: object, expected: str +) -> None: + """Refuse the ceilings that bound nothing. + + Driven directly rather than over this repository's workflows, which all + declare sensible values. Parametrized over correct sources the rule passes + whether or not it discriminates, and a mutation from `type(...) is int` + back to `isinstance` survived until this test existed: `True` is an `int` + to Python, so `timeout-minutes: true` would have satisfied the contract + while bounding the job at one minute. + """ + _declare(monkeypatch, {"runs-on": HOSTED, "steps": [], "timeout-minutes": declared}) + with pytest.raises(AssertionError, match=expected): + rules.ceiling("w.yml", "j") + + +def test_a_usable_ceiling_is_returned(monkeypatch: pytest.MonkeyPatch) -> None: + """Prove the rule narrow: an ordinary positive integer must pass.""" + _declare(monkeypatch, {"runs-on": HOSTED, "steps": [], "timeout-minutes": 45}) + assert rules.ceiling("w.yml", "j") == 45, "a positive integer is a valid ceiling" diff --git a/tests/test_ci_placement_properties.py b/tests/test_ci_placement_properties.py new file mode 100644 index 000000000..473492928 --- /dev/null +++ b/tests/test_ci_placement_properties.py @@ -0,0 +1,294 @@ +"""Property tests for the placement reader over its whole input space. + +The reader is a parser, and the example tests beside it pin the shapes this +repository happens to declare today. Three of the defects found in review were +inputs nobody had thought to write down: a composite expression whose +references went unread, a quoted string treated as falsy, and a padded +expression read as the bare one. Generating the space is how that class is +covered rather than enumerated. + +Each property states an invariant the contracts depend on, so a reader change +that quietly narrowed one would fail here rather than in a workflow months +later. +""" + +from __future__ import annotations + +import contextlib +import typing as typ +from unittest import mock + +import pytest +from hypothesis import given, settings +from hypothesis import strategies as st + +from tests.helpers import ci_job_rules as rules +from tests.helpers import ci_placement as reader + +if typ.TYPE_CHECKING: + import collections.abc as cabc + + from tests.helpers.workflow_types import Job + +#: Hypothesis's default deadline measures the host, not the code, and these +#: cases are trivial; a flaky contract test is worse than a slow one. +SETTINGS = settings(deadline=None, max_examples=200) + +#: Horizontal whitespace that may sit *inside* an expression body without +#: changing it. Deliberately no newline: a line break in the parsed value is +#: the folded-scalar defect, refused by its own rule and generated by its own +#: property below. Padding *outside* the braces is a third thing, also refused. +INNER_SPACE = st.sampled_from(["", " ", " ", "\t"]) +#: The three junctions inside a fork body, drawn together so the property takes +#: one argument rather than three. +INNER_SPACING = st.tuples(INNER_SPACE, INNER_SPACE, INNER_SPACE) +LABELS = st.sampled_from([ + "ubuntu-latest", + "ubuntu-22.04", + "windows-2022", + "macos-latest", + "macos-15-intel", + "ubicloud-standard-2", + "ubicloud-standard-4", +]) +PROPERTY_PATHS = st.sampled_from([ + reader.FORK_FIELD, + "github.event.pull_request.head.repo.private", + "matrix.os", + "matrix.python-version", + "needs.changes.outputs.bench", +]) +MATRIX_KEYS = st.sampled_from(["os", "runner", "platform", "image-label"]) + + +def declaring( + declared: cabc.Mapping[str, object], +) -> contextlib.AbstractContextManager[object]: + """Make the reader see one synthetic declaration, for one block. + + A context manager rather than `monkeypatch`: a function-scoped fixture is + not reset between the inputs `@given` generates, so state would leak from + one example to the next. Hypothesis fails the health check for exactly that + reason, and suppressing it would keep the leak rather than remove it. + + Returns + ------- + contextlib.AbstractContextManager[object] + A context manager that installs the declaration in both reader modules + and restores them on exit, including when the block raises. + """ + + def _declared(workflow_name: str, job_name: str) -> Job: + """Return the synthetic declaration whatever job is asked for.""" + del workflow_name, job_name + return typ.cast("Job", declared) + + stack = contextlib.ExitStack() + # Both modules import `job` into their own namespace, so patching one would + # leave the other reading the repository's real workflows and the property + # would assert nothing. + for module in (reader, rules): + stack.enter_context(mock.patch.object(module, "job", _declared)) + return stack + + +@SETTINGS +@given( + condition=PROPERTY_PATHS, + fork_arm=LABELS, + owned_arm=LABELS, + spacing=INNER_SPACING, +) +def test_a_fork_expression_preserves_its_arms_by_position( + condition: str, + fork_arm: str, + owned_arm: str, + spacing: tuple[str, str, str], +) -> None: + """Read the arms by position however the body is spaced. + + Position is the whole point: an expression naming one hosted and one paid + label in either order satisfies a membership test, and one of those two + orders sends every fork to a runner it cannot obtain. + """ + before, after_condition, after_or = spacing + body = ( + f"{before}{condition}{after_condition}&& '{fork_arm}' ||{after_or}'{owned_arm}'" + ) + with declaring({"runs-on": f"${{{{{body}}}}}", "steps": []}): + placed = reader.placement("w.yml", "j") + assert placed.kind == "fork", f"body {body!r} read as {placed.kind}" + assert placed.fork == fork_arm, f"fork arm was {placed.fork!r}" + assert placed.owned == owned_arm, f"owned arm was {placed.owned!r}" + assert placed.references == frozenset({condition}), ( + f"condition read as {sorted(placed.references)}" + ) + + +@SETTINGS +@given(key=MATRIX_KEYS, labels=st.lists(LABELS, min_size=1, max_size=5, unique=True)) +def test_a_matrix_placement_resolves_every_leg(key: str, labels: list[str]) -> None: + """Resolve `runs-on` through the matrix rather than stopping at the key.""" + declared = { + "runs-on": f"${{{{ matrix.{key} }}}}", + "steps": [], + "strategy": {"matrix": {"include": [{key: label} for label in labels]}}, + } + with declaring(declared): + placed = reader.placement("w.yml", "j") + assert placed.kind == "matrix", f"read as {placed.kind}" + assert placed.labels == frozenset(labels), ( + f"legs {labels} resolved to {sorted(placed.labels)}" + ) + assert placed.references == frozenset({f"matrix.{key}"}), ( + f"references were {sorted(placed.references)}" + ) + assert placed.owned is None, "a matrix placement has no single owned label" + + +@SETTINGS +@given(label=LABELS) +def test_a_literal_label_round_trips(label: str) -> None: + """Report a bare label as itself, reading no expression from it. + + `ubuntu-22.04` is the case that matters: it carries a dot, and a reference + reader that did not exclude literals would report it as a context property. + """ + with declaring({"runs-on": label, "steps": []}): + placed = reader.placement("w.yml", "j") + assert placed.kind == "literal", f"{label!r} read as {placed.kind}" + assert placed.labels == frozenset({label}), f"labels were {sorted(placed.labels)}" + assert placed.references == frozenset(), ( + f"a literal reads no property, got {sorted(placed.references)}" + ) + + +@SETTINGS +@given( + prefix=st.sampled_from(["", " ", "runner-", "ubuntu-"]), + suffix=st.sampled_from(["", " ", "-latest"]), + path=PROPERTY_PATHS, +) +def test_anything_around_an_expression_is_refused( + prefix: str, suffix: str, path: str +) -> None: + """Refuse a value that interpolates without being wholly an expression. + + GitHub substitutes into the surrounding string and keeps what is around it, + so every one of these resolves to a label no runner matches. The empty + prefix and suffix pair is excluded because that is the supported shape. + """ + if not prefix and not suffix: + return + declared = {"runs-on": f"{prefix}${{{{ {path} }}}}{suffix}", "steps": []} + with ( + declaring(declared), + pytest.raises(AssertionError, match="interpolates its runner label"), + ): + reader.placement("w.yml", "j") + + +@SETTINGS +@given( + condition=PROPERTY_PATHS, + fork_arm=LABELS, + owned_arm=LABELS, + position=st.integers(min_value=0, max_value=3), +) +def test_a_line_break_anywhere_in_the_value_is_refused( + condition: str, fork_arm: str, owned_arm: str, position: int +) -> None: + """Refuse an embedded line break wherever the fold put it. + + A folded-scalar continuation indented one level deeper keeps its line break + and puts a newline inside the expression. GitHub evaluates the broken value + regardless, so a green run is not evidence and only the parsed document + shows it. The break is generated at each junction rather than at the one + place a handwritten fixture would put it. + """ + parts = [condition, "&&", f"'{fork_arm}'", "||", f"'{owned_arm}'"] + separators = [" "] * 4 + separators[position] = "\n" + body = "".join( + part + (separators[index] if index < len(separators) else "") + for index, part in enumerate(parts) + ) + with ( + declaring({"runs-on": f"${{{{{body}}}}}", "steps": []}), + pytest.raises(AssertionError, match="embedded line break"), + ): + reader.placement("w.yml", "j") + + +@SETTINGS +@given(paths=st.lists(PROPERTY_PATHS, min_size=1, max_size=4, unique=True)) +def test_every_property_in_a_composite_expression_is_read( + paths: list[str], +) -> None: + """Read every property a composite expression names, not the body. + + A body-level reader stops at the first `}`, which is inside `{0}`, and + reports no reference at all. The job-name stability rule compares + references, so that silence let an unstable name pass. + """ + arguments = ", ".join(paths) + template = "-".join(f"{{{index}}}" for index in range(len(paths))) + value = f"a ${{{{ format('{template}', {arguments}) }}}} b" + assert rules.references(value) == frozenset(paths), ( + f"{value!r} read {sorted(rules.references(value))}" + ) + + +#: Guards built the way a workflow builds them. Generated rather than listed, +#: because the risk here is a false positive on an ordinary condition, and a +#: rule that refused those would be switched off. +GUARD_SUBJECTS = st.sampled_from([ + "github.event_name", + "github.ref", + "needs.changes.outputs.bench", + "matrix.python-suite", + "inputs.publish-baseline", +]) +GUARD_VALUES = st.sampled_from([ + "'push'", + "'pull_request'", + "'refs/heads/main'", + "'true'", + "'false'", + "true", +]) + + +#: A comparison and the two ways a workflow dresses it: negated, wrapped in +#: `${{ }}`, both, or neither. Drawn as one value so the property keeps a +#: readable signature. +GUARD_SHAPES = st.tuples( + GUARD_SUBJECTS, + st.sampled_from(["==", "!="]), + GUARD_VALUES, + st.booleans(), + st.booleans(), +) + + +@SETTINGS +@given(shape=GUARD_SHAPES) +def test_a_real_guard_is_never_reported_as_dead( + shape: tuple[str, str, str, bool, bool], +) -> None: + """Keep the dead-lane rule off conditions that can be true. + + Note `'false'` among the generated values: GitHub treats a non-empty string + as truthy, so a comparison against it is an ordinary guard on a job that + runs. Encoding the quoted form as falsy was the defect this covers. + """ + subject, operator, value, negated, wrapped = shape + text = f"{subject} {operator} {value}" + if negated: + text = f"!({text})" + if wrapped: + text = f"${{{{ {text} }}}}" + with declaring({"runs-on": "ubuntu-latest", "steps": [], "if": text}): + assert not rules.never_runs("w.yml", "j"), ( + f"{text!r} names no falsy literal, so the job can run" + ) diff --git a/tests/test_ci_placement_reader.py b/tests/test_ci_placement_reader.py new file mode 100644 index 000000000..b44e87028 --- /dev/null +++ b/tests/test_ci_placement_reader.py @@ -0,0 +1,290 @@ +"""Tests for the placement reader itself, driven by synthetic declarations. + +The placement contracts in `tests/test_ci_runner_placement.py` are +parametrized over this repository's own workflows, which are correct. A rule +cannot be proved by the sources it guards: parametrized over correct files it +passes whether or not it discriminates, so the reader is driven directly here +with the shapes a workflow could acquire. + +The shapes that must be *refused* matter as much as the ones that must be +read. An unparsable `runs-on` recorded as one opaque label carries no vendor +prefix, so the Ubicloud classifier drops the lane and it sits exempt from every +placement, ceiling, and registry assertion while still asking for a paid runner +(axinite #372). +""" + +from __future__ import annotations + +import pytest + +from tests.helpers import ci_placement as reader + +OWNED = "ubicloud-standard-2" +HOSTED = "ubuntu-latest" +FORK_EXPRESSION = f"${{{{ {reader.FORK_FIELD} && '{HOSTED}' || '{OWNED}' }}}}" + + +def _declare(monkeypatch: pytest.MonkeyPatch, declared: dict[str, object]) -> None: + """Make the reader see one synthetic job declaration.""" + monkeypatch.setattr(reader, "job", lambda *_: declared) + + +def test_a_bare_label_is_read_as_itself(monkeypatch: pytest.MonkeyPatch) -> None: + """Keep the common case simple: one literal label, no arms, no references.""" + _declare(monkeypatch, {"runs-on": "windows-2022", "steps": []}) + placed = reader.placement("w.yml", "j") + assert placed.kind == "literal", f"a bare label is a literal, got {placed.kind}" + assert placed.owned == "windows-2022", f"owned arm was {placed.owned!r}" + assert placed.fork is None, ( + f"a literal placement has no fork arm, got {placed.fork!r}" + ) + assert placed.labels == frozenset({"windows-2022"}), ( + f"a literal resolves to one label, got {sorted(placed.labels)}" + ) + assert placed.references == frozenset(), ( + f"a literal reads no expression, got {sorted(placed.references)}" + ) + + +def test_the_fork_expression_is_read_by_position( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Report which arm is which, so a caller can assert them separately.""" + _declare(monkeypatch, {"runs-on": FORK_EXPRESSION, "steps": []}) + placed = reader.placement("w.yml", "j") + assert placed.kind == "fork", f"the fallback is a fork placement, got {placed.kind}" + assert placed.fork == HOSTED, f"fork arm was {placed.fork!r}" + assert placed.owned == OWNED, f"owned arm was {placed.owned!r}" + assert placed.labels == frozenset({HOSTED, OWNED}), ( + f"both arms are labels in use, got {sorted(placed.labels)}" + ) + assert placed.references == frozenset({reader.FORK_FIELD}), ( + f"the condition must be the fork field, got {sorted(placed.references)}" + ) + + +def test_swapped_arms_are_reported_as_swapped( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Discriminate the defect a membership test cannot see. + + Nile-valley #106's contract asserted that the expression named one hosted + and one Ubicloud label somewhere. This declaration satisfies that and sends + every fork to a runner no fork can obtain. + """ + swapped = f"${{{{ {reader.FORK_FIELD} && '{OWNED}' || '{HOSTED}' }}}}" + _declare(monkeypatch, {"runs-on": swapped, "steps": []}) + placed = reader.placement("w.yml", "j") + assert placed.fork == OWNED, "the reader must report the arm as declared" + assert placed.owned == HOSTED, "the owned arm here is the hosted label" + + +def test_a_sibling_field_is_reported_as_a_different_condition( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Separate a well-formed expression from a correct one. + + `head.repo.private` produces an expression with the right shape and the + right two arms that branches on the wrong thing. Only the condition itself + distinguishes it, so the reader returns the condition rather than a + boolean. + """ + sibling = ( + "${{ github.event.pull_request.head.repo.private" + f" && '{HOSTED}' || '{OWNED}' }}}}" + ) + _declare(monkeypatch, {"runs-on": sibling, "steps": []}) + placed = reader.placement("w.yml", "j") + assert placed.kind == "fork", f"the shape is still a fork arm, got {placed.kind}" + assert placed.references != frozenset({reader.FORK_FIELD}), ( + "a sibling field must be distinguishable from the fork field" + ) + + +def test_a_hyphenated_condition_is_read(monkeypatch: pytest.MonkeyPatch) -> None: + """Read a condition whose property path carries a hyphen. + + `matrix.python-version` is a legitimate condition, and a character class + without `-` refused the whole expression as unmodellable. The refusal + failed safe, so nothing went unchecked, but it would have rejected a + correct lane. Found by the generated property in + `test_ci_placement_properties.py` rather than by any example. + """ + hyphenated = f"${{{{ matrix.python-version && '{HOSTED}' || '{OWNED}' }}}}" + _declare(monkeypatch, {"runs-on": hyphenated, "steps": []}) + placed = reader.placement("w.yml", "j") + assert placed.kind == "fork", f"read as {placed.kind}" + assert placed.references == frozenset({"matrix.python-version"}), ( + f"condition read as {sorted(placed.references)}" + ) + + +def test_a_matrix_placement_resolves_through_its_include( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Follow `runs-on` into the matrix values it selects, not merely to the key.""" + _declare( + monkeypatch, + { + "runs-on": "${{ matrix.os }}", + "steps": [], + "strategy": { + "matrix": {"include": [{"os": "macos-latest"}, {"os": HOSTED}]} + }, + }, + ) + placed = reader.placement("w.yml", "j") + assert placed.kind == "matrix", f"a matrix key placement, got {placed.kind}" + assert placed.owned is None, ( + f"a matrix placement has no single owned label, got {placed.owned!r}" + ) + assert placed.labels == frozenset({"macos-latest", HOSTED}), ( + f"every include entry is a label, got {sorted(placed.labels)}" + ) + assert placed.references == frozenset({"matrix.os"}), ( + f"the placement reads the matrix key, got {sorted(placed.references)}" + ) + + +def test_a_direct_matrix_list_also_reads_its_include_legs( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An `include` leg with a new value for the key runs on that label too. + + An `include` entry without the key only extends the existing legs, so it + is read past rather than refused. + """ + _declare( + monkeypatch, + { + "runs-on": "${{ matrix.os }}", + "steps": [], + "strategy": { + "matrix": { + "os": [HOSTED], + "include": [{"os": "windows-2022", "target": "x"}, {"target": "y"}], + } + }, + }, + ) + placed = reader.placement("w.yml", "j") + assert placed.labels == frozenset({HOSTED, "windows-2022"}), ( + f"the include leg's label must be read, got {sorted(placed.labels)}" + ) + + +@pytest.mark.parametrize( + "value", + [FORK_EXPRESSION, "ubuntu-${{ inputs.release }}", " ${{ matrix.x }}"], + ids=["whole-expression", "partial-interpolation", "padded-expression"], +) +def test_a_matrix_value_holding_an_expression_is_refused( + monkeypatch: pytest.MonkeyPatch, value: str +) -> None: + """Refuse the shape rstest-bdd #788's contract read straight through. + + A reader that stopped at `matrix.os` never saw that the value behind the + key was itself the fork expression, so a name reading the fork field shared + no reference with `runs-on` and the stability contract passed while the + name rendered differently per event. A partial interpolation is the same + defect: GitHub evaluates it, so the recorded text is not the label that + runs. + """ + _declare( + monkeypatch, + { + "runs-on": "${{ matrix.os }}", + "steps": [], + "strategy": {"matrix": {"include": [{"os": value}]}}, + }, + ) + with pytest.raises(AssertionError, match="cannot model"): + reader.placement("w.yml", "j") + + +@pytest.mark.parametrize( + ("declared", "expected"), + [ + pytest.param( + {"runs-on": ["self-hosted", "linux"], "steps": []}, + "list runs-on", + id="list", + ), + pytest.param({"runs-on": [], "steps": []}, "list runs-on", id="empty-list"), + pytest.param({"steps": []}, "non-empty string", id="absent"), + pytest.param({"runs-on": "", "steps": []}, "non-empty string", id="blank"), + pytest.param( + {"runs-on": "${{ inputs.chosen-os }}", "steps": []}, + "cannot model", + id="opaque-expression", + ), + pytest.param( + { + "runs-on": f"${{{{ {reader.FORK_FIELD}\n && '{HOSTED}' }}}}", + "steps": [], + }, + "line break", + id="folded-scalar", + ), + ], +) +def test_unmodelled_declarations_are_refused( + monkeypatch: pytest.MonkeyPatch, declared: dict[str, object], expected: str +) -> None: + """Fail on a shape the reader cannot model rather than record it as read.""" + _declare(monkeypatch, declared) + with pytest.raises(AssertionError, match=expected): + reader.placement("w.yml", "j") + + +def test_an_interpolated_label_is_refused(monkeypatch: pytest.MonkeyPatch) -> None: + """Refuse a value that interpolates without being wholly an expression. + + `ubuntu-${{ matrix.release }}` is not a literal label, and recording it as + one is the same defect as recording an opaque expression as one: the label + it resolves to at run time is invisible to every placement and registry + assertion, while the job still asks for a runner. + """ + _declare(monkeypatch, {"runs-on": "ubuntu-${{ matrix.release }}", "steps": []}) + with pytest.raises(AssertionError, match="interpolates its runner label"): + reader.placement("w.yml", "j") + + +@pytest.mark.parametrize( + "declared", + [ + pytest.param(f" {FORK_EXPRESSION}", id="leading-space"), + pytest.param(f"{FORK_EXPRESSION} ", id="trailing-space"), + pytest.param(" ${{ inputs.chosen-os }}", id="padded-opaque"), + ], +) +def test_a_padded_expression_is_refused( + monkeypatch: pytest.MonkeyPatch, declared: str +) -> None: + """Refuse a padded expression rather than reading it as the bare one. + + GitHub interpolates into the surrounding string and keeps what is around + the expression, so `" ${{ ... }}"` resolves to a label with a leading space + that matches no runner. Reading it as the unpadded expression would report + a broken lane as a correct fork placement, which is the dangerous + direction: the contract would go green on a job that can never be + scheduled. + """ + _declare(monkeypatch, {"runs-on": declared, "steps": []}) + with pytest.raises(AssertionError, match="interpolates its runner label"): + reader.placement("w.yml", "j") + + +def test_the_refusal_of_an_opaque_expression_is_still_needed( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Fail if the reader ever learns the shape the test above refuses. + + A refusal contract goes quiet the day the reader gains the ability it was + written to compensate for, and a quiet contract reads exactly like a + passing one. This makes that day loud: the refusal becomes redundant, not + invisible, and both tests are then rewritten together. + """ + _declare(monkeypatch, {"runs-on": "${{ inputs.chosen-os }}", "steps": []}) + with pytest.raises(AssertionError): + reader.placement("w.yml", "j") diff --git a/tests/test_ci_resource_sampling.py b/tests/test_ci_resource_sampling.py index 93356882f..5ca17eb5c 100644 --- a/tests/test_ci_resource_sampling.py +++ b/tests/test_ci_resource_sampling.py @@ -22,6 +22,10 @@ #: every 15 s is interference rather than observation; its disk figure was #: taken once, in run 33857764655. SAMPLED_JOBS: typ.Final = ( + # `lint-test` joined this list when it moved onto the paid lane: it builds + # Whitaker, clippy under two toolchains and a Windows cross-target check on + # two vCPU, which is exactly the shape whose failure mode is a silent death. + ("ci.yml", "lint-test"), ("ci.yml", "typecheck-test"), ("ci.yml", "extension-tests"), ("ci.yml", "coverage"), diff --git a/tests/test_ci_runner_placement.py b/tests/test_ci_runner_placement.py index 673901dc6..ed2969995 100644 --- a/tests/test_ci_runner_placement.py +++ b/tests/test_ci_runner_placement.py @@ -1,68 +1,53 @@ -"""Contract tests for where Cuprum's CI jobs run and how wide they fan out. - -Runner placement and worker counts are declarations no functional test can -reach: a job that drifts back to a GitHub-hosted label, or that asks -`pytest-xdist` for more workers than the runner has cores, still produces a -green suite while queueing for hours or thrashing two vCPUs. These tests read -the declarations back against the manifests in `tests/helpers/ci_runners.py`. +"""Contract tests for where Cuprum's CI jobs run and whether they run. + +Runner placement is a declaration no functional test can reach: a job that +drifts back to a GitHub-hosted label, asks for a paid runner a fork can never +obtain, or carries no ceiling at all still produces a green suite while +queueing for hours or billing for six. These tests read the declarations back +against the manifests in `tests/helpers/ci_runners.py`, through the one reader +in `tests/helpers/ci_placement.py`. + +Companion modules cover the neighbouring declarations: the label registry in +`test_ci_actionlint_registry.py`, worker counts in `test_ci_worker_bounds.py`, +and tool installation in `test_ci_tool_installation.py`. """ from __future__ import annotations -import re -import typing as typ - import pytest import yaml from tests.helpers.ci_runners import ( + CONTINUE_ON_ERROR_JOBS, + EXPERIMENTAL_LEG_KEY, + FORK_FIELD, + FORK_REACHABLE_UBICLOUD_JOBS, GITHUB_HOSTED_JOBS, GITHUB_LABEL, - ROOT, + RUNNER_NAMED_JOBS, UBICLOUD_JOBS, UBICLOUD_LABEL, - UBICLOUD_VCPUS, WINDOWS_HOSTED_JOBS, WINDOWS_LABEL, + all_jobs, + ceiling, + declares_steps, expand, job, - step_inputs, - steps, + never_runs, + placement, + references, workflow_document, - workflow_env, workflow_sources, ) -if typ.TYPE_CHECKING: - from tests.helpers.workflow_types import Step - -ACTIONLINT_CONFIG = ROOT / ".github" / "actionlint.yaml" -MAKEFILE = ROOT / "Makefile" -VCPU_CONSTANT = "LINUX_RUNNER_VCPUS" -#: Any spelling of the tool, so `nextest@` and `cargo-nextest@` both match. -NEXTEST_TOOL_NAME = "nextest" -#: Shell fragments that mean "fetch a binary" rather than "run one". -INSTALL_VERBS = ("install", "curl", "wget") -#: The upstream install host. Its name does not contain "nextest", so it needs -#: its own token, and it only ever appears in an install command. -NEXTEST_INSTALL_HOST = "get.nexte.st" -#: Steps invoking a shared action are exempt: the coverage action installs -#: nextest on purpose, and that is the only sanctioned place. -SHARED_ACTION_PREFIX = "leynos/shared-actions/" -#: A `vars.*` reference in a workflow. actionlint checks such a reference -#: against `config-variables`, so the declared list and the references read -#: back from the workflows have to agree in both directions: an undeclared -#: name is a lint error, and a declared name nothing reads is a stale -#: allowance that would let a deleted variable's typo pass. -CONFIGURATION_VARIABLE = re.compile(r"vars\.([A-Za-z_][A-Za-z0-9_]*)") -#: Make variables that carry the runner's vCPU count into the test command. -#: Only the pytest one remains here: the Rust suite moved to the coverage job, -#: which bounds itself through `CARGO_BUILD_JOBS` and `NEXTEST_TEST_THREADS`. -PARALLELISM_OVERRIDES = ("PYTEST_CARGO_BUILD_JOBS",) - UBICLOUD_CASES = expand(UBICLOUD_JOBS) GITHUB_HOSTED_CASES = expand(GITHUB_HOSTED_JOBS) WINDOWS_HOSTED_CASES = expand(WINDOWS_HOSTED_JOBS) +FORK_REACHABLE_CASES = expand(FORK_REACHABLE_UBICLOUD_JOBS) +ALL_CASES = all_jobs() +STEP_CASES = [case for case in ALL_CASES if declares_steps(*case)] +CALLER_CASES = [case for case in ALL_CASES if not declares_steps(*case)] @pytest.mark.parametrize(("workflow_name", "job_name"), UBICLOUD_CASES) @@ -70,19 +55,194 @@ def test_linux_build_and_test_jobs_use_the_ubicloud_default_shape( workflow_name: str, job_name: str ) -> None: """Keep every repository-owned Linux gate on the reviewed Ubicloud shape.""" - runner = job(workflow_name, job_name).get("runs-on") - assert runner == UBICLOUD_LABEL, ( + owned = placement(workflow_name, job_name).owned + assert owned == UBICLOUD_LABEL, ( f"{workflow_name}:{job_name} must run on {UBICLOUD_LABEL}; escalating to a " - f"larger shape needs recorded measurements, got {runner!r}" + f"larger shape needs recorded measurements, got {owned!r}" ) -@pytest.mark.parametrize(("workflow_name", "job_name"), UBICLOUD_CASES) -def test_ubicloud_jobs_declare_a_timeout(workflow_name: str, job_name: str) -> None: - """Bound a wedged paid runner rather than paying for its default six hours.""" - timeout = job(workflow_name, job_name).get("timeout-minutes") - assert isinstance(timeout, int), ( - f"{workflow_name}:{job_name} must declare timeout-minutes, got {timeout!r}" +@pytest.mark.parametrize(("workflow_name", "job_name"), FORK_REACHABLE_CASES) +def test_fork_reachable_lanes_fall_back_by_position( + workflow_name: str, job_name: str +) -> None: + """Send a fork's pull request to a runner it can actually obtain. + + Read by position, not by membership. Nile-valley #106's contract asserted + that the expression named one hosted and one Ubicloud label somewhere, so + swapping the arms passed while sending forks to a runner no fork can get. + The condition is full-matched too: a sibling field such as + `head.repo.private` produces a well-formed expression that branches on the + wrong thing. + """ + placed = placement(workflow_name, job_name) + assert placed.kind == "fork", ( + f"{workflow_name}:{job_name} must select its runner with the fork " + f"fallback expression, got a {placed.kind} placement" + ) + assert placed.references == frozenset({FORK_FIELD}), ( + f"{workflow_name}:{job_name} must branch on {FORK_FIELD}, " + f"got {sorted(placed.references)}" + ) + assert placed.fork == GITHUB_LABEL, ( + f"{workflow_name}:{job_name} must send a fork to {GITHUB_LABEL}, " + f"got {placed.fork!r}" + ) + assert placed.owned == UBICLOUD_LABEL, ( + f"{workflow_name}:{job_name} must keep {UBICLOUD_LABEL} on the owned " + f"arm, got {placed.owned!r}" + ) + + +@pytest.mark.parametrize(("workflow_name", "job_name"), STEP_CASES) +def test_placement_expressions_parse_to_one_line( + workflow_name: str, job_name: str +) -> None: + """Refuse a folded scalar whose continuation kept its line break. + + A continuation indented one level deeper puts a newline inside the + expression. GitHub evaluates the broken value regardless, so a green run is + no evidence; only the parsed document shows it (dev-env-rocky #216). + + Reading the declaration is the assertion: the reader refuses an embedded + line break, a list, an absent value and any expression it cannot model. + """ + placement(workflow_name, job_name) + + +def test_fork_reachability_matches_the_manifest() -> None: + """Hold the fallback manifest against the triggers it rests on. + + `runs-on` can never check the premise these rules depend on: that a fork + can reach the lane at all. Without the assertion a silent trigger change + leaves the consequence looking deliberate (falcon-pachinko). YAML 1.1 reads + the bare word `on` as the boolean `True`, so the triggers are read under + that key; reading the string `"on"` would find nothing and pass vacuously. + """ + ci_triggers = workflow_document("ci.yml")[True] + assert isinstance(ci_triggers, dict), "ci.yml must declare its triggers" + assert "pull_request" in ci_triggers, ( + "ci.yml must declare the pull_request trigger; without it no fork " + "reaches these lanes and every fallback arm below is dead code" + ) + caller = job("ci.yml", "build-wheels").get("uses") + assert caller == "./.github/workflows/build-wheels.yml", ( + "ci.yml must call build-wheels.yml; that call, not build-wheels.yml's " + "own workflow_call trigger, is what exposes its jobs to forks" + ) + coverage_triggers = workflow_document("coverage-main.yml")[True] + assert isinstance(coverage_triggers, dict), ( + "coverage-main.yml must declare its triggers" + ) + assert "pull_request" not in coverage_triggers, ( + "coverage-main.yml is absent from the fork manifest because no fork " + "can trigger it; a pull_request trigger here would make that false" + ) + + +@pytest.mark.parametrize(("workflow_name", "job_name"), STEP_CASES) +def test_every_job_running_steps_declares_a_ceiling( + workflow_name: str, job_name: str +) -> None: + """Bound a wedged runner rather than paying for the six-hour default. + + Keyed on declaring steps rather than on carrying an Ubicloud label. Once a + label is an expression, "an Ubicloud lane" is a property of the event, so a + rule keyed on the label stops applying on exactly the arm that hangs. + """ + # The rule lives in `ceiling`, which `tests/test_ci_placement_guards.py` + # drives directly over the values this repository does not happen to + # declare. No upper bound: 360 is the GitHub-hosted execution limit, not a + # repository-wide job ceiling. + ceiling(workflow_name, job_name) + + +@pytest.mark.parametrize(("workflow_name", "job_name"), CALLER_CASES) +def test_reusable_workflow_callers_declare_no_ceiling( + workflow_name: str, job_name: str +) -> None: + """Keep placing a job and bounding it as separate ideas. + + GitHub rejects `timeout-minutes` on a job with `uses:`, so the bound lives + in the callee. Each of this repository's callers calls a workflow whose own + jobs are bounded by the rule above (weaver). + """ + declared = job(workflow_name, job_name) + assert "uses" in declared, ( + f"{workflow_name}:{job_name} declares neither steps nor uses" + ) + assert declared.get("timeout-minutes") is None, ( + f"{workflow_name}:{job_name} calls a reusable workflow, where GitHub " + "rejects timeout-minutes; bound the callee instead" + ) + + +@pytest.mark.parametrize(("workflow_name", "job_name"), ALL_CASES) +def test_no_reviewed_lane_is_switched_off(workflow_name: str, job_name: str) -> None: + """Read whether a job runs, not only how it is configured. + + Every placement, budget and cache rule in this repository reads a + declaration. `if: false` leaves all of them satisfied and runs nothing + (lille #349). A containment test would accept `false && matrix.x == 'y'`, + so the leading clause is matched explicitly. + """ + assert not never_runs(workflow_name, job_name), ( + f"{workflow_name}:{job_name} can never run, so every rule about where " + "it runs and what it costs holds vacuously" + ) + + +@pytest.mark.parametrize(("workflow_name", "job_name"), ALL_CASES) +def test_only_the_experimental_matrix_leg_may_fail_silently( + workflow_name: str, job_name: str +) -> None: + """Keep `continue-on-error` to the leg that is allowed to be broken.""" + declared = job(workflow_name, job_name).get("continue-on-error") + if declared is None: + return + assert (workflow_name, job_name) in CONTINUE_ON_ERROR_JOBS, ( + f"{workflow_name}:{job_name} tolerates its own failure but is not in " + "the manifest; a gate that cannot fail the workflow gates nothing" + ) + assert references(declared) == frozenset({f"matrix.{EXPERIMENTAL_LEG_KEY}"}), ( + f"{workflow_name}:{job_name} must tolerate failure only on the leg its " + f"matrix marks {EXPERIMENTAL_LEG_KEY!r}, got {declared!r}" + ) + + +@pytest.mark.parametrize(("workflow_name", "job_name"), STEP_CASES) +def test_job_names_do_not_follow_their_runner( + workflow_name: str, job_name: str +) -> None: + """Keep a required check's context stable across events. + + The hazard is event dependence, not matrix dependence. With the fork + fallback the runner follows the event, so a name reading that condition + renders differently on a fork's pull request and an internal one, and no + single required context exists on both. + + A name reading a matrix key is a different thing: the values behind the key + are literals, so the name varies per leg and is stable per event, which is + how matrix legs get distinct contexts at all. Those jobs are listed rather + than waved through, because a context spelling a runner label is still a + context that a placement change would rename (shared-actions #480). + + Resolved through the matrix either way: rstest-bdd #788's contract compared + the name's references with `runs-on`'s, which read `matrix.os` and stopped, + while the matrix value behind that key held the fork expression. + """ + placed = placement(workflow_name, job_name) + shared = references(job(workflow_name, job_name).get("name")) & placed.references + if placed.kind == "matrix": + assert (workflow_name, job_name) in RUNNER_NAMED_JOBS or not shared, ( + f"{workflow_name}:{job_name} spells its runner in its name but is " + "not in RUNNER_NAMED_JOBS; a placement change would rename its " + "check context" + ) + return + assert not shared, ( + f"{workflow_name}:{job_name} names itself from {sorted(shared)}, which " + "its runner also reads; the check context would follow the event" ) @@ -91,9 +251,10 @@ def test_administrative_and_serial_jobs_stay_github_hosted( workflow_name: str, job_name: str ) -> None: """Keep sleeping, API-bound, and publish-only work off metered build slots.""" - runner = job(workflow_name, job_name).get("runs-on") - assert runner == GITHUB_LABEL, ( - f"{workflow_name}:{job_name} must stay on {GITHUB_LABEL}, got {runner!r}" + placed = placement(workflow_name, job_name) + assert placed.labels == frozenset({GITHUB_LABEL}), ( + f"{workflow_name}:{job_name} must stay on {GITHUB_LABEL}, " + f"got {sorted(placed.labels)}" ) @@ -101,19 +262,31 @@ def test_administrative_and_serial_jobs_stay_github_hosted( def test_windows_native_jobs_stay_on_github_hosted_windows( workflow_name: str, job_name: str ) -> None: - """Keep native Windows validation on the reviewed hosted runner image.""" - runner = job(workflow_name, job_name).get("runs-on") - assert runner == WINDOWS_LABEL, ( - f"{workflow_name}:{job_name} must stay on {WINDOWS_LABEL}, got {runner!r}" + """Keep native Windows validation on the reviewed hosted runner image. + + A hosted-at-all predicate is not a placement predicate: an API-bound job + moved to windows-latest satisfied a contract whose message named + ubuntu-latest (lille #349), so the label is asserted by equality. + """ + placed = placement(workflow_name, job_name) + assert placed.labels == frozenset({WINDOWS_LABEL}), ( + f"{workflow_name}:{job_name} must stay on {WINDOWS_LABEL}, " + f"got {sorted(placed.labels)}" ) def test_native_wheel_matrix_keeps_its_platform_runners() -> None: """Ubicloud has no Windows or macOS capacity, so the matrix stays hosted.""" matrix_job = job("build-wheels.yml", "build-native-wheels") - assert matrix_job.get("runs-on") == "${{ matrix.os }}", ( + placed = placement("build-wheels.yml", "build-native-wheels") + assert placed.kind == "matrix", ( "build-wheels.yml:build-native-wheels must keep its platform matrix" ) + assert UBICLOUD_LABEL not in placed.labels, ( + "both ubuntu legs are named verbatim in the main-required-checks " + "ruleset, so moving either renames a required context; that is the " + "repository owner's decision, not a placement change" + ) strategy = matrix_job.get("strategy") assert isinstance(strategy, dict), "the native wheel job must declare a strategy" matrix = strategy.get("matrix") @@ -153,236 +326,6 @@ def test_every_workflow_job_appears_in_one_placement_manifest() -> None: ) -def test_actionlint_registers_exactly_the_self_hosted_labels_in_use() -> None: - """Register intentional labels so a typo fails lint instead of queueing.""" - config = yaml.safe_load(ACTIONLINT_CONFIG.read_text(encoding="utf-8")) - declared = config["self-hosted-runner"]["labels"] - used = { - str(job(workflow_name, job_name).get("runs-on")) - for workflow_name, job_name in UBICLOUD_CASES - } - assert sorted(declared) == sorted(used), ( - "actionlint must list every self-hosted label the workflows use and no " - f"others; declared {declared}, used {sorted(used)}" - ) - # Compared as sorted lists: the config registers names for lint to - # resolve, and actionlint does not care what order they appear in. Sorting - # still fails on a duplicate, so the comparison loses nothing. - read = { - match.group(1) - for _, source in workflow_sources() - for match in CONFIGURATION_VARIABLE.finditer(source) - } - assert sorted(config["config-variables"]) == sorted(read), ( - "list only the configuration variables the workflows read, so a typo " - f"in a vars.* reference fails lint; declared " - f"{config['config-variables']}, read {sorted(read)}" - ) - - -def test_no_retired_runner_labels_remain() -> None: - """Leave no Namespace label or cache action behind after the migration.""" - for workflow_name, source in workflow_sources(): - assert "namespace-profile" not in source, ( - f"{workflow_name} still references a Namespace runner profile" - ) - assert "nscloud" not in source, ( - f"{workflow_name} still references the Namespace cache action" - ) - - -def test_the_vcpu_constant_matches_the_assigned_label() -> None: - """Tie the one parallelism constant to the shape the job is billed for.""" - declared = workflow_env("ci.yml")[VCPU_CONSTANT] - assert declared == str(UBICLOUD_VCPUS), ( - f"{VCPU_CONSTANT} must equal the vCPU count of {UBICLOUD_LABEL}, " - f"got {declared!r}" - ) - - -def test_python_tests_derive_their_worker_counts_from_that_constant() -> None: - """Size the matrix suite's Cargo work from the constant, not a literal.""" - script = next( - step["run"] - for step in steps("ci.yml", "typecheck-test") - if step.get("name") == "Run tests" - ) - assert isinstance(script, str), "ci.yml:typecheck-test must run a test script" - for variable in PARALLELISM_OVERRIDES: - assert f'{variable}="${{{VCPU_CONSTANT}}}"' in script, ( - f"ci.yml:typecheck-test must pass {variable} from {VCPU_CONSTANT}" - ) - - -def test_extension_and_benchmark_builds_are_bounded_too() -> None: - """Bound the two jobs that compile outside `make test` to the same count.""" - for job_name, step_name in ( - ("extension-tests", "Build the native extension"), - ("benchmark-ratchet", "Run throughput benchmarks and ratchet comparison"), - ): - script = next( - step["run"] - for step in steps("ci.yml", job_name) - if step.get("name") == step_name - ) - assert isinstance(script, str), f"ci.yml:{job_name} must run {step_name!r}" - assert f'CARGO_BUILD_JOBS="${{{VCPU_CONSTANT}}}"' in script, ( - f"ci.yml:{job_name} must bound Cargo build jobs by {VCPU_CONSTANT}" - ) - - -def test_python_suites_never_ask_for_unbounded_workers() -> None: - """Reject `-n auto`: the runner has two cores whatever the host reports.""" - sources = [source for _, source in workflow_sources()] - sources.append(MAKEFILE.read_text(encoding="utf-8")) - for source in sources: - assert "-n auto" not in source, "xdist worker counts must be explicit" - - -def test_the_python_suite_stays_serial() -> None: - """Keep pytest serial while its batches contend on one Cargo target.""" - makefile = MAKEFILE.read_text(encoding="utf-8") - assert re.search(r"^PYTEST_WORKERS \?= 0$", makefile, re.MULTILINE), ( - "PYTEST_WORKERS must default to 0; the batches compile and reuse the " - "same Rust artefacts, so xdist workers would contend on one build lock" - ) - for workflow_name, job_name in (("ci.yml", "coverage"),): - coverage_step = next( - step - for step in steps(workflow_name, job_name) - if str(step.get("uses", "")).startswith( - "leynos/shared-actions/.github/actions/generate-coverage@" - ) - ) - inputs = step_inputs(coverage_step, f"{workflow_name}:{job_name} inputs") - workers = inputs.get("pytest-workers") - assert isinstance(workers, str), ( - f"{workflow_name}:{job_name} coverage must declare pytest-workers" - ) - assert not workers, ( - f"{workflow_name}:{job_name} coverage must run pytest serially, " - f"got {workers!r}" - ) - - -def test_ci_does_not_build_tools_from_source() -> None: - """Keep CI tool installation on trusted, pinned prebuilt paths.""" - for workflow_name, source in workflow_sources(): - assert "cargo install" not in source, ( - f"{workflow_name} must not source-build a Cargo tool" - ) - assert "get.nexte.st/latest" not in source, ( - f"{workflow_name} must pin the nextest binary version" - ) - if "cargo binstall" in source: - assert "--disable-strategies compile" in source, ( - f"{workflow_name} must stop cargo-binstall falling back to a " - "source build" - ) - - -def test_no_workflow_installs_cargo_nextest() -> None: - """Leave the nextest install to the coverage action that now needs it. - - The matrix jobs stopped running the Rust suite, so an installer here would - be an unused download whose failure mode nothing in this repository - exercises. - - Checked structurally rather than by one literal string. A `tool:` input - naming either spelling, a different installer action, or a shell command - that fetches nextest would all pass a check for `tool: nextest@`. - """ - offenders: list[str] = [] - for workflow_name, _ in workflow_sources(): - document = workflow_document(workflow_name) - jobs_mapping = document.get("jobs") - if not isinstance(jobs_mapping, dict): - continue - for job_name in jobs_mapping: - offenders.extend( - _nextest_installers(workflow_name, str(job_name)), - ) - assert not offenders, ( - f"only the shared coverage action may install cargo-nextest; found {offenders}" - ) - - -def _declares_own_steps(workflow_name: str, job_name: str) -> bool: - """Report whether a job runs steps rather than calling a reusable workflow.""" - return isinstance(job(workflow_name, job_name).get("steps"), list) - - -def _installs_nextest_by_input(step: Step) -> bool: - """Report whether a step asks an installer action for cargo-nextest.""" - if str(step.get("uses", "")).startswith(SHARED_ACTION_PREFIX): - # The coverage action installs nextest deliberately; that is the one - # place it is meant to happen. - return False - inputs = step.get("with") - if not isinstance(inputs, dict): - return False - return any(NEXTEST_TOOL_NAME in str(value).lower() for value in inputs.values()) - - -def _installs_nextest_by_script(step: Step) -> bool: - """Report whether a step's script fetches cargo-nextest rather than runs it.""" - script = str(step.get("run", "")).lower() - named = NEXTEST_TOOL_NAME in script and any( - verb in script for verb in INSTALL_VERBS - ) - # The install host needs its own token: its name does not contain - # "nextest", and it only ever appears in an install command. - return named or NEXTEST_INSTALL_HOST in script - - -def _nextest_installers(workflow_name: str, job_name: str) -> list[str]: - """Return descriptions of steps in one job that would install nextest.""" - if not _declares_own_steps(workflow_name, job_name): - return [] - return [ - f"{workflow_name}:{job_name}:{step.get('name') or step.get('uses')}" - for step in steps(workflow_name, job_name) - if _installs_nextest_by_input(step) or _installs_nextest_by_script(step) - ] - - -def test_markdown_lint_runs_through_the_pinned_action() -> None: - """Keep the Markdown linter on a reproducible, SHA-pinned release. - - The estate's markdown-formatting-baseline rule requires CI to lint - Markdown through the upstream markdownlint-cli2 action rather than a - shell install, so the pin lives on the action reference. - """ - lint_steps = [ - step - for step in steps("ci.yml", "lint-test") - if str(step.get("uses", "")).startswith("DavidAnson/markdownlint-cli2-action@") - ] - assert len(lint_steps) == 1, ( - "ci.yml:lint-test must lint Markdown with the action once" - ) - reference = str(lint_steps[0]["uses"]).split("@", 1)[1] - assert re.fullmatch(r"[0-9a-f]{40}", reference), ( - "ci.yml:lint-test must pin the markdownlint-cli2 action to a full SHA" - ) - inputs = lint_steps[0].get("with") - assert isinstance(inputs, dict), "ci.yml:lint-test action step must carry inputs" - assert str(inputs.get("globs", "")).splitlines() == [ - "**/*.md", - "**/*.markdown", - "**/*.mdx", - ], "ci.yml:lint-test must lint every supported Markdown extension" - script = next( - step["run"] - for step in steps("ci.yml", "lint-test") - if step.get("name") == "Install CLI tools" - ) - assert isinstance(script, str), "ci.yml:Install CLI tools must run a script" - assert "markdownlint-cli2" not in script, ( - "ci.yml:Install CLI tools must not install markdownlint-cli2 from npm" - ) - - def test_ci_can_be_dispatched_for_warm_cache_measurement() -> None: """Keep a way to re-run CI on trunk without inventing a code change. diff --git a/tests/test_ci_tool_installation.py b/tests/test_ci_tool_installation.py new file mode 100644 index 000000000..52c16f6b2 --- /dev/null +++ b/tests/test_ci_tool_installation.py @@ -0,0 +1,178 @@ +"""Contract tests for how Cuprum's CI installs its tools. + +Every tool a gate runs is fetched from a pinned, prebuilt, checksum-verified +path. A source build is slow, unpinned in its dependencies, and fails against a +pinned toolchain in ways no code change explains; and an installer for a tool +nothing runs is a download whose failure mode nothing exercises. +""" + +from __future__ import annotations + +import re +import typing as typ + +import pytest + +from tests.helpers.ci_placement import declares_steps +from tests.helpers.ci_runners import ( + GENERATE_COVERAGE, + steps, + workflow_document, + workflow_sources, +) + +if typ.TYPE_CHECKING: + from tests.helpers.workflow_types import Step + +#: Any spelling of the tool, so `nextest@` and `cargo-nextest@` both match. +NEXTEST_TOOL_NAME = "nextest" +#: Shell fragments that mean "fetch a binary" rather than "run one". +INSTALL_VERBS = ("install", "curl", "wget") +#: The upstream install host. Its name does not contain "nextest", so it needs +#: its own token, and it only ever appears in an install command. +NEXTEST_INSTALL_HOST = "get.nexte.st" +#: The one sanctioned installer, matched exactly. A prefix over the whole +#: shared-actions namespace would exempt setup-rust, install-mdtablefix and +#: every sibling, so any of them could take a nextest input unremarked. + + +def test_ci_does_not_build_tools_from_source() -> None: + """Keep CI tool installation on trusted, pinned prebuilt paths.""" + for workflow_name, source in workflow_sources(): + assert "cargo install" not in source, ( + f"{workflow_name} must not source-build a Cargo tool" + ) + assert "get.nexte.st/latest" not in source, ( + f"{workflow_name} must pin the nextest binary version" + ) + if "cargo binstall" in source: + assert "--disable-strategies compile" in source, ( + f"{workflow_name} must stop cargo-binstall falling back to a " + "source build" + ) + + +def test_no_workflow_installs_cargo_nextest() -> None: + """Leave the nextest install to the coverage action that now needs it. + + The matrix jobs stopped running the Rust suite, so an installer here would + be an unused download whose failure mode nothing in this repository + exercises. + + Checked structurally rather than by one literal string. A `tool:` input + naming either spelling, a different installer action, or a shell command + that fetches nextest would all pass a check for `tool: nextest@`. + """ + offenders: list[str] = [] + for workflow_name, _ in workflow_sources(): + document = workflow_document(workflow_name) + jobs_mapping = document.get("jobs") + if not isinstance(jobs_mapping, dict): + continue + for job_name in jobs_mapping: + offenders.extend( + _nextest_installers(workflow_name, str(job_name)), + ) + assert not offenders, ( + f"only the shared coverage action may install cargo-nextest; found {offenders}" + ) + + +def _installs_nextest_by_input(step: Step) -> bool: + """Report whether a step asks an installer action for cargo-nextest.""" + if str(step.get("uses", "")) == GENERATE_COVERAGE: + # The coverage action installs nextest deliberately; that is the one + # place it is meant to happen, and it is matched exactly rather than by + # namespace so no sibling action inherits the exemption. + return False + inputs = step.get("with") + if not isinstance(inputs, dict): + return False + return any(NEXTEST_TOOL_NAME in str(value).lower() for value in inputs.values()) + + +def _installs_nextest_by_script(step: Step) -> bool: + """Report whether a step's script fetches cargo-nextest rather than runs it.""" + script = str(step.get("run", "")).lower() + named = NEXTEST_TOOL_NAME in script and any( + verb in script for verb in INSTALL_VERBS + ) + # The install host needs its own token: its name does not contain + # "nextest", and it only ever appears in an install command. + return named or NEXTEST_INSTALL_HOST in script + + +def _nextest_installers(workflow_name: str, job_name: str) -> list[str]: + """Return descriptions of steps in one job that would install nextest.""" + if not declares_steps(workflow_name, job_name): + return [] + return [ + f"{workflow_name}:{job_name}:{step.get('name') or step.get('uses')}" + for step in steps(workflow_name, job_name) + if _installs_nextest_by_input(step) or _installs_nextest_by_script(step) + ] + + +def test_markdown_lint_runs_through_the_pinned_action() -> None: + """Keep the Markdown linter on a reproducible, SHA-pinned release. + + The estate's markdown-formatting-baseline rule requires CI to lint + Markdown through the upstream markdownlint-cli2 action rather than a + shell install, so the pin lives on the action reference. + """ + lint_steps = [ + step + for step in steps("ci.yml", "lint-test") + if str(step.get("uses", "")).startswith("DavidAnson/markdownlint-cli2-action@") + ] + assert len(lint_steps) == 1, ( + "ci.yml:lint-test must lint Markdown with the action once" + ) + reference = str(lint_steps[0]["uses"]).split("@", 1)[1] + assert re.fullmatch(r"[0-9a-f]{40}", reference), ( + "ci.yml:lint-test must pin the markdownlint-cli2 action to a full SHA" + ) + inputs = lint_steps[0].get("with") + assert isinstance(inputs, dict), "ci.yml:lint-test action step must carry inputs" + assert str(inputs.get("globs", "")).splitlines() == [ + "**/*.md", + "**/*.markdown", + "**/*.mdx", + ], "ci.yml:lint-test must lint every supported Markdown extension" + script = next( + step["run"] + for step in steps("ci.yml", "lint-test") + if step.get("name") == "Install CLI tools" + ) + assert isinstance(script, str), "ci.yml:Install CLI tools must run a script" + assert "markdownlint-cli2" not in script, ( + "ci.yml:Install CLI tools must not install markdownlint-cli2 from npm" + ) + + +@pytest.mark.parametrize( + ("uses", "reported"), + [ + pytest.param(GENERATE_COVERAGE, False, id="the-sanctioned-action"), + pytest.param( + "leynos/shared-actions/.github/actions/setup-rust@" + "0" * 40, + True, + id="a-sibling-shared-action", + ), + pytest.param("actions/checkout@" + "0" * 40, True, id="a-third-party-action"), + ], +) +def test_only_the_coverage_action_may_ask_for_nextest( + uses: str, *, reported: bool +) -> None: + """Restrict the exemption to the one action that installs nextest on purpose. + + Driven directly because no workflow here gives a sibling action a nextest + input, so a contract read over the real files passes whether the exemption + is exact or spans the whole namespace. Widening it back to a prefix + survived every test until this one existed. + """ + step = typ.cast("Step", {"uses": uses, "with": {"tool": "cargo-nextest@0.9.0"}}) + assert _installs_nextest_by_input(step) is reported, ( + f"{uses} must {'be' if reported else 'not be'} reported as installing nextest" + ) diff --git a/tests/test_ci_worker_bounds.py b/tests/test_ci_worker_bounds.py new file mode 100644 index 000000000..1517ad863 --- /dev/null +++ b/tests/test_ci_worker_bounds.py @@ -0,0 +1,102 @@ +"""Contract tests for how wide Cuprum's CI jobs fan out. + +A job that asks `pytest-xdist` or Cargo for more workers than the runner has +cores still produces a green suite while thrashing two vCPU, so the worker +counts are derived from one constant that names the shape the job is billed +for. Nothing in a passing run says whether that constant and the label agree. +""" + +from __future__ import annotations + +import re + +from tests.helpers.ci_runners import ( + ROOT, + UBICLOUD_LABEL, + UBICLOUD_VCPUS, + step_inputs, + steps, + workflow_env, + workflow_sources, +) + +MAKEFILE = ROOT / "Makefile" +VCPU_CONSTANT = "LINUX_RUNNER_VCPUS" +#: Make variables that carry the runner's vCPU count into the test command. +#: Only the pytest one remains here: the Rust suite moved to the coverage job, +#: which bounds itself through `CARGO_BUILD_JOBS` and `NEXTEST_TEST_THREADS`. +PARALLELISM_OVERRIDES = ("PYTEST_CARGO_BUILD_JOBS",) + + +def test_the_vcpu_constant_matches_the_assigned_label() -> None: + """Tie the one parallelism constant to the shape the job is billed for.""" + declared = workflow_env("ci.yml")[VCPU_CONSTANT] + assert declared == str(UBICLOUD_VCPUS), ( + f"{VCPU_CONSTANT} must equal the vCPU count of {UBICLOUD_LABEL}, " + f"got {declared!r}" + ) + + +def test_python_tests_derive_their_worker_counts_from_that_constant() -> None: + """Size the matrix suite's Cargo work from the constant, not a literal.""" + script = next( + step["run"] + for step in steps("ci.yml", "typecheck-test") + if step.get("name") == "Run tests" + ) + assert isinstance(script, str), "ci.yml:typecheck-test must run a test script" + for variable in PARALLELISM_OVERRIDES: + assert f'{variable}="${{{VCPU_CONSTANT}}}"' in script, ( + f"ci.yml:typecheck-test must pass {variable} from {VCPU_CONSTANT}" + ) + + +def test_extension_and_benchmark_builds_are_bounded_too() -> None: + """Bound the two jobs that compile outside `make test` to the same count.""" + for job_name, step_name in ( + ("extension-tests", "Build the native extension"), + ("benchmark-ratchet", "Run throughput benchmarks and ratchet comparison"), + ): + script = next( + step["run"] + for step in steps("ci.yml", job_name) + if step.get("name") == step_name + ) + assert isinstance(script, str), f"ci.yml:{job_name} must run {step_name!r}" + assert f'CARGO_BUILD_JOBS="${{{VCPU_CONSTANT}}}"' in script, ( + f"ci.yml:{job_name} must bound Cargo build jobs by {VCPU_CONSTANT}" + ) + + +def test_python_suites_never_ask_for_unbounded_workers() -> None: + """Reject `-n auto`: the runner has two cores whatever the host reports.""" + sources = [source for _, source in workflow_sources()] + sources.append(MAKEFILE.read_text(encoding="utf-8")) + for source in sources: + assert "-n auto" not in source, "xdist worker counts must be explicit" + + +def test_the_python_suite_stays_serial() -> None: + """Keep pytest serial while its batches contend on one Cargo target.""" + makefile = MAKEFILE.read_text(encoding="utf-8") + assert re.search(r"^PYTEST_WORKERS \?= 0$", makefile, re.MULTILINE), ( + "PYTEST_WORKERS must default to 0; the batches compile and reuse the " + "same Rust artefacts, so xdist workers would contend on one build lock" + ) + for workflow_name, job_name in (("ci.yml", "coverage"),): + coverage_step = next( + step + for step in steps(workflow_name, job_name) + if str(step.get("uses", "")).startswith( + "leynos/shared-actions/.github/actions/generate-coverage@" + ) + ) + inputs = step_inputs(coverage_step, f"{workflow_name}:{job_name} inputs") + workers = inputs.get("pytest-workers") + assert isinstance(workers, str), ( + f"{workflow_name}:{job_name} coverage must declare pytest-workers" + ) + assert not workers, ( + f"{workflow_name}:{job_name} coverage must run pytest serially, " + f"got {workers!r}" + ) diff --git a/tests/test_ci_workflow_loader.py b/tests/test_ci_workflow_loader.py new file mode 100644 index 000000000..520a7097f --- /dev/null +++ b/tests/test_ci_workflow_loader.py @@ -0,0 +1,105 @@ +"""Tests for the workflow loader's failure boundary. + +Every placement, event and cache query reaches the filesystem through +`read_source` and `read_workflow`. These drive both with real files under a +temporary directory, because this repository's own workflows are readable and +valid, so a contract parametrized over them cannot show what a read or parse +failure looks like. +""" + +from __future__ import annotations + +import typing as typ + +import pytest + +from tests.helpers.ci_workflows import read_source, read_workflow, workflow_sources + +if typ.TYPE_CHECKING: + from pathlib import Path + + +def test_a_missing_workflow_fails_naming_the_file(tmp_path: Path) -> None: + """A workflow that does not exist is a contract failure citing its name.""" + with pytest.raises(AssertionError, match=r"absent\.yml could not be read"): + read_workflow(tmp_path / "absent.yml") + + +def test_an_unreadable_source_fails_naming_the_file(tmp_path: Path) -> None: + """A path that cannot be read as text fails as a contract, not an OSError. + + A directory stands in for an unreadable file: reading it raises an + ``OSError`` on every platform, without depending on permission bits the + test process may be privileged enough to ignore. + """ + unreadable = tmp_path / "unreadable.yml" + unreadable.mkdir() + with pytest.raises(AssertionError, match=r"unreadable\.yml could not be read"): + read_source(unreadable) + + +def test_invalid_yaml_fails_naming_the_file(tmp_path: Path) -> None: + """A parser error is translated and attributed to the file that caused it. + + The case is genuinely unparsable. A document that parses but has the + wrong shape exercises only the mapping check, which is covered separately. + """ + broken = tmp_path / "broken.yml" + broken.write_text("jobs: [unclosed\n", encoding="utf-8") + with pytest.raises(AssertionError, match=r"broken\.yml is not valid YAML"): + read_workflow(broken) + + +def test_a_document_that_is_not_a_mapping_fails_naming_the_file( + tmp_path: Path, +) -> None: + """Valid YAML of the wrong shape is refused with the file's name.""" + listed = tmp_path / "listed.yml" + listed.write_text("- not a mapping\n", encoding="utf-8") + with pytest.raises(AssertionError, match=r"listed\.yml must parse to a mapping"): + read_workflow(listed) + + +def test_a_valid_workflow_keeps_its_boolean_trigger_key(tmp_path: Path) -> None: + """A readable workflow parses unnarrowed, with YAML 1.1's `on` as `True`.""" + valid = tmp_path / "valid.yml" + valid.write_text("on: push\njobs: {}\n", encoding="utf-8") + parsed = read_workflow(valid) + assert parsed == {True: "push", "jobs": {}}, ( + f"valid.yml should parse unnarrowed, got {parsed!r}" + ) + + +def test_the_source_sweep_reads_through_the_named_boundary(tmp_path: Path) -> None: + """The sweep over every workflow fails by name, not with a bare OSError. + + `workflow_sources` feeds the text-level contracts, so it must use the same + boundary as the parsed readers. A directory matching the glob stands in + for a workflow that cannot be read. + """ + (tmp_path / "ci.yml").write_text("on: push\n", encoding="utf-8") + (tmp_path / "stuck.yml").mkdir() + with pytest.raises(AssertionError, match=r"stuck\.yml could not be read"): + workflow_sources(tmp_path) + + +@pytest.mark.parametrize("shape", ["missing", "file", "empty"]) +def test_the_source_sweep_refuses_a_directory_it_cannot_sweep( + tmp_path: Path, shape: str +) -> None: + """A sweep that finds nothing fails instead of reporting a clean estate. + + ``Path.glob`` on a missing directory returns an empty list, and every + "no workflow does X" contract would then pass having read nothing. A path + that is a file, and a directory holding no workflow, are the same hazard. + """ + target = tmp_path / "workflows" + match shape: + case "file": + target.write_text("", encoding="utf-8") + case "empty": + target.mkdir() + case _: + pass + with pytest.raises(AssertionError, match=r"workflow directory|holds no workflow"): + workflow_sources(target)