Fixed null string comparison (shouldly#538)

* Failing tests that difference should not be shown for missing string * Stop showing misleading difference for missing string
james00harper · Nov 6, 2018 · c4aa6f5 · c4aa6f5
1 parent e7a25cc
commit c4aa6f5
Show file tree

Hide file tree

Showing 5 changed files with 109 additions and 21 deletions.
diff --git a/src/Shouldly.Tests/ShouldBe/ShouldBeScenarios.cs b/src/Shouldly.Tests/ShouldBe/ShouldBeScenarios.cs
@@ -181,14 +181,6 @@ should be
 """"
     but was
 null
-    difference
-Difference     |  |    |    |    |   
-               | \|/  \|/  \|/  \|/  
-Index          | 0    1    2    3    
-Expected Value |                     
-Actual Value   | n    u    l    l    
-Expected Code  |                     
-Actual Code    | 110  117  108  108  
 
 Additional Info:
     Some additional context",
@@ -198,14 +190,6 @@ Actual Code    | 110  117  108  108
     should be
 """"
     but was not
-    difference
-Difference     |  |    |    |    |   
-               | \|/  \|/  \|/  \|/  
-Index          | 0    1    2    3    
-Expected Value |                     
-Actual Value   | n    u    l    l    
-Expected Code  |                     
-Actual Code    | 110  117  108  108  
 
 Additional Info:
     Some additional context");

diff --git a/src/Shouldly.Tests/Strings/DetailedDifference/CaseInsensitive/NullScenario.cs b/src/Shouldly.Tests/Strings/DetailedDifference/CaseInsensitive/NullScenario.cs
@@ -0,0 +1,49 @@
+using Xunit;
+
+namespace Shouldly.Tests.Strings.DetailedDifference.CaseInsensitive
+{
+    public static class NullScenario
+    {
+        [Fact]
+        public static void ShouldNotShowDifferenceWhenActualIsMissing()
+        {
+            var str = (string)null;
+            Verify.ShouldFail(() =>
+str.ShouldBe("null", StringCompareShould.IgnoreCase),
+
+errorWithSource:
+@"str
+    should be with options: Ignoring case
+""null""
+    but was
+null",
+
+errorWithoutSource:
+@"null
+    should be with options: Ignoring case
+""null""
+    but was not");
+        }
+
+        [Fact]
+        public static void ShouldNotShowDifferenceWhenExpectedIsMissing()
+        {
+            var str = "null";
+            Verify.ShouldFail(() =>
+str.ShouldBe(null, StringCompareShould.IgnoreCase),
+
+errorWithSource:
+@"str
+    should be with options: Ignoring case
+null
+    but was
+""null""",
+
+errorWithoutSource:
+@"""null""
+    should be with options: Ignoring case
+null
+    but was not");
+        }
+    }
+}
diff --git a/src/Shouldly.Tests/Strings/DetailedDifference/CaseSensitive/NullScenario.cs b/src/Shouldly.Tests/Strings/DetailedDifference/CaseSensitive/NullScenario.cs
@@ -0,0 +1,49 @@
+using Xunit;
+
+namespace Shouldly.Tests.Strings.DetailedDifference.CaseSensitive
+{
+    public static class NullScenario
+    {
+        [Fact]
+        public static void ShouldNotShowDifferenceWhenActualIsMissing()
+        {
+            var str = (string)null;
+            Verify.ShouldFail(() =>
+str.ShouldBe("null"),
+
+errorWithSource:
+@"str
+    should be
+""null""
+    but was
+null",
+
+errorWithoutSource:
+@"null
+    should be
+""null""
+    but was not");
+        }
+
+        [Fact]
+        public static void ShouldNotShowDifferenceWhenExpectedIsMissing()
+        {
+            var str = "null";
+            Verify.ShouldFail(() =>
+str.ShouldBe(null),
+
+errorWithSource:
+@"str
+    should be
+null
+    but was
+""null""",
+
+errorWithoutSource:
+@"""null""
+    should be
+null
+    but was not");
+        }
+    }
+}
diff --git a/src/Shouldly/DifferenceHighlighting/StringDifferenceHighlighter.cs b/src/Shouldly/DifferenceHighlighting/StringDifferenceHighlighter.cs
@@ -9,7 +9,7 @@ internal class StringDifferenceHighlighter : IStringDifferenceHighlighter
     {
         int maxDiffLength = 21;
         int maxNumberOfDiffs = 10;
-        
+
         readonly Case _sensitivity;
         readonly Func<string, string> _transform;
 
@@ -20,8 +20,7 @@ public StringDifferenceHighlighter(Case sensitivity, Func<string, string> transf
         }
         public string HighlightDifferences(string expected, string actual)
         {
-            if (expected == null) expected = "null";
-            if (actual == null) actual = "null";
+            if (expected == null || actual == null) return null;
 
             expected = _transform(expected);
             actual = _transform(actual);

diff --git a/src/Shouldly/Internals/Assertions/StringShouldBeAssertion.cs b/src/Shouldly/Internals/Assertions/StringShouldBeAssertion.cs
@@ -37,15 +37,22 @@ public string GenerateMessage(string customMessage)
             var actualValue = _actual.ToStringAwesomely();
             var expectedValue = _expected.ToStringAwesomely();
 
+            var differences = _diffHighlighter.HighlightDifferences(_expected, _actual);
+
             var actual = codeText == actualValue ? " not" : $@"
 {actualValue}";
             var message =
 $@"{codeText}
     {_shouldlyMethod}{withOption}
 {expectedValue}
-    but was{actual}
+    but was{actual}";
+
+            if (differences != null)
+            {
+                message += $@"
     difference
-{_diffHighlighter.HighlightDifferences(_expected, _actual)}";
+{differences}";
+            }
 
             if (customMessage != null)
             {