Apply structured assertion messages to IsTrue, IsFalse, IsNull, IsNotNull by Evangelink · Pull Request #8187 · microsoft/testfx

Evangelink · 2026-05-13T16:21:26Z

Summary

Applies the structured assertion message infrastructure (RFC 012) to the boolean and null assertion methods: IsTrue, IsFalse, IsNull, and IsNotNull.

Changes

Assert.IsTrue / Assert.IsFalse: Use StructuredAssertionMessage with evidence blocks showing expected/actual boolean values and call-site expressions
Assert.IsNull / Assert.IsNotNull: Use StructuredAssertionMessage with evidence blocks showing the actual value and call-site expressions
Assert.cs: Added FormatCallSiteExpression helper methods for formatting call-site expressions in structured messages
Tests: Updated existing tests to verify the new structured message format

Context

This builds on the structured assertion message infrastructure merged in the parent PR, extending it to the first set of assertion methods (boolean and null checks).

Introduce the foundational types and helpers for structured multi-line assertion failure messages as described in RFC 012: - EvidenceLine: labeled line record struct for evidence blocks - EvidenceBlock: collection of labeled lines with automatic alignment - StructuredAssertionMessage: builder producing the new multi-line format (prefix + summary + user message + evidence block + call-site) - AssertionValueRenderer: renders values per RFC 012 rules (null, quoted strings with escape sequences, booleans, collections as JSON arrays) - AssertFailedException: add ExpectedText/ActualText public properties - Assert: add ReportAssertFailed/ThrowAssertFailed overloads accepting StructuredAssertionMessage No existing assertion methods are changed yet - this PR only introduces the infrastructure that subsequent PRs will use to migrate each assertion method to the new format.

…Null - Update Assert.IsTrue/IsFalse to use StructuredAssertionMessage with evidence block showing actual value and call-site expression - Update Assert.IsNull to use StructuredAssertionMessage with evidence block showing actual value - Update Assert.IsNotNull to use StructuredAssertionMessage without evidence block (actual is always null per RFC) - Update interpolated string handlers to store condition/value for passing to the new reporting methods - Add FormatCallSiteExpression helper to Assert.cs for formatting call-site display lines - Remove unused BuildUserMessageForConditionExpression method - Update all related test expectations to match new message format

# Conflicts: # src/TestFramework/TestFramework/Assertions/Assert.cs # src/TestFramework/TestFramework/Assertions/AssertionValueRenderer.cs # src/TestFramework/TestFramework/Assertions/EvidenceBlock.cs # src/TestFramework/TestFramework/Assertions/StructuredAssertionMessage.cs # test/UnitTests/TestFramework.UnitTests/Assertions/AssertFailedExceptionTests.cs # test/UnitTests/TestFramework.UnitTests/Assertions/AssertionValueRendererTests.cs # test/UnitTests/TestFramework.UnitTests/Assertions/StructuredAssertionMessageTests.cs

Copilot

Pull request overview

Extends the RFC 012 structured assertion message infrastructure to the core boolean and null assertions (IsTrue, IsFalse, IsNull, IsNotNull) in MSTest’s TestFramework, and updates unit tests to validate the new multi-line message format.

Changes:

Updated Assert.IsTrue / Assert.IsFalse to emit StructuredAssertionMessage with an actual: evidence line and a reconstructed call-site expression.
Updated Assert.IsNull / Assert.IsNotNull to emit StructuredAssertionMessage (with IsNotNull omitting evidence per RFC guidance).
Added Assert.FormatCallSiteExpression(...) helper(s) and updated unit tests to assert the new message layouts.

Show a summary per file

File	Description
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.ScopeTests.cs	Updates soft-assert scope tests to assert the new structured failure messages for `IsTrue`/`IsNotNull`.
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.IsTrueTests.cs	Updates `IsTrue`/`IsFalse` tests to validate structured message formatting, including user message behavior.
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.IsNull.cs	Updates `IsNull`/`IsNotNull` tests to validate structured message formatting.
src/TestFramework/TestFramework/Assertions/Assert.IsTrue.cs	Implements structured assertion messages for `IsTrue`/`IsFalse`, including evidence and call-site expression output.
src/TestFramework/TestFramework/Assertions/Assert.IsNull.cs	Implements structured assertion messages for `IsNull`/`IsNotNull`, including the RFC-mandated omission of evidence for `IsNotNull`.
src/TestFramework/TestFramework/Assertions/Assert.cs	Adds call-site expression formatting helper methods used by the updated assertions.

Copilot's findings

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

Evangelink · 2026-05-13T16:30:48Z

+        EvidenceBlock evidence = EvidenceBlock.Create()
+            .AddLine("actual:", actualValue);
+
+        StructuredAssertionMessage structured = new("Expected value to be null.");


THis text should be localized

Fixed. Moved to FrameworkMessages.IsNullFailedSummary.

Evangelink · 2026-05-13T16:30:58Z

+    private static void ReportAssertIsNotNullFailed(string? message, string valueExpression)
+    {
+        // RFC: IsNotNull omits the evidence block since actual is always null
+        StructuredAssertionMessage structured = new("Expected value to not be null.");


This should be localized

Fixed. Moved to FrameworkMessages.IsNotNullFailedSummary.

Evangelink · 2026-05-13T16:31:11Z

+        EvidenceBlock evidence = EvidenceBlock.Create()
+            .AddLine("actual:", actualValue);
+
+        StructuredAssertionMessage structured = new("Expected condition to be true.");


This should be localized

Fixed. Moved to FrameworkMessages.IsTrueFailedSummary.

Evangelink · 2026-05-13T16:31:26Z

+        EvidenceBlock evidence = EvidenceBlock.Create()
+            .AddLine("actual:", actualValue);
+
+        StructuredAssertionMessage structured = new("Expected condition to be false.");


This should be localized

Fixed. Moved to FrameworkMessages.IsFalseFailedSummary.

…ions

Evangelink · 2026-05-13T16:53:34Z

Code Review: PR #8187 — Design-Level Concerns

🔴 Backward Compatibility (BLOCKING)

The assertion failure message format changes completely and unconditionally:

	Before	After
Prefix	`Assert.IsNull failed.`	`Assertion failed.`
Structure	Single line	Multi-line with `\n\n`
Content	Optional user message appended	Structured blocks: summary / user-msg / evidence / call-site

Concrete breakages for existing consumers upgrading to this version:

Any code doing ex.Message.Contains("Assert.IsNull failed") → now fails silently (returns false)
Any CI pipeline, test result parser, or log aggregator matching on the "Assert.Xxx failed." prefix → misses assertion failures
Snapshot / approval tests capturing AssertFailedException.Message exactly → all fail
Any Regex written against the old single-line format → broken by embedded \n\n

There is no opt-in/opt-out mechanism and RFC 012 (docs/RFCs/012-Structured-Assertion-Messages.md) is marked - [ ] Approved in principle (unchecked) — i.e., the RFC has not been formally approved yet.

Recommendation: Before merging, either:

Gate the new format behind an opt-in (e.g., .runsettings key or AssertionOptions.UseStructuredMessages = true), defaulting to the legacy format, or
Explicitly document this as a breaking change in the release notes and ensure RFC 012 is formally approved first

i️ Localization Note (Known Tradeoff)

Strings like "Expected value to be null.", "Expected condition to be true.", "actual:" are hardcoded English literals, diverging from the .resx / .xlf pipeline used by all existing assertion messages. This appears to be an intentional design decision per RFC 012 (structured messages use hardcoded English). If localization of these messages is in scope for a future RFC, it should be tracked explicitly.

Generated by Expert Code Review (on open) for issue #8187 · ● 100M · ◷

Evangelink · 2026-05-13T16:53:35Z

[MAJOR] Localization regression — structured message strings are hardcoded English

The PR hardcodes four user-facing summary strings directly in C# source:

String	File
`"Expected value to be null."`	`Assert.IsNull.cs`
`"Expected value to not be null."`	`Assert.IsNull.cs`
`"Expected condition to be true."`	`Assert.IsTrue.cs`
`"Expected condition to be false."`	`Assert.IsTrue.cs`

Plus the evidence label "actual:" (also hardcoded in both files).

The existing assertion messages ("Assert.IsNull failed." etc.) were routed through FrameworkMessages.resx and localized via satellite assemblies into every supported language. After this PR, a user running MSTest under es-ES or fr-FR will receive the English strings above instead of their previously-localized messages.

The repo guidelines explicitly require: "Add a corresponding entry in the resource file (.resx). NEVER manually modify *.xlf files. Instead, build the project to automatically generate the corresponding *.xlf files."

Recommendation: Either:

Add the five strings to FrameworkMessages.resx and reference them as FrameworkMessages.*, or
Explicitly document in RFC 012 that structured messages are intentionally English-only (for machine-parsability) and call this out as a known localization regression in the PR description — so translators can be notified and the decision is deliberate rather than accidental.

Generated by Expert Code Review (on open) for issue #8187 · ● 100M · ◷

Evangelink

Review Summary

#	Dimension	Verdict
1	Algorithmic Correctness	🟡 1 MODERATE
2	Threading & Concurrency	✅ LGTM
3	Security & IPC	✅ LGTM
4	Public API & Binary Compatibility	✅ LGTM (struct field additions are safe in .NET)
5	Performance & Allocations	✅ LGTM (all new alloc on failure path only)
6	Cross-TFM Compatibility	✅ LGTM
7	Resource & IDisposable	✅ LGTM
8	Defensive Coding	✅ LGTM
9	Localization	⚠️ Known tradeoff per RFC 012 (hardcoded English, intentional)
10	Backward Compatibility	🔴 BLOCKING
11	Assertion Quality	✅ LGTM
12	Test Completeness	✅ LGTM
13	Data-Driven Test Coverage	✅ LGTM
14	Code Structure	✅ LGTM (minor duplication is acceptable)
15	Naming & Conventions	✅ LGTM
16	Documentation Accuracy	✅ LGTM
17	Scope & PR Discipline	✅ LGTM

✅ 15/17 dimensions clean.

Blocking

Backward Compatibility: The assertion failure message format changes completely and unconditionally — no opt-in, no feature flag, RFC 012 not yet formally approved. Consumers doing message matching (Contains("Assert.IsNull failed")), log parsers, or snapshot tests will break silently on upgrade. Needs either an opt-in mechanism or explicit RFC approval + release-notes breaking-change documentation.

Moderate

WithExpectedAndActual(null, actualValue) in IsTrue/IsFalse: Sets AssertFailedException.ExpectedText = null, incorrectly signaling "no expected value." The expected values are true and false. Fix: pass AssertionValueRenderer.RenderValue(true/false) as the first argument.

Generated by Expert Code Review (on open) for issue #8187 · ● 100M

…r multiline expressions

Copilot

Copilot's findings

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

… null

…press CA1507 for IsNotNull

Copilot

Copilot's findings

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

+    private static void ReportAssertIsNotNullFailed(string? message, string valueExpression)
+    {
+        // RFC: IsNotNull omits the evidence block since actual is always null
+        StructuredAssertionMessage structured = new(FrameworkMessages.IsNotNullFailedSummary);
+        structured.WithUserMessage(message);
+#pragma warning disable CA1507 // Use nameof - 'value' is the parameter name of the public IsNotNull method, not available in this scope
+        structured.WithCallSiteExpression(FormatCallSiteExpression("Assert.IsNotNull", valueExpression, "value"));
+#pragma warning restore CA1507
+


+        StructuredAssertionMessage structured = new(FrameworkMessages.IsNotNullFailedSummary);
+        structured.WithUserMessage(message);
+#pragma warning disable CA1507 // Use nameof - 'value' is the parameter name of the public IsNotNull method, not available in this scope
+        structured.WithCallSiteExpression(FormatCallSiteExpression("Assert.IsNotNull", valueExpression, "value"));
+#pragma warning restore CA1507


+    {
+        // RFC: IsNotNull omits the evidence block since actual is always null
+        StructuredAssertionMessage structured = new(FrameworkMessages.IsNotNullFailedSummary);


Evangelink added 3 commits May 12, 2026 18:06

Copilot AI review requested due to automatic review settings May 13, 2026 16:21

Copilot started reviewing on behalf of Evangelink May 13, 2026 16:22 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

Comment thread src/TestFramework/TestFramework/Assertions/Assert.cs Outdated

Evangelink commented May 13, 2026

View reviewed changes

Use raw string literals instead of Environment.NewLine in test assert…

7df3617

…ions

Evangelink commented May 13, 2026

View reviewed changes

Evangelink mentioned this pull request May 13, 2026

[aw] Expert Code Review (on open) failed #8190

Closed

Address PR review: localize assertion messages and use placeholder fo…

b0af4de

…r multiline expressions

Copilot AI review requested due to automatic review settings May 13, 2026 16:55

Copilot started reviewing on behalf of Evangelink May 13, 2026 16:56 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

Comment thread src/TestFramework/TestFramework/Assertions/Assert.cs Outdated

Comment thread src/TestFramework/TestFramework/Assertions/Assert.IsNull.cs

Evangelink added 2 commits May 13, 2026 19:02

Fix WithExpectedAndActual to pass rendered expected values instead of…

d0d7e7e

… null

Fix cross-TFM compat (IndexOf vs Contains), remove dead overload, sup…

956e9bb

…press CA1507 for IsNotNull

Copilot AI review requested due to automatic review settings May 13, 2026 17:10

Copilot started reviewing on behalf of Evangelink May 13, 2026 17:10 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

Copilot AI mentioned this pull request May 13, 2026

fix(review-workflow): prevent context overflow in Expert Code Review workflow #8199

Merged

Conversation

Evangelink commented May 13, 2026

Summary

Changes

Context

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Evangelink commented May 13, 2026

Code Review: PR #8187 — Design-Level Concerns

🔴 Backward Compatibility (BLOCKING)

i️ Localization Note (Known Tradeoff)

Uh oh!

Evangelink commented May 13, 2026

Uh oh!

Evangelink left a comment

Choose a reason for hiding this comment

Review Summary

Blocking

Moderate

Uh oh!

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!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants