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 a summary stating the expected boolean (e.g. Expected condition to be true.), an evidence block showing the actual: value, and a call-site expression. Expected/actual metadata is also exposed via WithExpectedAndActual for runner consumption.
Assert.IsNull / Assert.IsNotNull: Use StructuredAssertionMessage with a summary, an evidence block showing the actual: value, and a call-site expression. Expected/actual metadata is also exposed via WithExpectedAndActual.
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

…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

- Populate WithExpectedAndActual("not null", "null") and add an `actual: null` evidence block in ReportAssertIsNotNullFailed so the failure output matches the PR description and ExpectedText/ActualText are surfaced for tooling. - Plumb a paramName argument through ReportAssertIsNotNullFailed; the public IsNotNull overloads now pass nameof(value), removing the CA1507 pragma. - Update unit and acceptance test expectations to match the new structured failure output (blank line before evidence; new "Assertion failed. Expected value to not be null." prefix).

- Add [DoesNotReturn] to ReportAssertIsTrueFailed and ReportAssertIsNullFailed for parity with their IsFalse/IsNotNull siblings. - Use angle-bracket placeholder convention "<not-null>" for ExpectedText in IsNotNull failures (consistent with the <paramName> token style used elsewhere; there is no C# literal for "not null").

Copilot

Copilot's findings

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

Per Copilot review feedback: the angle-bracket form `<not-null>` can be confused with markup and requires escaping in XML outputs like TRX. Use the plain `"not null"` string instead. Also updates the PR description to accurately describe the implemented evidence block content (only `actual:` is shown; expected is in the summary line and `WithExpectedAndActual`).

Repo enforces StyleCop SA1518 (file must end with a single newline). Adding the new resource entries had inadvertently removed the trailing CRLF; restore it.

Copilot

Copilot's findings

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

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