-
Notifications
You must be signed in to change notification settings - Fork 281
Enhance Assert.AreEqual string comparison to show first difference with index and context #6163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's have expected and actual on new lines. Also add an example of a diff that appears far in a long string (e.g. position 90 of a 200 char string) to confirm we do not display all 90 char but use ellipsis to cut long before and after
I've implemented both requested changes:
Changes in commit bf7af04:
The ellipsis properly truncates long strings while maintaining readability around the difference point. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure all newly added tests assert full message and not only contains
Updated all newly added string comparison tests to assert the full exact message instead of using Addressed in commit aaa23dc. |
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.AreEqualTests.cs
Outdated
Show resolved
Hide resolved
src/TestFramework/TestFramework/Resources/FrameworkMessages.resx
Outdated
Show resolved
Hide resolved
Ensure that all added tests are written using FluentAssertions (don't add package) |
Converted all newly added string comparison tests to use FluentAssertions syntax. Added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic here should probably be changed quite a bit, to think about the shortening of the string as "masking" it with 3 dots where we have 4 or more characters to render. Otherwise it will have different visual bugs.
I would avoid replacing the utf chars, I want my smileys to be rendered for me normally.
Other small fixes should be applied to maximize the info user sees in cases where the diff is near the end of the string.
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.AreEqualTests.cs
Outdated
Show resolved
Hide resolved
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.AreEqualTests.cs
Show resolved
Hide resolved
result.Append(CultureInfo.InvariantCulture, $"\\u{(int)c:X4}"); | ||
#else | ||
result.Append($"\\u{(int)c:X4}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we add more chars to the string but we already figured out the diference so the arrow will point at earlier character:
Exception ex = VerifyThrows(() => Assert.AreEqual("🤮aaa", "🤮aab"));
Assert.AreEqual failed. String lengths are both 5 but differ at index 4.
Expected: "\uD83E\uDD2Eaaa"
But was: "\uD83E\uDD2Eaab"
---------------^
Also replacing the characters with their unicode counterparts seems like we are giving up convenience for supporting the worst case scenario, I would not do that.
This case also highlights that the code is counting byte length (string.lenght) and not the text element size, which might align better with the user expectation.
In either case if we manipulate the string we should make sure that the difference counting reflects that.
To me I'd keep the string as is. replace the non-printable chars at the beginning with their printable counterparts e.g \e
to ␛
and leave that as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed the caret positioning issue by tracking character replacements in CreateStringPreviewWithCaretInfo()
. Now the caret correctly points to the actual difference position even when characters are escaped. Also changed to preserve Unicode characters (like emojis) rather than escaping them unnecessarily. Commit bc3be4c.
Exception ex = VerifyThrows(() => Assert.AreEqual("aa\ta", "aa a")); | ||
ex.Message.Should().Be(""" | ||
Assert.AreEqual failed. String lengths are both 4 but differ at index 2. | ||
Expected: "aa\ta" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected: "aa\ta" | |
Expected: "aa\\ta" |
If the intent was to show \t
and not tab char.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed the test expectation to properly show escaped tab character as "aa\\ta"
to match what the implementation actually produces when displaying the \t escape sequence. Commit bc3be4c.
I've implemented the suggested improvements in commit bc3be4c:
The logic now maximizes the information shown to users while addressing the visual bugs mentioned. |
string expected, string actual, int diffIndex, int contextLength) | ||
{ | ||
int maxLength = Math.Max(expected.Length, actual.Length); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/TestFramework/TestFramework/Assertions/Assert.AreEqual.cs(804,1): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055)
Converted all newly added string comparison tests to use FluentAssertions syntax. Changed |
…ce highlighting Co-authored-by: Evangelink <[email protected]>
…and add test Co-authored-by: Evangelink <[email protected]>
…long strings Co-authored-by: Evangelink <[email protected]>
…g Contains Co-authored-by: Evangelink <[email protected]>
Co-authored-by: Evangelink <[email protected]>
Co-authored-by: Evangelink <[email protected]>
Co-authored-by: Evangelink <[email protected]>
Co-authored-by: nohwnd <[email protected]>
…n logic Co-authored-by: nohwnd <[email protected]>
3effc10
to
7441410
Compare
MSTest's
Assert.AreEqual
previously provided minimal information when string comparisons failed, making it difficult for developers to quickly identify the differences. This enhancement brings MSTest's string comparison messages in line with NUnit and XUnit by showing the exact location and context of differences.Before
After
Key Features
Examples
Basic difference:
Different lengths:
Special characters:
Long strings (shows contextual preview):
Implementation Details
ThrowAssertAreEqualFailed
to detect string types and apply special formattingThe enhancement only affects string comparison failures and maintains backward compatibility for all other assertion types.
Fixes #4261 and fixes #978.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.