Skip to content

[code-simplifier] refactor: replace nested ternaries with if/else in DotnetTestDataConsumer#9398

Closed
Evangelink wants to merge 2 commits into
mainfrom
code-simplify/nested-ternaries-dotnet-test-consumer-9c9855cda9fb3722
Closed

[code-simplifier] refactor: replace nested ternaries with if/else in DotnetTestDataConsumer#9398
Evangelink wants to merge 2 commits into
mainfrom
code-simplify/nested-ternaries-dotnet-test-consumer-9c9855cda9fb3722

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Code Simplification — 2026-06-24

This PR simplifies recently modified code in DotnetTestDataConsumer.cs to improve clarity while preserving all functionality.

Files Simplified

  • src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/DotnetTestDataConsumer.cs — Replaced nested ternary operators with if/else chains

Improvement Made

PR #9380 introduced a single-pass PropertyBag collection pattern that included two nested ternary expressions:

// Before — nested ternary operators
FileArtifactProperty[] artifacts = firstArtifact is null
    ? []
    : artifactsOverflow is not null ? [.. artifactsOverflow] : [firstArtifact];
TestMetadataProperty[] traits = firstTrait is null
    ? []
    : traitsOverflow is not null ? [.. traitsOverflow] : [firstTrait];

Rewritten as explicit if/else chains per project coding guidelines, which state:

Avoid nested ternary operators — prefer switch statements or if/else chains. Choose clarity over brevity.

// After — clear if/else chains
FileArtifactProperty[] artifacts;
if (firstArtifact is null)
{
    artifacts = [];
}
else if (artifactsOverflow is not null)
{
    artifacts = [.. artifactsOverflow];
}
else
{
    artifacts = [firstArtifact];
}
// (identical pattern for traits)

Changes Based On

Recent changes from:

Testing

  • ✅ No functional changes — behavior is identical (purely structural refactor)
  • ✅ Logic equivalence verified: all three branches (null, overflow, single-item) remain identical

Review Focus

Please verify:

  • The three-way branch logic is preserved exactly
  • Style matches project conventions

🤖 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 Code Simplifier workflow. · 93.4 AIC · ⌖ 17.7 AIC · ⊞ 9.3K · [◷]( · )

Add this agentic workflows to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/code-simplifier.md@main
  • expires on Jun 25, 2026, 1:57 PM UTC

…umer

Convert nested ternary operators in GetTestNodeDetails to
if/else chains per project coding guidelines that prefer
explicit control flow over compact nested expressions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 24, 2026 13:58
@Evangelink Evangelink added type/automation Created or maintained by an agentic workflow. type/tech-debt Code health, refactoring, simplification. labels Jun 24, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors DotnetTestDataConsumer (Microsoft.Testing.Platform server-mode dotnet test IPC) to improve readability by replacing nested ternary expressions with explicit if/else chains while preserving the existing “null / overflow list / single-item” branching behavior for artifacts and traits.

Changes:

  • Replace nested ternary construction of FileArtifactProperty[] with an equivalent if/else chain.
  • Replace nested ternary construction of TestMetadataProperty[] with an equivalent if/else chain.
Show a summary per file
File Description
src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/DotnetTestDataConsumer.cs Refactors array materialization logic for artifacts/traits from nested ternaries to explicit if/else branches for clarity.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@Evangelink

Copy link
Copy Markdown
Member Author

🔍 Build Failure Analysis

Summary — The build fails in Microsoft.Testing.Platform (and all projects that depend on it) because two newly introduced if-else blocks in DotnetTestDataConsumer.cs violate the IDE0045 analyzer rule ("if statement can be simplified"), which is treated as a compile error in this repo.

Root cause: IDE0045if statements should be conditional expressions

The PR expanded two ternary/conditional expressions (one for artifacts, one for traits) into explicit if-else if-else blocks. The project's .editorconfig / Roslyn configuration enforces IDE0045 (prefer conditional expression over if-with-assignment) as an error. This caused 6 identical CS-severity-lifted analyzer errors (3 TFMs × 2 locations), blocking compilation of Microsoft.Testing.Platform.csproj and every project that depends on it.

Affected file / errors

Code File Line Message
IDE0045 DotnetTestDataConsumer.cs:280 280 'if' statement can be simplified
IDE0045 DotnetTestDataConsumer.cs:294 294 'if' statement can be simplified

(Each fires 3 times — once per target framework compiled.)

Proposed fix

Revert both if-else blocks back to the original conditional expressions the PR replaced:

-        FileArtifactProperty[] artifacts;
-        if (firstArtifact is null)
-        {
-            artifacts = [];
-        }
-        else if (artifactsOverflow is not null)
-        {
-            artifacts = [.. artifactsOverflow];
-        }
-        else
-        {
-            artifacts = [firstArtifact];
-        }
-
-        TestMetadataProperty[] traits;
-        if (firstTrait is null)
-        {
-            traits = [];
-        }
-        else if (traitsOverflow is not null)
-        {
-            traits = [.. traitsOverflow];
-        }
-        else
-        {
-            traits = [firstTrait];
-        }
+        FileArtifactProperty[] artifacts = firstArtifact is null
+            ? []
+            : artifactsOverflow is not null ? [.. artifactsOverflow] : [firstArtifact];
+        TestMetadataProperty[] traits = firstTrait is null
+            ? []
+            : traitsOverflow is not null ? [.. traitsOverflow] : [firstTrait];

An inline suggestion is attached to the diff for a one-click apply.


Build overview
  • Result: FAILED
  • Duration: 142.3s
  • MSBuild: 18.7.0-preview
  • Projects: 46 total, 7 failed
  • Errors: 7 (6× IDE0045 + final "Build failed.")
  • Warnings: 0
  • Failed projects: Microsoft.Testing.Platform.csproj, Microsoft.Testing.Extensions.AzureFoundry.csproj, Microsoft.Testing.Platform.AI.csproj, Microsoft.Testing.Extensions.CrashDump.csproj, MSTest.TestAdapter.csproj (all downstream of MTP), plus the solution wrapper.
All MSBuild errors (6 unique, 7 total)
Code Project File:Line Message
IDE0045 Microsoft.Testing.Platform DotnetTestDataConsumer.cs:280 'if' statement can be simplified
IDE0045 Microsoft.Testing.Platform DotnetTestDataConsumer.cs:294 'if' statement can be simplified
(×3 TFMs each)

🤖 Generated by the Build Failure Analysis workflow using (a href="(dev.azure.com/redacted) · commit 07c09cb

🤖 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 Build Failure Analysis workflow. · 225.1 AIC · ⌖ 13.8 AIC · ⊞ 46.9K · [◷]( · )

@Evangelink Evangelink left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 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 Build Failure Analysis workflow. · 225.1 AIC · ⌖ 13.8 AIC · ⊞ 46.9K ·

@Evangelink Evangelink marked this pull request as ready for review June 24, 2026 18:18
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@Evangelink Evangelink left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

✅ 21/22 dimensions clean (7 N/A: #10 Test Isolation, #11 Assertion Quality, #12 Flakiness, #14 Data-Driven Coverage, #18 Analyzer Quality, #20 Build Infrastructure, #22 PowerShell Hygiene).

  • Code Structure — switch expressions would be more idiomatic than if/else chains per project conventions ("use pattern matching and switch expressions wherever possible"). No logic or correctness issues.

Logic equivalence verified: The three-way branch is preserved exactly. The artifactsOverflow/traitsOverflow lists are always initialized with firstArtifact/firstTrait as the first element (see lines 257, 268), so [.. overflow] correctly includes the single-item case when an overflow list exists.

@Evangelink Evangelink enabled auto-merge (squash) June 24, 2026 18:43
@Evangelink Evangelink added the state/needs-review Awaiting review from the team. label Jun 24, 2026
@Evangelink Evangelink closed this Jun 24, 2026
auto-merge was automatically disabled June 24, 2026 18:43

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-review Awaiting review from the team. type/automation Created or maintained by an agentic workflow. type/tech-debt Code health, refactoring, simplification.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants