Skip to content

Commit 6859575

Browse files
vbreussvbtig
vbreuss
and
vbtig
authored
Improve failure message for string assertions when checking for equality (fluentassertions#2307)
* Draft improved failure message when checking for equality. * Use line and column for multiline strings * Fix failing tests * Fix Qodana issues * Fix further failing tests * Also fix tests on MacOS (due to different newline) * Calculate expected index * Fix paranthesis * Move specific methods from common StringExtensions to the `StringEqualityStrategy` class * Fix review comments from @dennisdoomen and add tests for word boundary algorithm * Improve tests by using [raw string literals](https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/strings/#raw-string-literals) * Update releases.md * Implement review comments from @jnyrup * Adapt due to survived mutants * Adapt test names --------- Co-authored-by: Valentin Breuß <[email protected]>
1 parent 3c97a30 commit 6859575

File tree

13 files changed

+356
-42
lines changed

13 files changed

+356
-42
lines changed

‎Src/FluentAssertions/Common/StringExtensions.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,4 +150,13 @@ public static int CountSubstring(this string str, string substring, StringCompar
150150

151151
return count;
152152
}
153+
154+
/// <summary>
155+
/// Determines if the <paramref name="value"/> is longer than 8 characters or contains an <see cref="Environment.NewLine"/>.
156+
/// </summary>
157+
public static bool IsLongOrMultiline(this string value)
158+
{
159+
const int humanReadableLength = 8;
160+
return value.Length > humanReadableLength || value.Contains(Environment.NewLine, StringComparison.Ordinal);
161+
}
153162
}

‎Src/FluentAssertions/Primitives/StringEqualityStrategy.cs‎

Lines changed: 164 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
using System;
2+
using System.Linq;
3+
using System.Text;
24
using FluentAssertions.Common;
35
using FluentAssertions.Execution;
46

@@ -13,9 +15,65 @@ public StringEqualityStrategy(StringComparison comparisonMode)
1315
this.comparisonMode = comparisonMode;
1416
}
1517

16-
private bool ValidateAgainstSuperfluousWhitespace(IAssertionScope assertion, string subject, string expected)
18+
public void ValidateAgainstMismatch(IAssertionScope assertion, string subject, string expected)
1719
{
18-
return assertion
20+
ValidateAgainstSuperfluousWhitespace(assertion, subject, expected);
21+
22+
if (expected.IsLongOrMultiline() || subject.IsLongOrMultiline())
23+
{
24+
int indexOfMismatch = subject.IndexOfFirstMismatch(expected, comparisonMode);
25+
26+
if (indexOfMismatch == -1)
27+
{
28+
ValidateAgainstLengthDifferences(assertion, subject, expected);
29+
return;
30+
}
31+
32+
string locationDescription = $"at index {indexOfMismatch}";
33+
var matchingString = subject[..indexOfMismatch];
34+
int lineNumber = matchingString.Count(c => c == '\n');
35+
36+
if (lineNumber > 0)
37+
{
38+
var indexOfLastNewlineBeforeMismatch = matchingString.LastIndexOf('\n');
39+
var column = matchingString.Length - indexOfLastNewlineBeforeMismatch;
40+
locationDescription = $"on line {lineNumber + 1} and column {column} (index {indexOfMismatch})";
41+
}
42+
43+
assertion.FailWith(
44+
ExpectationDescription + "the same string{reason}, but they differ " + locationDescription + ":" +
45+
Environment.NewLine
46+
+ GetMismatchSegmentForLongStrings(subject, expected, indexOfMismatch) + ".");
47+
}
48+
else if (ValidateAgainstLengthDifferences(assertion, subject, expected))
49+
{
50+
int indexOfMismatch = subject.IndexOfFirstMismatch(expected, comparisonMode);
51+
52+
if (indexOfMismatch != -1)
53+
{
54+
assertion.FailWith(
55+
ExpectationDescription + "{0}{reason}, but {1} differs near " + subject.IndexedSegmentAt(indexOfMismatch) +
56+
".",
57+
expected, subject);
58+
}
59+
}
60+
}
61+
62+
public string ExpectationDescription
63+
{
64+
get
65+
{
66+
string predicateDescription = IgnoreCase ? "be equivalent to" : "be";
67+
return "Expected {context:string} to " + predicateDescription + " ";
68+
}
69+
}
70+
71+
private bool IgnoreCase
72+
=> comparisonMode == StringComparison.OrdinalIgnoreCase;
73+
74+
private void ValidateAgainstSuperfluousWhitespace(IAssertionScope assertion, string subject, string expected)
75+
{
76+
assertion
1977
.ForCondition(!(expected.Length > subject.Length && expected.TrimEnd().Equals(subject, comparisonMode)))
2078
.FailWith(ExpectationDescription + "{0}{reason}, but it misses some extra whitespace at the end.", expected)
2179
.Then
@@ -55,33 +113,122 @@ private string GetMismatchSegmentForStringsOfDifferentLengths(string subject, st
55113
return subject.IndexedSegmentAt(indexOfMismatch);
56114
}
57115

58-
public void ValidateAgainstMismatch(IAssertionScope assertion, string subject, string expected)
116+
/// <summary>
117+
/// Get the mismatch segment between <paramref name="expected"/> and <paramref name="subject"/> for long strings,
118+
/// when they differ at index <paramref name="firstIndexOfMismatch"/>.
119+
/// </summary>
120+
private static string GetMismatchSegmentForLongStrings(string subject, string expected, int firstIndexOfMismatch)
121+
{
122+
int trimStart = GetStartIndexOfPhraseToShowBeforeTheMismatchingIndex(subject, firstIndexOfMismatch);
123+
const string prefix = " \"";
124+
const string suffix = "\"";
125+
const char arrowDown = '\u2193';
126+
const char arrowUp = '\u2191';
127+
128+
int whiteSpaceCountBeforeArrow = (firstIndexOfMismatch - trimStart) + prefix.Length;
129+
130+
if (trimStart > 0)
131+
{
132+
whiteSpaceCountBeforeArrow++;
133+
}
134+
135+
var visibleText = subject[trimStart..firstIndexOfMismatch];
136+
whiteSpaceCountBeforeArrow += visibleText.Count(c => c is '\r' or '\n');
137+
138+
var sb = new StringBuilder();
139+
140+
sb.Append(' ', whiteSpaceCountBeforeArrow).Append(arrowDown).AppendLine(" (actual)");
141+
AppendPrefixAndEscapedPhraseToShowWithEllipsisAndSuffix(sb, prefix, subject, trimStart, suffix);
142+
AppendPrefixAndEscapedPhraseToShowWithEllipsisAndSuffix(sb, prefix, expected, trimStart, suffix);
143+
sb.Append(' ', whiteSpaceCountBeforeArrow).Append(arrowUp).Append(" (expected)");
144+
145+
return sb.ToString();
146+
}
147+
148+
/// <summary>
149+
/// Appends the <paramref name="prefix"/>, the escaped visible <paramref name="text"/> phrase decorated with ellipsis and the <paramref name="suffix"/> to the <paramref name="stringBuilder"/>.
150+
/// </summary>
151+
/// <remarks>When text phrase starts at <paramref name="indexOfStartingPhrase"/> and with a calculated length omits text on start or end, an ellipsis is added.</remarks>
152+
private static void AppendPrefixAndEscapedPhraseToShowWithEllipsisAndSuffix(StringBuilder stringBuilder,
153+
string prefix, string text, int indexOfStartingPhrase, string suffix)
59154
{
60-
if (!ValidateAgainstSuperfluousWhitespace(assertion, subject, expected) ||
61-
!ValidateAgainstLengthDifferences(assertion, subject, expected))
155+
var subjectLength = GetLengthOfPhraseToShowOrDefaultLength(text[indexOfStartingPhrase..]);
156+
const char ellipsis = '\u2026';
157+
158+
stringBuilder.Append(prefix);
159+
160+
if (indexOfStartingPhrase > 0)
62161
{
63-
return;
162+
stringBuilder.Append(ellipsis);
64163
}
65164

66-
int indexOfMismatch = subject.IndexOfFirstMismatch(expected, comparisonMode);
165+
stringBuilder.Append(text
166+
.Substring(indexOfStartingPhrase, subjectLength)
167+
.Replace("\r", "\\r", StringComparison.OrdinalIgnoreCase)
168+
.Replace("\n", "\\n", StringComparison.OrdinalIgnoreCase));
67169

68-
if (indexOfMismatch != -1)
170+
if (text.Length > (indexOfStartingPhrase + subjectLength))
69171
{
70-
assertion.FailWith(
71-
ExpectationDescription + "{0}{reason}, but {1} differs near " + subject.IndexedSegmentAt(indexOfMismatch) + ".",
72-
expected, subject);
172+
stringBuilder.Append(ellipsis);
73173
}
174+
175+
stringBuilder.AppendLine(suffix);
74176
}
75177

76-
public string ExpectationDescription
178+
/// <summary>
179+
/// Calculates the start index of the visible segment from <paramref name="value"/> when highlighting the difference at <paramref name="indexOfFirstMismatch"/>.
180+
/// </summary>
181+
/// <remarks>
182+
/// Either keep the last 10 characters before <paramref name="indexOfFirstMismatch"/> or a word begin (separated by whitespace) between 15 and 5 characters before <paramref name="indexOfFirstMismatch"/>.
183+
/// </remarks>
184+
private static int GetStartIndexOfPhraseToShowBeforeTheMismatchingIndex(string value, int indexOfFirstMismatch)
77185
{
78-
get
186+
const int defaultCharactersToKeep = 10;
187+
const int minCharactersToKeep = 5;
188+
const int maxCharactersToKeep = 15;
189+
const int lengthOfWhitespace = 1;
190+
const int phraseLengthToCheckForWordBoundary = (maxCharactersToKeep - minCharactersToKeep) + lengthOfWhitespace;
191+
192+
if (indexOfFirstMismatch <= defaultCharactersToKeep)
79193
{
80-
string predicateDescription = IgnoreCase ? "be equivalent to" : "be";
81-
return "Expected {context:string} to " + predicateDescription + " ";
194+
return 0;
82195
}
196+
197+
var indexToStartSearchingForWordBoundary = Math.Max(indexOfFirstMismatch - (maxCharactersToKeep + lengthOfWhitespace), 0);
198+
199+
var indexOfWordBoundary = value
200+
.IndexOf(' ', indexToStartSearchingForWordBoundary, phraseLengthToCheckForWordBoundary) -
201+
indexToStartSearchingForWordBoundary;
202+
203+
if (indexOfWordBoundary >= 0)
204+
{
205+
return indexToStartSearchingForWordBoundary + indexOfWordBoundary + lengthOfWhitespace;
206+
}
207+
208+
return indexOfFirstMismatch - defaultCharactersToKeep;
83209
}
84210

85-
private bool IgnoreCase
86-
=> comparisonMode == StringComparison.OrdinalIgnoreCase;
211+
/// <summary>
212+
/// Calculates how many characters to keep in <paramref name="value"/>.
213+
/// </summary>
214+
/// <remarks>
215+
/// If a word end is found between 15 and 25 characters, use this word end, otherwise keep 20 characters.
216+
/// </remarks>
217+
private static int GetLengthOfPhraseToShowOrDefaultLength(string value)
218+
{
219+
const int defaultLength = 20;
220+
const int minLength = 15;
221+
const int maxLength = 25;
222+
const int lengthOfWhitespace = 1;
223+
224+
var indexOfWordBoundary = value
225+
.LastIndexOf(' ', Math.Min(maxLength + lengthOfWhitespace, value.Length) - 1);
226+
227+
if (indexOfWordBoundary >= minLength)
228+
{
229+
return indexOfWordBoundary;
230+
}
231+
232+
return Math.Min(defaultLength, value.Length);
233+
}
87234
}

‎Src/FluentAssertions/Primitives/StringValidator.cs‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
1-
using System;
1+
using FluentAssertions.Common;
22
using FluentAssertions.Execution;
33

44
namespace FluentAssertions.Primitives;
55

66
internal class StringValidator
77
{
8-
private const int HumanReadableLength = 8;
9-
108
private readonly IStringComparisonStrategy comparisonStrategy;
119
private IAssertionScope assertion;
1210

@@ -28,7 +26,7 @@ public void Validate(string subject, string expected)
2826
return;
2927
}
3028

31-
if (IsLongOrMultiline(expected) || IsLongOrMultiline(subject))
29+
if (expected.IsLongOrMultiline() || subject.IsLongOrMultiline())
3230
{
3331
assertion = assertion.UsingLineBreaks;
3432
}
@@ -46,9 +44,4 @@ private bool ValidateAgainstNulls(string subject, string expected)
4644
assertion.FailWith(comparisonStrategy.ExpectationDescription + "{0}{reason}, but found {1}.", expected, subject);
4745
return false;
4846
}
49-
50-
private static bool IsLongOrMultiline(string value)
51-
{
52-
return value.Length > HumanReadableLength || value.Contains(Environment.NewLine, StringComparison.Ordinal);
53-
}
5447
}

‎Tests/FluentAssertions.Equivalency.Specs/BasicSpecs.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ public void When_treating_a_value_type_in_a_collection_as_a_complex_type_it_shou
221221
options => options.ComparingByMembers<ClassWithValueSemanticsOnSingleProperty>());
222222

223223
// Assert
224-
act.Should().Throw<XunitException>().WithMessage("*NestedProperty*OtherValue*SomeValue*");
224+
act.Should().Throw<XunitException>().WithMessage("*NestedProperty*SomeValue*OtherValue*");
225225
}
226226

227227
[Fact]
@@ -237,7 +237,7 @@ public void When_treating_a_value_type_as_a_complex_type_it_should_compare_them_
237237
options => options.ComparingByMembers<ClassWithValueSemanticsOnSingleProperty>());
238238

239239
// Assert
240-
act.Should().Throw<XunitException>().WithMessage("*NestedProperty*OtherValue*SomeValue*");
240+
act.Should().Throw<XunitException>().WithMessage("*NestedProperty*SomeValue*OtherValue*");
241241
}
242242

243243
[Fact]

‎Tests/FluentAssertions.Equivalency.Specs/CyclicReferencesSpecs.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ public void Allow_ignoring_cyclic_references_in_value_types_compared_by_members(
340340

341341
// Assert
342342
act.Should().Throw<XunitException>()
343-
.WithMessage("*subject.Next.Title*Second*SecondDifferent*")
343+
.WithMessage("*subject.Next.Title*SecondDifferent*Second*")
344344
.Which.Message.Should().NotContain("maximum recursion depth was reached");
345345
}
346346

‎Tests/FluentAssertions.Equivalency.Specs/NestedPropertiesSpecs.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ public void When_deeply_nested_properties_do_not_have_all_equal_values_it_should
270270
act
271271
.Should().Throw<XunitException>()
272272
.WithMessage(
273-
"Expected*Level.Level.Text*to be *A wrong text value*but*\"Level2\"*length*");
273+
"Expected*Level.Level.Text*to be *\"Level2\"*A wrong text value*");
274274
}
275275

276276
[Fact]

‎Tests/FluentAssertions.Equivalency.Specs/SelectionRulesSpecs.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1353,7 +1353,7 @@ public void When_a_property_is_internal_and_it_should_be_included_it_should_fail
13531353

13541354
// Assert
13551355
act.Should().Throw<XunitException>()
1356-
.WithMessage("*InternalProperty*also internal*internal*ProtectedInternalProperty*");
1356+
.WithMessage("*InternalProperty*internal*also internal*ProtectedInternalProperty*");
13571357
}
13581358

13591359
private class ClassWithInternalProperty
@@ -1409,7 +1409,7 @@ public void When_a_field_is_internal_and_it_should_be_included_it_should_fail_th
14091409
Action act = () => actual.Should().BeEquivalentTo(expected, options => options.IncludingInternalFields());
14101410

14111411
// Assert
1412-
act.Should().Throw<XunitException>().WithMessage("*InternalField*also internal*internal*ProtectedInternalField*");
1412+
act.Should().Throw<XunitException>().WithMessage("*InternalField*internal*also internal*ProtectedInternalField*");
14131413
}
14141414

14151415
private class ClassWithInternalField

‎Tests/FluentAssertions.Specs/FluentAssertions.Specs.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<Project Sdk="Microsoft.NET.Sdk">
1+
<Project Sdk="Microsoft.NET.Sdk">
22

33
<PropertyGroup>
44
<TargetFrameworks>net47;net6.0</TargetFrameworks>

‎Tests/FluentAssertions.Specs/Formatting/FormatterSpecs.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public void When_the_subject_or_expectation_contains_reserved_symbols_it_should_
7474
Action act = () => result.Should().Be(expectedJson);
7575

7676
// Assert
77-
act.Should().Throw<XunitException>().WithMessage("*near*index 37*");
77+
act.Should().Throw<XunitException>().WithMessage("*at*index 37*");
7878
}
7979

8080
[Fact]

‎Tests/FluentAssertions.Specs/Primitives/ObjectAssertionSpecs.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -695,7 +695,7 @@ public void When_object_is_of_the_expected_type_it_should_cast_the_returned_obje
695695
Action act = () => someObject.Should().BeOfType<Exception>().Which.Message.Should().Be("Other Message");
696696

697697
// Assert
698-
act.Should().Throw<XunitException>().WithMessage("*Expected*Other*Actual*");
698+
act.Should().Throw<XunitException>().WithMessage("*Expected*Actual*Other*");
699699
}
700700

701701
[Fact]
@@ -906,7 +906,7 @@ public void When_to_the_expected_type_it_should_cast_the_returned_object_for_cha
906906
Action act = () => someObject.Should().BeAssignableTo<Exception>().Which.Message.Should().Be("Other Message");
907907

908908
// Assert
909-
act.Should().Throw<XunitException>().WithMessage("*Expected*Other*Actual*");
909+
act.Should().Throw<XunitException>().WithMessage("*Expected*Actual*Other*");
910910
}
911911

912912
[Fact]
@@ -1117,7 +1117,7 @@ public void
11171117
.Which.Message.Should().Be("Other Message");
11181118

11191119
// Assert
1120-
act.Should().Throw<XunitException>().WithMessage("*Expected*Other*Actual*");
1120+
act.Should().Throw<XunitException>().WithMessage("*Expected*Actual*Other*");
11211121
}
11221122

11231123
[Fact]

0 commit comments

Comments
 (0)