Add DisposeHelper unit tests

[Amaury Levé (Evangelink)]

Summary

• add direct coverage for the linked-source DisposeHelper
• verify cleanup ordering, synchronous fallback, and async-disposal preference
• verify exception and cancellation propagation plus repeated invocation behavior
• exercise the conditional implementation on net462, net8.0, and net9.0

Validation

• warning-free project builds for net462, net8.0, and net9.0
• full Microsoft.Testing.Platform.UnitTests suite:
  • net462: 2,164 passed
  • net8.0: 2,215 passed
  • net9.0: 2,216 passed

Closes #10646

[github-actions]

Note

🤖 Automated review by GitHub Copilot. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review Summary

#	Dimension	Verdict
1	Algorithmic Correctness	✅ Pass — tests correctly verify all branches of DisposeHelper.DisposeAsync (null, non-disposable, sync dispose, async dispose, cleanup-then-dispose ordering, exception propagation).
2	Threading & Concurrency	N/A — test helpers are single-threaded by design.
3	Security & IPC Contract Safety	N/A — no IPC or security-sensitive code.
4	Public API & Binary Compatibility	N/A — test-only change, no public API surface.
5	Performance & Allocations	N/A — test code, not hot-path.
6	Cross-TFM Compatibility	✅ Pass — #if NETCOREAPP guards correctly mirror the production code's conditional compilation for IAsyncDisposable.
7	Test Quality	✅ Pass — good coverage of happy path, null input, exception propagation (both cleanup and dispose), double-dispose idempotency, and async-preferred-over-sync ordering. Uses MSTest Assert (correct per BannedSymbols.txt).
8–22	Remaining dimensions	N/A — no localization, no MSBuild changes, no CLI options, no documentation, no style violations detected.

Verdict: No issues found. The tests are well-structured, correctly use Assert.AreSequenceEqual for invocation-order verification, and the recording test doubles are minimal and clear.

[Copilot]

Pull request overview

Adds direct multi-target unit coverage for the linked-source DisposeHelper.

Changes:

• Tests cleanup/disposal ordering and repeated invocation.
• Tests exception, cancellation, and target-specific disposal behavior.
• Links DisposeHelper.cs into the unit-test project.

Show a summary per file

File	Description
DisposeHelperTests.cs	Adds comprehensive disposal behavior tests.
Microsoft.Testing.Platform.UnitTests.csproj	Links the embedded helper source for testing.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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

[github-actions]

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 85.2 AIC · ⌖ 5.21 AIC · ⊞ 16.9K · ◷

[github-actions]

🧪 Expert test review — PR #10649

Grade	Test	Mutation	Notes	How to improve
B (80–89)	new DisposeHelperTests. DisposeAsync_Null_ DoesNothing	1/1 killed	Only verifies the null guard doesn't throw; no explicit assertion beyond the implicit no-throw.	Wrap in try/no-throw idiom or assert a sentinel wasn't touched to make the "does nothing" claim explicit.
B (80–89)	new DisposeHelperTests. DisposeAsync_ UnsupportedObject_ DoesNothing	1/1 killed	Relies solely on absence of an exception to prove the unsupported-type branch is a no-op.	Use an instrumented plain object (e.g. a counter field) to assert no callback fired, not just no throw.
A (90–100)	new DisposeHelperTests. DisposeAsync_ AsyncDisposeIsCanceled_ PropagatesCancellationWithoutSynchronousDispose	4/4 killed	Verifies cancellation exception identity/token and exact invocation order excluding sync Dispose.	—
A (90–100)	new DisposeHelperTests. DisposeAsync_ AsyncDisposable_ CleansUpThenPrefersAsyncDispose	3/3 killed	Confirms cleanup runs before DisposeAsync and that sync Dispose is skipped when async is available.	—
A (90–100)	new DisposeHelperTests. DisposeAsync_ CalledTwice_ InvokesCleanupAndDisposeTwice	2/2 killed	Exercises repeated invocation and asserts the full call sequence for both calls.	—
A (90–100)	new DisposeHelperTests. DisposeAsync_ CleanableDisposable_ CleansUpBeforeDisposing	3/3 killed	Verifies both cleanup and dispose fire and in the correct order.	—
A (90–100)	new DisposeHelperTests. DisposeAsync_ CleanupThrows_ PropagatesExceptionWithoutDisposing	3/3 killed	Confirms exception identity propagates and Dispose is never reached after cleanup fails.	—
A (90–100)	new DisposeHelperTests. DisposeAsync_ SynchronousDisposeThrows_ PropagatesException	3/3 killed	Confirms exception identity propagates and cleanup still ran prior to the throwing Dispose.	—

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

[github-actions]

🧵 Parallel-safety audit — PR #10649

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

The only change is a new DisposeHelperTests.cs file testing DisposeHelper.DisposeAsync, a stateless static helper that operates purely on locally-constructed instance objects (RecordingSyncResource / RecordingAsyncResource) passed as method arguments. Each test creates its own instance with per-instance mutable state (List<string> Invocations); there are no static fields, no environment variable, current-directory, console, or culture mutations, no filesystem paths, and no changed [ResourceLock] / [DoNotParallelize] / [Parallelize] declarations. The accompanying .csproj change only adds a <Compile Include> link and has no parallelization impact.

Audited Microsoft.Testing.Platform.UnitTests at scope MethodLevel (from [assembly: Parallelize(Scope = ExecutionScope.MethodLevel, Workers = 0)] in Program.cs, unchanged by this PR), workers CPU count.

_{Re-run with /parallel-audit.}

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

[Copilot]

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

test/UnitTests/Microsoft.Testing.Platform.UnitTests/Helpers/DisposeHelperTests.cs:103

• ValueTask.FromException creates a faulted operation, even when its exception is an OperationCanceledException, so this test does not exercise the canceled-ValueTask behavior named here and claimed by the PR. Construct the value with FromCanceled and use the derived-type assertion because awaiting it can throw TaskCanceledException.

        RecordingAsyncResource resource = new(() => ValueTask.FromException(expectedException));

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