feat(ci): per-PR selective test selection wired into tests.yml (audit mode)#18127
Draft
radical wants to merge 43 commits into
Draft
feat(ci): per-PR selective test selection wired into tests.yml (audit mode)#18127radical wants to merge 43 commits into
radical wants to merge 43 commits into
Conversation
Adds a descriptive map of which CI targets must run when a given repo path changes, covering the .NET test projects and the validation/polyglot jobs in tests.yml. Intended to audit/validate a future selective-CI implementation; nothing consumes it yet (CI still runs the full matrix, gated only by eng/testing/github-ci-trigger-patterns.txt). - docs/ci/test-trigger-map.yml: machine-readable rules (aliases, test_self, run_all, test_hubs, shared_source, core_source, curated_jobs, loose_file_deps, leaf_source, shared_compiled_source, gaps) - docs/ci/test-trigger-map.md: methodology, target vocabulary, caveats Derivation: ProjectReference closure over src->src edges (so leaf integrations are not inflated by the shared Aspire.Hosting.Tests hub), plus file-granular <Compile Include> coupling for shared source link-compiled across projects. Curated rules layer on for runtime/loose-file deps and the non-.NET jobs (polyglot/typescript/extension/cli/api/deployment). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Design for a tool that maps a PR's changed files to the subset of test
projects and CI jobs to run, so PR CI runs a relevant subset instead of
the full matrix. Companion to the descriptive test-trigger-map.{md,yml}.
Two layers, split by who can know the dependency:
- Derived (zero maintenance): dotnet-affected over the MSBuild graph,
scoped to Aspire.slnx, covering ProjectReference reverse closure,
foreign linked <Compile Include>, CPM, and Directory.Build.* changes.
- Curated (~80 lines): the non-.NET jobs and runtime loose-file reads
the project graph cannot see.
Records design decisions and the measured selectivity profile: any
hosting-integration or data-component change fans out to ~36 hosting
tests via two structural edges (the Aspire.Hosting.Tests hub and the
tests/testproject mega-apphost). This fan-out is accepted deliberately
-- still far cheaper than the full matrix -- and the edges are not
pruned. CLI (1), Dashboard (7), and component-to-component isolation
stay tightly scoped.
Also specifies audit mode (full matrix still runs; a GitHub step summary
shows the would-have-been-skipped set), a verifier test for the curated
layer, the pipeline insertion point, and operational constraints proven
by probes (Aspire.slnx scoping, DOTNET_ROOT, internal-feed mirroring).
Proposal only; nothing consumes it yet.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The curated test-trigger-map listed three run_all_globs entries that match no tracked file (eng/Tools.props, eng/Packaging.targets, eng/targets/**), so they could never trigger anything. Replaced them with eng/*.props and eng/*.targets so present and future root build-infra files fail-open to ALL. Also added a shared_compiled_source entry for src/Components/Common/ConfigurationSchemaAttributes.cs -> ConfigurationSchemaGenerator.Tests: that test link-compiles the file but was covered by no rule, so a change to it would have skipped a test that compiles it. Updated the design doc to use dotnet-affected's current --filter-file-path flag (--solution-path is obsolete in 6.2.0). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds Infrastructure.Tests coverage that keeps the hand-maintained docs/ci/test-trigger-map.yml honest against repo reality: referential integrity of test:/job: targets, alias resolution, every glob matches a tracked file, every src project is reachable by some rule, structural hygiene, and that every <Compile Include> of a shared file into a test project is selected by the map. Plus an outerloop, self-skipping oracle cross-check against dotnet-affected. Parses the map with YamlDotNet (test-only) and matches globs with Microsoft.Extensions.FileSystemGlobbing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extracts the dependency kinds the curated test-trigger-map describes into a behavior spec for the planned SelectTests selector: leaf/core ProjectReference closure, component isolation, file-granular <Compile Include>, test hubs, shared source dirs, run_all catch-all + fail-open, test_self, curated jobs, runtime/loose-file deps, additive union, alias expansion, Layer-1 union, and the kill switch. The 25 tests are intentionally failing: TestSelector.Select throws until the selector is implemented. Implementing it to make these pass is the definition of done. Adds the tools/SelectTests skeleton (contract types + throwing impl) so the tests compile, and a map rule (tools/SelectTests/** -> Infrastructure.Tests). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
TestSelector.Select threw NotImplementedException, so the 25 acceptance tests in SelectTestsAcceptanceTests (the behavior spec for the selective-CI tool) were RED. Implement the Layer 2 resolution that consumes docs/ci/test-trigger-map.yml: - Match each changed file against every rule section (run_all_globs, test_self, test_hubs, shared/core/leaf/shared_compiled source, loose_file_deps, curated_jobs); union the targets. - Expand ALL_HOSTING_TESTS / ALL_COMPONENT_TESTS aliases to concrete test projects; route test: targets to TestProjects and job: targets to Jobs. - Fail open: a src/** file matched by no rule escalates to the full matrix (a missed test is a silent regression; an extra run is just slower). - Kill switch (ForceAll) and any run_all match escalate to the full matrix with an EscalationReason; ALL expands TestProjects to the matrix passed in and Jobs to every curated job token. - Union the injected Layer 1 (graph-derived) affected projects. The tool gets its own internal YamlDotNet parse model (TriggerMap) rather than sharing the verifier's model: the test project references the tool, so sharing would be a circular dependency. The model is internal, so it adds no public API surface. YamlDotNet is needed by the tool under central package management, so its PackageVersion moves from tests/Directory.Packages.props (which imports the root) up to the root Directory.Packages.props. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Program.cs was a stub. Turn it into the real entry point that runs the
two-layer selection the design describes, in audit mode by default.
Flow:
- Read the enumerate-tests all_tests matrix JSON; the projectName values are
the universe an ALL selection expands to.
- Resolve changed files via `git diff --name-only <from> <to>` (or
--changed-files). Layer 2 globs match these directly.
- Layer 1: run dotnet-affected over the MSBuild ProjectGraph and union its
reported test projects in. It reports the union of changed + affected
projects (one affected.json: [{Name, FilePath}]); we keep the names that
are matrix test projects.
- Call TestSelector (Layer 2), then emit the matrix, per-job booleans
(run_polyglot, run_extension_e2e, ...), and a step summary.
Audit vs enforce: without --enforce the full matrix is emitted unchanged
(audit rollout — CI keeps running everything while the map is validated);
--enforce emits the filtered matrix. --force-all is the kill switch.
dotnet-affected is invoked as a local tool ('dotnet tool run
dotnet-affected') so CI only needs 'dotnet tool restore'; it is added to
.config/dotnet-tools.json and resolves from the dotnet-public feed (its '*'
package-source mapping), so no NuGet.config change is needed.
--dotnet-affected overrides with a standalone executable.
Layer 1 failure handling matches the rollout: audit mode warns and falls
back to Layer 2 only; --enforce fails loudly, since under-selecting would
silently skip real tests.
The step summary now reports the options, the incoming changed files, and
the final selection (selected projects, would-have-been-skipped, jobs) so an
audit reader can see exactly what drove the result.
Verified end-to-end against a normal checkout: dotnet-affected reports the
correct affected projects and the pipeline produces the filtered matrix.
Note: dotnet-affected's --from/--to reads git blobs through libgit2 and
throws inside a git worktree (where .git is a file); CI uses a normal
checkout, and a code comment documents using --skip-layer1 for local
worktree dev.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dotnet-affected silently ignores changed files it can't map to a project (extension/**, docs/**, loose tooling dirs, ...) — it returns no signal for them. Those are exactly the files the curated map (Layer 2) must cover, so a newly added loose-file dependency could otherwise select nothing and go untested with no warning. Add SelectionResult.UnmatchedFiles: the changed files that matched no Layer 2 rule. Because the verifier keeps every src project rule-reachable, in a green repo these are precisely the loose, non-project files Layer 1 also cannot attribute — i.e. the "neither layer" set. The audit summary now lists them under "Unattributed changed files" as an early warning to add a curated rule. Tested: loose unmapped file is reported; mapped files (leaf rule, curated job, test_self, run_all) are not; the set unions across changed files; and an unmapped src file is both reported and fails open to ALL. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The map's leaf_source / core_source / test_hubs sections duplicated the MSBuild project-graph closure that dotnet-affected (Layer 1) computes live, so they were a maintenance trap. Delete them and keep only what the graph provably cannot see (Layer 2): run_all_globs, curated_jobs, loose_file_deps, shared_source, shared_compiled_source, test_self, and named groups. The selector unions Layer 1's affected projects with these rules, so the deleted fan-out is reproduced at runtime and can never drift. The yml drops from ~430 to ~160 lines. Named groups: generalize the old test-only aliases into "groups" that can hold both test: and job: members, expanded by routing each member by prefix. Use it to map eng/Bundle.proj (CLI bundle assembly) to a CLI_BUNDLE group (Aspire.Cli.EndToEnd.Tests + job:cli-starter + job:extension-e2e) rather than the full matrix. .proj traversal files are invisible to dotnet-affected (not in Aspire.slnx, and not a supported project type), so they must be curated; eng/OuterPreBuild.proj (build-wide validation) goes to run_all. Fail-open change: the selector no longer escalates an unmapped src file to ALL. Post-trim most src files match no Layer 2 rule on purpose (Layer 1 owns the closure), so that would over-escalate. Src safety is now "Layer 1 must run" (the Program entry point fails closed in enforce mode, falls back to the full matrix in audit mode) plus the unmatched-files audit signal. Verifier reworked to match: "every src project is reachable by a rule" becomes "every src project is in Aspire.slnx (so dotnet-affected sees it) or covered by a curated rule" — the ~16 deliberately out-of-slnx src projects (template placeholders, tools) are covered by loose_file_deps. The alias check becomes a groups check that allows job: members. The link-compile coverage check still guards shared_compiled_source. The oracle test (project-level subset vs dotnet-affected) is removed: with the graph sections gone it no longer has anything to cross-check. Acceptance tests rewritten to a pure input -> SelectionResult contract: given changed files (and the injected Layer 1 set where the closure is now Layer 1's job), assert on selected projects / jobs / SelectsAll / UnmatchedFiles, instead of asserting on map section or group names. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The curated test-trigger map hand-maintained edges the MSBuild graph already sees, and split "glob -> targets" across several keys that the selector treated identically. Validated with dotnet-affected 6.2.0 over Aspire.slnx: - a link-compiled `.cs` file reports its consuming test projects in the *changed* set (so a `shared_compiled_source` section was redundant); - a `src/Components/Common/*` change reports 71 `.Tests` (so a `Components/Common -> all-component-tests` rule was redundant); - the vendored OTel instrumentation dirs are graph-covered, and `src/Vendoring/OpenTelemetry.Shared/**` is compiled by nothing (inert). The curated layer now has exactly four matchers -- a section is its own key only when the selector treats it differently: - `conventions`: a `<name>`-capture pattern -> target template, additive and existence-guarded (emitted only if the derived test exists in the matrix). Covers a test's own folder (`tests/<name>/**`) and the Hosting/Components integration dirs -- a backstop for non-MSBuild files (data/resource, compile-excluded) the graph can't attribute. - `ignore`: shared dirs Layer 1 already covers (`src/Components/Common`, the vendored OTel instrumentation dirs) or that are inert, listed so they don't trip the run-all fallback. - `path_rules`: the one general glob -> targets matcher (targets may be `test:` / `job:` / a group / `ALL`). The catch-all-to-ALL, the `Azure.<service>` convention misses, the non-.NET jobs, and the loose-file reads all live here under comment headers. - `derived_targets`: "if any of these selected tests, also run these targets", applied to the union of both layers to a cycle-safe fixpoint (CLI tests -> cli-starter; acquisition -> the winget/homebrew installer jobs). Selector: a changed `src/**` file is "Layer-1-owned" when it lives under a project dir in Aspire.slnx (dotnet-affected emits no per-file map, so this is the proxy). Only a leftover `src/**` file that is neither owned, matched, nor ignored forces the full matrix; non-src leftovers are audit-only. Groups expand recursively (cycle-safe). Verifier: referential integrity (every test:/job:/derived target resolves; every convention pattern substitutes its `<name>`), `src`-project reachability, and dead-glob checks. Acceptance tests drive the engine with synthetic maps so they assert mechanisms, not the real map's contents. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The curated test-trigger map repeated the same src/<Project>/** path globs across many job rules (src/Aspire.Cli/** on 7 rules, src/Aspire.Hosting*/** on 3, src/Aspire.TypeSystem/** on 4, ...). The duplication was a maintenance trap and, worse, under-selected: a literal path glob only fires on files inside that directory, so a change to a library the CLI depends on would not trigger the CLI jobs even though the CLI build is affected. Root cause: Layer 1 (dotnet-affected) already computes the full transitive affected set, but RunLayer1 intersected it down to test-project names and discarded the production-project identity. Layer 2 then had to re-derive "is the CLI affected" from raw file paths. Keep the full affected set from Layer 1 and add a project_rules matcher: an affected production project (matched by project-NAME glob against the affected set) selects jobs/tests. This dedupes the repeated globs (Aspire.Hosting* collapses 66 projects to one line), survives a project moving directories, and follows the graph's transitive closure so a dependency change triggers the dependent's jobs. It is additive and inert when Layer 1 is skipped; the loose-file path_rules still cover those triggers. Only genuine slnx project-root globs migrated. Paths the graph cannot attribute stay in path_rules: loose files, narrow sub-paths (src/Aspire.Cli/Templating/Templates/ts-starter/**), the non-compiled *.ats.txt / *.tscompat.suppression.txt baselines, and src/Aspire/Cli/** (link-compiled into Aspire.Hosting.CodeGeneration.TypeScript, with no owning project) which keeps an explicit rule carrying its old targets. Add tests/Infrastructure.Tests and tests/Aspire.Hosting.Maui.Tests to Aspire.slnx. Both were absent, so Layer 1 could never fan their ProjectReference dependencies into them: Infrastructure.Tests would miss changes to the tools it tests (ExtractTestPartitions, GenerateCITimeline, CreateFailingTestIssue, GenerateTestSummary, TypeScriptApiCompat), and Maui.Tests would miss core Aspire.Hosting changes. Both are plain net10.0 projects with no MAUI workload dependency. Infrastructure.Tests still needs its path_rules because it reads workflows/eng files as loose inputs the graph cannot see. Verifier gains a check that every project_rules name glob resolves to a real Aspire.slnx project; acceptance tests cover the project_rules mechanism (name globs, additive union, group/ALL expansion, feeding derived_targets). Docs updated to five matchers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two follow-ups to the selective-CI review. Audit mode emitted the full test matrix but still wrote the *selective* per-job booleans. Once those gate the non-.NET jobs via `if:`, audit mode would run every test yet skip unselected jobs -- contradicting "audit changes nothing; CI runs everything". This got sharper with project_rules, which only fire when Layer 1 ran: a skipped or failed Layer 1 in audit would switch the project-rule-driven job booleans off. Force every job boolean true in audit mode, matching the full matrix WriteMatrix already emits. The step summary still reports the real (selective) computation, so audit stays advisory: compute and show what enforcing would do, then run everything. Add a verifier asserting every tests/<Name>/<Name>.csproj is in Aspire.slnx (allow-list for deliberate exclusions, empty today). A test project absent from the solution is invisible to Layer 1, so a change to a production dependency could never fan into it and its tests would silently never run in enforcing mode -- the exact gap the Infrastructure.Tests and Aspire.Hosting.Maui.Tests slnx additions closed. This catches the next such regression at PR time. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Run SelectTests as part of tests.yml setup_for_tests and rename project_rules to affected_project_rules so the section name says what it keys off. Wiring (audit by default, enforce is a one-line flip): - setup_for_tests gains a select_tests step: after enumerate-tests it runs SelectTests over the all_tests matrix with --from/--to (PR base..head) and Layer 1 (dotnet-affected, restored via `dotnet tool restore`). It writes the selected matrix, the run_* job booleans, and the advisory summary. - A new setup_for_tests output, selection_enforced, defaults to 'false'. While false, SelectTests emits the full matrix and run_* = true, and every gate reads `selection_enforced != 'true' || run_X == 'true'`, so the full matrix and all jobs run -- behavior is unchanged during the audit phase. Flipping it to 'true' (and adding --enforce) makes the matrix and gates selective. - The step falls back to the full enumerated matrix on any failure, so coverage is never silently reduced. The [full ci] PR-body token and run-all-tests label pass --force-all; untrusted PR fields go through env vars, never interpolated into the script, to avoid shell injection. Job gates: - Drop the hand-rolled extension_e2e_changes regex job; gate extension_e2e_tests on run_extension_e2e instead. The map's extension-e2e rule now carries the same path set the regex had, so it is faithfully replaced. - Gate the previously-unconditional typescript_sdk_tests and typescript_api_compat on their run_* outputs, and teach the results gate to tolerate an enforce-mode skip of those jobs. extension-e2e is intentionally double-routed in the map: its production triggers live in affected_project_rules (transitive coverage when Layer 1 runs) AND as path_rules globs, so a src/Aspire.Cli|Hosting*|Dashboard* change still fires it by pure path match when Layer 1 is unavailable. Other job gates stay project-only. Note: the tests.yml logic cannot be validated locally (GitHub Actions) -- it needs a real CI run. dotnet-affected adds an MSBuild ProjectGraph evaluation to the critical-path setup_for_tests job and must be available on the CI feeds (dnceng). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18127Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18127" |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a per-PR selective test selection system (in audit mode) that computes the subset of test projects and non-.NET CI jobs affected by a PR's changed files, aiming to eventually reduce CI runtime by running only relevant tests.
Changes:
- Adds
tools/SelectTests/— a C# tool that unions graph-derived (Layer 1 viadotnet-affected) and curated (Layer 2 viadocs/ci/test-trigger-map.yml) dependency information to select relevant tests. - Wires the tool into
.github/workflows/tests.ymlin audit mode (full matrix still runs; advisory summary is written to step summary), removes the oldextension_e2e_changesregex job, and gates non-.NET jobs via selector outputs. - Adds comprehensive verifier and acceptance tests in
Infrastructure.Tests/TestTriggerMap/, design documentation, and the curated trigger map.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tools/SelectTests/SelectTests.csproj |
New tool project with YamlDotNet, FileSystemGlobbing, and System.CommandLine deps |
tools/SelectTests/TriggerMap.cs |
YAML map parser + glob/convention/project-name matching utilities |
tools/SelectTests/TestSelector.cs |
Core selection engine: Layer 1/2 union, conventions, fallback, derived targets |
tools/SelectTests/Program.cs |
CLI entry point: git diff, dotnet-affected invocation, matrix/summary output |
tests/Infrastructure.Tests/TestTriggerMap/TestTriggerMapModel.cs |
Parallel YAML model for verifier tests |
tests/Infrastructure.Tests/TestTriggerMap/TestTriggerMapTests.cs |
Verifier: every target/glob/project resolves to repo reality |
tests/Infrastructure.Tests/TestTriggerMap/SelectTestsAcceptanceTests.cs |
40+ acceptance tests for the selector engine using synthetic maps |
tests/Infrastructure.Tests/Infrastructure.Tests.csproj |
Adds YamlDotNet, FileSystemGlobbing, SelectTests project reference |
docs/ci/test-trigger-map.yml |
The curated Layer 2 map (conventions, ignore, path_rules, project_rules, derived) |
docs/ci/test-trigger-map.md |
Human-readable documentation of the map structure and maintenance guide |
docs/ci/test-trigger-selector-design.md |
Design doc: layers, tool flow, audit mode, measured selectivity |
.github/workflows/tests.yml |
Wiring: select_tests step, selector-gated jobs, removes extension_e2e_changes job |
.config/dotnet-tools.json |
Adds dotnet-affected 6.2.0 as a local tool |
Directory.Packages.props |
Adds YamlDotNet 16.3.0 to central package management |
Aspire.slnx |
Adds Infrastructure.Tests and Aspire.Hosting.Maui.Tests to the solution |
Comment on lines
+78
to
+86
| # dotnet-affected (Layer 1 graph closure) ships as a local tool. | ||
| ./dotnet.sh tool restore | ||
|
|
||
| # Audit mode (no --enforce): SelectTests emits the FULL matrix and writes run_* = true, plus | ||
| # the advisory summary. If anything fails, fall back to the full enumerated matrix so test | ||
| # coverage is never silently reduced. (To enforce, add --enforce here and flip | ||
| # selection_enforced above.) | ||
| ./dotnet.sh run --project tools/SelectTests/SelectTests.csproj -- "${args[@]}" \ | ||
| || cp all_tests.json selected_matrix.json |
Extend the selector wiring so every non-.NET job is gated by its run_* output, not just extension-e2e and the typescript jobs. Now also gated: polyglot_validation, cli_starter_validation_windows, extension_tests_win, extension_bootstrap_linux, and the WinGet/Homebrew installer-prepare jobs. Each gate is ANDed with the job's existing event conditions and reads `selection_enforced != 'true' || run_X == 'true'`, so audit keeps running everything and enforce makes them selective. Adds the run_winget_installer / run_homebrew_installer outputs, and wraps each newly-gated job's skip-as-failure check in the results gate so an enforce-mode skip is not treated as a failure (audit behavior is unchanged). The extension-unit jobs gate on run_extension_unit OR run_extension_e2e: extension_e2e_tests `needs:` them, so gating them off while e2e is selected would skip e2e through need-propagation. Base builds and the .NET test jobs stay ungated -- base builds are upstream needs, and the .NET jobs are already gated by their matrix bucket emptying once SelectTests filters the matrix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…enforced The previous wiring gated every job on `selection_enforced != 'true' || run_X == 'true'`, duplicating the audit/enforce decision into every gate. That was redundant: in audit the tool already returns run-all (run_* = true + the full matrix), so the gates only ever need to read what the selector returns. Collapse the audit/enforce decision to a single knob, ENFORCE_SELECTION, in the select_tests step (it just controls whether SelectTests gets --enforce). Every job gate now reads plain `run_X == 'true'` (combined with each job's existing event conditions), and the results skip-checks read `result == 'skipped' && run_Y == 'true'`. Remove the selection_enforced job output entirely. Fail-open is now explicit: if SelectTests fails, the step emits the full matrix and sets every run_* to true, so a tool failure runs everything rather than skipping gated jobs (the role selection_enforced incidentally played before). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| fi | ||
|
|
||
| # dotnet-affected (Layer 1 graph closure) ships as a local tool. | ||
| ./dotnet.sh tool restore |
The "Select relevant tests" step failed at startup with:
An error occurred trying to start process '/usr/bin/bash' with
working directory '/home/runner/work/aspire/aspire'.
Argument list too long
Root cause: the step received the full expanded test matrix through the
ALL_TESTS environment variable. The canonical matrix is ~148KB and the
OS-expanded form is larger, so the single env string exceeds Linux's
MAX_ARG_STRLEN (~128KB). When the runner calls execve to start bash for
the step, the oversized environment makes the call fail with E2BIG,
surfaced as "Argument list too long" — the script never runs.
Fix: write the matrix to all_tests.json in a preceding pwsh step. pwsh
writes its run-script to a file, so interpolating the (trusted,
build-generated) matrix into a literal here-string keeps it out of the
process environment entirely. The bash step then reads the file it
already expected via --matrix all_tests.json, so its logic is unchanged.
PR_BODY stays in env: GitHub caps PR bodies well under the limit, and
keeping it out of the script body avoids shell injection.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rash
The "Select relevant tests" step ran the selector but it threw every time:
Unhandled exception: System.InvalidOperationException:
Provide either --changed-files or --from (with optional --to).
Two defects:
1. SelectTests crash. Selection.Run resolved the changed-file set before
the --force-all short-circuit, so --force-all (whose job is to run the
full matrix *regardless* of the diff) still demanded a --from or
--changed-files input and threw. The workflow reaches --force-all on
the [full ci] kill switch or when no diff base is available — i.e. the
exact case where there are deliberately no diff inputs. Skip
ResolveChangedFiles (and Layer 1) when ForceAll; the selector already
returns the full matrix in that mode.
2. The crash was invisible. The step wrapped the tool in
`if ! ...; then <fallback>; fi`, which swallowed *any* non-zero exit,
emitted the full matrix + run-all, and exited 0 — so the job went
green and the broken selector went unnoticed. The tool already fails
OPEN by design when it *decides* to (force-all / unavailable base ->
full matrix + run-all), so a non-zero exit means an actual crash. Let
it fail the step (and the run); surfacing it is the point of the audit
phase.
Regression test drives Selection.Run with --force-all and no diff inputs
and asserts exit 0 + the full matrix passes through. Reverting fix #1
reproduces the original throw at ResolveChangedFiles.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+106
to
+114
| private static TestSelector Selector(IEnumerable<string>? projectDirs = null) | ||
| { | ||
| var dir = Directory.CreateTempSubdirectory("selecttests"); | ||
| var path = Path.Combine(dir.FullName, "map.yml"); | ||
| File.WriteAllText(path, SyntheticMap); | ||
| return new TestSelector( | ||
| path, | ||
| s_matrix.ToHashSet(StringComparer.Ordinal), | ||
| (projectDirs ?? []).ToHashSet(StringComparer.Ordinal)); |
| (github.event_name == 'pull_request' && | ||
| (needs.extension_tests_win.result == 'skipped' || | ||
| (needs.extension_e2e_changes.outputs.run_extension_e2e == 'true' && | ||
| ((needs.extension_tests_win.result == 'skipped' && (needs.setup_for_tests.outputs.run_extension_unit == 'true')) || |
When a PR has a base SHA but the shallow `git fetch` of that commit fails (or it's otherwise unavailable), the selector step used to fall back to `--force-all` and run everything. That masks a real problem: `base.sha` is always reachable on origin, so a fetch failure means something is wrong (a bad fetch, a rewritten base), and silently running run-all teaches the audit nothing about what selective CI would have picked. Fail the step with a clear error instead. The remaining `--force-all` paths are deliberate: the `[full ci]`/`run-all-tests` kill switch, and non-PR events that have no base SHA at all. Also update the design doc: the prior "fails open on any SelectTests failure" note is stale (the swallow was removed), and document that an unfetchable PR base now fails rather than forcing run-all. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| # summary still reports what enforcing would have skipped. 'true' (enforce): a non-ALL | ||
| # selection writes BeforeBuildProps.props so enumerate-tests enumerates ONLY the selected | ||
| # projects, and run_* gate the non-.NET jobs. | ||
| ENFORCE_SELECTION: 'true' |
Comment on lines
+106
to
+115
| private static TestSelector Selector(IEnumerable<string>? projectDirs = null) | ||
| { | ||
| var dir = Directory.CreateTempSubdirectory("selecttests"); | ||
| var path = Path.Combine(dir.FullName, "map.yml"); | ||
| File.WriteAllText(path, SyntheticMap); | ||
| return new TestSelector( | ||
| path, | ||
| s_matrix.ToHashSet(StringComparer.Ordinal), | ||
| (projectDirs ?? []).ToHashSet(StringComparer.Ordinal)); | ||
| } |
| - [`test-trigger-map.md`](./test-trigger-map.md) — the descriptive path → target map. | ||
| - [`eng/test-trigger-map.yml`](../../eng/test-trigger-map.yml) — its machine-readable form. | ||
|
|
||
| **Status: enforcing.** `tests.yml`'s `setup_for_tests` runs `SelectTests` |
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
eng/test-trigger-map.yml was listed under `ignore`, so a PR that edits only the map selected no test projects. But Infrastructure.Tests/TestTriggerMap (TestTriggerMapTests) is the verifier that validates the map against repo reality -- every test:/job: target names a real project, every glob matches a tracked file, every affected_project glob matches a solution project. With the map ignored, a broken map edit (a typo'd test name, a glob matching nothing) would bypass its own validator in enforce mode and merge unchecked. Route eng/test-trigger-map.yml -> test:Infrastructure.Tests (folded into the existing tools/SelectTests rule, since both are validated by the same project) so a map edit runs the verifier. Also drop two stale comments in TestSelector.cs: a `run_all_globs` reference (that section was renamed to path_rules with an ALL target) and a "resolution logic is not implemented yet; Select throws until it is" remark (Select is fully implemented). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
…olution The Layer 2 changed-file diff ran `git diff --name-only <base> <head>` with git's default rename detection on, which reports only the destination of a rename. A file moved OUT of a loose-file-mapped directory (e.g. `eng/clipack/foo` -> `eng/elsewhere`) therefore hid the old path, so the `eng/clipack/**` rule never fired and that directory's mapped tests (Cli.Tests, Cli.EndToEnd.Tests, Infrastructure.Tests, the installer jobs) were silently skipped in enforce mode -- a real under-selection. Pass `--no-renames` so a rename decomposes into delete(old) + add(new) and both paths are glob-matched. This mirrors Layer 1, which already captures both sides via `-M`. Add SelectTestsCliTests.RenameOutOfMappedPathStillSelectsItsTests: it git-mv's a mapped file out of its path and asserts the source's test is still selected. Verified falsifiable -- it goes red when `--no-renames` is removed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
write-class-mode-test-props.ps1 and scan-test-partitions-from-source.ps1 drive test enumeration for every split test project, but neither was in the trigger map, so a change to one selected no tests in enforce mode. In particular write-class-mode-test-props.ps1 has a dedicated verifier (Infrastructure.Tests/PowerShellScripts/WriteClassModeTestPropsTests) that would not have run on a PR editing it. Add both to the build-infrastructure catch-all (-> ALL), alongside the existing split-test-matrix-by-deps.ps1: a bug in either skews the matrix for every PR, so erring toward the full run is the safe direction, and ALL includes the verifier. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…dates Infrastructure.Tests/PowerShellScripts has dedicated tests that invoke five more real eng/scripts (build-test-matrix.ps1, expand-test-matrix-github.ps1, download-native-archives.ps1, stage-native-cli-tool-packages.ps1, validate-npm-package-signatures.ps1). None were in the trigger map, so a PR editing one selected no tests in enforce mode and skipped that script's verifier -- the same under-selection class as the trigger map and class-mode scripts. - build-test-matrix.ps1 / expand-test-matrix-github.ps1 -> ALL, alongside the existing split-test-matrix-by-deps.ps1: they shape every PR's matrix, so a bug is repo-wide and the full run is the safe direction. - download-native-archives.ps1 / stage-native-cli-tool-packages.ps1 / validate-npm-package-signatures.ps1 -> test:Infrastructure.Tests: the build jobs that run them are unconditional needs, so the unit verifier is the trigger that must fire. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Flip ENFORCE_SELECTION from 'true' to 'false' so the selector runs as an advisory: it still computes and reports the selection (and what enforcing would have skipped) in the job summary and PR comment, but writes no restriction props and forces every run_* job boolean true. enumerate-tests therefore builds the full matrix and all jobs run -- identical coverage to before selective CI. Verified: a docs-only change (which enforcing would reduce to 0/97 test projects) yields run_*=true for every job, before_build_props empty, and has_dotnet_tests=true, while the summary still reports "Would have been skipped (97)". Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The new class-mode / source-partition CI scripts raised fatal errors with Write-Error. Under PowerShell's default ConciseView, a script Write-Error is rendered with ANSI color codes and wrapped across multiple lines at the host's buffer width. The wrap point is environment-dependent (~120 cols on an ubuntu runner vs. the local terminal), so the message splits at different places. That made WriteClassModeTestPropsTests.FailsWhenClassModeMetadataDoesNotContain ProjectPath flaky-by-environment: a newline landed inside "does not contain testProjectPath", so Assert.Contains missed it. The test passed locally and failed on the 120-col ubuntu Infrastructure job (which also tripped the two Final Results gates). Emit the fatal message via [Console]::Error.WriteLine + exit 1 instead, so the diagnostic is a single contiguous, ANSI-free stderr line in every environment. Applied to both new scripts (write-class-mode-test-props.ps1 and scan-test-partitions-from-source.ps1). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Addresses review findings on the per-PR test selector: - Isolate the PR-write token: post the sticky selection comment from a separate comment_selection job that runs no PR code. The job that checks out and runs PR-authored code (SelectTests, the test build) drops to contents:read and persist-credentials:false. The summary is handed over as an artifact. - Selector: with --skip-layer1 there is no graph attribution, so pass an empty project-directory set; otherwise a src/** file under a solution project dir looked "Layer-1-owned" and skipped the run-all fallback, silently selecting nothing under --enforce. Add a falsifiability test. - Partition source-scan: accept the Trait shapes the compiled extractor does (namespace-qualified, Attribute suffix, case-insensitive key) so a non-canonical Partition trait can't be silently dropped from enumeration. - Remove the temporary ci.yml dogfood trigger; fix a stale tests.yml comment about a repo restore that no longer happens. - Tests: add the missing prefix-guard and convention-guard acceptance cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+106
to
+114
| private static TestSelector Selector(IEnumerable<string>? projectDirs = null) | ||
| { | ||
| var dir = Directory.CreateTempSubdirectory("selecttests"); | ||
| var path = Path.Combine(dir.FullName, "map.yml"); | ||
| File.WriteAllText(path, SyntheticMap); | ||
| return new TestSelector( | ||
| path, | ||
| s_matrix.ToHashSet(StringComparer.Ordinal), | ||
| (projectDirs ?? []).ToHashSet(StringComparer.Ordinal)); |
| - [`test-trigger-map.md`](./test-trigger-map.md) — the descriptive path → target map. | ||
| - [`eng/test-trigger-map.yml`](../../eng/test-trigger-map.yml) — its machine-readable form. | ||
|
|
||
| **Status: enforcing.** `tests.yml`'s `setup_for_tests` runs `SelectTests` |
| private static void WriteJobBooleans(RunOptions options, SelectionResult result) | ||
| { | ||
| var githubOutput = Environment.GetEnvironmentVariable("GITHUB_OUTPUT"); | ||
| var allJobs = TriggerMap.Load(options.MapPath).AllJobTokens().ToHashSet(StringComparer.Ordinal); |
This was referenced Jun 13, 2026
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Contributor
|
❓ CLI E2E Tests unknown — 115 passed, 0 failed, 2 unknown (commit View all recordings
📹 Recordings uploaded automatically from CI run #27451670026 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this adds
Selective CI for PRs: a tool (
tools/SelectTests) that, given a PR's changed files, computes the subset of test projects and non-.NET jobs actually affected, so PR CI can run a relevant subset instead of the full matrix.This lands in audit mode — nothing's behavior changes yet.
tests.ymlruns the selector and writes an advisory "what enforcing would have run" summary, but the full matrix and all jobs still run. Flipping one env knob (ENFORCE_SELECTION) to'true'makes it selective, once audit data shows the skip set is safe.How it works
Two layers, split by who can know a dependency:
ProjectGraph(tools/SelectTests/GraphAffectedProjects.cs), built fromAspire.slnxat the PR head, returns the affected projects (production + test) for the diff. File→project attribution uses evaluatedProjectInstanceitems (resolvedFullPath, so cross-project linked/shared files map to every consumer) plusImportPaths(Directory.Build.*,eng/Versions.props, etc.); the transitive reverseProjectReferenceclosure pulls in dependents. Computed live every run, so it never drifts.docs/ci/test-trigger-map.yml): only what the graph can't see — non-.NET jobs, loose-file reads, a convention backstop, andDirectory.Packages.props → ALL(the graph is HEAD-only and does not diff package versions).The selector unions both, applies
affected_project_rulesandderived_targets, and falls back to the full matrix for unattributedsrc/**files. A kill switch —[full ci]in the PR body or arun-all-testslabel — forces everything. Layer 1 is not optional: if the graph can't be computed, the step fails (under-selecting would silently skip tests).What's in the diff
tools/SelectTests/— the selector, the Layer 1 graph provider, and the curated-map parser.docs/ci/test-trigger-map.yml+ two design docs.Aspire.slnx— addedInfrastructure.TestsandAspire.Hosting.Maui.Tests(both were absent, so Layer 1 could not fan their dependencies into them).tests/Infrastructure.Tests/TestTriggerMap/— a map verifier, selector-engine acceptance tests, and the Layer 1 graph tests..github/workflows/tests.yml— the wiring.Wiring (
tests.yml)setup_for_testsgains aselect_testsstep afterenumerate-tests:ENFORCE_SELECTIONenv defaults to'false'→ selector returns run-all (full matrix + everyrun_* = true); each non-.NET job gates onrun_X == 'true'. Flip to'true'to go selective.base.shais always reachable, so a fetch failure is a real problem. Non-PR events with no base force the full set.extension_e2e_changesregex job;extension_e2e_testsgates on the selector'srun_extension_e2e. Previously-unconditionaltypescript_sdk_tests/typescript_api_compatare gated too.Call-outs
ENFORCE_SELECTIONis flipped. This PR's own CI run is the first audit data point.Microsoft.Buildloaded from the repo-local SDK viaMSBuildLocator(ExcludeAssets=runtime); all packages are on the approved dnceng feeds.SelectTestsandInfrastructure.Teststargetnet10.0(theMicrosoft.Build18.3.x line ships onlynet10.0/net472).ProjectGraphbuild (~8s) to the critical-pathsetup_for_testsjob.affected_project_rules).Validation
Local: 64
TestTriggerMaptests pass (map verifier, selector acceptance, and Layer 1 graph tests), full Infrastructure.Tests suite green (346); selector audit/enforce, git-diff, deleted-file, and hard-fail paths smoke-verified end-to-end. GitHub Actions execution is validated by this PR's run.CI
setup_for_testsoptimizations (folded in)This PR also speeds up the gating
setup_for_testsjob (warm, ALL case ≈ 123s → 103s):./restore.shin the setup job — run a minimal SDK bootstrap + enumerate the matrix from MSBuild evaluation, restoring/building only the class-mode split test projects rather than all 415 projects.[Trait("Partition","N")]from source) instead of compiling the hosting closure just to reflect the traits off the built assembly.actions/setup-dotnet(job-level + the enumerate composite's):global.jsondeclarestools.runtimes, so Arcade always installs the SDK into repo-local.dotnetand never reads a system SDK — the step was a wasted download..dotnet(SDK + runtime toolsets), keyed onglobal.json+eng/Versions.props, with restore-everywhere / save-only-on-main/releaseso PR runs reuse the shared entry but never write a per-PR 363 MB copy (avoids churning the repo's 10 GB Actions cache budget).The canonical matrix is unchanged (176 entries: 78 class / 7 collection / 91 regular).