Use StructuredAssertionMessage for StringAssert and CollectionAssert membership (#9275)#9297
Use StructuredAssertionMessage for StringAssert and CollectionAssert membership (#9275)#9297Evangelink wants to merge 6 commits into
Conversation
…rloads (#9279) Each Assert.*InterpolatedStringHandler struct must declare the same ~10 AppendLiteral/AppendFormatted overloads because the C# interpolated string handler pattern resolves them via ordinary member lookup on each handler struct (structs cannot inherit them). #9282 centralized the append *logic* into AssertInterpolatedStringHandlerAppender but the ~23 forwarder blocks (~580 lines) remained, near-identical, in a single file. This adds a small source generator (TestFramework.SourceGeneration) that emits those forwarders for any handler annotated with [GenerateAssertInterpolatedStringAppendMethods]. The generated public API is byte-identical to the previous hand-written overloads (verified by the PublicAPI analyzer passing unchanged across netstandard2.0/net462/net8.0/ net9.0), so there is no public API change. The handler files now only declare their state, constructor, and ComputeAssertion logic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rload regression tests - The generator now emits the containing type with its real accessibility/modifiers (e.g. 'public sealed partial class Assert') derived from the symbol, instead of a bare 'partial class'. (Partial declarations may omit modifiers, so the build was already valid, but matching them is clearer and more defensive.) - Added AssertInterpolatedStringHandlerGeneratedOverloadsTests covering each generated AppendLiteral/AppendFormatted overload case (literal, generic value, format, alignment, alignment+format, string, object, and ReadOnlySpan<char>) end-to-end through real failing interpolated assertions, plus cross-handler cases (IsFalse, AreEqual generic+nullable-literal, IsNull) so a handler that lost its marker attribute fails the suite. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…membership (#9275) Migrate the legacy string.Format + FrameworkMessages.*Fail failure path of StringAssert (Contains, StartsWith, EndsWith, Matches, DoesNotMatch) and the CollectionAssert membership methods (Contains, DoesNotContain, AllItemsAreNotNull, AllItemsAreUnique) to the modern StructuredAssertionMessage infrastructure. Failures now expose machine-readable ExpectedText/ActualText and exception.Data entries plus aligned evidence blocks, matching the diagnostics already produced by the equivalent Assert.* methods. All reused summary resources already exist, so no .resx/.xlf or public API changes are needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR modernizes legacy MSTest assertions by routing StringAssert and CollectionAssert membership failures through StructuredAssertionMessage, enabling consistent structured diagnostics (ExpectedText/ActualText and exception.Data["assert.expected"/"assert.actual"]) similar to existing Assert.* APIs. It also introduces an internal source generator to eliminate duplicated interpolated-string-handler AppendLiteral/AppendFormatted boilerplate across Assert.*InterpolatedStringHandler structs.
Changes:
- Migrated
StringAssertsubstring/regex failure paths toStructuredAssertionMessage, with evidence blocks and populated expected/actual metadata. - Migrated
CollectionAssertmembership failures toStructuredAssertionMessage, including duplicate surfacing forAllItemsAreUnique. - Added a new internal source generator project and updated
Assert.*InterpolatedStringHandlerstructs to opt-in via a marker attribute; added regression tests for generated overload sets.
Show a summary per file
| File | Description |
|---|---|
| TestFx.slnx | Adds the new TestFramework.SourceGeneration project to the solution. |
| src/TestFramework/TestFramework/TestFramework.csproj | References the generator project as an analyzer to emit handler append-method overloads at build time. |
| src/TestFramework/TestFramework/Assertions/StringAssert.cs | Converts failure reporting to structured messages for substring and regex assertions. |
| src/TestFramework/TestFramework/Assertions/CollectionAssert.Membership.cs | Converts membership failure reporting to structured messages and adds evidence blocks. |
| test/UnitTests/TestFramework.UnitTests/Assertions/StringAssertTests.cs | Updates message expectations and adds structured expected/actual/data assertions for StringAssert.*. |
| test/UnitTests/TestFramework.UnitTests/Assertions/CollectionAssertTests.cs | Adds structured message/data assertions for CollectionAssert membership methods. |
| test/UnitTests/TestFramework.UnitTests/Assertions/AssertInterpolatedStringHandlerGeneratedOverloadsTests.cs | Adds regression coverage to ensure generated handler overloads compile/bind and render correctly end-to-end. |
| src/TestFramework/TestFramework/Assertions/GenerateAssertInterpolatedStringAppendMethodsAttribute.cs | Adds an internal marker attribute to opt handler structs into source generation. |
| src/TestFramework/TestFramework/Assertions/Assert.InterpolatedStringHandlers.AppendMethods.cs | Removes the duplicated per-handler append-method implementations in favor of generator output. |
| src/TestFramework/TestFramework/Assertions/Assert.IsTrue.cs | Opts handler structs into generated append-method overloads. |
| src/TestFramework/TestFramework/Assertions/Assert.IsNull.cs | Opts handler structs into generated append-method overloads. |
| src/TestFramework/TestFramework/Assertions/Assert.IsInstanceOfType.cs | Opts handler structs into generated append-method overloads. |
| src/TestFramework/TestFramework/Assertions/Assert.IsExactInstanceOfType.cs | Opts handler structs into generated append-method overloads. |
| src/TestFramework/TestFramework/Assertions/Assert.IsEmpty.cs | Opts handler structs into generated append-method overloads. |
| src/TestFramework/TestFramework/Assertions/Assert.Count.InterpolatedStringHandlers.cs | Opts handler structs into generated append-method overloads. |
| src/TestFramework/TestFramework/Assertions/Assert.ContainsSingle.cs | Opts handler structs into generated append-method overloads. |
| src/TestFramework/TestFramework/Assertions/Assert.AreSame.cs | Opts handler structs into generated append-method overloads. |
| src/TestFramework/TestFramework/Assertions/Assert.AreEqual.InterpolatedStringHandlers.cs | Opts handler structs into generated append-method overloads (including nullable literal configuration for the one handler). |
| src/TestFramework/TestFramework/Assertions/Assert.ThrowsException.InterpolatedStringHandlers.cs | Opts handler structs into generated append-method overloads. |
| src/TestFramework/TestFramework.SourceGeneration/TestFramework.SourceGeneration.csproj | Introduces the internal build-time generator project (not packed/shipped). |
| src/TestFramework/TestFramework.SourceGeneration/AssertInterpolatedStringAppendMethodsGenerator.cs | Implements the incremental generator emitting append-method overloads per opt-in handler struct. |
Copilot's findings
- Files reviewed: 21/21 changed files
- Comments generated: 4
| StructuredAssertionMessage structured = new(FrameworkMessages.ContainsItemFailedSummary); | ||
| structured.WithUserMessage(userMessage); | ||
| structured.WithEvidence(evidence); | ||
| structured.WithExpectedAndActual(expectedText, collectionText); | ||
|
|
There was a problem hiding this comment.
Addressed in cc157bf — ReportContainsFailed now calls WithExpectedAndActual(expectedText, null), leaving ActualText unset and keeping the collection only in the evidence block, matching Assert.Contains.
| StructuredAssertionMessage structured = new(FrameworkMessages.DoesNotContainItemFailedSummary); | ||
| structured.WithUserMessage(userMessage); | ||
| structured.WithEvidence(evidence); | ||
| structured.WithExpectedAndActual(notExpectedText, collectionText); | ||
|
|
There was a problem hiding this comment.
Addressed in cc157bf — ReportDoesNotContainFailed now calls WithExpectedAndActual(notExpectedText, null), matching Assert.DoesNotContain.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Evangelink
left a comment
Evangelink
left a comment
There was a problem hiding this comment.
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.
Expert Review — PR #9297: Use StructuredAssertionMessage for StringAssert and CollectionAssert membership
Verdict table
| # | Dimension | Status | Notes |
|---|---|---|---|
| 1 | Algorithmic Correctness | ✅ | Logic for AllItemsAreUnique / DoesNotContain is correct: [DoesNotReturn] helpers throw and unwind the foreach correctly. |
| 2 | Threading & Concurrency | ✅ N/A | No shared state introduced. |
| 3 | Security | ✅ | No path traversal, injection, or leaks. |
| 4 | Public API & Binary Compatibility | ✅ | No new public API surface; GenerateAssertInterpolatedStringAppendMethodsAttribute is internal; no init accessors introduced. |
| 5 | Performance | ✅ | Collection rendering is on the failure path only; AssertionValueRenderer already exists for this. |
| 6 | Cross-TFM Compatibility | ✅ | #if NETCOREAPP3_1_OR_GREATER guards the ReadOnlySpan<char> overloads in generated code; generator targets netstandard2.0. |
| 7 | Resource & IDisposable | ✅ N/A | |
| 8 | Defensive Coding | ✅ | |
| 9 | Localization & Resources | ✅ | All new summary strings are in FrameworkMessages.resx; no .xlf manually edited; no hard-coded user-facing strings. |
| 10 | Test Isolation | ✅ | No static mutable state. |
| 11 | Assertion Quality | See inline comment — comparison: evidence line not tested. |
|
| 12 | Flakiness | ✅ | No time-dependent or port-dependent patterns. |
| 13 | Test Completeness | See inline comment — comparison: line gap. |
|
| 14 | Data-Driven Tests | ✅ N/A | |
| 15 | Code Structure | ✅ | Extraction of Report*Failed helpers is clean; [DoesNotReturn] is correctly applied. |
| 16 | Naming & Conventions | ✅ | Naming consistent with existing ReportXxxFailed pattern. |
| 17 | Documentation | ✅ | Generator has a clear XML doc comment; attribute has an accurate doc comment. |
| 18 | Analyzer / Code Fix Quality | ✅ N/A | |
| 19 | IPC Wire Compatibility | ✅ N/A | |
| 20 | Build Infrastructure | TestFramework.SourceGeneration.csproj missing from MSTest.slnf and NonWindowsTests.slnf — see inline comment. |
|
| 21 | Scope & PR Discipline | Two distinct concerns (source generator + structured messages) in one PR, explicitly acknowledged in the description. See note below. | |
| 22 | PowerShell Scripting | ✅ N/A |
Summary
The core migration from string.Format + FrameworkMessages.*Fail to StructuredAssertionMessage is clean, consistent, and follows the established pattern from the existing Assert.* methods. The structured evidence blocks for StringAssert and CollectionAssert.Membership are well-chosen and the ExpectedText / ActualText / Data["assert.expected"|"assert.actual"] population is verified by the new tests.
Three actionable findings (see inline comments):
-
Generator null-safety (MODERATE) —
GetHandlerInfodoes not guard against a top-level (non-nested) struct annotated with the attribute.structSymbol.ContainingTypeis nullable, and passingnulltoGetContainingTypeDeclarationcauses anNullReferenceExceptionat generator time that manifests as a confusingCS8785diagnostic. The fix is a one-line predicate refinement or an early-return guard. -
Filtered solutions not updated (MODERATE) —
MSTest.slnfandNonWindowsTests.slnfare missing the newTestFramework.SourceGeneration.csproj. Every other generator project (MSTest.AotReflection.SourceGeneration,MSTest.SourceGeneration) appears in these solution filters. Leaving the new generator out makes it invisible to the IDE's solution explorer for anyone using the filtered solution view. -
Test gap on
comparison:evidence line (MODERATE) —ReportContainsFailed,ReportStartsWithFailed, andReportEndsWithFailedeach appendcomparison: <StringComparison>to the evidence block. This is new diagnostic output that no current test asserts on. A one-liner addition to three of the five newStringAssertXxxPopulatesStructuredExpectedAndActualtests would provide protection against future regressions.
Scope note: The source generator (removal of ~575 lines of boilerplate, new 183-line generator) is a self-contained, independently-useful change that the PR description itself labels "a separate concern." Merging it into the StringAssert/CollectionAssert migration makes the PR harder to review, bisect, and selectively revert. For future PRs, please consider splitting such combinations, but this alone is not a blocker for this PR given that the two changes are cohesive in authorship and effect.
No blocking issues found; the correctness, backwards-compatibility, and localization of the main change are solid.
🧪 Test quality grade — PR #929728 tests graded (21 A, 7 B — nothing below B) across 3 files. The new
This advisory comment was generated automatically. Grades are heuristic
|
Drop ActualText for CollectionAssert membership failures and use LINQ Any in the source generator. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Align CollectionAssert.Contains/DoesNotContain membership failures with Assert.Contains/DoesNotContain by leaving ActualText unset (collection stays in the evidence block only) - Update CollectionAssert tests to assert ActualText/assert.actual are absent - Add comparison-line assertions to StringAssert Contains/StartsWith/EndsWith tests - Simplify the source generator named-argument scan with LINQ Any - Guard the generator predicate against top-level structs to avoid a NullReferenceException at generation time - Add TestFramework.SourceGeneration to MSTest.slnf and NonWindowsTests.slnf Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| <ItemGroup> | ||
| <!-- Emits the repetitive AppendLiteral/AppendFormatted overloads for the Assert.*InterpolatedStringHandler | ||
| structs (see GenerateAssertInterpolatedStringAppendMethodsAttribute). --> | ||
| <ProjectReference Include="..\TestFramework.SourceGeneration\TestFramework.SourceGeneration.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" /> | ||
| </ItemGroup> |
There was a problem hiding this comment.
After merging latest main (7ac9689), this concern is largely moot: the source-generator project (TestFramework.SourceGeneration) and its Analyzer ProjectReference from TestFramework.csproj already landed on main via #9290, so they are no longer net-new in this PR — git diff origin/main -- TestFramework.csproj is now empty. The only remaining generator delta in this PR versus main is the two review-driven fixes (the nested-struct predicate guard and the LINQ Any refactor). So the PR's effective scope is back to the StringAssert/CollectionAssert membership StructuredAssertionMessage migration described in the title/description, with a minimal generator footprint.
…es-legacy-asserts-9275 # Conflicts: # src/TestFramework/TestFramework.SourceGeneration/AssertInterpolatedStringAppendMethodsGenerator.cs
Fixes #9275 (Tasks 1, 2, 3, 5).
Migrates the legacy
string.Format+FrameworkMessages.*Failfailure path to the modernStructuredAssertionMessageinfrastructure so failures expose machine-readableExpectedText/ActualText, populateexception.Data["assert.expected"|"assert.actual"], and render aligned evidence blocks — matching the diagnostics already produced by the equivalentAssert.*methods.Changes
StringAssert.cs):Contains,StartsWith,EndsWith,Matches,DoesNotMatch.CollectionAssert.Membership.cs):Contains,DoesNotContain,AllItemsAreNotNull,AllItemsAreUnique(the duplicate item is surfaced forAllItemsAreUnique).ExpectedText/ActualText/Data; updated 6 existingStringAssertTeststhat checked the old"StringAssert.X failed"text.Notes
.resx/.xlfregeneration and no public API changes were required.CollectionAssertequality/equivalence:AreEqual/AreNotEqual/AreEquivalent/AreNotEquivalent) is intentionally deferred to a follow-up — it is a larger/riskier change requiring full-collection rendering while preserving index-level mismatch detail across 6 source files.Validation
TestFramework.csprojbuilds clean across all TFMs (net462/netstandard2.0/net8.0/net9.0), 0 warnings.TestFramework.UnitTests: 1319/1319 passed.