Improve failure message for string assertions when checking for equality

[vbreuss]

When asserting that two strings are equal, and one of the strings is longer than 8 characters or contains a NewLine, instead of simply stating at which index the first difference occurs, this difference is made visible using arrows, e.g.

var subject = "this is a long text that differs in between";
var expected = "this was too short";
subject.Should().Be(expected, "because we use arrows now");

throws with message

Expected subject to be the same string because we use arrows now, but they differ at index 5:
        ↓ (actual)
  "this is a long text…"
  "this was too short"
        ↑ (expected)."

For multi-line strings, instead of only providing information about the index, the failure message also contains the line and column information, e.g. …but they differ on line 5 and column 1 (index 93):

For short single-line strings the behaviour remains unchanged!

This closes #2050.

IMPORTANT

• If the PR touches the public API, the changes have been approved in a separate issue with the "api-approved" label.
• The code complies with the Coding Guidelines for C#.
• The changes are covered by unit tests which follow the Arrange-Act-Assert syntax and the naming conventions such as is used in these tests.
• If the PR adds a feature or fixes a bug, please update the release notes with a functional description that explains what the change means to consumers of this library, which are published on the website.
• If the PR changes the public API the changes needs to be included by running AcceptApiChanges.ps1 or AcceptApiChanges.sh.
• If the PR affects the documentation, please include your changes in this pull request so the documentation will appear on the website.
  • Please also run ./build.sh --target spellcheck or .\build.ps1 --target spellcheck before pushing and check the good outcome

[github-actions]

Qodana for .NET

5 new problems were found

Inspection name	Severity	Problems
Class overrides Object.Equals(object o) but not Object.GetHashCode()	🔴 Failure	1
Parameter can be declared with the base type	◽️ Notice	3
Convert into lambda expression	◽️ Notice	1

💡 Qodana analysis was run in the pull request mode: only the changed files were checked

View the detailed Qodana report

To be able to view the detailed Qodana report, you can either:

1. Register at Qodana Cloud and configure the action
2. Use GitHub Code Scanning with Qodana
3. Host Qodana report at GitHub Pages
4. Inspect and use qodana.sarif.json (see the Qodana SARIF format for details)

To get *.log files or any other Qodana artifacts, run the action with upload-result option set to true,
so that the action will upload the files as the job artifacts:

      - name: 'Qodana Scan'
        uses: JetBrains/[email protected]
        with:
          upload-result: true

Contact Qodana team

Contact us at [email protected]

• Or via our issue tracker: https://jb.gg/qodana-issue
• Or share your feedback: https://jb.gg/qodana-discussions

[dennisdoomen]

That's a pretty impressive improvement. I had some quite some issues understanding the algorithm, so I've gone through a pretty thorough review.

[vbreuss]

That's a pretty impressive improvement. I had some quite some issues understanding the algorithm, so I've gone through a pretty thorough review.

@dennisdoomen. Thanks for the extensive review. It's really appreciated!

I implemented your requested changes and answered your questions. I hope I didn't overlook anything :-)
I also added this change to the releases.md

[jnyrup]

This is really great!

I haven't gone through all the relational operators to check if they are correct.
I'll try to run mutation tests on StringEqualityStrategy.cs sometime this week to help verifying this. If you want to try mutation tests before I get to it, there are some pointers in #1068 and jnyrup#6

[ITaluone]

I'll try to run mutation tests on StringEqualityStrategy.cs sometime this week to help verifying this.

Shall we add that to our pipeline?

[jnyrup]

I'll try to run mutation tests on StringEqualityStrategy.cs sometime this week to help verifying this.

Shall we add that to our pipeline?

That's my plan.
It takes quite a while to do a full run, so my current idea is to only consider the changed files in PRs and then do a scheduled full run e.g. once a week.

[vbreuss]

I'll try to run mutation tests on StringEqualityStrategy.cs sometime this week to help verifying this. If you want to try mutation tests before I get to it, there are some pointers in #1068 and jnyrup#6

I also use Stryker in my project and just ran it against fluentassertions

Mutation Testing Summary

File	Score	Killed	Survived	Timeout	No Coverage	Ignored	Compile Errors	Total Detected	Total Undetected	Total Mutants
Primitives\StringEqualityStrategy.cs	97,12%	101	3	0	0	21	8	101	3	133

I will add tests for the 3 survived mutants together with the changes for your review :-)

[vbreuss]

@jnyrup
I implemented your review comments and added the missing tests for the surviving mutants.

Mutation Testing Summary

File	Score	Killed	Survived	Timeout	No Coverage	Ignored	Compile Errors	Total Detected	Total Undetected	Total Mutants
Primitives\StringEqualityStrategy.cs	100,00%	103	0	0	0	20	8	103	0	131

[dennisdoomen]

As I said before, great work. The new comments and improved naming really helps (even though it is still quite an algorithm).

Since we've tried to get rid of words like when and should in test names and focus more on the fact that such a test covers, I've suggested some improvements. Up to you if you want to include them now. Otherwise, I'll do it at a later point.