From f4c66c59e56da1e250b1489b2234b1043e3231b3 Mon Sep 17 00:00:00 2001 From: Sebastian Gozin Date: Mon, 31 Aug 2026 19:12:48 +0200 Subject: [PATCH] Add --test-command, wire real coverage for it, fix locale-dependent formatting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit crap4java's coverage step hardcoded `mvn ... test ...`, which requires JUnit-discoverable tests. Projects that run tests through a dedicated runner instead of `mvn test` (as SwarmForge's own engineering constitution requires for Java projects) got 0% coverage for everything and inflated CRAP scores, since no tests ever actually ran. - Add --test-command , mirroring mutate4java's existing flag: runs instead of `mvn test`. Coverage is still real: the JaCoCo runtime agent is resolved once per module (mvn dependency:copy, cached under target/) and attached to via JAVA_TOOL_OPTIONS, so any JVM launches contributes coverage; the report goal runs afterward. - CommandExecutor grows a runShell(cmd, dir, env) default method so existing lambda-based test doubles keep compiling unchanged. - Fix ReportFormatter and CliApplication's threshold message: %f formatting used the JVM default locale, rendering e.g. "85,0%" under non-English locales instead of "85.0%". Pinned to Locale.ROOT. - Update spec.md (§4.4, §7.2.1) and README to document the new flag. --- README.md | 11 ++++ spec.md | 18 ++++++ src/crap4java/CliApplication.java | 9 +-- src/crap4java/CliArguments.java | 2 +- src/crap4java/CliArgumentsParser.java | 45 ++++++++++--- src/crap4java/CommandExecutor.java | 9 +++ src/crap4java/CoverageRunner.java | 57 ++++++++++++++--- src/crap4java/Main.java | 8 +++ src/crap4java/ProcessCommandExecutor.java | 18 ++++-- src/crap4java/ReportFormatter.java | 9 +-- test/crap4java/CliArgumentsParserTest.java | 35 +++++++++++ test/crap4java/CoverageRunnerTest.java | 73 +++++++++++++++++++++- 12 files changed, 264 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 90d5e35..5713b2e 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,15 @@ For each invocation: 3. Read `target/site/jacoco/jacoco.xml` 4. Analyze selected Java files +### Projects That Don't Run Tests Through Maven + +Pass `--test-command ` to run `` instead of step 2's `mvn ... test`. This +is for projects whose tests aren't run via `mvn test` (a dedicated test runner +invoked directly with `java`, for example). `crap4java` still resolves and attaches +the JaCoCo runtime agent to `` via `JAVA_TOOL_OPTIONS` (so any JVM it launches +contributes real coverage, not just an `mvn test`-driven one), then runs the JaCoCo +`report` goal afterward. ``'s exit code still determines pass/fail. + ## Build and Test ```bash @@ -51,6 +60,7 @@ java -jar target/crap4java-0.1.0-SNAPSHOT.jar --changed Analyze changed Java files under src/ Analyze only these files Analyze all Java files under each directory's src/ subtree +--test-command CMD Run CMD instead of `mvn test` (combinable with the forms above) ``` Examples: @@ -61,6 +71,7 @@ java -jar target/crap4java-0.1.0-SNAPSHOT.jar java -jar target/crap4java-0.1.0-SNAPSHOT.jar --changed java -jar target/crap4java-0.1.0-SNAPSHOT.jar src/main/java/demo/Sample.java java -jar target/crap4java-0.1.0-SNAPSHOT.jar module-a module-b +java -jar target/crap4java-0.1.0-SNAPSHOT.jar --test-command "scripts/run-unit-tests.sh" ``` ## Exit codes diff --git a/spec.md b/spec.md index 2234226..0293fb4 100644 --- a/spec.md +++ b/spec.md @@ -80,6 +80,14 @@ The tool shall exit with usage error when argument parsing fails. The tool shall print usage text on CLI usage failure. +### 4.4 Test Command Override + +The tool shall support an optional `--test-command ` flag, combinable with any form in §4.1 except `--help`. + +When present, `` replaces the test-execution step described in §7.2 as the mechanism that exercises the module's tests. Coverage instrumentation and JaCoCo report generation remain Maven-based; only test execution itself is substituted. This exists for projects whose tests are not run through `mvn test` (see §7.2.1). + +`--test-command` shall require a value; omitting one is a usage error per §4.3. + ## 5. File Selection Rules ### 5.1 Default Source Discovery @@ -146,6 +154,16 @@ Before coverage generation, the tool shall delete stale module-local coverage ar Coverage generation shall invoke Maven against the module root and generate JaCoCo XML for that module. +#### 7.2.1 Overridden Test Command + +When `--test-command` (§4.4) is supplied, the tool shall: + +1. resolve the JaCoCo runtime agent jar for the module (caching it under the module's `target/` so repeated runs do not re-resolve it) +2. run `` with the JaCoCo agent attached via the `JAVA_TOOL_OPTIONS` environment variable, so any JVM `` launches contributes coverage +3. invoke Maven's JaCoCo report goal against the module root to produce the JaCoCo XML report + +A non-zero exit from `` shall fail the run per §14 before the report goal runs. + ### 7.3 Missing Coverage XML If the expected JaCoCo XML file does not exist after coverage generation: diff --git a/src/crap4java/CliApplication.java b/src/crap4java/CliApplication.java index c8f5ab7..196887d 100644 --- a/src/crap4java/CliApplication.java +++ b/src/crap4java/CliApplication.java @@ -8,6 +8,7 @@ import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; @@ -37,25 +38,25 @@ int execute(String[] args) throws Exception { return 0; } - List metrics = analyzeByModule(filesToAnalyze); + List metrics = analyzeByModule(filesToAnalyze, parsed.testCommand()); metrics.sort(Comparator.comparing(MethodMetrics::crapScore, Comparator.nullsLast(Comparator.reverseOrder()))); out.print(ReportFormatter.format(metrics)); double max = Main.maxCrap(metrics); if (thresholdExceeded(max)) { - err.printf("CRAP threshold exceeded: %.1f > 8.0%n", max); + err.printf(Locale.ROOT, "CRAP threshold exceeded: %.1f > 8.0%n", max); return 2; } return 0; } - private List analyzeByModule(List filesToAnalyze) throws Exception { + private List analyzeByModule(List filesToAnalyze, String testCommand) throws Exception { List metrics = new ArrayList<>(); for (Map.Entry> entry : groupByModuleRoot(filesToAnalyze).entrySet()) { Path moduleRoot = entry.getKey(); Path jacocoXml = moduleRoot.resolve("target/site/jacoco/jacoco.xml"); - coverageRunner.generateCoverage(moduleRoot); + coverageRunner.generateCoverage(moduleRoot, testCommand); if (!Files.exists(jacocoXml)) { err.println("Warning: JaCoCo XML not found at " + jacocoXml + ". Coverage will be N/A."); } diff --git a/src/crap4java/CliArguments.java b/src/crap4java/CliArguments.java index e710ee6..9144cbb 100644 --- a/src/crap4java/CliArguments.java +++ b/src/crap4java/CliArguments.java @@ -2,7 +2,7 @@ import java.util.List; -record CliArguments(CliMode mode, List fileArgs) { +record CliArguments(CliMode mode, List fileArgs, String testCommand) { } /* mutate4java-manifest diff --git a/src/crap4java/CliArgumentsParser.java b/src/crap4java/CliArgumentsParser.java index 30d640d..6977216 100644 --- a/src/crap4java/CliArgumentsParser.java +++ b/src/crap4java/CliArgumentsParser.java @@ -8,22 +8,53 @@ final class CliArgumentsParser { private CliArgumentsParser() { } - static CliArguments parse(String[] args) { - if (args.length == 0) { - return new CliArguments(CliMode.ALL_SRC, List.of()); + static CliArguments parse(String[] rawArgs) { + if (rawArgs.length == 0) { + return new CliArguments(CliMode.ALL_SRC, List.of(), null); } - if (containsFlag(args, "--help")) { - return new CliArguments(CliMode.HELP, List.of()); + if (containsFlag(rawArgs, "--help")) { + return new CliArguments(CliMode.HELP, List.of(), null); + } + + String testCommand = extractTestCommand(rawArgs); + String[] args = withoutTestCommand(rawArgs); + + if (args.length == 0) { + return new CliArguments(CliMode.ALL_SRC, List.of(), testCommand); } boolean changed = containsFlag(args, "--changed"); List values = nonFlagArgs(args); ensureChangedIsNotCombined(changed, values); if (changed) { - return new CliArguments(CliMode.CHANGED_SRC, List.of()); + return new CliArguments(CliMode.CHANGED_SRC, List.of(), testCommand); + } + return new CliArguments(CliMode.EXPLICIT_FILES, List.copyOf(values), testCommand); + } + + private static String extractTestCommand(String[] args) { + for (int i = 0; i < args.length; i++) { + if ("--test-command".equals(args[i])) { + if (i + 1 >= args.length) { + throw new IllegalArgumentException("--test-command requires a value"); + } + return args[i + 1]; + } + } + return null; + } + + private static String[] withoutTestCommand(String[] args) { + List remaining = new ArrayList<>(); + for (int i = 0; i < args.length; i++) { + if ("--test-command".equals(args[i])) { + i++; + continue; + } + remaining.add(args[i]); } - return new CliArguments(CliMode.EXPLICIT_FILES, List.copyOf(values)); + return remaining.toArray(new String[0]); } private static boolean containsFlag(String[] args, String flag) { diff --git a/src/crap4java/CommandExecutor.java b/src/crap4java/CommandExecutor.java index b499869..fe5e4e3 100644 --- a/src/crap4java/CommandExecutor.java +++ b/src/crap4java/CommandExecutor.java @@ -2,9 +2,18 @@ import java.nio.file.Path; import java.util.List; +import java.util.Map; interface CommandExecutor { int run(List command, Path directory) throws Exception; + + default int runShell(String commandText, Path directory) throws Exception { + return runShell(commandText, directory, Map.of()); + } + + default int runShell(String commandText, Path directory, Map extraEnv) throws Exception { + return run(List.of("/bin/sh", "-lc", commandText), directory); + } } /* mutate4java-manifest diff --git a/src/crap4java/CoverageRunner.java b/src/crap4java/CoverageRunner.java index 6a21041..7209f67 100644 --- a/src/crap4java/CoverageRunner.java +++ b/src/crap4java/CoverageRunner.java @@ -5,28 +5,71 @@ import java.nio.file.Path; import java.util.Comparator; import java.util.List; +import java.util.Map; final class CoverageRunner { + private static final String JACOCO_VERSION = "0.8.12"; + private final CommandExecutor executor; CoverageRunner(CommandExecutor executor) { this.executor = executor; } - void generateCoverage(Path projectRoot) throws Exception { + void generateCoverage(Path projectRoot, String testCommand) throws Exception { deleteIfExists(projectRoot.resolve("target/site/jacoco")); deleteIfExists(projectRoot.resolve("target/jacoco.exec")); - int exit = executor.run(List.of( - "mvn", "-q", - "org.jacoco:jacoco-maven-plugin:0.8.12:prepare-agent", - "test", - "org.jacoco:jacoco-maven-plugin:0.8.12:report" - ), projectRoot); + if (testCommand == null) { + run(List.of( + "mvn", "-q", + "org.jacoco:jacoco-maven-plugin:" + JACOCO_VERSION + ":prepare-agent", + "test", + "org.jacoco:jacoco-maven-plugin:" + JACOCO_VERSION + ":report" + ), projectRoot, "Coverage command failed with exit "); + return; + } + + runWithCustomTestCommand(projectRoot, testCommand); + } + + private void runWithCustomTestCommand(Path projectRoot, String testCommand) throws Exception { + Path agentJar = resolveJacocoAgentJar(projectRoot); + Path destFile = projectRoot.resolve("target/jacoco.exec").toAbsolutePath(); + Map env = Map.of("JAVA_TOOL_OPTIONS", + "-javaagent:" + agentJar + "=destfile=" + destFile + ",append=true"); + + int exit = executor.runShell(testCommand, projectRoot, env); if (exit != 0) { throw new IllegalStateException("Coverage command failed with exit " + exit); } + + run(List.of("mvn", "-q", "org.jacoco:jacoco-maven-plugin:" + JACOCO_VERSION + ":report"), + projectRoot, "Coverage report command failed with exit "); + } + + private Path resolveJacocoAgentJar(Path projectRoot) throws Exception { + Path agentJar = projectRoot.resolve("target/jacoco-agent/org.jacoco.agent-runtime.jar"); + if (Files.exists(agentJar)) { + return agentJar.toAbsolutePath(); + } + run(List.of("mvn", "-q", "dependency:copy", + "-Dartifact=org.jacoco:org.jacoco.agent:" + JACOCO_VERSION + ":jar:runtime", + "-DoutputDirectory=target/jacoco-agent", + "-Dmdep.stripVersion=true" + ), projectRoot, "Unable to resolve the JaCoCo agent jar, exit "); + if (!Files.exists(agentJar)) { + throw new IllegalStateException("JaCoCo agent jar not found after resolution: " + agentJar); + } + return agentJar.toAbsolutePath(); + } + + private void run(List command, Path directory, String failureMessage) throws Exception { + int exit = executor.run(command, directory); + if (exit != 0) { + throw new IllegalStateException(failureMessage + exit); + } } private void deleteIfExists(Path path) throws IOException { diff --git a/src/crap4java/Main.java b/src/crap4java/Main.java index 132c61a..1e69c53 100644 --- a/src/crap4java/Main.java +++ b/src/crap4java/Main.java @@ -32,6 +32,14 @@ static String usage() { crap4java --changed Analyze changed Java files under src/ crap4java Analyze files, or for directory args analyze /src/**/*.java crap4java --help Print this help message + + Options: + --test-command CMD Run CMD instead of the default `mvn test` coverage + command, for projects that don't run tests through + Maven. CMD runs with the JaCoCo agent attached via + JAVA_TOOL_OPTIONS, so any JVM it launches (including + a custom test runner) still contributes coverage; + CMD's exit code determines pass/fail. """; } diff --git a/src/crap4java/ProcessCommandExecutor.java b/src/crap4java/ProcessCommandExecutor.java index 7fdf3b7..ed28579 100644 --- a/src/crap4java/ProcessCommandExecutor.java +++ b/src/crap4java/ProcessCommandExecutor.java @@ -2,16 +2,26 @@ import java.nio.file.Path; import java.util.List; +import java.util.Map; final class ProcessCommandExecutor implements CommandExecutor { @Override public int run(List command, Path directory) throws Exception { - Process process = new ProcessBuilder(command) + return start(command, directory, Map.of()).waitFor(); + } + + @Override + public int runShell(String commandText, Path directory, Map extraEnv) throws Exception { + return start(List.of("/bin/sh", "-lc", commandText), directory, extraEnv).waitFor(); + } + + private Process start(List command, Path directory, Map extraEnv) throws Exception { + ProcessBuilder builder = new ProcessBuilder(command) .directory(directory.toFile()) - .inheritIO() - .start(); - return process.waitFor(); + .inheritIO(); + builder.environment().putAll(extraEnv); + return builder.start(); } } diff --git a/src/crap4java/ReportFormatter.java b/src/crap4java/ReportFormatter.java index 53c5613..47c6520 100644 --- a/src/crap4java/ReportFormatter.java +++ b/src/crap4java/ReportFormatter.java @@ -3,6 +3,7 @@ import java.util.ArrayList; import java.util.Comparator; import java.util.List; +import java.util.Locale; final class ReportFormatter { @@ -15,7 +16,7 @@ static String format(List entries) { .comparing((MethodMetrics e) -> e.crapScore() == null) .thenComparing(e -> e.crapScore() == null ? 0.0 : -e.crapScore())); - String header = String.format("%-30s %-35s %4s %7s %8s", "Method", "Class", "CC", "Cov%", "CRAP"); + String header = String.format(Locale.ROOT, "%-30s %-35s %4s %7s %8s", "Method", "Class", "CC", "Cov%", "CRAP"); String separator = "-".repeat(header.length()); StringBuilder builder = new StringBuilder(); builder.append("CRAP Report\n"); @@ -24,7 +25,7 @@ static String format(List entries) { builder.append(separator).append('\n'); for (MethodMetrics entry : sorted) { - builder.append(String.format("%-30s %-35s %4d %7s %8s%n", + builder.append(String.format(Locale.ROOT, "%-30s %-35s %4d %7s %8s%n", entry.methodName(), entry.className(), entry.complexity(), @@ -39,14 +40,14 @@ private static String formatCoverage(Double coverage) { if (coverage == null) { return " N/A "; } - return String.format("%5.1f%%", coverage); + return String.format(Locale.ROOT, "%5.1f%%", coverage); } private static String formatCrap(Double score) { if (score == null) { return " N/A"; } - return String.format("%8.1f", score); + return String.format(Locale.ROOT, "%8.1f", score); } } diff --git a/test/crap4java/CliArgumentsParserTest.java b/test/crap4java/CliArgumentsParserTest.java index a8fe96d..03bc81f 100644 --- a/test/crap4java/CliArgumentsParserTest.java +++ b/test/crap4java/CliArgumentsParserTest.java @@ -55,4 +55,39 @@ void plainFilesDoNotTriggerChangedMode() { assertEquals(CliMode.EXPLICIT_FILES, args.mode()); assertEquals(List.of("src/main/java/demo/A.java"), args.fileArgs()); } + + @Test + void noTestCommandByDefault() { + CliArguments args = CliArgumentsParser.parse(new String[]{}); + assertEquals(null, args.testCommand()); + } + + @Test + void testCommandFlagCapturesItsValue() { + CliArguments args = CliArgumentsParser.parse( + new String[]{"--test-command", "scripts/run-unit-tests.sh"}); + + assertEquals("scripts/run-unit-tests.sh", args.testCommand()); + assertEquals(CliMode.ALL_SRC, args.mode()); + } + + @Test + void testCommandCanCombineWithChangedAndExplicitFiles() { + CliArguments changed = CliArgumentsParser.parse( + new String[]{"--changed", "--test-command", "scripts/run-unit-tests.sh"}); + assertEquals(CliMode.CHANGED_SRC, changed.mode()); + assertEquals("scripts/run-unit-tests.sh", changed.testCommand()); + + CliArguments explicit = CliArgumentsParser.parse( + new String[]{"src/main/java/demo/A.java", "--test-command", "scripts/run-unit-tests.sh"}); + assertEquals(CliMode.EXPLICIT_FILES, explicit.mode()); + assertEquals(List.of("src/main/java/demo/A.java"), explicit.fileArgs()); + assertEquals("scripts/run-unit-tests.sh", explicit.testCommand()); + } + + @Test + void testCommandRequiresAValue() { + assertThrows(IllegalArgumentException.class, + () -> CliArgumentsParser.parse(new String[]{"--test-command"})); + } } diff --git a/test/crap4java/CoverageRunnerTest.java b/test/crap4java/CoverageRunnerTest.java index ef9c7ae..e55be8b 100644 --- a/test/crap4java/CoverageRunnerTest.java +++ b/test/crap4java/CoverageRunnerTest.java @@ -7,10 +7,12 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.List; +import java.util.Map; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; class CoverageRunnerTest { @@ -29,7 +31,7 @@ void deletesStaleCoverageAndRunsMavenCoverageCommand() throws Exception { RecordingExecutor executor = new RecordingExecutor(0); CoverageRunner runner = new CoverageRunner(executor); - runner.generateCoverage(tempDir); + runner.generateCoverage(tempDir, null); assertFalse(Files.exists(jacocoDir)); assertFalse(Files.exists(exec)); @@ -48,24 +50,89 @@ void failsWhenCoverageCommandFails() { CoverageRunner runner = new CoverageRunner(executor); IllegalStateException ex = assertThrows(IllegalStateException.class, - () -> runner.generateCoverage(tempDir)); + () -> runner.generateCoverage(tempDir, null)); assertEquals("Coverage command failed with exit 2", ex.getMessage()); } + @Test + void testCommandRunsUnderTheJacocoAgentAndGeneratesAReport() throws Exception { + RecordingExecutor executor = new RecordingExecutor(0); + CoverageRunner runner = new CoverageRunner(executor); + + runner.generateCoverage(tempDir, "scripts/run-unit-tests.sh"); + + assertEquals("scripts/run-unit-tests.sh", executor.shellCommand); + assertEquals(tempDir, executor.shellDirectory); + String javaToolOptions = executor.shellEnv.get("JAVA_TOOL_OPTIONS"); + assertTrue(javaToolOptions.startsWith("-javaagent:"), javaToolOptions); + assertTrue(javaToolOptions.contains("org.jacoco.agent-runtime.jar"), javaToolOptions); + assertTrue(javaToolOptions.endsWith("=destfile=" + tempDir.resolve("target/jacoco.exec") + ",append=true"), + javaToolOptions); + + assertEquals(List.of("mvn", "-q", "dependency:copy", + "-Dartifact=org.jacoco:org.jacoco.agent:0.8.12:jar:runtime", + "-DoutputDirectory=target/jacoco-agent", + "-Dmdep.stripVersion=true"), executor.commands.get(0)); + assertEquals(List.of("mvn", "-q", "org.jacoco:jacoco-maven-plugin:0.8.12:report"), executor.commands.get(1)); + } + + @Test + void reusesAnAlreadyResolvedJacocoAgentJar() throws Exception { + Path agentJar = tempDir.resolve("target/jacoco-agent/org.jacoco.agent-runtime.jar"); + Files.createDirectories(agentJar.getParent()); + Files.writeString(agentJar, "already-resolved"); + + RecordingExecutor executor = new RecordingExecutor(0); + CoverageRunner runner = new CoverageRunner(executor); + + runner.generateCoverage(tempDir, "scripts/run-unit-tests.sh"); + + assertEquals(List.of("mvn", "-q", "org.jacoco:jacoco-maven-plugin:0.8.12:report"), executor.commands.get(0)); + } + + @Test + void failsWhenTestCommandFails() { + RecordingExecutor executor = new RecordingExecutor(1); + CoverageRunner runner = new CoverageRunner(executor); + + IllegalStateException ex = assertThrows(IllegalStateException.class, + () -> runner.generateCoverage(tempDir, "scripts/run-unit-tests.sh")); + + assertEquals("Coverage command failed with exit 1", ex.getMessage()); + assertEquals(1, executor.commands.size(), "the report goal must not run after a failed test command"); + } + private static final class RecordingExecutor implements CommandExecutor { private final int exitCode; private final List> commands = new ArrayList<>(); private final List directories = new ArrayList<>(); + private String shellCommand; + private Path shellDirectory; + private Map shellEnv; private RecordingExecutor(int exitCode) { this.exitCode = exitCode; } @Override - public int run(List command, Path directory) { + public int run(List command, Path directory) throws java.io.IOException { commands.add(command); directories.add(directory); + if (command.contains("dependency:copy")) { + Path agentJar = directory.resolve("target/jacoco-agent/org.jacoco.agent-runtime.jar"); + Files.createDirectories(agentJar.getParent()); + Files.writeString(agentJar, "fake-agent-jar"); + return 0; + } + return exitCode; + } + + @Override + public int runShell(String commandText, Path directory, Map extraEnv) { + shellCommand = commandText; + shellDirectory = directory; + shellEnv = extraEnv; return exitCode; } }