-
Notifications
You must be signed in to change notification settings - Fork 2
feat(cargo-anvil): tell the ADO job wrapper which check group it renders #85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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", | ||
|
|
@@ -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"][..], | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖: Non-blocking — These expected group names are a third hardcoded copy alongside |
||
| ), | ||
| ( | ||
| 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"); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) (2) The value set is elided here (" |
||
| # 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. | ||
|
|
@@ -29,6 +45,9 @@ | |
| parameters: | ||
| - name: name | ||
| type: string | ||
| - name: group | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| type: string | ||
| default: '' | ||
| - name: pool | ||
| type: object | ||
| - name: steps | ||
|
|
||
There was a problem hiding this comment.
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 itsname/pool/steps/artifactsparameter contract".ado.md:207-208— the artifact tree: "takesname,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 callssteps/job.ymlwith nogroup:, whilestages_identify_the_check_group_for_every_jobnow 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
groupto the two enumerations, qualifying the divergence/merge-free-update promise to the wrapper body, and addinggroup: impact/group: pr-fastto 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.