diff --git a/README.md b/README.md index ff9773e..2d7572d 100644 --- a/README.md +++ b/README.md @@ -88,6 +88,18 @@ That's it. With zero config, the plugin will: Every invocation resolves to exactly one **Situation** and exactly one **Action**, both of which appear in the log line so an operator can tell — at a glance — why the plugin chose what it did. +### Summary log format + +Every `affectedTest` run prints exactly one summary line in the form `Affected Tests: () —
`: + +``` +Affected Tests: SELECTED (DISCOVERY_SUCCESS) — 3 changed file(s), 2 production class(es), 5 test class(es) affected +Affected Tests: FULL_SUITE (UNMAPPED_FILE) — 1 changed file(s); running full suite (runAllOnNonJavaChange=true / onUnmappedFile=FULL_SUITE — non-Java or unmapped file in diff). +Affected Tests: SKIPPED (ALL_FILES_IGNORED) — 1 changed file(s); every changed file matched ignorePaths. +``` + +The outcome (`SELECTED` / `FULL_SUITE` / `SKIPPED`) and the situation that produced it are first-class fields on every branch, so CI dashboards can `grep -E '^Affected Tests: (SELECTED|FULL_SUITE|SKIPPED)'` and bucket runs without parsing the tail. Every pre-v2 phrase (`running full suite`, `runAllIfNoMatches=true`, `runAllOnNonJavaChange=true`, `no affected tests discovered`) still appears verbatim in the reason segment, so existing greps keep working. + ### Situations (what the engine saw) | Situation | Fires when | diff --git a/affected-tests-gradle/src/main/java/io/affectedtests/gradle/AffectedTestTask.java b/affected-tests-gradle/src/main/java/io/affectedtests/gradle/AffectedTestTask.java index 24ffec9..7f1a5e9 100644 --- a/affected-tests-gradle/src/main/java/io/affectedtests/gradle/AffectedTestTask.java +++ b/affected-tests-gradle/src/main/java/io/affectedtests/gradle/AffectedTestTask.java @@ -338,29 +338,33 @@ public void runAffectedTests() { return; } - // The summary line is the single place the task names the trigger; - // downstream lines must not repeat the reason or CI logs drift into - // contradictory duplicate phrasing. We hand the logger a format - // string + args pair instead of a pre-formatted string so any future - // phrase containing `{` or `}` characters (Liquibase file names, + // The summary line is the single place the task names the + // outcome (SELECTED / FULL_SUITE / SKIPPED), the situation that + // produced it, and the reason phrase. Downstream lines must not + // repeat any of those fields or CI logs drift into contradictory + // duplicate phrasing. We hand the logger a format string + args + // pair instead of a pre-formatted string so any future phrase + // containing `{` or `}` characters (Liquibase file names, // JSONpath expressions) renders literally rather than silently // disappearing into the placeholder parser. LogLine summary = renderSummary(result); getLogger().lifecycle(summary.format(), summary.args()); + // Skipped results return here without touching the executor. + // Intentionally no follow-up line — the summary already carried + // the {@code SKIPPED (SITUATION)} prefix and the reason phrase, + // so a separate "Skipping test execution (…)" log would only + // duplicate what the operator already read. if (result.skipped()) { - // Skipped is its own first-class outcome in v2: the engine - // deliberately chose to run no tests (e.g. api-test-only diff - // with onAllFilesOutOfScope=SKIPPED). Logging it distinctly - // from "selection empty" keeps the user's --explain expectation - // honest — "skipped (situation)" vs "no affected tests". - getLogger().lifecycle("Skipping test execution ({}: {}).", - result.situation(), result.action()); return; } if (result.testClassFqns().isEmpty() && !result.runAll()) { - getLogger().lifecycle("No affected tests to run. Skipping test execution."); + // Defensive fallthrough: the engine should have already + // routed this into {@code SKIPPED} via + // {@link AffectedTestsEngine.Situation#DISCOVERY_EMPTY}, + // but we keep the guard so a future regression surfaces as + // a log line instead of a silent no-op. return; } @@ -649,17 +653,6 @@ static String describeEscalation(EscalationReason reason) { }; } - /** - * Builds the single summary line printed to Gradle's lifecycle log for - * every {@code affectedTest} run. The runAll branch names the real - * trigger (so "0 production classes, 0 test classes affected" never - * sits next to "running full suite"), and non-runAll runs emit the - * selection counts the downstream test dispatch will honour. - * - *

Pluralisation is deliberately fixed to {@code file(s)}, - * {@code class(es)}, and {@code class(es)} on both branches so CI - * greps stay stable across runs with different selection sizes. - */ /** Cap on files listed per bucket in the {@code --explain} trace. */ static final int EXPLAIN_SAMPLE_LIMIT = 10; @@ -778,21 +771,90 @@ private static String describeSource(ActionSource source) { }; } + /** + * Builds the single summary line printed to Gradle's lifecycle log + * for every {@code affectedTest} run. Every branch names the + * outcome ({@link Action#SELECTED}, {@link Action#FULL_SUITE}, + * {@link Action#SKIPPED}) and the {@link Situation} that produced + * it, followed by the file count and branch-specific details. + * + *

Shape: + * 

+     * Affected Tests: SELECTED (DISCOVERY_SUCCESS) — N changed file(s), P production class(es), T test class(es) affected
+     * Affected Tests: FULL_SUITE (UNMAPPED_FILE) — N changed file(s); running full suite (reason)
+     * Affected Tests: SKIPPED (ALL_FILES_IGNORED) — N changed file(s); every changed file matched ignorePaths
+     *
+ * + *

Pluralisation is deliberately fixed to {@code file(s)} and + * {@code class(es)} across every branch so CI greps stay stable + * across runs with different selection sizes. Every phrase that + * pre-v2 or Phase-1 CI pipelines grep for ({@code "running full + * suite"}, {@code "runAllIfNoMatches=true"}, {@code "no affected + * tests discovered"}, etc.) survives verbatim — this change adds + * the outcome/situation prefix without removing any existing + * vocabulary. + */ static LogLine renderSummary(AffectedTestsResult result) { + String prefix = "Affected Tests: " + result.action().name() + + " (" + result.situation().name() + ") — "; if (result.runAll()) { + // runAll branch keeps the pre-v2 "running full suite (reason)" + // wording verbatim so existing CI greps for that substring + // continue to match every FULL_SUITE run. Reason phrase is + // sourced from describeEscalation to avoid duplicating the + // legacy vocabulary across two files. return new LogLine( - "Affected Tests: {} changed file(s); running full suite ({}).", + prefix + "{} changed file(s); running full suite ({}).", new Object[] { result.changedFiles().size(), describeEscalation(result.escalationReason()) }); } + if (result.skipped()) { + return new LogLine( + prefix + "{} changed file(s); {}.", + new Object[] { + result.changedFiles().size(), + describeSkipReason(result.situation()) + }); + } return new LogLine( - "Affected Tests: {} changed file(s), {} production class(es), {} test class(es) affected", + prefix + "{} changed file(s), {} production class(es), {} test class(es) affected", new Object[] { result.changedFiles().size(), result.changedProductionClasses().size(), result.testClassFqns().size() }); } + + /** + * Renders a {@link Situation} as a short human-readable phrase + * suitable for the summary line's {@code SKIPPED} branch. Only the + * five "ambiguous" situations can legitimately resolve to + * {@link Action#SKIPPED}; {@link Situation#DISCOVERY_SUCCESS} is + * rejected because the engine never skips when it found tests. + * + *

Package-private so {@code AffectedTestTaskLogFormatTest} can + * pin the exact wording — mirrors {@link #describeEscalation} so + * the two halves of the summary log share one vocabulary. + */ + static String describeSkipReason(Situation situation) { + Objects.requireNonNull(situation, "situation"); + return switch (situation) { + case EMPTY_DIFF -> + "no changed files detected"; + case ALL_FILES_IGNORED -> + "every changed file matched ignorePaths"; + case ALL_FILES_OUT_OF_SCOPE -> + "every changed file sat under out-of-scope dirs"; + case UNMAPPED_FILE -> + "onUnmappedFile=SKIPPED — non-Java or unmapped file in diff"; + case DISCOVERY_EMPTY -> + "no affected tests discovered"; + case DISCOVERY_SUCCESS -> throw new IllegalStateException( + "describeSkipReason must not be called for DISCOVERY_SUCCESS; " + + "the engine only produces that situation on a non-empty " + + "selection, which is never skipped"); + }; + } } diff --git a/affected-tests-gradle/src/test/java/io/affectedtests/gradle/AffectedTestTaskLogFormatTest.java b/affected-tests-gradle/src/test/java/io/affectedtests/gradle/AffectedTestTaskLogFormatTest.java index 7020dcb..f47b5c8 100644 --- a/affected-tests-gradle/src/test/java/io/affectedtests/gradle/AffectedTestTaskLogFormatTest.java +++ b/affected-tests-gradle/src/test/java/io/affectedtests/gradle/AffectedTestTaskLogFormatTest.java @@ -8,6 +8,7 @@ import org.junit.jupiter.api.Test; import org.slf4j.helpers.MessageFormatter; +import java.util.List; import java.util.Map; import java.util.Set; @@ -49,12 +50,106 @@ void nonEscalatedSelectionRendersProductionAndTestCounts() { String summary = render(AffectedTestTask.renderSummary(result)); assertEquals( - "Affected Tests: 1 changed file(s), 1 production class(es), 2 test class(es) affected", + "Affected Tests: SELECTED (DISCOVERY_SUCCESS) — 1 changed file(s), " + + "1 production class(es), 2 test class(es) affected", summary, - "Non-runAll summary must spell out the exact selection the downstream test " - + "task will receive, so operators can cross-check against the module-routed " - + "logs below — and the pluralisation form must match the runAll branch to " - + "keep CI greps stable"); + "SELECTED summary must name the outcome and the situation up front so the " + + "operator can tell at a glance what the engine decided, then spell out " + + "the exact selection the downstream test task will receive"); + } + + @Test + void summaryPrefixNamesOutcomeAndSituationOnEveryBranch() { + // The Phase 1 close-out contract: every summary line must start + // with "OUTCOME (SITUATION) —" so CI greps and human readers can + // bucket runs by outcome without having to parse the tail. + AffectedTestsResult selected = new AffectedTestsResult( + Set.of("com.example.FooTest"), Map.of(), + Set.of("src/main/java/com/example/Foo.java"), + Set.of("com.example.Foo"), Set.of(), + Buckets.empty(), + false, false, + Situation.DISCOVERY_SUCCESS, Action.SELECTED, + EscalationReason.NONE); + AffectedTestsResult runAll = new AffectedTestsResult( + Set.of(), Map.of(), + Set.of("src/main/resources/application.yml"), + Set.of(), Set.of(), + Buckets.empty(), + true, false, + Situation.UNMAPPED_FILE, Action.FULL_SUITE, + EscalationReason.RUN_ALL_ON_NON_JAVA_CHANGE); + AffectedTestsResult skipped = new AffectedTestsResult( + Set.of(), Map.of(), + Set.of("README.md"), Set.of(), Set.of(), + Buckets.empty(), + false, true, + Situation.ALL_FILES_IGNORED, Action.SKIPPED, + EscalationReason.NONE); + + assertTrue(render(AffectedTestTask.renderSummary(selected)) + .startsWith("Affected Tests: SELECTED (DISCOVERY_SUCCESS) —"), + "SELECTED prefix must name both action and situation — previously the summary " + + "carried neither, so any operator seeing 'Affected Tests: 1 changed file(s)…' " + + "had no idea whether discovery had actually landed or the engine was about " + + "to full-suite"); + assertTrue(render(AffectedTestTask.renderSummary(runAll)) + .startsWith("Affected Tests: FULL_SUITE (UNMAPPED_FILE) —"), + "FULL_SUITE prefix must name the situation so 'UNMAPPED_FILE vs DISCOVERY_EMPTY vs " + + "EMPTY_DIFF' is visible without cross-referencing the escalation reason"); + assertTrue(render(AffectedTestTask.renderSummary(skipped)) + .startsWith("Affected Tests: SKIPPED (ALL_FILES_IGNORED) —"), + "SKIPPED branch must also carry the prefix — previously skipped runs printed a " + + "separate 'Skipping test execution (...)' line without the outcome vocabulary"); + } + + @Test + void skippedBranchNamesReasonPhraseForEachSituation() { + // The SKIPPED branch is new — previously the task emitted a + // separate side-line. Every non-DISCOVERY_SUCCESS situation + // must produce a distinct reason phrase so operators can tell + // "README-only MR" from "api-test-only MR" from "discovery + // found nothing" at a glance. + java.util.Map expected = java.util.Map.of( + Situation.EMPTY_DIFF, "no changed files detected", + Situation.ALL_FILES_IGNORED, "every changed file matched ignorePaths", + Situation.ALL_FILES_OUT_OF_SCOPE, "every changed file sat under out-of-scope dirs", + Situation.UNMAPPED_FILE, "non-Java or unmapped file in diff", + Situation.DISCOVERY_EMPTY, "no affected tests discovered"); + + expected.forEach((situation, phrase) -> { + AffectedTestsResult skipped = new AffectedTestsResult( + Set.of(), Map.of(), + Set.of(), Set.of(), Set.of(), + Buckets.empty(), + false, true, + situation, Action.SKIPPED, + EscalationReason.NONE); + String summary = render(AffectedTestTask.renderSummary(skipped)); + assertTrue(summary.contains(phrase), + "SKIPPED summary for " + situation + " must contain phrase '" + phrase + + "' — operators rely on that substring to distinguish this skip " + + "from every other skip reason"); + assertTrue(summary.contains("SKIPPED (" + situation + ")"), + "SKIPPED summary must name the situation in the prefix"); + }); + } + + @Test + void describeSkipReasonRejectsDiscoverySuccess() { + // DISCOVERY_SUCCESS + SKIPPED is an engine bug — if discovery + // returned tests we never skip. The helper must fail loudly so + // such a drift cannot surface a placeholder phrase in CI. + assertThrows(IllegalStateException.class, + () -> AffectedTestTask.describeSkipReason(Situation.DISCOVERY_SUCCESS), + "describeSkipReason(DISCOVERY_SUCCESS) must throw — that combination is an " + + "engine contract violation, not a log-formatting concern"); + } + + @Test + void describeSkipReasonRejectsNull() { + assertThrows(NullPointerException.class, + () -> AffectedTestTask.describeSkipReason(null)); } @Test @@ -171,7 +266,7 @@ void pluralisationIsConsistentAcrossSummaryLines() { } @Test - void formatPlaceholderCountMatchesArgsLengthOnBothBranches() { + void formatPlaceholderCountMatchesArgsLengthOnEveryBranch() { // Regression guard for the SLF4J-placeholder contract: every `{}` // pair in the format string is consumed by exactly one positional // arg, and every arg must have a placeholder to render into. @@ -197,14 +292,20 @@ void formatPlaceholderCountMatchesArgsLengthOnBothBranches() { false, false, Situation.DISCOVERY_SUCCESS, Action.SELECTED, EscalationReason.NONE); + AffectedTestsResult skipped = new AffectedTestsResult( + Set.of(), Map.of(), + Set.of("README.md"), Set.of(), Set.of(), + Buckets.empty(), + false, true, + Situation.ALL_FILES_IGNORED, Action.SKIPPED, + EscalationReason.NONE); - AffectedTestTask.LogLine escalatedLine = AffectedTestTask.renderSummary(escalated); - AffectedTestTask.LogLine normalLine = AffectedTestTask.renderSummary(normal); - - assertEquals(countPlaceholders(escalatedLine.format()), escalatedLine.args().length, - "runAll branch must have one {} per arg so SLF4J's formatter consumes all args"); - assertEquals(countPlaceholders(normalLine.format()), normalLine.args().length, - "non-runAll branch must have one {} per arg so SLF4J's formatter consumes all args"); + for (AffectedTestsResult r : List.of(escalated, normal, skipped)) { + AffectedTestTask.LogLine line = AffectedTestTask.renderSummary(r); + assertEquals(countPlaceholders(line.format()), line.args().length, + "Summary branch for " + r.action() + "/" + r.situation() + + " must have one {} per arg so SLF4J's formatter consumes all args"); + } } private static int countPlaceholders(String format) {