Skip to content

CI test selection: symbol cap as a share, migration names, replay counts, local-filter, and Guard on repo-walking tests (#5798) - #5801

Merged
erikdarlingdata merged 2 commits into
devfrom
fix/5798-test-selection-by-symbol
Oct 14, 2026
Merged

erikdarlingdata merged 2 commits into
devfrom
fix/5798-test-selection-by-symbol

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Oct 14, 2026 •

Copy link
Copy Markdown
Owner

Claude posting for Erik Darling.

Fixes #5798.

Why

Pull requests went red at their first CI run on test classes the lane never ran locally. On dev at d85564e, #5776 had already added the changed-symbol reason to map-select and the local-classes verb. This PR finishes the issue's remaining parts: the symbol bound is now a share of the tree, migrations count as symbols, the replay measures the rule, a lane gets the filter CI would use, and repo-walking test classes join the guard set.

What changes

  1. Symbol selection (.github/scripts/ci-select.py). The rule still only adds classes to the file-map selection (a union, never a replacement). Changes:
    • The token bound is now a share of the test files: SYMBOL_CAP_SHARE = 0.01, with the old SYMBOL_CAP = 25 as its floor. That is 30 of today's 3,023 test files. A token named in more test files than that is dropped as noisy. Minimum token length stays 4, and only declarations count.
    • Store migrations are symbols now. private const string V183Sql and new Migration(183, ...) both declare V183, which is how the tests name a migration.
    • In-lane fix: a line like new Migration(183, ...) was no longer read as declaring Migration. Reverted in review round 1 (H1): it took tokens dev gives.
    • Small refactor: symbol_classes() can diff a given head commit against its merge base, so the replay can call it, and a new named_classes() replaces two copies of the same loop.
  2. Replay measure. --replay --map FILE --symbols BASE reports how many classes each corpus row selects before and after the rule. A miss is still counted on the selection without the rule, so the rule cannot hide one. The plain replay is unchanged: 0 misses. Numbers with the 2026-10-14 nightly map (414 rows; the other 317 head commits are not in this clone; 3,826 classes):
    • The rule on its own names a median of 51 classes, at most 700 (18% of the tree).
    • On the 238 rows whose map answer is not FULL, it adds a median of 1 class, at most 202 (before: median 2,054.5, max 3,017; after: median 2,060.5, max 3,017).
    • Across all rows (FULL counted as the whole tree): before median 2,682, max 3,826; after median 2,687.5, max 3,826.
    • That is well under a third of the tree, so the 1% cap stays.
  3. Evidence. For each PR, the first CI run with test failures. Draft runs skip CI, so these are the runs after each PR was readied, found by grepping each run's failed log for [FAIL]. My class counts differ from the 9/7/22 in the cost note, which may have counted methods or later runs.
    • Alerts: a Client Timeouts alert from attention events, with an always-on capture, for Lite and Darling (#5640) #5714 (run 38356036720, 6 classes): the rule selects StatementCutSourceScanTests, DefaultTraceEventsCollectorDefinitionTests and XeSessionPermissionLogLevelTests. DarlingWebEndpointsTests, WebPlainTextTests and XeTimeFilterSweepTests were already Stage=Guard. All 6 are now in the local set, and StatementCutSourceScanTests is also tagged by item 4.
    • Darling records the collection slots it stepped over (#5735) #5741 (run 38350003642, 4 classes): the rule selects CollectionSweepCommandTimeoutTests (also tagged by item 4), DarlingPeerDisclosureTests and FleetGateStatsTests. Missed: SkipCreditFloorTests. The token that links it is named in more test files than the cap allows (it is selected at a cap of 90). Item 4 does not cover it; the file map does. The PR's next red run (38426281271) was LiteReadsTraitGuardTests, already Guard.
    • [BUG] Web UI audit on 3.9.0-nightly.20261006.515: 22 defects (blocking counts, CPU duplicates, FinOps, charts, UI wording) #5644, through PR Web UI audit fixes (#5644) #5705 (run 38345781037, 15 classes): the rule selects 10 (AlertNotebookRenderClientTests, DailySummaryAzureMasterScopeLiveTests, EventWindowedReadsAreBoundedLivePostgresTests, FinOpsDatabaseResourcesGoldenLiveTests, FinOpsTabIndexAnalysisPageTests, FinOpsTabUtilizationRawPageTests, PgIndexBloatCoverageTests, ProductiveZeroBandingTests, ServerPageTabsTests, WebDataStartNoteTests). CrossAppGuardCiGateTests is now Guard through item 4. Missed: AlertHistoryRangePageTests, ViewerDailySummaryAzureMasterScopeLiveTests, WebServerTrendsBehaviourTests and WebWaitsActivityTrendsBehaviourTests. These run a page or a view and name no declaration the change edited (the changed lines were in web assets or in SQL text inside strings). They do not walk the tree, so item 4 does not cover them either.
    • In all three cases the CI map selection was already FULL or near FULL (2,864 of about 3,000 classes for Darling records the collection slots it stepped over (#5735) #5741), so CI did run these classes. What failed is the lane's local set, and local-filter (item 5) is what fixes that.
  4. Guard trait on repo walkers. The scan looked for untagged test classes outside Census/Ratchet names that recursively enumerate a repo path (EnumerateFiles/GetFiles/GetFileSystemEntries with AllDirectories, from a RepoRoot/RepoFile/ParitySource root). It skipped any class that needs a store, skips, or sits in a collection, any file with several classes, and anything tagged Cost=Slow. Results:
    • 47 classes are tagged (48 before review round 1 took McpReadCommandTimeoutTests off), each at class level.
    • Left untagged:
    • CI job: build.yml's guard-tests job (windows-latest, -trait Stage=Guard -failSkips, both suites) now runs these classes, and the shards leave them out when they used the guard build.
    • Guard job time: the tagged classes add about 265 s of serial class time (nightly map). xunit runs classes in parallel: locally, after the change, the Darling guard pass took 86 s (4,602 tests) and the Lite pass 28 s (1,278 tests). I did not measure a baseline before the change. The shards lose the same classes, so total CI time is about unchanged.
    • Darling (Darling/Darling.Tests/): AlertHistoryRowSeverityTests, CollectionSweepCommandTimeoutTests, ControlPlaneReloadDurabilityTests, CSharpSourceWalkerTests, CurrentDatabaseScopeLoggerThreadingTests, DarlingWebSecurityHeadersTests, DeltaGapPolicyTests, FactSourceRegistryTests, FailedSendRetryTrackerClockStepTests, FailureNoteRedactionTests, FleetCardPostgresCpuTests, McpReadCommandTimeoutTests, PeakTimeTieBreakTests, PgServerConfigScopeRungTests, QueryStoreIntervalLatestRungTests, QueryStoreIntervalWideHistoryNoteTests, QueryStoreTouchGuardSingleSourceTests, RdsEndpointVerifierTests (RdsEndpointVerifierTests.Login.cs), RepoFileResolutionEquivalenceTests, RollupCoverageRoutingTests, SerialLoopStoreSizeSourceTests, StatementCutSourceScanTests, StatementFilterPlanReaderSourceScanTests, StatementScrubViewerPlanWithheldTests, StatementScrubViewerSavePathsTests, UnmeasuredMetricsAreNotHealthyTests, Viewer4577Tests, Viewer4627EdgeCallerTests, ViewerChartUtcFrameTests, ViewerCommandTimeoutTests, ViewerServerSetSyncTests, ViewerSidebarDotRendersTheCardStatusTests, ViewerThemeColorOverridesTests, XamlGridRowRangeTests (each in <Class>.cs).
    • Lite (Lite.Tests/): AlertDeliveryChannelTests, ArchivableTableBareReadSweepTests, CrossAppGuardCiGateTests, EntraDeviceCodeTests, LiteRuntimePrerequisiteDocsTests, ParameterSensitivityFiringSignatureParityTests, PlanViewer4632FallbackTests, QueryHighDopStaleMaxDopParityTests, ScreenReaderNamesTests, StatementScrubLivePlanDisplayTests, StatementScrubLiveReadTests, StoreBusyReportTests, StoredEventCopiesSweepTests, TabLoadsOffUiThreadTests (each in <Class>.cs).
  5. local-filter verb. python .github/scripts/ci-select.py local-filter [--base origin/dev] [--map test-map.json.gz] prints, per suite, the filter (40 classes a line since review round 1) (lite: -class "*.A" -class "*.B") and the class list on the next (lite classes: A B). The last line is the guard set (guard: -trait Stage=Guard).
    • With --map it gives map-select's own answer. Without a map, or when that answer is FULL or over 300 classes, it gives the part a lane can run: the classes that name a changed symbol, plus the test classes in changed test files. A note on stderr says which.
    • Only classes that hold a [Fact]/[Theory] are listed (helper classes are left out), and Stage=Guard classes are left to the guard line.
    • It is documented in the module docstring, next to the other verbs.
    • Decision: the brief asked for a dotnet test --filter expression, but these suites are xunit.v3 MTP executables, where dotnet test does not work. So the filter is printed in the form the executables and CI's shards take (-class/-trait).
  6. Tests (Lite.Tests/CiSelectLocalClassesGuardTests.cs):
    • SymbolExtractor_ReadsTheDeclarationsOfCSharpAndSqlHunks: on a fixture diff with C# and SQL hunks, changed_symbols returns exactly CountWidgetSpans, PgMigrations, V184, V184Sql, v_widget_rollup and widget_spans. It skips a call, a parameter and a test-only name. It failed on the old code (Migration was taken as a declared name) before the fix above.
    • LocalFilter_PrintsTheClassFilter_ThenTheClassList_ThenTheGuardSet: the class line, the list line and the guard line; a helper class and a Guard class are not listed.

Test plan

  • Built Lite.Tests and Darling.Tests: 0 errors. The only warnings are the existing NU1902/NU1903 package advisories.
  • Lite.Tests.exe -class CiSelectLocalClassesGuardTests, CiSelectReplayGuardTests, CiSelectMapSelectGuardTests, TestMapShadowGuardTests, CrossAppGuardCiGateTests: Total 85, Failed 0.
  • Lite.Tests.exe -trait Stage=Guard -failSkips: Total 1,278, Failed 0, 27.8 s.
  • Darling.Tests.exe -trait Stage=Guard -failSkips (includes every tagged class and GuardStageCensusTests): Total 4,602, Failed 0, 86.5 s. A first run failed GuardStageCensusTests R3 on three tagged classes; I removed their tags (listed above).
  • Darling.Tests.exe -class SlowClassCensusTests, GuardStageWorkflowTests: Total 23, Failed 0.
  • ci-select.py --replay: 0 misses. With --map (2026-10-14 nightly) and --symbols origin/dev: map replay 0 misses, plus the numbers in item 2.
  • Guard job time measured on CI before and after (local numbers only).
  • Full suites (laptop rule; CI decides).

Leftovers

Review round 1

Commit d6ea302. One rule above the rest: the selection is a union with dev's and never smaller, and the replay below shows that.

  • H1, tokens only grow. _NEW_EXPRESSION is gone. On dev, a line that starts new X( gives the token X; the exclusion took that token away, so on 10 corpus heads the symbol rule named 66 fewer classes than dev (lost tokens: Migration 26 heads, HostMemory 5, NpgsqlConnectionStringBuilder 3, CollectorColumn 2, CollectorParameter 2, PgExtensionDependency, AppLoggerAdapter, ...). It also dropped a real member, new int[] Widgets => _w;. _MIGRATION now adds the V-number on top of what the other rules take, and the comment says that a rule there may only add tokens. The fixture now expects Migration (the cap drops it as noisy: 48 test files name it) and Widgets.
  • M2, local-filter's size. A suite's filter is printed 40 classes a line through _class_args, as local-classes prints it; 700 classes on one line came near Windows' 32,767-character command line (CI: the sharded test steps put every class on one command line, which is at the Windows 32,767-character limit #5100). When it falls back (no map, map FULL, or over 300 classes), the answer is cut at limit (300), most specific first: a changed test file's classes, then the text-pattern classes (I7), then the classes that name a changed symbol in ordered_hits order. A note on stderr gives the count it cut from.
  • M3. McpReadCommandTimeoutTests loses its Stage=Guard tag: 49.882 s in the nightly map, so it stays on the shards with the other slow classes. 47 classes are tagged now, not 48. GuardStageCensusTests passes with it untagged: its R2 rule does not catch this class.
  • L4. New SymbolCap_IsOnePercentOfTheTestFiles_NeverUnder25 asserts symbol_cap gives 30 for 3,023 test files and 25 for 100. The extractor fixture gains new Migration(185, "x", Build()), which pins V185 through the new Migration(N, branch alone.
  • L5. The _MIGRATION comment says \bV(\d+)Sql\b also fires on a reference (sb.Append(V183Sql)), and that this is wanted. The fixture pins it: that line gives V183.
  • L6. The same comment names the known limit: an edit inside an existing migration's verbatim SQL body gives no token. (It sits in the comment above _MIGRATION, next to the rule it limits.)
  • I7. With --map, when local-filter falls back, it adds the classes whose text pattern in the map matches a changed file: the "text pattern" reason of map_select, now in text_pattern_classes(). It reads the map directly, so it works when the map answer is FULL too, and it is still cut at limit. map_select and this function share one matcher (_pattern_hit), so their answers cannot drift apart. Test: LocalFilter_WhenTheMapIsFull_AddsTheTextPatternClasses (a wwwroot/js/** class is added, a class whose pattern matches nothing is not).
  • I8. .claude/skills/lane-orders/SKILL.md, Build and test: a five-line bullet on local-filter (--base origin/dev, --map when a nightly map is at hand, strip the darling: / lite: prefix and paste the rest). This replaces the leftover above that left the file alone.

Verification (round 1)

  • python .github/scripts/ci-select.py --replay: replay: 731 rows, 2035 failing classes checked, 1 exempt (class no longer in the tree: 1 distinct), 27 accepted misses (replay-accepted.txt), 0 misses
  • --replay --map <run 38521722522 test-map> --symbols origin/dev:
    • map replay: 731 rows (312 FULL), 2035 failing classes checked, 59 accepted, 0 misses (reported, not failing yet)
    • symbol rule (#5798): 414 rows measured (317 without their head commit here), 3826 classes in the tree, cap 30 test files
    • selected per row: before median 2682 max 3826, after median 2687.5 max 3826
    • the rule names on its own: median 51 max 700
    • rows not FULL (238): before median 2054.5 max 3017, after median 2060.5 max 3017, added median 1 max 202
  • Never smaller than dev. A scratch script ran dev's token rules and cap (25) and this branch's on the same diff of every corpus head, then ran each side's map_select.
    • This branch: lost vs dev: 731 rows, 414 measured (317 without their head commit here), 0 rows select fewer classes than dev (0 classes lost), 26 rows select more and named by the symbol rule alone: 0 rows name fewer classes than dev (0 classes); dev tokens this branch does not take: [].
    • Control, the same script on 8e04003: named by the symbol rule alone: 10 rows name fewer classes than dev (66 classes). That matches the review's count. At map level that head lost nothing either (each of those heads was FULL, or its classes were selected for another reason), so the named-set line is the one that proves the fix.
  • Lite.Tests, CiSelectLocalClassesGuardTests, CiSelectMapSelectGuardTests, CiSelectReplayGuardTests, CrossAppGuardCiGateTests, DarlingReadsTraitGuardTests, TestMapShadowGuardTests, WatermarkPolicyTests: Total: 108, Errors: 0, Failed: 0
  • Darling.Tests, McpReadCommandTimeoutTests, GuardStageCensusTests, GuardStageWorkflowTests, FleetIdentifierScrubTests, CommandDeadlineScannerAdoptionTests: Total: 75, Errors: 0, Failed: 0. SlowClassCensusTests alone: Total: 7, Errors: 0, Failed: 0.
  • Builds: 0 errors. The only warnings are NuGet audit advisories, NU1902/NU1903 on System.Security.Cryptography.Pkcs 9.0.18 via PerformanceMonitor.PlanAnalysis. This PR does not cause them.
  • local-filter --base origin/dev --map <that map> on this branch: CI's map answer runs 1839 classes, over 300 then 743 classes, over 300: below are the first 300, most specific first. That prints 8 darling: filter lines of up to 40 classes each.
  • Full suites: not run (laptop rule). CI decides.

Round 1 leftovers

SECTION: Changed
ENTRY: - CI selects every test class that names a symbol a change defines or edits, repo-walking tests run in the guard set, and a local mode prints the filter CI will use ([#PR]) - The changed-symbol rule now bounds a token by a share of the test files (1%, at least 25) and reads store migration names, the failure replay reports how many classes the rule adds per run, 47 test classes that walk the repository tree carry Stage=Guard so the guard job and a lane's local set run them, and ci-select.py local-filter prints the per-suite class filter (40 classes a line, at most 300 classes) and the guard filter for a branch.

@erikdarlingdata
erikdarlingdata marked this pull request as ready for review October 14, 2026 12:52
…lits and cuts its answer (#5798)

- Drop the `new X(` exclusion: on 10 corpus heads it took tokens dev's rule gives (CollectorColumn, HostMemory,
  ...), so the symbol rule named 66 fewer classes than dev. The migration V-number now only adds a token.
- local-filter prints 40 classes a filter line, cuts its fallback at the limit, and with --map adds the classes
  whose text pattern matches a changed file (the web tests that read wwwroot/js).
- Pin symbol_cap (30 for 3,023 test files, 25 for 100) and the `new Migration(N,` branch.
- McpReadCommandTimeoutTests stays on the shards (49.9 s in the nightly map).
- lane-orders: how to use local-filter's output.
@erikdarlingdata
erikdarlingdata merged commit 2bd0aa2 into dev Oct 14, 2026
26 of 28 checks passed
@erikdarlingdata
erikdarlingdata deleted the fix/5798-test-selection-by-symbol branch October 14, 2026 14:09
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