[code-simplifier] refactor: use switch expression for TargetInvocationException unwrap in BaseRunTests - #16181
Conversation
…in BaseRunTests Replace the conditional ternary with a switch expression, which is the idiomatic pattern in this codebase per project guidelines. No behavioral change — the switch expression is semantically equivalent to the ternary it replaces. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR performs a small refactor in the CrossPlatEngine execution path to simplify the TargetInvocationException unwrapping logic in BaseRunTests.RunTests, using a switch expression while keeping behavior unchanged.
Changes:
- Replaced the ternary-based
TargetInvocationExceptionunwrap with a switch expression using a property pattern. - Kept the existing behavior: unwrap only when
InnerExceptionis non-null; otherwise preserve the original exception.
Jakub Jareš (nohwnd)
left a comment
There was a problem hiding this comment.
🧠 Reviewed by expert-reviewer (Parallel Execution & Scheduling Safety · Error Reporting & Diagnostic Clarity · Algorithmic Correctness)
Scope
Single-file refactor in BaseRunTests.cs — replaces a ternary with a switch expression in the TargetInvocationException unwrap logic. Activated dimensions: Parallel Execution & Scheduling Safety, Error Reporting & Diagnostic Clarity, Algorithmic Correctness, Defensive Coding at Boundaries.
Semantic Equivalence ✅
The two forms are provably equivalent:
| Form | Condition | Result |
|---|---|---|
| Old ternary | ex is TargetInvocationException tie && tie.InnerException is not null |
tie.InnerException |
| New switch arm | TargetInvocationException { InnerException: { } inner } |
inner |
The { } property pattern in C# is a non-null object pattern — it matches any non-null value, exactly mirroring is not null. No behavioral change.
Error Reporting & Diagnostic Clarity ✅
The switch expression preserves all exception propagation semantics introduced in #16167. realException.Message and realException as inner exception are unaffected.
Parallel Execution & Scheduling Safety ✅
No threading-related changes. The catch block is local to the RunTests execution thread.
Defensive Coding at Boundaries ✅
The _ => ex fallback arm correctly handles all non-TargetInvocationException cases, including TargetInvocationException with a null InnerException (which the { } pattern rejects, falling through to _ => ex).
PR Description Alignment ✅
Title, description, before/after snippets, and reference to #16167 all match the diff exactly.
No issues found. The refactor is correct and clean.
🧠 Reviewed by Expert Code Reviewer 🧠
Jakub Jareš (nohwnd)
left a comment
There was a problem hiding this comment.
🧠 Reviewed by expert-reviewer (Parallel Execution & Scheduling Safety · Error Reporting & Diagnostic Clarity · Algorithmic Correctness)
Scope
Single-file refactor in BaseRunTests.cs — replaces a ternary with a switch expression in the TargetInvocationException unwrap logic. Activated dimensions: Parallel Execution & Scheduling Safety, Error Reporting & Diagnostic Clarity, Algorithmic Correctness, Defensive Coding at Boundaries.
Semantic Equivalence ✅
The two forms are provably equivalent:
| Form | Condition | Result |
|---|---|---|
| Old ternary | ex is TargetInvocationException tie && tie.InnerException is not null |
tie.InnerException |
| New switch arm | TargetInvocationException { InnerException: { } inner } |
inner |
The { } property pattern in C# is a non-null object pattern — it matches any non-null value, exactly mirroring is not null. The _ => ex arm handles all non-matching cases, including TargetInvocationException with a null InnerException. No behavioral change.
Error Reporting & Diagnostic Clarity ✅
The switch expression preserves all exception propagation semantics introduced in #16167. realException.Message and realException as inner exception are unaffected.
Parallel Execution & Scheduling Safety ✅
No threading-related changes. The catch block is local to the RunTests execution thread.
Defensive Coding at Boundaries ✅
The _ => ex fallback arm is exhaustive and correctly handles all non-TargetInvocationException cases.
PR Description Alignment ✅
Title, description, before/after snippets, and reference to #16167 all match the diff exactly.
No issues found. The refactor is correct and clean.
🧠 Reviewed by Expert Code Reviewer 🧠
🧠 Reviewed by Expert Code Reviewer 🧠
Code Simplification — 2026-06-28
This PR simplifies recently modified code to improve clarity and consistency while preserving all functionality.
Files Simplified
src/Microsoft.TestPlatform.CrossPlatEngine/Execution/BaseRunTests.cs— replaced conditional ternary with a switch expression for theTargetInvocationExceptionunwrap logicImprovements Made
Applied Project Standards
is ... && ... ? ... : ...ternary with a switch expression, per the project guideline: "Use pattern matching and switch expressions wherever possible"TargetInvocationException { InnerException: { } inner }makes both the type check and the null guard explicit in one arm, removing the intermediatetiebinding variableBefore:
After:
Changes Based On
Recent changes from:
Testing
Microsoft.TestPlatform.CrossPlatEngine.UnitTeststests pass (net11.0|x64)Microsoft.TestPlatform.CommunicationUtilities.UnitTeststests pass (net11.0|x64)