Incorrect line breaks handling with custom string comparer

[bart-vmware]

Description

Follow-up for #2413 (comment). #2413 was supposed to fix this, but it still doesn't work correctly.

Reproduction Steps

Checkout the latest develop branch (600d360) and run the below tests. Both tests should succeed, but they fail with:

Xunit.Sdk.XunitException
Expected logLines to be equivalent to "one\r\ntwo\r\nthree", but it misses some extra whitespace at the end.
   at FluentAssertions.Execution.XUnit2TestFramework.Throw(String message) in C:\Source\Repos\fluentassertions\Src\FluentAssertions\Execution\XUnit2TestFramework.cs:line 38
   at FluentAssertions.Execution.TestFrameworkProvider.Throw(String message) in C:\Source\Repos\fluentassertions\Src\FluentAssertions\Execution\TestFrameworkProvider.cs:line 39
   at FluentAssertions.Execution.DefaultAssertionStrategy.HandleFailure(String message) in C:\Source\Repos\fluentassertions\Src\FluentAssertions\Execution\DefaultAssertionStrategy.cs:line 18
   at FluentAssertions.Execution.AssertionScope.FailWith(Func`1 failReasonFunc) in C:\Source\Repos\fluentassertions\Src\FluentAssertions\Execution\AssertionScope.cs:line 274
   at FluentAssertions.Execution.AssertionScope.FailWith(Func`1 failReasonFunc) in C:\Source\Repos\fluentassertions\Src\FluentAssertions\Execution\AssertionScope.cs:line 242
   at FluentAssertions.Execution.AssertionScope.FailWith(String message, Object[] args) in C:\Source\Repos\fluentassertions\Src\FluentAssertions\Execution\AssertionScope.cs:line 296
   at FluentAssertions.Primitives.StringEqualityStrategy.ValidateAgainstSuperfluousWhitespace(IAssertionScope assertion, String subject, String expected) in C:\Source\Repos\fluentassertions\Src\FluentAssertions\Primitives\StringEqualityStrategy.cs:line 69
   at FluentAssertions.Primitives.StringEqualityStrategy.ValidateAgainstMismatch(IAssertionScope assertion, String subject, String expected) in C:\Source\Repos\fluentassertions\Src\FluentAssertions\Primitives\StringEqualityStrategy.cs:line 23
   at FluentAssertions.Primitives.StringValidator.Validate(String subject, String expected) in C:\Source\Repos\fluentassertions\Src\FluentAssertions\Primitives\StringValidator.cs:line 34
   at FluentAssertions.Primitives.StringAssertions`1.BeEquivalentTo(String expected, Func`2 config, String because, Object[] becauseArgs) in C:\Source\Repos\fluentassertions\Src\FluentAssertions\Primitives\StringAssertions.cs:line 157
   at FluentAssertions.Specs.AndWhichConstraintSpecs.BeEquivalentTo_LineEndingsComparer() in C:\Source\Repos\fluentassertions\Tests\FluentAssertions.Specs\AndWhichConstraintSpecs.cs:line 35

[Fact]
public void BeEquivalentTo_LineEndingsComparer()
{
    string logLines = @"one\ntwo\nthree";
    string expected = @"one\r\ntwo\r\nthree";

    logLines.Should().BeEquivalentTo(expected, options => options.Using(IgnoreLineEndingsComparer.Instance));
}

[Fact]
public void MatchEquivalentOf_LineEndingsComparer()
{
    string logLines = @"one\ntwo\nthree";
    string expected = @"one\r\ntwo\r\nthree";

    logLines.Should().MatchEquivalentOf(expected);
}

See #2339 (comment) for the contents of the IgnoreLineEndingsComparer class.

Expected behavior

Tests succeed.

Actual behavior

Tests fail.

Regression?

No.

Known Workarounds

Cast subject (typed as string) to object before executing the assertion, see #2339.

Configuration

Latest develop branch (600d360)

Other information

Possibly unrelated, but while investigating I spotted another bug at:

fluentassertions/Src/FluentAssertions/Common/StringExtensions.cs

Lines 118 to 122 in 600d360

	public static string RemoveNewLines(this string @this)
	{
	return @this.Replace("\n", string.Empty, StringComparison.Ordinal)
	.Replace("\r", string.Empty, StringComparison.Ordinal)
	.Replace(@"\r\n", string.Empty, StringComparison.Ordinal);

Note the @ verbatim string modifier on the last line, which is equivalent to matching "\\r\\n" instead of "\r\n".
While this may be effectively dead code, it looks wrong.

Are you willing to help with a pull-request?

No

[vbreuss]

I had a look at the issue and it seems to be quite difficult to fix. The root cause is the following:
In order to create a good error message we need to determine the IndexOfFirstMismatch.
In your example it would check the following:

1. comparer.Equals("o", "o") // yes
2. comparer.Equals("on", "on") // yes
3. comparer.Equals("one", "one") // yes
4. comparer.Equals("one\n", "one\r") // yes, due to custom comparer
5. comparer.Equals("one\nt", "one\r\n") // no

Currently the custom string comparers only work, when they don't "change the length" (does that make sense?) of the compared strings.

I don't see any real solution for this to support arbitrary comparers which change the length, however for your specific use case I am working on specific string options to ignore the newline style (see #2496).
You could use it like

subject.Should().BeEquivalentTo(expected, o => o.IgnoringNewlineStyle());

and it would replace "\r\n" with "\n" in the subject and the expected string before comparison.
I think this should behave the same as your custom comparer.

Would this help?

[bart-vmware]

Yes, that would solve my use case. We use this for assertions on the exact text of formatted log output (so casing, whitespace, and line breaks must match exactly). Running in ci-build (where the line endings differ per OS) makes it problematic, which is why I created the custom comparer.

Thanks for working on this.

How about the RemoveNewLines method, is it dead code?