[perf-improver] perf: defer class-cleanup TestContextImplementation allocation to last test in class

[Evangelink]

Goal and Rationale

In RunSingleTestAsync, a TestContextImplementation for class cleanup was allocated unconditionally for every test — even when isLastTestInClass was false and the context was never used. The TestContextImplementation constructor copies the testContextProperties dictionary and registers a CancellationTokenRegistration, so this is a non-trivial per-test allocation.

The fix is the same pattern as PR #9433 (which deferred assembly-init and class-init contexts to the slow path): defer the class-cleanup context to only the last test in each class.

Approach

Move the GetTestContext(...) call for testContextForClassCleanup from an unconditional position before MarkTestComplete into the if (isLastTestInClass) block.

Safety invariant: the assembly-cleanup branch reads testContextForClassCleanup!.Context.CurrentTestOutcome. That branch only runs when ShouldRunEndOfAssemblyCleanup is true, which requires MarkClassComplete to have been called — and MarkClassComplete is called exclusively inside the isLastTestInClass block. So testContextForClassCleanup is guaranteed non-null when the assembly-cleanup branch is reached. A comment documents this reasoning.

Performance Evidence

Allocation savings: For a test suite with C classes averaging K tests each:

• Before: allocates C × K class-cleanup TestContextImplementation objects per run
• After: allocates C × 1 (one per class, only for the last test)
• Savings: C × (K−1) allocations eliminated — each saving one dictionary copy + one CancellationTokenRegistration

For a typical suite with 50 test classes averaging 10 tests each:

• Eliminated ~450 unnecessary TestContextImplementation allocations per run

This is the same pattern and magnitude as the assembly-init/class-init fast paths introduced in #9433.

Methodology: Code inspection + allocation counting via TestablePlatformServiceProvider.GetTestContextCallCount (same mechanism used in the existing unit tests for #9433).

Trade-offs

None. The context was allocated but never used; removing the allocation changes no observable behaviour.

Test Status

✅ Build/tests: local SDK not available in CI agent (pinned SDK 11.0.100-preview, not installed). Verification delegated to CI.

The new unit test RunSingleTestShouldDeferClassCleanupContextAllocationToLastTestInClass verifies that:

1. A middle test in a class (warm path) allocates fewer contexts than the last test (which correctly allocates the class-cleanup and assembly-cleanup contexts).
2. All three tests pass with UnitTestOutcome.Passed.

Reproducibility

// TestablePlatformServiceProvider.GetTestContextCallCount tracks all GetTestContext() calls.
// Middle test: count increases by 1 (test-exec context only).
// Last test: count increases by 3 (test-exec + class-cleanup + assembly-cleanup contexts).

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Perf Improver workflow. · 2.3K AIC · ⌖ 40 AIC · ⊞ 57.7K · [◷]( · ◷)

Add this agentic workflows to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/perf-improver.md@main

[Copilot]

Pull request overview

This PR improves MSTest adapter execution performance by avoiding unnecessary TestContextImplementation allocations for class cleanup when the current test is not the last test in its class, aligning the cleanup path with earlier fast-path optimizations for init contexts.

Changes:

• Move class-cleanup GetTestContext(...) allocation behind the isLastTestInClass check in UnitTestRunner.RunSingleTestAsync.
• Add a unit test that compares GetTestContext allocation behavior for a middle test vs. the last test in a class (where cleanup contexts are expected).

Show a summary per file

File	Description
src/Adapter/MSTestAdapter.PlatformServices/Execution/UnitTestRunner.cs	Defers class-cleanup TestContextImplementation allocation to the last test in each class and documents the non-null invariant used for assembly cleanup.
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/UnitTestRunnerTests.cs	Adds a unit test intended to validate the deferred class-cleanup context allocation behavior.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Low

[Evangelink]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

#	Dimension	Verdict
15	Code Structure & Simplification	🟢 1 NIT
16	Naming & Conventions	🟢 1 NIT

✅ 20/22 dimensions clean.

• Code Structure — testContextForClassCleanup! uses the null-forgiving ! operator; DebugEx.Assert is the codebase's established idiom for invariant documentation (matches line 152 pattern)
• Naming & Conventions — new test bypasses CreateUnitTestRunner(...) helper without justification; structurally identical sibling test uses the helper correctly

---

Correctness ✅ — The invariant that testContextForClassCleanup is non-null whenever ShouldRunEndOfAssemblyCleanup is true holds under the production sequential execution model (TestExecutionManager.Runner.cs drives tests one at a time via a sequential foreach/await loop). Tracing the flow: ShouldRunEndOfAssemblyCleanup becomes true only after the dictionary is empty; the dictionary is emptied only via MarkClassComplete; MarkClassComplete is called exclusively inside the if (isLastTestInClass) block where testContextForClassCleanup is allocated. No code path exists where ShouldRunEndOfAssemblyCleanup == true with testContextForClassCleanup == null. The ! operator is safe.

Test ✅ — The allocation-delta assertion allocationsForThird.Should().Be(allocationsForSecond + 2) correctly guards the regression: if the optimization regresses (non-last test mistakenly allocates the class-cleanup context), allocationsForSecond increases by 1 while allocationsForThird stays the same, making the assertion fail. The exact-delta approach is intentional and well-explained in the comment.

Disposal ✅ — The finally block's (testContextForClassCleanup as IDisposable)?.Dispose() is a no-op when testContextForClassCleanup is null (non-last tests), which is correct.

Threading ✅ — testContextForClassCleanup is a method-local variable; there is no shared-state concern between concurrent invocations.

[Copilot]

Review details

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

[Evangelink]

🧪 Test quality grade — PR #9461

Δ	Test	Grade	Band	Notes
new	UnitTestRunnerTests. RunSingleTestShouldDeferClassCleanupContextAllocationToLastTestInClass	B	80–89	Precise equality assertion on the deferred-allocation delta; body is ~40 lines for a single behavior, which is the only mild concern.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.}

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 242.8 AIC · ⌖ 13.3 AIC · ⊞ 45.7K · [◷]( · ◷)