Skip to content

Commit ab4a6cf

Browse files
radicalCopilot
andcommitted
feat(ci): run test selection before enumerate-tests; enumerate only selected projects
SelectTests ran after enumerate-tests and filtered the already-built full matrix. But enumerate-tests does a full `./build.sh -test` to shard every test project, so the expensive work happened regardless of the selection. Invert the order. SelectTests now runs first (after checkout+restore), computes the affected test projects from the diff (Layer 1 graph + curated map), and in enforce mode writes an OverrideProjectToBuild props file. enumerate-tests then reuses the job's checkout+restore and builds/enumerates ONLY the selected projects via $(BeforeBuildPropsPath) -- the same hook the quarantine/outerloop runners use. In audit mode no props are written and enumerate-tests produces the full matrix unchanged. SelectTests contract: - The test-project universe (ALL-expansion + existence guard) now comes from Aspire.slnx (tests/*.Tests), not an enumerated matrix, since the selector runs before enumeration. - Dropped --matrix/--output; added --before-build-props. Emits a before_build_props output so the workflow passes /p:BeforeBuildPropsPath to enumerate-tests. - run_* job booleans are unchanged and still drive the non-.NET job gates. enumerate-tests action gains checkout/restore/beforeBuildPropsPath inputs; defaults preserve deployment-tests.yml. tests.yml shares one checkout+restore so the props file survives (a fresh checkout's git clean would remove it). Also: - Move the curated map docs/ci/test-trigger-map.yml -> eng/test-trigger-map.yml and update all references. - Ignore **/*.md and the map file itself, so doc/map edits no longer appear as unattributed changed files. - Flip ENFORCE_SELECTION to 'true'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c306ba1 commit ab4a6cf

12 files changed

Lines changed: 395 additions & 213 deletions

File tree

.github/actions/enumerate-tests/action.yml

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,18 @@ inputs:
66
type: string
77
default: ''
88
description: 'Additional MSBuild arguments passed to the test matrix generation step (e.g., /p:IncludeTemplateTests=true /p:OnlyDeploymentTests=true)'
9+
checkout:
10+
required: false
11+
default: 'true'
12+
description: 'Whether to check out the repo. Set false when the caller has already checked out (e.g. a prior select-tests step in the same job whose working tree must be preserved).'
13+
restore:
14+
required: false
15+
default: 'true'
16+
description: 'Whether to set up .NET and run ./restore.sh. Set false when the caller has already restored in the same job.'
17+
beforeBuildPropsPath:
18+
required: false
19+
default: ''
20+
description: 'Path to an OverrideProjectToBuild props file (imported by eng/Build.props via $(BeforeBuildPropsPath)) that restricts the -test build, and thus the enumeration, to a subset of test projects. Empty enumerates everything.'
921

1022
# Output format: JSON with structure {"include": [{...}, ...]}
1123
# Each entry contains:
@@ -31,28 +43,34 @@ runs:
3143
using: "composite"
3244
steps:
3345
- name: Checkout code
46+
if: ${{ inputs.checkout == 'true' }}
3447
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
3548

3649
- name: Set up .NET Core
50+
if: ${{ inputs.restore == 'true' }}
3751
uses: actions/setup-dotnet@c2fa09f4bde5ebb9d1777cf28262a3eb3db3ced7 # v5.2.0
3852
with:
3953
global-json-file: ${{ github.workspace }}/global.json
4054

4155
- name: Restore
56+
if: ${{ inputs.restore == 'true' }}
4257
shell: bash
4358
run: ./restore.sh
4459

4560
- name: Build ExtractTestPartitions tool
4661
shell: bash
4762
run: dotnet build tools/ExtractTestPartitions/ExtractTestPartitions.csproj -c Release --nologo -v quiet
4863

64+
# When beforeBuildPropsPath is set, eng/Build.props imports it and its OverrideProjectToBuild
65+
# items REPLACE the default test set, so the -test build enumerates only the selected projects.
4966
- name: Generate canonical test matrix
5067
shell: bash
5168
run: >
5269
./build.sh -test
5370
/p:TestRunnerName=TestEnumerationRunsheetBuilder
5471
/p:TestMatrixOutputPath=artifacts/canonical-test-matrix.json
5572
/p:GenerateCIPartitions=true
73+
${{ inputs.beforeBuildPropsPath != '' && format('/p:BeforeBuildPropsPath={0}', inputs.beforeBuildPropsPath) || '' }}
5674
/bl
5775
${{ inputs.buildArgs }}
5876

.github/workflows/tests.yml

Lines changed: 55 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -36,24 +36,16 @@ jobs:
3636
steps:
3737
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
3838

39-
- uses: ./.github/actions/enumerate-tests
40-
id: generate_tests_matrix
39+
- name: Set up .NET Core
40+
uses: actions/setup-dotnet@c2fa09f4bde5ebb9d1777cf28262a3eb3db3ced7 # v5.2.0
4141
with:
42-
buildArgs: '/p:IncludeTemplateTests=true /p:IncludeCliE2ETests=${{ github.event_name == ''pull_request'' }}'
42+
global-json-file: ${{ github.workspace }}/global.json
4343

44-
# The expanded matrix can exceed 128KB. It MUST reach the bash step below via a file, not an
45-
# environment variable: a single env string over Linux's MAX_ARG_STRLEN (~128KB) makes the
46-
# step's execve("/usr/bin/bash") fail with E2BIG ("Argument list too long"). pwsh writes its
47-
# run-script to a file, so interpolating the (trusted, build-generated) matrix into a literal
48-
# here-string here keeps it out of the environment entirely.
49-
- name: Write test matrix to file
50-
shell: pwsh
51-
run: |
52-
@'
53-
${{ steps.generate_tests_matrix.outputs.all_tests }}
54-
'@.Trim() | Set-Content -Path "${{ github.workspace }}/all_tests.json" -Encoding utf8 -NoNewline
44+
- name: Restore
45+
shell: bash
46+
run: ./restore.sh
5547

56-
- name: Select relevant tests (audit)
48+
- name: Select relevant tests
5749
id: select_tests
5850
shell: bash
5951
# Untrusted PR fields (body) are passed via env, never interpolated into the script body, to
@@ -63,24 +55,29 @@ jobs:
6355
HAS_RUN_ALL_LABEL: ${{ contains(github.event.pull_request.labels.*.name, 'run-all-tests') }}
6456
PR_BASE_SHA: ${{ github.event.pull_request.base.sha }}
6557
HEAD_SHA: ${{ github.sha }}
66-
# The ONE audit/enforce knob. 'false' (audit): the selector returns run-all -- the full
67-
# matrix + run_* = true, plus the advisory "what enforcing would run" summary. Flip to
68-
# 'true' to enforce (selective matrix + selective run_*) once the audit data looks safe.
69-
ENFORCE_SELECTION: 'false'
58+
# Absolute path so eng/Build.props ($(BeforeBuildPropsPath)) resolves it regardless of cwd.
59+
# Under artifacts/ (gitignored) so it is not picked up as a source change.
60+
BEFORE_BUILD_PROPS: ${{ github.workspace }}/artifacts/BeforeBuildProps.props
61+
# The ONE audit/enforce knob. 'false' (audit): no restriction props are written, so the
62+
# enumerate-tests step below builds the FULL matrix and run_* are all true; the advisory
63+
# summary still reports what enforcing would have skipped. 'true' (enforce): a non-ALL
64+
# selection writes BeforeBuildProps.props so enumerate-tests enumerates ONLY the selected
65+
# projects, and run_* gate the non-.NET jobs.
66+
ENFORCE_SELECTION: 'true'
7067
run: |
7168
set -euo pipefail
7269
73-
args=(--repo-root . --matrix all_tests.json --output selected_matrix.json)
70+
args=(--repo-root . --before-build-props "$BEFORE_BUILD_PROPS")
7471
[ "$ENFORCE_SELECTION" = "true" ] && args+=(--enforce)
7572
7673
# Kill switch: a [full ci] token in the PR body or the run-all-tests label forces everything.
7774
if printf '%s' "$PR_BODY" | grep -qiF '[full ci]' || [ "$HAS_RUN_ALL_LABEL" = "true" ]; then
7875
args+=(--force-all)
7976
elif [ -n "$PR_BASE_SHA" ]; then
80-
# Pull requests diff base..head. The enumerate-tests checkout is shallow, so fetch the
81-
# base commit. If it can't be made available we FAIL rather than fall back to --force-all:
82-
# base.sha is always reachable on origin, so a failure here is a real problem (a bad fetch
83-
# or a rewritten base), and masking it with run-all would teach the audit nothing.
77+
# Pull requests diff base..head. This job's checkout is shallow, so fetch the base commit.
78+
# If it can't be made available we FAIL rather than fall back to --force-all: base.sha is
79+
# always reachable on origin, so a failure here is a real problem (a bad fetch or a
80+
# rewritten base), and masking it with run-all would teach the audit nothing.
8481
if ! git fetch --no-tags --depth=1 origin "$PR_BASE_SHA"; then
8582
echo "::error::Failed to fetch PR base commit $PR_BASE_SHA; cannot compute the changed-file diff." >&2
8683
exit 1
@@ -95,22 +92,46 @@ jobs:
9592
args+=(--force-all)
9693
fi
9794
98-
# SelectTests computes the Layer 1 affected-projects graph in-process from Aspire.slnx
99-
# (Microsoft.Build, loaded from the repo-local SDK via MSBuildLocator -- no extra tool to
100-
# restore). It writes selected_matrix.json (full in audit, filtered in enforce), the run_*
101-
# job booleans (to $GITHUB_OUTPUT), and the summary. The tool selects the full matrix *by
102-
# design* when it is told to (--force-all from the kill switch, or a non-PR event with no
103-
# base), so a non-zero exit here means the selector itself crashed (incl. a Layer 1 graph
104-
# failure, which is fatal -- under-selecting would silently skip tests). Let that fail the
105-
# step (and the run) rather than masking the bug behind a silent fallback; surfacing it is
106-
# the whole point of the audit phase.
95+
# SelectTests runs BEFORE enumerate-tests. It computes the Layer 1 affected-projects graph
96+
# in-process from Aspire.slnx (Microsoft.Build via MSBuildLocator -- needs the restore
97+
# above) and, when enforcing a non-ALL selection, writes the OverrideProjectToBuild props at
98+
# $BEFORE_BUILD_PROPS (consumed by the enumerate-tests step's beforeBuildPropsPath). It also
99+
# writes the run_* job booleans and before_build_props (to $GITHUB_OUTPUT) and the summary.
100+
# The tool selects ALL *by design* when told to (--force-all from the kill switch, or a
101+
# non-PR event with no base), so a non-zero exit means the selector itself crashed (incl. a
102+
# fatal Layer 1 graph failure -- under-selecting would silently skip tests). Let that fail
103+
# the run rather than mask the bug behind a silent fallback.
107104
./dotnet.sh run --project tools/SelectTests/SelectTests.csproj -- "${args[@]}"
108105
106+
# enumerate-tests reuses this job's checkout + restore (checkout/restore=false) so the props
107+
# file the selector wrote survives -- a fresh checkout would git-clean it away. When a
108+
# restriction was written, beforeBuildPropsPath points enumerate at the selected subset; in
109+
# audit mode before_build_props is empty and enumerate builds the full matrix.
110+
- uses: ./.github/actions/enumerate-tests
111+
id: generate_tests_matrix
112+
with:
113+
buildArgs: '/p:IncludeTemplateTests=true /p:IncludeCliE2ETests=${{ github.event_name == ''pull_request'' }}'
114+
checkout: 'false'
115+
restore: 'false'
116+
beforeBuildPropsPath: ${{ steps.select_tests.outputs.before_build_props }}
117+
118+
# The expanded matrix can exceed 128KB. It MUST reach the bash step below via a file, not an
119+
# environment variable: a single env string over Linux's MAX_ARG_STRLEN (~128KB) makes the
120+
# step's execve("/usr/bin/bash") fail with E2BIG ("Argument list too long"). pwsh writes its
121+
# run-script to a file, so interpolating the (trusted, build-generated) matrix into a literal
122+
# here-string here keeps it out of the environment entirely.
123+
- name: Write test matrix to file
124+
shell: pwsh
125+
run: |
126+
@'
127+
${{ steps.generate_tests_matrix.outputs.all_tests }}
128+
'@.Trim() | Set-Content -Path "${{ github.workspace }}/all_tests.json" -Encoding utf8 -NoNewline
129+
109130
- name: Split matrix by dependency type
110131
id: split_matrix
111132
shell: pwsh
112133
run: |
113-
$matrix = Get-Content -Raw "${{ github.workspace }}/selected_matrix.json"
134+
$matrix = Get-Content -Raw "${{ github.workspace }}/all_tests.json"
114135
$splitScript = "${{ github.workspace }}/eng/scripts/split-test-matrix-by-deps.ps1"
115136
& $splitScript -AllTestsMatrix $matrix -OutputToGitHubEnv
116137

docs/ci/test-trigger-map.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ A map of **repo path → CI targets that must run** when a matching file changes
44
covering the .NET test projects and the validation/polyglot jobs in
55
[`tests.yml`](../../.github/workflows/tests.yml).
66

7-
The machine-readable form lives next to this doc:
8-
[`test-trigger-map.yml`](./test-trigger-map.yml). The tool that consumes it and
7+
The machine-readable form lives at
8+
[`eng/test-trigger-map.yml`](../../eng/test-trigger-map.yml). The tool that consumes it and
99
the rollout plan are in
1010
[`test-trigger-selector-design.md`](./test-trigger-selector-design.md).
1111

docs/ci/test-trigger-selector-design.md

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,15 @@ matrix.
77
Companion documents:
88

99
- [`test-trigger-map.md`](./test-trigger-map.md) — the descriptive path → target map.
10-
- [`test-trigger-map.yml`](./test-trigger-map.yml) — its machine-readable form.
10+
- [`eng/test-trigger-map.yml`](../../eng/test-trigger-map.yml) — its machine-readable form.
1111

12-
**Status: wired in audit mode.** `tests.yml`'s `setup_for_tests` runs
13-
`SelectTests` and emits the advisory summary, but it runs in audit mode
14-
(`ENFORCE_SELECTION: 'false'`), so the selector returns run-all and the full
15-
matrix and all jobs still run.
12+
**Status: enforcing.** `tests.yml`'s `setup_for_tests` runs `SelectTests`
13+
*before* `enumerate-tests`. When `ENFORCE_SELECTION: 'true'` and the selection is
14+
not ALL, the selector writes an `OverrideProjectToBuild` props file so
15+
`enumerate-tests` builds and enumerates only the selected projects; in audit mode
16+
(`ENFORCE_SELECTION: 'false'`) it writes no props and `enumerate-tests` produces
17+
the full matrix unchanged while the summary still reports what enforcing would
18+
have skipped.
1619

1720
Audit mode does not soften Layer 1 failures. If the affected-projects graph
1821
cannot be computed, `SelectTests` fails the step because under-selecting would
@@ -25,7 +28,9 @@ Input: the list of files changed in a PR. Output:
2528
- the subset of `test:<Project>` entries to run, and
2629
- which non-.NET jobs to trigger (`job:polyglot`, `job:extension-e2e`, …).
2730

28-
Filter the existing full matrix down — do **not** rebuild it.
31+
Select *before* enumeration: pick the affected test projects, then have
32+
`enumerate-tests` build/shard only those — do **not** enumerate the full matrix
33+
and filter it after.
2934

3035
## Why not just consume `test-trigger-map.yml`?
3136

@@ -192,22 +197,28 @@ These are sourced from the corresponding sections of `test-trigger-map.yml`.
192197

193198
## The tool (`tools/SelectTests`)
194199

195-
`SelectTests` is a small C# console tool, run after `enumerate-tests` and before
196-
the matrix split.
200+
`SelectTests` is a small C# console tool, run *before* `enumerate-tests`. It
201+
decides which test projects are affected; `enumerate-tests` then builds and
202+
shards only those.
197203

198204
Main options:
199205

200206
- `--repo-root`: repository root, defaulting to the current directory.
201-
- `--map`: curated map path, defaulting to `docs/ci/test-trigger-map.yml`.
202-
- `--matrix`: the `enumerate-tests` all-tests JSON.
207+
- `--map`: curated map path, defaulting to `eng/test-trigger-map.yml`.
203208
- `--from` / `--to`: git refs for the PR diff.
204209
- `--changed-files`: newline-delimited changed file list, instead of
205210
`--from` / `--to`.
206211
- `--skip-layer1`: skip the graph closure for explicit diagnostics.
207-
- `--force-all`: kill switch; force the full matrix.
208-
- `--enforce`: emit the filtered matrix. Without this, audit mode emits the full
209-
matrix.
210-
- `--output`: matrix output path, defaulting to stdout.
212+
- `--force-all`: kill switch; force ALL.
213+
- `--enforce`: write the restriction props for a non-ALL selection. Without this
214+
(audit), no props are written and `enumerate-tests` runs the full matrix.
215+
- `--before-build-props`: path for the `OverrideProjectToBuild` props file
216+
(consumed by `enumerate-tests` via `BeforeBuildPropsPath`).
217+
218+
The test-project universe (the set an `ALL` selection expands to, and the
219+
existence guard) is the `tests/<Name>/<Name>.csproj` projects ending in `.Tests`
220+
in `Aspire.slnx` — derived directly from the slnx because the selector runs
221+
before any matrix exists.
211222

212223
Flow:
213224

@@ -220,48 +231,52 @@ Flow:
220231
5. Apply `derived_targets` to a cycle-safe fixpoint.
221232
6. Escalate to `ALL` for a kill switch, an `ALL` path rule, or an unattributed
222233
`src/**` file that is not under a project directory in `Aspire.slnx`.
223-
7. Emit the filtered matrix and per-job booleans in enforce mode, or run-all
224-
outputs in audit mode.
234+
7. Emit the per-job booleans, and — in enforce mode for a non-ALL selection —
235+
the `OverrideProjectToBuild` props restricting the downstream build.
225236

226237
Selection only decides *which* projects survive. OS expansion, timeouts,
227238
`requiresNugets` / `requiresCliArchive` flags, and the matrix split stay owned
228-
by the existing scripts.
239+
by the existing scripts (downstream of `enumerate-tests`).
229240

230241
## Pipeline integration
231242

232243
The flow in `tests.yml`'s `setup_for_tests` job:
233244

234245
```text
235-
enumerate-tests (action)
236-
-> all_tests JSON {"include":[...]}
237-
-> SelectTests (--from base --to head; reads all_tests + curated map;
238-
computes Layer 1 in process)
239-
-> selected_matrix.json + run_* outputs + audit summary
246+
checkout -> restore
247+
-> SelectTests (--from base --to head; curated map + Layer 1 in process)
248+
-> run_* outputs + summary
249+
-> (enforce && !ALL) BeforeBuildProps.props (OverrideProjectToBuild)
250+
-> enumerate-tests (action; checkout/restore reused; beforeBuildPropsPath)
251+
-> all_tests JSON {"include":[...]} (only the selected projects in enforce)
240252
-> split-test-matrix-by-deps.ps1
241253
-> run-tests.yml (per-dependency matrices)
242254
```
243255

244-
`SelectTests` runs as one step after `enumerate-tests` and before the split. The
245-
split, per-OS/per-dependency bucketing, and `run-tests.yml` are unchanged.
256+
`SelectTests` runs first; `enumerate-tests` reuses the job's checkout+restore
257+
(`checkout: 'false'`, `restore: 'false'`) so the props file survives — a fresh
258+
checkout's `git clean` would otherwise remove it. The split, per-OS/per-dependency
259+
bucketing, and `run-tests.yml` are unchanged.
246260

247261
The `run_*` step outputs become `setup_for_tests` job outputs that gate every
248262
non-.NET job, such as `polyglot_validation`, `typescript_sdk_tests`,
249263
`typescript_api_compat`, extension jobs, `cli_starter_validation_windows`, and
250264
the WinGet/Homebrew installer-prepare jobs.
251265

252266
The .NET test jobs need no `run_*` gate: they are already gated by their matrix
253-
bucket being empty once `SelectTests` filters the matrix. Base builds stay
254-
ungated because they are upstream `needs:` that run whenever a dependent runs.
267+
bucket being empty once `enumerate-tests` produces only the selected projects.
268+
Base builds stay ungated because they are upstream `needs:` that run whenever a
269+
dependent runs.
255270

256271
The extension-unit jobs (`extension_tests_win` / `extension_bootstrap_linux`)
257272
gate on `run_extension_unit` **or** `run_extension_e2e`, because
258273
`extension_e2e_tests` needs them. Gating them off while e2e runs would skip e2e
259274
via need-propagation.
260275

261276
**Audit vs. enforce is a single knob in the `select_tests` step:
262-
`ENFORCE_SELECTION`.** Audit (`'false'`, no `--enforce`) returns the full matrix
263-
plus `run_* = true`, with the advisory summary showing what enforcing would
264-
select.
277+
`ENFORCE_SELECTION`.** Audit (`'false'`, no `--enforce`) writes no restriction
278+
props, so `enumerate-tests` builds the full matrix and `run_*` are all true, with
279+
the advisory summary showing what enforcing would select.
265280

266281
Flipping `ENFORCE_SELECTION` to `'true'` makes the same selector return the
267282
selective matrix and selective `run_*` outputs. The downstream gates do not need
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ ignore:
8080
- src/Components/Common/** # link-compiled into many components; Layer 1 covers
8181
- src/Vendoring/OpenTelemetry.Instrumentation.*/** # glob-compiled into the Redis/Kafka components; Layer 1 covers
8282
- src/Vendoring/OpenTelemetry.Shared/** # compiled by nothing (each instrumentation dir has its own Shared/); inert
83+
- "**/*.md" # documentation; never gates a test (Layer 1 still owns any README it actually packs/builds)
84+
- eng/test-trigger-map.yml # this map itself; consumed at selection time, it changes no test code
8385

8486
# Path rules: a glob set -> a target set. One mechanism; the comment headers below are organization
8587
# only. targets may be test: / job: / a GROUP name / ALL.

tests/Infrastructure.Tests/TestTriggerMap/SelectTestsAcceptanceTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ namespace Infrastructure.Tests.TestTriggerMap;
1111
/// small SYNTHETIC maps (a temp <c>map.yml</c> + a fake matrix + fake project dirs), so they assert
1212
/// the resolution mechanisms — conventions, overrides, ignore, Layer-1 attribution, the run-all
1313
/// fallback, derived targets, and group expansion — without coupling to the contents of the real
14-
/// <c>docs/ci/test-trigger-map.yml</c>. A thin set of real-map invariant smokes (computed from the
14+
/// <c>eng/test-trigger-map.yml</c>. A thin set of real-map invariant smokes (computed from the
1515
/// filesystem, never hardcoding project names) lives at the end; structural invariants of the real
1616
/// map are covered by <see cref="TestTriggerMapTests"/>.
1717
/// </summary>
@@ -606,7 +606,7 @@ public void UnmatchedAffectedProjectAddsNothing()
606606
[Fact]
607607
public void RealMapLoadsAndConventionSelectsAComponentsTestWithoutSelectingAll()
608608
{
609-
var mapPath = Path.Combine(RepoRoot.Path, "docs", "ci", "test-trigger-map.yml");
609+
var mapPath = Path.Combine(RepoRoot.Path, "eng", "test-trigger-map.yml");
610610
var matrix = EnumerateMatrixTestProjects();
611611
var projectDirs = LoadProjectDirectories();
612612
var selector = new TestSelector(mapPath, matrix, projectDirs);

0 commit comments

Comments
 (0)