From e07747a6a12f7d33f2d7feaebee0a684c35e2a0e Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Thu, 19 Feb 2026 11:20:44 +0100 Subject: [PATCH 01/25] Add approved file inventory for orphan detection Track approved files in .approvej/inventory.properties so that orphaned files from renamed or deleted tests can be detected and cleaned up. The inventory is collected in-memory during a test run and written at JVM shutdown, merging with existing entries. Adds inventoryEnabled configuration (auto-disabled in CI), Gradle tasks approvejFindOrphans and approvejCleanup, and reflection-based orphan detection via a main() entry point. --- build.gradle.kts | 20 ++ .../java/org/approvej/ApprovalBuilder.java | 4 + .../approve/ApprovedFileInventory.java | 232 ++++++++++++++++++ .../org/approvej/approve/InventoryEntry.java | 7 + .../approvej/configuration/Configuration.java | 19 +- .../approve/ApprovedFileInventoryTest.java | 204 +++++++++++++++ .../configuration/ConfigurationTest.java | 36 +++ 7 files changed, 520 insertions(+), 2 deletions(-) create mode 100644 modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java create mode 100644 modules/core/src/main/java/org/approvej/approve/InventoryEntry.java create mode 100644 modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java diff --git a/build.gradle.kts b/build.gradle.kts index 1175036e..1f334b31 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -160,3 +160,23 @@ val updatePages by from(module.tasks.named("javadoc")) { into("javadoc/${module.name}") } } } + +project(":modules:core").afterEvaluate { + val testClasspath = the()["test"].runtimeClasspath + + tasks.register("approvejFindOrphans") { + group = "verification" + description = "List orphaned approved files" + classpath = testClasspath + mainClass.set("org.approvej.approve.ApprovedFileInventory") + args("--find") + } + + tasks.register("approvejCleanup") { + group = "verification" + description = "Detect and remove orphaned approved files" + classpath = testClasspath + mainClass.set("org.approvej.approve.ApprovedFileInventory") + args("--remove") + } +} diff --git a/modules/core/src/main/java/org/approvej/ApprovalBuilder.java b/modules/core/src/main/java/org/approvej/ApprovalBuilder.java index 4b8b7d06..2729c27a 100644 --- a/modules/core/src/main/java/org/approvej/ApprovalBuilder.java +++ b/modules/core/src/main/java/org/approvej/ApprovalBuilder.java @@ -11,6 +11,7 @@ import java.nio.file.Path; import java.util.function.Function; import java.util.function.UnaryOperator; +import org.approvej.approve.ApprovedFileInventory; import org.approvej.approve.Approver; import org.approvej.approve.PathProvider; import org.approvej.approve.PathProviders; @@ -201,6 +202,9 @@ public void byValue(final String previouslyApproved) { public void byFile(PathProvider pathProvider) { PathProvider updatedPathProvider = pathProvider.filenameAffix(name).filenameExtension(filenameExtension); + if (configuration.inventoryEnabled()) { + ApprovedFileInventory.record(updatedPathProvider); + } if (!(value instanceof String)) { printed().byFile(updatedPathProvider); return; diff --git a/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java new file mode 100644 index 00000000..fc831eed --- /dev/null +++ b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java @@ -0,0 +1,232 @@ +package org.approvej.approve; + +import static java.util.Arrays.stream; +import static java.util.stream.Collectors.joining; + +import java.io.BufferedReader; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.Properties; +import java.util.TreeMap; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; +import org.jspecify.annotations.NullMarked; + +/** + * Tracks approved files in an inventory so that orphaned files (from renamed or deleted tests) can + * be detected and cleaned up. + * + *

During a test run, each {@code byFile()} call records the approved file path and its + * originating test method. At JVM shutdown, the inventory is merged with any existing inventory + * file and written to {@code .approvej/inventory.properties}. + */ +@NullMarked +public class ApprovedFileInventory { + + private static final Path DEFAULT_INVENTORY_FILE = Path.of(".approvej/inventory.properties"); + private static final String HEADER = + "# ApproveJ Approved File Inventory (auto-generated, do not edit)"; + + private static final ConcurrentHashMap entries = new ConcurrentHashMap<>(); + private static final ConcurrentHashMap executedMethods = + new ConcurrentHashMap<>(); + private static final AtomicBoolean shutdownHookRegistered = new AtomicBoolean(false); + + private static Path inventoryFile = DEFAULT_INVENTORY_FILE; + + private ApprovedFileInventory() {} + + /** + * Records an approved file path in the inventory. + * + * @param pathProvider the path provider for the approved file + */ + public static void record(PathProvider pathProvider) { + TestMethod testMethod; + try { + testMethod = StackTraceTestFinderUtil.currentTestMethod(); + } catch (TestMethodNotFoundInStackTraceError e) { + return; + } + + String testReference = + "%s#%s".formatted(testMethod.testClass().getName(), testMethod.testCaseName()); + String relativePath = + Path.of("").toAbsolutePath().relativize(pathProvider.approvedPath()).toString(); + + addEntry(relativePath, testReference); + + if (shutdownHookRegistered.compareAndSet(false, true)) { + Runtime.getRuntime().addShutdownHook(new Thread(ApprovedFileInventory::writeInventory)); + } + } + + static void writeInventory() { + TreeMap merged = loadInventory(); + + merged.values().removeAll(executedMethods.keySet()); + merged.putAll(entries); + + saveInventory(merged); + } + + /** + * Finds orphaned inventory entries whose test methods no longer exist. + * + * @return a list of orphaned inventory entries + */ + static List findOrphans() { + return loadInventory().entrySet().stream() + .map(entry -> new InventoryEntry(entry.getKey(), entry.getValue())) + .filter(ApprovedFileInventory::isOrphan) + .toList(); + } + + private static boolean isOrphan(InventoryEntry entry) { + String testReference = entry.testReference(); + int hashIndex = testReference.indexOf('#'); + if (hashIndex < 0) { + return true; + } + String className = testReference.substring(0, hashIndex); + String methodName = testReference.substring(hashIndex + 1); + try { + return stream(Class.forName(className).getDeclaredMethods()) + .noneMatch(method -> method.getName().equals(methodName)); + } catch (ClassNotFoundException e) { + return true; + } + } + + /** + * Removes orphaned approved files and updates the inventory. + * + * @return the list of removed orphan entries + */ + static List removeOrphans() { + List orphans = findOrphans(); + if (orphans.isEmpty()) { + return orphans; + } + + TreeMap inventory = loadInventory(); + orphans.forEach( + orphan -> { + try { + Files.deleteIfExists(Path.of(orphan.relativePath())); + } catch (IOException e) { + System.err.printf("Failed to delete orphaned file: %s%n", orphan.relativePath()); + } + inventory.remove(orphan.relativePath()); + }); + + saveInventory(inventory); + + return orphans; + } + + static TreeMap loadInventory() { + TreeMap result = new TreeMap<>(); + if (!Files.exists(inventoryFile)) { + return result; + } + Properties properties = new Properties(); + try (BufferedReader reader = Files.newBufferedReader(inventoryFile)) { + properties.load(reader); + } catch (IOException e) { + System.err.printf("Failed to read inventory file: %s%n", e.getMessage()); + return result; + } + properties.stringPropertyNames().forEach(key -> result.put(key, properties.getProperty(key))); + return result; + } + + private static void saveInventory(TreeMap inventory) { + try { + if (inventory.isEmpty()) { + Files.deleteIfExists(inventoryFile); + } else { + Files.createDirectories(inventoryFile.getParent()); + String content = + "%s\n%s" + .formatted( + HEADER, + inventory.entrySet().stream() + .map(e -> "%s = %s".formatted(escapeKey(e.getKey()), e.getValue())) + .collect(joining("\n", "", "\n"))); + Files.writeString(inventoryFile, content); + } + } catch (IOException e) { + System.err.printf("Failed to write inventory file: %s%n", e.getMessage()); + } + } + + private static String escapeKey(String key) { + return key.replace("\\", "\\\\").replace(" ", "\\ ").replace("=", "\\=").replace(":", "\\:"); + } + + /** Adds an entry directly. For testing only. */ + static void addEntry(String relativePath, String testReference) { + entries.put(relativePath, testReference); + executedMethods.put(testReference, Boolean.TRUE); + } + + /** Resets static state and sets the inventory file path. For testing only. */ + static void reset(Path testInventoryFile) { + entries.clear(); + executedMethods.clear(); + shutdownHookRegistered.set(false); + inventoryFile = testInventoryFile; + } + + /** Resets static state to defaults. For testing only. */ + static void reset() { + reset(DEFAULT_INVENTORY_FILE); + } + + /** + * CLI entry point for Gradle tasks. + * + * @param args {@code --find} to list orphans, {@code --remove} to delete them + */ + public static void main(String[] args) { + String usage = "Usage: ApprovedFileInventory --find | --remove"; + if (args.length == 0) { + System.err.println(usage); + System.exit(1); + } + + String command = args[0]; + + switch (command) { + case "--find" -> { + List orphans = findOrphans(); + if (orphans.isEmpty()) { + System.out.println("No orphaned approved files found."); + } else { + System.out.println("Orphaned approved files:"); + orphans.forEach( + orphan -> + System.out.printf( + " %s (from %s)%n", orphan.relativePath(), orphan.testReference())); + } + } + case "--remove" -> { + List removed = removeOrphans(); + if (removed.isEmpty()) { + System.out.println("No orphaned approved files found."); + } else { + System.out.println("Removed orphaned approved files:"); + removed.forEach(orphan -> System.out.printf(" %s%n", orphan.relativePath())); + } + } + default -> { + System.err.printf("Unknown command: %s%n", command); + System.err.println(usage); + System.exit(1); + } + } + } +} diff --git a/modules/core/src/main/java/org/approvej/approve/InventoryEntry.java b/modules/core/src/main/java/org/approvej/approve/InventoryEntry.java new file mode 100644 index 00000000..cc81c270 --- /dev/null +++ b/modules/core/src/main/java/org/approvej/approve/InventoryEntry.java @@ -0,0 +1,7 @@ +package org.approvej.approve; + +import org.jspecify.annotations.NullMarked; + +/** An entry in the approved file inventory, mapping a file path to its originating test method. */ +@NullMarked +record InventoryEntry(String relativePath, String testReference) {} diff --git a/modules/core/src/main/java/org/approvej/configuration/Configuration.java b/modules/core/src/main/java/org/approvej/configuration/Configuration.java index 23cfce7d..c7925393 100644 --- a/modules/core/src/main/java/org/approvej/configuration/Configuration.java +++ b/modules/core/src/main/java/org/approvej/configuration/Configuration.java @@ -31,14 +31,18 @@ * @param defaultPrintFormat the {@link PrintFormat} that will be used if none is specified * otherwise * @param defaultFileReviewer the {@link FileReviewer} that will be used if none is specified + * @param inventoryEnabled whether the approved file inventory is enabled */ @NullMarked public record Configuration( - PrintFormat defaultPrintFormat, FileReviewer defaultFileReviewer) { + PrintFormat defaultPrintFormat, + FileReviewer defaultFileReviewer, + boolean inventoryEnabled) { private static final String DEFAULT_PRINT_FORMAT_PROPERTY = "defaultPrintFormat"; private static final String DEFAULT_FILE_REVIEWER_PROPERTY = "defaultFileReviewer"; private static final String DEFAULT_FILE_REVIEWER_SCRIPT_PROPERTY = "defaultFileReviewerScript"; + private static final String INVENTORY_ENABLED_PROPERTY = "inventoryEnabled"; /** The loaded {@link Configuration} object. */ public static final Configuration configuration = @@ -50,7 +54,9 @@ static Configuration loadConfiguration(ConfigurationLoader loader) { FileReviewer fileReviewer = resolveFileReviewer(loader); - return new Configuration(printFormat, fileReviewer); + boolean inventoryEnabled = resolveInventoryEnabled(loader); + + return new Configuration(printFormat, fileReviewer, inventoryEnabled); } @SuppressWarnings("unchecked") @@ -69,4 +75,13 @@ private static FileReviewer resolveFileReviewer(ConfigurationLoader loader) { return Registry.resolve(loader.get(DEFAULT_FILE_REVIEWER_PROPERTY, "none"), FileReviewer.class); } + + private static boolean resolveInventoryEnabled(ConfigurationLoader loader) { + String configured = loader.get(INVENTORY_ENABLED_PROPERTY); + if (configured != null) { + return Boolean.parseBoolean(configured); + } + String ci = System.getenv("CI"); + return ci == null || ci.isBlank(); + } } diff --git a/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java b/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java new file mode 100644 index 00000000..c3de94b8 --- /dev/null +++ b/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java @@ -0,0 +1,204 @@ +package org.approvej.approve; + +import static java.nio.file.Files.writeString; +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.List; +import java.util.TreeMap; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class ApprovedFileInventoryTest { + + @TempDir private Path tempDir; + + private Path inventoryFile; + + @BeforeEach + void setUp() { + inventoryFile = tempDir.resolve("inventory.properties"); + ApprovedFileInventory.reset(inventoryFile); + } + + @AfterEach + void tearDown() { + ApprovedFileInventory.reset(); + } + + @Test + void record() { + ApprovedFileInventory.addEntry( + "src/test/MyTest-myTest-approved.txt", "com.example.MyTest#myTest"); + + ApprovedFileInventory.writeInventory(); + + TreeMap inventory = ApprovedFileInventory.loadInventory(); + assertThat(inventory) + .containsEntry("src/test/MyTest-myTest-approved.txt", "com.example.MyTest#myTest"); + } + + @Test + void record_multiple_entries() { + ApprovedFileInventory.addEntry("src/test/BTest-b-approved.txt", "com.example.BTest#b"); + ApprovedFileInventory.addEntry("src/test/ATest-a-approved.txt", "com.example.ATest#a"); + + ApprovedFileInventory.writeInventory(); + + TreeMap inventory = ApprovedFileInventory.loadInventory(); + assertThat(inventory).hasSize(2); + assertThat(inventory.firstKey()).isEqualTo("src/test/ATest-a-approved.txt"); + } + + @Test + void record_named_approvals() { + ApprovedFileInventory.addEntry( + "src/test/MyTest-myTest-alpha-approved.txt", "com.example.MyTest#myTest"); + ApprovedFileInventory.addEntry( + "src/test/MyTest-myTest-beta-approved.txt", "com.example.MyTest#myTest"); + + ApprovedFileInventory.writeInventory(); + + TreeMap inventory = ApprovedFileInventory.loadInventory(); + assertThat(inventory).hasSize(2); + assertThat(inventory) + .containsEntry("src/test/MyTest-myTest-alpha-approved.txt", "com.example.MyTest#myTest") + .containsEntry("src/test/MyTest-myTest-beta-approved.txt", "com.example.MyTest#myTest"); + } + + @Test + void record_replaces_entries_for_executed_methods() throws IOException { + writeString( + inventoryFile, + """ + # ApproveJ Approved File Inventory (auto-generated, do not edit) + src/test/MyTest-myTest-alpha-approved.txt = com.example.MyTest#myTest + src/test/MyTest-myTest-beta-approved.txt = com.example.MyTest#myTest + """, + StandardOpenOption.CREATE); + + ApprovedFileInventory.addEntry( + "src/test/MyTest-myTest-gamma-approved.txt", "com.example.MyTest#myTest"); + ApprovedFileInventory.addEntry( + "src/test/MyTest-myTest-beta-approved.txt", "com.example.MyTest#myTest"); + + ApprovedFileInventory.writeInventory(); + + TreeMap inventory = ApprovedFileInventory.loadInventory(); + assertThat(inventory).hasSize(2); + assertThat(inventory) + .containsEntry("src/test/MyTest-myTest-gamma-approved.txt", "com.example.MyTest#myTest") + .containsEntry("src/test/MyTest-myTest-beta-approved.txt", "com.example.MyTest#myTest") + .doesNotContainKey("src/test/MyTest-myTest-alpha-approved.txt"); + } + + @Test + void record_preserves_entries_for_unexecuted_methods() throws IOException { + writeString( + inventoryFile, + """ + # ApproveJ Approved File Inventory (auto-generated, do not edit) + src/test/OtherTest-other-approved.txt = com.example.OtherTest#other + """, + StandardOpenOption.CREATE); + + ApprovedFileInventory.addEntry( + "src/test/MyTest-myTest-approved.txt", "com.example.MyTest#myTest"); + + ApprovedFileInventory.writeInventory(); + + TreeMap inventory = ApprovedFileInventory.loadInventory(); + assertThat(inventory).hasSize(2); + assertThat(inventory) + .containsEntry("src/test/OtherTest-other-approved.txt", "com.example.OtherTest#other") + .containsEntry("src/test/MyTest-myTest-approved.txt", "com.example.MyTest#myTest"); + } + + @Test + void record_merges_with_existing() throws IOException { + writeString( + inventoryFile, + """ + # ApproveJ Approved File Inventory (auto-generated, do not edit) + src/test/ExistingTest-existing-approved.txt = com.example.ExistingTest#existing + """, + StandardOpenOption.CREATE); + + ApprovedFileInventory.addEntry( + "src/test/NewTest-newTest-approved.txt", "com.example.NewTest#newTest"); + + ApprovedFileInventory.writeInventory(); + + TreeMap inventory = ApprovedFileInventory.loadInventory(); + assertThat(inventory).hasSize(2); + assertThat(inventory) + .containsEntry( + "src/test/ExistingTest-existing-approved.txt", "com.example.ExistingTest#existing") + .containsEntry("src/test/NewTest-newTest-approved.txt", "com.example.NewTest#newTest"); + } + + @Test + void findOrphans() throws IOException { + writeString( + inventoryFile, + """ + # ApproveJ Approved File Inventory (auto-generated, do not edit) + src/test/NonExistent-test-approved.txt = com.nonexistent.NonExistentTest#test + """, + StandardOpenOption.CREATE); + + List orphans = ApprovedFileInventory.findOrphans(); + + assertThat(orphans).hasSize(1); + assertThat(orphans.getFirst().relativePath()) + .isEqualTo("src/test/NonExistent-test-approved.txt"); + assertThat(orphans.getFirst().testReference()) + .isEqualTo("com.nonexistent.NonExistentTest#test"); + } + + @Test + void findOrphans_missing_method() throws IOException { + writeString( + inventoryFile, + """ + # ApproveJ Approved File Inventory (auto-generated, do not edit) + src/test/ApprovedFileInventoryTest-nonExistentMethod-approved.txt = org.approvej.approve.ApprovedFileInventoryTest#nonExistentMethod + """, + StandardOpenOption.CREATE); + + List orphans = ApprovedFileInventory.findOrphans(); + + assertThat(orphans).hasSize(1); + assertThat(orphans.getFirst().testReference()) + .isEqualTo("org.approvej.approve.ApprovedFileInventoryTest#nonExistentMethod"); + } + + @Test + void removeOrphans() throws IOException { + Path orphanFile = tempDir.resolve("orphan-approved.txt"); + writeString(orphanFile, "old content", StandardOpenOption.CREATE); + + Path validFile = tempDir.resolve("valid-approved.txt"); + + writeString( + inventoryFile, + "# ApproveJ Approved File Inventory (auto-generated, do not edit)\n" + + orphanFile + + " = com.nonexistent.NonExistentTest#test\n" + + validFile + + " = org.approvej.approve.ApprovedFileInventoryTest#removeOrphans\n", + StandardOpenOption.CREATE); + + List removed = ApprovedFileInventory.removeOrphans(); + + assertThat(removed).hasSize(1); + assertThat(orphanFile).doesNotExist(); + TreeMap inventory = ApprovedFileInventory.loadInventory(); + assertThat(inventory).doesNotContainKey(orphanFile.toString()); + assertThat(inventory).containsKey(validFile.toString()); + } +} diff --git a/modules/core/src/test/java/org/approvej/configuration/ConfigurationTest.java b/modules/core/src/test/java/org/approvej/configuration/ConfigurationTest.java index bc460197..cbed618f 100644 --- a/modules/core/src/test/java/org/approvej/configuration/ConfigurationTest.java +++ b/modules/core/src/test/java/org/approvej/configuration/ConfigurationTest.java @@ -197,4 +197,40 @@ void configurationLoader_emptyBuilder_returnsLoaderWithNoSources() { assertThat(loader.get("anyKey", "default")).isEqualTo("default"); } + + @Test + void loadConfiguration_inventoryEnabled_defaults_to_true() { + ConfigurationLoader loader = ConfigurationLoader.builder().build(); + + Configuration config = Configuration.loadConfiguration(loader); + + assertThat(config.inventoryEnabled()).isTrue(); + } + + @Test + void loadConfiguration_inventoryEnabled_from_properties() { + Properties props = new Properties(); + props.setProperty("inventoryEnabled", "false"); + ConfigurationLoader loader = ConfigurationLoader.builder().withProperties(props).build(); + + Configuration config = Configuration.loadConfiguration(loader); + + assertThat(config.inventoryEnabled()).isFalse(); + } + + @Test + void loadConfiguration_inventoryEnabled_env_overrides_properties() { + Map env = Map.of("APPROVEJ_INVENTORY_ENABLED", "false"); + Properties props = new Properties(); + props.setProperty("inventoryEnabled", "true"); + ConfigurationLoader loader = + ConfigurationLoader.builder() + .withEnvironmentVariables(env::get) + .withProperties(props) + .build(); + + Configuration config = Configuration.loadConfiguration(loader); + + assertThat(config.inventoryEnabled()).isFalse(); + } } From 302e25b92706fab9bd6442ed9b56b5d83f32d620 Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Thu, 19 Feb 2026 11:21:34 +0100 Subject: [PATCH 02/25] Add .approvej to .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index b8d6898f..7435375d 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ docs/** *.key pages banner-*.* +.approvej From c2931e39a4195b1bf011fd37cdfde4df1e62a91f Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Thu, 19 Feb 2026 11:47:19 +0100 Subject: [PATCH 03/25] Add Gradle plugin --- bom/build.gradle.kts | 4 +- build.gradle.kts | 20 ------- modules/gradle-plugin/build.gradle.kts | 35 +++++++++++ .../org/approvej/gradle/ApproveJPlugin.java | 54 +++++++++++++++++ .../approvej/gradle/ApproveJPluginTest.java | 60 +++++++++++++++++++ settings.gradle.kts | 2 + 6 files changed, 154 insertions(+), 21 deletions(-) create mode 100644 modules/gradle-plugin/build.gradle.kts create mode 100644 modules/gradle-plugin/src/main/java/org/approvej/gradle/ApproveJPlugin.java create mode 100644 modules/gradle-plugin/src/test/java/org/approvej/gradle/ApproveJPluginTest.java diff --git a/bom/build.gradle.kts b/bom/build.gradle.kts index b3677879..b1121be4 100644 --- a/bom/build.gradle.kts +++ b/bom/build.gradle.kts @@ -8,7 +8,9 @@ repositories { mavenCentral() } dependencies { constraints { rootProject.subprojects - .filter { it != project && it.name != "manual" && it.subprojects.isEmpty() } + .filter { + it != project && it.name !in listOf("gradle-plugin", "manual") && it.subprojects.isEmpty() + } .sortedBy { it.name } .forEach { api(it) } diff --git a/build.gradle.kts b/build.gradle.kts index 1f334b31..1175036e 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -160,23 +160,3 @@ val updatePages by from(module.tasks.named("javadoc")) { into("javadoc/${module.name}") } } } - -project(":modules:core").afterEvaluate { - val testClasspath = the()["test"].runtimeClasspath - - tasks.register("approvejFindOrphans") { - group = "verification" - description = "List orphaned approved files" - classpath = testClasspath - mainClass.set("org.approvej.approve.ApprovedFileInventory") - args("--find") - } - - tasks.register("approvejCleanup") { - group = "verification" - description = "Detect and remove orphaned approved files" - classpath = testClasspath - mainClass.set("org.approvej.approve.ApprovedFileInventory") - args("--remove") - } -} diff --git a/modules/gradle-plugin/build.gradle.kts b/modules/gradle-plugin/build.gradle.kts new file mode 100644 index 00000000..0c47072a --- /dev/null +++ b/modules/gradle-plugin/build.gradle.kts @@ -0,0 +1,35 @@ +plugins { + `java-gradle-plugin` + `jvm-test-suite` + `maven-publish` +} + +java { toolchain { languageVersion = JavaLanguageVersion.of(21) } } + +repositories { mavenCentral() } + +testing { + suites { + val test by + getting(JvmTestSuite::class) { + useJUnitJupiter() + dependencies { + implementation(platform(libs.junit.bom)) + implementation(libs.junit.jupiter.api) + implementation(libs.assertj.core) + + runtimeOnly(libs.junit.platform.launcher) + runtimeOnly(libs.junit.jupiter.engine) + } + } + } +} + +gradlePlugin { + plugins { + create("approvej") { + id = "org.approvej" + implementationClass = "org.approvej.gradle.ApproveJPlugin" + } + } +} diff --git a/modules/gradle-plugin/src/main/java/org/approvej/gradle/ApproveJPlugin.java b/modules/gradle-plugin/src/main/java/org/approvej/gradle/ApproveJPlugin.java new file mode 100644 index 00000000..0ea11238 --- /dev/null +++ b/modules/gradle-plugin/src/main/java/org/approvej/gradle/ApproveJPlugin.java @@ -0,0 +1,54 @@ +package org.approvej.gradle; + +import org.gradle.api.Plugin; +import org.gradle.api.Project; +import org.gradle.api.plugins.JavaPlugin; +import org.gradle.api.tasks.JavaExec; +import org.gradle.api.tasks.SourceSetContainer; + +/** Gradle plugin that registers tasks to find and remove orphaned approved files. */ +@SuppressWarnings("unused") +public final class ApproveJPlugin implements Plugin { + + @Override + public void apply(Project project) { + project + .getPlugins() + .withType( + JavaPlugin.class, + javaPlugin -> { + var testClasspath = + project + .getExtensions() + .getByType(SourceSetContainer.class) + .getByName("test") + .getRuntimeClasspath(); + + project + .getTasks() + .register( + "approvejFindOrphans", + JavaExec.class, + task -> { + task.setGroup("verification"); + task.setDescription("List orphaned approved files"); + task.setClasspath(testClasspath); + task.getMainClass().set("org.approvej.approve.ApprovedFileInventory"); + task.args("--find"); + }); + + project + .getTasks() + .register( + "approvejCleanup", + JavaExec.class, + task -> { + task.setGroup("verification"); + task.setDescription("Detect and remove orphaned approved files"); + task.setClasspath(testClasspath); + task.getMainClass().set("org.approvej.approve.ApprovedFileInventory"); + task.args("--remove"); + }); + }); + } +} diff --git a/modules/gradle-plugin/src/test/java/org/approvej/gradle/ApproveJPluginTest.java b/modules/gradle-plugin/src/test/java/org/approvej/gradle/ApproveJPluginTest.java new file mode 100644 index 00000000..44da0bd6 --- /dev/null +++ b/modules/gradle-plugin/src/test/java/org/approvej/gradle/ApproveJPluginTest.java @@ -0,0 +1,60 @@ +package org.approvej.gradle; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import org.gradle.testkit.runner.GradleRunner; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class ApproveJPluginTest { + + @TempDir Path tempProjectDir; + + @Test + void apply() throws IOException { + Files.writeString( + tempProjectDir.resolve("build.gradle"), + """ + plugins { + id 'java' + id 'org.approvej' + } + """); + + var result = + GradleRunner.create() + .withProjectDir(tempProjectDir.toFile()) + .withPluginClasspath() + .withArguments("tasks", "--group", "verification") + .build(); + + assertThat(result.getOutput()) + .contains("approvejFindOrphans - List orphaned approved files") + .contains("approvejCleanup - Detect and remove orphaned approved files"); + } + + @Test + void apply_without_java_plugin() throws IOException { + Files.writeString( + tempProjectDir.resolve("build.gradle"), + """ + plugins { + id 'org.approvej' + } + """); + + var result = + GradleRunner.create() + .withProjectDir(tempProjectDir.toFile()) + .withPluginClasspath() + .withArguments("tasks", "--all") + .build(); + + assertThat(result.getOutput()) + .doesNotContain("approvejFindOrphans") + .doesNotContain("approvejCleanup"); + } +} diff --git a/settings.gradle.kts b/settings.gradle.kts index 321dace2..56caed3d 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -6,6 +6,8 @@ include("bom") include("modules:core") +include("modules:gradle-plugin") + include("modules:json-jackson") include("modules:json-jackson3") From 75de625183826af69e83d8a32b276603bc23e017 Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Thu, 19 Feb 2026 11:58:34 +0100 Subject: [PATCH 04/25] Add Gradle Plugin Portal publishing support --- gradle/libs.versions.toml | 1 + modules/gradle-plugin/build.gradle.kts | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 33204904..0beea342 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -30,5 +30,6 @@ asciidoctor = { id = "org.asciidoctor.jvm.convert", version = "4.0.5" } asciidoctor-pdf = { id = "org.asciidoctor.jvm.pdf", version = "4.0.5" } jreleaser = { id = "org.jreleaser", version = "1.20.0" } kotlin-jvm = { id = "org.jetbrains.kotlin.jvm", version = "2.3.10" } +plugin-publish = { id = "com.gradle.plugin-publish", version = "2.0.0" } sonar = { id = "org.sonarqube", version = "7.2.2.6593" } spotless = { id = "com.diffplug.spotless", version = "7.2.1" } diff --git a/modules/gradle-plugin/build.gradle.kts b/modules/gradle-plugin/build.gradle.kts index 0c47072a..927e55da 100644 --- a/modules/gradle-plugin/build.gradle.kts +++ b/modules/gradle-plugin/build.gradle.kts @@ -1,7 +1,7 @@ plugins { `java-gradle-plugin` `jvm-test-suite` - `maven-publish` + alias(libs.plugins.plugin.publish) } java { toolchain { languageVersion = JavaLanguageVersion.of(21) } } @@ -26,10 +26,15 @@ testing { } gradlePlugin { + website = "https://approvej.org" + vcsUrl = "https://github.com/mkutz/approvej" plugins { create("approvej") { id = "org.approvej" implementationClass = "org.approvej.gradle.ApproveJPlugin" + displayName = "ApproveJ" + description = "Find and remove orphaned approved files" + tags = listOf("testing", "approval-testing", "snapshot-testing") } } } From 921ea08d0a5cba7e164bcee80a8c0eafaf9938e5 Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Thu, 19 Feb 2026 13:03:01 +0100 Subject: [PATCH 05/25] Publish Gradle plugin to Plugin Portal in CI --- .github/workflows/publish.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index d440f614..1e6a3a5e 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -40,6 +40,10 @@ jobs: - run: ./gradlew jreleaserConfig -Pversion=${{ github.event.inputs.version }} --git-root-search - run: ./gradlew publish -Pversion=${{ github.event.inputs.version }} + - run: ./gradlew :modules:gradle-plugin:publishPlugins -Pversion=${{ github.event.inputs.version }} + env: + GRADLE_PUBLISH_KEY: ${{ secrets.GRADLE_PUBLISH_KEY }} + GRADLE_PUBLISH_SECRET: ${{ secrets.GRADLE_PUBLISH_SECRET }} - run: ./gradlew jreleaserFullRelease -Pversion=${{ github.event.inputs.version }} --git-root-search --stacktrace - uses: actions/upload-artifact@v6 with: From 04817a33f0a960e35920aa1b55bd922d5ec3c108 Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Thu, 19 Feb 2026 13:43:51 +0100 Subject: [PATCH 06/25] Add Maven plugin module Add maven-plugin module with find-orphans and cleanup goals that fork a JVM to run ApprovedFileInventory, mirroring the existing Gradle plugin functionality for Maven users. --- bom/build.gradle.kts | 4 +- gradle/libs.versions.toml | 5 ++ .../approve/ApprovedFileInventory.java | 2 +- modules/maven-plugin/build.gradle.kts | 37 ++++++++++ modules/maven-plugin/gradle.properties | 2 + .../java/org/approvej/maven/CleanupMojo.java | 21 ++++++ .../org/approvej/maven/FindOrphansMojo.java | 21 ++++++ .../java/org/approvej/maven/MojoHelper.java | 71 +++++++++++++++++++ .../org/approvej/maven/MojoHelperTest.java | 44 ++++++++++++ settings.gradle.kts | 2 + 10 files changed, 207 insertions(+), 2 deletions(-) create mode 100644 modules/maven-plugin/build.gradle.kts create mode 100644 modules/maven-plugin/gradle.properties create mode 100644 modules/maven-plugin/src/main/java/org/approvej/maven/CleanupMojo.java create mode 100644 modules/maven-plugin/src/main/java/org/approvej/maven/FindOrphansMojo.java create mode 100644 modules/maven-plugin/src/main/java/org/approvej/maven/MojoHelper.java create mode 100644 modules/maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java diff --git a/bom/build.gradle.kts b/bom/build.gradle.kts index b1121be4..1d2660bd 100644 --- a/bom/build.gradle.kts +++ b/bom/build.gradle.kts @@ -9,7 +9,9 @@ dependencies { constraints { rootProject.subprojects .filter { - it != project && it.name !in listOf("gradle-plugin", "manual") && it.subprojects.isEmpty() + it != project && + it.name !in listOf("gradle-plugin", "maven-plugin", "manual") && + it.subprojects.isEmpty() } .sortedBy { it.name } .forEach { api(it) } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 0beea342..7db2c1bd 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -3,6 +3,7 @@ junit = "6.0.3" assertj = "3.27.7" jackson2 = "2.21.0" jackson3 = "3.0.4" +maven = "3.9.12" wiremock = "3.13.2" [libraries] @@ -16,6 +17,9 @@ jackson2-jsr310 = { module = "com.fasterxml.jackson.datatype:jackson-datatype-js jackson3-databind = { module = "tools.jackson.core:jackson-databind", version.ref = "jackson3" } jackson3-dataformat-yaml = { module = "tools.jackson.dataformat:jackson-dataformat-yaml", version.ref = "jackson3" } jspecify = { module = "org.jspecify:jspecify", version = "1.0.0" } +maven-core = { module = "org.apache.maven:maven-core", version.ref = "maven" } +maven-plugin-annotations = { module = "org.apache.maven.plugin-tools:maven-plugin-annotations", version = "3.15.2" } +maven-plugin-api = { module = "org.apache.maven:maven-plugin-api", version.ref = "maven" } junit-bom = { module = "org.junit:junit-bom", version.ref = "junit" } junit-jupiter-api = { module = "org.junit.jupiter:junit-jupiter-api", version.ref = "junit" } junit-jupiter-engine = { module = "org.junit.jupiter:junit-jupiter-engine", version.ref = "junit" } @@ -30,6 +34,7 @@ asciidoctor = { id = "org.asciidoctor.jvm.convert", version = "4.0.5" } asciidoctor-pdf = { id = "org.asciidoctor.jvm.pdf", version = "4.0.5" } jreleaser = { id = "org.jreleaser", version = "1.20.0" } kotlin-jvm = { id = "org.jetbrains.kotlin.jvm", version = "2.3.10" } +maven-plugin-development = { id = "org.gradlex.maven-plugin-development", version = "1.0.3" } plugin-publish = { id = "com.gradle.plugin-publish", version = "2.0.0" } sonar = { id = "org.sonarqube", version = "7.2.2.6593" } spotless = { id = "com.diffplug.spotless", version = "7.2.1" } diff --git a/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java index fc831eed..2c5bc07c 100644 --- a/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java +++ b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java @@ -187,7 +187,7 @@ static void reset() { } /** - * CLI entry point for Gradle tasks. + * CLI entry point for build tool plugins. * * @param args {@code --find} to list orphans, {@code --remove} to delete them */ diff --git a/modules/maven-plugin/build.gradle.kts b/modules/maven-plugin/build.gradle.kts new file mode 100644 index 00000000..3cd5d469 --- /dev/null +++ b/modules/maven-plugin/build.gradle.kts @@ -0,0 +1,37 @@ +plugins { + `java-library` + `jvm-test-suite` + `maven-publish` + alias(libs.plugins.maven.plugin.development) +} + +java { + withJavadocJar() + withSourcesJar() + toolchain { languageVersion = JavaLanguageVersion.of(21) } +} + +repositories { mavenCentral() } + +dependencies { + compileOnly(libs.maven.plugin.annotations) + implementation(libs.maven.plugin.api) + implementation(libs.maven.core) +} + +testing { + suites { + val test by + getting(JvmTestSuite::class) { + useJUnitJupiter() + dependencies { + implementation(platform(libs.junit.bom)) + implementation(libs.junit.jupiter.api) + implementation(libs.assertj.core) + + runtimeOnly(libs.junit.platform.launcher) + runtimeOnly(libs.junit.jupiter.engine) + } + } + } +} diff --git a/modules/maven-plugin/gradle.properties b/modules/maven-plugin/gradle.properties new file mode 100644 index 00000000..8a455e35 --- /dev/null +++ b/modules/maven-plugin/gradle.properties @@ -0,0 +1,2 @@ +mavenPomName = ApproveJ Maven Plugin +mavenPomDescription = Maven plugin to find and remove orphaned approved files diff --git a/modules/maven-plugin/src/main/java/org/approvej/maven/CleanupMojo.java b/modules/maven-plugin/src/main/java/org/approvej/maven/CleanupMojo.java new file mode 100644 index 00000000..e6dac8c7 --- /dev/null +++ b/modules/maven-plugin/src/main/java/org/approvej/maven/CleanupMojo.java @@ -0,0 +1,21 @@ +package org.approvej.maven; + +import org.apache.maven.plugin.AbstractMojo; +import org.apache.maven.plugin.MojoExecutionException; +import org.apache.maven.plugins.annotations.Mojo; +import org.apache.maven.plugins.annotations.Parameter; +import org.apache.maven.plugins.annotations.ResolutionScope; +import org.apache.maven.project.MavenProject; + +/** Detects and removes orphaned approved files whose originating test method no longer exists. */ +@Mojo(name = "cleanup", requiresDependencyResolution = ResolutionScope.TEST, threadSafe = true) +public class CleanupMojo extends AbstractMojo { + + @Parameter(defaultValue = "${project}", readonly = true, required = true) + private MavenProject project; + + @Override + public void execute() throws MojoExecutionException { + MojoHelper.executeInventory(project, "--remove", getLog()); + } +} diff --git a/modules/maven-plugin/src/main/java/org/approvej/maven/FindOrphansMojo.java b/modules/maven-plugin/src/main/java/org/approvej/maven/FindOrphansMojo.java new file mode 100644 index 00000000..4d1eb56d --- /dev/null +++ b/modules/maven-plugin/src/main/java/org/approvej/maven/FindOrphansMojo.java @@ -0,0 +1,21 @@ +package org.approvej.maven; + +import org.apache.maven.plugin.AbstractMojo; +import org.apache.maven.plugin.MojoExecutionException; +import org.apache.maven.plugins.annotations.Mojo; +import org.apache.maven.plugins.annotations.Parameter; +import org.apache.maven.plugins.annotations.ResolutionScope; +import org.apache.maven.project.MavenProject; + +/** Lists orphaned approved files whose originating test method no longer exists. */ +@Mojo(name = "find-orphans", requiresDependencyResolution = ResolutionScope.TEST, threadSafe = true) +public class FindOrphansMojo extends AbstractMojo { + + @Parameter(defaultValue = "${project}", readonly = true, required = true) + private MavenProject project; + + @Override + public void execute() throws MojoExecutionException { + MojoHelper.executeInventory(project, "--find", getLog()); + } +} diff --git a/modules/maven-plugin/src/main/java/org/approvej/maven/MojoHelper.java b/modules/maven-plugin/src/main/java/org/approvej/maven/MojoHelper.java new file mode 100644 index 00000000..07638aa3 --- /dev/null +++ b/modules/maven-plugin/src/main/java/org/approvej/maven/MojoHelper.java @@ -0,0 +1,71 @@ +package org.approvej.maven; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import org.apache.maven.plugin.MojoExecutionException; +import org.apache.maven.plugin.logging.Log; +import org.apache.maven.project.MavenProject; + +/** Shared helper for forking a JVM to run {@code ApprovedFileInventory}. */ +final class MojoHelper { + + private static final String MAIN_CLASS = "org.approvej.approve.ApprovedFileInventory"; + + private MojoHelper() {} + + static void executeInventory(MavenProject project, String command, Log log) + throws MojoExecutionException { + List classpathElements; + try { + classpathElements = project.getTestClasspathElements(); + } catch (Exception e) { + throw new MojoExecutionException("Failed to resolve test classpath", e); + } + + List cmd = buildCommand(classpathElements, command); + + try { + ProcessBuilder processBuilder = new ProcessBuilder(cmd); + processBuilder.directory(project.getBasedir()); + processBuilder.redirectErrorStream(false); + + Process process = processBuilder.start(); + + try (BufferedReader stdout = + new BufferedReader(new InputStreamReader(process.getInputStream()))) { + stdout.lines().forEach(log::info); + } + try (BufferedReader stderr = + new BufferedReader(new InputStreamReader(process.getErrorStream()))) { + stderr.lines().forEach(log::error); + } + + int exitCode = process.waitFor(); + if (exitCode != 0) { + throw new MojoExecutionException("ApprovedFileInventory exited with code " + exitCode); + } + } catch (IOException e) { + throw new MojoExecutionException("Failed to execute ApprovedFileInventory", e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new MojoExecutionException("Interrupted while running ApprovedFileInventory", e); + } + } + + static List buildCommand(List classpathElements, String command) { + String javaExecutable = Path.of(System.getProperty("java.home"), "bin", "java").toString(); + String classpath = String.join(System.getProperty("path.separator"), classpathElements); + + List cmd = new ArrayList<>(); + cmd.add(javaExecutable); + cmd.add("-cp"); + cmd.add(classpath); + cmd.add(MAIN_CLASS); + cmd.add(command); + return cmd; + } +} diff --git a/modules/maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java b/modules/maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java new file mode 100644 index 00000000..67fb259a --- /dev/null +++ b/modules/maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java @@ -0,0 +1,44 @@ +package org.approvej.maven; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.nio.file.Path; +import java.util.List; +import org.junit.jupiter.api.Test; + +class MojoHelperTest { + + @Test + void buildCommand_find() { + var classpathElements = List.of("/lib/a.jar", "/lib/b.jar"); + + var command = MojoHelper.buildCommand(classpathElements, "--find"); + + String expectedJava = Path.of(System.getProperty("java.home"), "bin", "java").toString(); + String expectedClasspath = "/lib/a.jar" + System.getProperty("path.separator") + "/lib/b.jar"; + assertThat(command) + .containsExactly( + expectedJava, + "-cp", + expectedClasspath, + "org.approvej.approve.ApprovedFileInventory", + "--find"); + } + + @Test + void buildCommand_remove() { + var classpathElements = List.of("/lib/a.jar", "/lib/b.jar"); + + var command = MojoHelper.buildCommand(classpathElements, "--remove"); + + String expectedJava = Path.of(System.getProperty("java.home"), "bin", "java").toString(); + String expectedClasspath = "/lib/a.jar" + System.getProperty("path.separator") + "/lib/b.jar"; + assertThat(command) + .containsExactly( + expectedJava, + "-cp", + expectedClasspath, + "org.approvej.approve.ApprovedFileInventory", + "--remove"); + } +} diff --git a/settings.gradle.kts b/settings.gradle.kts index 56caed3d..57b30dbb 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -8,6 +8,8 @@ include("modules:core") include("modules:gradle-plugin") +include("modules:maven-plugin") + include("modules:json-jackson") include("modules:json-jackson3") From cd8f7a5de80792ec9f30c436c2eefcd8e32b2681 Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Thu, 19 Feb 2026 14:28:24 +0100 Subject: [PATCH 07/25] Fix version catalog ordering and CI test failure Move maven-* library entries after junit-* entries in libs.versions.toml to restore alphabetical order. Route CI environment variable lookup through ConfigurationLoader so tests can control it. Previously resolveInventoryEnabled called System.getenv("CI") directly, causing the defaults test to fail in CI where CI=true. --- gradle/libs.versions.toml | 6 +++--- .../org/approvej/configuration/Configuration.java | 2 +- .../configuration/ConfigurationLoader.java | 14 ++++++++++++-- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 7db2c1bd..02f0d61d 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -17,14 +17,14 @@ jackson2-jsr310 = { module = "com.fasterxml.jackson.datatype:jackson-datatype-js jackson3-databind = { module = "tools.jackson.core:jackson-databind", version.ref = "jackson3" } jackson3-dataformat-yaml = { module = "tools.jackson.dataformat:jackson-dataformat-yaml", version.ref = "jackson3" } jspecify = { module = "org.jspecify:jspecify", version = "1.0.0" } -maven-core = { module = "org.apache.maven:maven-core", version.ref = "maven" } -maven-plugin-annotations = { module = "org.apache.maven.plugin-tools:maven-plugin-annotations", version = "3.15.2" } -maven-plugin-api = { module = "org.apache.maven:maven-plugin-api", version.ref = "maven" } junit-bom = { module = "org.junit:junit-bom", version.ref = "junit" } junit-jupiter-api = { module = "org.junit.jupiter:junit-jupiter-api", version.ref = "junit" } junit-jupiter-engine = { module = "org.junit.jupiter:junit-jupiter-engine", version.ref = "junit" } junit-jupiter-params = { module = "org.junit.jupiter:junit-jupiter-params", version.ref = "junit" } junit-platform-launcher = { module = "org.junit.platform:junit-platform-launcher", version = "6.0.3" } +maven-core = { module = "org.apache.maven:maven-core", version.ref = "maven" } +maven-plugin-annotations = { module = "org.apache.maven.plugin-tools:maven-plugin-annotations", version = "3.15.2" } +maven-plugin-api = { module = "org.apache.maven:maven-plugin-api", version.ref = "maven" } spock = { module = "org.spockframework:spock-core", version = "2.4-M7-groovy-5.0" } testng = { module = "org.testng:testng", version = "7.12.0" } wiremock = { module = "org.wiremock:wiremock", version.ref = "wiremock" } diff --git a/modules/core/src/main/java/org/approvej/configuration/Configuration.java b/modules/core/src/main/java/org/approvej/configuration/Configuration.java index c7925393..596b514b 100644 --- a/modules/core/src/main/java/org/approvej/configuration/Configuration.java +++ b/modules/core/src/main/java/org/approvej/configuration/Configuration.java @@ -81,7 +81,7 @@ private static boolean resolveInventoryEnabled(ConfigurationLoader loader) { if (configured != null) { return Boolean.parseBoolean(configured); } - String ci = System.getenv("CI"); + String ci = loader.getenv("CI"); return ci == null || ci.isBlank(); } } diff --git a/modules/core/src/main/java/org/approvej/configuration/ConfigurationLoader.java b/modules/core/src/main/java/org/approvej/configuration/ConfigurationLoader.java index 58609d4b..25531ceb 100644 --- a/modules/core/src/main/java/org/approvej/configuration/ConfigurationLoader.java +++ b/modules/core/src/main/java/org/approvej/configuration/ConfigurationLoader.java @@ -18,9 +18,12 @@ final class ConfigurationLoader { private static final String ENV_PREFIX = "APPROVEJ_"; private final List sources; + private final Function envLookup; - private ConfigurationLoader(List sources) { + private ConfigurationLoader( + List sources, Function envLookup) { this.sources = List.copyOf(sources); + this.envLookup = envLookup; } @Nullable String get(String key) { @@ -33,6 +36,11 @@ private ConfigurationLoader(List sources) { return null; } + /** Looks up a raw environment variable by its exact name (no prefix transformation). */ + @Nullable String getenv(String name) { + return envLookup.apply(name); + } + String get(String key, String defaultValue) { String value = get(key); if (value == null) { @@ -66,12 +74,14 @@ static String toEnvironmentVariableName(String propertyName) { static final class Builder { private final List sources = new ArrayList<>(); + private Function envLookup = key -> null; Builder withEnvironmentVariables() { return withEnvironmentVariables(System::getenv); } Builder withEnvironmentVariables(Function envLookup) { + this.envLookup = envLookup; sources.add(key -> envLookup.apply(toEnvironmentVariableName(key))); return this; } @@ -112,7 +122,7 @@ Builder withUserHomeProperties() { } ConfigurationLoader build() { - return new ConfigurationLoader(sources); + return new ConfigurationLoader(sources, envLookup); } } } From 3e22da221ad171c2f58c25c3369bce570bbade6c Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Thu, 19 Feb 2026 14:53:43 +0100 Subject: [PATCH 08/25] Add ProjectBuilder unit tests for Gradle plugin Add in-process unit tests using ProjectBuilder alongside the existing GradleRunner functional tests. These run in the same JVM so coverage tools can track them, and they verify task registration, group, description, main class, and args. Also tests that plugin order does not matter (Java before or after ApproveJ). --- .../approvej/gradle/ApproveJPluginTest.java | 67 ++++++++++++++++++- 1 file changed, 65 insertions(+), 2 deletions(-) diff --git a/modules/gradle-plugin/src/test/java/org/approvej/gradle/ApproveJPluginTest.java b/modules/gradle-plugin/src/test/java/org/approvej/gradle/ApproveJPluginTest.java index 44da0bd6..dde535c2 100644 --- a/modules/gradle-plugin/src/test/java/org/approvej/gradle/ApproveJPluginTest.java +++ b/modules/gradle-plugin/src/test/java/org/approvej/gradle/ApproveJPluginTest.java @@ -5,6 +5,9 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import org.gradle.api.Project; +import org.gradle.api.tasks.JavaExec; +import org.gradle.testfixtures.ProjectBuilder; import org.gradle.testkit.runner.GradleRunner; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -14,7 +17,67 @@ class ApproveJPluginTest { @TempDir Path tempProjectDir; @Test - void apply() throws IOException { + void apply() { + Project project = ProjectBuilder.builder().build(); + project.getPluginManager().apply("java"); + + project.getPluginManager().apply(ApproveJPlugin.class); + + assertThat(project.getTasks().findByName("approvejFindOrphans")).isNotNull(); + assertThat(project.getTasks().findByName("approvejCleanup")).isNotNull(); + } + + @Test + void apply_without_java_plugin() { + Project project = ProjectBuilder.builder().build(); + + project.getPluginManager().apply(ApproveJPlugin.class); + + assertThat(project.getTasks().findByName("approvejFindOrphans")).isNull(); + assertThat(project.getTasks().findByName("approvejCleanup")).isNull(); + } + + @Test + void apply_java_plugin_after_approvej() { + Project project = ProjectBuilder.builder().build(); + project.getPluginManager().apply(ApproveJPlugin.class); + + project.getPluginManager().apply("java"); + + assertThat(project.getTasks().findByName("approvejFindOrphans")).isNotNull(); + assertThat(project.getTasks().findByName("approvejCleanup")).isNotNull(); + } + + @Test + void approvejFindOrphans_task_configuration() { + Project project = ProjectBuilder.builder().build(); + project.getPluginManager().apply("java"); + project.getPluginManager().apply(ApproveJPlugin.class); + + var task = (JavaExec) project.getTasks().getByName("approvejFindOrphans"); + + assertThat(task.getGroup()).isEqualTo("verification"); + assertThat(task.getDescription()).isEqualTo("List orphaned approved files"); + assertThat(task.getMainClass().get()).isEqualTo("org.approvej.approve.ApprovedFileInventory"); + assertThat(task.getArgs()).containsExactly("--find"); + } + + @Test + void approvejCleanup_task_configuration() { + Project project = ProjectBuilder.builder().build(); + project.getPluginManager().apply("java"); + project.getPluginManager().apply(ApproveJPlugin.class); + + var task = (JavaExec) project.getTasks().getByName("approvejCleanup"); + + assertThat(task.getGroup()).isEqualTo("verification"); + assertThat(task.getDescription()).isEqualTo("Detect and remove orphaned approved files"); + assertThat(task.getMainClass().get()).isEqualTo("org.approvej.approve.ApprovedFileInventory"); + assertThat(task.getArgs()).containsExactly("--remove"); + } + + @Test + void apply_functional() throws IOException { Files.writeString( tempProjectDir.resolve("build.gradle"), """ @@ -37,7 +100,7 @@ void apply() throws IOException { } @Test - void apply_without_java_plugin() throws IOException { + void apply_functional_without_java_plugin() throws IOException { Files.writeString( tempProjectDir.resolve("build.gradle"), """ From e5a1569ecaffe611359382b6fd11a5df0f588543 Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Thu, 19 Feb 2026 15:01:09 +0100 Subject: [PATCH 09/25] Add JaCoCo to Gradle and Maven plugin modules Enable the jacoco plugin and XML report generation so SonarCloud can pick up test coverage for both plugin modules. --- modules/gradle-plugin/build.gradle.kts | 3 +++ modules/maven-plugin/build.gradle.kts | 3 +++ 2 files changed, 6 insertions(+) diff --git a/modules/gradle-plugin/build.gradle.kts b/modules/gradle-plugin/build.gradle.kts index 927e55da..26ca2f49 100644 --- a/modules/gradle-plugin/build.gradle.kts +++ b/modules/gradle-plugin/build.gradle.kts @@ -1,5 +1,6 @@ plugins { `java-gradle-plugin` + jacoco `jvm-test-suite` alias(libs.plugins.plugin.publish) } @@ -25,6 +26,8 @@ testing { } } +tasks.jacocoTestReport { reports { xml.required = true } } + gradlePlugin { website = "https://approvej.org" vcsUrl = "https://github.com/mkutz/approvej" diff --git a/modules/maven-plugin/build.gradle.kts b/modules/maven-plugin/build.gradle.kts index 3cd5d469..f517cddc 100644 --- a/modules/maven-plugin/build.gradle.kts +++ b/modules/maven-plugin/build.gradle.kts @@ -1,5 +1,6 @@ plugins { `java-library` + jacoco `jvm-test-suite` `maven-publish` alias(libs.plugins.maven.plugin.development) @@ -19,6 +20,8 @@ dependencies { implementation(libs.maven.core) } +tasks.jacocoTestReport { reports { xml.required = true } } + testing { suites { val test by From 7cb22d085daba0261aa65f30c62f8c25a0ae1b71 Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Thu, 19 Feb 2026 15:13:34 +0100 Subject: [PATCH 10/25] Improve test coverage for inventory and MojoHelper Enable inventoryEnabled in test properties so the ApprovalBuilder.byFile() inventory recording line is covered in CI where the CI env var would otherwise disable it. Add executeInventory test that exercises the process forking and non-zero exit code path in MojoHelper. --- .../core/src/test/resources/approvej.properties | 1 + .../java/org/approvej/maven/MojoHelperTest.java | 17 +++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/modules/core/src/test/resources/approvej.properties b/modules/core/src/test/resources/approvej.properties index 70f4c044..b9918f0b 100644 --- a/modules/core/src/test/resources/approvej.properties +++ b/modules/core/src/test/resources/approvej.properties @@ -1,2 +1,3 @@ defaultPrintFormat = org.approvej.print.SingleLineStringPrintFormat +inventoryEnabled = true ignored = some value diff --git a/modules/maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java b/modules/maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java index 67fb259a..03d736b2 100644 --- a/modules/maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java +++ b/modules/maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java @@ -1,10 +1,15 @@ package org.approvej.maven; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import java.nio.file.Path; import java.util.List; +import org.apache.maven.plugin.MojoExecutionException; +import org.apache.maven.plugin.logging.SystemStreamLog; +import org.apache.maven.project.MavenProject; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; class MojoHelperTest { @@ -41,4 +46,16 @@ void buildCommand_remove() { "org.approvej.approve.ApprovedFileInventory", "--remove"); } + + @Test + void executeInventory_nonzero_exit_code(@TempDir Path tempDir) { + var project = new MavenProject(); + project.getBuild().setOutputDirectory(tempDir.resolve("classes").toString()); + project.getBuild().setTestOutputDirectory(tempDir.resolve("test-classes").toString()); + project.setFile(tempDir.resolve("pom.xml").toFile()); + + assertThatExceptionOfType(MojoExecutionException.class) + .isThrownBy(() -> MojoHelper.executeInventory(project, "--find", new SystemStreamLog())) + .withMessageContaining("exited with code"); + } } From d2104e13b8e43e194a59d494d356888b3155329f Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Thu, 19 Feb 2026 15:26:13 +0100 Subject: [PATCH 11/25] Fix SonarCloud issues in inventory code Rename record() to registerApprovedFile() to avoid restricted identifier. Join assertion chains in ApprovedFileInventoryTest where multiple assertions share the same subject. --- .../main/java/org/approvej/ApprovalBuilder.java | 2 +- .../approvej/approve/ApprovedFileInventory.java | 2 +- .../approve/ApprovedFileInventoryTest.java | 15 ++++++++------- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/modules/core/src/main/java/org/approvej/ApprovalBuilder.java b/modules/core/src/main/java/org/approvej/ApprovalBuilder.java index 2729c27a..e40eb905 100644 --- a/modules/core/src/main/java/org/approvej/ApprovalBuilder.java +++ b/modules/core/src/main/java/org/approvej/ApprovalBuilder.java @@ -203,7 +203,7 @@ public void byFile(PathProvider pathProvider) { PathProvider updatedPathProvider = pathProvider.filenameAffix(name).filenameExtension(filenameExtension); if (configuration.inventoryEnabled()) { - ApprovedFileInventory.record(updatedPathProvider); + ApprovedFileInventory.registerApprovedFile(updatedPathProvider); } if (!(value instanceof String)) { printed().byFile(updatedPathProvider); diff --git a/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java index 2c5bc07c..a5624cc0 100644 --- a/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java +++ b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java @@ -43,7 +43,7 @@ private ApprovedFileInventory() {} * * @param pathProvider the path provider for the approved file */ - public static void record(PathProvider pathProvider) { + public static void registerApprovedFile(PathProvider pathProvider) { TestMethod testMethod; try { testMethod = StackTraceTestFinderUtil.currentTestMethod(); diff --git a/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java b/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java index c3de94b8..d64f7be4 100644 --- a/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java +++ b/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java @@ -31,7 +31,7 @@ void tearDown() { } @Test - void record() { + void registerApprovedFile() { ApprovedFileInventory.addEntry( "src/test/MyTest-myTest-approved.txt", "com.example.MyTest#myTest"); @@ -64,8 +64,8 @@ void record_named_approvals() { ApprovedFileInventory.writeInventory(); TreeMap inventory = ApprovedFileInventory.loadInventory(); - assertThat(inventory).hasSize(2); assertThat(inventory) + .hasSize(2) .containsEntry("src/test/MyTest-myTest-alpha-approved.txt", "com.example.MyTest#myTest") .containsEntry("src/test/MyTest-myTest-beta-approved.txt", "com.example.MyTest#myTest"); } @@ -89,8 +89,8 @@ void record_replaces_entries_for_executed_methods() throws IOException { ApprovedFileInventory.writeInventory(); TreeMap inventory = ApprovedFileInventory.loadInventory(); - assertThat(inventory).hasSize(2); assertThat(inventory) + .hasSize(2) .containsEntry("src/test/MyTest-myTest-gamma-approved.txt", "com.example.MyTest#myTest") .containsEntry("src/test/MyTest-myTest-beta-approved.txt", "com.example.MyTest#myTest") .doesNotContainKey("src/test/MyTest-myTest-alpha-approved.txt"); @@ -112,8 +112,8 @@ void record_preserves_entries_for_unexecuted_methods() throws IOException { ApprovedFileInventory.writeInventory(); TreeMap inventory = ApprovedFileInventory.loadInventory(); - assertThat(inventory).hasSize(2); assertThat(inventory) + .hasSize(2) .containsEntry("src/test/OtherTest-other-approved.txt", "com.example.OtherTest#other") .containsEntry("src/test/MyTest-myTest-approved.txt", "com.example.MyTest#myTest"); } @@ -134,8 +134,8 @@ void record_merges_with_existing() throws IOException { ApprovedFileInventory.writeInventory(); TreeMap inventory = ApprovedFileInventory.loadInventory(); - assertThat(inventory).hasSize(2); assertThat(inventory) + .hasSize(2) .containsEntry( "src/test/ExistingTest-existing-approved.txt", "com.example.ExistingTest#existing") .containsEntry("src/test/NewTest-newTest-approved.txt", "com.example.NewTest#newTest"); @@ -198,7 +198,8 @@ void removeOrphans() throws IOException { assertThat(removed).hasSize(1); assertThat(orphanFile).doesNotExist(); TreeMap inventory = ApprovedFileInventory.loadInventory(); - assertThat(inventory).doesNotContainKey(orphanFile.toString()); - assertThat(inventory).containsKey(validFile.toString()); + assertThat(inventory) + .doesNotContainKey(orphanFile.toString()) + .containsKey(validFile.toString()); } } From edb9c838c3075c27e8eb31b54f784e2bc9c336c6 Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Thu, 19 Feb 2026 15:39:44 +0100 Subject: [PATCH 12/25] Move plugin modules to plugins/ directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Separate build tool plugins from library modules by moving them from modules/ to a dedicated plugins/ directory: - modules/gradle-plugin/ → plugins/gradle/ - modules/maven-plugin/ → plugins/maven/ Update all references in settings.gradle.kts, CI workflows, bom/build.gradle.kts, CLAUDE.md, and CONTRIBUTING.md. --- .github/workflows/build-main.yml | 4 +++- .github/workflows/build-pr.yml | 4 +++- .github/workflows/publish.yml | 2 +- CLAUDE.md | 2 ++ CONTRIBUTING.md | 7 +++++-- bom/build.gradle.kts | 4 +--- {modules/gradle-plugin => plugins/gradle}/build.gradle.kts | 0 .../src/main/java/org/approvej/gradle/ApproveJPlugin.java | 0 .../test/java/org/approvej/gradle/ApproveJPluginTest.java | 0 {modules/maven-plugin => plugins/maven}/build.gradle.kts | 0 {modules/maven-plugin => plugins/maven}/gradle.properties | 0 .../src/main/java/org/approvej/maven/CleanupMojo.java | 0 .../src/main/java/org/approvej/maven/FindOrphansMojo.java | 0 .../src/main/java/org/approvej/maven/MojoHelper.java | 0 .../src/test/java/org/approvej/maven/MojoHelperTest.java | 0 settings.gradle.kts | 4 ++-- 16 files changed, 17 insertions(+), 10 deletions(-) rename {modules/gradle-plugin => plugins/gradle}/build.gradle.kts (100%) rename {modules/gradle-plugin => plugins/gradle}/src/main/java/org/approvej/gradle/ApproveJPlugin.java (100%) rename {modules/gradle-plugin => plugins/gradle}/src/test/java/org/approvej/gradle/ApproveJPluginTest.java (100%) rename {modules/maven-plugin => plugins/maven}/build.gradle.kts (100%) rename {modules/maven-plugin => plugins/maven}/gradle.properties (100%) rename {modules/maven-plugin => plugins/maven}/src/main/java/org/approvej/maven/CleanupMojo.java (100%) rename {modules/maven-plugin => plugins/maven}/src/main/java/org/approvej/maven/FindOrphansMojo.java (100%) rename {modules/maven-plugin => plugins/maven}/src/main/java/org/approvej/maven/MojoHelper.java (100%) rename {modules/maven-plugin => plugins/maven}/src/test/java/org/approvej/maven/MojoHelperTest.java (100%) diff --git a/.github/workflows/build-main.yml b/.github/workflows/build-main.yml index 48317e83..1aedb678 100644 --- a/.github/workflows/build-main.yml +++ b/.github/workflows/build-main.yml @@ -46,4 +46,6 @@ jobs: if: failure() with: name: modules-test-reports - path: modules/**/build/reports/tests/test/ + path: | + modules/**/build/reports/tests/test/ + plugins/**/build/reports/tests/test/ diff --git a/.github/workflows/build-pr.yml b/.github/workflows/build-pr.yml index f252ba3e..0fbf6d7e 100644 --- a/.github/workflows/build-pr.yml +++ b/.github/workflows/build-pr.yml @@ -57,7 +57,9 @@ jobs: if: failure() with: name: modules-test-reports - path: modules/**/build/reports/tests/test/ + path: | + modules/**/build/reports/tests/test/ + plugins/**/build/reports/tests/test/ - name: Enable auto merge for Dependabot PRs run: | diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 1e6a3a5e..5ba744e4 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -40,7 +40,7 @@ jobs: - run: ./gradlew jreleaserConfig -Pversion=${{ github.event.inputs.version }} --git-root-search - run: ./gradlew publish -Pversion=${{ github.event.inputs.version }} - - run: ./gradlew :modules:gradle-plugin:publishPlugins -Pversion=${{ github.event.inputs.version }} + - run: ./gradlew :plugins:gradle:publishPlugins -Pversion=${{ github.event.inputs.version }} env: GRADLE_PUBLISH_KEY: ${{ secrets.GRADLE_PUBLISH_KEY }} GRADLE_PUBLISH_SECRET: ${{ secrets.GRADLE_PUBLISH_SECRET }} diff --git a/CLAUDE.md b/CLAUDE.md index 7ce9701c..b9198218 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -41,6 +41,8 @@ It provides a fluent API to compare actual values against previously approved "g - **modules/yaml-jackson3** - YAML support using Jackson 3.x - **modules/http** - HTTP stub server for approving HTTP requests - **modules/http-wiremock** - WireMock adapter for HTTP testing +- **plugins/gradle** - Gradle plugin for managing approved files +- **plugins/maven** - Maven plugin for managing approved files - **bom** - Maven Bill of Materials - **manual** - AsciiDoc documentation (code samples included from tests in `manual/src/test`) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9e9ab15c..15ee58ed 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -110,13 +110,16 @@ E.g. `1.0.0` is written as `1.0`, `1.0.1` is written as `1.0.1`, and `1.1.0` is ### Project Structure -It is structured in four main modules: +It is structured in four main directories: -- The [modules](modules) directory contains all the published code modules: +- The [modules](modules) directory contains all the published library modules: - [core](modules/core) contains the code for the core framework and should not have any dependencies to other modules and only very few (if any) to external libraries, - [json-jackson](modules/json-jackson) contains JSON-related code using Jackson, - [yaml-jackson](modules/yaml-jackson) contains YAML-related code using Jackson - [http](modules/http) contains code to create an HTTP server for approving requests +- The [plugins](plugins) directory contains build tool plugins: + - [gradle](plugins/gradle) contains the Gradle plugin for managing approved files + - [maven](plugins/maven) contains the Maven plugin for managing approved files - the [bom](bom) directory contains the build file to generate a [Maven Bill of Material (BOM)](https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#bill-of-materials-bom-poms) for all the ApproveJ modules, and - the [manual](manual) directory contains the projects documentation written in [AsciiDoc](https://docs.asciidoctor.org/asciidoc/latest/). diff --git a/bom/build.gradle.kts b/bom/build.gradle.kts index 1d2660bd..3acf28fb 100644 --- a/bom/build.gradle.kts +++ b/bom/build.gradle.kts @@ -9,9 +9,7 @@ dependencies { constraints { rootProject.subprojects .filter { - it != project && - it.name !in listOf("gradle-plugin", "maven-plugin", "manual") && - it.subprojects.isEmpty() + it != project && it.name !in listOf("gradle", "maven", "manual") && it.subprojects.isEmpty() } .sortedBy { it.name } .forEach { api(it) } diff --git a/modules/gradle-plugin/build.gradle.kts b/plugins/gradle/build.gradle.kts similarity index 100% rename from modules/gradle-plugin/build.gradle.kts rename to plugins/gradle/build.gradle.kts diff --git a/modules/gradle-plugin/src/main/java/org/approvej/gradle/ApproveJPlugin.java b/plugins/gradle/src/main/java/org/approvej/gradle/ApproveJPlugin.java similarity index 100% rename from modules/gradle-plugin/src/main/java/org/approvej/gradle/ApproveJPlugin.java rename to plugins/gradle/src/main/java/org/approvej/gradle/ApproveJPlugin.java diff --git a/modules/gradle-plugin/src/test/java/org/approvej/gradle/ApproveJPluginTest.java b/plugins/gradle/src/test/java/org/approvej/gradle/ApproveJPluginTest.java similarity index 100% rename from modules/gradle-plugin/src/test/java/org/approvej/gradle/ApproveJPluginTest.java rename to plugins/gradle/src/test/java/org/approvej/gradle/ApproveJPluginTest.java diff --git a/modules/maven-plugin/build.gradle.kts b/plugins/maven/build.gradle.kts similarity index 100% rename from modules/maven-plugin/build.gradle.kts rename to plugins/maven/build.gradle.kts diff --git a/modules/maven-plugin/gradle.properties b/plugins/maven/gradle.properties similarity index 100% rename from modules/maven-plugin/gradle.properties rename to plugins/maven/gradle.properties diff --git a/modules/maven-plugin/src/main/java/org/approvej/maven/CleanupMojo.java b/plugins/maven/src/main/java/org/approvej/maven/CleanupMojo.java similarity index 100% rename from modules/maven-plugin/src/main/java/org/approvej/maven/CleanupMojo.java rename to plugins/maven/src/main/java/org/approvej/maven/CleanupMojo.java diff --git a/modules/maven-plugin/src/main/java/org/approvej/maven/FindOrphansMojo.java b/plugins/maven/src/main/java/org/approvej/maven/FindOrphansMojo.java similarity index 100% rename from modules/maven-plugin/src/main/java/org/approvej/maven/FindOrphansMojo.java rename to plugins/maven/src/main/java/org/approvej/maven/FindOrphansMojo.java diff --git a/modules/maven-plugin/src/main/java/org/approvej/maven/MojoHelper.java b/plugins/maven/src/main/java/org/approvej/maven/MojoHelper.java similarity index 100% rename from modules/maven-plugin/src/main/java/org/approvej/maven/MojoHelper.java rename to plugins/maven/src/main/java/org/approvej/maven/MojoHelper.java diff --git a/modules/maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java b/plugins/maven/src/test/java/org/approvej/maven/MojoHelperTest.java similarity index 100% rename from modules/maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java rename to plugins/maven/src/test/java/org/approvej/maven/MojoHelperTest.java diff --git a/settings.gradle.kts b/settings.gradle.kts index 57b30dbb..06960f66 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -6,9 +6,9 @@ include("bom") include("modules:core") -include("modules:gradle-plugin") +include("plugins:gradle") -include("modules:maven-plugin") +include("plugins:maven") include("modules:json-jackson") From ad3835cd447284c43e93e6b7f65a0b8a3ccdc32f Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Thu, 19 Feb 2026 16:08:14 +0100 Subject: [PATCH 13/25] Use %n instead of \n --- .../main/java/org/approvej/approve/ApprovedFileInventory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java index a5624cc0..aa52a8ee 100644 --- a/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java +++ b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java @@ -150,7 +150,7 @@ private static void saveInventory(TreeMap inventory) { } else { Files.createDirectories(inventoryFile.getParent()); String content = - "%s\n%s" + "%s%n%s" .formatted( HEADER, inventory.entrySet().stream() From 371dca7ce38a6d8cae437d890f2c835274d91fda Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Thu, 19 Feb 2026 17:01:11 +0100 Subject: [PATCH 14/25] Move test reference parsing into InventoryEntry InventoryEntry now validates the className#methodName format in its constructor and stores className and methodName as record components, replacing the inline parsing in ApprovedFileInventory. --- .../approve/ApprovedFileInventory.java | 11 ++------- .../org/approvej/approve/InventoryEntry.java | 24 ++++++++++++++++++- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java index aa52a8ee..7bdc6c53 100644 --- a/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java +++ b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java @@ -85,16 +85,9 @@ static List findOrphans() { } private static boolean isOrphan(InventoryEntry entry) { - String testReference = entry.testReference(); - int hashIndex = testReference.indexOf('#'); - if (hashIndex < 0) { - return true; - } - String className = testReference.substring(0, hashIndex); - String methodName = testReference.substring(hashIndex + 1); try { - return stream(Class.forName(className).getDeclaredMethods()) - .noneMatch(method -> method.getName().equals(methodName)); + return stream(Class.forName(entry.className()).getDeclaredMethods()) + .noneMatch(method -> method.getName().equals(entry.methodName())); } catch (ClassNotFoundException e) { return true; } diff --git a/modules/core/src/main/java/org/approvej/approve/InventoryEntry.java b/modules/core/src/main/java/org/approvej/approve/InventoryEntry.java index cc81c270..27f6bc0b 100644 --- a/modules/core/src/main/java/org/approvej/approve/InventoryEntry.java +++ b/modules/core/src/main/java/org/approvej/approve/InventoryEntry.java @@ -4,4 +4,26 @@ /** An entry in the approved file inventory, mapping a file path to its originating test method. */ @NullMarked -record InventoryEntry(String relativePath, String testReference) {} +record InventoryEntry(String relativePath, String className, String methodName) { + + InventoryEntry(String relativePath, String testReference) { + this(relativePath, parseClassName(testReference), parseMethodName(testReference)); + } + + String testReference() { + return className + "#" + methodName; + } + + private static String parseClassName(String testReference) { + int hashIndex = testReference.indexOf('#'); + if (hashIndex < 0) { + throw new IllegalArgumentException( + "Invalid test reference (expected 'className#methodName'): " + testReference); + } + return testReference.substring(0, hashIndex); + } + + private static String parseMethodName(String testReference) { + return testReference.substring(testReference.indexOf('#') + 1); + } +} From 5ec74ed7c0b81d496cb66e214b02eebcef66933d Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Thu, 19 Feb 2026 21:48:32 +0100 Subject: [PATCH 15/25] Add documentation for inventory and cleanup plugins --- .../docs/asciidoc/chapters/10-cleanup.adoc | 151 ++++++++++++++++++ ...nfiguration.adoc => 11-configuration.adoc} | 8 + ...1-cheat-sheet.adoc => 12-cheat-sheet.adoc} | 24 +++ manual/src/docs/asciidoc/index.adoc | 5 +- 4 files changed, 186 insertions(+), 2 deletions(-) create mode 100644 manual/src/docs/asciidoc/chapters/10-cleanup.adoc rename manual/src/docs/asciidoc/chapters/{10-configuration.adoc => 11-configuration.adoc} (96%) rename manual/src/docs/asciidoc/chapters/{11-cheat-sheet.adoc => 12-cheat-sheet.adoc} (93%) diff --git a/manual/src/docs/asciidoc/chapters/10-cleanup.adoc b/manual/src/docs/asciidoc/chapters/10-cleanup.adoc new file mode 100644 index 00000000..f13abab2 --- /dev/null +++ b/manual/src/docs/asciidoc/chapters/10-cleanup.adoc @@ -0,0 +1,151 @@ +[id=cleanup] += Cleaning Up Orphaned Approved Files + + +[id="cleanup_problem"] +== The Problem + +When you rename or delete a test method, its approved file stays behind on disk. +Over time these orphaned files accumulate and clutter the repository. + +ApproveJ provides an inventory-based mechanism to detect and remove orphaned approved files automatically. + + +[id="cleanup_inventory"] +== The Approved File Inventory + +Every time a test calls `byFile()`, ApproveJ records the approved file path and the originating test method. +At the end of the test run, all entries are merged into a project-level inventory file: + +---- +.approvej/inventory.properties +---- + +The file uses Java Properties format. +Each entry maps a relative file path to the test reference that created it: + +[source,properties] +---- +src/test/resources/MyTest-myTest-approved.txt = com.example.MyTest#myTest +---- + +TIP: Commit `.approvej/inventory.properties` to version control so that the inventory is shared across the team. + +The inventory is updated incrementally. +Only entries for test methods that ran in the current execution are refreshed. +Entries for tests that did not run are preserved. + + +[id="cleanup_configuration"] +== Enabling the Inventory + +The inventory is controlled by the `inventoryEnabled` property. + +|=== +|Environment |Default + +|Local development (no `CI` environment variable) +|`true` + +|CI (the `CI` environment variable is set) +|`false` +|=== + +You can override this default in any <>: + +.`src/test/resources/approvej.properties` +[source,properties] +---- +inventoryEnabled = true +---- + +Or via environment variable: + +[source,bash] +---- +export APPROVEJ_INVENTORY_ENABLED=false +---- + + +[id="cleanup_finding_orphans"] +== Finding and Removing Orphans + +After running your tests with the inventory enabled, use the build plugin to detect orphans. +An approved file is considered orphaned when its originating test method no longer exists in the compiled test classes. + + +[id="cleanup_gradle"] +=== Gradle Plugin + +Apply the plugin in your build file: + +.Gradle +[source,groovy,subs=attributes+,role="primary"] +---- +plugins { + id 'org.approvej' version '{revnumber}' +} +---- +.Gradle.kts +[source,kotlin,subs=attributes+,role="secondary"] +---- +plugins { + id("org.approvej") version "{revnumber}" +} +---- + +The plugin registers two tasks in the `verification` group: + +[cols="1,2"] +|=== +|Task |Description + +|`./gradlew approvejFindOrphans` +|Lists orphaned approved files without deleting them + +|`./gradlew approvejCleanup` +|Detects and removes orphaned approved files +|=== + +NOTE: Both tasks require the Java plugin to be applied in the same project. +They use the test runtime classpath to resolve test classes. + + +[id="cleanup_maven"] +=== Maven Plugin + +Add the plugin to your `pom.xml`: + +[source,xml,subs=attributes+] +---- + + org.approvej + approvej-maven-plugin + {revnumber} + +---- + +The plugin provides two goals: + +[cols="1,2"] +|=== +|Goal |Description + +|`mvn approvej:find-orphans` +|Lists orphaned approved files without deleting them + +|`mvn approvej:cleanup` +|Detects and removes orphaned approved files +|=== + + +[id="cleanup_workflow"] +== Typical Workflow + +1. Run your tests locally so the inventory is populated. +2. Run the find-orphans task/goal to see which approved files are orphaned. +3. Run the cleanup task/goal to delete the orphaned files. +4. Commit the updated inventory and the removal of orphaned files. + +WARNING: Only approved files that have been recorded in the inventory can be detected as orphans. +Make sure you have run all relevant tests with the inventory enabled at least once before cleaning up. diff --git a/manual/src/docs/asciidoc/chapters/10-configuration.adoc b/manual/src/docs/asciidoc/chapters/11-configuration.adoc similarity index 96% rename from manual/src/docs/asciidoc/chapters/10-configuration.adoc rename to manual/src/docs/asciidoc/chapters/11-configuration.adoc index fcc30a5c..2a20d7f9 100644 --- a/manual/src/docs/asciidoc/chapters/10-configuration.adoc +++ b/manual/src/docs/asciidoc/chapters/11-configuration.adoc @@ -45,6 +45,11 @@ Can be an alias (e.g., `none`, `automatic`), a fully-qualified class name, or a |The script to execute for reviewing differences. This implicitly sets the `defaultFileReviewer` to script |_(none)_ + +|`inventoryEnabled` +|Whether the approved file inventory is enabled. +When enabled, ApproveJ records approved file paths in `.approvej/inventory.properties` during test runs. +|`true` locally, `false` in CI |=== @@ -121,6 +126,9 @@ Environment variables use the `APPROVEJ_` prefix and convert camelCase property |`defaultFileReviewer` |`APPROVEJ_DEFAULT_FILE_REVIEWER` + +|`inventoryEnabled` +|`APPROVEJ_INVENTORY_ENABLED` |=== .Example: Set default print format via environment variable diff --git a/manual/src/docs/asciidoc/chapters/11-cheat-sheet.adoc b/manual/src/docs/asciidoc/chapters/12-cheat-sheet.adoc similarity index 93% rename from manual/src/docs/asciidoc/chapters/11-cheat-sheet.adoc rename to manual/src/docs/asciidoc/chapters/12-cheat-sheet.adoc index e7e336c9..6ae95fc8 100644 --- a/manual/src/docs/asciidoc/chapters/11-cheat-sheet.adoc +++ b/manual/src/docs/asciidoc/chapters/12-cheat-sheet.adoc @@ -277,6 +277,26 @@ endif::[] |=== +== Cleanup Tasks + +[cols="2,3"] +|=== +|Command |Description + +|`./gradlew approvejFindOrphans` +|List orphaned approved files (Gradle) + +|`./gradlew approvejCleanup` +|Remove orphaned approved files (Gradle) + +|`mvn approvej:find-orphans` +|List orphaned approved files (Maven) + +|`mvn approvej:cleanup` +|Remove orphaned approved files (Maven) +|=== + + == Configuration Properties [cols="2,2,1"] @@ -294,6 +314,10 @@ endif::[] |`defaultFileReviewerScript` |`APPROVEJ_DEFAULT_FILE_REVIEWER_SCRIPT` |_(none)_ + +|`inventoryEnabled` +|`APPROVEJ_INVENTORY_ENABLED` +|`true` locally, `false` in CI |=== Configuration is resolved in priority order: environment variables > project properties (`src/test/resources/approvej.properties`) > user home properties (`~/.config/approvej/approvej.properties`) > defaults. diff --git a/manual/src/docs/asciidoc/index.adoc b/manual/src/docs/asciidoc/index.adoc index e988a841..bb36b2a9 100644 --- a/manual/src/docs/asciidoc/index.adoc +++ b/manual/src/docs/asciidoc/index.adoc @@ -21,5 +21,6 @@ include::chapters/06-reviewing.adoc[leveloffset=1] include::chapters/07-json-jackson.adoc[leveloffset=1] include::chapters/08-yaml-jackson.adoc[leveloffset=1] include::chapters/09-http.adoc[leveloffset=1] -include::chapters/10-configuration.adoc[leveloffset=1] -include::chapters/11-cheat-sheet.adoc[leveloffset=1] +include::chapters/10-cleanup.adoc[leveloffset=1] +include::chapters/11-configuration.adoc[leveloffset=1] +include::chapters/12-cheat-sheet.adoc[leveloffset=1] From 8cc5f53680a7e86e89b63f5c71b52f25cd02c274 Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Thu, 19 Feb 2026 22:10:11 +0100 Subject: [PATCH 16/25] Fix plugin publishing configuration Exclude Gradle plugin from Maven Central deployment since it is published to the Gradle Plugin Portal only. Set Maven plugin artifact ID to approvej-maven-plugin via gradle.properties to match Maven's plugin naming convention. --- build.gradle.kts | 8 ++++++-- plugins/maven/gradle.properties | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 1175036e..a4be5b99 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -10,11 +10,12 @@ repositories { mavenCentral() } subprojects { afterEvaluate { - if (plugins.hasPlugin("maven-publish")) { + if (plugins.hasPlugin("maven-publish") && !plugins.hasPlugin("com.gradle.plugin-publish")) { publishing { publications { create(name) { from(components.findByName("java") ?: components.getByName("javaPlatform")) + project.properties["mavenPomArtifactId"]?.let { artifactId = "$it" } pom { project.properties["mavenPomName"]?.let { name = "$it" } project.properties["mavenPomDescription"]?.let { description = "$it" } @@ -78,7 +79,10 @@ gradle.projectsEvaluated { mavenCentral { named("sonatype") { subprojects - .filter { it.plugins.hasPlugin("maven-publish") } + .filter { + it.plugins.hasPlugin("maven-publish") && + !it.plugins.hasPlugin("com.gradle.plugin-publish") + } .sortedBy { it.name } .forEach { stagingRepository("${it.projectDir.relativeTo(rootDir)}/build/staging-deploy") diff --git a/plugins/maven/gradle.properties b/plugins/maven/gradle.properties index 8a455e35..5f2ddbe6 100644 --- a/plugins/maven/gradle.properties +++ b/plugins/maven/gradle.properties @@ -1,2 +1,3 @@ +mavenPomArtifactId = approvej-maven-plugin mavenPomName = ApproveJ Maven Plugin mavenPomDescription = Maven plugin to find and remove orphaned approved files From 932abdabb821990b6cda9b3b177a33c746f98204 Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Fri, 20 Feb 2026 10:55:30 +0100 Subject: [PATCH 17/25] Address review feedback Name shutdown hook thread for easier debugging. Only remove inventory entry when file deletion succeeds. Read process stdout and stderr concurrently to prevent potential deadlock. Add test for inventory disabled in CI environments. --- .../approve/ApprovedFileInventory.java | 6 ++-- .../configuration/ConfigurationTest.java | 11 +++++++ .../java/org/approvej/maven/MojoHelper.java | 32 ++++++++++++++----- 3 files changed, 39 insertions(+), 10 deletions(-) diff --git a/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java index 7bdc6c53..b860abf8 100644 --- a/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java +++ b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java @@ -59,7 +59,9 @@ public static void registerApprovedFile(PathProvider pathProvider) { addEntry(relativePath, testReference); if (shutdownHookRegistered.compareAndSet(false, true)) { - Runtime.getRuntime().addShutdownHook(new Thread(ApprovedFileInventory::writeInventory)); + Runtime.getRuntime() + .addShutdownHook( + new Thread(ApprovedFileInventory::writeInventory, "ApproveJ-Inventory-Writer")); } } @@ -109,10 +111,10 @@ static List removeOrphans() { orphan -> { try { Files.deleteIfExists(Path.of(orphan.relativePath())); + inventory.remove(orphan.relativePath()); } catch (IOException e) { System.err.printf("Failed to delete orphaned file: %s%n", orphan.relativePath()); } - inventory.remove(orphan.relativePath()); }); saveInventory(inventory); diff --git a/modules/core/src/test/java/org/approvej/configuration/ConfigurationTest.java b/modules/core/src/test/java/org/approvej/configuration/ConfigurationTest.java index cbed618f..3075ee3f 100644 --- a/modules/core/src/test/java/org/approvej/configuration/ConfigurationTest.java +++ b/modules/core/src/test/java/org/approvej/configuration/ConfigurationTest.java @@ -207,6 +207,17 @@ void loadConfiguration_inventoryEnabled_defaults_to_true() { assertThat(config.inventoryEnabled()).isTrue(); } + @Test + void loadConfiguration_inventoryEnabled_defaults_to_false_in_ci() { + Map env = Map.of("CI", "true"); + ConfigurationLoader loader = + ConfigurationLoader.builder().withEnvironmentVariables(env::get).build(); + + Configuration config = Configuration.loadConfiguration(loader); + + assertThat(config.inventoryEnabled()).isFalse(); + } + @Test void loadConfiguration_inventoryEnabled_from_properties() { Properties props = new Properties(); diff --git a/plugins/maven/src/main/java/org/approvej/maven/MojoHelper.java b/plugins/maven/src/main/java/org/approvej/maven/MojoHelper.java index 07638aa3..87ef3df2 100644 --- a/plugins/maven/src/main/java/org/approvej/maven/MojoHelper.java +++ b/plugins/maven/src/main/java/org/approvej/maven/MojoHelper.java @@ -35,16 +35,32 @@ static void executeInventory(MavenProject project, String command, Log log) Process process = processBuilder.start(); - try (BufferedReader stdout = - new BufferedReader(new InputStreamReader(process.getInputStream()))) { - stdout.lines().forEach(log::info); - } - try (BufferedReader stderr = - new BufferedReader(new InputStreamReader(process.getErrorStream()))) { - stderr.lines().forEach(log::error); - } + Thread stdoutThread = + new Thread( + () -> { + try (BufferedReader stdout = + new BufferedReader(new InputStreamReader(process.getInputStream()))) { + stdout.lines().forEach(log::info); + } catch (IOException e) { + log.error("Error reading stdout from ApprovedFileInventory", e); + } + }); + Thread stderrThread = + new Thread( + () -> { + try (BufferedReader stderr = + new BufferedReader(new InputStreamReader(process.getErrorStream()))) { + stderr.lines().forEach(log::error); + } catch (IOException e) { + log.error("Error reading stderr from ApprovedFileInventory", e); + } + }); + stdoutThread.start(); + stderrThread.start(); int exitCode = process.waitFor(); + stdoutThread.join(); + stderrThread.join(); if (exitCode != 0) { throw new MojoExecutionException("ApprovedFileInventory exited with code " + exitCode); } From 911f45e0684fea66bf7c5c077f0a3eb6e22ff2fd Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Fri, 20 Feb 2026 12:22:54 +0100 Subject: [PATCH 18/25] Rename plugins, use leftover terminology, add clickable URIs Rename plugin directories from plugins/gradle and plugins/maven to plugins/approvej-gradle-plugin and plugins/approvej-maven-plugin so the Gradle project names produce correct Maven artifact IDs. Exclude the Gradle plugin from Maven Central publishing since it is published to the Gradle Plugin Portal separately. Replace "orphan" terminology with "leftover" throughout code, tests, plugin tasks/goals, and documentation for sensitivity. Print file:// URIs in CLI output so paths are clickable in IDEs. --- .github/workflows/publish.yml | 2 +- CLAUDE.md | 4 +- bom/build.gradle.kts | 4 +- build.gradle.kts | 1 - .../docs/asciidoc/chapters/10-cleanup.adoc | 34 +++++----- .../asciidoc/chapters/12-cheat-sheet.adoc | 12 ++-- .../approve/ApprovedFileInventory.java | 63 ++++++++++--------- .../approve/ApprovedFileInventoryTest.java | 34 +++++----- .../build.gradle.kts | 2 +- .../org/approvej/gradle/ApproveJPlugin.java | 8 +-- .../approvej/gradle/ApproveJPluginTest.java | 20 +++--- .../build.gradle.kts | 0 .../approvej-maven-plugin/gradle.properties | 2 + .../java/org/approvej/maven/CleanupMojo.java | 2 +- .../approvej/maven/FindLeftoversMojo.java} | 9 ++- .../java/org/approvej/maven/MojoHelper.java | 0 .../org/approvej/maven/MojoHelperTest.java | 0 plugins/maven/gradle.properties | 3 - settings.gradle.kts | 4 +- 19 files changed, 104 insertions(+), 100 deletions(-) rename plugins/{gradle => approvej-gradle-plugin}/build.gradle.kts (94%) rename plugins/{gradle => approvej-gradle-plugin}/src/main/java/org/approvej/gradle/ApproveJPlugin.java (85%) rename plugins/{gradle => approvej-gradle-plugin}/src/test/java/org/approvej/gradle/ApproveJPluginTest.java (84%) rename plugins/{maven => approvej-maven-plugin}/build.gradle.kts (100%) create mode 100644 plugins/approvej-maven-plugin/gradle.properties rename plugins/{maven => approvej-maven-plugin}/src/main/java/org/approvej/maven/CleanupMojo.java (92%) rename plugins/{maven/src/main/java/org/approvej/maven/FindOrphansMojo.java => approvej-maven-plugin/src/main/java/org/approvej/maven/FindLeftoversMojo.java} (72%) rename plugins/{maven => approvej-maven-plugin}/src/main/java/org/approvej/maven/MojoHelper.java (100%) rename plugins/{maven => approvej-maven-plugin}/src/test/java/org/approvej/maven/MojoHelperTest.java (100%) delete mode 100644 plugins/maven/gradle.properties diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 5ba744e4..76263a99 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -40,7 +40,7 @@ jobs: - run: ./gradlew jreleaserConfig -Pversion=${{ github.event.inputs.version }} --git-root-search - run: ./gradlew publish -Pversion=${{ github.event.inputs.version }} - - run: ./gradlew :plugins:gradle:publishPlugins -Pversion=${{ github.event.inputs.version }} + - run: ./gradlew :plugins:approvej-gradle-plugin:publishPlugins -Pversion=${{ github.event.inputs.version }} env: GRADLE_PUBLISH_KEY: ${{ secrets.GRADLE_PUBLISH_KEY }} GRADLE_PUBLISH_SECRET: ${{ secrets.GRADLE_PUBLISH_SECRET }} diff --git a/CLAUDE.md b/CLAUDE.md index b9198218..f87fc108 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -41,8 +41,8 @@ It provides a fluent API to compare actual values against previously approved "g - **modules/yaml-jackson3** - YAML support using Jackson 3.x - **modules/http** - HTTP stub server for approving HTTP requests - **modules/http-wiremock** - WireMock adapter for HTTP testing -- **plugins/gradle** - Gradle plugin for managing approved files -- **plugins/maven** - Maven plugin for managing approved files +- **plugins/approvej-gradle-plugin** - Gradle plugin for managing approved files +- **plugins/approvej-maven-plugin** - Maven plugin for managing approved files - **bom** - Maven Bill of Materials - **manual** - AsciiDoc documentation (code samples included from tests in `manual/src/test`) diff --git a/bom/build.gradle.kts b/bom/build.gradle.kts index 3acf28fb..c11bb4b6 100644 --- a/bom/build.gradle.kts +++ b/bom/build.gradle.kts @@ -9,7 +9,9 @@ dependencies { constraints { rootProject.subprojects .filter { - it != project && it.name !in listOf("gradle", "maven", "manual") && it.subprojects.isEmpty() + it != project && + it.name !in listOf("approvej-gradle-plugin", "approvej-maven-plugin", "manual") && + it.subprojects.isEmpty() } .sortedBy { it.name } .forEach { api(it) } diff --git a/build.gradle.kts b/build.gradle.kts index a4be5b99..84453c47 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -15,7 +15,6 @@ subprojects { publications { create(name) { from(components.findByName("java") ?: components.getByName("javaPlatform")) - project.properties["mavenPomArtifactId"]?.let { artifactId = "$it" } pom { project.properties["mavenPomName"]?.let { name = "$it" } project.properties["mavenPomDescription"]?.let { description = "$it" } diff --git a/manual/src/docs/asciidoc/chapters/10-cleanup.adoc b/manual/src/docs/asciidoc/chapters/10-cleanup.adoc index f13abab2..583bc53c 100644 --- a/manual/src/docs/asciidoc/chapters/10-cleanup.adoc +++ b/manual/src/docs/asciidoc/chapters/10-cleanup.adoc @@ -1,14 +1,14 @@ [id=cleanup] -= Cleaning Up Orphaned Approved Files += Cleaning Up Leftover Approved Files [id="cleanup_problem"] == The Problem When you rename or delete a test method, its approved file stays behind on disk. -Over time these orphaned files accumulate and clutter the repository. +Over time these leftover files accumulate and clutter the repository. -ApproveJ provides an inventory-based mechanism to detect and remove orphaned approved files automatically. +ApproveJ provides an inventory-based mechanism to detect and remove leftover approved files automatically. [id="cleanup_inventory"] @@ -67,11 +67,11 @@ export APPROVEJ_INVENTORY_ENABLED=false ---- -[id="cleanup_finding_orphans"] -== Finding and Removing Orphans +[id="cleanup_finding_leftovers"] +== Finding and Removing Leftovers -After running your tests with the inventory enabled, use the build plugin to detect orphans. -An approved file is considered orphaned when its originating test method no longer exists in the compiled test classes. +After running your tests with the inventory enabled, use the build plugin to detect leftovers. +An approved file is considered a leftover when its originating test method no longer exists in the compiled test classes. [id="cleanup_gradle"] @@ -100,11 +100,11 @@ The plugin registers two tasks in the `verification` group: |=== |Task |Description -|`./gradlew approvejFindOrphans` -|Lists orphaned approved files without deleting them +|`./gradlew approvejFindLeftovers` +|Lists leftover approved files without deleting them |`./gradlew approvejCleanup` -|Detects and removes orphaned approved files +|Detects and removes leftover approved files |=== NOTE: Both tasks require the Java plugin to be applied in the same project. @@ -131,11 +131,11 @@ The plugin provides two goals: |=== |Goal |Description -|`mvn approvej:find-orphans` -|Lists orphaned approved files without deleting them +|`mvn approvej:find-leftovers` +|Lists leftover approved files without deleting them |`mvn approvej:cleanup` -|Detects and removes orphaned approved files +|Detects and removes leftover approved files |=== @@ -143,9 +143,9 @@ The plugin provides two goals: == Typical Workflow 1. Run your tests locally so the inventory is populated. -2. Run the find-orphans task/goal to see which approved files are orphaned. -3. Run the cleanup task/goal to delete the orphaned files. -4. Commit the updated inventory and the removal of orphaned files. +2. Run the find-leftovers task/goal to see which approved files are leftovers. +3. Run the cleanup task/goal to delete the leftover files. +4. Commit the updated inventory and the removal of leftover files. -WARNING: Only approved files that have been recorded in the inventory can be detected as orphans. +WARNING: Only approved files that have been recorded in the inventory can be detected as leftovers. Make sure you have run all relevant tests with the inventory enabled at least once before cleaning up. diff --git a/manual/src/docs/asciidoc/chapters/12-cheat-sheet.adoc b/manual/src/docs/asciidoc/chapters/12-cheat-sheet.adoc index 6ae95fc8..c7869a4f 100644 --- a/manual/src/docs/asciidoc/chapters/12-cheat-sheet.adoc +++ b/manual/src/docs/asciidoc/chapters/12-cheat-sheet.adoc @@ -283,17 +283,17 @@ endif::[] |=== |Command |Description -|`./gradlew approvejFindOrphans` -|List orphaned approved files (Gradle) +|`./gradlew approvejFindLeftovers` +|List leftover approved files (Gradle) |`./gradlew approvejCleanup` -|Remove orphaned approved files (Gradle) +|Remove leftover approved files (Gradle) -|`mvn approvej:find-orphans` -|List orphaned approved files (Maven) +|`mvn approvej:find-leftovers` +|List leftover approved files (Maven) |`mvn approvej:cleanup` -|Remove orphaned approved files (Maven) +|Remove leftover approved files (Maven) |=== diff --git a/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java index b860abf8..e2297e28 100644 --- a/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java +++ b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java @@ -15,7 +15,7 @@ import org.jspecify.annotations.NullMarked; /** - * Tracks approved files in an inventory so that orphaned files (from renamed or deleted tests) can + * Tracks approved files in an inventory so that leftover files (from renamed or deleted tests) can * be detected and cleaned up. * *

During a test run, each {@code byFile()} call records the approved file path and its @@ -75,18 +75,18 @@ static void writeInventory() { } /** - * Finds orphaned inventory entries whose test methods no longer exist. + * Finds leftover inventory entries whose test methods no longer exist. * - * @return a list of orphaned inventory entries + * @return a list of leftover inventory entries */ - static List findOrphans() { + static List findLeftovers() { return loadInventory().entrySet().stream() .map(entry -> new InventoryEntry(entry.getKey(), entry.getValue())) - .filter(ApprovedFileInventory::isOrphan) + .filter(ApprovedFileInventory::isLeftover) .toList(); } - private static boolean isOrphan(InventoryEntry entry) { + private static boolean isLeftover(InventoryEntry entry) { try { return stream(Class.forName(entry.className()).getDeclaredMethods()) .noneMatch(method -> method.getName().equals(entry.methodName())); @@ -96,30 +96,30 @@ private static boolean isOrphan(InventoryEntry entry) { } /** - * Removes orphaned approved files and updates the inventory. + * Removes leftover approved files and updates the inventory. * - * @return the list of removed orphan entries + * @return the list of removed leftover entries */ - static List removeOrphans() { - List orphans = findOrphans(); - if (orphans.isEmpty()) { - return orphans; + static List removeLeftovers() { + List leftovers = findLeftovers(); + if (leftovers.isEmpty()) { + return leftovers; } TreeMap inventory = loadInventory(); - orphans.forEach( - orphan -> { + leftovers.forEach( + leftover -> { try { - Files.deleteIfExists(Path.of(orphan.relativePath())); - inventory.remove(orphan.relativePath()); + Files.deleteIfExists(Path.of(leftover.relativePath())); + inventory.remove(leftover.relativePath()); } catch (IOException e) { - System.err.printf("Failed to delete orphaned file: %s%n", orphan.relativePath()); + System.err.printf("Failed to delete leftover file: %s%n", leftover.relativePath()); } }); saveInventory(inventory); - return orphans; + return leftovers; } static TreeMap loadInventory() { @@ -184,7 +184,7 @@ static void reset() { /** * CLI entry point for build tool plugins. * - * @param args {@code --find} to list orphans, {@code --remove} to delete them + * @param args {@code --find} to list leftovers, {@code --remove} to delete them */ public static void main(String[] args) { String usage = "Usage: ApprovedFileInventory --find | --remove"; @@ -194,27 +194,28 @@ public static void main(String[] args) { } String command = args[0]; - switch (command) { case "--find" -> { - List orphans = findOrphans(); - if (orphans.isEmpty()) { - System.out.println("No orphaned approved files found."); + List leftovers = findLeftovers(); + if (leftovers.isEmpty()) { + System.out.println("No leftover approved files found."); } else { - System.out.println("Orphaned approved files:"); - orphans.forEach( - orphan -> + System.out.println("Leftover approved files:"); + leftovers.forEach( + leftover -> System.out.printf( - " %s (from %s)%n", orphan.relativePath(), orphan.testReference())); + " %s%n from %s%n", + Path.of(leftover.relativePath()).toUri(), leftover.testReference())); } } case "--remove" -> { - List removed = removeOrphans(); + List removed = removeLeftovers(); if (removed.isEmpty()) { - System.out.println("No orphaned approved files found."); + System.out.println("No leftover approved files found."); } else { - System.out.println("Removed orphaned approved files:"); - removed.forEach(orphan -> System.out.printf(" %s%n", orphan.relativePath())); + System.out.println("Removed leftover approved files:"); + removed.forEach( + leftover -> System.out.printf(" %s%n", Path.of(leftover.relativePath()).toUri())); } } default -> { diff --git a/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java b/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java index d64f7be4..527443e2 100644 --- a/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java +++ b/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java @@ -142,7 +142,7 @@ void record_merges_with_existing() throws IOException { } @Test - void findOrphans() throws IOException { + void findLeftovers() throws IOException { writeString( inventoryFile, """ @@ -151,17 +151,17 @@ void findOrphans() throws IOException { """, StandardOpenOption.CREATE); - List orphans = ApprovedFileInventory.findOrphans(); + List leftovers = ApprovedFileInventory.findLeftovers(); - assertThat(orphans).hasSize(1); - assertThat(orphans.getFirst().relativePath()) + assertThat(leftovers).hasSize(1); + assertThat(leftovers.getFirst().relativePath()) .isEqualTo("src/test/NonExistent-test-approved.txt"); - assertThat(orphans.getFirst().testReference()) + assertThat(leftovers.getFirst().testReference()) .isEqualTo("com.nonexistent.NonExistentTest#test"); } @Test - void findOrphans_missing_method() throws IOException { + void findLeftovers_missing_method() throws IOException { writeString( inventoryFile, """ @@ -170,36 +170,36 @@ void findOrphans_missing_method() throws IOException { """, StandardOpenOption.CREATE); - List orphans = ApprovedFileInventory.findOrphans(); + List leftovers = ApprovedFileInventory.findLeftovers(); - assertThat(orphans).hasSize(1); - assertThat(orphans.getFirst().testReference()) + assertThat(leftovers).hasSize(1); + assertThat(leftovers.getFirst().testReference()) .isEqualTo("org.approvej.approve.ApprovedFileInventoryTest#nonExistentMethod"); } @Test - void removeOrphans() throws IOException { - Path orphanFile = tempDir.resolve("orphan-approved.txt"); - writeString(orphanFile, "old content", StandardOpenOption.CREATE); + void removeLeftovers() throws IOException { + Path leftoverFile = tempDir.resolve("leftover-approved.txt"); + writeString(leftoverFile, "old content", StandardOpenOption.CREATE); Path validFile = tempDir.resolve("valid-approved.txt"); writeString( inventoryFile, "# ApproveJ Approved File Inventory (auto-generated, do not edit)\n" - + orphanFile + + leftoverFile + " = com.nonexistent.NonExistentTest#test\n" + validFile - + " = org.approvej.approve.ApprovedFileInventoryTest#removeOrphans\n", + + " = org.approvej.approve.ApprovedFileInventoryTest#removeLeftovers\n", StandardOpenOption.CREATE); - List removed = ApprovedFileInventory.removeOrphans(); + List removed = ApprovedFileInventory.removeLeftovers(); assertThat(removed).hasSize(1); - assertThat(orphanFile).doesNotExist(); + assertThat(leftoverFile).doesNotExist(); TreeMap inventory = ApprovedFileInventory.loadInventory(); assertThat(inventory) - .doesNotContainKey(orphanFile.toString()) + .doesNotContainKey(leftoverFile.toString()) .containsKey(validFile.toString()); } } diff --git a/plugins/gradle/build.gradle.kts b/plugins/approvej-gradle-plugin/build.gradle.kts similarity index 94% rename from plugins/gradle/build.gradle.kts rename to plugins/approvej-gradle-plugin/build.gradle.kts index 26ca2f49..8fb8e741 100644 --- a/plugins/gradle/build.gradle.kts +++ b/plugins/approvej-gradle-plugin/build.gradle.kts @@ -36,7 +36,7 @@ gradlePlugin { id = "org.approvej" implementationClass = "org.approvej.gradle.ApproveJPlugin" displayName = "ApproveJ" - description = "Find and remove orphaned approved files" + description = "Find and remove leftover approved files" tags = listOf("testing", "approval-testing", "snapshot-testing") } } diff --git a/plugins/gradle/src/main/java/org/approvej/gradle/ApproveJPlugin.java b/plugins/approvej-gradle-plugin/src/main/java/org/approvej/gradle/ApproveJPlugin.java similarity index 85% rename from plugins/gradle/src/main/java/org/approvej/gradle/ApproveJPlugin.java rename to plugins/approvej-gradle-plugin/src/main/java/org/approvej/gradle/ApproveJPlugin.java index 0ea11238..60f5e56e 100644 --- a/plugins/gradle/src/main/java/org/approvej/gradle/ApproveJPlugin.java +++ b/plugins/approvej-gradle-plugin/src/main/java/org/approvej/gradle/ApproveJPlugin.java @@ -6,7 +6,7 @@ import org.gradle.api.tasks.JavaExec; import org.gradle.api.tasks.SourceSetContainer; -/** Gradle plugin that registers tasks to find and remove orphaned approved files. */ +/** Gradle plugin that registers tasks to find and remove leftover approved files. */ @SuppressWarnings("unused") public final class ApproveJPlugin implements Plugin { @@ -27,11 +27,11 @@ public void apply(Project project) { project .getTasks() .register( - "approvejFindOrphans", + "approvejFindLeftovers", JavaExec.class, task -> { task.setGroup("verification"); - task.setDescription("List orphaned approved files"); + task.setDescription("List leftover approved files"); task.setClasspath(testClasspath); task.getMainClass().set("org.approvej.approve.ApprovedFileInventory"); task.args("--find"); @@ -44,7 +44,7 @@ public void apply(Project project) { JavaExec.class, task -> { task.setGroup("verification"); - task.setDescription("Detect and remove orphaned approved files"); + task.setDescription("Detect and remove leftover approved files"); task.setClasspath(testClasspath); task.getMainClass().set("org.approvej.approve.ApprovedFileInventory"); task.args("--remove"); diff --git a/plugins/gradle/src/test/java/org/approvej/gradle/ApproveJPluginTest.java b/plugins/approvej-gradle-plugin/src/test/java/org/approvej/gradle/ApproveJPluginTest.java similarity index 84% rename from plugins/gradle/src/test/java/org/approvej/gradle/ApproveJPluginTest.java rename to plugins/approvej-gradle-plugin/src/test/java/org/approvej/gradle/ApproveJPluginTest.java index dde535c2..5f526d4d 100644 --- a/plugins/gradle/src/test/java/org/approvej/gradle/ApproveJPluginTest.java +++ b/plugins/approvej-gradle-plugin/src/test/java/org/approvej/gradle/ApproveJPluginTest.java @@ -23,7 +23,7 @@ void apply() { project.getPluginManager().apply(ApproveJPlugin.class); - assertThat(project.getTasks().findByName("approvejFindOrphans")).isNotNull(); + assertThat(project.getTasks().findByName("approvejFindLeftovers")).isNotNull(); assertThat(project.getTasks().findByName("approvejCleanup")).isNotNull(); } @@ -33,7 +33,7 @@ void apply_without_java_plugin() { project.getPluginManager().apply(ApproveJPlugin.class); - assertThat(project.getTasks().findByName("approvejFindOrphans")).isNull(); + assertThat(project.getTasks().findByName("approvejFindLeftovers")).isNull(); assertThat(project.getTasks().findByName("approvejCleanup")).isNull(); } @@ -44,20 +44,20 @@ void apply_java_plugin_after_approvej() { project.getPluginManager().apply("java"); - assertThat(project.getTasks().findByName("approvejFindOrphans")).isNotNull(); + assertThat(project.getTasks().findByName("approvejFindLeftovers")).isNotNull(); assertThat(project.getTasks().findByName("approvejCleanup")).isNotNull(); } @Test - void approvejFindOrphans_task_configuration() { + void approvejFindLeftovers_task_configuration() { Project project = ProjectBuilder.builder().build(); project.getPluginManager().apply("java"); project.getPluginManager().apply(ApproveJPlugin.class); - var task = (JavaExec) project.getTasks().getByName("approvejFindOrphans"); + var task = (JavaExec) project.getTasks().getByName("approvejFindLeftovers"); assertThat(task.getGroup()).isEqualTo("verification"); - assertThat(task.getDescription()).isEqualTo("List orphaned approved files"); + assertThat(task.getDescription()).isEqualTo("List leftover approved files"); assertThat(task.getMainClass().get()).isEqualTo("org.approvej.approve.ApprovedFileInventory"); assertThat(task.getArgs()).containsExactly("--find"); } @@ -71,7 +71,7 @@ void approvejCleanup_task_configuration() { var task = (JavaExec) project.getTasks().getByName("approvejCleanup"); assertThat(task.getGroup()).isEqualTo("verification"); - assertThat(task.getDescription()).isEqualTo("Detect and remove orphaned approved files"); + assertThat(task.getDescription()).isEqualTo("Detect and remove leftover approved files"); assertThat(task.getMainClass().get()).isEqualTo("org.approvej.approve.ApprovedFileInventory"); assertThat(task.getArgs()).containsExactly("--remove"); } @@ -95,8 +95,8 @@ void apply_functional() throws IOException { .build(); assertThat(result.getOutput()) - .contains("approvejFindOrphans - List orphaned approved files") - .contains("approvejCleanup - Detect and remove orphaned approved files"); + .contains("approvejFindLeftovers - List leftover approved files") + .contains("approvejCleanup - Detect and remove leftover approved files"); } @Test @@ -117,7 +117,7 @@ void apply_functional_without_java_plugin() throws IOException { .build(); assertThat(result.getOutput()) - .doesNotContain("approvejFindOrphans") + .doesNotContain("approvejFindLeftovers") .doesNotContain("approvejCleanup"); } } diff --git a/plugins/maven/build.gradle.kts b/plugins/approvej-maven-plugin/build.gradle.kts similarity index 100% rename from plugins/maven/build.gradle.kts rename to plugins/approvej-maven-plugin/build.gradle.kts diff --git a/plugins/approvej-maven-plugin/gradle.properties b/plugins/approvej-maven-plugin/gradle.properties new file mode 100644 index 00000000..cea5a3fb --- /dev/null +++ b/plugins/approvej-maven-plugin/gradle.properties @@ -0,0 +1,2 @@ +mavenPomName = ApproveJ Maven Plugin +mavenPomDescription = Maven plugin to find and remove leftover approved files diff --git a/plugins/maven/src/main/java/org/approvej/maven/CleanupMojo.java b/plugins/approvej-maven-plugin/src/main/java/org/approvej/maven/CleanupMojo.java similarity index 92% rename from plugins/maven/src/main/java/org/approvej/maven/CleanupMojo.java rename to plugins/approvej-maven-plugin/src/main/java/org/approvej/maven/CleanupMojo.java index e6dac8c7..fbcfa37f 100644 --- a/plugins/maven/src/main/java/org/approvej/maven/CleanupMojo.java +++ b/plugins/approvej-maven-plugin/src/main/java/org/approvej/maven/CleanupMojo.java @@ -7,7 +7,7 @@ import org.apache.maven.plugins.annotations.ResolutionScope; import org.apache.maven.project.MavenProject; -/** Detects and removes orphaned approved files whose originating test method no longer exists. */ +/** Detects and removes leftover approved files whose originating test method no longer exists. */ @Mojo(name = "cleanup", requiresDependencyResolution = ResolutionScope.TEST, threadSafe = true) public class CleanupMojo extends AbstractMojo { diff --git a/plugins/maven/src/main/java/org/approvej/maven/FindOrphansMojo.java b/plugins/approvej-maven-plugin/src/main/java/org/approvej/maven/FindLeftoversMojo.java similarity index 72% rename from plugins/maven/src/main/java/org/approvej/maven/FindOrphansMojo.java rename to plugins/approvej-maven-plugin/src/main/java/org/approvej/maven/FindLeftoversMojo.java index 4d1eb56d..254668c3 100644 --- a/plugins/maven/src/main/java/org/approvej/maven/FindOrphansMojo.java +++ b/plugins/approvej-maven-plugin/src/main/java/org/approvej/maven/FindLeftoversMojo.java @@ -7,9 +7,12 @@ import org.apache.maven.plugins.annotations.ResolutionScope; import org.apache.maven.project.MavenProject; -/** Lists orphaned approved files whose originating test method no longer exists. */ -@Mojo(name = "find-orphans", requiresDependencyResolution = ResolutionScope.TEST, threadSafe = true) -public class FindOrphansMojo extends AbstractMojo { +/** Lists leftover approved files whose originating test method no longer exists. */ +@Mojo( + name = "find-leftovers", + requiresDependencyResolution = ResolutionScope.TEST, + threadSafe = true) +public class FindLeftoversMojo extends AbstractMojo { @Parameter(defaultValue = "${project}", readonly = true, required = true) private MavenProject project; diff --git a/plugins/maven/src/main/java/org/approvej/maven/MojoHelper.java b/plugins/approvej-maven-plugin/src/main/java/org/approvej/maven/MojoHelper.java similarity index 100% rename from plugins/maven/src/main/java/org/approvej/maven/MojoHelper.java rename to plugins/approvej-maven-plugin/src/main/java/org/approvej/maven/MojoHelper.java diff --git a/plugins/maven/src/test/java/org/approvej/maven/MojoHelperTest.java b/plugins/approvej-maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java similarity index 100% rename from plugins/maven/src/test/java/org/approvej/maven/MojoHelperTest.java rename to plugins/approvej-maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java diff --git a/plugins/maven/gradle.properties b/plugins/maven/gradle.properties deleted file mode 100644 index 5f2ddbe6..00000000 --- a/plugins/maven/gradle.properties +++ /dev/null @@ -1,3 +0,0 @@ -mavenPomArtifactId = approvej-maven-plugin -mavenPomName = ApproveJ Maven Plugin -mavenPomDescription = Maven plugin to find and remove orphaned approved files diff --git a/settings.gradle.kts b/settings.gradle.kts index 06960f66..3c56cda7 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -6,9 +6,9 @@ include("bom") include("modules:core") -include("plugins:gradle") +include("plugins:approvej-gradle-plugin") -include("plugins:maven") +include("plugins:approvej-maven-plugin") include("modules:json-jackson") From 812ad578e7baea1388712210de232ebfeb332331 Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Fri, 20 Feb 2026 12:51:38 +0100 Subject: [PATCH 19/25] Fix test method naming and ordering conventions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename test methods to start with the method under test: - registerApprovedFile/record_* → writeInventory_* in ApprovedFileInventoryTest - approvejFindLeftovers/approvejCleanup_* → apply_* in ApproveJPluginTest Reorder test methods to match production code order: - Group loadConfiguration_inventoryEnabled_* with other loadConfiguration tests - Move toEnvironmentVariableName after get tests in ConfigurationTest - Move executeInventory before buildCommand in MojoHelperTest --- .../approve/ApprovedFileInventoryTest.java | 12 +- .../configuration/ConfigurationTest.java | 108 +++++++++--------- .../approvej/gradle/ApproveJPluginTest.java | 4 +- .../org/approvej/maven/MojoHelperTest.java | 24 ++-- 4 files changed, 74 insertions(+), 74 deletions(-) diff --git a/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java b/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java index 527443e2..f7ea5509 100644 --- a/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java +++ b/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java @@ -31,7 +31,7 @@ void tearDown() { } @Test - void registerApprovedFile() { + void writeInventory() { ApprovedFileInventory.addEntry( "src/test/MyTest-myTest-approved.txt", "com.example.MyTest#myTest"); @@ -43,7 +43,7 @@ void registerApprovedFile() { } @Test - void record_multiple_entries() { + void writeInventory_multiple_entries() { ApprovedFileInventory.addEntry("src/test/BTest-b-approved.txt", "com.example.BTest#b"); ApprovedFileInventory.addEntry("src/test/ATest-a-approved.txt", "com.example.ATest#a"); @@ -55,7 +55,7 @@ void record_multiple_entries() { } @Test - void record_named_approvals() { + void writeInventory_named_approvals() { ApprovedFileInventory.addEntry( "src/test/MyTest-myTest-alpha-approved.txt", "com.example.MyTest#myTest"); ApprovedFileInventory.addEntry( @@ -71,7 +71,7 @@ void record_named_approvals() { } @Test - void record_replaces_entries_for_executed_methods() throws IOException { + void writeInventory_replaces_entries_for_executed_methods() throws IOException { writeString( inventoryFile, """ @@ -97,7 +97,7 @@ void record_replaces_entries_for_executed_methods() throws IOException { } @Test - void record_preserves_entries_for_unexecuted_methods() throws IOException { + void writeInventory_preserves_entries_for_unexecuted_methods() throws IOException { writeString( inventoryFile, """ @@ -119,7 +119,7 @@ void record_preserves_entries_for_unexecuted_methods() throws IOException { } @Test - void record_merges_with_existing() throws IOException { + void writeInventory_merges_with_existing() throws IOException { writeString( inventoryFile, """ diff --git a/modules/core/src/test/java/org/approvej/configuration/ConfigurationTest.java b/modules/core/src/test/java/org/approvej/configuration/ConfigurationTest.java index 3075ee3f..13667972 100644 --- a/modules/core/src/test/java/org/approvej/configuration/ConfigurationTest.java +++ b/modules/core/src/test/java/org/approvej/configuration/ConfigurationTest.java @@ -81,6 +81,53 @@ void loadConfiguration_invalidPrintFormatThrowsError() { .withMessageContaining("org.nonexistent.InvalidFormat"); } + @Test + void loadConfiguration_inventoryEnabled_defaults_to_true() { + ConfigurationLoader loader = ConfigurationLoader.builder().build(); + + Configuration config = Configuration.loadConfiguration(loader); + + assertThat(config.inventoryEnabled()).isTrue(); + } + + @Test + void loadConfiguration_inventoryEnabled_defaults_to_false_in_ci() { + Map env = Map.of("CI", "true"); + ConfigurationLoader loader = + ConfigurationLoader.builder().withEnvironmentVariables(env::get).build(); + + Configuration config = Configuration.loadConfiguration(loader); + + assertThat(config.inventoryEnabled()).isFalse(); + } + + @Test + void loadConfiguration_inventoryEnabled_from_properties() { + Properties props = new Properties(); + props.setProperty("inventoryEnabled", "false"); + ConfigurationLoader loader = ConfigurationLoader.builder().withProperties(props).build(); + + Configuration config = Configuration.loadConfiguration(loader); + + assertThat(config.inventoryEnabled()).isFalse(); + } + + @Test + void loadConfiguration_inventoryEnabled_env_overrides_properties() { + Map env = Map.of("APPROVEJ_INVENTORY_ENABLED", "false"); + Properties props = new Properties(); + props.setProperty("inventoryEnabled", "true"); + ConfigurationLoader loader = + ConfigurationLoader.builder() + .withEnvironmentVariables(env::get) + .withProperties(props) + .build(); + + Configuration config = Configuration.loadConfiguration(loader); + + assertThat(config.inventoryEnabled()).isFalse(); + } + @Test void configurationLoader_priorityChain() { // Simulate: env > project props > user home props @@ -129,16 +176,6 @@ void configurationLoader_envOverridesAll() { assertThat(config.defaultPrintFormat()).isInstanceOf(SingleLineStringPrintFormat.class); } - @Test - void toEnvironmentVariableName() { - assertThat(ConfigurationLoader.toEnvironmentVariableName("defaultPrintFormat")) - .isEqualTo("APPROVEJ_DEFAULT_PRINT_FORMAT"); - assertThat(ConfigurationLoader.toEnvironmentVariableName("timeout")) - .isEqualTo("APPROVEJ_TIMEOUT"); - assertThat(ConfigurationLoader.toEnvironmentVariableName("maxRetryCount")) - .isEqualTo("APPROVEJ_MAX_RETRY_COUNT"); - } - @Test void configurationLoader_get_returnsDefaultWhenNoSourcesConfigured() { ConfigurationLoader loader = ConfigurationLoader.builder().build(); @@ -199,49 +236,12 @@ void configurationLoader_emptyBuilder_returnsLoaderWithNoSources() { } @Test - void loadConfiguration_inventoryEnabled_defaults_to_true() { - ConfigurationLoader loader = ConfigurationLoader.builder().build(); - - Configuration config = Configuration.loadConfiguration(loader); - - assertThat(config.inventoryEnabled()).isTrue(); - } - - @Test - void loadConfiguration_inventoryEnabled_defaults_to_false_in_ci() { - Map env = Map.of("CI", "true"); - ConfigurationLoader loader = - ConfigurationLoader.builder().withEnvironmentVariables(env::get).build(); - - Configuration config = Configuration.loadConfiguration(loader); - - assertThat(config.inventoryEnabled()).isFalse(); - } - - @Test - void loadConfiguration_inventoryEnabled_from_properties() { - Properties props = new Properties(); - props.setProperty("inventoryEnabled", "false"); - ConfigurationLoader loader = ConfigurationLoader.builder().withProperties(props).build(); - - Configuration config = Configuration.loadConfiguration(loader); - - assertThat(config.inventoryEnabled()).isFalse(); - } - - @Test - void loadConfiguration_inventoryEnabled_env_overrides_properties() { - Map env = Map.of("APPROVEJ_INVENTORY_ENABLED", "false"); - Properties props = new Properties(); - props.setProperty("inventoryEnabled", "true"); - ConfigurationLoader loader = - ConfigurationLoader.builder() - .withEnvironmentVariables(env::get) - .withProperties(props) - .build(); - - Configuration config = Configuration.loadConfiguration(loader); - - assertThat(config.inventoryEnabled()).isFalse(); + void toEnvironmentVariableName() { + assertThat(ConfigurationLoader.toEnvironmentVariableName("defaultPrintFormat")) + .isEqualTo("APPROVEJ_DEFAULT_PRINT_FORMAT"); + assertThat(ConfigurationLoader.toEnvironmentVariableName("timeout")) + .isEqualTo("APPROVEJ_TIMEOUT"); + assertThat(ConfigurationLoader.toEnvironmentVariableName("maxRetryCount")) + .isEqualTo("APPROVEJ_MAX_RETRY_COUNT"); } } diff --git a/plugins/approvej-gradle-plugin/src/test/java/org/approvej/gradle/ApproveJPluginTest.java b/plugins/approvej-gradle-plugin/src/test/java/org/approvej/gradle/ApproveJPluginTest.java index 5f526d4d..be97f71e 100644 --- a/plugins/approvej-gradle-plugin/src/test/java/org/approvej/gradle/ApproveJPluginTest.java +++ b/plugins/approvej-gradle-plugin/src/test/java/org/approvej/gradle/ApproveJPluginTest.java @@ -49,7 +49,7 @@ void apply_java_plugin_after_approvej() { } @Test - void approvejFindLeftovers_task_configuration() { + void apply_findLeftovers_task_configuration() { Project project = ProjectBuilder.builder().build(); project.getPluginManager().apply("java"); project.getPluginManager().apply(ApproveJPlugin.class); @@ -63,7 +63,7 @@ void approvejFindLeftovers_task_configuration() { } @Test - void approvejCleanup_task_configuration() { + void apply_cleanup_task_configuration() { Project project = ProjectBuilder.builder().build(); project.getPluginManager().apply("java"); project.getPluginManager().apply(ApproveJPlugin.class); diff --git a/plugins/approvej-maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java b/plugins/approvej-maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java index 03d736b2..0081c230 100644 --- a/plugins/approvej-maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java +++ b/plugins/approvej-maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java @@ -13,6 +13,18 @@ class MojoHelperTest { + @Test + void executeInventory_nonzero_exit_code(@TempDir Path tempDir) { + var project = new MavenProject(); + project.getBuild().setOutputDirectory(tempDir.resolve("classes").toString()); + project.getBuild().setTestOutputDirectory(tempDir.resolve("test-classes").toString()); + project.setFile(tempDir.resolve("pom.xml").toFile()); + + assertThatExceptionOfType(MojoExecutionException.class) + .isThrownBy(() -> MojoHelper.executeInventory(project, "--find", new SystemStreamLog())) + .withMessageContaining("exited with code"); + } + @Test void buildCommand_find() { var classpathElements = List.of("/lib/a.jar", "/lib/b.jar"); @@ -46,16 +58,4 @@ void buildCommand_remove() { "org.approvej.approve.ApprovedFileInventory", "--remove"); } - - @Test - void executeInventory_nonzero_exit_code(@TempDir Path tempDir) { - var project = new MavenProject(); - project.getBuild().setOutputDirectory(tempDir.resolve("classes").toString()); - project.getBuild().setTestOutputDirectory(tempDir.resolve("test-classes").toString()); - project.setFile(tempDir.resolve("pom.xml").toFile()); - - assertThatExceptionOfType(MojoExecutionException.class) - .isThrownBy(() -> MojoHelper.executeInventory(project, "--find", new SystemStreamLog())) - .withMessageContaining("exited with code"); - } } From 149befcfa4d08f9818e34dd8c612f1f433548c57 Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Fri, 20 Feb 2026 12:54:57 +0100 Subject: [PATCH 20/25] Fix outdated paths and project name in CONTRIBUTING.md Update plugin paths to match actual directory names, add missing modules (json-jackson3, yaml-jackson3, http-wiremock), and replace leftover "Shakespeare" reference with "ApproveJ". --- CONTRIBUTING.md | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 15ee58ed..2d1afdfe 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -103,7 +103,7 @@ All pipelines can be found in [.github/workflows](.github/workflows). ### Versioning -Shakespeare is using [SemVer 2.0](https://semver.org/spec/v2.0.0.html) but omits the patch digit in case it is `0`. +ApproveJ is using [SemVer 2.0](https://semver.org/spec/v2.0.0.html) but omits the patch digit in case it is `0`. E.g. `1.0.0` is written as `1.0`, `1.0.1` is written as `1.0.1`, and `1.1.0` is written as `1.1`. @@ -114,12 +114,15 @@ It is structured in four main directories: - The [modules](modules) directory contains all the published library modules: - [core](modules/core) contains the code for the core framework and should not have any dependencies to other modules and only very few (if any) to external libraries, - - [json-jackson](modules/json-jackson) contains JSON-related code using Jackson, - - [yaml-jackson](modules/yaml-jackson) contains YAML-related code using Jackson - - [http](modules/http) contains code to create an HTTP server for approving requests + - [json-jackson](modules/json-jackson) contains JSON-related code using Jackson 2.x, + - [json-jackson3](modules/json-jackson3) contains JSON-related code using Jackson 3.x, + - [yaml-jackson](modules/yaml-jackson) contains YAML-related code using Jackson 2.x, + - [yaml-jackson3](modules/yaml-jackson3) contains YAML-related code using Jackson 3.x, + - [http](modules/http) contains code to create an HTTP server for approving requests, + - [http-wiremock](modules/http-wiremock) contains the WireMock adapter for HTTP testing - The [plugins](plugins) directory contains build tool plugins: - - [gradle](plugins/gradle) contains the Gradle plugin for managing approved files - - [maven](plugins/maven) contains the Maven plugin for managing approved files + - [approvej-gradle-plugin](plugins/approvej-gradle-plugin) contains the Gradle plugin for managing approved files + - [approvej-maven-plugin](plugins/approvej-maven-plugin) contains the Maven plugin for managing approved files - the [bom](bom) directory contains the build file to generate a [Maven Bill of Material (BOM)](https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#bill-of-materials-bom-poms) for all the ApproveJ modules, and - the [manual](manual) directory contains the projects documentation written in [AsciiDoc](https://docs.asciidoctor.org/asciidoc/latest/). From 0a00566c7176c45c8ad42138a30fff22d163e11c Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Fri, 20 Feb 2026 13:05:30 +0100 Subject: [PATCH 21/25] Remove duplicate tests and parameterize buildCommand Remove writeInventory_merges_with_existing (duplicate of writeInventory_preserves_entries_for_unexecuted_methods) and configurationLoader_emptyBuilder_returnsLoaderWithNoSources (duplicate of configurationLoader_get_returnsDefaultWhenNoSourcesConfigured). Collapse buildCommand_find and buildCommand_remove into a single parameterized test. --- .../approve/ApprovedFileInventoryTest.java | 23 -------------- .../configuration/ConfigurationTest.java | 7 ----- .../org/approvej/maven/MojoHelperTest.java | 30 +++++-------------- 3 files changed, 8 insertions(+), 52 deletions(-) diff --git a/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java b/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java index f7ea5509..65aefc48 100644 --- a/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java +++ b/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java @@ -118,29 +118,6 @@ void writeInventory_preserves_entries_for_unexecuted_methods() throws IOExceptio .containsEntry("src/test/MyTest-myTest-approved.txt", "com.example.MyTest#myTest"); } - @Test - void writeInventory_merges_with_existing() throws IOException { - writeString( - inventoryFile, - """ - # ApproveJ Approved File Inventory (auto-generated, do not edit) - src/test/ExistingTest-existing-approved.txt = com.example.ExistingTest#existing - """, - StandardOpenOption.CREATE); - - ApprovedFileInventory.addEntry( - "src/test/NewTest-newTest-approved.txt", "com.example.NewTest#newTest"); - - ApprovedFileInventory.writeInventory(); - - TreeMap inventory = ApprovedFileInventory.loadInventory(); - assertThat(inventory) - .hasSize(2) - .containsEntry( - "src/test/ExistingTest-existing-approved.txt", "com.example.ExistingTest#existing") - .containsEntry("src/test/NewTest-newTest-approved.txt", "com.example.NewTest#newTest"); - } - @Test void findLeftovers() throws IOException { writeString( diff --git a/modules/core/src/test/java/org/approvej/configuration/ConfigurationTest.java b/modules/core/src/test/java/org/approvej/configuration/ConfigurationTest.java index 13667972..30d38f61 100644 --- a/modules/core/src/test/java/org/approvej/configuration/ConfigurationTest.java +++ b/modules/core/src/test/java/org/approvej/configuration/ConfigurationTest.java @@ -228,13 +228,6 @@ void configurationLoader_chainedSources_firstNonNullWins() { assertThat(loader.get("shared", null)).isEqualTo("fromFirst"); } - @Test - void configurationLoader_emptyBuilder_returnsLoaderWithNoSources() { - ConfigurationLoader loader = ConfigurationLoader.builder().build(); - - assertThat(loader.get("anyKey", "default")).isEqualTo("default"); - } - @Test void toEnvironmentVariableName() { assertThat(ConfigurationLoader.toEnvironmentVariableName("defaultPrintFormat")) diff --git a/plugins/approvej-maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java b/plugins/approvej-maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java index 0081c230..6bd555f6 100644 --- a/plugins/approvej-maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java +++ b/plugins/approvej-maven-plugin/src/test/java/org/approvej/maven/MojoHelperTest.java @@ -10,6 +10,8 @@ import org.apache.maven.project.MavenProject; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; class MojoHelperTest { @@ -25,37 +27,21 @@ void executeInventory_nonzero_exit_code(@TempDir Path tempDir) { .withMessageContaining("exited with code"); } - @Test - void buildCommand_find() { - var classpathElements = List.of("/lib/a.jar", "/lib/b.jar"); - - var command = MojoHelper.buildCommand(classpathElements, "--find"); - - String expectedJava = Path.of(System.getProperty("java.home"), "bin", "java").toString(); - String expectedClasspath = "/lib/a.jar" + System.getProperty("path.separator") + "/lib/b.jar"; - assertThat(command) - .containsExactly( - expectedJava, - "-cp", - expectedClasspath, - "org.approvej.approve.ApprovedFileInventory", - "--find"); - } - - @Test - void buildCommand_remove() { + @ParameterizedTest + @ValueSource(strings = {"--find", "--remove"}) + void buildCommand(String command) { var classpathElements = List.of("/lib/a.jar", "/lib/b.jar"); - var command = MojoHelper.buildCommand(classpathElements, "--remove"); + var result = MojoHelper.buildCommand(classpathElements, command); String expectedJava = Path.of(System.getProperty("java.home"), "bin", "java").toString(); String expectedClasspath = "/lib/a.jar" + System.getProperty("path.separator") + "/lib/b.jar"; - assertThat(command) + assertThat(result) .containsExactly( expectedJava, "-cp", expectedClasspath, "org.approvej.approve.ApprovedFileInventory", - "--remove"); + command); } } From 9be849ab93d144b427b08871f51f1d3d86ac6666 Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Fri, 20 Feb 2026 13:21:12 +0100 Subject: [PATCH 22/25] Harden ApprovedFileInventory - Use removeIf for clearer intent when removing executed method entries - Return only successfully deleted entries from removeLeftovers - Make inventoryFile volatile for thread safety - Track shutdown hook via AtomicReference to avoid duplicate registration - Handle path relativization failure across filesystem roots --- .../approve/ApprovedFileInventory.java | 56 ++++++++++++------- 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java index e2297e28..cc0c089f 100644 --- a/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java +++ b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java @@ -7,11 +7,12 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; import java.util.List; import java.util.Properties; import java.util.TreeMap; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import org.jspecify.annotations.NullMarked; /** @@ -32,9 +33,9 @@ public class ApprovedFileInventory { private static final ConcurrentHashMap entries = new ConcurrentHashMap<>(); private static final ConcurrentHashMap executedMethods = new ConcurrentHashMap<>(); - private static final AtomicBoolean shutdownHookRegistered = new AtomicBoolean(false); + private static final AtomicReference shutdownHook = new AtomicReference<>(); - private static Path inventoryFile = DEFAULT_INVENTORY_FILE; + private static volatile Path inventoryFile = DEFAULT_INVENTORY_FILE; private ApprovedFileInventory() {} @@ -53,22 +54,27 @@ public static void registerApprovedFile(PathProvider pathProvider) { String testReference = "%s#%s".formatted(testMethod.testClass().getName(), testMethod.testCaseName()); - String relativePath = - Path.of("").toAbsolutePath().relativize(pathProvider.approvedPath()).toString(); + Path cwd = Path.of("").toAbsolutePath(); + Path approvedPath = pathProvider.approvedPath().toAbsolutePath(); + String relativePath; + try { + relativePath = cwd.relativize(approvedPath).toString(); + } catch (IllegalArgumentException e) { + relativePath = approvedPath.toString(); + } addEntry(relativePath, testReference); - if (shutdownHookRegistered.compareAndSet(false, true)) { - Runtime.getRuntime() - .addShutdownHook( - new Thread(ApprovedFileInventory::writeInventory, "ApproveJ-Inventory-Writer")); + Thread hook = new Thread(ApprovedFileInventory::writeInventory, "ApproveJ-Inventory-Writer"); + if (shutdownHook.compareAndSet(null, hook)) { + Runtime.getRuntime().addShutdownHook(hook); } } static void writeInventory() { TreeMap merged = loadInventory(); - merged.values().removeAll(executedMethods.keySet()); + merged.entrySet().removeIf(entry -> executedMethods.containsKey(entry.getValue())); merged.putAll(entries); saveInventory(merged); @@ -107,19 +113,20 @@ static List removeLeftovers() { } TreeMap inventory = loadInventory(); - leftovers.forEach( - leftover -> { - try { - Files.deleteIfExists(Path.of(leftover.relativePath())); - inventory.remove(leftover.relativePath()); - } catch (IOException e) { - System.err.printf("Failed to delete leftover file: %s%n", leftover.relativePath()); - } - }); + List removed = new ArrayList<>(); + for (InventoryEntry leftover : leftovers) { + try { + Files.deleteIfExists(Path.of(leftover.relativePath())); + inventory.remove(leftover.relativePath()); + removed.add(leftover); + } catch (IOException e) { + System.err.printf("Failed to delete leftover file: %s%n", leftover.relativePath()); + } + } saveInventory(inventory); - return leftovers; + return removed; } static TreeMap loadInventory() { @@ -172,7 +179,14 @@ static void addEntry(String relativePath, String testReference) { static void reset(Path testInventoryFile) { entries.clear(); executedMethods.clear(); - shutdownHookRegistered.set(false); + Thread hook = shutdownHook.getAndSet(null); + if (hook != null) { + try { + Runtime.getRuntime().removeShutdownHook(hook); + } catch (IllegalStateException e) { + // JVM is already shutting down + } + } inventoryFile = testInventoryFile; } From d10e2bff1881fdc8982a3ca43c524ab1f0503ce5 Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Fri, 20 Feb 2026 16:46:45 +0100 Subject: [PATCH 23/25] Remove toAbsolutePath from path resolution PathProvider and StackTraceTestFinderUtil now return normalized relative paths instead of absolute ones. This eliminates the unnecessary absolutize-then-relativize round-trip in ApprovedFileInventory. InventoryEntry uses Path instead of String for relativePath, giving proper path comparison semantics. --- .../approve/ApprovedFileInventory.java | 61 ++++++++-------- .../org/approvej/approve/FileApprover.java | 2 +- .../org/approvej/approve/InventoryEntry.java | 5 +- .../org/approvej/approve/PathProvider.java | 6 +- .../approve/StackTraceTestFinderUtil.java | 1 - .../StackTraceTestFinderUtilSpockSpec.groovy | 6 +- .../approve/ApprovedFileInventoryTest.java | 71 +++++++++++-------- .../approvej/approve/PathProviderTest.java | 46 +++--------- .../approvej/approve/PathProvidersTest.java | 15 +--- .../approve/StackTraceTestFinderUtilTest.java | 6 +- .../StackTraceTestFinderUtilTestNgTest.java | 6 +- 11 files changed, 97 insertions(+), 128 deletions(-) diff --git a/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java index cc0c089f..0a37d390 100644 --- a/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java +++ b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java @@ -14,6 +14,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicReference; import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; /** * Tracks approved files in an inventory so that leftover files (from renamed or deleted tests) can @@ -30,10 +31,10 @@ public class ApprovedFileInventory { private static final String HEADER = "# ApproveJ Approved File Inventory (auto-generated, do not edit)"; - private static final ConcurrentHashMap entries = new ConcurrentHashMap<>(); + private static final ConcurrentHashMap entries = new ConcurrentHashMap<>(); private static final ConcurrentHashMap executedMethods = new ConcurrentHashMap<>(); - private static final AtomicReference shutdownHook = new AtomicReference<>(); + private static final AtomicReference<@Nullable Thread> shutdownHook = new AtomicReference<>(); private static volatile Path inventoryFile = DEFAULT_INVENTORY_FILE; @@ -54,16 +55,8 @@ public static void registerApprovedFile(PathProvider pathProvider) { String testReference = "%s#%s".formatted(testMethod.testClass().getName(), testMethod.testCaseName()); - Path cwd = Path.of("").toAbsolutePath(); - Path approvedPath = pathProvider.approvedPath().toAbsolutePath(); - String relativePath; - try { - relativePath = cwd.relativize(approvedPath).toString(); - } catch (IllegalArgumentException e) { - relativePath = approvedPath.toString(); - } - addEntry(relativePath, testReference); + addEntry(pathProvider.approvedPath(), testReference); Thread hook = new Thread(ApprovedFileInventory::writeInventory, "ApproveJ-Inventory-Writer"); if (shutdownHook.compareAndSet(null, hook)) { @@ -72,9 +65,11 @@ public static void registerApprovedFile(PathProvider pathProvider) { } static void writeInventory() { - TreeMap merged = loadInventory(); + TreeMap merged = loadInventory(); - merged.entrySet().removeIf(entry -> executedMethods.containsKey(entry.getValue())); + merged + .entrySet() + .removeIf(entry -> executedMethods.containsKey(entry.getValue().testReference())); merged.putAll(entries); saveInventory(merged); @@ -86,10 +81,7 @@ static void writeInventory() { * @return a list of leftover inventory entries */ static List findLeftovers() { - return loadInventory().entrySet().stream() - .map(entry -> new InventoryEntry(entry.getKey(), entry.getValue())) - .filter(ApprovedFileInventory::isLeftover) - .toList(); + return loadInventory().values().stream().filter(ApprovedFileInventory::isLeftover).toList(); } private static boolean isLeftover(InventoryEntry entry) { @@ -112,11 +104,11 @@ static List removeLeftovers() { return leftovers; } - TreeMap inventory = loadInventory(); + TreeMap inventory = loadInventory(); List removed = new ArrayList<>(); for (InventoryEntry leftover : leftovers) { try { - Files.deleteIfExists(Path.of(leftover.relativePath())); + Files.deleteIfExists(leftover.relativePath()); inventory.remove(leftover.relativePath()); removed.add(leftover); } catch (IOException e) { @@ -129,8 +121,8 @@ static List removeLeftovers() { return removed; } - static TreeMap loadInventory() { - TreeMap result = new TreeMap<>(); + static TreeMap loadInventory() { + TreeMap result = new TreeMap<>(); if (!Files.exists(inventoryFile)) { return result; } @@ -141,11 +133,17 @@ static TreeMap loadInventory() { System.err.printf("Failed to read inventory file: %s%n", e.getMessage()); return result; } - properties.stringPropertyNames().forEach(key -> result.put(key, properties.getProperty(key))); + properties + .stringPropertyNames() + .forEach( + key -> { + Path path = Path.of(key); + result.put(path, new InventoryEntry(path, properties.getProperty(key))); + }); return result; } - private static void saveInventory(TreeMap inventory) { + private static void saveInventory(TreeMap inventory) { try { if (inventory.isEmpty()) { Files.deleteIfExists(inventoryFile); @@ -155,8 +153,12 @@ private static void saveInventory(TreeMap inventory) { "%s%n%s" .formatted( HEADER, - inventory.entrySet().stream() - .map(e -> "%s = %s".formatted(escapeKey(e.getKey()), e.getValue())) + inventory.values().stream() + .map( + e -> + "%s = %s" + .formatted( + escapeKey(e.relativePath().toString()), e.testReference())) .collect(joining("\n", "", "\n"))); Files.writeString(inventoryFile, content); } @@ -170,8 +172,8 @@ private static String escapeKey(String key) { } /** Adds an entry directly. For testing only. */ - static void addEntry(String relativePath, String testReference) { - entries.put(relativePath, testReference); + static void addEntry(Path relativePath, String testReference) { + entries.put(relativePath, new InventoryEntry(relativePath, testReference)); executedMethods.put(testReference, Boolean.TRUE); } @@ -219,7 +221,7 @@ public static void main(String[] args) { leftover -> System.out.printf( " %s%n from %s%n", - Path.of(leftover.relativePath()).toUri(), leftover.testReference())); + leftover.relativePath().toUri(), leftover.testReference())); } } case "--remove" -> { @@ -228,8 +230,7 @@ public static void main(String[] args) { System.out.println("No leftover approved files found."); } else { System.out.println("Removed leftover approved files:"); - removed.forEach( - leftover -> System.out.printf(" %s%n", Path.of(leftover.relativePath()).toUri())); + removed.forEach(leftover -> System.out.printf(" %s%n", leftover.relativePath().toUri())); } } default -> { diff --git a/modules/core/src/main/java/org/approvej/approve/FileApprover.java b/modules/core/src/main/java/org/approvej/approve/FileApprover.java index f0997f22..2351dc6c 100644 --- a/modules/core/src/main/java/org/approvej/approve/FileApprover.java +++ b/modules/core/src/main/java/org/approvej/approve/FileApprover.java @@ -77,7 +77,7 @@ private void handleOldApprovedFiles() { paths .filter( path -> - !path.toAbsolutePath().equals(approvedPath) + !path.normalize().equals(approvedPath) && baseFilenamePattern.matcher(path.getFileName().toString()).matches()) .sorted( comparing( diff --git a/modules/core/src/main/java/org/approvej/approve/InventoryEntry.java b/modules/core/src/main/java/org/approvej/approve/InventoryEntry.java index 27f6bc0b..52356ee7 100644 --- a/modules/core/src/main/java/org/approvej/approve/InventoryEntry.java +++ b/modules/core/src/main/java/org/approvej/approve/InventoryEntry.java @@ -1,12 +1,13 @@ package org.approvej.approve; +import java.nio.file.Path; import org.jspecify.annotations.NullMarked; /** An entry in the approved file inventory, mapping a file path to its originating test method. */ @NullMarked -record InventoryEntry(String relativePath, String className, String methodName) { +record InventoryEntry(Path relativePath, String className, String methodName) { - InventoryEntry(String relativePath, String testReference) { + InventoryEntry(Path relativePath, String testReference) { this(relativePath, parseClassName(testReference), parseMethodName(testReference)); } diff --git a/modules/core/src/main/java/org/approvej/approve/PathProvider.java b/modules/core/src/main/java/org/approvej/approve/PathProvider.java index c65172b0..eabb2ed5 100644 --- a/modules/core/src/main/java/org/approvej/approve/PathProvider.java +++ b/modules/core/src/main/java/org/approvej/approve/PathProvider.java @@ -82,7 +82,7 @@ public PathProvider filenameExtension(String filenameExtension) { * {@link #baseFilename}, followed by {@link #approvedLabel} (if any), followed by the {@link * #filenameExtension} (if any). * - * @return the absolute and normalized {@link Path} to the approved file + * @return the normalized {@link Path} to the approved file */ public Path approvedPath() { return directory @@ -93,7 +93,6 @@ public Path approvedPath() { filenameAffix.isBlank() ? "" : "-%s".formatted(filenameAffix), approvedLabel.isBlank() ? "" : "-%s".formatted(approvedLabel), filenameExtension.isBlank() ? "" : ".%s".formatted(filenameExtension))) - .toAbsolutePath() .normalize(); } @@ -102,7 +101,7 @@ public Path approvedPath() { * {@link #baseFilename}, followed by {@value RECEIVED} (if any), followed by the {@link * #filenameExtension} (if any). * - * @return the absolute and normalized {@link Path} to the received file + * @return the normalized {@link Path} to the received file */ public Path receivedPath() { return directory @@ -113,7 +112,6 @@ public Path receivedPath() { filenameAffix.isBlank() ? "" : "-%s".formatted(filenameAffix), "-%s".formatted(RECEIVED), filenameExtension.isBlank() ? "" : ".%s".formatted(filenameExtension))) - .toAbsolutePath() .normalize(); } } diff --git a/modules/core/src/main/java/org/approvej/approve/StackTraceTestFinderUtil.java b/modules/core/src/main/java/org/approvej/approve/StackTraceTestFinderUtil.java index 9c09bd8b..80186037 100644 --- a/modules/core/src/main/java/org/approvej/approve/StackTraceTestFinderUtil.java +++ b/modules/core/src/main/java/org/approvej/approve/StackTraceTestFinderUtil.java @@ -77,7 +77,6 @@ public static Path findTestSourcePath(Method testMethod) { attributes.isRegularFile() && path.normalize().toString().matches(pathRegex))) { return pathStream .findFirst() - .map(Path::toAbsolutePath) .map(Path::normalize) .orElseThrow(() -> new FileApproverError("Could not locate test source file")); } catch (IOException e) { diff --git a/modules/core/src/spock/groovy/org/approvej/approve/StackTraceTestFinderUtilSpockSpec.groovy b/modules/core/src/spock/groovy/org/approvej/approve/StackTraceTestFinderUtilSpockSpec.groovy index 06a4dda0..4b7934ac 100644 --- a/modules/core/src/spock/groovy/org/approvej/approve/StackTraceTestFinderUtilSpockSpec.groovy +++ b/modules/core/src/spock/groovy/org/approvej/approve/StackTraceTestFinderUtilSpockSpec.groovy @@ -32,7 +32,7 @@ class StackTraceTestFinderUtilSpockSpec extends Specification { StackTraceTestFinderUtil.currentTestMethod().method()) then: - testSourcePath == thisTestSourcePath.toAbsolutePath().normalize() + testSourcePath == thisTestSourcePath.normalize() } def 'findTestSourcePath_file_in_build'() { @@ -50,7 +50,7 @@ class StackTraceTestFinderUtilSpockSpec extends Specification { StackTraceTestFinderUtil.currentTestMethod().method()) then: - testSourcePath == thisTestSourcePath.toAbsolutePath().normalize() + testSourcePath == thisTestSourcePath.normalize() cleanup: delete(wrongTestSourcePath) @@ -69,7 +69,7 @@ class StackTraceTestFinderUtilSpockSpec extends Specification { Path testSourcePath = StackTraceTestFinderUtil.findTestSourcePath(StackTraceTestFinderUtil.currentTestMethod().method()) then: - testSourcePath == thisTestSourcePath.toAbsolutePath().normalize() + testSourcePath == thisTestSourcePath.normalize() cleanup: delete(wrongTestSourcePath) diff --git a/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java b/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java index 65aefc48..9a17fb5f 100644 --- a/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java +++ b/modules/core/src/test/java/org/approvej/approve/ApprovedFileInventoryTest.java @@ -33,41 +33,46 @@ void tearDown() { @Test void writeInventory() { ApprovedFileInventory.addEntry( - "src/test/MyTest-myTest-approved.txt", "com.example.MyTest#myTest"); + Path.of("src/test/MyTest-myTest-approved.txt"), "com.example.MyTest#myTest"); ApprovedFileInventory.writeInventory(); - TreeMap inventory = ApprovedFileInventory.loadInventory(); - assertThat(inventory) - .containsEntry("src/test/MyTest-myTest-approved.txt", "com.example.MyTest#myTest"); + TreeMap inventory = ApprovedFileInventory.loadInventory(); + assertThat(inventory.values()) + .containsExactly( + new InventoryEntry( + Path.of("src/test/MyTest-myTest-approved.txt"), "com.example.MyTest#myTest")); } @Test void writeInventory_multiple_entries() { - ApprovedFileInventory.addEntry("src/test/BTest-b-approved.txt", "com.example.BTest#b"); - ApprovedFileInventory.addEntry("src/test/ATest-a-approved.txt", "com.example.ATest#a"); + ApprovedFileInventory.addEntry(Path.of("src/test/BTest-b-approved.txt"), "com.example.BTest#b"); + ApprovedFileInventory.addEntry(Path.of("src/test/ATest-a-approved.txt"), "com.example.ATest#a"); ApprovedFileInventory.writeInventory(); - TreeMap inventory = ApprovedFileInventory.loadInventory(); + TreeMap inventory = ApprovedFileInventory.loadInventory(); assertThat(inventory).hasSize(2); - assertThat(inventory.firstKey()).isEqualTo("src/test/ATest-a-approved.txt"); + assertThat(inventory.firstKey()).isEqualTo(Path.of("src/test/ATest-a-approved.txt")); } @Test void writeInventory_named_approvals() { ApprovedFileInventory.addEntry( - "src/test/MyTest-myTest-alpha-approved.txt", "com.example.MyTest#myTest"); + Path.of("src/test/MyTest-myTest-alpha-approved.txt"), "com.example.MyTest#myTest"); ApprovedFileInventory.addEntry( - "src/test/MyTest-myTest-beta-approved.txt", "com.example.MyTest#myTest"); + Path.of("src/test/MyTest-myTest-beta-approved.txt"), "com.example.MyTest#myTest"); ApprovedFileInventory.writeInventory(); - TreeMap inventory = ApprovedFileInventory.loadInventory(); - assertThat(inventory) + TreeMap inventory = ApprovedFileInventory.loadInventory(); + assertThat(inventory.values()) .hasSize(2) - .containsEntry("src/test/MyTest-myTest-alpha-approved.txt", "com.example.MyTest#myTest") - .containsEntry("src/test/MyTest-myTest-beta-approved.txt", "com.example.MyTest#myTest"); + .contains( + new InventoryEntry( + Path.of("src/test/MyTest-myTest-alpha-approved.txt"), "com.example.MyTest#myTest"), + new InventoryEntry( + Path.of("src/test/MyTest-myTest-beta-approved.txt"), "com.example.MyTest#myTest")); } @Test @@ -82,18 +87,21 @@ void writeInventory_replaces_entries_for_executed_methods() throws IOException { StandardOpenOption.CREATE); ApprovedFileInventory.addEntry( - "src/test/MyTest-myTest-gamma-approved.txt", "com.example.MyTest#myTest"); + Path.of("src/test/MyTest-myTest-gamma-approved.txt"), "com.example.MyTest#myTest"); ApprovedFileInventory.addEntry( - "src/test/MyTest-myTest-beta-approved.txt", "com.example.MyTest#myTest"); + Path.of("src/test/MyTest-myTest-beta-approved.txt"), "com.example.MyTest#myTest"); ApprovedFileInventory.writeInventory(); - TreeMap inventory = ApprovedFileInventory.loadInventory(); - assertThat(inventory) + TreeMap inventory = ApprovedFileInventory.loadInventory(); + assertThat(inventory.values()) .hasSize(2) - .containsEntry("src/test/MyTest-myTest-gamma-approved.txt", "com.example.MyTest#myTest") - .containsEntry("src/test/MyTest-myTest-beta-approved.txt", "com.example.MyTest#myTest") - .doesNotContainKey("src/test/MyTest-myTest-alpha-approved.txt"); + .contains( + new InventoryEntry( + Path.of("src/test/MyTest-myTest-gamma-approved.txt"), "com.example.MyTest#myTest"), + new InventoryEntry( + Path.of("src/test/MyTest-myTest-beta-approved.txt"), "com.example.MyTest#myTest")); + assertThat(inventory).doesNotContainKey(Path.of("src/test/MyTest-myTest-alpha-approved.txt")); } @Test @@ -107,15 +115,18 @@ void writeInventory_preserves_entries_for_unexecuted_methods() throws IOExceptio StandardOpenOption.CREATE); ApprovedFileInventory.addEntry( - "src/test/MyTest-myTest-approved.txt", "com.example.MyTest#myTest"); + Path.of("src/test/MyTest-myTest-approved.txt"), "com.example.MyTest#myTest"); ApprovedFileInventory.writeInventory(); - TreeMap inventory = ApprovedFileInventory.loadInventory(); - assertThat(inventory) + TreeMap inventory = ApprovedFileInventory.loadInventory(); + assertThat(inventory.values()) .hasSize(2) - .containsEntry("src/test/OtherTest-other-approved.txt", "com.example.OtherTest#other") - .containsEntry("src/test/MyTest-myTest-approved.txt", "com.example.MyTest#myTest"); + .contains( + new InventoryEntry( + Path.of("src/test/OtherTest-other-approved.txt"), "com.example.OtherTest#other"), + new InventoryEntry( + Path.of("src/test/MyTest-myTest-approved.txt"), "com.example.MyTest#myTest")); } @Test @@ -132,7 +143,7 @@ void findLeftovers() throws IOException { assertThat(leftovers).hasSize(1); assertThat(leftovers.getFirst().relativePath()) - .isEqualTo("src/test/NonExistent-test-approved.txt"); + .isEqualTo(Path.of("src/test/NonExistent-test-approved.txt")); assertThat(leftovers.getFirst().testReference()) .isEqualTo("com.nonexistent.NonExistentTest#test"); } @@ -174,9 +185,7 @@ void removeLeftovers() throws IOException { assertThat(removed).hasSize(1); assertThat(leftoverFile).doesNotExist(); - TreeMap inventory = ApprovedFileInventory.loadInventory(); - assertThat(inventory) - .doesNotContainKey(leftoverFile.toString()) - .containsKey(validFile.toString()); + TreeMap inventory = ApprovedFileInventory.loadInventory(); + assertThat(inventory).doesNotContainKey(leftoverFile).containsKey(validFile); } } diff --git a/modules/core/src/test/java/org/approvej/approve/PathProviderTest.java b/modules/core/src/test/java/org/approvej/approve/PathProviderTest.java index 9db9dc76..d4b9c06d 100644 --- a/modules/core/src/test/java/org/approvej/approve/PathProviderTest.java +++ b/modules/core/src/test/java/org/approvej/approve/PathProviderTest.java @@ -16,13 +16,10 @@ void constructor_blank_approvedLabel() { assertThat(new PathProvider(Path.of("./src/test/resources/"), "base", "affix", "", "xml")) .hasFieldOrPropertyWithValue( "approvedPath", - Path.of("./src/test/resources/" + "base" + "-affix" + ".xml") - .toAbsolutePath() - .normalize()) + Path.of("./src/test/resources/" + "base" + "-affix" + ".xml").normalize()) .hasFieldOrPropertyWithValue( "receivedPath", Path.of("./src/test/resources/" + "base" + "-affix" + "-received" + ".xml") - .toAbsolutePath() .normalize()); } @@ -30,15 +27,10 @@ void constructor_blank_approvedLabel() { void directory() { assertThat(pathProvider.directory(Path.of("/tmp/"))) .hasFieldOrPropertyWithValue( - "approvedPath", - Path.of("/tmp/" + "base" + "-affix" + "-approved" + ".xml") - .toAbsolutePath() - .normalize()) + "approvedPath", Path.of("/tmp/" + "base" + "-affix" + "-approved" + ".xml").normalize()) .hasFieldOrPropertyWithValue( "receivedPath", - Path.of("/tmp/" + "base" + "-affix" + "-received" + ".xml") - .toAbsolutePath() - .normalize()); + Path.of("/tmp/" + "base" + "-affix" + "-received" + ".xml").normalize()); } @Test @@ -47,12 +39,10 @@ void filenameAffix() { .hasFieldOrPropertyWithValue( "approvedPath", Path.of("./src/test/resources/" + "base" + "-special" + "-approved" + ".xml") - .toAbsolutePath() .normalize()) .hasFieldOrPropertyWithValue( "receivedPath", Path.of("./src/test/resources/" + "base" + "-special" + "-received" + ".xml") - .toAbsolutePath() .normalize()); } @@ -61,14 +51,10 @@ void filenameAffix_blank() { assertThat(pathProvider.filenameAffix(" ")) .hasFieldOrPropertyWithValue( "approvedPath", - Path.of("./src/test/resources/" + "base" + "-approved" + ".xml") - .toAbsolutePath() - .normalize()) + Path.of("./src/test/resources/" + "base" + "-approved" + ".xml").normalize()) .hasFieldOrPropertyWithValue( "receivedPath", - Path.of("./src/test/resources/" + "base" + "-received" + ".xml") - .toAbsolutePath() - .normalize()); + Path.of("./src/test/resources/" + "base" + "-received" + ".xml").normalize()); } @Test @@ -76,13 +62,10 @@ void filenameExtension() { assertThat(pathProvider.filenameExtension("yml")) .hasFieldOrPropertyWithValue( "approvedPath", - Path.of("./src/test/resources/" + "base" + "-affix" + "-approved" + ".yml") - .toAbsolutePath() - .normalize()) + Path.of("./src/test/resources/" + "base" + "-affix" + "-approved" + ".yml").normalize()) .hasFieldOrPropertyWithValue( "receivedPath", Path.of("./src/test/resources/" + "base" + "-affix" + "-received" + ".yml") - .toAbsolutePath() .normalize()); } @@ -91,14 +74,10 @@ void filenameExtension_blank() { assertThat(pathProvider.filenameExtension(" ")) .hasFieldOrPropertyWithValue( "approvedPath", - Path.of("./src/test/resources/" + "base" + "-affix" + "-approved") - .toAbsolutePath() - .normalize()) + Path.of("./src/test/resources/" + "base" + "-affix" + "-approved").normalize()) .hasFieldOrPropertyWithValue( "receivedPath", - Path.of("./src/test/resources/" + "base" + "-affix" + "-received") - .toAbsolutePath() - .normalize()); + Path.of("./src/test/resources/" + "base" + "-affix" + "-received").normalize()); } @Test @@ -114,7 +93,6 @@ void filenameExtension_default_blank() { + "-approved" + "." + DEFAULT_FILENAME_EXTENSION) - .toAbsolutePath() .normalize()) .hasFieldOrPropertyWithValue( "receivedPath", @@ -125,7 +103,6 @@ void filenameExtension_default_blank() { + "-received" + "." + DEFAULT_FILENAME_EXTENSION) - .toAbsolutePath() .normalize()); } @@ -136,13 +113,10 @@ void filenameExtension_default_already_set() { pathProviderFilenameExtensionAlreadySet.filenameExtension(DEFAULT_FILENAME_EXTENSION)) .hasFieldOrPropertyWithValue( "approvedPath", - Path.of("./src/test/resources/" + "base" + "-affix" + "-approved" + ".xml") - .toAbsolutePath() - .normalize()) + Path.of("./src/test/resources/" + "base" + "-affix" + "-approved" + ".xml").normalize()) .hasFieldOrPropertyWithValue( "receivedPath", Path.of("./src/test/resources/" + "base" + "-affix" + "-received" + ".xml") - .toAbsolutePath() .normalize()); } @@ -151,7 +125,6 @@ void approvedPath() { assertThat(pathProvider.approvedPath()) .isEqualTo( Path.of("./src/test/resources/" + "base" + "-affix" + "-approved" + ".xml") - .toAbsolutePath() .normalize()); } @@ -160,7 +133,6 @@ void receivedPath() { assertThat(pathProvider.receivedPath()) .isEqualTo( Path.of("./src/test/resources/" + "base" + "-affix" + "-received" + ".xml") - .toAbsolutePath() .normalize()); } } diff --git a/modules/core/src/test/java/org/approvej/approve/PathProvidersTest.java b/modules/core/src/test/java/org/approvej/approve/PathProvidersTest.java index 22fafe36..188a7aed 100644 --- a/modules/core/src/test/java/org/approvej/approve/PathProvidersTest.java +++ b/modules/core/src/test/java/org/approvej/approve/PathProvidersTest.java @@ -12,10 +12,9 @@ void approvedPath() { String giveApprovedPath = "./src/test/resources/some file"; PathProvider pathProvider = PathProviders.approvedPath(giveApprovedPath); - assertThat(pathProvider.approvedPath()) - .isEqualTo(Path.of(giveApprovedPath).toAbsolutePath().normalize()); + assertThat(pathProvider.approvedPath()).isEqualTo(Path.of(giveApprovedPath).normalize()); assertThat(pathProvider.receivedPath()) - .isEqualTo(Path.of("./src/test/resources/some file-received").toAbsolutePath().normalize()); + .isEqualTo(Path.of("./src/test/resources/some file-received").normalize()); } @Test @@ -29,7 +28,6 @@ void nextToTest() { + "PathProvidersTest" + "-nextToTest" + "-approved.txt") - .toAbsolutePath() .normalize()); assertThat(pathProvider.receivedPath()) .isEqualTo( @@ -38,7 +36,6 @@ void nextToTest() { + "PathProvidersTest" + "-nextToTest" + "-received.txt") - .toAbsolutePath() .normalize()); } @@ -52,7 +49,6 @@ void nextToTest_filenameExtension() { + "PathProvidersTest" + "-nextToTest_filenameExtension" + "-approved.json") - .toAbsolutePath() .normalize()); assertThat(pathProvider.receivedPath()) .isEqualTo( @@ -61,7 +57,6 @@ void nextToTest_filenameExtension() { + "PathProvidersTest" + "-nextToTest_filenameExtension" + "-received.json") - .toAbsolutePath() .normalize()); } @@ -75,7 +70,6 @@ void nextToTestInSubdirectory() { + "PathProvidersTest/" + "nextToTestInSubdirectory" + "-approved.txt") - .toAbsolutePath() .normalize()); assertThat(pathProvider.receivedPath()) .isEqualTo( @@ -84,7 +78,6 @@ void nextToTestInSubdirectory() { + "PathProvidersTest/" + "nextToTestInSubdirectory" + "-received.txt") - .toAbsolutePath() .normalize()); } @@ -96,13 +89,11 @@ void nextToTest_directory() { .isEqualTo( directory .resolve("PathProvidersTest" + "-nextToTest_directory" + "-approved.txt") - .toAbsolutePath() .normalize()); assertThat(pathProvider.receivedPath()) .isEqualTo( directory .resolve("PathProvidersTest" + "-nextToTest_directory" + "-received.txt") - .toAbsolutePath() .normalize()); } @@ -117,7 +108,6 @@ void nextToTest_filenameAffix() { + "-nextToTest_filenameAffix" + "-additional info" + "-approved.txt") - .toAbsolutePath() .normalize()); assertThat(pathProvider.receivedPath()) .isEqualTo( @@ -127,7 +117,6 @@ void nextToTest_filenameAffix() { + "-nextToTest_filenameAffix" + "-additional info" + "-received.txt") - .toAbsolutePath() .normalize()); } } diff --git a/modules/core/src/test/java/org/approvej/approve/StackTraceTestFinderUtilTest.java b/modules/core/src/test/java/org/approvej/approve/StackTraceTestFinderUtilTest.java index 190f62d9..1e9dd5e1 100644 --- a/modules/core/src/test/java/org/approvej/approve/StackTraceTestFinderUtilTest.java +++ b/modules/core/src/test/java/org/approvej/approve/StackTraceTestFinderUtilTest.java @@ -58,7 +58,7 @@ void findTestSourcePath() { StackTraceTestFinderUtil.findTestSourcePath( StackTraceTestFinderUtil.currentTestMethod().method()); - assertThat(testSourcePath).isEqualTo(thisTestSourcePath.toAbsolutePath().normalize()); + assertThat(testSourcePath).isEqualTo(thisTestSourcePath.normalize()); } @Test @@ -74,7 +74,7 @@ void findTestSourcePath_file_in_build() throws IOException { StackTraceTestFinderUtil.findTestSourcePath( StackTraceTestFinderUtil.currentTestMethod().method()); - assertThat(testSourcePath).isEqualTo(thisTestSourcePath.toAbsolutePath().normalize()); + assertThat(testSourcePath).isEqualTo(thisTestSourcePath.normalize()); } @Test @@ -90,6 +90,6 @@ void findTestSourcePath_file_in_target() throws IOException { StackTraceTestFinderUtil.findTestSourcePath( StackTraceTestFinderUtil.currentTestMethod().method()); - assertThat(testSourcePath).isEqualTo(thisTestSourcePath.toAbsolutePath().normalize()); + assertThat(testSourcePath).isEqualTo(thisTestSourcePath.normalize()); } } diff --git a/modules/core/src/testng/java/org/approvej/approve/StackTraceTestFinderUtilTestNgTest.java b/modules/core/src/testng/java/org/approvej/approve/StackTraceTestFinderUtilTestNgTest.java index 9903235f..401245ad 100644 --- a/modules/core/src/testng/java/org/approvej/approve/StackTraceTestFinderUtilTestNgTest.java +++ b/modules/core/src/testng/java/org/approvej/approve/StackTraceTestFinderUtilTestNgTest.java @@ -43,7 +43,7 @@ void findTestSourcePath() { StackTraceTestFinderUtil.findTestSourcePath( StackTraceTestFinderUtil.currentTestMethod().method()); - assertEquals(testSourcePath, thisTestSourcePath.toAbsolutePath().normalize()); + assertEquals(testSourcePath, thisTestSourcePath.normalize()); } @Test @@ -59,7 +59,7 @@ void findTestSourcePath_file_in_build() throws IOException { StackTraceTestFinderUtil.findTestSourcePath( StackTraceTestFinderUtil.currentTestMethod().method()); - assertEquals(testSourcePath, thisTestSourcePath.toAbsolutePath().normalize()); + assertEquals(testSourcePath, thisTestSourcePath.normalize()); } @Test @@ -75,6 +75,6 @@ void findTestSourcePath_file_in_target() throws IOException { StackTraceTestFinderUtil.findTestSourcePath( StackTraceTestFinderUtil.currentTestMethod().method()); - assertEquals(testSourcePath, thisTestSourcePath.toAbsolutePath().normalize()); + assertEquals(testSourcePath, thisTestSourcePath.normalize()); } } From c2d58bf6441f47068b8f7476ad13e63ddd32a93d Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Fri, 20 Feb 2026 19:26:18 +0100 Subject: [PATCH 24/25] Use formatted strings and List.of for clarity --- .../java/org/approvej/approve/InventoryEntry.java | 4 ++-- .../main/java/org/approvej/maven/MojoHelper.java | 15 +++++---------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/modules/core/src/main/java/org/approvej/approve/InventoryEntry.java b/modules/core/src/main/java/org/approvej/approve/InventoryEntry.java index 52356ee7..b25e1a28 100644 --- a/modules/core/src/main/java/org/approvej/approve/InventoryEntry.java +++ b/modules/core/src/main/java/org/approvej/approve/InventoryEntry.java @@ -12,14 +12,14 @@ record InventoryEntry(Path relativePath, String className, String methodName) { } String testReference() { - return className + "#" + methodName; + return "%s#%s".formatted(className, methodName); } private static String parseClassName(String testReference) { int hashIndex = testReference.indexOf('#'); if (hashIndex < 0) { throw new IllegalArgumentException( - "Invalid test reference (expected 'className#methodName'): " + testReference); + "Invalid test reference (expected 'className#methodName'): %s".formatted(testReference)); } return testReference.substring(0, hashIndex); } diff --git a/plugins/approvej-maven-plugin/src/main/java/org/approvej/maven/MojoHelper.java b/plugins/approvej-maven-plugin/src/main/java/org/approvej/maven/MojoHelper.java index 87ef3df2..3b286c58 100644 --- a/plugins/approvej-maven-plugin/src/main/java/org/approvej/maven/MojoHelper.java +++ b/plugins/approvej-maven-plugin/src/main/java/org/approvej/maven/MojoHelper.java @@ -1,10 +1,10 @@ package org.approvej.maven; import java.io.BufferedReader; +import java.io.File; import java.io.IOException; import java.io.InputStreamReader; import java.nio.file.Path; -import java.util.ArrayList; import java.util.List; import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugin.logging.Log; @@ -62,7 +62,8 @@ static void executeInventory(MavenProject project, String command, Log log) stdoutThread.join(); stderrThread.join(); if (exitCode != 0) { - throw new MojoExecutionException("ApprovedFileInventory exited with code " + exitCode); + throw new MojoExecutionException( + "ApprovedFileInventory exited with code %d".formatted(exitCode)); } } catch (IOException e) { throw new MojoExecutionException("Failed to execute ApprovedFileInventory", e); @@ -74,14 +75,8 @@ static void executeInventory(MavenProject project, String command, Log log) static List buildCommand(List classpathElements, String command) { String javaExecutable = Path.of(System.getProperty("java.home"), "bin", "java").toString(); - String classpath = String.join(System.getProperty("path.separator"), classpathElements); + String classpath = String.join(File.pathSeparator, classpathElements); - List cmd = new ArrayList<>(); - cmd.add(javaExecutable); - cmd.add("-cp"); - cmd.add(classpath); - cmd.add(MAIN_CLASS); - cmd.add(command); - return cmd; + return List.of(javaExecutable, "-cp", classpath, MAIN_CLASS, command); } } From 5a3ba5e71f52c77038aed47f24e135d6544fe0ae Mon Sep 17 00:00:00 2001 From: Michael Kutz Date: Fri, 20 Feb 2026 19:32:31 +0100 Subject: [PATCH 25/25] Replace volatile with AtomicReference for inventoryFile --- .../approvej/approve/ApprovedFileInventory.java | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java index 0a37d390..eb3b23b9 100644 --- a/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java +++ b/modules/core/src/main/java/org/approvej/approve/ApprovedFileInventory.java @@ -36,7 +36,8 @@ public class ApprovedFileInventory { new ConcurrentHashMap<>(); private static final AtomicReference<@Nullable Thread> shutdownHook = new AtomicReference<>(); - private static volatile Path inventoryFile = DEFAULT_INVENTORY_FILE; + private static final AtomicReference inventoryFile = + new AtomicReference<>(DEFAULT_INVENTORY_FILE); private ApprovedFileInventory() {} @@ -123,11 +124,12 @@ static List removeLeftovers() { static TreeMap loadInventory() { TreeMap result = new TreeMap<>(); - if (!Files.exists(inventoryFile)) { + Path inventoryPath = inventoryFile.get(); + if (!Files.exists(inventoryPath)) { return result; } Properties properties = new Properties(); - try (BufferedReader reader = Files.newBufferedReader(inventoryFile)) { + try (BufferedReader reader = Files.newBufferedReader(inventoryPath)) { properties.load(reader); } catch (IOException e) { System.err.printf("Failed to read inventory file: %s%n", e.getMessage()); @@ -144,11 +146,12 @@ static TreeMap loadInventory() { } private static void saveInventory(TreeMap inventory) { + Path inventoryPath = inventoryFile.get(); try { if (inventory.isEmpty()) { - Files.deleteIfExists(inventoryFile); + Files.deleteIfExists(inventoryPath); } else { - Files.createDirectories(inventoryFile.getParent()); + Files.createDirectories(inventoryPath.getParent()); String content = "%s%n%s" .formatted( @@ -160,7 +163,7 @@ private static void saveInventory(TreeMap inventory) { .formatted( escapeKey(e.relativePath().toString()), e.testReference())) .collect(joining("\n", "", "\n"))); - Files.writeString(inventoryFile, content); + Files.writeString(inventoryPath, content); } } catch (IOException e) { System.err.printf("Failed to write inventory file: %s%n", e.getMessage()); @@ -189,7 +192,7 @@ static void reset(Path testInventoryFile) { // JVM is already shutting down } } - inventoryFile = testInventoryFile; + inventoryFile.set(testInventoryFile); } /** Resets static state to defaults. For testing only. */