Apply structured assertion messages (RFC 012) to Assert.Contains family#8265
Conversation
Migrates Assert.ContainsSingle (collection-only and predicate overloads, including the interpolated-string-handler), Assert.Contains (item, predicate, substring overloads with optional comparer/StringComparison), Assert.DoesNotContain (item, predicate, substring overloads with optional comparer/StringComparison), and Assert.IsInRange to the structured assertion message format defined in RFC 012. ContainsSingle uses 'expected count:' / 'actual count:' (no predicate) or 'expected matches:' / 'actual matches:' (predicate). Item-based Contains/DoesNotContain show 'expected:' / 'unexpected:' values plus a 'comparer:' line when a non-default comparer is supplied. Substring-based Contains/DoesNotContain expose 'expected substring:' / 'unexpected substring:' alongside 'actual:' and 'comparison:' lines. IsInRange uses 'expected: [min, max]' / 'actual: value' and a custom call-site formatter so the three captured arguments (minValue, maxValue, value) remain visible. Removes the now-unused BuildUserMessageForSubstringExpression* / BuildUserMessageForExpected/NotExpected/Predicate*CollectionExpression / BuildUserMessageForMinValue*MaxValue*Value* / BuildUserMessageForThreeExpressions helpers from Assert.cs. Updates the message-format tests in AssertTests.Contains.cs, AssertTests.IsInRange.cs, AssertTests.Items.cs (ContainsSingle scenarios), and AssertTests.ScopeTests.cs to assert against the new layout. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR continues the RFC 012 “Structured Assertion Messages” rollout by migrating the Assert.Contains* family (ContainsSingle, Contains, DoesNotContain, and IsInRange) from legacy single-line messages to the structured multi-line message format, including new localized resource summary entries and updated unit-test expectations.
Changes:
- Reworked
Assert.Contains*/Assert.DoesNotContain*/Assert.ContainsSingle*failure paths to buildStructuredAssertionMessageinstances (summary + optional user message + evidence + call-site). - Migrated
Assert.IsInRangefailures to structured messages, including a custom call-site formatter that preserves all 3 arguments. - Added new
*FailedSummaryresource keys and regenerated XLF entries; updated affected message-format unit tests.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.ScopeTests.cs | Updates soft-assert (AssertScope) expectations for ContainsSingle to structured message output. |
| test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.Items.cs | Updates ContainsSingle unit tests to match structured message formatting. |
| test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.IsInRange.cs | Updates IsInRange unit tests to assert on structured evidence/summary content. |
| test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.Contains.cs | Updates Contains/DoesNotContain/ContainsSingle unit tests to match structured failure messages. |
| src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.zh-Hant.xlf | Adds new *FailedSummary trans-units for structured assertion summaries. |
| src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.zh-Hans.xlf | Adds new *FailedSummary trans-units for structured assertion summaries. |
| src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.tr.xlf | Adds new *FailedSummary trans-units for structured assertion summaries. |
| src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.ru.xlf | Adds new *FailedSummary trans-units for structured assertion summaries. |
| src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.pt-BR.xlf | Adds new *FailedSummary trans-units for structured assertion summaries. |
| src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.pl.xlf | Adds new *FailedSummary trans-units for structured assertion summaries. |
| src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.ko.xlf | Adds new *FailedSummary trans-units for structured assertion summaries. |
| src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.ja.xlf | Adds new *FailedSummary trans-units for structured assertion summaries. |
| src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.it.xlf | Adds new *FailedSummary trans-units for structured assertion summaries. |
| src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.fr.xlf | Adds new *FailedSummary trans-units for structured assertion summaries. |
| src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.es.xlf | Adds new *FailedSummary trans-units for structured assertion summaries. |
| src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.de.xlf | Adds new *FailedSummary trans-units for structured assertion summaries. |
| src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.cs.xlf | Adds new *FailedSummary trans-units for structured assertion summaries. |
| src/TestFramework/TestFramework/Resources/FrameworkMessages.resx | Adds new *FailedSummary resource entries backing structured summaries for migrated assertions. |
| src/TestFramework/TestFramework/Assertions/Assert.cs | Removes now-unused legacy “BuildUserMessageForExpression” helpers. |
| src/TestFramework/TestFramework/Assertions/Assert.Contains.cs | Core migration: converts Contains*/DoesNotContain*/ContainsSingle*/IsInRange failure reporting to structured message builders. |
Copilot's findings
Comments suppressed due to low confidence (1)
src/TestFramework/TestFramework/Assertions/Assert.Contains.cs:241
- Same issue in the non-generic predicate overload: when predicateExpression is empty, it reports the non-predicate ContainsSingle count-based failure even though actualCount represents predicate matches. This can yield incorrect/misleading failure summaries on compilers that don't supply CallerArgumentExpression. Prefer reporting the predicate-specific structured message and relying on placeholders for missing expressions; reserve the count-based message for the true collection-only overload.
if (string.IsNullOrEmpty(predicateExpression))
{
ReportAssertContainsSingleFailed(matchCount, message, collectionExpression);
}
- Files reviewed: 20/20 changed files
- Comments generated: 1
Evangelink
left a comment
There was a problem hiding this comment.
Review: PR #8265 — RFC 012 structured messages for Assert.Contains family
Good migration work. One minor duplication to fix; everything else is clean.
Verdict Table
| # | Dimension | Status | Notes |
|---|---|---|---|
| 1 | Correctness | ✅ LGTM | All report helpers are [DoesNotReturn]; logic is correct |
| 2 | Null safety | ✅ LGTM | CheckParameterNotNull guards kept; comparer is object? by design (only .GetType().Name used) |
| 3 | Error messages | ✅ LGTM | Summary strings are clear and consistent with RFC 012 pattern |
| 4 | API compat | ✅ LGTM | All new methods are private static; no PublicAPI.Unshipped.txt changes needed |
| 5 | Performance | ✅ LGTM | Allocations are failure-path only |
| 6 | Cross-TFM | ✅ LGTM | IndexOf(char) used correctly; netstandard2.0/net462 safe |
| 7 | No init accessors on new public API |
✅ LGTM | No new public API introduced |
| 8 | [DoesNotReturn] annotation |
✅ LGTM | All Report* helpers carry the attribute |
| 9 | Localization / .resx |
✅ LGTM | All 9 new *FailedSummary entries present in .resx |
| 10 | XLF files | ✅ LGTM | Generated via UpdateXlf build target (not hand-edited); all 13 locales have state="new" |
| 11 | Test coverage | ✅ LGTM | Unit tests updated for new wildcard format; ScopeTests updated with exact structured output |
| 12 | Code duplication | ContainsLineBreak duplicates IsMultiline from Assert.cs — same partial class — see inline comment |
|
| 13 | Pre-existing: DoesNotContain object loop |
i️ NOTE | foreach loops (lines 536, 589) don't break after calling the [DoesNotReturn] helper, so inside an AssertScope duplicate errors are recorded for each matching element. This is pre-existing, not introduced here, but the refactor would have been a good time to add a return/break. |
| 14–21 | All other dimensions | ✅ LGTM | Style, naming, nameof, pattern matching, file endings, SA1028 — all clean |
One actionable item: The ContainsLineBreak private helper introduced in Assert.Contains.cs is identical to the existing IsMultiline in Assert.cs (same partial class). See inline comment.
Generated by Expert Code Review (on open) for issue #8265 · ● 16.9M
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@copilot resolve the merge conflicts in this pull request |
…icts in Assert.cs and FrameworkMessages.resx Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Resolved in 7146652. There were two conflicting files:
|
|
@copilot resolve the merge conflicts in this pull request |
…icts with IComparable RFC 012 (#8260) Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Resolved in c731573. New commits on
|
Continues the rollout of RFC 012 — Structured Assertion Messages by migrating the
AssertContains family to the structured-message format. Follows the patterns established by previously merged work (#8170, #8187, #8210, #8214) and the in-flight series (#8258, #8259, #8260, #8262).APIs migrated
Assert.ContainsSingle(collection-only and predicate overloads, including the interpolated-string-handler)Assert.Contains(item, predicate, substring overloads, with optional comparer /StringComparison)Assert.DoesNotContain(item, predicate, substring overloads, with optional comparer /StringComparison)Assert.IsInRangeOutput format
Assert.ContainsSingle(new[] { 1, 2, 3 }):Assert.Contains(20, collection, ""Item 20 not found""):Assert.DoesNotContain(""brown"", ""The quick brown fox"", StringComparison.Ordinal, ""Unexpected substring""):Assert.IsInRange(1, 5, 10, ""Custom error message""):Notes
*FailedSummaryresource entries for ContainsSingle, ContainsSingleMatch, ContainsItem, ContainsPredicate, ContainsSubstring, DoesNotContainItem, DoesNotContainPredicate, DoesNotContainSubstring, IsInRange (XLF files regenerated viaUpdateXlf).comparer:line is added to the evidence block when an item-basedContains/DoesNotContainis invoked with a non-defaultIEqualityComparer/IEqualityComparer<T>.IsInRangeuses a small custom call-site formatter so all three captured arguments (minValue,maxValue,value) appear in the call-site line.BuildUserMessageForSubstringExpression*,BuildUserMessageForExpected/NotExpected/Predicate*CollectionExpression,BuildUserMessageForMinValue*MaxValue*Value*, andBuildUserMessageForThreeExpressionshelpers fromAssert.cs.AssertTests.Contains.cs,AssertTests.IsInRange.cs,AssertTests.Items.cs(ContainsSingle scenarios), andAssertTests.ScopeTests.cs.Validation
dotnet build src/TestFramework/TestFramework/TestFramework.csproj -c Debug— clean.dotnet test test/UnitTests/TestFramework.UnitTests/TestFramework.UnitTests.csproj -c Debug— 3535/3535 passed.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com