Skip to content

Commit

Permalink
Make Diff java 7 compatible.
Browse files Browse the repository at this point in the history
Improve diff output
Partial code clean up
  • Loading branch information
joel-costigliola committed Dec 12, 2015
1 parent 389b7cb commit dab83a3
Show file tree
Hide file tree
Showing 50 changed files with 2,240 additions and 2,263 deletions.
Expand Up @@ -16,14 +16,10 @@

import org.assertj.core.description.Description;
import org.assertj.core.presentation.Representation;

import org.assertj.core.util.diff.Delta;

/**
*
* Base class for text content error.
*
* @author Joel Costigliola
*
*/
public class AbstractShouldHaveTextContent extends BasicErrorMessageFactory {

Expand All @@ -39,21 +35,24 @@ public String create(Description d, Representation representation) {
//
// case 1 - append diffs to String passed in super :
// super("file:<%s> and file:<%s> do not have equal content:" + diffs, actual, expected);
// this leads to a MissingFormatArgumentException if diffs contains a format specifier (like %s) because the String will
// this leads to a MissingFormatArgumentException if diffs contains a format specifier (like %s) because the String
// will
// finally be evaluated with String.format
//
// case 2 - add as format arg to the String passed in super :
// super("file:<%s> and file:<%s> do not have equal content:"actual, expected, diffs);
// this is better than case 1 but the diffs String will be quoted before the class to String.format as all String in AssertJ
// this is better than case 1 but the diffs String will be quoted before the class to String.format as all String in
// AssertJ
// error message. This is not what we want
//
// The solution is to keep diffs as an attribute and append it after String.format has been applied on the error message.
// The solution is to keep diffs as an attribute and append it after String.format has been applied on the error
// message.
return super.create(d, representation) + diffs;
}

protected static String diffsAsString(List<String> diffsList) {
protected static String diffsAsString(List<Delta<String>> diffsList) {
StringBuilder stringBuilder = new StringBuilder();
for (String diff : diffsList)
for (Delta<String> diff : diffsList)
stringBuilder.append(org.assertj.core.util.Compatibility.System.lineSeparator()).append(diff);
return stringBuilder.toString();
}
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/org/assertj/core/error/ShouldHaveContent.java
Expand Up @@ -17,6 +17,8 @@
import java.nio.file.Path;
import java.util.List;

import org.assertj.core.util.diff.Delta;


/**
* Creates an error message indicating that an assertion that verifies that a file/path has a given text content failed.
Expand All @@ -32,7 +34,7 @@ public class ShouldHaveContent extends AbstractShouldHaveTextContent {
* @param diffs the differences between {@code actual} and the expected text that was provided in the assertion.
* @return the created {@code ErrorMessageFactory}.
*/
public static ErrorMessageFactory shouldHaveContent(File actual, Charset charset, List<String> diffs) {
public static ErrorMessageFactory shouldHaveContent(File actual, Charset charset, List<Delta<String>> diffs) {
return new ShouldHaveContent(actual, charset, diffsAsString(diffs));
}

Expand All @@ -43,7 +45,7 @@ public static ErrorMessageFactory shouldHaveContent(File actual, Charset charset
* @param diffs the differences between {@code actual} and the expected text that was provided in the assertion.
* @return the created {@code ErrorMessageFactory}.
*/
public static ErrorMessageFactory shouldHaveContent(Path actual, Charset charset, List<String> diffs) {
public static ErrorMessageFactory shouldHaveContent(Path actual, Charset charset, List<Delta<String>> diffs) {
return new ShouldHaveContent(actual, charset, diffsAsString(diffs));
}

Expand Down
Expand Up @@ -18,6 +18,8 @@
import java.nio.file.Path;
import java.util.List;

import org.assertj.core.util.diff.Delta;

/**
* Creates an error message indicating that an assertion that verifies that two files/inputStreams/paths have same content failed.
*
Expand All @@ -34,7 +36,7 @@ public class ShouldHaveSameContent extends AbstractShouldHaveTextContent {
* @param diffs the differences between {@code actual} and {@code expected}.
* @return the created {@code ErrorMessageFactory}.
*/
public static ErrorMessageFactory shouldHaveSameContent(File actual, File expected, List<String> diffs) {
public static ErrorMessageFactory shouldHaveSameContent(File actual, File expected, List<Delta<String>> diffs) {
return new ShouldHaveSameContent(actual, expected, diffsAsString(diffs));
}

Expand All @@ -45,7 +47,7 @@ public static ErrorMessageFactory shouldHaveSameContent(File actual, File expect
* @param diffs the differences between {@code actual} and {@code expected}.
* @return the created {@code ErrorMessageFactory}.
*/
public static ErrorMessageFactory shouldHaveSameContent(InputStream actual, InputStream expected, List<String> diffs) {
public static ErrorMessageFactory shouldHaveSameContent(InputStream actual, InputStream expected, List<Delta<String>> diffs) {
return new ShouldHaveSameContent(actual, expected, diffsAsString(diffs));
}

Expand All @@ -56,7 +58,7 @@ public static ErrorMessageFactory shouldHaveSameContent(InputStream actual, Inpu
* @param diffs the differences between {@code actual} and {@code expected}.
* @return the created {@code ErrorMessageFactory}.
*/
public static ErrorMessageFactory shouldHaveSameContent(Path actual, Path expected, List<String> diffs) {
public static ErrorMessageFactory shouldHaveSameContent(Path actual, Path expected, List<Delta<String>> diffs) {
return new ShouldHaveSameContent(actual, expected, diffsAsString(diffs));
}

Expand Down
60 changes: 24 additions & 36 deletions src/main/java/org/assertj/core/internal/Diff.java
Expand Up @@ -12,22 +12,25 @@
*/
package org.assertj.core.internal;

import org.assertj.core.util.VisibleForTesting;
import org.assertj.core.util.diff.Delta;
import org.assertj.core.util.diff.DiffUtils;
import org.assertj.core.util.diff.Patch;
import org.assertj.core.util.diff.StringUtils;
import static java.util.Collections.unmodifiableList;
import static org.assertj.core.util.Closeables.closeQuietly;

import java.io.*;
import java.io.BufferedReader;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.StringReader;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;

import static java.util.Collections.unmodifiableList;
import static org.assertj.core.util.Closeables.closeQuietly;
import org.assertj.core.util.VisibleForTesting;
import org.assertj.core.util.diff.Delta;
import org.assertj.core.util.diff.DiffUtils;
import org.assertj.core.util.diff.Patch;


/**
Expand All @@ -42,10 +45,9 @@
*/
@VisibleForTesting
public class Diff {
private static final String EOF = "EOF";

@VisibleForTesting
public List<String> diff(InputStream actual, InputStream expected) throws IOException {
public List<Delta<String>> diff(InputStream actual, InputStream expected) throws IOException {
BufferedReader reader1 = null;
BufferedReader reader2 = null;
try {
Expand All @@ -59,12 +61,12 @@ public List<String> diff(InputStream actual, InputStream expected) throws IOExce
}

@VisibleForTesting
public List<String> diff(File actual, File expected) throws IOException {
public List<Delta<String>> diff(File actual, File expected) throws IOException {
return diff(actual.toPath(), expected.toPath());
}

@VisibleForTesting
public List<String> diff(Path actual, Path expected) throws IOException {
public List<Delta<String>> diff(Path actual, Path expected) throws IOException {
BufferedReader reader1 = null;
BufferedReader reader2 = null;
try {
Expand All @@ -78,15 +80,15 @@ public List<String> diff(Path actual, Path expected) throws IOException {
}

@VisibleForTesting
public List<String> diff(File actual, String expected, Charset charset) throws IOException {
public List<Delta<String>> diff(File actual, String expected, Charset charset) throws IOException {
return diff(actual.toPath(), expected, charset);
}

@VisibleForTesting
public List<String> diff(Path actual, String expected, Charset charset) throws IOException {
public List<Delta<String>> diff(Path actual, String expected, Charset charset) throws IOException {
BufferedReader reader1 = null;
try {
reader1 = Files.newBufferedReader(actual, charset);
reader1 = Files.newBufferedReader(actual, charset);
BufferedReader reader2 = readerFor(expected);
return unmodifiableList(diff(reader1, reader2));
} finally {
Expand All @@ -102,32 +104,18 @@ private BufferedReader readerFor(String string) {
return new BufferedReader(new StringReader(string));
}

private List<String> diff(BufferedReader actual, BufferedReader expected) throws IOException {
private List<Delta<String>> diff(BufferedReader actual, BufferedReader expected) throws IOException {
List<String> actualLines = linesFromBufferedReader(actual);
List<String> expectedLines = linesFromBufferedReader(expected);

Patch<String> patch = DiffUtils.diff(actualLines, expectedLines);

return patch.getDeltas().stream().map(d -> output(d)).collect(Collectors.toList());
}

private String output(Delta<String> delta) {
int line = delta.getRevised().getPosition() + 1;
String expected = endOfFileOrJoinList(delta.getRevised().getLines());
String actual = endOfFileOrJoinList(delta.getOriginal().getLines());
return String.format("line:<%d>, expected:<%s> but was:<%s>", line, expected, actual);
}

private String endOfFileOrJoinList(List<String> lines) {
return lines.isEmpty() ? "EOF" : StringUtils.join(lines, "\n");
Patch<String> patch = DiffUtils.diff(expectedLines, actualLines);
return patch.getDeltas();
}

private List<String> linesFromBufferedReader(BufferedReader reader) throws IOException {
String line;
List<String> lines = new ArrayList<>();

while ((line = reader.readLine()) != null)
{
while ((line = reader.readLine()) != null) {
lines.add(line);
}
return lines;
Expand Down
23 changes: 12 additions & 11 deletions src/main/java/org/assertj/core/internal/Files.java
Expand Up @@ -12,15 +12,6 @@
*/
package org.assertj.core.internal;

import org.assertj.core.api.AssertionInfo;
import org.assertj.core.api.exception.RuntimeIOException;
import org.assertj.core.util.VisibleForTesting;

import java.io.File;
import java.io.IOException;
import java.nio.charset.Charset;
import java.util.List;

import static org.assertj.core.error.ShouldBeAbsolutePath.shouldBeAbsolutePath;
import static org.assertj.core.error.ShouldBeDirectory.shouldBeDirectory;
import static org.assertj.core.error.ShouldBeFile.shouldBeFile;
Expand All @@ -38,6 +29,16 @@
import static org.assertj.core.error.ShouldNotExist.shouldNotExist;
import static org.assertj.core.util.Objects.areEqual;

import java.io.File;
import java.io.IOException;
import java.nio.charset.Charset;
import java.util.List;

import org.assertj.core.api.AssertionInfo;
import org.assertj.core.api.exception.RuntimeIOException;
import org.assertj.core.util.VisibleForTesting;
import org.assertj.core.util.diff.Delta;


/**
* Reusable assertions for <code>{@link File}</code>s.
Expand Down Expand Up @@ -87,7 +88,7 @@ public void assertSameContentAs(AssertionInfo info, File actual, File expected)
verifyIsFile(expected);
assertIsFile(info, actual);
try {
List<String> diffs = diff.diff(actual, expected);
List<Delta<String>> diffs = diff.diff(actual, expected);
if (diffs.isEmpty()) return;
throw failures.failure(info, shouldHaveSameContent(actual, expected, diffs));
} catch (IOException e) {
Expand Down Expand Up @@ -136,7 +137,7 @@ public void assertHasContent(AssertionInfo info, File actual, String expected, C
if (expected == null) throw new NullPointerException("The text to compare to should not be null");
assertIsFile(info, actual);
try {
List<String> diffs = diff.diff(actual, expected, charset);
List<Delta<String>> diffs = diff.diff(actual, expected, charset);
if (diffs.isEmpty()) return;
throw failures.failure(info, shouldHaveContent(actual, charset, diffs));
} catch (IOException e) {
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/assertj/core/internal/InputStreams.java
Expand Up @@ -13,7 +13,6 @@
package org.assertj.core.internal;

import static java.lang.String.format;

import static org.assertj.core.error.ShouldHaveSameContent.shouldHaveSameContent;

import java.io.IOException;
Expand All @@ -22,6 +21,7 @@

import org.assertj.core.api.AssertionInfo;
import org.assertj.core.util.VisibleForTesting;
import org.assertj.core.util.diff.Delta;


/**
Expand Down Expand Up @@ -64,7 +64,7 @@ public void assertSameContentAs(AssertionInfo info, InputStream actual, InputStr
if (expected == null) throw new NullPointerException("The InputStream to compare to should not be null");
assertNotNull(info, actual);
try {
List<String> diffs = diff.diff(actual, expected);
List<Delta<String>> diffs = diff.diff(actual, expected);
if (diffs.isEmpty()) return;
throw failures.failure(info, shouldHaveSameContent(actual, expected, diffs));
} catch (IOException e) {
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/org/assertj/core/internal/Paths.java
Expand Up @@ -45,6 +45,7 @@
import org.assertj.core.api.exception.PathsException;
import org.assertj.core.api.exception.RuntimeIOException;
import org.assertj.core.util.VisibleForTesting;
import org.assertj.core.util.diff.Delta;

/**
* Core assertion class for {@link Path} assertions
Expand Down Expand Up @@ -273,7 +274,7 @@ public void assertHasContent(final AssertionInfo info, Path actual, String expec
if (expected == null) throw new NullPointerException("The text to compare to should not be null");
assertIsReadable(info, actual);
try {
List<String> diffs = diff.diff(actual, expected, charset);
List<Delta<String>> diffs = diff.diff(actual, expected, charset);
if (diffs.isEmpty()) return;
throw failures.failure(info, shouldHaveContent(actual, charset, diffs));
} catch (IOException e) {
Expand Down Expand Up @@ -302,7 +303,7 @@ public void assertHasSameContentAs(AssertionInfo info, Path actual, Path expecte
// @format:on
assertIsReadable(info, actual);
try {
List<String> diffs = diff.diff(actual, expected);
List<Delta<String>> diffs = diff.diff(actual, expected);
if (diffs.isEmpty()) return;
throw failures.failure(info, shouldHaveSameContent(actual, expected, diffs));
} catch (IOException e) {
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/assertj/core/util/GroupFormatUtil.java
Expand Up @@ -15,9 +15,9 @@
public class GroupFormatUtil {

static final String ELEMENT_SEPARATOR = ",";
static final String ELEMENT_SEPARATOR_WITH_NEWLINE = ELEMENT_SEPARATOR + org.assertj.core.util.Compatibility.System.lineSeparator();
static final String DEFAULT_END = "]";
static final String DEFAULT_START = "[";
public static final String ELEMENT_SEPARATOR_WITH_NEWLINE = ELEMENT_SEPARATOR + org.assertj.core.util.Compatibility.System.lineSeparator();
public static final String DEFAULT_END = "]";
public static final String DEFAULT_START = "[";
// 4 spaces indentation : 2 space indentation after new line + '<' + '['
static final String INDENTATION_AFTER_NEWLINE = " ";
// used when formatting iterable to a single line
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/assertj/core/util/IterableUtil.java
Expand Up @@ -149,7 +149,7 @@ private static boolean doesDescriptionFitOnSingleLine(String singleLineDescripti
return singleLineDescription == null || singleLineDescription.length() < maxLengthForSingleLineDescription;
}

private static String format(Representation representation, Iterable<?> iterable, String start, String end,
public static String format(Representation representation, Iterable<?> iterable, String start, String end,
String elementSeparator, String indentation) {
if (iterable == null) return null;
Iterator<?> iterator = iterable.iterator();
Expand Down

0 comments on commit dab83a3

Please sign in to comment.