Improve diff rendering (#38)

Improve rendering of big unified diffs by leaving out unchanged parts. For text-only comparison this must be explicitly configured using `TextSnapshot.text().withContextLines(10)`. Unified diffs that are automatically generated by the framework during structure comparison will use a default value of `5` context lines. For now this value is not configurable.

Closes #37

.gitattributes

@@ -0,0 +1 @@
+snapshot-tests-core/src/test/resources/de/skuzzle/test/snapshots/impl/FailingSnapshotTests$FailBecauseSnapshotMismatchWithWhitespaces_snapshots/testWithSnapshot_0.snapshot eol=crlf

RELEASE_NOTES.md

@@ -1,6 +1,7 @@
 * [#23](https://github.com/skuzzle/snapshot-tests/issues/33): Allow to configure strictness of JSON comparison
 * [#34](https://github.com/skuzzle/snapshot-tests/issues/34): Support for HTML snapshots
 * [#36](https://github.com/skuzzle/snapshot-tests/issues/36): Throw proper `AssertionError` if actual snapshot input is null
+* [#37](https://github.com/skuzzle/snapshot-tests/issues/37): Improve rendering of huge diffs by leaving out large equal parts
 * Build against JUnit 5.9.1 (coming from 5.8.2)
 
 

readme/RELEASE_NOTES.md

@@ -1,6 +1,7 @@
 * [#23](https://github.com/skuzzle/snapshot-tests/issues/33): Allow to configure strictness of JSON comparison
 * [#34](https://github.com/skuzzle/snapshot-tests/issues/34): Support for HTML snapshots
 * [#36](https://github.com/skuzzle/snapshot-tests/issues/36): Throw proper `AssertionError` if actual snapshot input is null
+* [#37](https://github.com/skuzzle/snapshot-tests/issues/37): Improve rendering of huge diffs by leaving out large equal parts
 * Build against JUnit 5.9.1 (coming from 5.8.2)
 
 

snapshot-tests-common/pom.xml

@@ -10,7 +10,7 @@
     <artifactId>snapshot-tests-common</artifactId>
     <packaging>jar</packaging>
 
-    <name>Common Utilities</name>
+    <name>Snapshot Tests Common Utilities</name>
     <description></description>
 
     <properties>

snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/SnapshotDsl.java

@@ -120,7 +120,8 @@ public interface ChooseDataFormat {
          * Calling this method is equivalent to calling
          * <code>.as(TextSnapshot.text)</code>.
          * <p>
-         * <em>Note:</em> This method will always ignore whitespace changes!
+         * <em>Note:</em> This method will always ignore whitespace changes. This includes
+         * any changes in line breaks.
          *
          * @return Fluent API object for performing the snapshot assertion. Do NOT assume
          *         it is the same object as 'this'!

snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/DiffInterpreter.java

@@ -1,15 +1,25 @@
 package de.skuzzle.test.snapshots.data.text;
 
 import java.util.Collection;
+import java.util.Iterator;
 import java.util.regex.Pattern;
 
 import de.skuzzle.test.snapshots.data.text.diff_match_patch.Diff;
 import de.skuzzle.test.snapshots.data.text.diff_match_patch.Operation;
+import de.skuzzle.test.snapshots.validation.Arguments;
 
 final class DiffInterpreter {
 
-    private static final Pattern WHITESPACE_ONLY = Pattern.compile("\\s+");
+    private static final Pattern WHITESPACE_ONLY = Pattern.compile("\\h+");
+
     private boolean ignoreWhitespaceChanges = true;
+    private int contextLines = Integer.MAX_VALUE;
+
+    public DiffInterpreter withContextLines(int contextLines) {
+        Arguments.check(contextLines >= 0, "contextLines must be > 0 but was %d", contextLines);
+        this.contextLines = contextLines;
+        return this;
+    }
 
     public DiffInterpreter withIgnoreWhitespaceChanges(boolean ignoreWhitespaceChanges) {
         this.ignoreWhitespaceChanges = ignoreWhitespaceChanges;
@@ -34,7 +44,14 @@ private boolean isFailureDifference(Diff diff) {
         return true;
     }
 
-    public String getDisplayDiff(Diff diff) {
+    boolean hasLineSeparatorDifference(LineSeparator l1, LineSeparator l2) {
+        if (ignoreWhitespaceChanges) {
+            return false;
+        }
+        return l1 != l2;
+    }
+
+    public String renderFailureDiff(Diff diff) {
         if (isFailureDifference(diff)) {
             switch (diff.operation) {
             case DELETE:
@@ -50,14 +67,97 @@ public String getDisplayDiff(Diff diff) {
     }
 
     private String einklammern(String text) {
-        if (text.endsWith("\r\n")) {
-            return "[" + text.substring(0, text.length() - 2) + "]\r\n";
-        } else if (text.endsWith("\r")) {
-            return "[" + text.substring(0, text.length() - 1) + "]\r";
-        } else if (text.endsWith("\n")) {
-            return "[" + text.substring(0, text.length() - 1) + "]\n";
+        // invariant: Diff instances always use System line separators
+        final String lineSeparator = LineSeparator.SYSTEM.toString();
+
+        if (text.endsWith(lineSeparator)) {
+            return "[" + text.substring(0, text.length() - lineSeparator.length()) + "]" + lineSeparator;
         } else {
             return "[" + text + "]";
         }
     }
+
+    public String renderEqualsDiff(String diffText, EqualDiffPosition position) {
+        return position.render(diffText, contextLines);
+    }
+
+    enum EqualDiffPosition {
+        START {
+
+            @Override
+            protected String render(String diffText, int contextLines) {
+                final int totalLines = (int) diffText.lines().count();
+
+                final StringBuilder b = new StringBuilder();
+                boolean appendOnce = true;
+                final Iterator<String> lineIterator = diffText.lines().iterator();
+                for (int lineNr = 0; lineNr < totalLines; ++lineNr) {
+                    final String nextLine = lineIterator.next();
+
+                    if (lineNr >= totalLines - contextLines) {
+                        b.append(nextLine);
+                        if (lineNr < totalLines - 1 || LineSeparator.SYSTEM.endsWith(diffText)) {
+                            b.append(LineSeparator.SYSTEM);
+                        }
+                    } else if (appendOnce) {
+                        b.append("[...]").append(LineSeparator.SYSTEM);
+                        appendOnce = false;
+                    }
+                }
+                return b.toString();
+            }
+
+        },
+        MIDDLE {
+            @Override
+            protected String render(String diffText, int contextLines) {
+                final int totalLines = (int) diffText.lines().count();
+
+                final StringBuilder b = new StringBuilder();
+                boolean appendOnce = true;
+                final Iterator<String> lineIterator = diffText.lines().iterator();
+                for (int lineNr = 0; lineNr < totalLines; ++lineNr) {
+                    final String nextLine = lineIterator.next();
+
+                    if (lineNr < contextLines || lineNr >= totalLines - contextLines) {
+                        b.append(nextLine);
+                        if (lineNr < totalLines - 1) {
+                            b.append(LineSeparator.SYSTEM);
+                        }
+                    } else if (appendOnce) {
+                        b.append("[...]").append(LineSeparator.SYSTEM);
+                        appendOnce = false;
+                    }
+                }
+                return b.toString();
+            }
+        },
+        END {
+            @Override
+            protected String render(String diffText, int contextLines) {
+                final int totalLines = (int) diffText.lines().count();
+
+                final StringBuilder b = new StringBuilder();
+                boolean appendOnce = true;
+                final Iterator<String> lineIterator = diffText.lines().iterator();
+                for (int lineNr = 0; lineNr < totalLines; ++lineNr) {
+                    final String nextLine = lineIterator.next();
+
+                    if (lineNr < contextLines) {
+                        b.append(nextLine);
+                        if (lineNr < totalLines - 1) {
+                            b.append(LineSeparator.SYSTEM);
+                        }
+                    } else if (appendOnce) {
+                        b.append("[...]");
+                        appendOnce = false;
+                    }
+                }
+                return b.toString();
+            }
+        };
+
+        protected abstract String render(String diffText, int contextLines);
+    }
+
 }

snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/LineSeparator.java

@@ -0,0 +1,67 @@
+package de.skuzzle.test.snapshots.data.text;
+
+import java.util.stream.Collectors;
+
+/**
+ * Used to represent a file's line separator.
+ *
+ * @since 1.5.0
+ * @author Simon Taddiken
+ */
+enum LineSeparator {
+    // note: don't reorder!
+    CRLF("\r\n", "\\r\\n"),
+    LF("\n", "\\n"),
+    CR("\r", "\\r"),
+    DEFAULT("\n", "\\n"),
+    SYSTEM(System.lineSeparator(), System.lineSeparator().replace("\r", "\\r").replace("\n", "\\n"));
+
+    private final String characters;
+    private final String displayName;
+
+    private LineSeparator(String characters, String displayName) {
+        this.characters = characters;
+        this.displayName = displayName;
+    }
+
+    public static LineSeparator determineFrom(String s) {
+        for (final LineSeparator lineSeparator : LineSeparator.values()) {
+            if (lineSeparator.existsIn(s)) {
+                return lineSeparator;
+            }
+        }
+        return DEFAULT;
+    }
+
+    private boolean existsIn(String s) {
+        return s.indexOf(characters) >= 0;
+    }
+
+    public String displayName() {
+        return name() + "(" + displayName + ")";
+    }
+
+    public boolean endsWith(String s) {
+        return s.endsWith(this.characters);
+    }
+
+    public boolean startsWith(String s) {
+        return s.startsWith(this.characters);
+    }
+
+    /**
+     * Converts all line separators in the given String to the line separator represented
+     * by the enum constant on which this method is called.
+     *
+     * @param s The string in which to convert the line separators.
+     * @return The string with converted line separators.
+     */
+    public String convert(String s) {
+        return s.lines().collect(Collectors.joining(characters));
+    }
+
+    @Override
+    public String toString() {
+        return characters;
+    }
+}

snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/TextDiff.java

@@ -1,11 +1,16 @@
 package de.skuzzle.test.snapshots.data.text;
 
 import java.util.LinkedList;
+import java.util.List;
+import java.util.ListIterator;
 
 import org.apiguardian.api.API;
 import org.apiguardian.api.API.Status;
 
+import de.skuzzle.test.snapshots.data.text.DiffInterpreter.EqualDiffPosition;
 import de.skuzzle.test.snapshots.data.text.diff_match_patch.Diff;
+import de.skuzzle.test.snapshots.data.text.diff_match_patch.Operation;
+import de.skuzzle.test.snapshots.validation.Arguments;
 
 /**
  * Creates a unified diff of 2 Strings using the popular Neil Fraser diff_match_patch
@@ -17,33 +22,90 @@
 public final class TextDiff {
 
     private final DiffInterpreter diffInterpreter;
-    private final LinkedList<Diff> diffs;
+    private final List<Diff> diffs;
+    private final LineSeparator expectedLineSeparator;
+    private final LineSeparator actualLineSeparator;
 
-    private TextDiff(DiffInterpreter diffInterpreter, LinkedList<Diff> diffs) {
+    private TextDiff(DiffInterpreter diffInterpreter, List<Diff> diffs,
+            LineSeparator expectedLineSeparator, LineSeparator actualLineSeparator) {
         this.diffInterpreter = diffInterpreter;
         this.diffs = diffs;
+        this.expectedLineSeparator = expectedLineSeparator;
+        this.actualLineSeparator = actualLineSeparator;
     }
 
-    public static TextDiff diffOf(String expected, String actual) {
-        return diffOf(new DiffInterpreter().withIgnoreWhitespaceChanges(false), expected, actual);
+    public static TextDiff diffOf(String expected, String actual, int contextLines) {
+        return diffOf(new DiffInterpreter()
+                .withIgnoreWhitespaceChanges(false)
+                .withContextLines(contextLines),
+                expected, actual);
     }
 
     static TextDiff diffOf(DiffInterpreter diffInterpreter, String expected, String actual) {
+        Arguments.requireNonNull(expected, "expected String must not be null");
+        Arguments.requireNonNull(actual, "actual String must not be null");
+
+        final LineSeparator expectedLineSeparator = LineSeparator.determineFrom(expected);
+        final LineSeparator actualLineSeparator = LineSeparator.determineFrom(actual);
+
         final diff_match_patch diff_match_patch = new diff_match_patch();
-        final LinkedList<Diff> diffs = diff_match_patch.diff_main(expected, actual);
-        diff_match_patch.diff_cleanupEfficiency(diffs);
-        return new TextDiff(diffInterpreter, diffs);
+
+        final LinkedList<Diff> diffs = diff_match_patch.diff_main(
+                sanitizeLineSeparators(expected),
+                sanitizeLineSeparators(actual));
+        diff_match_patch.diff_cleanupSemanticLossless(diffs);
+
+        return new TextDiff(diffInterpreter, diffs, expectedLineSeparator, actualLineSeparator);
+    }
+
+    private static String sanitizeLineSeparators(String s) {
+        return LineSeparator.SYSTEM.convert(s);
     }
 
     public boolean hasDifference() {
-        return diffInterpreter.hasFailures(diffs);
+        return diffInterpreter.hasFailures(diffs)
+                || diffInterpreter.hasLineSeparatorDifference(expectedLineSeparator, actualLineSeparator);
     }
 
     @Override
     public String toString() {
+        if (diffs.isEmpty()) {
+            return "";
+        }
+
         final StringBuilder message = new StringBuilder();
-        for (final Diff diff : diffs) {
-            message.append(diffInterpreter.getDisplayDiff(diff));
+
+        if (diffInterpreter.hasLineSeparatorDifference(expectedLineSeparator, actualLineSeparator)) {
+            message.append(String.format(
+                    "Strings differ in linebreaks. Expected: '%s', Actual encountered: '%s'",
+                    expectedLineSeparator.displayName(), actualLineSeparator.displayName()));
+
+            if (diffs.size() == 1 && diffs.get(0).operation == Operation.EQUAL) {
+                return message.toString();
+            }
+
+            message.append(LineSeparator.SYSTEM).append(LineSeparator.SYSTEM);
+        }
+
+        final ListIterator<Diff> cursor = diffs.listIterator();
+        while (cursor.hasNext()) {
+            final boolean hasPrevious = cursor.hasPrevious();
+
+            final Diff current = cursor.next();
+            if (current.operation == Operation.EQUAL && !hasPrevious) {
+                // equal operation at the beginning
+                message.append(diffInterpreter.renderEqualsDiff(current.text, EqualDiffPosition.START));
+            } else if (current.operation == Operation.EQUAL) {
+                if (cursor.hasNext()) {
+                    // equal operation between 2 changes
+                    message.append(diffInterpreter.renderEqualsDiff(current.text, EqualDiffPosition.MIDDLE));
+                } else {
+                    // equal diff at the end
+                    message.append(diffInterpreter.renderEqualsDiff(current.text, EqualDiffPosition.END));
+                }
+            } else {
+                message.append(diffInterpreter.renderFailureDiff(current));
+            }
         }
         return message.toString();
     }