diff --git a/CHANGELOG.md b/CHANGELOG.md index 397b263..a9e9785 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,23 @@ adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] -Nothing yet. +### Added + +- `outOfScopeTestDirs` and `outOfScopeSourceDirs` now accept Ant-style + globs (`api-test/**`, `**/api-test/**`, `{api,perf}-test/**`) in + addition to the existing literal directory prefixes. Each list entry + is classified independently, so users can mix both shapes in the + same config. Surfaced after a real adopter configured + `outOfScopeTestDirs = ['api-test/**']` and the engine silently + treated it as a literal prefix — which never matched. +- New `Hint:` line on `affectedTest --explain` when + `outOfScopeTestDirs` / `outOfScopeSourceDirs` are configured but + zero files in the diff landed in the out-of-scope bucket. Points at + the configured knob so the operator learns about the silent + misconfiguration on the trace instead of after a 30-minute full-suite + CI run. The hint is suppressed on empty diffs, on runs where the + bucket is non-empty, and on zero-config installs — so its rarity is + itself a signal. ## [1.9.12] — 2026-04-22 diff --git a/README.md b/README.md index ae50db5..0ae8468 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,8 @@ 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. + Sample output: ``` @@ -164,9 +166,24 @@ affectedTests { // Test source sets the plugin must not dispatch via the affectedTest // task (e.g. Cucumber, Gatling). A diff entirely under these dirs // routes to ALL_FILES_OUT_OF_SCOPE → SKIPPED by default. - outOfScopeTestDirs = ["api-test/src/test/java", "api-test/src/test/resources"] + // + // Entries may be either: + // • literal directory prefixes — "api-test/src/test/java" matches + // that path at the repo root or under any module + // (e.g. "services/orders/api-test/src/test/java/..."), and + // "api-test" (no source-dir suffix) never claims sibling names + // like "api-test-utils/..."; + // • Ant-style globs — "api-test/**" or "**/api-test/**" — using + // the standard JVM glob syntax ("*", "**", "?", "[abc]", "{a,b}"). + // + // Mix both shapes freely; the plugin picks the right semantics per + // entry. If you configure this knob but see "Hint:" on --explain + // saying zero files matched, your paths/globs don't bite — double + // check them (that's the silent-failure trap sanity testing caught). + outOfScopeTestDirs = ["api-test/**", "performance-test/**"] // Production source sets the plugin must treat as out-of-scope. + // Accepts the same literal-prefix / glob shapes as outOfScopeTestDirs. outOfScopeSourceDirs = [] // ---------------- Per-situation actions (v2) ---------------- diff --git a/affected-tests-core/src/main/java/io/affectedtests/core/mapping/PathToClassMapper.java b/affected-tests-core/src/main/java/io/affectedtests/core/mapping/PathToClassMapper.java index eff3faa..3790fa6 100644 --- a/affected-tests-core/src/main/java/io/affectedtests/core/mapping/PathToClassMapper.java +++ b/affected-tests-core/src/main/java/io/affectedtests/core/mapping/PathToClassMapper.java @@ -6,9 +6,11 @@ import java.nio.file.FileSystems; import java.nio.file.PathMatcher; +import java.util.ArrayList; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; +import java.util.function.Predicate; /** * Maps file paths (from git diff output) to fully-qualified Java class names. @@ -29,12 +31,21 @@ public final class PathToClassMapper { private final AffectedTestsConfig config; private final List ignoreMatchers; + private final List> outOfScopeTestMatchers; + private final List> outOfScopeSourceMatchers; public PathToClassMapper(AffectedTestsConfig config) { this.config = config; this.ignoreMatchers = config.ignorePaths().stream() .map(p -> FileSystems.getDefault().getPathMatcher("glob:" + p)) .toList(); + // Out-of-scope dirs are compiled once, up-front, so every diff + // pays only the matcher cost instead of re-parsing the config + // strings per-file. The pre-compile step is also where we decide + // between glob and literal-prefix semantics, so users can mix + // both shapes in the same list without surprise. + this.outOfScopeTestMatchers = compileOutOfScopeMatchers(config.outOfScopeTestDirs()); + this.outOfScopeSourceMatchers = compileOutOfScopeMatchers(config.outOfScopeSourceDirs()); } /** @@ -94,8 +105,8 @@ public MappingResult mapChangedFiles(Set changedFiles) { // not mis-filed as an in-scope test class. Source-dir check is // first because real code is more common in diffs than test // code under an out-of-scope test dir. - if (isUnder(filePath, config.outOfScopeSourceDirs()) - || isUnder(filePath, config.outOfScopeTestDirs())) { + if (matchesAny(filePath, outOfScopeSourceMatchers) + || matchesAny(filePath, outOfScopeTestMatchers)) { outOfScopeFiles.add(filePath); log.debug("Out-of-scope: {}", filePath); continue; @@ -227,20 +238,95 @@ private boolean isIgnored(String filePath) { } /** - * Boundary-aware "is this file under any of the given dirs?" check. - * Uses the same normalisation as {@link #tryMapToClass} so - * {@code "api-test/src/test/java/..."} matches {@code "api-test/src/test/java"} - * but {@code "my-api-test/..."} does not. + * Evaluates the pre-compiled out-of-scope matcher list against the + * normalised file path. Returns {@code true} as soon as any matcher + * claims the path so large configs short-circuit on the first hit. */ - private static boolean isUnder(String filePath, List dirs) { - if (dirs.isEmpty()) return false; + private static boolean matchesAny(String filePath, List> matchers) { + if (matchers.isEmpty()) return false; String normalized = filePath.replace('\\', '/'); + for (Predicate matcher : matchers) { + if (matcher.test(normalized)) return true; + } + return false; + } + + /** + * Compiles each raw out-of-scope dir string into a {@link Predicate} + * that answers "does this (normalised) file path sit under this + * entry?". + * + *

Each entry is classified into one of two semantics based on + * whether it contains any glob metacharacter ({@code *}, {@code ?}, + * {@code [}, {@code \{}): + * + *

    + *
  • Glob entries (e.g. {@code "api-test/**"}) compile to + * a {@link PathMatcher} using the JVM's default file system + * {@code glob:} syntax, so {@code **} crosses directory + * boundaries as users expect from Ant/Gradle conventions.
    • + *
  • Literal entries (e.g. {@code "api-test/src/test/java"}) + * keep the boundary-aware prefix semantics the README has + * documented since v1: the entry matches only when it sits at + * the start of the path or is preceded by {@code '/'}, so + * {@code "api-test"} never claims + * {@code "api-test-utils/..."}.
    • + *
+ * + *

Mixing both shapes in the same list is supported — this is + * what lets existing adopters migrate at their own pace without the + * plugin ever silently losing coverage. + * + *

{@code null} and blank entries are dropped quietly: a + * mis-concatenated list literal on the Gradle side is not worth + * failing a build over, and the {@code --explain} hint already + * surfaces the more likely "configured but nothing matched" failure. + */ + private static List> compileOutOfScopeMatchers(List dirs) { + if (dirs == null || dirs.isEmpty()) return List.of(); + List> matchers = new ArrayList<>(dirs.size()); for (String dir : dirs) { if (dir == null || dir.isBlank()) continue; String normalizedDir = dir.replace('\\', '/'); - if (!normalizedDir.endsWith("/")) normalizedDir += "/"; - if (normalized.startsWith(normalizedDir)) return true; - if (normalized.contains("/" + normalizedDir)) return true; + if (hasGlobMetachar(normalizedDir)) { + PathMatcher pm = FileSystems.getDefault() + .getPathMatcher("glob:" + normalizedDir); + matchers.add(path -> { + try { + return pm.matches(java.nio.file.Path.of(path)); + } catch (java.nio.file.InvalidPathException e) { + // A changed file whose path contains characters the + // platform refuses to parse (mostly a Windows-in-git + // edge case) can't match any glob — fail closed so + // the engine routes it through the unmapped bucket + // instead of pretending the glob bit. + return false; + } + }); + } else { + // Literal-prefix branch preserves the pre-v2 "boundary-aware + // prefix" semantics verbatim: leading-edge or /-bounded. + String prefix = normalizedDir.endsWith("/") ? normalizedDir : normalizedDir + "/"; + matchers.add(path -> + path.startsWith(prefix) || path.contains("/" + prefix)); + } + } + return List.copyOf(matchers); + } + + /** + * True if the string contains any character the JVM's + * {@code glob:} syntax treats as a metacharacter. Kept deliberately + * small: these four cover every pattern the README, Gradle docs, + * and user bug reports mention. Anything more exotic still routes + * through the literal-prefix branch and will fail closed (i.e. + * match nothing), which is safer than inferring glob intent from a + * stray character. + */ + private static boolean hasGlobMetachar(String s) { + for (int i = 0; i < s.length(); i++) { + char c = s.charAt(i); + if (c == '*' || c == '?' || c == '[' || c == '{') return true; } return false; } diff --git a/affected-tests-core/src/test/java/io/affectedtests/core/mapping/PathToClassMapperTest.java b/affected-tests-core/src/test/java/io/affectedtests/core/mapping/PathToClassMapperTest.java index 87e94ae..332cd97 100644 --- a/affected-tests-core/src/test/java/io/affectedtests/core/mapping/PathToClassMapperTest.java +++ b/affected-tests-core/src/test/java/io/affectedtests/core/mapping/PathToClassMapperTest.java @@ -106,6 +106,112 @@ void outOfScopeTestDirsFileRoutesToOutOfScopeBucket() { "An out-of-scope file must not leak into the unmapped bucket"); } + @Test + void outOfScopeTestDirsAcceptGlobPattern() { + // Users reach for glob patterns (e.g. "api-test/**") as often as + // literal prefixes because the `ignorePaths` knob one line above + // accepts globs too. The matcher must accept both shapes and fail + // closed on patterns it can't interpret, otherwise users silently + // lose their out-of-scope safety net while the config file still + // looks intentional. + AffectedTestsConfig globConfig = AffectedTestsConfig.builder() + .outOfScopeTestDirs(java.util.List.of("api-test/**")) + .build(); + PathToClassMapper globMapper = new PathToClassMapper(globConfig); + + Set changed = Set.of( + "api-test/src/test/java/com/example/api/FooSteps.java", + "api-test/src/test/resources/feature.feature"); + MappingResult result = globMapper.mapChangedFiles(changed); + + assertEquals(changed, result.outOfScopeFiles(), + "'api-test/**' glob must route every api-test file to the out-of-scope bucket"); + assertTrue(result.testClasses().isEmpty()); + assertTrue(result.unmappedChangedFiles().isEmpty()); + } + + @Test + void outOfScopeTestDirsGlobMatchesNestedModules() { + // Real multi-module repos put api-test under a services/ parent, + // so the matcher must support a `**/` prefix that crosses any + // number of directories — exactly the shape users copy from + // Ant/Gradle docs without thinking. A literal-prefix-only + // implementation would quietly miss the nested case. + AffectedTestsConfig globConfig = AffectedTestsConfig.builder() + .outOfScopeTestDirs(java.util.List.of("**/api-test/**")) + .build(); + PathToClassMapper globMapper = new PathToClassMapper(globConfig); + + Set changed = Set.of( + "services/orders/api-test/src/test/java/com/example/OrderSteps.java"); + MappingResult result = globMapper.mapChangedFiles(changed); + + assertEquals(changed, result.outOfScopeFiles(), + "Nested api-test dir must match the '**/api-test/**' glob"); + } + + @Test + void outOfScopeSourceDirsAcceptGlobPattern() { + // Symmetry: whatever shape outOfScopeTestDirs accepts, + // outOfScopeSourceDirs must accept too. Users configure both at + // the same time for mono-repo setups where a whole module is + // carved out of the unit/integration dispatch. + AffectedTestsConfig globConfig = AffectedTestsConfig.builder() + .outOfScopeSourceDirs(java.util.List.of("legacy-service/**")) + .build(); + PathToClassMapper globMapper = new PathToClassMapper(globConfig); + + Set changed = Set.of( + "legacy-service/src/main/java/com/example/LegacyFoo.java"); + MappingResult result = globMapper.mapChangedFiles(changed); + + assertEquals(changed, result.outOfScopeFiles(), + "'legacy-service/**' glob on outOfScopeSourceDirs must route under " + + "legacy-service/ to the out-of-scope bucket"); + assertTrue(result.productionClasses().isEmpty(), + "A file bucketed as out-of-scope must not also be mapped as production"); + } + + @Test + void outOfScopeLiteralPrefixStillWorksAfterGlobSupportAdded() { + // Regression guard: the existing literal-prefix shape documented + // in the README ("api-test/src/test/java") must keep working. + // Losing this would silently break every adopter who migrated to + // v2 before glob support existed. + AffectedTestsConfig prefixConfig = AffectedTestsConfig.builder() + .outOfScopeTestDirs(java.util.List.of( + "api-test/src/test/java", + "api-test/src/test/resources")) + .build(); + PathToClassMapper prefixMapper = new PathToClassMapper(prefixConfig); + + Set changed = Set.of( + "api-test/src/test/java/com/example/FooSteps.java", + "api-test/src/test/resources/feature.feature"); + MappingResult result = prefixMapper.mapChangedFiles(changed); + + assertEquals(changed, result.outOfScopeFiles()); + } + + @Test + void outOfScopeGlobDoesNotMatchSiblingDirectory() { + // Positive-boundary: 'api-test/**' must NOT match a directory + // whose name merely starts with 'api-test' ("api-test-utils/..."). + // The glob layer needs to preserve the same guarantee the literal + // prefix already had via trailing-slash normalisation. + AffectedTestsConfig globConfig = AffectedTestsConfig.builder() + .outOfScopeTestDirs(java.util.List.of("api-test/**")) + .build(); + PathToClassMapper globMapper = new PathToClassMapper(globConfig); + + Set changed = Set.of( + "api-test-utils/src/main/java/com/example/Foo.java"); + MappingResult result = globMapper.mapChangedFiles(changed); + + assertTrue(result.outOfScopeFiles().isEmpty(), + "'api-test/**' must not claim 'api-test-utils/...' — name prefixes are not paths"); + } + @Test void javaFileOutsideConfiguredDirsIsUnmapped() { // A .java file that does not sit under any configured source or test 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 33a017c..f941b63 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 @@ -724,6 +724,16 @@ static List renderExplainTrace(AffectedTestsConfig config, AffectedTests } lines.add("Outcome: " + outcome); + // Diagnostic hint: when out-of-scope dirs are configured but the + // bucket is empty, the config is almost certainly silently + // broken (wrong path, wrong glob shape, trailing-slash typo). + // We call it out inline so the operator sees the misconfiguration + // on the same trace that shows the buckets, rather than finding + // out 30 minutes later when a full suite runs that should have + // been skipped. Suppressed on empty-diff runs — there's nothing + // for the config to have bitten. + appendOutOfScopeHint(lines, config, result); + // The full action matrix is cheap to print (five rows) and // invaluable for debugging "why did my explicit setting not // win?" — so we always include it, not only on ambiguous @@ -740,6 +750,58 @@ static List renderExplainTrace(AffectedTestsConfig config, AffectedTests return lines; } + /** + * Emits the "configured but matched nothing" hint for out-of-scope + * dirs when the signal points to a silent misconfiguration. Kept + * 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).
    • + *
+ */ + static void appendOutOfScopeHint(List lines, + AffectedTestsConfig config, + AffectedTestsResult result) { + if (result.changedFiles().isEmpty()) { + return; + } + if (!result.buckets().outOfScopeFiles().isEmpty()) { + return; + } + int testEntries = config.outOfScopeTestDirs().size(); + int sourceEntries = config.outOfScopeSourceDirs().size(); + int totalEntries = testEntries + sourceEntries; + if (totalEntries == 0) { + return; + } + + List configuredKnobs = new ArrayList<>(2); + if (testEntries > 0) { + configuredKnobs.add("outOfScopeTestDirs"); + } + if (sourceEntries > 0) { + configuredKnobs.add("outOfScopeSourceDirs"); + } + String knobs = String.join(" / ", configuredKnobs); + String verb = configuredKnobs.size() == 1 ? "is" : "are"; + String entryWord = totalEntries == 1 ? "entry" : "entries"; + + lines.add("Hint: " + knobs + " " + verb + " configured (" + + totalEntries + " " + entryWord + ") but no file in the diff matched."); + lines.add(" Values are directory prefixes " + + "(e.g. 'api-test/src/test/java') or globs (e.g. 'api-test/**')."); + } + private static void appendSample(List lines, String label, Set files) { if (files.isEmpty()) { return; 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 ef53e10..4fb9d44 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 @@ -293,4 +293,136 @@ void outcomeLineDistinguishesFullSuiteSkippedAndSelected() { assertFalse(selectedTrace.contains("Outcome: FULL_SUITE"), "SELECTED result must not render FULL_SUITE on the Outcome line"); } + + @Test + void hintFiresWhenOutOfScopeTestDirsIsConfiguredButNoFilesMatch() { + // Sanity-testing the v1.9.13 adopter surfaced exactly this silent + // failure: a config entry like 'api-test/**' looked intentional + // but matched nothing, and the operator only noticed when a full + // CI run burned 30 minutes. The trace must call this out on every + // run where out-of-scope dirs are configured but zero diff files + // landed in the out-of-scope bucket. + AffectedTestsConfig config = AffectedTestsConfig.builder() + .outOfScopeTestDirs(List.of("api-test/**")) + .build(); + AffectedTestsResult result = 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("com.example.FooTest"), + new Buckets(Set.of(), Set.of(), + Set.of("src/main/java/com/example/Foo.java"), + Set.of(), Set.of()), + false, false, + Situation.DISCOVERY_SUCCESS, Action.SELECTED, + EscalationReason.NONE); + + String trace = joined(AffectedTestTask.renderExplainTrace(config, result)); + + assertTrue(trace.contains("Hint:"), + "A silent zero-match on configured out-of-scope dirs must produce a Hint line " + + "— that's the failure mode sanity testing caught on v1.9.13 adopters"); + assertTrue(trace.contains("outOfScopeTestDirs") || trace.contains("outOfScopeSourceDirs"), + "Hint must name the knob that didn't bite so the operator knows where to look"); + } + + @Test + void hintFiresWhenOutOfScopeSourceDirsIsConfiguredButNoFilesMatch() { + // Symmetry: the same misconfiguration on outOfScopeSourceDirs has + // the same silent failure mode, so the hint must fire for it too. + AffectedTestsConfig config = AffectedTestsConfig.builder() + .outOfScopeSourceDirs(List.of("legacy-service/**")) + .build(); + AffectedTestsResult result = 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("com.example.FooTest"), + new Buckets(Set.of(), Set.of(), + Set.of("src/main/java/com/example/Foo.java"), + Set.of(), Set.of()), + false, false, + Situation.DISCOVERY_SUCCESS, Action.SELECTED, + EscalationReason.NONE); + + String trace = joined(AffectedTestTask.renderExplainTrace(config, result)); + + assertTrue(trace.contains("Hint:"), + "Hint must fire for outOfScopeSourceDirs zero-match too, not just the test-dirs knob"); + } + + @Test + void hintDoesNotFireWhenOutOfScopeBucketIsPopulated() { + // Negative case: when the config IS biting, the trace already + // shows a non-zero 'out-of-scope' count and the hint would be + // noise. Keep the trace clean so the hint's rarity is itself a + // signal. + AffectedTestsConfig config = AffectedTestsConfig.builder() + .outOfScopeTestDirs(List.of("api-test/**")) + .build(); + Buckets buckets = new Buckets(Set.of(), + Set.of("api-test/src/test/java/com/example/FooSteps.java"), + Set.of(), Set.of(), Set.of()); + AffectedTestsResult result = new AffectedTestsResult( + Set.of(), Map.of(), + Set.of("api-test/src/test/java/com/example/FooSteps.java"), + Set.of(), Set.of(), + buckets, + 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 not render when the out-of-scope bucket is non-empty — " + + "the config is working and a hint would just be log noise"); + } + + @Test + void hintDoesNotFireWhenOutOfScopeDirsAreNotConfigured() { + // Negative case: zero-config users never opted in, so no hint is + // warranted. Rendering one here would train operators to ignore + // the hint line, defeating its purpose. + AffectedTestsConfig config = AffectedTestsConfig.builder().build(); + AffectedTestsResult result = 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("com.example.FooTest"), + new Buckets(Set.of(), Set.of(), + Set.of("src/main/java/com/example/Foo.java"), + Set.of(), Set.of()), + false, false, + Situation.DISCOVERY_SUCCESS, Action.SELECTED, + EscalationReason.NONE); + + String trace = joined(AffectedTestTask.renderExplainTrace(config, result)); + + assertFalse(trace.contains("Hint:"), + "Hint must stay silent for zero-config installs — nobody opted into out-of-scope dirs"); + } + + @Test + void hintDoesNotFireOnEmptyDiff() { + // Negative case: with no changed files the hint has nothing to + // diagnose — there's no diff for the config to have bitten. We + // silence it so an empty-diff CI re-run doesn't spam a false + // alarm every time master gets a merge and the next MR rebases. + AffectedTestsConfig config = AffectedTestsConfig.builder() + .outOfScopeTestDirs(List.of("api-test/**")) + .build(); + AffectedTestsResult result = new AffectedTestsResult( + Set.of(), Map.of(), + Set.of(), Set.of(), Set.of(), + Buckets.empty(), + false, true, + Situation.EMPTY_DIFF, Action.SKIPPED, + EscalationReason.NONE); + + String trace = joined(AffectedTestTask.renderExplainTrace(config, result)); + + assertFalse(trace.contains("Hint:"), + "Hint must stay silent when there are no changed files — nothing to diagnose"); + } }