Skip to content

Suppress redundant in-progress test updates - #10483

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/fix-redundant-in-progress-updates
Aug 6, 2026
Merged

Suppress redundant in-progress test updates#10483
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/fix-redundant-in-progress-updates

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Suppress redundant in-progress TestNodeUpdateMessage entries when the same test UID has a terminal state in the emitted batch. Discovered, custom, and state-less updates remain untouched, retained updates preserve order, and separate batches remain independent.

Adds focused coverage for every known terminal state, custom non-terminal states, both update orders, unrelated UIDs, retained order and identity, and batch isolation.

Closes#5348

Validation

  • Microsoft.Testing.Platform.UnitTests targeted build with binary log
  • 13 focused aggregator tests
  • Full 2,134-test Microsoft.Testing.Platform.UnitTests net8 suite

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 6, 2026 14:08
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 6, 2026

CopilotAI 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

Suppresses redundant in-progress test updates when the same batch contains a terminal update for that test UID.

Changes:

  • Filters superseded in-progress updates while preserving order.
  • Adds coverage for terminal states, ordering, UIDs, and batch isolation.
Show a summary per file
FileDescription
TestNodeStateChangeAggregator.csImplements batch-level suppression.
TestNodeStateChangeAggregatorTests.csAdds focused aggregator tests.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

@github-actions

Copy link
Copy Markdown
Contributor

Warning

Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding.

What happened

The threat detection results could not be parsed.

Review the workflow run logs for details.

🧪 Test quality grade — PR #10483

GradeTestMutationNotesHow to improve
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
InProgressThenTerminalForSameUid_
OmitsInProgress
2/2 killedCovers all terminal states via DynamicData and verifies identity + count of the retained change.
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
TerminalThenInProgressForSameUid_
OmitsInProgress
2/2 killedExercises the reverse ordering, protecting the "seen-before" branch of the aggregation.
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
NonTerminalStateForSameUid_
RetainsInProgress
2/2 killedConfirms Discovered/custom states do not trigger suppression, guarding the terminal-state predicate.
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
NoStateForSameUid_
RetainsInProgress
1/1 killedProtects the null-state branch of the property lookup.
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
TerminalForDifferentUid_
RetainsInProgress
1/1 killedGuards that suppression is scoped per test UID and does not leak across unrelated nodes.
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
WhenSuppressingInProgress_
PreservesRetainedOrderAndIdentity
3/3 killedMixed-batch scenario verifies ordering and identity of every retained item is preserved.
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
SeparateAggregatorBatches_
DoNotInfluenceEachOther
1/1 killedConfirms per-instance state isolation, guarding against a shared/static tracking bug.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/grade-tests.

🤖 Automated content by GitHub Copilot. Generated by the Grade Tests on PR (on open / sync) workflow. · auto · 56.6 AIC · ⊞ 16.2K · [◷]( · )

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 6, 2026 15:03

CopilotAI 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.

Review details

  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@Evangelink
Amaury Levé (Evangelink) merged commit e218387 into mainAug 6, 2026
36 of 38 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/fix-redundant-in-progress-updates branch August 6, 2026 16:40
@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10483

Nothing audited here touches process-global state, shared filesystem paths, or
[ResourceLock] / [DoNotParallelize] declarations. Nothing to flag for
parallel-safety.

  • src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/TestNodeStateChangeAggregator.cs — pure algorithmic change (filters in-progress updates superseded by a terminal state for the same UID); operates only on locally-scoped collections passed into the method, no static/shared state.
  • test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/TestNodeStateChangeAggregatorTests.cs (new) — each test constructs its own TestNodeStateChangeAggregator instance and local TestNodeUpdateMessage values; no statics, no filesystem/env/culture/console mutation, no [ResourceLock]/[DoNotParallelize] declared or needed.
  • test/IntegrationTests/MSTest.Acceptance.IntegrationTests/ServerModeTests.cs — one-line assertion change only (Assert.HasCount(2, ...)Assert.ContainsSingle(x => x.Node.ExecutionState == "passed", ...)); no change to shared state or fixtures.

Audited Microsoft.Testing.Platform.UnitTests at scope MethodLevel, workers CPU count ([assembly: Parallelize(Scope = MethodLevel, Workers = 0)] in Program.cs).
Audited MSTest.Acceptance.IntegrationTests at scope MethodLevel, workers CPU count (same attribute pattern in Program.cs); note per-file assembly-attribute overrides exist elsewhere in that project (e.g. ParallelExecutionTests.cs, ResourceLockExecutionTests.cs) but this PR does not touch them.

Re-run with /parallel-audit.

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 58 AIC · ⌖ 3.65 AIC · ⊞ 24.6K · [◷]( · )

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10483

GradeTestMutationNotesHow to improve
B (80–89)mod ServerModeTests.
DiscoverAndRun
1/2 killedChecks a single "passed" node exists but no longer asserts total update count, so a stray duplicate in-progress leak would survive.Also assert the total update count (e.g. exactly 1) alongside the "passed" filter.
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
InProgressThenTerminalForSameUid_
OmitsInProgress
4/4 killedDynamicData over every terminal state; asserts count and identity of the retained node.
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
TerminalThenInProgressForSameUid_
OmitsInProgress
2/2 killedCovers reverse ordering of terminal-then-in-progress; identity-checked.
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
NonTerminalStateForSameUid_
RetainsInProgress
3/3 killedDynamicData over non-terminal states confirms in-progress is retained and ordering preserved.
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
NoStateForSameUid_
RetainsInProgress
2/2 killedCovers the null-state edge case explicitly with identity + count assertions.
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
TerminalForDifferentUid_
RetainsInProgress
3/3 killedConfirms suppression is scoped per-Uid, not global.
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
WhenSuppressingInProgress_
PreservesRetainedOrderAndIdentity
4/4 killedStrong combined scenario asserting exact order and identity of all retained updates.
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
SeparateAggregatorBatches_
DoNotInfluenceEachOther
2/2 killedVerifies aggregator instances don't share the suppression HashSet across batches.

7 of 8 tests are A-grade with strong pseudo-mutation resilience, covering the new terminal-state suppression logic thoroughly (per-Uid scoping, ordering, batch isolation). The one B is the minor integration-test assertion tweak in ServerModeTests.DiscoverAndRun, which trades an exact update-count check for a predicate-based one — still valid but slightly less precise.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 57.2 AIC · ⌖ 4.95 AIC · ⊞ 16.7K · [◷]( · )

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tests are sent back as in-progress and passed

3 participants

@Evangelink@0101