From 4ea619c35e6d2337447ff4c238588d322b924fd9 Mon Sep 17 00:00:00 2001 From: vedanthvasudev Date: Wed, 22 Apr 2026 16:30:23 +0100 Subject: [PATCH] fix/hint-noise-and-lifecycle-fqn-tail: Silence false-positive --explain hints and preview selected FQNs at lifecycle level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes from the v1.9.17 sanity-test pass on the security-service pilot, plus an upstream refactor that falls out of tightening the second one. The --explain "Hint: outOfScopeTestDirs is configured but no files matched" line fired on 5 of 9 representative MR shapes — including markdown-only diffs and empty diffs where no source-tree matcher could have bitten. The false-positive rate trained reviewers to ignore the line, which defeated its diagnostic purpose. The hint now requires the resolved Situation to be DISCOVERY_SUCCESS or DISCOVERY_EMPTY (the only two branches where a correctly-configured out-of-scope pattern could have changed the outcome). The four suppressed situations — EMPTY_DIFF, ALL_FILES_IGNORED, ALL_FILES_OUT_OF_SCOPE, UNMAPPED_FILE — each have an explicit negative test so the gate can't silently drift. SELECTED dispatches previously printed only the module summary at lifecycle level and demoted every FQN to info, so a reviewer scrolling the default CI log could see the dispatch size but not which tests ran. The new renderLifecycleDispatchPreview helper surfaces the first five FQNs per module with a "… and N more (use --info for full list)" tail on larger dispatches, keeping the default log diagnosable without breaching the 4 MiB GitHub Actions step cap that forced the info-level demotion in the first place. The helper is a pure function with its own unit-test file so the exact format stays pinned without a Gradle runtime. Hoisting FQN validation out of the per-module log loop to feed the lifecycle preview also fixed a latent header inconsistency — the "Running N affected test classes" line used the pre-validation count while the per-module counts underneath it were post-validation — and closed a pre-existing silent-fallback: a module whose entire discovered FQN set failed isValidFqn used to dispatch its bare taskPath with no --tests filter, silently escalating to a full module suite. Modules with zero valid FQNs are now dropped from dispatch, and a dispatch where zero FQNs survive validation across ALL modules throws GradleException instead of invoking Gradle with an empty task list (environment-dependent behaviour). Both changes are documented in the CHANGELOG so adopters relying on the accidental fallback can migrate to explicit runAllIfNoMatches / Action.FULL_SUITE config. --- CHANGELOG.md | 56 ++++++ README.md | 23 ++- .../gradle/AffectedTestTask.java | 174 ++++++++++++++---- .../AffectedTestTaskDispatchPreviewTest.java | 139 ++++++++++++++ .../AffectedTestTaskExplainFormatTest.java | 124 +++++++++++++ 5 files changed, 478 insertions(+), 38 deletions(-) create mode 100644 affected-tests-gradle/src/test/java/io/affectedtests/gradle/AffectedTestTaskDispatchPreviewTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index c904f97..74bb677 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,62 @@ adheres to [Semantic Versioning](https://semver.org/). malformed-glob error path is now the single source of truth for `Affected Tests: invalid glob at outOfScope*Dirs[N]` messages. +### Fixed — post-v1.9.17 sanity-test pass + +- The `Hint:` line on `affectedTest --explain` no longer fires on + situations where an out-of-scope pattern could not have mattered. + Running v1.9.17 through nine representative MR shapes on the + security-service pilot (empty diff, api-test-only, performance-test- + only, production Java only, markdown only, mixed api-test + prod, + test-only, `git rm` markdown, gradle + prod) showed the hint firing + on five of them — including a bare markdown-only MR where the diff + contained nothing a source-tree matcher could have bitten. That + 5-of-9 false-positive rate trains reviewers to ignore the line, + defeating its purpose. + + The hint now requires the situation to be `DISCOVERY_SUCCESS` or + `DISCOVERY_EMPTY` (the only two branches where a correctly-configured + out-of-scope pattern could have changed the outcome) in addition to + the existing "diff non-empty AND out-of-scope bucket empty AND at + least one out-of-scope dir configured" guard. `EMPTY_DIFF`, + `ALL_FILES_IGNORED`, `ALL_FILES_OUT_OF_SCOPE`, and `UNMAPPED_FILE` + all suppress the hint — none of them is a case where an operator + could act on it. + +- Lifecycle output for `SELECTED` dispatches now previews the first + five FQNs per module with a "… and N more (use --info for full list)" + tail on larger dispatches. Pre-v1.9.18 the task printed only the + module summary at lifecycle level and demoted every FQN to info, + leaving a reviewer scrolling the default CI log unable to see *which* + tests were selected without either rerunning with `--info` or opening + the JUnit report after the fact. The info-level per-FQN log is + retained for `--info` users, and the bounded preview keeps total + lifecycle output well under the 4 MiB GitHub Actions step cap that + forced the demotion in the first place. + +- Dispatch-side FQN validation was hoisted out of the per-module log + loop so the "Running N affected test classes across M module(s)" + header now reports the post-validation count and stays arithmetically + consistent with the per-module previews underneath it. If any FQNs + were skipped (an `isValidFqn` WARN was already logged) the header + gains a ` (K malformed FQN skipped — see WARN above)` suffix so the + mismatch between "discovered" and "dispatched" is visible at a + glance rather than a puzzle only the WARN log can solve. Two latent + side-effects fall out of the refactor: + - A module whose entire discovered FQN set fails validation is now + dropped from dispatch. Pre-v1.9.18 the task appended that module's + bare `taskPath` with no `--tests` filter, which silently degraded + into a full module test suite run — the exact safety posture + `isValidFqn` was added to prevent. Operators relying on this + accidental fallback should configure `runAllIfNoMatches = true` / + `Action.FULL_SUITE` explicitly instead. + - A dispatch where zero FQNs survive validation across ALL modules + now throws `GradleException` instead of invoking Gradle with an + empty task list (environment-dependent behaviour, ranging from + "fail with No tasks specified" to "silently run the default task"). + The WARN logs above the failure name every rejected FQN, so the + mis-discovery is diagnosable from the same console output. + ### Fixed — post-v1.9.16 review batch - `GitChangeDetector.uncommittedChanges` and `stagedChanges` now diff --git a/README.md b/README.md index c037374..95f34e5 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ plugins { Prints the full decision trace — bucket counts, situation, action, and the tier of the priority ladder (explicit / legacy / mode / hardcoded) that picked each action — without running a single test. Useful when a CI run escalated to the full suite and the operator needs to know *why* before filing a bug. -When `outOfScopeTestDirs` / `outOfScopeSourceDirs` are configured but zero files in the diff land in the out-of-scope bucket, the trace emits a one-line `Hint:` pointing at the configured knob. That's the silent-failure trap a real adopter hit: a perfectly valid-looking glob that never bit anything, which the plugin would otherwise only surface after a 30-minute full-suite CI run. +When `outOfScopeTestDirs` / `outOfScopeSourceDirs` are configured but zero files in the diff land in the out-of-scope bucket *and* the situation is `DISCOVERY_SUCCESS` or `DISCOVERY_EMPTY`, the trace emits a one-line `Hint:` pointing at the configured knob. That's the silent-failure trap a real adopter hit: a perfectly valid-looking glob that never bit anything, which the plugin would otherwise only surface after a 30-minute full-suite CI run. The hint is suppressed on `EMPTY_DIFF`, `ALL_FILES_IGNORED`, `ALL_FILES_OUT_OF_SCOPE`, and `UNMAPPED_FILE` because those branches ran the way they did for reasons an out-of-scope pattern could not have influenced. Sample output: @@ -102,6 +102,27 @@ Affected Tests: SKIPPED (ALL_FILES_IGNORED) — 1 changed file(s); every changed 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. +On a `SELECTED` outcome, the task also prints the first few FQNs per module at lifecycle level so a reviewer can sanity-check the dispatch from the default CI log without rerunning with `--info`: + +``` +Running 17 affected test classes across 2 module(s): + application:test (12 test classes) + com.example.auth.LoginControllerTest + com.example.auth.TokenServiceTest + com.example.auth.PasswordHasherTest + com.example.auth.SessionRepositoryTest + com.example.auth.RoleMapperTest + … and 7 more (use --info for full list) + api:test (5 test classes) + com.example.api.PublicEndpointsTest + com.example.api.RateLimitTest + com.example.api.VersionHeaderTest + com.example.api.ErrorFormatTest + com.example.api.HealthProbeTest +``` + +The preview caps at five FQNs per module; `--info` still surfaces the full per-FQN list. The cap exists so a utility change that ripples into hundreds of test classes can't blow past the 4 MiB GitHub Actions per-step log cap before the nested test output even starts. + ### 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 77c2eb9..bfb5437 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 @@ -505,43 +505,82 @@ private void executeTests(Path projectDir, // happen to contain the FQN). Map> grouped = groupFqnsByModule(projectDir, testFqns, fqnToPath); - getLogger().lifecycle("Running {} affected test classes across {} module(s):", - testFqns.size(), grouped.size()); - + // Validate every discovered FQN up front so the header, + // per-module preview, and argv-append stay arithmetically + // consistent. Splitting validation from dispatch also + // means a module whose entire discovered set is malformed + // (vanishingly unlikely in practice, but possible from a + // buggy custom strategy) is dropped cleanly instead of + // silently falling through to "run the whole module's + // test suite" once the bare `taskPath` is appended to the + // argv with no `--tests` filter. + Map> validatedGroups = new LinkedHashMap<>(grouped.size()); + int totalValid = 0; for (Map.Entry> entry : grouped.entrySet()) { String modulePath = entry.getKey(); - List fqnsForModule = entry.getValue(); - String taskPath = modulePath.isEmpty() ? "test" : modulePath + ":test"; - args.add(taskPath); - // One module-level lifecycle line; the per-FQN entries - // are info-level because they can legitimately run into - // the thousands on a widely-used utility change and - // spamming lifecycle can blow past CI log-size caps - // (GitHub Actions truncates at 4 MiB/step) before the - // nested test output even starts. - getLogger().lifecycle(" {} ({} test class{})", - taskPath, fqnsForModule.size(), fqnsForModule.size() == 1 ? "" : "es"); - for (String fqn : fqnsForModule) { + List valid = new ArrayList<>(entry.getValue().size()); + for (String fqn : entry.getValue()) { if (!isValidFqn(fqn)) { - // Defence in depth against a compromised source tree - // sneaking shell-like tokens into a --tests argument. - // Gradle's CLI parser currently treats the next argv - // element as the value of --tests regardless of - // content, but that's an undocumented contract and a - // future parser change would turn this into real - // argument injection. Non-Java-shaped FQNs cannot - // correspond to a real JVM test class anyway, so - // dropping them costs nothing and forces the bad - // input to surface visibly. + // Defence in depth against a compromised source + // tree sneaking shell-like tokens into a --tests + // argument. The FQN cannot correspond to a real + // JVM test class, so dropping it is lossless. getLogger().warn( "Affected Tests: skipping malformed test FQN '{}' for task {} — " + "not a Java-shaped identifier, cannot correspond to a " + "real test class.", fqn, taskPath); continue; } + valid.add(fqn); + } + if (!valid.isEmpty()) { + validatedGroups.put(taskPath, valid); + totalValid += valid.size(); + } + } + + int skipped = testFqns.size() - totalValid; + + // Belt-and-braces: a dispatch with zero surviving FQNs + // across ALL modules would otherwise produce an argv of + // `[gradlew, -x, compileJava]` with no task specified — + // Gradle's behavior there (fail vs. default-task) is + // environment-dependent and definitely not the safety + // posture `runAll` promises. If we get here the WARN logs + // above already explain which FQNs were dropped; a hard + // fail surfaces the mis-discovery instead of silently + // running nothing. + if (validatedGroups.isEmpty()) { + throw new GradleException( + "Affected Tests: every discovered FQN (" + testFqns.size() + + ") was malformed and skipped — refusing to dispatch a taskless " + + "Gradle invocation. See WARN logs above for the rejected FQNs."); + } + + String skippedSuffix = skipped == 0 + ? "" + : " (" + skipped + (skipped == 1 ? " malformed FQN skipped" : " malformed FQNs skipped") + + " — see WARN above)"; + getLogger().lifecycle("Running {} affected test classes across {} module(s):{}", + totalValid, validatedGroups.size(), skippedSuffix); + + // Dispatch-side emission. Lifecycle per module is bounded + // by LIFECYCLE_FQN_PREVIEW_LIMIT; the full list stays at + // info level (see that constant's Javadoc for why). + for (Map.Entry> entry : validatedGroups.entrySet()) { + String taskPath = entry.getKey(); + List validFqns = entry.getValue(); + + args.add(taskPath); + for (String fqn : validFqns) { args.add("--tests"); args.add(fqn); + } + + renderLifecycleDispatchPreview(taskPath, validFqns) + .forEach(getLogger()::lifecycle); + for (String fqn : validFqns) { getLogger().info(" {} -> {}", taskPath, fqn); } } @@ -747,6 +786,56 @@ static String describeEscalation(EscalationReason reason) { /** Cap on files listed per bucket in the {@code --explain} trace. */ static final int EXPLAIN_SAMPLE_LIMIT = 10; + /** + * Cap on FQNs listed at lifecycle level per module in the + * "Running N affected test classes" dispatch output. Chosen at 5 + * to keep the preview tight enough that a reviewer can read it + * without scrolling yet large enough to sanity-check selection on + * most MRs, which dispatch single digits of classes per module. + * Larger dispatches still log every FQN at info level — this cap + * exists only to keep the default lifecycle log bounded and well + * under the 4 MiB GitHub Actions step cap that forced the + * per-FQN demotion in pre-v1.9.18 versions. + */ + static final int LIFECYCLE_FQN_PREVIEW_LIMIT = 5; + + /** + * Renders the lifecycle-level dispatch preview for a single + * module: the summary line, then up to + * {@link #LIFECYCLE_FQN_PREVIEW_LIMIT} FQNs indented underneath, + * and — when the dispatch exceeds the preview limit — a single + * "… and N more (use --info for full list)" tail. + * + *

Package-private so + * {@code AffectedTestTaskDispatchPreviewTest} can pin the format + * without spinning up the Gradle runtime. The helper is pure over + * its inputs, so the test treats it as a pure function and the + * caller in {@link #executeTests} just pipes each returned line + * to {@link org.gradle.api.logging.Logger#lifecycle(String)}. + * + * @param taskPath the Gradle task path the dispatch targets (for + * example {@code "application:test"}) + * @param fqns the validated FQNs being dispatched, in the + * order they will be passed to Gradle; preserving + * this order in the preview keeps the mental map + * from "what did I change" to "what is running" + * intact for the operator + */ + static List renderLifecycleDispatchPreview(String taskPath, List fqns) { + List lines = new ArrayList<>(Math.min(fqns.size(), LIFECYCLE_FQN_PREVIEW_LIMIT) + 2); + int size = fqns.size(); + String plural = size == 1 ? "" : "es"; + lines.add(" " + taskPath + " (" + size + " test class" + plural + ")"); + int preview = Math.min(size, LIFECYCLE_FQN_PREVIEW_LIMIT); + for (int i = 0; i < preview; i++) { + lines.add(" " + fqns.get(i)); + } + if (size > preview) { + lines.add(" … and " + (size - preview) + " more (use --info for full list)"); + } + return lines; + } + /** * Renders the human-readable decision trace produced by * {@code affectedTest --explain}. Returned as a list of lines so the @@ -837,22 +926,33 @@ static List renderExplainTrace(AffectedTestsConfig config, AffectedTests * package-private so the explain-format tests can pin the exact * conditions without spinning up Gradle. * - *

The hint fires only when all three conditions hold: - *

    - *
  • at least one changed file exists (nothing to diagnose on - * an empty diff, and a re-run after every merge to master - * would otherwise spam the false alarm);
    • - *
  • at least one of {@code outOfScopeTestDirs} / - * {@code outOfScopeSourceDirs} is configured (zero-config - * installs never opted in, so the hint would just be noise); - *
    • - *
  • the out-of-scope bucket is empty (if the config IS biting, - * the bucket count already tells the story).
    • - *
+ *

Gate: only {@link Situation#DISCOVERY_SUCCESS} and + * {@link Situation#DISCOVERY_EMPTY} can have been influenced by an + * out-of-scope pattern. The other four situations reach their + * outcome for reasons the out-of-scope bucket cannot change: + * {@link Situation#EMPTY_DIFF} (no files to match), + * {@link Situation#ALL_FILES_IGNORED} (ignore wins before + * out-of-scope ever looks at the file), + * {@link Situation#ALL_FILES_OUT_OF_SCOPE} (bucket is non-empty, so + * the "silent" precondition is false — also caught by the + * bucket-empty guard below), and {@link Situation#UNMAPPED_FILE} + * (by construction the file missed every pattern, including + * out-of-scope). Firing here trains reviewers to ignore the hint + * on routine docs-only / gradle-only MRs — that's the v1.9.17 + * sanity-test finding this gate exists to close. + * + *

Other preconditions: diff non-empty, at least one of + * {@code outOfScopeTestDirs} / {@code outOfScopeSourceDirs} + * configured, and the out-of-scope bucket empty. */ static void appendOutOfScopeHint(List lines, AffectedTestsConfig config, AffectedTestsResult result) { + Situation situation = result.situation(); + if (situation != Situation.DISCOVERY_SUCCESS + && situation != Situation.DISCOVERY_EMPTY) { + return; + } if (result.changedFiles().isEmpty()) { return; } diff --git a/affected-tests-gradle/src/test/java/io/affectedtests/gradle/AffectedTestTaskDispatchPreviewTest.java b/affected-tests-gradle/src/test/java/io/affectedtests/gradle/AffectedTestTaskDispatchPreviewTest.java new file mode 100644 index 0000000..949e6ea --- /dev/null +++ b/affected-tests-gradle/src/test/java/io/affectedtests/gradle/AffectedTestTaskDispatchPreviewTest.java @@ -0,0 +1,139 @@ +package io.affectedtests.gradle; + +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.stream.IntStream; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Pins the lifecycle-level dispatch preview emitted when + * {@code affectedTest} resolves to {@link io.affectedtests.core.config.Action#SELECTED}. + * + *

Before this renderer existed the task printed only the module + * summary line + * ({@code " application:test (47 test classes)"}) at lifecycle level + * and every individual FQN at info level. That made the v1.9.17 + * sanity-test pass on security-service Finding 3: an operator + * scrolling the default CI log could see the dispatch size but had to + * either rerun with {@code --info} (slow) or open the JUnit report + * (after the fact) to learn which tests actually ran. The preview + * gives the first {@link AffectedTestTask#LIFECYCLE_FQN_PREVIEW_LIMIT} + * FQNs at lifecycle level with a "… and N more (use --info for full + * list)" tail on larger dispatches, keeping the default log + * diagnosable without risking the 4 MiB step cap on GitHub Actions + * that forced the info-level demotion in the first place. + */ +class AffectedTestTaskDispatchPreviewTest { + + @Test + void emitsSummaryOnlyForEmptyDispatch() { + // The SELECTED branch should never reach this helper with an + // empty list — but if the caller ever does, we still produce + // exactly the summary line and nothing else. Pinning this + // shape prevents an accidental IndexOutOfBounds if a future + // caller passes the empty set without its own guard. + List lines = AffectedTestTask.renderLifecycleDispatchPreview( + "application:test", List.of()); + + assertEquals(1, lines.size(), + "Empty dispatch must produce one line — the summary — and no preview or tail"); + assertEquals(" application:test (0 test classes)", lines.get(0), + "Summary line must use plural form even for zero to keep the format stable"); + } + + @Test + void singletonDispatchPrintsSummaryAndOneFqnWithoutTail() { + // One-class dispatch: summary uses singular "class", FQN is + // printed at lifecycle level, and there is no "… and N more" + // tail because the preview covers the whole list. + List lines = AffectedTestTask.renderLifecycleDispatchPreview( + "application:test", List.of("com.example.FooTest")); + + assertEquals(2, lines.size(), + "Singleton dispatch must produce summary + FQN only — no tail"); + assertTrue(lines.get(0).contains("(1 test class)"), + "Singleton must use singular 'test class' — plural mistake is the kind of " + + "paper-cut that dents trust in log output"); + assertEquals(" com.example.FooTest", lines.get(1), + "Preview FQN must be indented under the summary so the reader can see it belongs"); + assertFalse(String.join("\n", lines).contains("more"), + "Singleton preview must not render the '… and N more' tail"); + } + + @Test + void dispatchAtPreviewLimitPrintsAllWithoutTail() { + // Exactly LIFECYCLE_FQN_PREVIEW_LIMIT FQNs: every FQN is in + // the preview, and the tail is suppressed because there is + // nothing hidden. Also pins LIFECYCLE_FQN_PREVIEW_LIMIT as + // a package-private constant so the test stays synced with + // the renderer on a limit change. + int limit = AffectedTestTask.LIFECYCLE_FQN_PREVIEW_LIMIT; + List fqns = IntStream.range(0, limit) + .mapToObj(i -> "com.example.Test" + i) + .toList(); + + List lines = AffectedTestTask.renderLifecycleDispatchPreview( + "application:test", fqns); + + assertEquals(1 + limit, lines.size(), + "At exactly the preview limit: summary + N FQNs, no '… more' tail"); + assertFalse(String.join("\n", lines).contains("more"), + "Tail must stay suppressed when every FQN is already in the preview"); + } + + @Test + void oversizedDispatchEmitsPreviewPlusMoreTail() { + // Twenty FQNs, limit is five (see LIFECYCLE_FQN_PREVIEW_LIMIT): + // lifecycle output must contain the first five FQNs and a + // single "… and 15 more (use --info for full list)" tail. + int limit = AffectedTestTask.LIFECYCLE_FQN_PREVIEW_LIMIT; + List fqns = IntStream.range(0, 20) + .mapToObj(i -> "com.example.Test" + i) + .toList(); + + List lines = AffectedTestTask.renderLifecycleDispatchPreview( + "application:test", fqns); + + assertEquals(1 + limit + 1, lines.size(), + "Oversized dispatch must produce summary + preview + exactly one tail line"); + assertEquals(" application:test (20 test classes)", lines.get(0), + "Summary line format (indent, count, pluralization) must be stable at oversized too — " + + "this is the only line a reviewer sees on a very large dispatch without --info"); + assertEquals(" com.example.Test0", lines.get(1), + "Preview must keep input order so reviewers can map to the diff without re-sorting"); + assertEquals(" com.example.Test" + (limit - 1), lines.get(limit), + "Preview must stop at the limit — no off-by-one into the 6th FQN"); + + String tail = lines.get(lines.size() - 1); + assertTrue(tail.contains("and " + (20 - limit) + " more"), + "Tail must name the hidden count so the reader knows how much is omitted, got: " + tail); + assertTrue(tail.contains("--info"), + "Tail must point at the flag that reveals the full list — otherwise the reader " + + "has to guess how to see the rest"); + } + + @Test + void extremelyLargeDispatchStillCapsPreviewAtLimit() { + // Sanity: a two-hundred-class dispatch (plausible on a + // utility change that ripples through the whole service) + // must not start logging thousands of lifecycle lines. The + // 4 MiB GitHub Actions step cap is the original reason the + // per-FQN log was demoted to info level — the tail exists + // to preserve that protection while still surfacing enough + // signal at lifecycle level. + List fqns = IntStream.range(0, 200) + .mapToObj(i -> "com.example.Test" + i) + .toList(); + + List lines = AffectedTestTask.renderLifecycleDispatchPreview( + "application:test", fqns); + + assertEquals(1 + AffectedTestTask.LIFECYCLE_FQN_PREVIEW_LIMIT + 1, lines.size(), + "Lifecycle preview must stay bounded regardless of dispatch size — " + + "this is the load-bearing property for the 4 MiB step cap"); + } +} diff --git a/affected-tests-gradle/src/test/java/io/affectedtests/gradle/AffectedTestTaskExplainFormatTest.java b/affected-tests-gradle/src/test/java/io/affectedtests/gradle/AffectedTestTaskExplainFormatTest.java index 4fb9d44..0b48cd8 100644 --- a/affected-tests-gradle/src/test/java/io/affectedtests/gradle/AffectedTestTaskExplainFormatTest.java +++ b/affected-tests-gradle/src/test/java/io/affectedtests/gradle/AffectedTestTaskExplainFormatTest.java @@ -403,6 +403,130 @@ void hintDoesNotFireWhenOutOfScopeDirsAreNotConfigured() { "Hint must stay silent for zero-config installs — nobody opted into out-of-scope dirs"); } + @Test + void hintDoesNotFireOnAllFilesIgnored() { + // Regression for Finding 1 from the v1.9.17 sanity-test pass on + // security-service: a markdown-only MR routed through + // ALL_FILES_IGNORED, yet the hint fired saying "outOfScopeTestDirs + // is configured but no file in the diff matched". Literally true + // but pure noise — the diff contained nothing a source-tree + // matcher could ever have bitten, so there is nothing for the + // operator to diagnose. Suppressing here prevents the hint from + // training reviewers to ignore it on every docs-only MR. + AffectedTestsConfig config = AffectedTestsConfig.builder() + .outOfScopeTestDirs(List.of("api-test/**")) + .build(); + AffectedTestsResult result = new AffectedTestsResult( + Set.of(), Map.of(), + Set.of("docs/README.md"), + Set.of(), Set.of(), + new Buckets(Set.of("docs/README.md"), + Set.of(), Set.of(), Set.of(), Set.of()), + false, true, + Situation.ALL_FILES_IGNORED, Action.SKIPPED, + EscalationReason.NONE); + + String trace = joined(AffectedTestTask.renderExplainTrace(config, result)); + + assertFalse(trace.contains("Hint:"), + "Hint must stay silent on ALL_FILES_IGNORED — the diff contained nothing " + + "that could have matched an out-of-scope pattern, so diagnosis is impossible"); + } + + @Test + void hintDoesNotFireOnUnmappedFileEscalation() { + // Regression for Finding 1: a gradle-file-only MR routed through + // UNMAPPED_FILE → FULL_SUITE, yet the hint fired. The gradle file + // was never a candidate for out-of-scope matching (it is not + // under any source tree), so the hint adds no diagnostic value + // and obscures the already-present "why did we run the full + // suite" signal. Also covers mixed diffs that route through + // UNMAPPED_FILE via an unrelated non-Java file. + AffectedTestsConfig config = AffectedTestsConfig.builder() + .outOfScopeTestDirs(List.of("api-test/**")) + .build(); + AffectedTestsResult result = new AffectedTestsResult( + Set.of(), Map.of(), + Set.of("build.gradle"), + Set.of(), Set.of(), + new Buckets(Set.of(), Set.of(), + Set.of(), Set.of(), + Set.of("build.gradle")), + true, false, + Situation.UNMAPPED_FILE, Action.FULL_SUITE, + EscalationReason.RUN_ALL_ON_NON_JAVA_CHANGE); + + String trace = joined(AffectedTestTask.renderExplainTrace(config, result)); + + assertFalse(trace.contains("Hint:"), + "Hint must stay silent on UNMAPPED_FILE — the diff's unmapped files " + + "could not have matched any out-of-scope source/test pattern"); + } + + @Test + void hintFiresOnDiscoveryEmptyFullSuiteEscalation() { + // Positive case: when the diff changed real production code but + // discovery found zero affected tests, the engine escalates to + // FULL_SUITE under the CI mode's safety net. The hint is STILL + // useful here — if the user had outOfScopeSourceDirs meant to + // exclude this production file from dispatching tests and the + // pattern silently didn't bite, the FULL_SUITE escalation is the + // unnecessary waste the hint exists to expose. Lock in that the + // situation-gate in appendOutOfScopeHint includes DISCOVERY_EMPTY + // alongside DISCOVERY_SUCCESS. + AffectedTestsConfig config = AffectedTestsConfig.builder() + .outOfScopeSourceDirs(List.of("legacy-service/**")) + .build(); + AffectedTestsResult result = new AffectedTestsResult( + Set.of(), Map.of(), + Set.of("src/main/java/com/example/Foo.java"), + Set.of("com.example.Foo"), + Set.of(), + new Buckets(Set.of(), Set.of(), + Set.of("src/main/java/com/example/Foo.java"), + Set.of(), Set.of()), + true, false, + Situation.DISCOVERY_EMPTY, Action.FULL_SUITE, + EscalationReason.RUN_ALL_IF_NO_MATCHES); + + String trace = joined(AffectedTestTask.renderExplainTrace(config, result)); + + assertTrue(trace.contains("Hint:"), + "Hint must fire on DISCOVERY_EMPTY too — that's the case where an " + + "over-broad source file that should have been out-of-scope is what " + + "drove the unnecessary FULL_SUITE escalation"); + } + + @Test + void hintDoesNotFireOnAllFilesOutOfScope() { + // Gate invariant: when every file landed in the out-of-scope + // bucket there is nothing silent to diagnose — the config IS + // biting, just hard enough to empty the discovery input. The + // pre-existing bucket-non-empty guard in appendOutOfScopeHint + // suppresses this case today; pinning the situation-gate here + // keeps the two guards from drifting apart if either gets + // refactored. + AffectedTestsConfig config = AffectedTestsConfig.builder() + .outOfScopeTestDirs(List.of("api-test/**")) + .build(); + AffectedTestsResult result = new AffectedTestsResult( + Set.of(), Map.of(), + Set.of("api-test/src/test/java/com/example/ApiFooTest.java"), + Set.of(), Set.of(), + new Buckets(Set.of(), + Set.of("api-test/src/test/java/com/example/ApiFooTest.java"), + Set.of(), Set.of(), Set.of()), + false, true, + Situation.ALL_FILES_OUT_OF_SCOPE, Action.SKIPPED, + EscalationReason.NONE); + + String trace = joined(AffectedTestTask.renderExplainTrace(config, result)); + + assertFalse(trace.contains("Hint:"), + "Hint must stay silent on ALL_FILES_OUT_OF_SCOPE — the config is biting as " + + "intended and there is nothing silent to diagnose"); + } + @Test void hintDoesNotFireOnEmptyDiff() { // Negative case: with no changed files the hint has nothing to