Skip to content

Fix: affected-test selection missed tests on multi-targeted projects - #50

Merged
csa7mdm merged 3 commits into
mainfrom
fix/tfm-variant-seeding
Sep 25, 2026
Merged

csa7mdm merged 3 commits into
mainfrom
fix/tfm-variant-seeding

Conversation

@csa7mdm

@csa7mdm csa7mdm commented Sep 25, 2026

Copy link
Copy Markdown
Owner

Bug

dotnet_test_affected could miss tests that call the changed code directly while still reporting selectionComplete: true. The result also varied between sessions.

Found by: the Wave 0 benchmark gate. On Polly commit 8ba491d7, a change to RandomUtil.cs:

  • selected 121 tests in one session and 118 in another;
  • the 118-test runs missed RandomUtilTests.Next_Ok and NextDouble_Ok;
  • fresh sessions on main gave 118 every time.

Cause:

  1. A multi-targeted project loads once per target framework, so a changed file exists as several documents.
  2. The walk seeded from the first in-scope document id and deduplicated visited members by documentation id.
  3. The search from one framework copy's symbol doesn't reliably reach the dependents of the other copies. Roslyn links copies by source position, and #if branches put the same member on different lines.
  4. Which copy came first wasn't stable, hence the session-to-session flip.

Fix

  • Seed from every in-scope copy of each changed file.
  • Track visited members per (project copy, documentation id). Tests are still reported once.

Evidence

Unit test: a new test reproduces Polly's shape: an internal class with #if NET / #else, one test reaching each framework copy.

  • On main it fails in both orders (each finds only its own copy's test).
  • With the fix it passes both.

Replay: Polly's last 40 source commits, sequential, maxSelectionSeconds: 120, compared by test name on the 22 commits complete in all runs.

Build Lost vs main Gained vs main Median / p90 selection Filtered runs
main - - 0.81 s / 2.24 s 17
this PR 0 14 (commit 708b02b0) 2.82 s / 6.05 s 16
rejected variant: per-copy keys only for #if files 32, 62 and 56 tests on 3 commits 14 1.17 s / 2.77 s 16

The rejected variant is why this PR keys every member per copy. The faster version wasn't safe. RandomUtil.cs now selects 121 tests on every run (6 of 6 in a fresh session).

Cost: selection is about 3× slower on multi-targeted solutions. A few more broad changes will fall back to whole test projects within the default 10 s budget. Correctness first; the persistent index planned for Wave 2 addresses speed.

Tests

166 in the solution pass (Testing 64, including the 2 new cases).

🤖 Generated with Claude Code

csa7mdm and others added 3 commits September 25, 2026 04:24
Roslyn links a symbol to its copies in other TFM variants of the same file
by source position. #if branches put the same member on different lines
(Polly's RandomUtil.cs), so the link breaks, and a walk seeded from one
variant finds only that variant's dependents. The walk seeded from the
first in-scope document id, whose order isn't stable: Polly commit
8ba491d7 selected 121 tests in one session and 118 in another, missing
RandomUtilTests.Next_Ok while reporting selectionComplete.

The walk now seeds from every in-scope variant and tracks visited
members per (project variant, documentation id). Tests are still
reported once.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A narrower variant (per-variant keys only for members of files with #if)
was tried and rejected: it lost 32-62 tests on 3 of Polly's last 40
commits. Keying every member per variant lost none.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@csa7mdm
csa7mdm merged commit 5c5a302 into main Sep 25, 2026
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant