Unify ApplicationStateGuard across copies (#9316) by Evangelink · Pull Request #9329 · microsoft/testfx

Evangelink · 2026-06-22T09:25:48Z

Fixes #9316. Updates the Platform and Adapter ApplicationStateGuard.Unreachable() copies to return UnreachableException, matching the TestFramework and Analyzer copies. All four projects already include the shared UnreachableException polyfill/global using, so no copy needed to stay on InvalidOperationException; Ensure() continues to throw InvalidOperationException. Adds source-copy comments pointing to the Platform canonical copy.

…9316) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR addresses #9316 by aligning the behavior of the duplicated ApplicationStateGuard.Unreachable() helpers across Platform, Adapter, TestFramework, and Analyzers so they consistently return UnreachableException (while Ensure() continues throwing InvalidOperationException). It also adds comments intended to document the intentional source-copy pattern across these layered projects.

Changes:

Switch ApplicationStateGuard.Unreachable() in Platform and Adapter copies to return UnreachableException for consistency with TestFramework and Analyzers.
Add “source-copy / keep in sync” comments to all four copies, pointing at the Platform file as canonical.

Show a summary per file

File	Description
src/TestFramework/TestFramework/Internal/ApplicationStateGuard.cs	Adds source-copy sync comment referencing the canonical Platform copy.
src/Platform/Microsoft.Testing.Platform/Helpers/ApplicationStateGuard.cs	Changes `Unreachable()` to return `UnreachableException`; adds source-copy sync comment.
src/Analyzers/MSTest.Analyzers/Helpers/ApplicationStateGuard.cs	Adds source-copy sync comment referencing the canonical Platform copy.
src/Adapter/MSTestAdapter.PlatformServices/Utilities/ApplicationStateGuard.cs	Changes `Unreachable()` to return `UnreachableException`; adds source-copy sync comment.

Copilot's findings

Files reviewed: 4/4 changed files
Comments generated: 1

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
17	Documentation Accuracy	🟢 1 NIT

✅ 21/22 dimensions clean.

Documentation — canonical Platform file's sync comment is self-referential (points to itself)

Notes on key dimensions checked:

Cross-TFM Compatibility ✅ — UnreachableException polyfill in src/Polyfills/UnreachableException.cs (guarded #if !NETCOREAPP) is glob-included by all four affected projects, so the type change compiles on net462, netstandard2.0, and all UWP/WinUI targets.
Backward Compatibility ✅ — Unreachable() is internal in all four classes; no public API surface changed.
Test Coverage ✅ — No existing test catches InvalidOperationException from a path routed through ApplicationStateGuard.Unreachable(). The one test that uses Unreachable() in the adapter unit tests (TestMethodRunnerTests.cs:399) uses it as an unreachable lambda, not in a catch assertion.
Algorithmic Correctness ✅ — UnreachableException (System.Diagnostics.UnreachableException) is the canonical type for "should never execute" guards; the type change is semantically correct and does not weaken any invariant.

Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>

… polyfill type identity mismatch On non-NETCOREAPP TFMs (net462/net48) UnreachableException is an internal, per-assembly [Embedded] polyfill. Asserting via the generic type parameter compared against each test assembly's own copy and could fail due to type identity mismatch with the copy thrown from a referenced assembly. Capture the exception as the base Exception type and assert on GetType().FullName instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot's findings

Files reviewed: 9/9 changed files
Comments generated: 8

…ess alias, normalize encoding Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Resolve conflicts in ClassCleanupManager.cs (adopt main's AdapterApplicationStateGuard alias) and three UnreachableException assertions (keep typeof(...).FullName form). Regenerate TerminalResources xlf for new strings introduced on main. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot's findings

Files reviewed: 20/20 changed files
Comments generated: 4

+/* This file is intentionally copied across layered projects; keep it in sync with the canonical copy at
+ * src\Platform\Microsoft.Testing.Platform\Helpers\ApplicationStateGuard.cs. */


+/* This file is intentionally copied across layered projects; keep it in sync with the canonical copy at
+ * src\Platform\Microsoft.Testing.Platform\Helpers\ApplicationStateGuard.cs. */


+/* This file is intentionally copied across layered projects; keep it in sync with the canonical copy at
+ * src\Platform\Microsoft.Testing.Platform\Helpers\ApplicationStateGuard.cs. */


+      <trans-unit id="Try">
+        <source>try {0}</source>
+        <target state="new">try {0}</target>
+        <note>number or tries of the current test assembly when test assembly is being retried. {0} is number that starts at 1</note>


Evangelink · 2026-06-22T13:04:34Z

🧪 Test quality grade — PR #9329

3 test methods graded (all modified) across 3 files. Distribution: 2 × C, 1 × A. Both C-grade tests use the broad Assert.Throws<Exception> / Assert.ThrowsAsync<Exception> catch required by cross-assembly polyfill constraints — the rubric mechanically flags this as High → C ceiling; the A-grade test avoids the broad-catch pattern entirely and scores cleanly.

Δ	Test	Grade	Band	Notes
mod	`ConfigurationExtensionsTests. ConfigurationExtensions_ TestedMethod_ ThrowsArgumentNullException`	C	70–79	`Assert.Throws(Exception)` (High → C ceiling) is intentional — follow-up FullName check makes it specific.
mod	`StopPoliciesServiceTests. RegisterOnMaxFailedTestsCallbackAsync_ ThrowsIfNotTestHost`	C	70–79	`Assert.ThrowsAsync(Exception)` (High → C); narrowed by type-name check; foreach over 2 roles could be `[DataRow]`.
mod	`ApplicationStateGuardTests. Unreachable_ ReturnsUnreachableException`	A	90–100	No issues found.

_{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. · 495 AIC · ⌖ 12.9 AIC · ⊞ 43.7K · [◷]( · ◷)

Unify ApplicationStateGuard.Unreachable exception type across copies (#…

33f478c

…9316) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings June 22, 2026 09:25

Copilot started reviewing on behalf of Evangelink June 22, 2026 09:26 View session

Copilot AI reviewed Jun 22, 2026

View reviewed changes

Comment thread src/Platform/Microsoft.Testing.Platform/Helpers/ApplicationStateGuard.cs Outdated

Evangelink commented Jun 22, 2026

View reviewed changes

Comment thread src/Platform/Microsoft.Testing.Platform/Helpers/ApplicationStateGuard.cs Outdated

Copilot AI and others added 6 commits June 22, 2026 13:34

Initial plan

a8be655

Unify ApplicationStateGuard unreachable exceptions

e4c2644

Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>

Tighten ApplicationStateGuard tests

b0d218a

Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>

Polish ApplicationStateGuard cleanup

f1f4e72

Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>

Clarify canonical ApplicationStateGuard sync comment

56105a5

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings June 22, 2026 11:35

Copilot started reviewing on behalf of Evangelink June 22, 2026 11:41 View session

Copilot AI reviewed Jun 22, 2026

View reviewed changes

Address review: typeof-based UnreachableException asserts, drop needl…

24df4d8

…ess alias, normalize encoding Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Evangelink added the state/needs-review Awaiting review from the team. label Jun 22, 2026

0101 approved these changes Jun 22, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings June 22, 2026 12:33

Copilot started reviewing on behalf of Evangelink June 22, 2026 12:35 View session

Evangelink merged commit 009410c into main Jun 22, 2026
14 of 21 checks passed

Evangelink deleted the fix/9316-dedup-applicationstateguard branch June 22, 2026 12:36

Copilot AI reviewed Jun 22, 2026

View reviewed changes

Evangelink mentioned this pull request Jun 22, 2026

Extract shared STA-thread dispatch helper (#9303) #9326

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Unify ApplicationStateGuard across copies (#9316)#9329

Unify ApplicationStateGuard across copies (#9316)#9329
Evangelink merged 9 commits into
mainfrom
fix/9316-dedup-applicationstateguard

Evangelink commented Jun 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Evangelink left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Evangelink commented Jun 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		/* This file is intentionally copied across layered projects; keep it in sync with the canonical copy at
		* src\Platform\Microsoft.Testing.Platform\Helpers\ApplicationStateGuard.cs. */

Uh oh!

Conversation

Evangelink commented Jun 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Evangelink left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Evangelink commented Jun 22, 2026

🧪 Test quality grade — PR #9329

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants