Skip to content

Group the selective-CI test-selector comment by trigger and lead with what runs - #18368

Closed
radical wants to merge 2 commits into
microsoft:mainfrom
radical:select-tests-comment-grouping
Closed

Group the selective-CI test-selector comment by trigger and lead with what runs#18368
radical wants to merge 2 commits into
microsoft:mainfrom
radical:select-tests-comment-grouping

Conversation

@radical

@radical radical commented Jun 20, 2026

Copy link
Copy Markdown
Member

What this changes

The Tests selector PR comment posted by tools/SelectTests (the
selective-CI advisory comment) was hard to read. It listed every selected
test project on its own line with the selection reason appended, so when one
change fans out across the project graph the same reason repeats on dozens
of lines
. A reviewer couldn't tell at a glance which projects and jobs run,
or which change drove the bulk of the selection.

Example — the old comment on #18332 had 46 near-identical lines like:

- `Aspire.Hosting.Azure.Kusto.Tests` — via graph from `tests/Aspire.Hosting.Tests/Dashboard/DashboardServiceHostTests.cs`
- `Aspire.Hosting.Blazor.Tests` — via graph from `tests/Aspire.Hosting.Tests/Dashboard/DashboardServiceHostTests.cs`
- … 30 more identical reasons …

The fix

Restructure the comment into what runs, then how it was chosen:

  • Lead with what runs — a flat list of selected jobs, then a flat list of
    selected test projects. Full impact is visible up front, even with many
    changed files.
  • Then how it was chosen — group the selected projects under each trigger
    (a changed file and its graph fan-out under one heading, an affected
    project, or a derived test). Each reason is stated once instead of per
    project; large fan-outs collapse into <details>; a per-job table names
    what triggered each job. A headline ⚠️ call-out names the single change
    driving a large share of a big selection.

Rendered on #18332's inputs, the 46 projects collapse to a handful of
trigger groups, with the headline surfacing that 33 of 46 come from one
changed test file
.

Call-outs

  • Full attribution is preserved — a project selected by several triggers
    appears under each group, so no "why" is dropped (the old "list every
    cause" guarantee still holds, just inverted).
  • Updated the CLI and Layer 1 integration tests to assert the new structure,
    and docs/ci/test-trigger-selector-design.md to describe it.
  • No change to the selection logic, the job booleans, the step-summary audit
    detail, or the one-comment-per-commit posting behavior — only the comment's
    body layout.

The "Tests selector" PR comment posted by tools/SelectTests listed every
selected test project on its own line with its selection reason appended.
When one change fans out across the project graph, the same reason repeats
on dozens of lines, so a reviewer can't tell at a glance which projects and
jobs run, or which change drove the bulk of the selection.

Restructure the comment:

- Lead with WHAT runs: a flat list of selected jobs, then a flat list of
  selected test projects, so the full impact is visible up front even when
  many files changed.
- Follow with HOW it was chosen: group the selected projects under each
  trigger (a changed file and its graph fan-out under one heading, an
  affected project, or a derived test), stating each reason once instead of
  per project, collapsing large fan-outs into <details>, plus a per-job
  reasons table. A headline call-out names the single change driving a large
  share of a big selection.

Full attribution is preserved -- a project selected by several triggers
appears under each -- so no "why" is lost. Update the CLI and Layer 1
integration tests to assert the new structure, and the design doc to
describe it.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 20, 2026 06:59
@github-actions

github-actions Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://github.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18368

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18368"

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR restructures the Tests selector advisory PR comment produced by tools/SelectTests (the selective-CI tool). The old comment listed each selected test project on its own line with the selection reason appended, so a single change fanning out across the project graph repeated the same reason on dozens of lines. The new layout leads with what runs (flat lists of selected jobs and projects) and then explains how it was chosen by inverting the per-project causes into per-trigger groups, collapsing large fan-outs into <details>, adding a per-job reasons table, and surfacing a ⚠️ headline when one trigger drives a large share of a big selection. There is no change to selection logic, job booleans, the step-summary, or the one-comment-per-commit posting behavior — only the comment body layout.

Changes:

  • Rewrote WriteSelectionComment and added grouping/rendering helpers (AppendSelectionRationale, CauseGroupKey/Header/Descriptor, RenderProjectList, MemberWithHops, JobCausesText, StripJobPrefix, WriteCommentFile); removed the now-unused AllCausesSuffix.
  • Updated the CLI and Layer 1 integration tests to assert the new comment structure.
  • Updated docs/ci/test-trigger-selector-design.md to describe the grouped, trigger-first layout.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
tools/SelectTests/Program.cs Core rewrite of the PR comment renderer: flat "what runs" lists, per-trigger grouping with direct/graph/other buckets, headline call-out, and a job-reasons table.
tests/Infrastructure.Tests/TestTriggerMap/SelectTestsCliTests.cs Updated assertions to match the new headings and multi-trigger grouping; comments revised accordingly.
tests/Infrastructure.Tests/TestTriggerMap/SelectTestsLayer1IntegrationTests.cs Updated graph-fan-out assertions to the new "via the project graph" grouping output.
docs/ci/test-trigger-selector-design.md Documentation updated to describe the trigger-first, grouped comment layout.

Notes from the review (both nit-level): the other bucket uses a fixed "tests" label that reads as "1 tests" for a single project (inconsistent with the careful pluralization elsewhere in the method), and the new headline call-out / <details> collapse thresholds are not covered by any test.

Comment thread tools/SelectTests/Program.cs Outdated
Comment thread tools/SelectTests/Program.cs
@github-actions

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@github-actions

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

- Pluralize the affected/derived ("other") bucket label so a single-project
  group reads "1 test" instead of "1 tests", matching the existing job/file
  pluralization.
- Add LargeFanOutEmitsHeadlineAndCollapsesProjectList: a 14-project fan-out
  from one changed file trips both the ⚠️ headline threshold (tests >= 10 &&
  largest group >= 5) and the <details> collapse (inline limit 12), so neither
  can regress silently.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@radical

radical commented Jun 20, 2026

Copy link
Copy Markdown
Member Author

Superseded by #18376, which is opened from a branch in this repo so the Tests selector selection comment can actually post (a fork PR runs the pull_request workflow with a read-only GITHUB_TOKEN, so it cannot comment on the base repo). Same change, plus fork-aware handling that surfaces the selection in the job step summary when posting is not permitted. Closing this one.

@radical radical closed this Jun 20, 2026
@microsoft-github-policy-service microsoft-github-policy-service Bot added this to the 13.5 milestone Jun 20, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 21, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants