Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Output diff of expected and actual values in ConsoleLauncher #4017

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
Draft
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*Date of Release:* ❓

*Scope:* ❓
* `ConsoleLauncher` output shows extra diff message for failed assertions on two `CharSequence` objects

For a complete list of all _closed_ issues and pull requests for this release, consult the
link:{junit5-repo}+/milestone/75?closed=1+[5.12.0-M1] milestone page in the
Expand Down
2 changes: 2 additions & 0 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ opentest4j = "1.3.0"
openTestReporting = "0.1.0-M2"
surefire = "3.5.0"
xmlunit = "2.10.0"
javadiffutils = "4.12"
XJ114514 marked this conversation as resolved.
Show resolved Hide resolved

[libraries]
ant = { module = "org.apache.ant:ant", version.ref = "ant" }
Expand All @@ -44,6 +45,7 @@ jfrunit = { module = "org.moditect.jfrunit:jfrunit-core", version = "1.0.0.Alpha
jimfs = { module = "com.google.jimfs:jimfs", version = "1.3.0" }
jmh-core = { module = "org.openjdk.jmh:jmh-core", version.ref = "jmh" }
jmh-generator-annprocess = { module = "org.openjdk.jmh:jmh-generator-annprocess", version.ref = "jmh" }
java-diff-utils = { module = "io.github.java-diff-utils:java-diff-utils", version.ref = "javadiffutils" }
joox = { module = "org.jooq:joox", version = "2.0.1" }
jte = { module = "gg.jte:jte", version = "3.1.12" }
junit4 = { module = "junit:junit", version = { require = "[4.12,)", prefer = "4.13.2" } }
Expand Down
6 changes: 5 additions & 1 deletion junit-platform-console/junit-platform-console.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ dependencies {

compileOnly(libs.openTestReporting.events)

implementation(libs.java.diff.utils)

shadowed(libs.picocli)

osgiVerification(projects.junitJupiterEngine)
Expand All @@ -28,7 +30,9 @@ tasks {
"--add-modules", "org.opentest4j.reporting.events",
"--add-reads", "${project.projects.junitPlatformReporting.dependencyProject.javaModuleName}=org.opentest4j.reporting.events",
"--add-modules", "info.picocli",
"--add-reads", "${javaModuleName}=info.picocli"
"--add-reads", "${javaModuleName}=info.picocli",
"--add-modules", "io.github.javadiffutils",
"--add-reads", "${javaModuleName}=io.github.javadiffutils"
))
}
shadowJar {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,15 @@
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.EnumSet;
import java.util.List;
import java.util.Optional;
import java.util.function.Supplier;

import com.github.difflib.text.DiffRow;
import com.github.difflib.text.DiffRowGenerator;

import org.apiguardian.api.API;
import org.junit.platform.commons.JUnitException;
import org.junit.platform.commons.util.ClassLoaderUtils;
Expand All @@ -36,6 +40,8 @@
import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
import org.junit.platform.launcher.listeners.TestExecutionSummary;
import org.junit.platform.reporting.legacy.xml.LegacyXmlReportGeneratingListener;
import org.opentest4j.AssertionFailedError;
import org.opentest4j.ValueWrapper;

/**
* @since 1.0
Expand Down Expand Up @@ -180,11 +186,39 @@ private Optional<TestExecutionListener> createXmlWritingListener(PrintWriter out
private void printSummary(TestExecutionSummary summary, PrintWriter out) {
// Otherwise the failures have already been printed in detail
if (EnumSet.of(Details.NONE, Details.SUMMARY, Details.TREE).contains(outputOptions.getDetails())) {
//adding diff code here
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be done in place where we print the exceptions (as it is in #3397). Otherwise, it would be difficult to find the test that belongs to the diff in case of multiple failing tests.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
image

Which part do you wish to change? The first print happens in DefaultLaucher and the second happens in MutableTestExecutionSummary. Both are in junit-platform-laucher module. Changes will affect all Junit API output I believe?
I can try to work on it over the weekend if you wish.
At the same time, can you help to find why the package documentation test case fails (Test cases can not find diff package once I add it to the module-info.java for documentation)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need to make sure that the diff is printed if a details mode other than Details.NONE, Details.SUMMARY, or Details.TREE is used. We should be able to extract the diff-printing code to a separate class and call it from MutableTestExecutionSummary, and the other listeners (FlatPrintingListener etc.).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need to make sure that the diff is printed if a details mode other than Details.NONE, Details.SUMMARY, or Details.TREE is used. We should be able to extract the diff-printing code to a separate class and call it from MutableTestExecutionSummary, and the other listeners (FlatPrintingListener etc.).

TBH, I do not know what you meant by the "details mode." All I did so far was dive into the code, find the corresponding output location of the console, and modify it. Is there a Javadoc or class diagram or something similar I can use to learn how the Junit5 handles output?

summary.getFailures().forEach(failure -> {
//get AssertionFailedError
if (failure.getException() instanceof AssertionFailedError) {
AssertionFailedError assertionFailedError = (AssertionFailedError) failure.getException();
ValueWrapper expected = assertionFailedError.getExpected();
ValueWrapper actual = assertionFailedError.getActual();
//apply diff function
if (isCharSequence(expected) && isCharSequence(actual)) {
DiffRowGenerator generator = DiffRowGenerator.create().showInlineDiffs(true).inlineDiffByWord(
true).oldTag(f -> "~").newTag(f -> "**").build();
List<DiffRow> rows = generator.generateDiffRows(
Arrays.asList(expected.getStringRepresentation()),
Arrays.asList(actual.getStringRepresentation()));

out.printf(
"\nPlease put the diff result below into a online markdown editor to see markdown effect: \n");
for (DiffRow row : rows) {
out.printf(" | %s | %s | \n", row.getOldLine(), row.getNewLine());
}
}

}
});
summary.printFailuresTo(out);
}
summary.printTo(out);
}

private boolean isCharSequence(ValueWrapper value) {
return value != null && CharSequence.class.isAssignableFrom(value.getType());
}

@FunctionalInterface
public interface Factory {
ConsoleTestExecutor create(TestDiscoveryOptions discoveryOptions, TestConsoleOutputOptions outputOptions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,6 @@
requires org.junit.platform.launcher;
requires org.junit.platform.reporting;


provides java.util.spi.ToolProvider with org.junit.platform.console.ConsoleLauncherToolProvider;
}
Loading