Report MSTest assertion expected/actual through a platform property

[Amaury Levé (Evangelink)]

Summary

The terminal's Expected: / Actual: block and the assert.expected / assert.actual protocol fields have been dead code for MSTest. Both paths that build a failed TestNode replace the original assertion exception with a synthetic one carrying only a message and stack trace string (MSTestTestNodeException, VSTestException), and the only channel for the structured values was Exception.Data, which those synthetic exceptions do not have. So all five consumers read null and MSTest users never saw the diff block, even though the renderer was fully implemented.

Verified with an A/B run of samples/Playground on Assert.AreEqual(5, 2):

 Assert.AreEqual(5, 2) <- before: block absent
Expected <- after
5
Actual
2

Approach

Add an AssertionFailureProperty to Microsoft.Testing.Platform so the values are a first-class property instead of riding on Exception.Data:

• Capture - MSTest's TestResult records the values as strings when TestFailureException is assigned, for the same reason ExceptionMessage / ExceptionStackTrace are recorded: the exception instance does not survive an AppDomain boundary and the adapter reports from these strings. The chain is walked because the adapter always wraps the assertion in TestFailedException, so the outermost exception is never the one holding the values.
• Produce - MSTestTestNodeConverter publishes the property alongside the failed state.
• Consume - the terminal, the MSBuild extension, the dotnet test pipe and both server-mode serializers prefer the property and fall back to Exception.Data for producers that have not been updated. The choice is all-or-nothing per node so the two halves of a diff always come from the same producer.

The wire format is deliberately unchanged: same protocol keys, same key ordering, same pipe field ids, so no protocol doc or golden-string churn. What the property adds is the ability to emit those keys when there is no exception at all, which Exception.Data structurally cannot express (relevant for FailedTestNodeStateProperty(string explanation), added in #2536 precisely so frameworks can fail without an exception).

Reporting policy worth a careful look

Values are reported only when the result contains exactly one assertion failure carrying a comparison:

Assertion failures carrying expected/actual	Behaviour
0	no property
1	reported, regardless of how many other failures occurred
2+	suppressed; per-assertion values remain in the failure message

Assert.Scope() throws a single AssertFailedException wrapping an AggregateException of every collected failure. Since assert.expected / assert.actual carry no label, two competing comparisons cannot both be reported and whichever survived would look authoritative next to a message saying "N assertion(s) failed". The count is accumulated across the whole result because TestFailureException can be assigned more than once (TestInitialize plus TestCleanup).

Failures that carry no comparison (Assert.Fail, or any non-assertion exception) deliberately do not count, so a cleanup Assert.Fail does not hide the body's diff while a cleanup throw would not. Note Assert.IsTrue / IsFalse do carry a comparison (true / false), so a scope mixing AreEqual and IsTrue is suppressed.

A list-shaped property that could carry N diffs properly is the better long-term answer, but it is a much larger public API surface and is out of scope here.

Notes for reviewers

• The VSTest bridge path still loses the values; it would need a VSTest-side carrier to benefit.
• Both serializers gained a property lookup, gated behind the O(1)SingleOrDefault<TestNodeStateProperty>() state check so discovery, in-progress and passed nodes never pay for a linked-list walk.
• SerializerUtilities.TestNodeSerializers.cs picked up a UTF-8 BOM on line 1 (the file had none). That matches the repo's stated encoding policy for rewritten .cs files, but happy to drop it if the unrelated diff line is unwanted.

Validation

Full build clean. Unit tests: Microsoft.Testing.Platform.UnitTests 1940 on net9.0 and 1899 on net462 (covering both the System.Text.Json and Jsonite serializers), TestFramework.UnitTests 1502, MSTestAdapter.PlatformServices.UnitTests 1062, Microsoft.Testing.Extensions.UnitTests 848, MSTestAdapter.UnitTests 64, Microsoft.Testing.Platform.MSBuild.UnitTests 20.

Acceptance tests after -pack: OutputTests and SoftAssertionTests 13/13 across net462, net8.0 and net10.0, including new guards that the block is rendered for a single-failure scope and is not rendered for a multi-failure one.

[Copilot]

Pull request overview

Adds structured MSTest assertion values to MTP so reporters can display expected/actual diffs without relying on Exception.Data.

Changes:

• Introduces and publishes AssertionFailureProperty.
• Updates terminal, MSBuild, pipe, and JSON consumers.
• Adds unit and acceptance coverage for assertion reporting.

Show a summary per file

File	Description
test/UnitTests/TestFramework.UnitTests/TestResultTests.cs	Tests assertion extraction and ambiguity rules.
test/UnitTests/MSTestAdapter.UnitTests/MSTestTestNodeConverterTests.cs	Tests property publication by MSTest.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs	Tests JSON assertion fields.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/DotnetTestDataConsumerTests.cs	Tests pipe consumer extraction.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Messages/AssertionFailurePropertyTests.cs	Tests the new property API.
test/IntegrationTests/MSTest.Acceptance.IntegrationTests/SoftAssertionTests.cs	Verifies scoped assertion output.
test/IntegrationTests/MSTest.Acceptance.IntegrationTests/OutputTests.cs	Verifies terminal diff rendering.
src/TestFramework/TestFramework/InternalAPI/InternalAPI.Unshipped.txt	Tracks new internal result members.
src/TestFramework/TestFramework/Attributes/TestMethod/TestResult.cs	Captures unambiguous assertion values.
src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.TestNodeSerializers.cs	Serializes property values through Jsonite.
src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.TestNodeSerializer.cs	Serializes property values through System.Text.Json.
src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/DotnetTestDataConsumer.cs	Forwards values through the pipe protocol.
src/Platform/Microsoft.Testing.Platform/PublicAPI/PublicAPI.Unshipped.txt	Tracks the new public property.
src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.DataConsumption.cs	Supplies values to terminal reporting.
src/Platform/Microsoft.Testing.Platform/Messages/AssertionFailureProperty.cs	Defines the structured assertion property.
src/Platform/Microsoft.Testing.Extensions.MSBuild/MSBuildConsumer.cs	Supplies values to MSBuild reporting.
src/Adapter/MSTest.TestAdapter/TestingPlatformAdapter/MSTestTestNodeConverter.cs	Publishes captured MSTest values.

Review details

Suppressed comments (1)

src/TestFramework/TestFramework/Attributes/TestMethod/TestResult.cs:265

• The depth cutoff can make a multi-comparison result look unambiguous. For example, if one aggregate branch contains a shallow comparison and another comparison remains beyond MaxDepth, the latter is ignored and this method returns 1, so the shallow pair is published as authoritative. Reaching the cutoff with an unvisited exception should conservatively mark the result ambiguous (or otherwise suppress the pair).

 exception = exception.InnerException;
depth++;

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

[Copilot]

Review details

Suppressed comments (1)

src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.DataConsumption.cs:260

• When duplicate assertion properties are encountered, the loop deliberately sets assertionFailure to null while leaving hasAssertionFailure true. These checks then fall back to Exception.Data, so a malformed node can still render a legacy diff instead of suppressing the ambiguous values. Gate the fallback on whether any assertion property was seen.

 expected: assertionFailure is not null ? assertionFailure.Expected : failedState.Exception?.Data["assert.expected"] as string,
actual: assertionFailure is not null ? assertionFailure.Actual : failedState.Exception?.Data["assert.actual"] as string,

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

[github-actions]

🤖 Automated content by GitHub Copilot. Generated by the Grade Tests on PR (on open / sync) workflow. · sonnet46 150.2 AIC · ⌖ 4.04 AIC · ⊞ 11.8K · ◷

[Copilot]

Review details

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

[Amaury Levé (Evangelink)]

Picking up the suppressed comment from the latest review, since it found a real bug and has no thread to reply on:

When duplicate assertion properties are encountered, the loop deliberately sets assertionFailure to null while leaving hasAssertionFailure true. These checks then fall back to Exception.Data, so a malformed node can still render a legacy diff instead of suppressing the ambiguous values. Gate the fallback on whether any assertion property was seen.

Correct, and it defeated the suppression I added in c7a7d82: a duplicate resolved the local to null, and the consumption site tested that null, so it fell straight through to the legacy Exception.Data values - rendering exactly the arbitrary diff the suppression exists to prevent.

Fixed in 9b6c7ec by gating on whether a property was seen rather than on the resolved value:

expected:hasAssertionFailure?assertionFailure?.Expected:failedState.Exception?.Data["assert.expected"]asstring,

Only the terminal path needed this. The other four consumers reject duplicates outright via SingleOrDefault-equivalent checks, so they can never reach the suppressed-null state.

[Copilot]

Review details

• Files reviewed: 17/17 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

Review details

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

[github-actions]

🧪 Test quality grade — PR #10353

Grade	Test	Mutation	Notes	How to improve
A (90–100)	new AssertionFailurePropertyTests. Constructor_ AllowsEmptyStrings	2/2 killed	Verifies empty strings are stored as-is without triggering the null-rejection guard.	—
A (90–100)	new AssertionFailurePropertyTests. Constructor_ WhenBothValuesAreNull_ Throws	1/1 killed	Exception assertion is self-contained; removing the guard causes the test to fail.	—
A (90–100)	new AssertionFailurePropertyTests. Constructor_ WhenOnlyActualIsProvided_ KeepsExpectedNull	2/2 killed	Checks both fields; swapping assignments or nulling either would fail an assertion.	—
A (90–100)	new AssertionFailurePropertyTests. Constructor_ WhenOnlyExpectedIsProvided_ KeepsActualNull	2/2 killed	Checks both fields; swapping assignments or nulling either would fail an assertion.	—
A (90–100)	new AssertionFailurePropertyTests. Equals_ WhenDifferentValues_ ReturnsFalse	2/2 killed	Two distinct AreNotEqual calls cover removing either field comparison from Equals.	—
A (90–100)	new AssertionFailurePropertyTests. Equals_ WhenOtherIsNullOrDifferentType_ ReturnsFalse	2/2 killed	Null guard and type guard each get their own assertion.	—
A (90–100)	new AssertionFailurePropertyTests. Equals_ WhenSameValues_ ReturnsTrue	3/3 killed	AreEqual and GetHashCode consistency both verified; always-false Equals mutation caught.	—
A (90–100)	new AssertionFailurePropertyTests. ToStringIsCorrect	4/4 killed	Exact string equality catches field swaps, separator changes, and casing mutations.	—
A (90–100)	new DotnetTestDataConsumerTests. GetTestNodeDetails_ WhenAssertionFailurePropertyIsOneSided_ DoesNotSpliceInExceptionData	2/2 killed	Expected value and null Actual both asserted; splicing legacy Actual would fail IsNull.	—
A (90–100)	new DotnetTestDataConsumerTests. GetTestNodeDetails_ WhenFailedWithAssertionFailureProperty_ PopulatesExpectedAndActual	3/3 killed	State, Expected, and Actual all asserted; omitting property extraction fails all three.	—
A (90–100)	new DotnetTestDataConsumerTests. GetTestNodeDetails_ WhenFailedWithAssertionFailureProperty_ PrefersPropertyOverExceptionData	2/2 killed	AreEqual on property values; returning legacy values ("legacy expected"/"legacy actual") fails both.	—
A (90–100)	new FormatterUtilitiesTests. Serialize_ FailedTestNodeWithAssertionFailureProperty_ EmitsAssertValues	4/4 killed	Exact JSON equality catches key renames, value swaps, and missing fields.	—
A (90–100)	new FormatterUtilitiesTests. Serialize_ FailedTestNodeWithAssertionFailureProperty_ PrefersPropertyOverExceptionData	3/3 killed	Property values asserted and DoesNotContain("legacy") proves the property won outright.	—
A (90–100)	new FormatterUtilitiesTests. Serialize_ OneSidedAssertionFailureProperty_ DoesNotSpliceInExceptionData	3/3 killed	Empty-string actual asserted; DoesNotContain("legacy") blocks any splice from Exception.Data.	—
A (90–100)	new MSTestTestNodeConverterTests. ToResultTestNode_ AddsAssertionFailureProperty_ WhenOnlyExpectedTextIsKnown	3/3 killed	NotBeNull, Expected value, and null Actual all asserted; the ||→&& guard mutation is caught because only one side is non-null.	—
A (90–100)	new MSTestTestNodeConverterTests. ToResultTestNode_ AddsAssertionFailureProperty_ WhenResultCarriesAssertionTexts	3/4 killed	NotBeNull, Expected, and Actual asserted; ||→&& mutation survives here (both non-null) but is caught by the sibling one-sided test.	—
A (90–100)	new MSTestTestNodeConverterTests. ToResultTestNode_ DoesNotAddAssertionFailureProperty_ WhenFailureIsNotAnAssertion	2/2 killed	BeFalse directly verifies the guard suppresses the property for non-assertion failures.	—
A (90–100)	mod OutputTests. DetailedOutputIsAsExpected	3/3 killed	Existing assertions plus new regex that specifically requires the Expected/Actual block to render.	—
A (90–100)	mod SoftAssertionTests. ScopeWithMultipleFailures_ TestFailsWithAggregatedMessage	2/2 killed	DoesNotMatchRegex ensures the structured block is suppressed when multiple assertions fail.	—
A (90–100)	mod SoftAssertionTests. ScopeWithSingleFailure_ TestFails	3/3 killed	Exit code, failure details, and new structured Expected/Actual block all verified.	—
A (90–100)	new TestResultTests. SettingTestFailureExceptionAfterAMultiFailureScopeShouldNotCaptureAssertionTexts	2/2 killed	Saturated count from scope blocks a subsequent single assertion from claiming the diff.	—
A (90–100)	new TestResultTests. SettingTestFailureExceptionAfterAValueLessAssertionShouldCaptureAssertionTexts	2/2 killed	Valueless first failure must not consume a comparison slot; values asserted after second set.	—
A (90–100)	new TestResultTests. SettingTestFailureExceptionShouldCaptureAssertionTextsFromAggregateException	2/2 killed	Verifies InnerExceptions walk on AggregateException; not walking would leave both null.	—
A (90–100)	new TestResultTests. SettingTestFailureExceptionShouldCaptureAssertionTextsFromDirectAssertFailedException	2/2 killed	Direct assignment path; not extracting either field causes the corresponding assertion to fail.	—
A (90–100)	new TestResultTests. SettingTestFailureExceptionShouldCaptureAssertionTextsFromWrappedAssertFailedException	2/2 killed	Two-level wrapping; stopping at outermost exception leaves both values null.	—
A (90–100)	new TestResultTests. SettingTestFailureExceptionShouldCaptureAssertionTextsWhenASecondAssertionCarriesNoValues	2/2 killed	Valueless second assertion must not increment the count; incorrectly counting it suppresses the diff.	—
A (90–100)	new TestResultTests. SettingTestFailureExceptionShouldCaptureAssertionTextsWhenScopeReportsASingleFailure	2/2 killed	Single-exception direct path; both fields checked independently.	—
A (90–100)	new TestResultTests. SettingTestFailureExceptionShouldNotCaptureAssertionTextsForNonAssertionFailure	2/2 killed	Non-assertion exception must yield null texts; erroneous extraction fails both BeNull checks.	—
A (90–100)	new TestResultTests. SettingTestFailureExceptionShouldNotCaptureAssertionTextsWhenScopeReportsMultipleFailures	2/2 killed	Two inner assertions with values; removing the count>1 suppression exposes both texts.	—
A (90–100)	new TestResultTests. SettingTestFailureExceptionShouldStopWalkingBeyondBoundedChainDepth	2/2 killed	15-deep chain beyond MaxDepth=10; removing the depth bound or raising it past 15 finds the assertion.	—
A (90–100)	new TestResultTests. SettingTestFailureExceptionTwiceShouldKeepAssertionTextsWhenSecondFailureIsNotAnAssertion	2/2 killed	Non-assertion second failure must not clear texts; erroneous clearing fails both Be assertions.	—
A (90–100)	new TestResultTests. SettingTestFailureExceptionTwiceWithTwoAssertionsShouldDropAmbiguousAssertionTexts	2/2 killed	Two distinct assertion failures; not accumulating the count leaves a surviving text pair exposed.	—

_{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. Generated by the Grade Tests on PR (on open / sync) workflow. · sonnet46 134.9 AIC · ⌖ 8.3 AIC · ⊞ 11.7K · [◷]( · ◷)