Skip to content

Preserve inner exceptions in server mode - #11221

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/fix-server-mode-inner-exceptions
Sep 12, 2026
Merged

Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/fix-server-mode-inner-exceptions

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Server-mode JSON-RPC currently sends only the outer exception message and stack trace, so IDE clients lose the underlying failure details that terminal and dotnet test output already preserve.

This change keeps the existing protocol shape while flattening exception details into error.message and error.stacktrace for failed, error, timed-out, and canceled test nodes. Both the System.Text.Json and Jsonite serializers use the same formatting helper, backed by the existing ExceptionFlattener. The source-only server client package now includes that shared helper as well.

Tests cover nested exceptions across all affected states and multi-branch AggregateException output on both serializer implementations.

Validation:

  • Microsoft.Testing.Platform.UnitTests FormatterUtilitiesTests: 84/84 passed on net8.0
  • Microsoft.Testing.Platform.UnitTests FormatterUtilitiesTests: 84/84 passed on net462
  • Microsoft.Testing.Platform.ServerMode.Client.Sources builds successfully

Fixes: #11202

Flatten exception chains into the existing JSON-RPC error fields across both serializer implementations and cover aggregate exceptions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 12, 2026 09:09
@github-actions

This comment has been minimized.

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.

Copilot review overview

🟡 Changes recommended

The new internal helper is missing from InternalAPI.Unshipped.txt, causing RS0016.

Get a fresh assessment by requesting another Copilot review.

Review tier: Balanced (auto)
Findings: 1 High severity

Note

Copilot is running an experiment and ran this review at Balanced.

Open findings (1)
What changed in this PR

Preserves nested exception details in server-mode JSON-RPC responses across both serializers and the source-only client package.

Changes:

  • Flattens nested and aggregate exception messages and stack traces.
  • Applies formatting to failed, error, timed-out, and canceled states.
  • Adds cross-serializer tests.
File Description
FormatterUtilitiesTests.cs Tests nested and aggregate exceptions.
SerializerUtilities.TestNodeSerializers.cs Adds shared exception formatting.
Json.TestNodeSerializer.cs Uses shared formatting for System.Text.Json.
Microsoft.Testing.Platform.ServerMode.Client.Sources.csproj Links the exception flattener into the source package.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Add the new internal SerializerUtilities helper to the unshipped API baseline.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 12, 2026 09:19
@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #11221

Parallelization — one row per test assembly audited:

Test assembly Scope Workers Analyzer coverage
Microsoft.Testing.Platform.UnitTests MethodLevel 0 (CPU count) coverable once the parallel-safety analyzers ship ([assembly: Parallelize] in Program.cs)

Findings: A (global-state) 0 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 0.

The PR only touches test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs (two new test methods, Serialize_ExceptionStates_IncludeInnerExceptionMessagesAndStackTraces and FormatException_AggregateException_IncludesEveryBranch, plus a private EscapeJsonString helper and a private nested FixedStackTraceException type) and the production exception-formatting code it exercises (SerializerUtilities.TestNodeSerializers.cs, Json.TestNodeSerializer.cs).

Both new tests are self-contained and deterministic:

  • They construct local exceptions/TestNode instances and call SerializerUtilities.Serialize, SerializerUtilities.FormatException, and the instance _formatter.SerializeAsync — all pure, stateless transformations over their own inputs.
  • _formatter is a per-instance field (MSTest constructs a fresh FormatterUtilitiesTests instance per test), not static, so it is not shared across tests.
  • No environment variables, current directory, culture, console state, registry, AppContext, or static fields are read or written.
  • No filesystem paths are constructed or touched.
  • No [ResourceLock] / [DoNotParallelize] / [Parallelize] declarations were added, removed, or otherwise touched, and no .runsettings / testconfig.json / Directory.Build.props parallelization settings changed.

No parallel-safety issues to flag under the detected MethodLevel scope.

Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.

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

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.

Copilot review overview

🟢 Approval recommended

The serializers, linked-source package, API baseline, and affected failure states are consistently covered.

Review tier: Balanced (auto)
Findings: None

Note

Copilot is running an experiment and ran this review at Balanced.

Resolved findings (1)

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #11221

GradeTestMutationNotesHow to improve
A (90–00) new FormatterUtilitiesTests.
FormatException_
AggregateException_
IncludesEveryBranch
4/4 killed Directly exercises FormatException with a two-branch AggregateException, asserting exact assembled message and stack trace via the actual aggregateException.Message (avoids hardcoding brittle framework text).
A (90–00) new FormatterUtilitiesTests.
Serialize_
ExceptionStates_
IncludeInnerExceptionMessagesAndStackTraces
5/5 killed Covers Failed/Error/Timeout/Canceled state properties with a chained inner exception, verifying both the raw property dictionary and the serialized JSON contain the correctly formatted message and stack trace.

Summary: 2 new test methods reviewed, both grade A. They give solid pseudo-mutation coverage of the new SerializerUtilities.FormatException helper: the multi-exception loop (message/stack-trace concatenation, --- Inner exception stack trace (...) --- header, NewLine joins, starting the loop at index 1) is exercised by both tests, and the AggregateException.Flatten() branch is exercised specifically by the second test. Neither test exercises the exceptions.Length == 1 short-circuit path in FormatException (single exception, no inner exception), but that branch is already covered by pre-existing tests in this file (e.g. Serialize_FailedTestNodeWithAssertionFailureProperty_PrefersPropertyOverExceptionData), so no gap results from the new tests alone. No anti-patterns, structural issues, or reliability concerns were found; no actionable inline suggestions are warranted.

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 · 109.8 AIC · ⌖ 2.46 AIC · ⊞ 15.9K · [◷]( · )

@Evangelink Amaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Sep 12, 2026
@Evangelink
Amaury Levé (Evangelink) merged commit a83c2e6 into main Sep 12, 2026
37 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/fix-server-mode-inner-exceptions branch September 12, 2026 13:51
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Server mode drops inner exceptions: error.message and error.stacktrace carry only the outermost exception

3 participants