From 3d3e0d29ffa49e79901ffc3aee7383355b2d0890 Mon Sep 17 00:00:00 2001 From: Simon Taddiken Date: Thu, 23 Mar 2023 09:51:44 +0100 Subject: [PATCH 1/9] Extract diff creation into own module. --- build.gradle | 3 +- diff-tool/README.md | 96 ++++++++++++ diff-tool/build.gradle | 9 ++ .../de/skuzzle/difftool/DiffAlgorithm.java | 8 + .../java/de/skuzzle/difftool/DiffLine.java | 55 +++++++ .../de/skuzzle/difftool/DiffRenderer.java | 18 +++ .../de/skuzzle/difftool/DiffSettings.java | 76 +++++++++ .../de/skuzzle/difftool}/LineSeparator.java | 39 +++-- .../skuzzle/difftool}/SplitDiffRenderer.java | 101 ++++++------ .../java/de/skuzzle/difftool/StringDiff.java | 147 ++++++++++++++++++ .../difftool}/UnifiedDiffRenderer.java | 53 ++++--- .../main/java/de/skuzzle/difftool/Util.java | 28 ++++ .../thirdparty/DiffUtilsDiffAlgorithm.java | 67 ++++++++ readme/RELEASE_NOTES.md | 33 ++-- settings.gradle | 3 +- snapshot-tests-core/build.gradle | 1 + .../data/text/DiffListLookahead.java | 22 --- .../snapshots/data/text/DiffRenderer.java | 10 -- .../test/snapshots/data/text/TextDiff.java | 56 ++++--- .../snapshots/data/text/TextSnapshot.java | 7 +- .../snapshots/data/text/TextDiffTest.java | 8 +- .../src/docs/asciidoc/index.adoc | 3 + 22 files changed, 669 insertions(+), 174 deletions(-) create mode 100644 diff-tool/README.md create mode 100644 diff-tool/build.gradle create mode 100644 diff-tool/src/main/java/de/skuzzle/difftool/DiffAlgorithm.java create mode 100644 diff-tool/src/main/java/de/skuzzle/difftool/DiffLine.java create mode 100644 diff-tool/src/main/java/de/skuzzle/difftool/DiffRenderer.java create mode 100644 diff-tool/src/main/java/de/skuzzle/difftool/DiffSettings.java rename {snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text => diff-tool/src/main/java/de/skuzzle/difftool}/LineSeparator.java (62%) rename {snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text => diff-tool/src/main/java/de/skuzzle/difftool}/SplitDiffRenderer.java (55%) create mode 100644 diff-tool/src/main/java/de/skuzzle/difftool/StringDiff.java rename {snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text => diff-tool/src/main/java/de/skuzzle/difftool}/UnifiedDiffRenderer.java (55%) create mode 100644 diff-tool/src/main/java/de/skuzzle/difftool/Util.java create mode 100644 diff-tool/src/main/java/de/skuzzle/difftool/thirdparty/DiffUtilsDiffAlgorithm.java delete mode 100644 snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/DiffListLookahead.java delete mode 100644 snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/DiffRenderer.java diff --git a/build.gradle b/build.gradle index fbdf2647..4a8109ee 100644 --- a/build.gradle +++ b/build.gradle @@ -66,7 +66,8 @@ ext { projects.snapshotTestsXml, projects.snapshotTestsJaxbJakarta, projects.snapshotTestsNormalize, - projects.snapshotTestsDirectoryParams + projects.snapshotTestsDirectoryParams, + projects.diffTool ); } diff --git a/diff-tool/README.md b/diff-tool/README.md new file mode 100644 index 00000000..b5feffe5 --- /dev/null +++ b/diff-tool/README.md @@ -0,0 +1,96 @@ +# Diff-Tool + +This is a thin wrapper around `java-diff-utils` which provides the convenience of rendering diffs to a nice human +readable String. + +## Usage + +```java +final String expected = "..."; +final String actual = "..."; + +// Create a diff using default configuration +final StringDiff diff1 = StringDiff.simple(expected, actual); + +// Render as unified diff +final String unifiedDiff = diff1.toString(); +// Render as split diff +final String splitDiff = diff1.toString(SplitDiffRenderer.INSTANCE); + +// Test whether compared Strings differed in line separators +final boolean lineSeparatorDifference = diff1.hasLineSeparatorDifference() +// Tests whether compared Strings differed in actual text +final boolean textDifference = diff1.hasTextDifference() + +// Create a diff by configuring the java-diff-utils _DiffRowGenerator_ +final DiffAlgorithm diffAlgorithm = DiffUtilsDiffAlgorithm.create(builder -> builder + .showInlineDiffs(true) + .inlineDiffByWord(true) + .ignoreWhiteSpaces(true)); + +final StringDiff diff2 = StringDiff.using(diffAlgorithm, expected, actual); +``` + +## Sample output + +Unified Diff: +``` +[...] + 6 6 Some unchanged lines6 + 7 7 Some unchanged lines7 + 8 8 Some unchanged lines8 + 9 - This is a test <>. + 9 + This is a test <>. + 10 10 This is the second line. + 11 11 Some unchanged lines9 + 12 12 Some unchanged lines10 + 13 13 Some unchanged lines11 + 14 14 Some unchanged lines12 + 15 - And here is the finish with way more than 80 characters and I'm very curious how this is going to be displayed in split view diff. + 16 15 This line is unchanged + 16 + This line has been added + 17 17 Some unchanged lines13 + 18 18 Some unchanged lines14 + 19 19 Some unchanged lines15 +[...] +[...] + 22 22 Some unchanged lines18 + 23 23 Some unchanged lines19 + 24 24 Some unchanged lines20 + 25 - <> <> + 25 + <> <> + 26 26 Some unchanged lines21 + 27 27 Some unchanged lines22 + 28 28 Some unchanged lines23 +[...] +``` + +Split Diff +``` +[...] + 6 Some unchanged lines6 | 6 Some unchanged lines6 + 7 Some unchanged lines7 | 7 Some unchanged lines7 + 8 Some unchanged lines8 | 8 Some unchanged lines8 + 9 ! This is a test <>. | 9 ! This is a test <>. + 10 This is the second line. | 10 This is the second line. + 11 Some unchanged lines9 | 11 Some unchanged lines9 + 12 Some unchanged lines10 | 12 Some unchanged lines10 + 13 Some unchanged lines11 | 13 Some unchanged lines11 + 14 Some unchanged lines12 | 14 Some unchanged lines12 + 15 - And here is the finish with way more than 80 characters and I'm very curious how this is going to be displayed in split view diff. | + 16 This line is unchanged | 15 This line is unchanged + | 16 + This line has been added + 17 Some unchanged lines13 | 17 Some unchanged lines13 + 18 Some unchanged lines14 | 18 Some unchanged lines14 + 19 Some unchanged lines15 | 19 Some unchanged lines15 +[...] +[...] + 22 Some unchanged lines18 | 22 Some unchanged lines18 + 23 Some unchanged lines19 | 23 Some unchanged lines19 + 24 Some unchanged lines20 | 24 Some unchanged lines20 + 25 ! <> <> | 25 ! <> <> + 26 Some unchanged lines21 | 26 Some unchanged lines21 + 27 Some unchanged lines22 | 27 Some unchanged lines22 + 28 Some unchanged lines23 | 28 Some unchanged lines23 +[...] +``` \ No newline at end of file diff --git a/diff-tool/build.gradle b/diff-tool/build.gradle new file mode 100644 index 00000000..5c01de3c --- /dev/null +++ b/diff-tool/build.gradle @@ -0,0 +1,9 @@ +plugins { + id("snapshot-tests.published-java-component") +} +description = "Diff Tool" +ext.automaticModuleName = "de.skuzzle.test.snapshots.difftool" + +dependencies { + api(libs.javadiffutils) +} diff --git a/diff-tool/src/main/java/de/skuzzle/difftool/DiffAlgorithm.java b/diff-tool/src/main/java/de/skuzzle/difftool/DiffAlgorithm.java new file mode 100644 index 00000000..e933fc37 --- /dev/null +++ b/diff-tool/src/main/java/de/skuzzle/difftool/DiffAlgorithm.java @@ -0,0 +1,8 @@ +package de.skuzzle.difftool; + +import java.util.List; + +public interface DiffAlgorithm { + + List diffOf(List left, List right); +} diff --git a/diff-tool/src/main/java/de/skuzzle/difftool/DiffLine.java b/diff-tool/src/main/java/de/skuzzle/difftool/DiffLine.java new file mode 100644 index 00000000..561e8805 --- /dev/null +++ b/diff-tool/src/main/java/de/skuzzle/difftool/DiffLine.java @@ -0,0 +1,55 @@ +package de.skuzzle.difftool; + +import java.util.Objects; + +public final class DiffLine { + + private final Type type; + private final String oldLine; + private final String newLine; + + public DiffLine(Type type, String oldLine, String newLine) { + this.type = Objects.requireNonNull(type); + this.oldLine = Objects.requireNonNull(oldLine); + this.newLine = Objects.requireNonNull(newLine); + } + + public enum Type { + INSERT, + DELETE, + CHANGE, + EQUAL + } + + public Type type() { + return type; + } + + public String oldLine() { + return oldLine; + } + + public String newLine() { + return newLine; + } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + final DiffLine diffLine = (DiffLine) o; + return type == diffLine.type && oldLine.equals(diffLine.oldLine) && newLine.equals(diffLine.newLine); + } + + @Override + public int hashCode() { + return Objects.hash(type, oldLine, newLine); + } + + @Override + public String toString() { + return "[" + this.type + "," + this.oldLine + "," + this.newLine + "]"; + } +} diff --git a/diff-tool/src/main/java/de/skuzzle/difftool/DiffRenderer.java b/diff-tool/src/main/java/de/skuzzle/difftool/DiffRenderer.java new file mode 100644 index 00000000..3aad4514 --- /dev/null +++ b/diff-tool/src/main/java/de/skuzzle/difftool/DiffRenderer.java @@ -0,0 +1,18 @@ +package de.skuzzle.difftool; + +import java.util.List; + +/** + * Defines how a diff is rendered as a String. + */ +public interface DiffRenderer { + + /** + * Creates a String representation of the provided diff. + * + * @param rows The diff. + * @param diffSettings Additional parameters. + * @return The String representation. + */ + String renderDiff(List rows, DiffSettings diffSettings); +} diff --git a/diff-tool/src/main/java/de/skuzzle/difftool/DiffSettings.java b/diff-tool/src/main/java/de/skuzzle/difftool/DiffSettings.java new file mode 100644 index 00000000..fb933b03 --- /dev/null +++ b/diff-tool/src/main/java/de/skuzzle/difftool/DiffSettings.java @@ -0,0 +1,76 @@ +package de.skuzzle.difftool; + +/** + * Defines parameters that are used when rendering a diff to String using a + * {@link DiffRenderer}. + */ +public final class DiffSettings { + + public static final DiffSettings DEFAULT = new DiffSettings(5, 0, DiffSymbols.DEFAULT); + + private final int contextLines; + private final int lineNumberOffset; + private final DiffSymbols symbols; + + public DiffSettings(int contextLines, int lineNumberOffset, DiffSymbols symbols) { + this.contextLines = contextLines; + this.lineNumberOffset = lineNumberOffset; + this.symbols = symbols; + } + + public static DiffSettings withDefaultSymbols(int contextLines, int lineNumberOffset) { + return new DiffSettings(contextLines, lineNumberOffset, DiffSymbols.DEFAULT); + } + + public int contextLines() { + return contextLines; + } + + public int lineNumberOffset() { + return lineNumberOffset; + } + + public DiffSymbols symbols() { + return symbols; + } + + public static final class DiffSymbols { + + public static DiffSymbols DEFAULT = new DiffSymbols("!", "+", "-", " ", LineSeparator.SYSTEM); + + private final String changedLine; + private final String addedLine; + private final String deletedLine; + private final String equalLine; + private final LineSeparator newLineCharacter; + + public DiffSymbols(String changedLine, String addedLine, String deletedLine, String equalLine, + LineSeparator newLineCharacter) { + this.changedLine = changedLine; + this.addedLine = addedLine; + this.deletedLine = deletedLine; + this.equalLine = equalLine; + this.newLineCharacter = newLineCharacter; + } + + public String changedLine() { + return changedLine; + } + + public String addedLine() { + return addedLine; + } + + public String deletedLine() { + return deletedLine; + } + + public String equalLine() { + return equalLine; + } + + public LineSeparator newLineCharacter() { + return newLineCharacter; + } + } +} diff --git a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/LineSeparator.java b/diff-tool/src/main/java/de/skuzzle/difftool/LineSeparator.java similarity index 62% rename from snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/LineSeparator.java rename to diff-tool/src/main/java/de/skuzzle/difftool/LineSeparator.java index 1f0108a0..32cfd975 100644 --- a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/LineSeparator.java +++ b/diff-tool/src/main/java/de/skuzzle/difftool/LineSeparator.java @@ -1,54 +1,59 @@ -package de.skuzzle.test.snapshots.data.text; +package de.skuzzle.difftool; import java.util.stream.Collectors; /** - * Used to represent and detect a file's line separator character(s). + * Used to represent and detect a file's line separator character(s). The + * {@link #toString()} method will return the plain line break characters. * - * @since 1.5.0 * @author Simon Taddiken + * @since 1.5.0 */ -enum LineSeparator { +public 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")); + 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) { + LineSeparator(String characters, String displayName) { this.characters = characters; this.displayName = displayName; } + /** + * Determines the line separator used in the given String. Will return the type of the + * first line separator found in the String. + *

+ * If the String does not contain any line separators, will return + * {@link LineSeparator#SYSTEM}. + *

+ * + * @param s The String to check. + * @return The type of line endings. + */ public static LineSeparator determineFrom(String s) { for (final LineSeparator lineSeparator : LineSeparator.values()) { if (lineSeparator.existsIn(s)) { return lineSeparator; } } - return DEFAULT; + return SYSTEM; } private boolean existsIn(String s) { - return s.indexOf(characters) >= 0; + return s.contains(characters); } 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. diff --git a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/SplitDiffRenderer.java b/diff-tool/src/main/java/de/skuzzle/difftool/SplitDiffRenderer.java similarity index 55% rename from snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/SplitDiffRenderer.java rename to diff-tool/src/main/java/de/skuzzle/difftool/SplitDiffRenderer.java index 16ac94b3..e1a0bd8d 100644 --- a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/SplitDiffRenderer.java +++ b/diff-tool/src/main/java/de/skuzzle/difftool/SplitDiffRenderer.java @@ -1,44 +1,26 @@ -package de.skuzzle.test.snapshots.data.text; +package de.skuzzle.difftool; import java.util.List; -import com.github.difflib.text.DiffRow; +/** + * Renders the diff in "split" view. + */ +public final class SplitDiffRenderer implements DiffRenderer { -final class SplitDiffRenderer implements DiffRenderer { - - private static final String OPERATION_CHANGE = "!"; - private static final String OPERATION_ADD = "+"; - private static final String OPERATION_DELETE = "-"; - private static final String OPERATION_EQUAL = " "; - - private String padLeft(String text, int targetWidth) { - final int missingSpaces = targetWidth - text.length(); - return " ".repeat(missingSpaces) + text; - } - - private String padRight(String text, int targetWidth) { - final int missingSpaces = targetWidth - text.length(); - return text + " ".repeat(missingSpaces); - } - - private void appendRow(StringBuilder b, int columnWidth, String paddedExpectedLineNumber, String expectedOperation, - String expectedLine, String paddedActualLineNumber, String actualOperation, String actualLine) { - b.append(paddedExpectedLineNumber) - .append(" ") - .append(expectedOperation) - .append(" ") - .append(padRight(expectedLine, columnWidth)) - .append(" |") - .append(paddedActualLineNumber) - .append(" ").append(actualOperation).append(" ").append(actualLine); + public static final DiffRenderer INSTANCE = new SplitDiffRenderer(); + private SplitDiffRenderer() { + // hidden } @Override - public String renderDiff(List rows, int contextLines, int lineNumberOffset) { - final int columnWidth = rows.stream().map(DiffRow::getOldLine).mapToInt(String::length).max().orElse(80); + public String renderDiff(List rows, DiffSettings diffSettings) { + final int columnWidth = rows.stream().map(DiffLine::oldLine).mapToInt(String::length).max().orElse(80); final int rowCountEstimate = rows.size(); final int rowNumberWidth = Math.max(2, (int) Math.log10(rowCountEstimate) + 1) + 1; + final int lineNumberOffset = diffSettings.lineNumberOffset(); + final int contextLines = diffSettings.contextLines(); + final DiffSettings.DiffSymbols symbols = diffSettings.symbols(); int expectedLine = 1 + lineNumberOffset; int actualLine = 1 + lineNumberOffset; @@ -47,23 +29,23 @@ public String renderDiff(List rows, int contextLines, int lineNumberOff int indexOfLastDifference = -1; for (int i = 0; i < rows.size(); i++) { - final DiffRow diffRow = rows.get(i); + final DiffLine diffRow = rows.get(i); - switch (diffRow.getTag()) { + switch (diffRow.type()) { case CHANGE: indexOfLastDifference = i; appendRow(b, columnWidth, - padLeft("" + expectedLine, rowNumberWidth), - OPERATION_CHANGE, - diffRow.getOldLine(), - padLeft("" + actualLine, rowNumberWidth), - OPERATION_CHANGE, diffRow.getNewLine()); + Util.padLeft("" + expectedLine, rowNumberWidth), + symbols.changedLine(), + diffRow.oldLine(), + Util.padLeft("" + actualLine, rowNumberWidth), + symbols.changedLine(), diffRow.newLine()); expectedLine++; actualLine++; break; case EQUAL: - final int indexOfNextDifference = DiffListLookahead.indexOfNextNonEqual(rows, i); + final int indexOfNextDifference = Util.indexOfNextNonEqual(rows, i); final int distanceToNextDifference = indexOfNextDifference == rows.size() ? Integer.MAX_VALUE @@ -74,11 +56,11 @@ public String renderDiff(List rows, int contextLines, int lineNumberOff if (distanceToNextDifference < contextLines || distanceToPrevDifference < contextLines) { appendRow(b, columnWidth, - padLeft("" + expectedLine, rowNumberWidth), - OPERATION_EQUAL, - diffRow.getOldLine(), - padLeft("" + actualLine, rowNumberWidth), - OPERATION_EQUAL, diffRow.getNewLine()); + Util.padLeft("" + expectedLine, rowNumberWidth), + symbols.equalLine(), + diffRow.oldLine(), + Util.padLeft("" + actualLine, rowNumberWidth), + symbols.equalLine(), diffRow.newLine()); } else if (distanceToNextDifference == contextLines || distanceToPrevDifference == contextLines) { b.append("[...]"); @@ -96,11 +78,11 @@ public String renderDiff(List rows, int contextLines, int lineNumberOff indexOfLastDifference = i; appendRow(b, columnWidth, - padLeft("" + expectedLine, rowNumberWidth), - OPERATION_DELETE, - diffRow.getOldLine(), + Util.padLeft("" + expectedLine, rowNumberWidth), + symbols.deletedLine(), + diffRow.oldLine(), " ".repeat(rowNumberWidth), - OPERATION_EQUAL, diffRow.getNewLine()); + symbols.equalLine(), diffRow.newLine()); expectedLine++; break; @@ -110,10 +92,10 @@ public String renderDiff(List rows, int contextLines, int lineNumberOff appendRow(b, columnWidth, " ".repeat(rowNumberWidth), - OPERATION_EQUAL, - diffRow.getOldLine(), - padLeft("" + actualLine, rowNumberWidth), - OPERATION_ADD, diffRow.getNewLine()); + symbols.equalLine(), + diffRow.oldLine(), + Util.padLeft("" + actualLine, rowNumberWidth), + symbols.addedLine(), diffRow.newLine()); actualLine++; break; @@ -123,10 +105,23 @@ public String renderDiff(List rows, int contextLines, int lineNumberOff } if (i < rows.size() - 1) { - b.append(LineSeparator.SYSTEM); + b.append(symbols.newLineCharacter()); } } return b.toString(); } + + private void appendRow(StringBuilder b, int columnWidth, String paddedExpectedLineNumber, String expectedOperation, + String expectedLine, String paddedActualLineNumber, String actualOperation, String actualLine) { + b.append(paddedExpectedLineNumber) + .append(" ") + .append(expectedOperation) + .append(" ") + .append(Util.padRight(expectedLine, columnWidth)) + .append(" |") + .append(paddedActualLineNumber) + .append(" ").append(actualOperation).append(" ").append(actualLine); + + } } diff --git a/diff-tool/src/main/java/de/skuzzle/difftool/StringDiff.java b/diff-tool/src/main/java/de/skuzzle/difftool/StringDiff.java new file mode 100644 index 00000000..e9e771b9 --- /dev/null +++ b/diff-tool/src/main/java/de/skuzzle/difftool/StringDiff.java @@ -0,0 +1,147 @@ +package de.skuzzle.difftool; + +import java.util.List; +import java.util.Objects; +import java.util.stream.Collectors; + +import de.skuzzle.difftool.thirdparty.DiffUtilsDiffAlgorithm; + +/** + * Represents the difference between two Strings. The difference is calculated + * row wise by a {@link DiffAlgorithm} and is internally represented as a List of + * {@link DiffLine}. + *

+ * Differences in line breaks are not part of the actual diff but will be determined as + * well. + *

+ * + * Whether other whitespaces are subject to comparison depends on the used + * {@linkplain DiffAlgorithm}. + *

+ * A {@linkplain StringDiff} can be rendered as a nice human readable String using a + * {@link DiffRenderer}. Rendering behavior can be fine tuned by providing a + * {@link DiffSettings} instance. + *

+ */ +public final class StringDiff { + + private static final DiffAlgorithm DEFAULT_ALGORITHM = DiffUtilsDiffAlgorithm.INSTANCE; + private static final DiffRenderer DEFAULT_RENDERER = UnifiedDiffRenderer.INSTANCE; + private static final DiffSettings DEFAULT_SETTINGS = DiffSettings.DEFAULT; + + private final List diff; + private final LineSeparator leftLineSeparator; + private final LineSeparator rightLineSeparator; + + private StringDiff(List diff, LineSeparator leftLineSeparator, LineSeparator rightLineSeparator) { + this.diff = Objects.requireNonNull(List.copyOf(diff)); + this.leftLineSeparator = Objects.requireNonNull(leftLineSeparator); + this.rightLineSeparator = Objects.requireNonNull(rightLineSeparator); + } + + public static StringDiff simple(String left, String right) { + return using(DEFAULT_ALGORITHM, left, right); + } + + public static StringDiff using(DiffAlgorithm algorithm, List left, List right) { + final List diff = algorithm.diffOf(left, right); + return new StringDiff(diff, LineSeparator.SYSTEM, LineSeparator.SYSTEM); + } + + public static StringDiff using(DiffAlgorithm algorithm, String left, String right) { + final LineSeparator leftLineSeparator = LineSeparator.determineFrom(left); + final LineSeparator rightLineSeparator = LineSeparator.determineFrom(right); + + final List diff = algorithm.diffOf( + left.lines().collect(Collectors.toUnmodifiableList()), + right.lines().collect(Collectors.toUnmodifiableList())); + + return new StringDiff(diff, leftLineSeparator, rightLineSeparator); + } + + /** + * Returns the internal list of differences (read-only). + * + * @return The list of differences. + */ + public List textDifferences() { + return diff; + } + + /** + * Returns the {@link LineSeparator} for the left file of the comparison. + * + * @return The line separator. + */ + public LineSeparator leftLineSeparator() { + return leftLineSeparator; + } + + /** + * Returns the {@link LineSeparator} of the right file of the comparison. + * + * @return The line separator. + */ + public LineSeparator rightLineSeparator() { + return rightLineSeparator; + } + + /** + * Whether the compared Strings differ in either line separators or at least one text + * difference was found. Equivalent to + * + * 
+     * hasDifference = hasLineSeparatorDifference() || hasTextDifference();
+     *
+ * + * @return Whether any difference has been found. + */ + public boolean hasDifference() { + return hasLineSeparatorDifference() || hasTextDifference(); + } + + /** + * Whether at least one text difference has been found. + * + * @return Whether at least one difference has been found. + */ + public boolean hasTextDifference() { + return diff.stream().map(DiffLine::type).anyMatch(type -> type != DiffLine.Type.EQUAL); + } + + /** + * Whether compared Strings differed in line separators. + * + * @return Whether compared Strings differed in line separators. + */ + public boolean hasLineSeparatorDifference() { + return !leftLineSeparator.equals(rightLineSeparator); + } + + /** + * Creates a String representation of this diff via the given {@link DiffRenderer} + * using given {@link DiffSettings}. + * + * @param diffRenderer The renderer. + * @param diffSettings The settings that will be passed to the renderer. + * @return The String representation. + */ + public String toString(DiffRenderer diffRenderer, DiffSettings diffSettings) { + return diffRenderer.renderDiff(diff, diffSettings); + } + + public String toString(DiffRenderer diffRenderer) { + return toString(diffRenderer, DEFAULT_SETTINGS); + } + + /** + * Creates a unified rendered diff of this StringDiff using default + * {@link DiffSettings}. + * + * @return A String representation of this diff. + */ + @Override + public String toString() { + return toString(DEFAULT_RENDERER, DEFAULT_SETTINGS); + } +} diff --git a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/UnifiedDiffRenderer.java b/diff-tool/src/main/java/de/skuzzle/difftool/UnifiedDiffRenderer.java similarity index 55% rename from snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/UnifiedDiffRenderer.java rename to diff-tool/src/main/java/de/skuzzle/difftool/UnifiedDiffRenderer.java index 90f9be8a..d30b1919 100644 --- a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/UnifiedDiffRenderer.java +++ b/diff-tool/src/main/java/de/skuzzle/difftool/UnifiedDiffRenderer.java @@ -1,20 +1,26 @@ -package de.skuzzle.test.snapshots.data.text; +package de.skuzzle.difftool; import java.util.List; -import com.github.difflib.text.DiffRow; +public final class UnifiedDiffRenderer implements DiffRenderer { -final class UnifiedDiffRenderer implements DiffRenderer { + public static final DiffRenderer INSTANCE = new UnifiedDiffRenderer(); - private String padLeft(String text, int targetWidth) { - final int missingSpaces = targetWidth - text.length(); - return " ".repeat(missingSpaces) + text; + private UnifiedDiffRenderer() { + // hidden } @Override - public String renderDiff(List rows, int contextLines, int lineNumberOffset) { + public String renderDiff(List rows, DiffSettings diffSettings) { final int rowCountEstimate = rows.size(); final int rowNumberWidth = Math.max(2, (int) Math.log10(rowCountEstimate) + 1) + 1; + final int lineNumberOffset = diffSettings.lineNumberOffset(); + final int contextLines = diffSettings.contextLines(); + final DiffSettings.DiffSymbols symbols = diffSettings.symbols(); + + final String equalLine = " " + symbols.equalLine() + " "; + final String deletedLine = " " + symbols.deletedLine() + " "; + final String addedLine = " " + symbols.addedLine() + " "; int expectedLine = 1 + lineNumberOffset; int actualLine = 1 + lineNumberOffset; @@ -22,11 +28,11 @@ public String renderDiff(List rows, int contextLines, int lineNumberOff int indexOfLastDifference = -1; for (int i = 0; i < rows.size(); i++) { - final DiffRow diffRow = rows.get(i); + final DiffLine diffRow = rows.get(i); - switch (diffRow.getTag()) { + switch (diffRow.type()) { case EQUAL: - final int indexOfNextDifference = DiffListLookahead.indexOfNextNonEqual(rows, i); + final int indexOfNextDifference = Util.indexOfNextNonEqual(rows, i); final int distanceToNextDifference = indexOfNextDifference == rows.size() ? Integer.MAX_VALUE @@ -37,9 +43,9 @@ public String renderDiff(List rows, int contextLines, int lineNumberOff if (distanceToNextDifference < contextLines || distanceToPrevDifference < contextLines) { b - .append(padLeft("" + expectedLine, rowNumberWidth)) - .append(padLeft("" + actualLine, rowNumberWidth)) - .append(" ").append(diffRow.getOldLine()); + .append(Util.padLeft("" + expectedLine, rowNumberWidth)) + .append(Util.padLeft("" + actualLine, rowNumberWidth)) + .append(equalLine).append(diffRow.oldLine()); } else if (distanceToNextDifference == contextLines || distanceToPrevDifference == contextLines) { b.append("[...]"); @@ -56,13 +62,13 @@ public String renderDiff(List rows, int contextLines, int lineNumberOff case CHANGE: indexOfLastDifference = i; b - .append(padLeft("" + expectedLine, rowNumberWidth)) + .append(Util.padLeft("" + expectedLine, rowNumberWidth)) .append(" ".repeat(rowNumberWidth)) - .append(" - ").append(diffRow.getOldLine()).append(LineSeparator.SYSTEM) + .append(deletedLine).append(diffRow.oldLine()).append(symbols.newLineCharacter()) .append(" ".repeat(rowNumberWidth)) - .append(padLeft("" + actualLine, rowNumberWidth)) - .append(" + ") - .append(diffRow.getNewLine()); + .append(Util.padLeft("" + actualLine, rowNumberWidth)) + .append(addedLine) + .append(diffRow.newLine()); expectedLine++; actualLine++; @@ -71,9 +77,9 @@ public String renderDiff(List rows, int contextLines, int lineNumberOff case DELETE: indexOfLastDifference = i; b - .append(padLeft("" + expectedLine, rowNumberWidth)) + .append(Util.padLeft("" + expectedLine, rowNumberWidth)) .append(" ".repeat(rowNumberWidth)) - .append(" - ").append(diffRow.getOldLine()); + .append(deletedLine).append(diffRow.oldLine()); expectedLine++; break; @@ -81,8 +87,8 @@ public String renderDiff(List rows, int contextLines, int lineNumberOff indexOfLastDifference = i; b .append(" ".repeat(rowNumberWidth)) - .append(padLeft("" + actualLine, rowNumberWidth)) - .append(" + ").append(diffRow.getNewLine()); + .append(Util.padLeft("" + actualLine, rowNumberWidth)) + .append(addedLine).append(diffRow.newLine()); actualLine++; break; @@ -91,10 +97,11 @@ public String renderDiff(List rows, int contextLines, int lineNumberOff } if (i < rows.size() - 1) { - b.append(LineSeparator.SYSTEM); + b.append(symbols.newLineCharacter()); } } return b.toString(); } + } diff --git a/diff-tool/src/main/java/de/skuzzle/difftool/Util.java b/diff-tool/src/main/java/de/skuzzle/difftool/Util.java new file mode 100644 index 00000000..4c76f9c6 --- /dev/null +++ b/diff-tool/src/main/java/de/skuzzle/difftool/Util.java @@ -0,0 +1,28 @@ +package de.skuzzle.difftool; + +import java.util.List; + +final class Util { + private Util() { + // hidden + } + + public static int indexOfNextNonEqual(List fullDiff, int startIndex) { + for (int i = startIndex; i < fullDiff.size(); ++i) { + if (fullDiff.get(i).type() != DiffLine.Type.EQUAL) { + return i; + } + } + return fullDiff.size(); + } + + public static String padLeft(String text, int targetWidth) { + final int missingSpaces = targetWidth - text.length(); + return " ".repeat(missingSpaces) + text; + } + + public static String padRight(String text, int targetWidth) { + final int missingSpaces = targetWidth - text.length(); + return text + " ".repeat(missingSpaces); + } +} diff --git a/diff-tool/src/main/java/de/skuzzle/difftool/thirdparty/DiffUtilsDiffAlgorithm.java b/diff-tool/src/main/java/de/skuzzle/difftool/thirdparty/DiffUtilsDiffAlgorithm.java new file mode 100644 index 00000000..4ddf4e37 --- /dev/null +++ b/diff-tool/src/main/java/de/skuzzle/difftool/thirdparty/DiffUtilsDiffAlgorithm.java @@ -0,0 +1,67 @@ +package de.skuzzle.difftool.thirdparty; + +import java.util.List; +import java.util.Objects; +import java.util.function.BiFunction; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.stream.Collectors; + +import de.skuzzle.difftool.DiffAlgorithm; +import de.skuzzle.difftool.DiffLine; + +import com.github.difflib.text.DiffRow; +import com.github.difflib.text.DiffRowGenerator; + +/** + * Based on java-diff-utils. + */ +public final class DiffUtilsDiffAlgorithm implements DiffAlgorithm { + + /** + * A default instance with sensible defaults for comparing texts. + */ + public static final DiffAlgorithm INSTANCE = create(builder -> builder + .showInlineDiffs(true) + .lineNormalizer(Function.identity()) + .inlineDiffByWord(true) + .ignoreWhiteSpaces(true) + .newTag(inlineMarker()) + .oldTag(inlineMarker())); + + private static BiFunction inlineMarker() { + return (tag, isOpening) -> { + if (tag != DiffRow.Tag.CHANGE) { + return ""; + } + return isOpening ? "<<" : ">>"; + }; + } + + private final DiffRowGenerator generator; + + private DiffUtilsDiffAlgorithm(DiffRowGenerator generator) { + this.generator = generator; + } + + public static DiffUtilsDiffAlgorithm using(DiffRowGenerator diffRowGenerator) { + return new DiffUtilsDiffAlgorithm(Objects.requireNonNull(diffRowGenerator)); + } + + public static DiffUtilsDiffAlgorithm create(Consumer configure) { + final DiffRowGenerator.Builder builder = DiffRowGenerator.create(); + Objects.requireNonNull(configure).accept(builder); + return new DiffUtilsDiffAlgorithm(builder.build()); + } + + @Override + public List diffOf(List left, List right) { + return generator.generateDiffRows(left, right).stream() + .map(row -> new DiffLine(translateTag(row.getTag()), row.getOldLine(), row.getNewLine())) + .collect(Collectors.toList()); + } + + private DiffLine.Type translateTag(DiffRow.Tag tag) { + return DiffLine.Type.valueOf(tag.name()); + } +} diff --git a/readme/RELEASE_NOTES.md b/readme/RELEASE_NOTES.md index 2ef7df0e..01553adf 100644 --- a/readme/RELEASE_NOTES.md +++ b/readme/RELEASE_NOTES.md @@ -20,22 +20,12 @@ ### Changes * [#67](https://github.com/skuzzle/snapshot-tests/issues/67): Fix inconsistent naming of structured data modules - -### Dependencies - -* Update to jsonassert `1.5.1` (coming from `1.5.0`) - - -### Internal - * [#60](https://github.com/skuzzle/snapshot-tests/issues/60): Internal API for plugging in test-framework specific behavior - - -### Build - * [#80](https://github.com/skuzzle/snapshot-tests/issues/80): Use spotless and reformat whole code base +* [#88](https://github.com/skuzzle/snapshot-tests/issues/88): Extract diff creation into own module + +* Update to jsonassert `1.5.1` (coming from `1.5.0`) -### Documentation * List notable changes since last release in reference documentation @@ -201,6 +191,23 @@ Directory Params testImplementation("@project.groupId@:snapshot-tests-directory-params:@project.version@") ``` +Diff-Tool + +[![Maven Central](https://img.shields.io/static/v1?label=MavenCentral&message=@project.version@&color=blue)](https://search.maven.org/artifact/@project.groupId@/diff-tool/@project.version@/jar) [![JavaDoc](https://img.shields.io/static/v1?label=JavaDoc&message=@project.version@&color=orange)](http://www.javadoc.io/doc/@project.groupId@/diff-tool/@project.version@) + +```xml + + @project.groupId@ + diff-tool + @project.version@ + test + +``` + +``` +testImplementation("@project.groupId@:diff-tool:@project.version@") +``` + Object normalization (⚠️ Experimental⚠) [![Maven Central](https://img.shields.io/static/v1?label=MavenCentral&message=@project.version@&color=blue)](https://search.maven.org/artifact/@project.groupId@/snapshot-tests-normalize/@project.version@/jar) [![JavaDoc](https://img.shields.io/static/v1?label=JavaDoc&message=@project.version@&color=orange)](http://www.javadoc.io/doc/@project.groupId@/snapshot-tests-normalize/@project.version@) diff --git a/settings.gradle b/settings.gradle index 80afbef4..c383645e 100644 --- a/settings.gradle +++ b/settings.gradle @@ -39,6 +39,7 @@ include ( // External utilities 'snapshot-tests-directory-params', 'snapshot-tests-normalize', + 'diff-tool', // Docs and build support 'snapshot-tests-documentation', @@ -47,7 +48,6 @@ include ( ) - enableFeaturePreview("TYPESAFE_PROJECT_ACCESSORS") dependencyResolutionManagement { @@ -78,4 +78,5 @@ dependencyResolutionManagement { } } } +include 'diff-tool' diff --git a/snapshot-tests-core/build.gradle b/snapshot-tests-core/build.gradle index 5b04ce45..d50da8c3 100644 --- a/snapshot-tests-core/build.gradle +++ b/snapshot-tests-core/build.gradle @@ -6,6 +6,7 @@ ext.automaticModuleName = "de.skuzzle.test.snapshots.core" dependencies { implementation(projects.snapshotTestsCommon) + implementation(projects.diffTool) testImplementation(projects.snapshotTestsTestCommon) api platform(libs.junit.bom) diff --git a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/DiffListLookahead.java b/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/DiffListLookahead.java deleted file mode 100644 index d2c811ce..00000000 --- a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/DiffListLookahead.java +++ /dev/null @@ -1,22 +0,0 @@ -package de.skuzzle.test.snapshots.data.text; - -import java.util.List; - -import com.github.difflib.text.DiffRow; -import com.github.difflib.text.DiffRow.Tag; - -final class DiffListLookahead { - - public static int indexOfNextNonEqual(List fullDiff, int startIndex) { - for (int i = startIndex; i < fullDiff.size(); ++i) { - if (fullDiff.get(i).getTag() != Tag.EQUAL) { - return i; - } - } - return fullDiff.size(); - } - - private DiffListLookahead() { - // hidden - } -} diff --git a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/DiffRenderer.java b/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/DiffRenderer.java deleted file mode 100644 index dc8ee9f8..00000000 --- a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/DiffRenderer.java +++ /dev/null @@ -1,10 +0,0 @@ -package de.skuzzle.test.snapshots.data.text; - -import java.util.List; - -import com.github.difflib.text.DiffRow; - -interface DiffRenderer { - - String renderDiff(List rows, int contextLines, int lineNumerOffset); -} diff --git a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/TextDiff.java b/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/TextDiff.java index 0a7cdb7a..0c461b95 100644 --- a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/TextDiff.java +++ b/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/TextDiff.java @@ -1,15 +1,18 @@ package de.skuzzle.test.snapshots.data.text; -import static java.util.stream.Collectors.toList; - -import java.util.List; import java.util.function.BiFunction; +import java.util.function.Consumer; import java.util.function.Function; +import de.skuzzle.difftool.DiffRenderer; +import de.skuzzle.difftool.DiffSettings; +import de.skuzzle.difftool.LineSeparator; +import de.skuzzle.difftool.StringDiff; +import de.skuzzle.difftool.UnifiedDiffRenderer; +import de.skuzzle.difftool.thirdparty.DiffUtilsDiffAlgorithm; import de.skuzzle.test.snapshots.SnapshotTestOptions; import de.skuzzle.test.snapshots.validation.Arguments; -import com.github.difflib.text.DiffRow; import com.github.difflib.text.DiffRow.Tag; import com.github.difflib.text.DiffRowGenerator; @@ -26,16 +29,11 @@ public final class TextDiff { private final Settings settings; - private final List diffRows; - private final LineSeparator expectedLineSeparator; - private final LineSeparator actualLineSeparator; + private final StringDiff diff; - private TextDiff(Settings settings, List diffRows, LineSeparator expectedLineSeparator, - LineSeparator actualLineSeparator) { + private TextDiff(Settings settings, StringDiff diff) { this.settings = settings; - this.diffRows = diffRows; - this.expectedLineSeparator = expectedLineSeparator; - this.actualLineSeparator = actualLineSeparator; + this.diff = diff; } public static TextDiff compare(Settings settings, String expected, String actual) { @@ -43,13 +41,10 @@ public static TextDiff compare(Settings settings, 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 List diffRows = settings.buildDiffRowGenerator().generateDiffRows( - expected.lines().collect(toList()), - actual.lines().collect(toList())); - return new TextDiff(settings, diffRows, expectedLineSeparator, actualLineSeparator); + final StringDiff diff = StringDiff.using(DiffUtilsDiffAlgorithm.create(settings.buildDiffRowGenerator()), + expected, + actual); + return new TextDiff(settings, diff); } @API(status = Status.INTERNAL, since = "1.7.0") @@ -58,7 +53,7 @@ public static final class Settings { private int contextLines = SnapshotTestOptions.DEFAULT_CONTEXT_LINES; private String inlineOpeningChangeMarker = "<<"; private String inlineClosingChangeMarker = ">>"; - private DiffRenderer diffRenderer = new UnifiedDiffRenderer(); + private DiffRenderer diffRenderer = UnifiedDiffRenderer.INSTANCE; private Settings() { // hidden @@ -106,24 +101,23 @@ private BiFunction inlineMarker() { }; } - private DiffRowGenerator buildDiffRowGenerator() { - return DiffRowGenerator.create() + Consumer buildDiffRowGenerator() { + return builder -> builder .showInlineDiffs(true) .lineNormalizer(Function.identity()) .inlineDiffByWord(true) .ignoreWhiteSpaces(ignoreWhitespaces) .newTag(inlineMarker()) - .oldTag(inlineMarker()) - .build(); + .oldTag(inlineMarker()); } } private boolean hasLinebreakDifference() { - return expectedLineSeparator != actualLineSeparator && !settings.ignoreWhitespaces; + return diff.hasLineSeparatorDifference() && !settings.ignoreWhitespaces; } private boolean hasTextDifference() { - return diffRows.stream().map(DiffRow::getTag).anyMatch(tag -> tag != Tag.EQUAL); + return diff.hasTextDifference(); } public boolean differencesDetected() { @@ -133,9 +127,12 @@ public boolean differencesDetected() { public String renderDiffWithOffsetAndContextLines(int lineNumberOffset, int contextLines) { final StringBuilder result = new StringBuilder(); final boolean hasTextDifference = hasTextDifference(); - final boolean hasLinebreakDifference = hasLinebreakDifference(); + final boolean hasSignificantLinebreakDifference = hasLinebreakDifference(); + + if (hasSignificantLinebreakDifference) { + final LineSeparator expectedLineSeparator = diff.leftLineSeparator(); + final LineSeparator actualLineSeparator = diff.rightLineSeparator(); - if (hasLinebreakDifference) { result.append("Strings differ in linebreaks. Expected: '") .append(expectedLineSeparator.displayName()) .append("', Actual encountered: '").append(actualLineSeparator.displayName()).append("'"); @@ -147,7 +144,8 @@ public String renderDiffWithOffsetAndContextLines(int lineNumberOffset, int cont } if (hasTextDifference) { - result.append(settings.diffRenderer.renderDiff(diffRows, contextLines, lineNumberOffset)); + result.append(diff.toString(settings.diffRenderer, + DiffSettings.withDefaultSymbols(contextLines, lineNumberOffset))); } return result.toString(); } diff --git a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/TextSnapshot.java b/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/TextSnapshot.java index 69a1e31c..94f94a7d 100644 --- a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/TextSnapshot.java +++ b/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/data/text/TextSnapshot.java @@ -1,5 +1,8 @@ package de.skuzzle.test.snapshots.data.text; +import de.skuzzle.difftool.DiffRenderer; +import de.skuzzle.difftool.SplitDiffRenderer; +import de.skuzzle.difftool.UnifiedDiffRenderer; import de.skuzzle.test.snapshots.SnapshotTestOptions; import de.skuzzle.test.snapshots.StructuralAssertions; import de.skuzzle.test.snapshots.StructuredData; @@ -118,12 +121,12 @@ public enum DiffFormat { * Render diffs in unified format, where changes are displayed line by line and * lines from expected and actual results are printed interleaved. */ - UNIFIED(new UnifiedDiffRenderer()), + UNIFIED(UnifiedDiffRenderer.INSTANCE), /** * Render diff in split view format, where the lines from the expected and actual * result will be printed side by side. */ - SPLIT(new SplitDiffRenderer()); + SPLIT(SplitDiffRenderer.INSTANCE); private final DiffRenderer renderer; diff --git a/snapshot-tests-core/src/test/java/de/skuzzle/test/snapshots/data/text/TextDiffTest.java b/snapshot-tests-core/src/test/java/de/skuzzle/test/snapshots/data/text/TextDiffTest.java index 29e871a0..b2b6dbd7 100644 --- a/snapshot-tests-core/src/test/java/de/skuzzle/test/snapshots/data/text/TextDiffTest.java +++ b/snapshot-tests-core/src/test/java/de/skuzzle/test/snapshots/data/text/TextDiffTest.java @@ -5,7 +5,9 @@ import java.util.Arrays; import java.util.stream.Collectors; -import de.skuzzle.test.snapshots.SnapshotDsl.Snapshot; +import de.skuzzle.difftool.LineSeparator; +import de.skuzzle.difftool.SplitDiffRenderer; +import de.skuzzle.test.snapshots.Snapshot; import de.skuzzle.test.snapshots.SnapshotException; import de.skuzzle.test.snapshots.SnapshotSerializer; import de.skuzzle.test.snapshots.data.text.TextDiff.Settings; @@ -199,7 +201,7 @@ void testRenderUnifiedDiffWithLinenumberOffset(Snapshot snapshot) throws Excepti void testRenderSplitDiffWithRawLinenumbers(Snapshot snapshot) throws Exception { final TextDiff textDiff = TextDiff.compare(Settings.defaultSettings() .withContextLines(3) - .withDiffRenderer(new SplitDiffRenderer()), expected, actual); + .withDiffRenderer(SplitDiffRenderer.INSTANCE), expected, actual); snapshot.assertThat(textDiff).asText().matchesSnapshotText(); } @@ -207,7 +209,7 @@ void testRenderSplitDiffWithRawLinenumbers(Snapshot snapshot) throws Exception { @Test void testRenderSplitDiffWithLinenumberOffset(Snapshot snapshot) throws Exception { final TextDiff textDiff = TextDiff.compare(Settings.defaultSettings() - .withDiffRenderer(new SplitDiffRenderer()), expected, actual); + .withDiffRenderer(SplitDiffRenderer.INSTANCE), expected, actual); snapshot.assertThat(textDiff).as(diffWithOffset(5, 3)).matchesSnapshotText(); } diff --git a/snapshot-tests-documentation/src/docs/asciidoc/index.adoc b/snapshot-tests-documentation/src/docs/asciidoc/index.adoc index 939d8f32..2c201d1f 100644 --- a/snapshot-tests-documentation/src/docs/asciidoc/index.adoc +++ b/snapshot-tests-documentation/src/docs/asciidoc/index.adoc @@ -50,6 +50,9 @@ favor of their respective drop-in replacement `snapshot-tests-json`, `snapshot-t CAUTION: The drop-ins come with a slightly different `Automatic-Module-Name`. If you are using JPMS you need to adjust the respective `requires` clause in your `module-info.java`. +* The logic of creating and rendering diffs has been moved into its own separate module so that it can be reused on its +own: https://search.maven.org/artifact/{groupId}/diff-tool/{revnumber}/jar[`diff-tool`]. + == Getting Started === Artifacts From 7d199a29dc96130acaf07d84152d3c8c2aa99a6a Mon Sep 17 00:00:00 2001 From: Simon Taddiken Date: Thu, 23 Mar 2023 09:52:36 +0100 Subject: [PATCH 2/9] Apply spotless --- diff-tool/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/diff-tool/README.md b/diff-tool/README.md index b5feffe5..f48abede 100644 --- a/diff-tool/README.md +++ b/diff-tool/README.md @@ -77,7 +77,7 @@ Split Diff 12 Some unchanged lines10 | 12 Some unchanged lines10 13 Some unchanged lines11 | 13 Some unchanged lines11 14 Some unchanged lines12 | 14 Some unchanged lines12 - 15 - And here is the finish with way more than 80 characters and I'm very curious how this is going to be displayed in split view diff. | + 15 - And here is the finish with way more than 80 characters and I'm very curious how this is going to be displayed in split view diff. | 16 This line is unchanged | 15 This line is unchanged | 16 + This line has been added 17 Some unchanged lines13 | 17 Some unchanged lines13 @@ -93,4 +93,4 @@ Split Diff 27 Some unchanged lines22 | 27 Some unchanged lines22 28 Some unchanged lines23 | 28 Some unchanged lines23 [...] -``` \ No newline at end of file +``` From 49e8f9355b950393f0e97f7a1084ddb17e85e9be Mon Sep 17 00:00:00 2001 From: Simon Taddiken Date: Thu, 23 Mar 2023 09:55:39 +0100 Subject: [PATCH 3/9] Make continuation string configurable --- .../main/java/de/skuzzle/difftool/DiffSettings.java | 10 ++++++++-- .../java/de/skuzzle/difftool/SplitDiffRenderer.java | 2 +- .../java/de/skuzzle/difftool/UnifiedDiffRenderer.java | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/diff-tool/src/main/java/de/skuzzle/difftool/DiffSettings.java b/diff-tool/src/main/java/de/skuzzle/difftool/DiffSettings.java index fb933b03..231ca69d 100644 --- a/diff-tool/src/main/java/de/skuzzle/difftool/DiffSettings.java +++ b/diff-tool/src/main/java/de/skuzzle/difftool/DiffSettings.java @@ -36,21 +36,23 @@ public DiffSymbols symbols() { public static final class DiffSymbols { - public static DiffSymbols DEFAULT = new DiffSymbols("!", "+", "-", " ", LineSeparator.SYSTEM); + public static DiffSymbols DEFAULT = new DiffSymbols("!", "+", "-", " ", LineSeparator.SYSTEM, "[...]"); private final String changedLine; private final String addedLine; private final String deletedLine; private final String equalLine; private final LineSeparator newLineCharacter; + private final String continuation; public DiffSymbols(String changedLine, String addedLine, String deletedLine, String equalLine, - LineSeparator newLineCharacter) { + LineSeparator newLineCharacter, String continuation) { this.changedLine = changedLine; this.addedLine = addedLine; this.deletedLine = deletedLine; this.equalLine = equalLine; this.newLineCharacter = newLineCharacter; + this.continuation = continuation; } public String changedLine() { @@ -72,5 +74,9 @@ public String equalLine() { public LineSeparator newLineCharacter() { return newLineCharacter; } + + public String continuation() { + return continuation; + } } } diff --git a/diff-tool/src/main/java/de/skuzzle/difftool/SplitDiffRenderer.java b/diff-tool/src/main/java/de/skuzzle/difftool/SplitDiffRenderer.java index e1a0bd8d..a74c0346 100644 --- a/diff-tool/src/main/java/de/skuzzle/difftool/SplitDiffRenderer.java +++ b/diff-tool/src/main/java/de/skuzzle/difftool/SplitDiffRenderer.java @@ -63,7 +63,7 @@ public String renderDiff(List rows, DiffSettings diffSettings) { symbols.equalLine(), diffRow.newLine()); } else if (distanceToNextDifference == contextLines || distanceToPrevDifference == contextLines) { - b.append("[...]"); + b.append(symbols.continuation()); } else { expectedLine++; actualLine++; diff --git a/diff-tool/src/main/java/de/skuzzle/difftool/UnifiedDiffRenderer.java b/diff-tool/src/main/java/de/skuzzle/difftool/UnifiedDiffRenderer.java index d30b1919..7e028063 100644 --- a/diff-tool/src/main/java/de/skuzzle/difftool/UnifiedDiffRenderer.java +++ b/diff-tool/src/main/java/de/skuzzle/difftool/UnifiedDiffRenderer.java @@ -48,7 +48,7 @@ public String renderDiff(List rows, DiffSettings diffSettings) { .append(equalLine).append(diffRow.oldLine()); } else if (distanceToNextDifference == contextLines || distanceToPrevDifference == contextLines) { - b.append("[...]"); + b.append(symbols.continuation()); } else { expectedLine++; actualLine++; From 373c875208f19969946f795f248e1286cfca7ca4 Mon Sep 17 00:00:00 2001 From: Simon Taddiken Date: Fri, 24 Mar 2023 09:02:26 +0100 Subject: [PATCH 4/9] Add package-info --- diff-tool/src/main/java/de/skuzzle/difftool/package-info.java | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 diff-tool/src/main/java/de/skuzzle/difftool/package-info.java diff --git a/diff-tool/src/main/java/de/skuzzle/difftool/package-info.java b/diff-tool/src/main/java/de/skuzzle/difftool/package-info.java new file mode 100644 index 00000000..99dac714 --- /dev/null +++ b/diff-tool/src/main/java/de/skuzzle/difftool/package-info.java @@ -0,0 +1,4 @@ +/** + *

Utility for creating and rendering diffs.

+ */ +package de.skuzzle.difftool; From 1d85bfe3022180f8988407eaf416002d63d36ef4 Mon Sep 17 00:00:00 2001 From: Simon Taddiken Date: Mon, 27 Mar 2023 13:22:16 +0200 Subject: [PATCH 5/9] Add possibility to normalize line endings. Closes #44 and Closes #90 --- .../de/skuzzle/difftool/GitLineSeparator.java | 63 +++++++++++++++++ .../de/skuzzle/difftool/LineSeparator.java | 70 ++++++++++++++++++- .../difftool/GitConfigLineSeparatorTest.java | 46 ++++++++++++ .../skuzzle/difftool/LineSeparatorTest.java | 44 ++++++++++++ readme/RELEASE_NOTES.md | 1 + .../test/snapshots/SnapshotTestOptions.java | 49 ++++++++++++- .../impl/DefaultSnapshotConfiguration.java | 8 +++ .../impl/LegacySnapshotConfiguration.java | 6 ++ ...ormalizeLineEndingsSnapshotSerializer.java | 54 ++++++++++++++ .../snapshots/impl/SnapshotConfiguration.java | 13 ++++ .../snapshots/impl/SnapshotDslResult.java | 8 ++- .../snapshots/SnapshotTestOptionsTest.java | 15 ++++ .../skuzzle/test/snapshots/SnapshotsTest.java | 29 ++++---- .../testNormalizeToWindows_0.snapshot | 8 +++ .../src/docs/asciidoc/index.adoc | 28 ++++++++ .../snippets/SnapshotTestOptionsTest.java | 8 +++ 16 files changed, 432 insertions(+), 18 deletions(-) create mode 100644 diff-tool/src/main/java/de/skuzzle/difftool/GitLineSeparator.java create mode 100644 diff-tool/src/test/java/de/skuzzle/difftool/GitConfigLineSeparatorTest.java create mode 100644 diff-tool/src/test/java/de/skuzzle/difftool/LineSeparatorTest.java create mode 100644 snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/NormalizeLineEndingsSnapshotSerializer.java create mode 100644 snapshot-tests-core/src/test/java/de/skuzzle/test/snapshots/SnapshotTestOptionsTest.java create mode 100644 snapshot-tests-core/src/test/resources/de/skuzzle/test/snapshots/SnapshotsTest_snapshots/testNormalizeToWindows_0.snapshot diff --git a/diff-tool/src/main/java/de/skuzzle/difftool/GitLineSeparator.java b/diff-tool/src/main/java/de/skuzzle/difftool/GitLineSeparator.java new file mode 100644 index 00000000..b9201ea5 --- /dev/null +++ b/diff-tool/src/main/java/de/skuzzle/difftool/GitLineSeparator.java @@ -0,0 +1,63 @@ +package de.skuzzle.difftool; + +import java.util.Locale; + +final class GitLineSeparator { + + static final LineSeparator GIT_LINE_SEPARATOR = determineGitLineSeparator(GitConfig.DEFAULT); + + static LineSeparator determineGitLineSeparator(GitConfig gitConfig) { + final String autocrlf = gitConfig.autocrlf(); + if (autocrlf == null) { + return LineSeparator.SYSTEM; + } + switch (autocrlf.toLowerCase(Locale.ROOT)) { + case "true": + return LineSeparator.CRLF; + case "input": + return LineSeparator.LF; + case "false": + final String eol = gitConfig.eol(); + if (eol == null) { + return LineSeparator.SYSTEM; + } + switch (eol.toLowerCase(Locale.ROOT)) { + case "crlf": + return LineSeparator.CRLF; + case "lf": + return LineSeparator.LF; + case "native": + default: + return LineSeparator.SYSTEM; + } + } + return LineSeparator.SYSTEM; + } + + static class GitConfig { + + static final GitConfig DEFAULT = new GitConfig(); + + String autocrlf() { + return execute("git config core.autocrlf"); + } + + String eol() { + return execute("git config core.eol"); + } + + static String execute(String command) { + try { + final Process exec = Runtime.getRuntime().exec(command); + exec.getErrorStream().readAllBytes(); + return new String(exec.getInputStream().readAllBytes()); + } catch (Exception e) { + return null; + } + } + } + + private GitLineSeparator() { + // hidden + } +} diff --git a/diff-tool/src/main/java/de/skuzzle/difftool/LineSeparator.java b/diff-tool/src/main/java/de/skuzzle/difftool/LineSeparator.java index 32cfd975..5e51aae5 100644 --- a/diff-tool/src/main/java/de/skuzzle/difftool/LineSeparator.java +++ b/diff-tool/src/main/java/de/skuzzle/difftool/LineSeparator.java @@ -26,6 +26,58 @@ public enum LineSeparator { this.displayName = displayName; } + /** + * Tries to determine the line separator from GIT according to its + * core.autocrlf and core.eol config values. + *

+ * WARNING: In order to determine the config values, this method will invoke a system + * process to call git config .... This will only be done once and the + * results will be cached in memory for the lifetime of the VM. + *

+ *

+ * If this method fails to determine the line separator from GIT config it will return + * {@link LineSeparator#SYSTEM} as fall back. Otherwise the line separator will be + * determined according to the following table: + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + *
core.autocrlfcore.eolResult
true-{@link LineSeparator#CRLF}
input-{@link LineSeparator#LF}
falsecrlf{@link LineSeparator#CRLF}
falself{@link LineSeparator#LF}
falsenative{@link LineSeparator#SYSTEM}
Logic of how to determine the LineSeparator from GIT
+ * + * @return The {@link LineSeparator} + */ + public static LineSeparator determineFromGitConfig() { + return GitLineSeparator.GIT_LINE_SEPARATOR; + } + /** * Determines the line separator used in the given String. Will return the type of the * first line separator found in the String. @@ -38,10 +90,19 @@ public enum LineSeparator { * @return The type of line endings. */ public static LineSeparator determineFrom(String s) { - for (final LineSeparator lineSeparator : LineSeparator.values()) { - if (lineSeparator.existsIn(s)) { - return lineSeparator; + final int r = s.indexOf('\r'); + if (r >= 0) { + if (s.length() > r + 1) { + final int n = s.charAt(r + 1); + if (n == '\n') { + return CRLF; + } } + return CR; + } + final int n = s.indexOf('\n'); + if (n >= 0) { + return LF; } return SYSTEM; } @@ -62,6 +123,9 @@ public String displayName() { * @return The string with converted line separators. */ public String convert(String s) { + if (existsIn(s)) { + return s; + } return s.lines().collect(Collectors.joining(characters)); } diff --git a/diff-tool/src/test/java/de/skuzzle/difftool/GitConfigLineSeparatorTest.java b/diff-tool/src/test/java/de/skuzzle/difftool/GitConfigLineSeparatorTest.java new file mode 100644 index 00000000..562c632b --- /dev/null +++ b/diff-tool/src/test/java/de/skuzzle/difftool/GitConfigLineSeparatorTest.java @@ -0,0 +1,46 @@ +package de.skuzzle.difftool; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +class GitConfigLineSeparatorTest { + + static class TestGitConfig extends GitLineSeparator.GitConfig { + private final String autocrlf; + private final String eol; + + TestGitConfig(String autocrlf, String eol) { + this.autocrlf = autocrlf; + this.eol = eol; + } + + @Override + public String autocrlf() { + return autocrlf; + } + + @Override + public String eol() { + return eol; + } + } + + @ParameterizedTest + @CsvSource(value = { + "null,null,SYSTEM", + "true, null, CRLF", + "input,null,LF", + "false,crlf,CRLF", + "false,lf,LF", + "false,native,SYSTEM", + "false,null,SYSTEM" + }) + void determineLineSeparatorFromGitConfig(String autocrlf, String eol, LineSeparator expected) { + final TestGitConfig git = new TestGitConfig(autocrlf, eol); + LineSeparator gitLineSeparator = GitLineSeparator.determineGitLineSeparator(git); + assertThat(gitLineSeparator).isEqualTo(expected); + } + +} diff --git a/diff-tool/src/test/java/de/skuzzle/difftool/LineSeparatorTest.java b/diff-tool/src/test/java/de/skuzzle/difftool/LineSeparatorTest.java new file mode 100644 index 00000000..331d5fc8 --- /dev/null +++ b/diff-tool/src/test/java/de/skuzzle/difftool/LineSeparatorTest.java @@ -0,0 +1,44 @@ +package de.skuzzle.difftool; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; + +class LineSeparatorTest { + + @Test + void testFindLfLineSeparator() { + final String s = "test\ntest"; + assertThat(LineSeparator.determineFrom(s)).isEqualTo(LineSeparator.LF); + } + + @Test + void testFindLfLineSeparatorAtEndOfString() { + final String s = "test\n"; + assertThat(LineSeparator.determineFrom(s)).isEqualTo(LineSeparator.LF); + } + + @Test + void testFinxCrLfLineSeparator() { + final String s = "test\r\ntest"; + assertThat(LineSeparator.determineFrom(s)).isEqualTo(LineSeparator.CRLF); + } + + @Test + void testFinxCrLfLineSeparatorAtEndOfString() { + final String s = "test\r\n"; + assertThat(LineSeparator.determineFrom(s)).isEqualTo(LineSeparator.CRLF); + } + + @Test + void testFindCrLineSeparator() { + final String s = "test\rtest"; + assertThat(LineSeparator.determineFrom(s)).isEqualTo(LineSeparator.CR); + } + + @Test + void testFindCrLineSeparatorAtEndOfString() { + final String s = "test\r"; + assertThat(LineSeparator.determineFrom(s)).isEqualTo(LineSeparator.CR); + } +} diff --git a/readme/RELEASE_NOTES.md b/readme/RELEASE_NOTES.md index 01553adf..2109724b 100644 --- a/readme/RELEASE_NOTES.md +++ b/readme/RELEASE_NOTES.md @@ -23,6 +23,7 @@ * [#60](https://github.com/skuzzle/snapshot-tests/issues/60): Internal API for plugging in test-framework specific behavior * [#80](https://github.com/skuzzle/snapshot-tests/issues/80): Use spotless and reformat whole code base * [#88](https://github.com/skuzzle/snapshot-tests/issues/88): Extract diff creation into own module +* [#44](https://github.com/skuzzle/snapshot-tests/issues/44)/[#90](https://github.com/skuzzle/snapshot-tests/issues/90): Add possibility to normalize line endings (according to local git config) * Update to jsonassert `1.5.1` (coming from `1.5.0`) diff --git a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/SnapshotTestOptions.java b/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/SnapshotTestOptions.java index f11051fe..072ee390 100644 --- a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/SnapshotTestOptions.java +++ b/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/SnapshotTestOptions.java @@ -30,7 +30,12 @@ /** * Default number of context lines that are displayed in diffs. */ - public static final int DEFAULT_CONTEXT_LINES = 5; + int DEFAULT_CONTEXT_LINES = 5; + + /** + * Default setting for {@link #normalizeLineEndings()}. + */ + NormalizeLineEndings DEFAULT_NORMALIZE_LINE_ENDINGS = NormalizeLineEndings.NEVER; /** * Defines the number of context lines that are printed around a comparison failure. @@ -98,13 +103,27 @@ */ boolean alwaysPersistRawResult() default false; + /** + * Advises the framework to convert the line endings after snapshot serialization to + * the given value. + *

+ * By default, line endings are not normalized but when using GIT it is advisable to + * set this to {@link NormalizeLineEndings#GIT} + *

+ * + * @return How to normalize line endings. + * @since 1.10.0 + */ + @API(since = "1.10.0", status = Status.EXPERIMENTAL) + NormalizeLineEndings normalizeLineEndings() default NormalizeLineEndings.NEVER; + /** * Defines whether an offset is added to the line numbers when rendering diffs in * assertion failure messages. * * @author Simon - * @since 1.7.1 * @see SnapshotTestOptions#renderLineNumbers() + * @since 1.7.1 */ @API(status = Status.EXPERIMENTAL, since = "1.7.1") enum DiffLineNumbers { @@ -128,4 +147,30 @@ enum DiffLineNumbers { ACCODRDING_TO_PERSISTED_SNAPSHOT_FILE; } + /** + * Defines how and if line endings will be normalized after snapshot serialization. + * Normalization happens before invoking the {@link StructuralAssertions}. + */ + @API(since = "1.1.0", status = Status.EXPERIMENTAL) + enum NormalizeLineEndings { + /** + * Line endings will not be normalized. Use this settings if line endings are + * significant for the test outcome. + */ + NEVER, + /** All line endings will be converted to LF (\n). */ + LF, + /** All line endings will be converted to CRLF (\r\n). */ + CRLF, + /** All line endings will be converted to the system's default line separator. */ + SYSTEM, + /** + * Line endings will be converted according to the local git's + * core.autocrlf and core.eol settings. Will fall back + * to the system's default line separator if git config values can not be + * determined. + */ + GIT + } + } diff --git a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/DefaultSnapshotConfiguration.java b/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/DefaultSnapshotConfiguration.java index f28f6186..440f56cd 100644 --- a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/DefaultSnapshotConfiguration.java +++ b/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/DefaultSnapshotConfiguration.java @@ -115,6 +115,14 @@ public boolean addOffsetToReportedLinenumbers(Method testMethod) { : snapshotTestOptions.renderLineNumbers() == DiffLineNumbers.ACCODRDING_TO_PERSISTED_SNAPSHOT_FILE; } + @Override + public SnapshotTestOptions.NormalizeLineEndings normalizeLineEndings(Method testMethod) { + final var snapshotTestOptions = determineOptions(testMethod); + return snapshotTestOptions == null + ? SnapshotTestOptions.DEFAULT_NORMALIZE_LINE_ENDINGS + : snapshotTestOptions.normalizeLineEndings(); + } + @Override public boolean isSoftAssertions() { return false; diff --git a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/LegacySnapshotConfiguration.java b/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/LegacySnapshotConfiguration.java index 641a03b5..229b0ec1 100644 --- a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/LegacySnapshotConfiguration.java +++ b/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/LegacySnapshotConfiguration.java @@ -4,6 +4,7 @@ import java.nio.file.Path; import de.skuzzle.test.snapshots.EnableSnapshotTests; +import de.skuzzle.test.snapshots.SnapshotTestOptions; import org.apiguardian.api.API; import org.apiguardian.api.API.Status; @@ -69,6 +70,11 @@ public boolean alwaysPersistActualResult(Method testMethod) { return delegate.alwaysPersistActualResult(testMethod); } + @Override + public SnapshotTestOptions.NormalizeLineEndings normalizeLineEndings(Method testMethod) { + return delegate.normalizeLineEndings(testMethod); + } + @Override public boolean alwaysPersistRawResult(Method testMethod) { return delegate.alwaysPersistRawResult(testMethod); diff --git a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/NormalizeLineEndingsSnapshotSerializer.java b/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/NormalizeLineEndingsSnapshotSerializer.java new file mode 100644 index 00000000..f317aaff --- /dev/null +++ b/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/NormalizeLineEndingsSnapshotSerializer.java @@ -0,0 +1,54 @@ +package de.skuzzle.test.snapshots.impl; + +import de.skuzzle.difftool.LineSeparator; +import de.skuzzle.test.snapshots.SnapshotException; +import de.skuzzle.test.snapshots.SnapshotSerializer; +import de.skuzzle.test.snapshots.SnapshotTestOptions.NormalizeLineEndings; +import de.skuzzle.test.snapshots.validation.Arguments; + +/** + * Serializer that wraps another serializer and normalizes the line endings in the output + * of the delegate serializer. + * + * @since 1.10.0 + */ +final class NormalizeLineEndingsSnapshotSerializer implements SnapshotSerializer { + + private final SnapshotSerializer delegate; + private final NormalizeLineEndings normalizeLineEndings; + + private NormalizeLineEndingsSnapshotSerializer(SnapshotSerializer delegate, + NormalizeLineEndings normalizeLineEndings) { + this.delegate = Arguments.requireNonNull(delegate, "delegate serializer must not be null"); + this.normalizeLineEndings = Arguments.requireNonNull(normalizeLineEndings, + "normalizeLineEndings must not be null"); + } + + static SnapshotSerializer wrap(SnapshotSerializer serializer, NormalizeLineEndings normalizeLineEndings) { + return new NormalizeLineEndingsSnapshotSerializer(serializer, normalizeLineEndings); + } + + @Override + public String serialize(Object testResult) throws SnapshotException { + final String result = delegate.serialize(testResult); + if (normalizeLineEndings == NormalizeLineEndings.NEVER) { + return result; + } + final LineSeparator lineSeparator = determineLineSeparator(); + return lineSeparator.convert(result); + } + + private LineSeparator determineLineSeparator() { + switch (normalizeLineEndings) { + case SYSTEM: + return LineSeparator.SYSTEM; + case CRLF: + return LineSeparator.CRLF; + case LF: + return LineSeparator.LF; + case GIT: + return LineSeparator.determineFromGitConfig(); + } + throw new IllegalStateException("Could not determine LineSeparator for: " + normalizeLineEndings); + } +} diff --git a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/SnapshotConfiguration.java b/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/SnapshotConfiguration.java index 5fd57efc..8695385c 100644 --- a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/SnapshotConfiguration.java +++ b/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/SnapshotConfiguration.java @@ -3,6 +3,9 @@ import java.lang.reflect.Method; import java.nio.file.Path; +import de.skuzzle.test.snapshots.SnapshotTestOptions; +import de.skuzzle.test.snapshots.SnapshotTestOptions.NormalizeLineEndings; + import org.apiguardian.api.API; import org.apiguardian.api.API.Status; @@ -110,6 +113,16 @@ static SnapshotConfiguration legacyConfigurationFor(Class testClass) { */ boolean isForceUpdateSnapshots(Method testMethod); + /** + * How to normalize line endings after default snapshot serialization. + * + * @param testMethod The test method. + * @return Whether to normalize line endings. + * @since 1.10.0 + */ + @API(since = "1.10.0", status = Status.INTERNAL) + NormalizeLineEndings normalizeLineEndings(Method testMethod); + /** * Whether soft assertions shall be used. When set to true, a failing snapshot * assertion will not make the test immediately fail. Instead, all snapshot test diff --git a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/SnapshotDslResult.java b/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/SnapshotDslResult.java index d27d631b..97e697e6 100644 --- a/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/SnapshotDslResult.java +++ b/snapshot-tests-core/src/main/java/de/skuzzle/test/snapshots/impl/SnapshotDslResult.java @@ -11,6 +11,7 @@ import de.skuzzle.test.snapshots.SnapshotFile.SnapshotHeader; import de.skuzzle.test.snapshots.SnapshotNaming; import de.skuzzle.test.snapshots.SnapshotSerializer; +import de.skuzzle.test.snapshots.SnapshotTestOptions.NormalizeLineEndings; import de.skuzzle.test.snapshots.impl.SnapshotAssertionInput.TerminalOperation; import de.skuzzle.test.snapshots.validation.Arguments; @@ -114,7 +115,12 @@ SnapshotAssertionInput createAssertionInput() throws Exception { if (actualWasNull) { serializedActual = UNAVAILABLE_BECAUSE_ACTUAL_WAS_NULL; } else { - serializedActual = snapshotSerializer.serialize(actual); + final NormalizeLineEndings normalizeLineEndings = configuration.normalizeLineEndings( + testMethod); + final SnapshotSerializer normalizingSerializer = NormalizeLineEndingsSnapshotSerializer.wrap( + snapshotSerializer, + normalizeLineEndings); + serializedActual = normalizingSerializer.serialize(actual); } final SnapshotFile actualSnapshotFile = SnapshotFile.of(snapshotHeader, serializedActual); diff --git a/snapshot-tests-core/src/test/java/de/skuzzle/test/snapshots/SnapshotTestOptionsTest.java b/snapshot-tests-core/src/test/java/de/skuzzle/test/snapshots/SnapshotTestOptionsTest.java new file mode 100644 index 00000000..b1af7afb --- /dev/null +++ b/snapshot-tests-core/src/test/java/de/skuzzle/test/snapshots/SnapshotTestOptionsTest.java @@ -0,0 +1,15 @@ +package de.skuzzle.test.snapshots; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; + +@SnapshotTestOptions +public class SnapshotTestOptionsTest { + + @Test + void testCorrectDefaultValueForNormalizeLineEndings() { + final SnapshotTestOptions annotation = SnapshotTestOptionsTest.class.getAnnotation(SnapshotTestOptions.class); + assertThat(annotation.normalizeLineEndings()).isEqualTo(SnapshotTestOptions.DEFAULT_NORMALIZE_LINE_ENDINGS); + } +} diff --git a/snapshot-tests-core/src/test/java/de/skuzzle/test/snapshots/SnapshotsTest.java b/snapshot-tests-core/src/test/java/de/skuzzle/test/snapshots/SnapshotsTest.java index 7a215e27..de93fbe2 100644 --- a/snapshot-tests-core/src/test/java/de/skuzzle/test/snapshots/SnapshotsTest.java +++ b/snapshot-tests-core/src/test/java/de/skuzzle/test/snapshots/SnapshotsTest.java @@ -16,9 +16,16 @@ @EnableSnapshotTests public class SnapshotsTest { + @Test + @SnapshotTestOptions(normalizeLineEndings = SnapshotTestOptions.NormalizeLineEndings.CRLF) + void testNormalizeToWindows(Snapshot snapshot) { + final SnapshotTestResult testResult = snapshot.assertThat("line1\nline2").asText().matchesSnapshotText(); + assertThat(testResult.serializedActual()).isEqualTo("line1\r\nline2"); + } + @Test @SnapshotTestOptions(alwaysPersistActualResult = true, alwaysPersistRawResult = true) - void testWriteContextFiles(Snapshot snapshot) throws Exception { + void testWriteContextFiles(Snapshot snapshot) { final Person simon = determinePerson(); final SnapshotTestResult testResult = snapshot.assertThat(simon).asText().matchesSnapshotText(); @@ -29,15 +36,14 @@ void testWriteContextFiles(Snapshot snapshot) throws Exception { } @Test - void testDisabledWithNullInput(Snapshot snapshot) throws Exception { + void testDisabledWithNullInput(Snapshot snapshot) { final SnapshotTestResult testResult = snapshot.assertThat(null).asText().disabled(); assertThat(testResult.serializedActual()).isEqualTo("<>"); } @Test @SnapshotTestOptions(alwaysPersistActualResult = true, alwaysPersistRawResult = true) - void testWithOneDisabledAssertionForWhichSnapshotHasNotYetBeenCreatedWithContextFiles(Snapshot snapshot) - throws Exception { + void testWithOneDisabledAssertionForWhichSnapshotHasNotYetBeenCreatedWithContextFiles(Snapshot snapshot) { final Person simon = determinePerson(); final SnapshotTestResult testResult = snapshot.assertThat(simon).asText().disabled(); @@ -50,8 +56,7 @@ void testWithOneDisabledAssertionForWhichSnapshotHasNotYetBeenCreatedWithContext @Test @SnapshotTestOptions(alwaysPersistActualResult = true, alwaysPersistRawResult = true) - void testWithOneDisabledAssertionForWhichSnapshotHasHasAlreadyBeenCreatedWithContextFiles(Snapshot snapshot) - throws Exception { + void testWithOneDisabledAssertionForWhichSnapshotHasHasAlreadyBeenCreatedWithContextFiles(Snapshot snapshot) { final Person simon = determinePerson(); final SnapshotTestResult testResult = snapshot.assertThat(simon).asText().disabled(); @@ -63,7 +68,7 @@ void testWithOneDisabledAssertionForWhichSnapshotHasHasAlreadyBeenCreatedWithCon } @Test - void testWithOneDisabledAssertionForWhichSnapshotHasNotYetBeenCreated(Snapshot snapshot) throws Exception { + void testWithOneDisabledAssertionForWhichSnapshotHasNotYetBeenCreated(Snapshot snapshot) { final Person simon = determinePerson(); final SnapshotTestResult testResultDisabled = snapshot.assertThat(simon).asText().disabledBecause("Reasons"); @@ -78,7 +83,7 @@ void testWithOneDisabledAssertionForWhichSnapshotHasNotYetBeenCreated(Snapshot s } @Test - void testWithOneDisabledAssertionForWhichSnapshotHasAlreadyBeenCreated(Snapshot snapshot) throws Exception { + void testWithOneDisabledAssertionForWhichSnapshotHasAlreadyBeenCreated(Snapshot snapshot) { final Person simon = determinePerson(); final SnapshotTestResult testResultDisabled = snapshot.assertThat(simon).asText().disabled(); @@ -91,7 +96,7 @@ void testWithOneDisabledAssertionForWhichSnapshotHasAlreadyBeenCreated(Snapshot } @Test - void testMultipleSnapshotsInOneTestCase(Snapshot snapshot) throws Throwable { + void testMultipleSnapshotsInOneTestCase(Snapshot snapshot) { final Person simon = determinePerson(); snapshot.assertThat(simon).asText().matchesSnapshotText(); final Person phil = determinePerson().setName("Phil"); @@ -99,7 +104,7 @@ void testMultipleSnapshotsInOneTestCase(Snapshot snapshot) throws Throwable { } @Test - void testWithExplicitSnapshotName(Snapshot snapshot) throws Exception { + void testWithExplicitSnapshotName(Snapshot snapshot) { final Person simon = determinePerson(); snapshot.named("simon").assertThat(simon).asText().matchesSnapshotText(); final Person phil = determinePerson().setName("Phil"); @@ -107,7 +112,7 @@ void testWithExplicitSnapshotName(Snapshot snapshot) throws Exception { } @Test - void testMixExplicitAndAutomaticNaming(Snapshot snapshot) throws Exception { + void testMixExplicitAndAutomaticNaming(Snapshot snapshot) { final Person simon = determinePerson(); snapshot.named("simon").assertThat(simon).asText().matchesSnapshotText(); final Person phil = determinePerson().setName("Phil"); @@ -115,7 +120,7 @@ void testMixExplicitAndAutomaticNaming(Snapshot snapshot) throws Exception { } @Test - void testCustomizeTextSnapshot(Snapshot snapshot) throws Exception { + void testCustomizeTextSnapshot(Snapshot snapshot) { final Person simon = determinePerson(); snapshot.assertThat(simon).as(TextSnapshot.text() diff --git a/snapshot-tests-core/src/test/resources/de/skuzzle/test/snapshots/SnapshotsTest_snapshots/testNormalizeToWindows_0.snapshot b/snapshot-tests-core/src/test/resources/de/skuzzle/test/snapshots/SnapshotsTest_snapshots/testNormalizeToWindows_0.snapshot new file mode 100644 index 00000000..5069635b --- /dev/null +++ b/snapshot-tests-core/src/test/resources/de/skuzzle/test/snapshots/SnapshotsTest_snapshots/testNormalizeToWindows_0.snapshot @@ -0,0 +1,8 @@ +dynamic-directory: false +snapshot-name: testNormalizeToWindows_0 +snapshot-number: 0 +test-class: de.skuzzle.test.snapshots.SnapshotsTest +test-method: testNormalizeToWindows + +line1 +line2 \ No newline at end of file diff --git a/snapshot-tests-documentation/src/docs/asciidoc/index.adoc b/snapshot-tests-documentation/src/docs/asciidoc/index.adoc index 2c201d1f..5e9ea7ef 100644 --- a/snapshot-tests-documentation/src/docs/asciidoc/index.adoc +++ b/snapshot-tests-documentation/src/docs/asciidoc/index.adoc @@ -53,6 +53,9 @@ the respective `requires` clause in your `module-info.java`. * The logic of creating and rendering diffs has been moved into its own separate module so that it can be reused on its own: https://search.maven.org/artifact/{groupId}/diff-tool/{revnumber}/jar[`diff-tool`]. +* The framework can be advised to normalize line endings used in snapshot files. This is especially useful when using +git and you do not care about which actual line ending is used in your snapshots. See <<_dealing_with_line_breaks, Dealing With Line Breaks>>. + == Getting Started === Artifacts @@ -421,6 +424,31 @@ Full unified diff of actual result and stored snapshot: [...] ---- +==== Dealing With Line Breaks +Most users probably don't care about line endings during snapshot comparison. However, different line ending formats +are likely to get in your way anyways when employing snapshot testing. That is because _git_ has its very own idea of +how to handle and convert different line endings. You might run into the following problems: + +* You checked out a project on windows with `core.autocrlf=true` but the `SnapshotSerializer` in place produces Unix +line endings. Snapshot tests might fail if the `StructuralAssertions` instance in place puts significance to line breaks. +* You force-updated some snapshots and all of them are marked as modified in git, even if there are no visible changes. + +In case you don't care about line endings you should advise the framework to normalize line endings before writing +snapshot files. You can do that with the following annotation on either the test method or test class: + +.Normalize line endings according to local git config +==== +[source,java] +---- +include::{testIncludes}/de/skuzzle/test/snapshots/snippets/SnapshotTestOptionsTest.java[tags=normalizeLineEndingsGit,indent=0] +---- +==== + +In case you _do_ care about line endings and you are using git, you can use a `.gitattributes` file to signify the +intended line ending to gitConfig during checkout. + +IMPORTANT: Line endings are only normalized when producing the _actual_ snapshot. Line endings are _not_ normalized when +reading the _expected_ snapshot result from disc. === Updating Snapshots One aspect that makes _snapshot testing_ so powerful is that you can generate the _expected output_ from your diff --git a/snapshot-tests-documentation/src/test/java/de/skuzzle/test/snapshots/snippets/SnapshotTestOptionsTest.java b/snapshot-tests-documentation/src/test/java/de/skuzzle/test/snapshots/snippets/SnapshotTestOptionsTest.java index d52ae24e..bb83becb 100644 --- a/snapshot-tests-documentation/src/test/java/de/skuzzle/test/snapshots/snippets/SnapshotTestOptionsTest.java +++ b/snapshot-tests-documentation/src/test/java/de/skuzzle/test/snapshots/snippets/SnapshotTestOptionsTest.java @@ -9,6 +9,14 @@ @EnableSnapshotTests class SnapshotTestOptionsTest { + @Test + // tag::normalizeLineEndingsGit[] + @SnapshotTestOptions(normalizeLineEndings = SnapshotTestOptions.NormalizeLineEndings.GIT) + // end::normalizeLineEndingsGit[] + void normalizeLineEndingsGit(Snapshot snapshot) throws Exception { + + } + // tag::alwaysPersistActualResult[] @Test @SnapshotTestOptions(alwaysPersistActualResult = true) From 759ac97c851e25031e2d0640f7b6eec776109db4 Mon Sep 17 00:00:00 2001 From: Simon Taddiken Date: Mon, 27 Mar 2023 13:33:16 +0200 Subject: [PATCH 6/9] Properly close streams --- .../main/java/de/skuzzle/difftool/GitLineSeparator.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/diff-tool/src/main/java/de/skuzzle/difftool/GitLineSeparator.java b/diff-tool/src/main/java/de/skuzzle/difftool/GitLineSeparator.java index b9201ea5..dfc97b54 100644 --- a/diff-tool/src/main/java/de/skuzzle/difftool/GitLineSeparator.java +++ b/diff-tool/src/main/java/de/skuzzle/difftool/GitLineSeparator.java @@ -49,8 +49,12 @@ String eol() { static String execute(String command) { try { final Process exec = Runtime.getRuntime().exec(command); - exec.getErrorStream().readAllBytes(); - return new String(exec.getInputStream().readAllBytes()); + try (var err = exec.getErrorStream()) { + err.readAllBytes(); + } + try (var in = exec.getInputStream()) { + return new String(in.readAllBytes()); + } } catch (Exception e) { return null; } From 120679bd38e1b627e54e111bc2e4f0b8a57193e5 Mon Sep 17 00:00:00 2001 From: Simon Taddiken Date: Mon, 27 Mar 2023 13:37:01 +0200 Subject: [PATCH 7/9] Replace lambda with method reference --- .../thirdparty/DiffUtilsDiffAlgorithm.java | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/diff-tool/src/main/java/de/skuzzle/difftool/thirdparty/DiffUtilsDiffAlgorithm.java b/diff-tool/src/main/java/de/skuzzle/difftool/thirdparty/DiffUtilsDiffAlgorithm.java index 4ddf4e37..461fb24f 100644 --- a/diff-tool/src/main/java/de/skuzzle/difftool/thirdparty/DiffUtilsDiffAlgorithm.java +++ b/diff-tool/src/main/java/de/skuzzle/difftool/thirdparty/DiffUtilsDiffAlgorithm.java @@ -1,18 +1,16 @@ package de.skuzzle.difftool.thirdparty; +import com.github.difflib.text.DiffRow; +import com.github.difflib.text.DiffRowGenerator; +import de.skuzzle.difftool.DiffAlgorithm; +import de.skuzzle.difftool.DiffLine; + import java.util.List; import java.util.Objects; -import java.util.function.BiFunction; import java.util.function.Consumer; import java.util.function.Function; import java.util.stream.Collectors; -import de.skuzzle.difftool.DiffAlgorithm; -import de.skuzzle.difftool.DiffLine; - -import com.github.difflib.text.DiffRow; -import com.github.difflib.text.DiffRowGenerator; - /** * Based on java-diff-utils. */ @@ -26,16 +24,14 @@ public final class DiffUtilsDiffAlgorithm implements DiffAlgorithm { .lineNormalizer(Function.identity()) .inlineDiffByWord(true) .ignoreWhiteSpaces(true) - .newTag(inlineMarker()) - .oldTag(inlineMarker())); - - private static BiFunction inlineMarker() { - return (tag, isOpening) -> { - if (tag != DiffRow.Tag.CHANGE) { - return ""; - } - return isOpening ? "<<" : ">>"; - }; + .newTag(DiffUtilsDiffAlgorithm::inlineMarker) + .oldTag(DiffUtilsDiffAlgorithm::inlineMarker)); + + private static String inlineMarker(DiffRow.Tag tag, boolean isOpening) { + if (tag != DiffRow.Tag.CHANGE) { + return ""; + } + return isOpening ? "<<" : ">>"; } private final DiffRowGenerator generator; From d75db083194451e52048009c1d58da1665d19ee3 Mon Sep 17 00:00:00 2001 From: Simon Taddiken Date: Mon, 27 Mar 2023 13:43:17 +0200 Subject: [PATCH 8/9] Wait for process to exit --- .../src/main/java/de/skuzzle/difftool/GitLineSeparator.java | 1 + 1 file changed, 1 insertion(+) diff --git a/diff-tool/src/main/java/de/skuzzle/difftool/GitLineSeparator.java b/diff-tool/src/main/java/de/skuzzle/difftool/GitLineSeparator.java index dfc97b54..13d5a811 100644 --- a/diff-tool/src/main/java/de/skuzzle/difftool/GitLineSeparator.java +++ b/diff-tool/src/main/java/de/skuzzle/difftool/GitLineSeparator.java @@ -49,6 +49,7 @@ String eol() { static String execute(String command) { try { final Process exec = Runtime.getRuntime().exec(command); + exec.waitFor(); try (var err = exec.getErrorStream()) { err.readAllBytes(); } From 354f201b53d7d55689d52afac7df6cfc9c75b88e Mon Sep 17 00:00:00 2001 From: Simon Taddiken Date: Mon, 27 Mar 2023 13:48:01 +0200 Subject: [PATCH 9/9] Apply spotless --- .../difftool/thirdparty/DiffUtilsDiffAlgorithm.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/diff-tool/src/main/java/de/skuzzle/difftool/thirdparty/DiffUtilsDiffAlgorithm.java b/diff-tool/src/main/java/de/skuzzle/difftool/thirdparty/DiffUtilsDiffAlgorithm.java index 461fb24f..90c07e26 100644 --- a/diff-tool/src/main/java/de/skuzzle/difftool/thirdparty/DiffUtilsDiffAlgorithm.java +++ b/diff-tool/src/main/java/de/skuzzle/difftool/thirdparty/DiffUtilsDiffAlgorithm.java @@ -1,16 +1,17 @@ package de.skuzzle.difftool.thirdparty; -import com.github.difflib.text.DiffRow; -import com.github.difflib.text.DiffRowGenerator; -import de.skuzzle.difftool.DiffAlgorithm; -import de.skuzzle.difftool.DiffLine; - import java.util.List; import java.util.Objects; import java.util.function.Consumer; import java.util.function.Function; import java.util.stream.Collectors; +import de.skuzzle.difftool.DiffAlgorithm; +import de.skuzzle.difftool.DiffLine; + +import com.github.difflib.text.DiffRow; +import com.github.difflib.text.DiffRowGenerator; + /** * Based on java-diff-utils. */