Reduce ServerMode notification allocations

[Amaury Levé (Evangelink)]

Summary

• replace the TestNodeStateChangedEventArgs array LINQ projection with a capacity-sized List<object> fill while preserving null/empty behavior and runtime collection typing
• isolate the aggregator's captured filtering predicate so batches without terminal states avoid its closure allocation
• preserve the existing filtered LINQ materialization because pre-sized array/list alternatives increased allocation for large, heavily suppressed batches
• add focused coverage for exact JSON shape/order, object typing, null and empty changes, retained identity/order, predicate behavior, and source immutability

Performance

A standalone faithful-shape harness measured median time and allocation over null, empty, 1, 8, 128, and 1,024 item inputs on .NET 8 and .NET Framework 4.6.2.

• serializer, 8 items: 2,336 B -> 2,232 B on .NET 8; approximately 2,367 B -> 2,247 B on .NET Framework
• serializer, 1,024 items: 286,984 B -> 278,584 B on .NET 8; approximately 287,857 B -> 279,413 B on .NET Framework
• aggregator without terminal states: 24 B less allocation per batch on .NET 8, with the terminal/filtering path retaining its prior allocation shape

The serializer percentages are intentionally omitted because the harness models the container work faithfully but uses a smaller per-item payload than production. .NET Framework allocation values are approximate because its counter is AppDomain-wide.

Validation

.\build.cmd -projects .\test\UnitTests\Microsoft.Testing.Platform.UnitTests\Microsoft.Testing.Platform.UnitTests.csproj -test -bl

Passed on net8.0, net9.0, and net462 with 0 warnings and 0 errors.

Closes #10669

[Copilot]

Pull request overview

Reduces ServerMode notification allocations while preserving serialization and aggregation behavior.

Changes:

• Replaces serializer LINQ projection with a capacity-sized list.
• Isolates the aggregator’s captured filtering predicate.
• Adds serialization and aggregation regression tests.

Show a summary per file

File	Description
SerializerUtilities.TestNodeSerializers.cs	Optimizes change serialization.
TestNodeStateChangeAggregator.cs	Avoids closure allocation for non-terminal batches.
FormatterUtilitiesTests.cs	Tests serialization shape, typing, order, and null handling.
TestNodeStateChangeAggregatorTests.cs	Tests repeated aggregation behavior.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Balanced

[github-actions]

🧪 Expert test review — PR #10670

Grade	Test	Mutation	Notes	How to improve
A (90–100)	new TestNodeStateChangeAggregatorTests. BuildAggregatedChange_ DoesNotMutateBufferedChanges	4/4 killed	Mutates a buffered terminal item between two builds and asserts the previously suppressed in-progress item reappears in the right slot, proving the buffer itself is never mutated.	—
A (90–100)	new FormatterUtilitiesTests. Serialize_ TestNodeStateChangedEventArgs_ Changes_ PreservesOrderObjectTypesAndSource	3/3 killed	Verifies element order, UID values, and reference identity of the original source array against both the intermediate dictionary and the final JSON string, directly exercising the new indexed serialization loop.	—
A (90–100)	new FormatterUtilitiesTests. Serialize_ TestNodeStateChangedEventArgs_ EmptyChanges_ PreservesEmptyObjectList	2/2 killed	Confirms an empty (non-null) changes array serializes to [] rather than null, guarding the new null-check branch.	—
A (90–100)	new FormatterUtilitiesTests. Serialize_ TestNodeStateChangedEventArgs_ NullChanges_ PreservesNull	2/2 killed	Confirms a null changes array is preserved as null rather than becoming an empty list, guarding the new null-check branch.	—

Summary: This PR refactors TestNodeStateChangeAggregator.BuildAggregatedChange and the
TestNodeStateChangedEventArgs serializer to use an indexed loop instead of LINQ. All four new/
modified tests are well-targeted regression tests for the exact behaviors the refactor could
break (null vs. empty changes, element order/identity preservation, and buffer non-mutation
across repeated aggregation calls). No high-confidence actionable findings — all four tests
graded A with no inline suggestions.

_{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 · 73.3 AIC · ⌖ 3.44 AIC · ⊞ 16.9K · [◷]( · ◷)

[github-actions]

🧵 Parallel-safety audit — PR #10670

Parallelization — one row per test assembly audited:

Test assembly	Scope	Workers	Analyzer coverage
Microsoft.Testing.Platform.UnitTests	MethodLevel (via [assembly: Parallelize(Scope = ExecutionScope.MethodLevel, Workers = 0)] in Program.cs)	CPU count	coverable once the parallel-safety analyzers ship (attribute-based opt-in)

Findings: A (global-state) 0 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 0.

Nothing to flag. Both changed test files add pure, instance-scoped test methods with no cross-test coordination concerns:

• TestNodeStateChangeAggregatorTests.BuildAggregatedChange_DoesNotMutateBufferedChanges (new) creates its own local TestNodeStateChangeAggregator instance and only mutates a locally-owned TestNode.Properties._testNodeStateProperty on a TestNodeUpdateMessage it constructed itself in the test body — no shared/static state, no environment variables, no filesystem paths, no Console/culture mutation.
• FormatterUtilitiesTests (new Serialize_TestNodeStateChangedEventArgs_* tests) exercise SerializerUtilities.Serialize against locally-constructed messages via the instance-level _formatter field created once per test-class instantiation (MSTest constructs the class per test). SerializerUtilities's backing Dictionary<Type, IObjectSerializer> fields are populated once in a static constructor and only read afterward by every test in the assembly — no test mutates them, so there is no cross-test write/write or read/write race even under MethodLevel.
• No [ResourceLock], [DoNotParallelize], or assembly-level parallelization declaration was added, removed, or otherwise touched by this PR.

No production-code changes in this PR (SerializerUtilities.TestNodeSerializers.cs, TestNodeStateChangeAggregator.cs) introduce process-global mutation either — both are pure refactors of allocation patterns (capacity-sized list construction, extracted LINQ filter method) with no new shared state.

_{Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.}

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

[Copilot]

Review details

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