Improve assertions error messages with structured format#7444
Improve assertions error messages with structured format#7444Evangelink wants to merge 47 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request modernizes assertion error messages by introducing a structured, multi-line format that improves readability and developer experience. The changes replace old-style concatenated messages with formatted parameter displays using raw string literals.
Changes:
- Introduced structured error message formatting with parameter names, expressions, and values on separate lines
- Added helper methods for value formatting, expression truncation, and redundancy detection
- Updated all assertion methods to use the new formatting approach
- Removed obsolete resource strings and added new simplified ones
- Updated all test expectations to match the new message format
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| Assert.cs | Core formatting infrastructure: FormatValue, FormatParameter, truncation logic |
| Assert.ThrowsException.cs | Updated to new structured format for exception type mismatches |
| Assert.StartsWith/EndsWith/Matches.cs | String assertion methods using new format |
| Assert.IsTrue/IsFalse.cs | Boolean assertion methods with condition parameter display |
| Assert.IsNull/IsNotNull.cs | Null checking assertions with value display |
| Assert.IsInstanceOfType/IsExactInstanceOfType.cs | Type checking with structured type comparison |
| Assert.IComparable.cs | Comparison assertions (greater/less than, positive/negative) |
| Assert.Count/Contains.cs | Collection assertions with expression parameters |
| Assert.AreSame.cs | Reference equality with hash code display for disambiguation |
| FrameworkMessages.resx | Resource string simplification and new message keys |
| xlf files | Localization files marked with untranslated entries |
| Test files | Updated expectations for all assertion error messages |
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.IsInstanceOfTypeTests.cs
Show resolved
Hide resolved
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.IsExactInstanceOfTypeTests.cs
Show resolved
Hide resolved
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.AreSame.cs
Outdated
Show resolved
Hide resolved
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.IsExactInstanceOfTypeTests.cs
Show resolved
Hide resolved
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.IsTrueTests.cs
Show resolved
Hide resolved
a55c762 to
49018c6
Compare
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.cs
Outdated
Show resolved
Hide resolved
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.cs
Outdated
Show resolved
Hide resolved
src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.zh-Hans.xlf
Show resolved
Hide resolved
src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.pt-BR.xlf
Show resolved
Hide resolved
49018c6 to
63e8257
Compare
|
looks great |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 46 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (2)
src/TestFramework/TestFramework/Assertions/Assert.IsInstanceOfType.cs:343
- When expectedType is null (one of the documented failure cases), the failure message currently only prints the value line and omits any indication that expectedType was null. This makes the assertion failure hard to diagnose. Consider either validating expectedType with CheckParameterNotNull (like other asserts do) or including an explicit "expectedType: (null)" / reason line in the structured output.
string message = string.IsNullOrEmpty(userMessage) ? string.Empty : userMessage!;
if (expectedType is not null && value is not null)
{
message += Environment.NewLine + FrameworkMessages.IsInstanceOfTypeFailNew;
}
message += Environment.NewLine + FormatParameter(nameof(value), valueExpression, value);
if (expectedType is not null && value is not null)
{
message += Environment.NewLine + $" expectedType: {expectedType}"
+ Environment.NewLine + $" actualType: {value.GetType()}";
}
src/TestFramework/TestFramework/Assertions/Assert.IsExactInstanceOfType.cs:336
- When expectedType is null, IsExactInstanceOfType fails but the structured message doesn't indicate that expectedType was null (it only prints the value line). Consider validating expectedType with CheckParameterNotNull or adding an explicit "expectedType: (null)" / reason line so callers can understand why the assert failed.
private static void ThrowAssertIsExactInstanceOfTypeFailed(object? value, Type? expectedType, string? userMessage, string valueExpression)
{
string message = string.IsNullOrEmpty(userMessage) ? string.Empty : userMessage!;
if (expectedType is not null && value is not null)
{
message += Environment.NewLine + FrameworkMessages.IsExactInstanceOfTypeFailNew;
}
message += Environment.NewLine + FormatParameter(nameof(value), valueExpression, value);
if (expectedType is not null && value is not null)
{
message += Environment.NewLine + $" expectedType: {expectedType}"
+ Environment.NewLine + $" actualType: {value.GetType()}";
}
src/TestFramework/TestFramework/Assertions/Assert.IsInstanceOfType.cs
Outdated
Show resolved
Hide resolved
| value!.GetType().ToString()); | ||
| message += Environment.NewLine + $" wrongType: {wrongType}" | ||
| + Environment.NewLine + $" actualType: {value!.GetType()}"; | ||
| } |
There was a problem hiding this comment.
When wrongType is null, IsNotExactInstanceOfType fails but the structured message omits any mention that wrongType was null. Since wrongType being null is a primary cause, consider validating wrongType with CheckParameterNotNull or including a "wrongType: (null)" / reason line in the message.
This issue also appears on line 323 of the same file.
| } | |
| } | |
| else | |
| { | |
| message += Environment.NewLine + " wrongType: (null)" | |
| + Environment.NewLine + " reason: The parameter 'wrongType' must not be null."; | |
| } |
There was a problem hiding this comment.
Fixed. When wrongType is null, the message now always includes the failure message and shows wrongType: (null) as a diagnostic. Also applied the same fix to ThrowAssertIsExactInstanceOfTypeFailed (line 323) to handle expectedType being null. Updated tests accordingly.
src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.zh-Hans.xlf
Show resolved
Hide resolved
| // the format produces a trailing space before the newline ("failed. \\r\\n"). | ||
| // Remove it to avoid trailing whitespace on the first line. | ||
| if (message is not null && message.StartsWith(Environment.NewLine, StringComparison.Ordinal)) | ||
| { | ||
| formattedMessage = $"{assertionName} failed.{message}"; |
There was a problem hiding this comment.
ThrowAssertFailed special-cases messages that start with a newline by constructing the prefix with a hardcoded English string ("{assertionName} failed."). This bypasses the localized FrameworkMessages.AssertionFailed format and will regress non-English assertion prefixes whenever structured messages start with a newline. Consider keeping the localization by deriving the prefix from FrameworkMessages.AssertionFailed (e.g., formatting with an empty message and trimming trailing whitespace) rather than hardcoding "failed.".
| // the format produces a trailing space before the newline ("failed. \\r\\n"). | |
| // Remove it to avoid trailing whitespace on the first line. | |
| if (message is not null && message.StartsWith(Environment.NewLine, StringComparison.Ordinal)) | |
| { | |
| formattedMessage = $"{assertionName} failed.{message}"; | |
| // the format produces a trailing space before the newline (for example, "failed. \\r\\n"). | |
| // Remove it to avoid trailing whitespace on the first line while preserving localization. | |
| if (message is not null && message.StartsWith(Environment.NewLine, StringComparison.Ordinal)) | |
| { | |
| string prefix = string.Format(CultureInfo.CurrentCulture, FrameworkMessages.AssertionFailed, assertionName, string.Empty); | |
| formattedMessage = prefix.TrimEnd() + message; |
| <trans-unit id="AreEqualFailNew"> | ||
| <source>Expected values to be equal.</source> | ||
| <target state="new">Expected values to be equal.</target> | ||
| <note /> |
There was a problem hiding this comment.
Per repo localization guidelines, *.xlf files shouldn't be edited manually. These XLF changes should be produced by updating the .resx and then building to regenerate the XLFs. Please revert any hand edits and regenerate the XLF file(s) from the build pipeline to avoid translation merge issues.
| <note /> | |
| <note></note> |
There was a problem hiding this comment.
The XLF files were regenerated by building the project (dotnet build). The format is now whatever the build tooling produces.
Assert.AreEqual("aaaa", "aaab") failed.
String lengths are both 4 but differ at index 3.
expected: "aaaa"
actual: "aaab"
--------------^
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.AreSame.cs
Show resolved
Hide resolved
…ent multiple enumeration Address review feedback: collection assertion methods (Contains, DoesNotContain, HasCount) now snapshot non-ICollection enumerables into a List before checking, ensuring the error message displays the same values the assertion actually tested. FormatCollectionParameter retains a safety-net guard for non-ICollection types. Added non-regression test with a non-deterministic iterator that yields different values on re-enumeration to prove the fix.
- Drop 'failed.' from assertion error first line - Move user message to end with 'User message:' label - Use consistent 'Expected...' voice across all assertions - Use human-friendly parameter labels (e.g. 'lower bound' not 'lowerBound') - Make relational assertion messages precise with embedded values - Align parameters consistently using FormatAlignedParameters - Remove redundant 'value: null' from IsNotNull - Update resource strings for voice consistency
Append type-disambiguating suffixes to numeric values in assertion error messages: long→L, ulong→UL, uint→U, float→f, decimal→m. int and double are left unsuffixed as they are the default types.
…display - Drop parentheses around values in relational messages: 'Expected value (5) to be greater than 10' -> 'Expected value 5 to be greater than 10' - Make HasCount message specific with embedded values: 'Expected collection to have the specified number of items.' -> 'Expected collection to have 3 item(s) but found 1.' - Display collection in ContainsSingle failure messages
…e+type - IsInstanceOfType: 'Expected value to be an instance of <System.String>.' with 'value: 5 (<System.Int32>)' on one line - IsExactInstanceOfType: 'Expected value to be exactly <System.String>.' - Remove redundant separate 'expected type:' and 'actual type:' lines - IsNot variants: 'Expected value to not be [an instance of|exactly] <Type>.' - Null type edge case preserved with fallback lines
- AreSame failures now show 'Objects are equal.' or 'Objects are not equal.' to help diagnose whether the issue is reference sharing vs wrong value - AreNotSame failures now show hash codes to confirm same instance - Equality check is guarded against Equals() throwing
User message now appears right after the call-site line, before the assertion explanation and parameter details. The 'User message:' prefix is removed - position alone distinguishes it.
- Converge all assertion messages to 'Expected [subject] to [verb phrase].' style - Add ellipsis (...) to callsite for overloads with omitted params (delta, culture) - Show delta, ignoreCase, culture as structured parameters - Rework CollectionAssert to use AppendUserMessage + FormatAlignedParameters - Rework StringAssert to use new structured format - Rework Assert.That to strip lambda wrapper and generate expression-aware messages (comparisons, bool members, negation, StartsWith/Contains/All/Any) - Update all test assertions to verify full messages
- Fix #1: Replace hardcoded 'Expected all elements...' with AllMatchPredicateFailNew resource - Fix #2: Remove 11 dead resource strings (ContainsFail, StartsWithFail, EndsWithFail, IsMatchFail, IsNotMatchFail, DoesNotEndWithFail, DoesNotStartWithFail, AssertThatFailedFormat, AssertThatMessageFormat, AssertThatDetailsPrefix, CollectionEqualReason) - Fix #3: Replace bare catch with catch (Exception) in TryEvaluateFormatted - Fix #5: Type-check StartsWith/EndsWith/Contains to only match string methods - Fix #6: Add explicit ellipsis sentinel handling in FormatCallSite - Fix #7: Fix grammar 'Both collection contain same elements' -> 'Both collections contain the same elements'
Fix #7421