Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/cargo-anvil/docs/design/ado.md
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ The contract is intentionally small and stable:
| Parameter | Type | Required | Meaning |
|-------------|------------|----------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `name` | `string` | yes | Job name; ADO derives the display name from it. |
| `group` | `string` | no | The anvil check group this job runs (`impact`, `pr-fast`, `pr-test`, `pr-runtime-analysis`, `pr-mutants`, `scheduled-*`). The default wrapper ignores it; extension-template wrappers use it to vary per-job `templateContext:`. Per-OS job names repeat across stages (`linux` / `windows` in every pr-\* and scheduled-\* stage, `compute_linux` / `compute_windows` in impact), so `name` alone cannot identify the group, and `templateContext:` is evaluated at template-expansion time so a runtime stage-name condition is not available either. Defaults to `''` so a caller may omit it — but a wrapper that does not *declare* the parameter is rejected by ADO once the stages templates pass it, so an adopter who owns this file takes the wrapper and stages updates together. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖: Non-blocking — This new row is the only place the widened contract and its adopter coupling are written down; five mirrors of the old contract elsewhere now contradict it:

  • ado.md:186 — "exposes through its name/pool/steps/artifacts parameter contract".
  • ado.md:207-208 — the artifact tree: "takes name, pool, steps, artifacts".
  • ado.md:228-230 — "The stages templates address the wrapper only via its parameter contract (name, pool, steps, artifacts), so the wrapper can diverge arbitrarily without blocking stage-shape updates."
  • ado.md:427-431 — "stages updates flow through without merging, and the user's wrapper changes survive every anvil upgrade."
  • ado.md:466-471 — the §4.2 sample stages template calls steps/job.yml with no group:, while stages_identify_the_check_group_for_every_job now requires one on every invocation.

The 228-230 / 427-431 claims are the ones that matter: what stays true is that the wrapper body can diverge arbitrarily, but a growth of the parameter contract now does block a stage-shape update for exactly the adopters the wrapper exists for. Worth adding group to the two enumerations, qualifying the divergence/merge-free-update promise to the wrapper body, and adding group: impact / group: pr-fast to the §4.2 sample.

Same stale claim in templates/ado/steps/job.yml:9-11 ("continue to update normally because they only know about this wrapper's parameter contract"), 18 lines above the new note that says otherwise — and that is the file adopters take ownership of.

| `pool` | `object` | yes | Pool block, passed verbatim to ADO's `pool:` key. `linuxPool` and `windowsPool` at the stage level are object parameters, so users can override their shape (e.g. `{ name, os, image }` for 1ESPT). |
| `steps` | `stepList` | yes | Body of the job. Templated step lists are fine — the wrapper splices them in via `${{ each step in parameters.steps }}: - ${{ step }}`. |
| `artifacts` | `object` | no | List of pipeline artifacts to publish. Each item: `{ name: string, path: string }`. Default wrapper appends one `PublishPipelineArtifact@1` per entry; 1ESPT wrappers translate the same list into `templateContext.outputs.pipelineArtifact` blocks. The stages templates don't need to know which backend they're targeting. |
Expand All @@ -377,6 +378,7 @@ The default wrapper anvil ships is six lines of logic:
```yaml
parameters:
- { name: name, type: string }
- { name: group, type: string, default: '' }
- { name: pool, type: object }
- { name: steps, type: stepList }
- { name: artifacts, type: object, default: [] }
Expand Down
39 changes: 39 additions & 0 deletions crates/cargo-anvil/src/anvil/artifacts/ado.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ mod tests {
fn job_wrapper_declares_expected_contract() {
for needle in [
"name: name",
"name: group",
"name: pool",
"name: steps",
"type: stepList",
Expand All @@ -264,6 +265,44 @@ mod tests {
}
}

#[test]
fn stages_identify_the_check_group_for_every_job() {
// Per-OS job names repeat across stages (`linux` / `windows` in every
// pr-* and scheduled-* stage, `compute_linux` / `compute_windows` in
// impact), so `name` cannot tell an extension-template wrapper which
// group it is rendering. `group` carries that, letting a wrapper vary
// per-job `templateContext:` (which is evaluated at template-expansion
// time, so a runtime stage-name condition is not an option).
for (template, groups) in [
(
PR_STAGES,
&["impact", "pr-fast", "pr-test", "pr-runtime-analysis", "pr-mutants"][..],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖: Non-blocking — These expected group names are a third hardcoded copy alongside GROUPS (ado.rs:48-57) and GROUP_STEPS (ado.rs:155-167), which the suite already cross-checks against each other. That makes this test narrower than its name: it catches a steps/job.yml invocation added without a group, but a group added to GROUPS/GROUP_STEPS and never threaded into the stages templates passes silently — the exact regression the test exists to prevent. GROUPS is #[cfg(test)] in this module and already in scope via use super::*, so deriving both lists from it (PR = once("impact").chain(GROUPS[..4]), scheduled = GROUPS[4..], or a split on the pr- / scheduled- prefix) would make adding a group fail here until the stages templates thread it, and drops the third copy.

),
(
SCHEDULED_STAGES,
&[
"scheduled-test",
"scheduled-advisories",
"scheduled-runtime-analysis",
"scheduled-exhaustive",
][..],
),
] {
for group in groups {
assert_eq!(
template.matches(&format!("group: {group}\n")).count(),
2,
"group '{group}' must be declared on both per-OS jobs"
);
}
assert_eq!(
template.matches("- template: steps/job.yml").count(),
template.matches("\n group: ").count(),
"every steps/job.yml invocation must pass a group"
);
}
}

#[test]
fn render_group_step_has_include_inputs_and_env() {
let body = render_group_step("pr-fast");
Expand Down
10 changes: 10 additions & 0 deletions crates/cargo-anvil/templates/ado/pr-stages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,14 @@ stages:
- template: steps/job.yml
parameters:
name: compute_linux
group: impact
pool: ${{ parameters.linuxPool }}
steps:
- template: steps/impact.yml
- template: steps/job.yml
parameters:
name: compute_windows
group: impact
pool: ${{ parameters.windowsPool }}
steps:
- template: steps/impact.yml
Expand Down Expand Up @@ -77,6 +79,7 @@ stages:
- template: steps/job.yml
parameters:
name: linux
group: pr-fast
pool: ${{ parameters.linuxPool }}
steps:
- template: steps/pr-fast.yml
Expand All @@ -97,6 +100,7 @@ stages:
- template: steps/job.yml
parameters:
name: windows
group: pr-fast
pool: ${{ parameters.windowsPool }}
steps:
- template: steps/pr-fast.yml
Expand Down Expand Up @@ -137,6 +141,7 @@ stages:
- template: steps/job.yml
parameters:
name: linux
group: pr-test
pool: ${{ parameters.linuxPool }}
steps:
- template: steps/pr-test.yml
Expand All @@ -153,6 +158,7 @@ stages:
- template: steps/job.yml
parameters:
name: windows
group: pr-test
pool: ${{ parameters.windowsPool }}
steps:
- template: steps/pr-test.yml
Expand Down Expand Up @@ -181,6 +187,7 @@ stages:
- template: steps/job.yml
parameters:
name: linux
group: pr-runtime-analysis
pool: ${{ parameters.linuxPool }}
steps:
- template: steps/pr-runtime-analysis.yml
Expand All @@ -191,6 +198,7 @@ stages:
- template: steps/job.yml
parameters:
name: windows
group: pr-runtime-analysis
pool: ${{ parameters.windowsPool }}
steps:
- template: steps/pr-runtime-analysis.yml
Expand All @@ -215,6 +223,7 @@ stages:
- template: steps/job.yml
parameters:
name: linux
group: pr-mutants
pool: ${{ parameters.linuxPool }}
steps:
- template: steps/pr-mutants.yml
Expand All @@ -225,6 +234,7 @@ stages:
- template: steps/job.yml
parameters:
name: windows
group: pr-mutants
pool: ${{ parameters.windowsPool }}
steps:
- template: steps/pr-mutants.yml
Expand Down
8 changes: 8 additions & 0 deletions crates/cargo-anvil/templates/ado/scheduled-stages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ stages:
- template: steps/job.yml
parameters:
name: linux
group: scheduled-test
pool: ${{ parameters.linuxPool }}
steps:
- template: steps/scheduled-test.yml
Expand All @@ -35,6 +36,7 @@ stages:
- template: steps/job.yml
parameters:
name: windows
group: scheduled-test
pool: ${{ parameters.windowsPool }}
steps:
- template: steps/scheduled-test.yml
Expand All @@ -54,12 +56,14 @@ stages:
- template: steps/job.yml
parameters:
name: linux
group: scheduled-advisories
pool: ${{ parameters.linuxPool }}
steps:
- template: steps/scheduled-advisories.yml
- template: steps/job.yml
parameters:
name: windows
group: scheduled-advisories
pool: ${{ parameters.windowsPool }}
steps:
- template: steps/scheduled-advisories.yml
Expand All @@ -75,12 +79,14 @@ stages:
- template: steps/job.yml
parameters:
name: linux
group: scheduled-runtime-analysis
pool: ${{ parameters.linuxPool }}
steps:
- template: steps/scheduled-runtime-analysis.yml
- template: steps/job.yml
parameters:
name: windows
group: scheduled-runtime-analysis
pool: ${{ parameters.windowsPool }}
steps:
- template: steps/scheduled-runtime-analysis.yml
Expand All @@ -97,12 +103,14 @@ stages:
- template: steps/job.yml
parameters:
name: linux
group: scheduled-exhaustive
pool: ${{ parameters.linuxPool }}
steps:
- template: steps/scheduled-exhaustive.yml
- template: steps/job.yml
parameters:
name: windows
group: scheduled-exhaustive
pool: ${{ parameters.windowsPool }}
steps:
- template: steps/scheduled-exhaustive.yml
19 changes: 19 additions & 0 deletions crates/cargo-anvil/templates/ado/steps/job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,22 @@
#
# Contract (intentionally small and stable):
# - name (string) Job name; ADO derives the display name from it.
# - group (string) Optional. The anvil check group this job runs
# (`impact`, `pr-fast`, `pr-mutants`, ...). The

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖: Non-blocking — Two things about this parameter's vocabulary, in the one emitted file adopters hand-edit and branch on.

(1) group already means something else one directory over. steps/setup.yml:8-19 takes a parameter also called group, with a different domain: '' = install the full catalog, none = skip, otherwise a check-group name. Both are visible inside the same job — the impact job passes group: impact to this wrapper while steps/impact.yml:27 passes group: none to setup. A wrapper author will reasonably assume one vocabulary. Either name this one for the domain it actually carries (stage — every value passed is the ADO stage identifier, which also makes impact natural where it is not an anvil check group per GROUPS, ado.rs:48-57), or add a line here saying this group is the job's check group plus impact, and is unrelated to setup.yml's installer selector. Worth settling before it becomes part of an "intentionally stable" contract.

(2) The value set is elided here ("impact, pr-fast, pr-mutants, ...") but a wrapper author has to write ${{ if eq(parameters.group, '...') }} against exact strings, and the exhaustive list lives only in docs/design/ado.md:371. Listing all nine would make the contract self-contained in the file people read.

# default wrapper ignores it; extension-template
# wrappers use it to vary per-job `templateContext:`,
# which cannot be expressed any other way: per-OS job
# names repeat across stages (`linux` / `windows` in
# every pr-* and scheduled-* stage, `compute_linux` /
# `compute_windows` in impact), so `name` alone does
# not identify the group, and `templateContext:` is
# evaluated at template-expansion time so a runtime
# stage-name condition is not available either.
# Defaults to '' so a caller may omit it. A wrapper
# that does not DECLARE the parameter is rejected by
# ADO once the stages templates pass it, so an adopter
# who owns this file takes the wrapper and stages
# updates together.
# - pool (object) Pool block, passed verbatim to ADO's `pool:` key.
# - steps (stepList) Body of the job. Templated step lists are fine.
# - artifacts (object) Optional list of pipeline artifacts to publish.
Expand All @@ -29,6 +45,9 @@
parameters:
- name: name
type: string
- name: group

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖: Non-blocking — This declaration is where the adopter-visible break lands: once the stages templates pass group unconditionally (pr-stages.yml:86 onward), a repository owning a customized steps/job.yml gets a committed pipeline that fails at ADO template expansion until it merges the .proposed wrapper — anvil overwrites the owned stages files but only proposes the wrapper change. The commit body states the migration precisely, which is most of the work; the gap is that it says it in prose on a feat(cargo-anvil): subject, so the changelog tooling classifies it as an ordinary feature and the one line adopters need never reaches the release notes. Consider marking the squash commit breaking (feat(cargo-anvil)!: or a BREAKING CHANGE: trailer), or carrying the migration line into the 0.4.0 CHANGELOG's 📝 Notes section: "if you own .pipelines/anvil/steps/job.yml, take the proposed wrapper update in the same change as the regenerated pr.yml / scheduled.yml." No code change implied.

type: string
default: ''
- name: pool
type: object
- name: steps
Expand Down
Loading
Loading