-
Notifications
You must be signed in to change notification settings - Fork 1
Add review-console module #196
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -57,6 +57,25 @@ include::../../../test/kotlin/examples/kotlin/BasicsDocTest.kt[tag=approve_revie | |||||||||||||||||||||||||||||
| ---- | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| == Console Review | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| The `review-console` module provides a non-blocking reviewer that prints a colored unified diff to the test output. | ||||||||||||||||||||||||||||||
| This lets you see what changed directly in your IDE's test panel or CI logs, without needing an external diff tool. | ||||||||||||||||||||||||||||||
| The test will still fail, so you can review the diff and then approve the received file manually or with the `automatic` reviewer. | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| To use it in your test code: | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| [source,java,indent=0] | ||||||||||||||||||||||||||||||
| ---- | ||||||||||||||||||||||||||||||
| .reviewedBy(ConsoleFileReviewer.console()) | ||||||||||||||||||||||||||||||
| ---- | ||||||||||||||||||||||||||||||
|
Comment on lines
+68
to
+71
|
||||||||||||||||||||||||||||||
| [source,java,indent=0] | |
| ---- | |
| .reviewedBy(ConsoleFileReviewer.console()) | |
| ---- | |
| [source,java,indent=0,role="primary"] | |
| .Java | |
| ---- | |
| .reviewedBy(ConsoleFileReviewer.console()) | |
| ---- | |
| [source,kotlin,indent=0,role="secondary"] | |
| .Kotlin | |
| ---- | |
| .reviewedBy(ConsoleFileReviewer.console()) | |
| ---- |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| @file:Suppress("UnstableApiUsage", "unused") | ||
|
|
||
| plugins { | ||
| `java-library` | ||
| jacoco | ||
| `jvm-test-suite` | ||
| `maven-publish` | ||
| } | ||
|
|
||
| java { | ||
| withJavadocJar() | ||
| withSourcesJar() | ||
| toolchain { languageVersion.set(JavaLanguageVersion.of(21)) } | ||
| } | ||
|
|
||
| repositories { mavenCentral() } | ||
|
|
||
| dependencies { | ||
| api(project(":modules:core")) | ||
| api(libs.jspecify) | ||
|
|
||
| implementation(libs.java.diff.utils) | ||
| } | ||
|
|
||
| testing { | ||
| suites { | ||
| val test by | ||
| getting(JvmTestSuite::class) { | ||
| useJUnitJupiter() | ||
| dependencies { | ||
| implementation(testFixtures(project(":modules:core"))) | ||
|
|
||
| implementation(platform(libs.junit.bom)) | ||
| implementation(libs.junit.jupiter.api) | ||
| implementation(libs.assertj.core) | ||
|
|
||
| runtimeOnly(libs.junit.platform.launcher) | ||
| runtimeOnly(libs.junit.jupiter.engine) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| tasks.jacocoTestReport { reports { xml.required = true } } | ||
|
Comment on lines
+1
to
+44
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| package org.approvej.review.console; | ||
|
|
||
| import static java.nio.file.Files.readString; | ||
|
|
||
| import com.github.difflib.DiffUtils; | ||
| import com.github.difflib.UnifiedDiffUtils; | ||
| import java.io.IOException; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
| import java.util.Arrays; | ||
| import java.util.List; | ||
| import java.util.logging.Logger; | ||
| import org.approvej.approve.PathProvider; | ||
| import org.approvej.review.FileReviewResult; | ||
| import org.approvej.review.FileReviewer; | ||
| import org.approvej.review.FileReviewerProvider; | ||
| import org.approvej.review.ReviewResult; | ||
| import org.jspecify.annotations.NullMarked; | ||
|
|
||
| /** | ||
| * A {@link FileReviewer} that prints a colored unified diff to {@code System.out}. | ||
| * | ||
| * <p>This is a non-blocking reviewer: it displays the differences between the received and approved | ||
| * files but does not approve them. The test will fail, allowing you to inspect the diff in the test | ||
| * output and then approve the received file manually or with the {@code automatic} reviewer. | ||
| */ | ||
| @NullMarked | ||
| public final class ConsoleFileReviewer implements FileReviewer, FileReviewerProvider { | ||
|
|
||
| private static final Logger LOGGER = Logger.getLogger(ConsoleFileReviewer.class.getName()); | ||
|
|
||
| static final String ANSI_RESET = "\033[0m"; | ||
| static final String ANSI_RED_STRIKETHROUGH = "\033[31;9m"; | ||
| static final String ANSI_GREEN_BOLD = "\033[32;1m"; | ||
| static final String ANSI_CYAN = "\033[36m"; | ||
|
|
||
| private final Terminal terminal; | ||
|
|
||
| /** Creates a new {@link ConsoleFileReviewer}. */ | ||
| public ConsoleFileReviewer() { | ||
| this(Terminal.system()); | ||
| } | ||
|
|
||
| ConsoleFileReviewer(Terminal terminal) { | ||
| this.terminal = terminal; | ||
| } | ||
|
|
||
| /** | ||
| * Creates a new {@link ConsoleFileReviewer} for use with {@code reviewedBy()}. | ||
| * | ||
| * @return a new {@link ConsoleFileReviewer} | ||
| */ | ||
| public static ConsoleFileReviewer console() { | ||
| return new ConsoleFileReviewer(); | ||
| } | ||
|
|
||
| @Override | ||
| public ReviewResult apply(PathProvider pathProvider) { | ||
| try { | ||
| List<String> unifiedDiff = | ||
| computeDiff(pathProvider.approvedPath(), pathProvider.receivedPath()); | ||
| boolean useColor = terminal.supportsColor(); | ||
| for (String line : unifiedDiff) { | ||
| terminal.print((useColor ? colorize(line) : line) + "\n"); | ||
| } | ||
| } catch (IOException exception) { | ||
| LOGGER.info("Console review failed with exception %s.".formatted(exception)); | ||
| } | ||
| return new FileReviewResult(false); | ||
| } | ||
|
|
||
| @Override | ||
| public String alias() { | ||
| return "console"; | ||
| } | ||
|
|
||
| @Override | ||
| public FileReviewer create() { | ||
| return new ConsoleFileReviewer(); | ||
| } | ||
|
|
||
| static List<String> computeDiff(Path approvedPath, Path receivedPath) throws IOException { | ||
| String approvedContent = Files.exists(approvedPath) ? readString(approvedPath) : ""; | ||
| String receivedContent = readString(receivedPath); | ||
| List<String> approvedLines = Arrays.asList(approvedContent.split("\n", -1)); | ||
| List<String> receivedLines = Arrays.asList(receivedContent.split("\n", -1)); | ||
| var patch = DiffUtils.diff(approvedLines, receivedLines); | ||
| return UnifiedDiffUtils.generateUnifiedDiff( | ||
| approvedPath.toString(), receivedPath.toString(), approvedLines, patch, 3); | ||
| } | ||
|
|
||
| static String colorize(String line) { | ||
| if (line.startsWith("@@")) { | ||
| return ANSI_CYAN + line + ANSI_RESET; | ||
| } | ||
| if (line.startsWith("+")) { | ||
| return ANSI_GREEN_BOLD + line + ANSI_RESET; | ||
| } | ||
| if (line.startsWith("-")) { | ||
| return ANSI_RED_STRIKETHROUGH + line + ANSI_RESET; | ||
| } | ||
| return line; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,42 @@ | ||||||||||||||||||||||||||||||||||||||||||
| package org.approvej.review.console; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| import org.jspecify.annotations.NullMarked; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||
| * An in-memory {@link Terminal} for testing. | ||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||
| * <p>Captures all output in a {@link StringBuilder}. | ||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||
| @NullMarked | ||||||||||||||||||||||||||||||||||||||||||
| final class NullableTerminal implements Terminal { | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| private final StringBuilder output = new StringBuilder(); | ||||||||||||||||||||||||||||||||||||||||||
| private final boolean supportsColor; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| NullableTerminal() { | ||||||||||||||||||||||||||||||||||||||||||
| this(false); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| NullableTerminal(boolean supportsColor) { | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+11
to
+20
|
||||||||||||||||||||||||||||||||||||||||||
| final class NullableTerminal implements Terminal { | |
| private final StringBuilder output = new StringBuilder(); | |
| private final boolean supportsColor; | |
| NullableTerminal() { | |
| this(false); | |
| } | |
| NullableTerminal(boolean supportsColor) { | |
| final class InMemoryTerminal implements Terminal { | |
| private final StringBuilder output = new StringBuilder(); | |
| private final boolean supportsColor; | |
| InMemoryTerminal() { | |
| this(false); | |
| } | |
| InMemoryTerminal(boolean supportsColor) { |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| package org.approvej.review.console; | ||
|
|
||
| import org.jspecify.annotations.NullMarked; | ||
|
|
||
| /** Abstraction for writing output to a terminal and querying its capabilities. */ | ||
| @NullMarked | ||
| interface Terminal { | ||
|
|
||
| /** | ||
| * Prints the given text to the terminal. | ||
| * | ||
| * @param text the text to print | ||
| */ | ||
| void print(String text); | ||
|
|
||
| /** | ||
| * Returns whether this terminal supports ANSI color codes. | ||
| * | ||
| * @return {@code true} if ANSI colors can be used | ||
| */ | ||
| boolean supportsColor(); | ||
|
|
||
| /** Creates a terminal that writes to {@code System.out} and respects the {@code NO_COLOR} env. */ | ||
| static Terminal system() { | ||
| return new Terminal() { | ||
| @Override | ||
| public void print(String text) { | ||
| System.out.print(text); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean supportsColor() { | ||
| return System.getenv("NO_COLOR") == null; | ||
| } | ||
| }; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| org.approvej.review.console.ConsoleFileReviewer |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| package org.approvej.review.console; | ||
|
|
||
| import static java.nio.file.Files.writeString; | ||
| import static org.approvej.approve.PathProviders.approvedPath; | ||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| import java.io.IOException; | ||
| import java.nio.file.Path; | ||
| import java.nio.file.StandardOpenOption; | ||
| import org.approvej.approve.PathProvider; | ||
| import org.approvej.review.ReviewResult; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.io.TempDir; | ||
|
|
||
| class ConsoleFileReviewerTest { | ||
|
|
||
| @TempDir private Path tempDir; | ||
|
|
||
| @Test | ||
| void apply() throws IOException { | ||
| var terminal = new NullableTerminal(); | ||
| var reviewer = new ConsoleFileReviewer(terminal); | ||
| PathProvider pathProvider = approvedPath(tempDir.resolve("apply-approved.txt")); | ||
| writeString(pathProvider.approvedPath(), "old content\n", StandardOpenOption.CREATE); | ||
| writeString(pathProvider.receivedPath(), "new content\n", StandardOpenOption.CREATE); | ||
|
|
||
| ReviewResult result = reviewer.apply(pathProvider); | ||
|
|
||
| assertThat(result.needsReapproval()).isFalse(); | ||
| assertThat(pathProvider.approvedPath()).content().isEqualTo("old content\n"); | ||
| assertThat(pathProvider.receivedPath()).content().isEqualTo("new content\n"); | ||
| assertThat(terminal.output()).contains("-old content"); | ||
| assertThat(terminal.output()).contains("+new content"); | ||
| assertThat(terminal.output()).doesNotContain("\033["); | ||
| } | ||
|
|
||
| @Test | ||
| void apply_colored() throws IOException { | ||
| var terminal = new NullableTerminal(true); | ||
| var reviewer = new ConsoleFileReviewer(terminal); | ||
| PathProvider pathProvider = approvedPath(tempDir.resolve("colored-approved.txt")); | ||
| writeString(pathProvider.approvedPath(), "old content\n", StandardOpenOption.CREATE); | ||
| writeString(pathProvider.receivedPath(), "new content\n", StandardOpenOption.CREATE); | ||
|
|
||
| ReviewResult result = reviewer.apply(pathProvider); | ||
|
|
||
| assertThat(result.needsReapproval()).isFalse(); | ||
| assertThat(terminal.output()).contains("\033[31;9m-old content\033[0m"); | ||
| assertThat(terminal.output()).contains("\033[32;1m+new content\033[0m"); | ||
| } | ||
|
|
||
| @Test | ||
| void apply_no_approved_file() throws IOException { | ||
| var terminal = new NullableTerminal(); | ||
| var reviewer = new ConsoleFileReviewer(terminal); | ||
| PathProvider pathProvider = approvedPath(tempDir.resolve("new-approved.txt")); | ||
| writeString(pathProvider.receivedPath(), "brand new content\n", StandardOpenOption.CREATE); | ||
|
|
||
| ReviewResult result = reviewer.apply(pathProvider); | ||
|
|
||
| assertThat(result.needsReapproval()).isFalse(); | ||
| assertThat(terminal.output()).contains("+brand new content"); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,4 +18,6 @@ include("modules:http") | |
|
|
||
| include("modules:http-wiremock") | ||
|
|
||
| include("modules:review-console") | ||
|
||
|
|
||
| include("manual") | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The alphabetical ordering in the versions section has been disrupted. The "junit" entry was moved from line 1 to line 5, breaking the alphabetical order (assertj, jackson2, jackson3, junit). The entries should be sorted alphabetically: assertj, jackson2, jackson3, junit, wiremock.