From 653e56f74ff7fa638c242412c883617d121a80e1 Mon Sep 17 00:00:00 2001 From: vedanthvasudev Date: Wed, 22 Apr 2026 11:44:02 +0100 Subject: [PATCH] feat/v2-deprecations: Warn on legacy config knobs and flip docs to v2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 of the v2 rollout per docs/DESIGN-v2.md §4: surface deprecation warnings for the three legacy config knobs (runAllIfNoMatches, runAllOnNonJavaChange, excludePaths) without breaking any existing build, and flip every documentation example to the v2 vocabulary so new users only see the supported API. AffectedTestsConfig now exposes a deprecationWarnings() list populated at build time by detecting which nullable builder fields the caller explicitly touched. Zero-config installs produce zero warnings even though the effective config still resolves via the legacy-boolean shim — the list tracks caller intent, not the engine's resolution path, so builds that never typed the old names stay silent. The AffectedTestTask emits each warning through getLogger().warn() right before the summary line so the message sits adjacent to the config it describes. Each warning names the deprecated knob, the removal version (v2.0.0), and the v2 replacement (onEmptyDiff/onDiscoveryEmpty, onUnmappedFile, or ignorePaths) so operators can fix their build.gradle from the log alone. The README gains a full "Migrating from v1 config" section with a deprecation timeline, before/after table, worked example (single mode="ci" line replaces two legacy booleans in the common case), and a decision tree for each legacy knob. The main configuration example drops the "Legacy booleans (still supported)" block so new users only see v2 vocabulary, and the AffectedTestsExtension javadoc sample flips to mode + onXxx + ignorePaths + outOfScopeTestDirs. The behaviour reference table also updates its "override knob" column to name v2 knobs instead of the legacy flags. Seven new tests in AffectedTestsConfigTest lock down the contract: zero warnings for zero-config builds, one warning per legacy knob (each containing the knob name and the v2 replacement), stable ordering when multiple legacy knobs are set, and an immutability guard so the task cannot accidentally leak mutations to the list. --- README.md | 120 ++++++++++++++---- .../core/config/AffectedTestsConfig.java | 56 ++++++++ .../core/config/AffectedTestsConfigTest.java | 115 +++++++++++++++++ .../gradle/AffectedTestTask.java | 10 ++ .../gradle/AffectedTestsExtension.java | 15 ++- 5 files changed, 289 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index 2d7572d..7dc44d8 100644 --- a/README.md +++ b/README.md @@ -169,10 +169,6 @@ affectedTests { // Production source sets the plugin must treat as out-of-scope. outOfScopeSourceDirs = [] - // Back-compat alias for ignorePaths. Still honoured so existing - // configs keep working. (default: []) - // excludePaths = ["**/generated/**"] - // ---------------- Per-situation actions (v2) ---------------- // Each takes one of "selected" | "full_suite" | "skipped". @@ -183,16 +179,6 @@ affectedTests { onUnmappedFile = "full_suite" // the key MR-safety knob onDiscoveryEmpty = "full_suite" // belt-and-braces for CI - // ---------------- Legacy booleans (still supported) ---------------- - - // Translated into onEmptyDiff + onDiscoveryEmpty at build() time. - // (default: false) - runAllIfNoMatches = false - - // Translated into onUnmappedFile at build() time. - // (default: true — "run more, never run less") - runAllOnNonJavaChange = true - // ---------------- Discovery tuning ---------------- // Discovery strategies: "naming", "usage", "impl", "transitive" (default: all four) @@ -276,24 +262,112 @@ Every row below shows the situation the engine resolved, and the action applied | Only mapped production/test `.java` files | `DISCOVERY_SUCCESS` (or `DISCOVERY_EMPTY` if no tests map) | `SELECTED` | discovery tuning | | Only files matching `ignorePaths` (docs, LICENSE, CHANGELOG, images, generated) | `ALL_FILES_IGNORED` | `SKIPPED` | `onAllFilesIgnored` or `mode=strict` | | Only files under `outOfScopeTestDirs` / `outOfScopeSourceDirs` (e.g. api-test only) | `ALL_FILES_OUT_OF_SCOPE` | `SKIPPED` | `onAllFilesOutOfScope` | -| Any YAML / Gradle / Liquibase / `.java` outside configured dirs | `UNMAPPED_FILE` | `FULL_SUITE` (via `runAllOnNonJavaChange=true`) | `onUnmappedFile = "selected"` / `runAllOnNonJavaChange = false` | -| No changed files at all | `EMPTY_DIFF` | `SKIPPED` | `onEmptyDiff = "full_suite"` / `runAllIfNoMatches = true` / `mode = strict` | -| Mapping succeeds but discovery returns zero tests | `DISCOVERY_EMPTY` | `SKIPPED` — or `FULL_SUITE` if `mode=ci`/`strict` or `runAllIfNoMatches=true` | `onDiscoveryEmpty` / `mode` / `runAllIfNoMatches` | +| Any YAML / Gradle / Liquibase / `.java` outside configured dirs | `UNMAPPED_FILE` | `FULL_SUITE` (via `onUnmappedFile = "full_suite"`) | `onUnmappedFile = "selected"` | +| No changed files at all | `EMPTY_DIFF` | `SKIPPED` | `onEmptyDiff = "full_suite"` / `mode = strict` | +| Mapping succeeds but discovery returns zero tests | `DISCOVERY_EMPTY` | `SKIPPED` — or `FULL_SUITE` if `mode=ci`/`strict` | `onDiscoveryEmpty` / `mode` | | Mixed diff: Java + unmapped file | `UNMAPPED_FILE` (takes precedence) | `FULL_SUITE` | `onUnmappedFile` — set to `"selected"` to fall through to discovery | | `baseRef` not resolvable | `FAILED` | Hard error (prevents silent test skipping in CI) | — | | Not a git work tree / JGit I/O error | `FAILED` | Hard error | — | The `onUnmappedFile = "full_suite"` default follows the "run more, never run less" principle: a change to `application.yml` can alter production behaviour just as surely as a change to a `.java` file, so the plugin cannot safely pick a subset from an empty Java mapping. -### Migration from pre-v2 +### Migrating from v1 config + +Existing configs keep working — **no pipeline breaks today**. But the legacy knobs are deprecated and will be removed in **v2.0.0**. If any of these appear in your `build.gradle`, the plugin will print a `WARNING` on every `affectedTest` run naming the replacement: + +- `runAllIfNoMatches` +- `runAllOnNonJavaChange` +- `excludePaths` + +#### Deprecation timeline + +| Release | What happens | +|---|---| +| **v1.9.x and earlier** | Legacy knobs work silently. No warnings. | +| **v1.10.x** (this release) | Legacy knobs still work. A per-run `WARNING: [affected-tests] '' is deprecated…` names each one and its replacement. Zero-config users see nothing. | +| **v2.0.0** (next major) | Legacy knobs removed. `excludePaths`, `runAllIfNoMatches`, `runAllOnNonJavaChange` become unknown properties — Gradle will fail configuration. | + +#### Before / after + +| v1 config | v2 equivalent | Why | +|---|---|---| +| `runAllIfNoMatches = true` | `onEmptyDiff = "full_suite"` **and** `onDiscoveryEmpty = "full_suite"` | The v1 flag conflated two different situations ("git diff is empty" vs "discovery found nothing"). v2 splits them so you can e.g. skip empty-diff runs but still fall back to full suite when discovery fails. | +| `runAllIfNoMatches = false` (explicit) | Leave `onEmptyDiff` / `onDiscoveryEmpty` unset (defaults to `SKIPPED`) or set `mode = "local"` | Same effect, zero config. | +| `runAllOnNonJavaChange = true` | `onUnmappedFile = "full_suite"` | Single-situation knob, same semantics. | +| `runAllOnNonJavaChange = false` | `onUnmappedFile = "selected"` | Plugin treats the unmapped file as if absent and continues to discovery. | +| `excludePaths = ["**/generated/**"]` | `ignorePaths = ["**/generated/**"]` | Identical semantics — just a rename. | +| `excludePaths = []` (explicit empty) | **Delete the line** | v2's default `ignorePaths` list is broader (markdown, licence, changelog, images, generated). Explicitly empty discards all of it. | + +#### Worked example + +**Before (v1):** + +```groovy +affectedTests { + baseRef = "origin/master" + runAllIfNoMatches = true + runAllOnNonJavaChange = true + excludePaths = ["**/generated/**"] + transitiveDepth = 4 +} +``` -Existing configs keep working. Specifically: +**After (v2):** -- `runAllIfNoMatches = true` is translated into `onEmptyDiff = FULL_SUITE` **and** `onDiscoveryEmpty = FULL_SUITE` (the pre-v2 behaviour conflated them). -- `runAllOnNonJavaChange = true` is translated into `onUnmappedFile = FULL_SUITE`. -- `excludePaths` continues to work as an alias for `ignorePaths`. +```groovy +affectedTests { + baseRef = "origin/master" + mode = "ci" // one line replaces both booleans in 95% of cases + // excludePaths / ignorePaths unset — v2 default already covers generated/ + // transitiveDepth = 4 now the default, no need to set it +} +``` + +Or if you want every situation explicit (recommended for production pipelines where you care about each edge case): + +```groovy +affectedTests { + baseRef = "origin/master" + onEmptyDiff = "full_suite" + onAllFilesIgnored = "skipped" + onAllFilesOutOfScope = "skipped" + onUnmappedFile = "full_suite" + onDiscoveryEmpty = "full_suite" +} +``` + +#### Decision tree — "which replacement do I need?" + +``` +Did you set runAllIfNoMatches? +├─ No → nothing to do for this knob +├─ runAllIfNoMatches = true +│ └─ set onEmptyDiff = "full_suite" AND onDiscoveryEmpty = "full_suite" +│ (or just set mode = "ci" / mode = "strict" — both imply it) +└─ runAllIfNoMatches = false + └─ just delete the line (v2 default is SKIPPED) + +Did you set runAllOnNonJavaChange? +├─ No → nothing to do +├─ runAllOnNonJavaChange = true → set onUnmappedFile = "full_suite" +│ (or just delete the line — it's the default) +└─ runAllOnNonJavaChange = false → set onUnmappedFile = "selected" + +Did you set excludePaths? +├─ No → nothing to do +├─ excludePaths = [] → delete the line (you probably want the broader v2 default) +└─ excludePaths = [...] → rename to ignorePaths with the same list +``` + +#### What the summary log tells you during migration + +Every `affectedTest` run prints the outcome + situation + legacy flag (if applicable) on one line, so existing grep-based CI dashboards keep matching during and after the migration: + +``` +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). +``` -The escalation log line names **both** the v2 decision and the legacy flag that would have produced it, so existing grep-based CI dashboards don't break. +Both vocabularies appear — the v2 name (`onUnmappedFile=FULL_SUITE`) and the legacy name (`runAllOnNonJavaChange=true`) — so you can migrate without breaking any existing alert rules. ## Project Structure diff --git a/affected-tests-core/src/main/java/io/affectedtests/core/config/AffectedTestsConfig.java b/affected-tests-core/src/main/java/io/affectedtests/core/config/AffectedTestsConfig.java index defe22f..c9f4cdd 100644 --- a/affected-tests-core/src/main/java/io/affectedtests/core/config/AffectedTestsConfig.java +++ b/affected-tests-core/src/main/java/io/affectedtests/core/config/AffectedTestsConfig.java @@ -1,5 +1,6 @@ package io.affectedtests.core.config; +import java.util.ArrayList; import java.util.EnumMap; import java.util.List; import java.util.Map; @@ -64,6 +65,7 @@ public final class AffectedTestsConfig { private final Mode effectiveMode; private final Map situationActions; private final Map situationActionSources; + private final List deprecationWarnings; private AffectedTestsConfig(Builder builder) { this.baseRef = builder.baseRef; @@ -114,6 +116,48 @@ private AffectedTestsConfig(Builder builder) { ResolvedActions resolved = resolveSituationActions(builder, this.effectiveMode); this.situationActions = resolved.actions; this.situationActionSources = resolved.sources; + + this.deprecationWarnings = buildDeprecationWarnings(builder); + } + + /** + * Collects the list of human-readable deprecation warnings that the + * Gradle layer should surface for this build. One entry per legacy + * knob the caller explicitly touched — we detect "caller touched it" + * via the nullable backing field on the builder, so that zero-config + * users never see a warning even though their effective config still + * resolves through the legacy shim. + * + *

The returned list is in stable order (runAllIfNoMatches, + * runAllOnNonJavaChange, excludePaths) so log output is grep-stable + * across runs. Each message names the legacy knob, when it will be + * removed, and the v2 replacement — operators should be able to fix + * their config from the message alone without opening the docs. + */ + private static List buildDeprecationWarnings(Builder builder) { + List warnings = new ArrayList<>(3); + if (builder.runAllIfNoMatches != null) { + warnings.add("[affected-tests] 'runAllIfNoMatches' is deprecated and will be " + + "removed in v2.0.0. Replace it with per-situation actions " + + "(onEmptyDiff / onAllFilesIgnored / onAllFilesOutOfScope / onDiscoveryEmpty), " + + "each set to 'full_suite' to match runAllIfNoMatches=true, or 'skipped' " + + "to match runAllIfNoMatches=false. See README.md section " + + "'Migrating from v1 config'."); + } + if (builder.runAllOnNonJavaChange != null) { + warnings.add("[affected-tests] 'runAllOnNonJavaChange' is deprecated and will be " + + "removed in v2.0.0. Replace it with 'onUnmappedFile' " + + "(full_suite / selected / skipped). See README.md section " + + "'Migrating from v1 config'."); + } + if (builder.excludePaths != null) { + warnings.add("[affected-tests] 'excludePaths' is deprecated and will be removed " + + "in v2.0.0. Rename it to 'ignorePaths' — identical semantics, and " + + "leaving it unset picks up the broader v2 default list " + + "(markdown, generated/, licence/changelog, images). See README.md " + + "section 'Migrating from v1 config'."); + } + return List.copyOf(warnings); } /** Parallel pair returned from the situation-action resolver. */ @@ -372,6 +416,18 @@ public ActionSource actionSourceFor(Situation situation) { */ public Map situationActionSources() { return situationActionSources; } + /** + * Human-readable deprecation warnings the caller should surface to + * its users (for the Gradle plugin: via {@code Logger.warn}). Empty + * when the caller only uses v2 knobs. One entry per legacy setter + * that was explicitly invoked — callers that rely on the pre-v2 + * defaults (zero config) get zero warnings. + * + * @return an unmodifiable list of warning strings; empty when the + * config was built without touching any legacy setter + */ + public List deprecationWarnings() { return deprecationWarnings; } + /** Creates a builder with sensible defaults. */ public static Builder builder() { return new Builder(); diff --git a/affected-tests-core/src/test/java/io/affectedtests/core/config/AffectedTestsConfigTest.java b/affected-tests-core/src/test/java/io/affectedtests/core/config/AffectedTestsConfigTest.java index 271b959..528300b 100644 --- a/affected-tests-core/src/test/java/io/affectedtests/core/config/AffectedTestsConfigTest.java +++ b/affected-tests-core/src/test/java/io/affectedtests/core/config/AffectedTestsConfigTest.java @@ -269,4 +269,119 @@ void configIsImmutable() { assertThrows(UnsupportedOperationException.class, () -> config.excludePaths().add("foo")); } + + // ----------------------------------------------------------------- + // Phase 2 — deprecation warnings for legacy v1 knobs. + // + // The warnings exist to nudge callers onto the v2 config without + // breaking their build. The core guarantee is: *only* explicit uses + // of a legacy setter trigger a warning, and every warning names the + // replacement knob so operators can fix their build.gradle without + // reading the docs first. + // ----------------------------------------------------------------- + + @Test + void deprecationWarningsAreEmptyForZeroConfigBuild() { + // Zero-config install must not emit warnings even though the + // effective config still resolves via the legacy-boolean shim: + // the warning tracks *caller intent* (did they type the old + // name), not the resolution path the engine walked. + AffectedTestsConfig config = AffectedTestsConfig.builder().build(); + assertTrue(config.deprecationWarnings().isEmpty(), + "Zero-config installs must not spam a deprecation warning — " + + "the legacy boolean shim is implementation detail, " + + "the caller never typed runAllIfNoMatches anywhere"); + } + + @Test + void deprecationWarningFiresWhenLegacyRunAllIfNoMatchesIsSet() { + AffectedTestsConfig config = AffectedTestsConfig.builder() + .runAllIfNoMatches(false) + .build(); + String warning = singleWarning(config, + "runAllIfNoMatches must produce exactly one warning"); + assertTrue(warning.contains("runAllIfNoMatches"), + "Warning must name the deprecated knob so a build.gradle 'grep' can " + + "find it: " + warning); + assertTrue(warning.contains("onEmptyDiff") + && warning.contains("onDiscoveryEmpty"), + "Warning must name the v2 replacements so the operator can migrate " + + "from the log alone, without opening the docs: " + warning); + } + + @Test + void deprecationWarningFiresWhenLegacyRunAllOnNonJavaChangeIsSet() { + AffectedTestsConfig config = AffectedTestsConfig.builder() + .runAllOnNonJavaChange(true) + .build(); + String warning = singleWarning(config, + "runAllOnNonJavaChange must produce exactly one warning"); + assertTrue(warning.contains("runAllOnNonJavaChange"), + "Warning must name the deprecated knob: " + warning); + assertTrue(warning.contains("onUnmappedFile"), + "Warning must name the v2 replacement (onUnmappedFile): " + warning); + } + + @Test + void deprecationWarningFiresWhenLegacyExcludePathsIsSet() { + AffectedTestsConfig config = AffectedTestsConfig.builder() + .excludePaths(List.of("**/generated/**")) + .build(); + String warning = singleWarning(config, + "excludePaths must produce exactly one warning"); + assertTrue(warning.contains("excludePaths")); + assertTrue(warning.contains("ignorePaths"), + "Warning must name the rename target (ignorePaths): " + warning); + } + + @Test + void deprecationWarningsListThreeEntriesWhenAllLegacyKnobsAreSet() { + // All three legacy knobs at once — the log must name each one + // individually so a caller with a chain of legacy config lines + // sees a full audit, not just the first deprecation. + AffectedTestsConfig config = AffectedTestsConfig.builder() + .runAllIfNoMatches(true) + .runAllOnNonJavaChange(true) + .excludePaths(List.of("**/generated/**")) + .build(); + assertEquals(3, config.deprecationWarnings().size(), + "Three legacy knobs set => three distinct warnings"); + // Stable order keeps CI log greps deterministic across runs. + assertTrue(config.deprecationWarnings().get(0).contains("runAllIfNoMatches")); + assertTrue(config.deprecationWarnings().get(1).contains("runAllOnNonJavaChange")); + assertTrue(config.deprecationWarnings().get(2).contains("excludePaths")); + } + + @Test + void deprecationWarningsAreUnmodifiable() { + AffectedTestsConfig config = AffectedTestsConfig.builder() + .runAllIfNoMatches(true) + .build(); + assertThrows(UnsupportedOperationException.class, + () -> config.deprecationWarnings().add("mutated"), + "List must be unmodifiable so the Gradle task cannot accidentally " + + "leak mutations to a shared config instance"); + } + + @Test + void deprecationWarningsDoNotFireForV2KnobsEvenWhenSet() { + // v2-native config: mode, onXxx, ignorePaths, outOfScope*. + // None of these should trigger a deprecation warning. + AffectedTestsConfig config = AffectedTestsConfig.builder() + .mode(Mode.CI) + .onUnmappedFile(Action.FULL_SUITE) + .onDiscoveryEmpty(Action.FULL_SUITE) + .onAllFilesIgnored(Action.SKIPPED) + .ignorePaths(List.of("**/generated/**", "*.md")) + .outOfScopeTestDirs(List.of("api-test/src/test/java")) + .build(); + assertTrue(config.deprecationWarnings().isEmpty(), + "v2-native config must never emit a deprecation — " + + "otherwise migrators have nothing to aim for"); + } + + private static String singleWarning(AffectedTestsConfig config, String message) { + assertEquals(1, config.deprecationWarnings().size(), message); + return config.deprecationWarnings().get(0); + } } 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 7f1a5e9..33a017c 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 @@ -322,6 +322,16 @@ public void runAffectedTests() { AffectedTestsConfig config = buildConfig(); + // Surface each deprecation warning exactly once before the + // engine runs so the message sits adjacent to the config it + // describes. Using {@code warn} (not {@code lifecycle}) keeps + // the warning visible in CI log excerpts that filter below + // lifecycle, and lets build-scan deprecation dashboards pick + // it up as a first-class warning rather than an info line. + for (String warning : config.deprecationWarnings()) { + getLogger().warn(warning); + } + AffectedTestsEngine engine = new AffectedTestsEngine(config, projectDir); AffectedTestsResult result = engine.run(); diff --git a/affected-tests-gradle/src/main/java/io/affectedtests/gradle/AffectedTestsExtension.java b/affected-tests-gradle/src/main/java/io/affectedtests/gradle/AffectedTestsExtension.java index 7ce9fe0..c226f6a 100644 --- a/affected-tests-gradle/src/main/java/io/affectedtests/gradle/AffectedTestsExtension.java +++ b/affected-tests-gradle/src/main/java/io/affectedtests/gradle/AffectedTestsExtension.java @@ -11,15 +11,22 @@ * affectedTests { * baseRef = "origin/master" * includeUncommitted = true - * runAllIfNoMatches = false + * // v2: per-situation actions (replaces runAllIfNoMatches / runAllOnNonJavaChange). + * // See README.md "Migrating from v1 config" for the full table. + * mode = "ci" + * onEmptyDiff = "full_suite" + * onAllFilesOutOfScope = "skipped" + * onUnmappedFile = "full_suite" + * onDiscoveryEmpty = "full_suite" + * ignorePaths = ["**/generated/**"] + * outOfScopeTestDirs = ["api-test/src/test/java"] * strategies = ["naming", "usage", "impl", "transitive"] - * transitiveDepth = 2 + * transitiveDepth = 4 * testSuffixes = ["Test", "IT", "ITTest", "IntegrationTest"] * sourceDirs = ["src/main/java"] * testDirs = ["src/test/java"] - * excludePaths = ["**/generated/**"] * includeImplementationTests = true - * implementationNaming = ["Impl"] + * implementationNaming = ["Impl", "Default"] * } * } */