From 04c5818ee024784b0670689a663d9fed389eb8ad Mon Sep 17 00:00:00 2001 From: vedanthvasudev Date: Wed, 22 Apr 2026 23:43:46 +0100 Subject: [PATCH] =?UTF-8?q?feat/v2.0-remove-legacy-knobs:=20Phase=203=20?= =?UTF-8?q?=E2=80=94=20delete=20the=20v1=20DSL=20surface=20for=20v2.0.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the breaking release. The three deprecation-warned v1 knobs (`runAllIfNoMatches`, `runAllOnNonJavaChange`, `excludePaths`) are gone from every public surface — extension, task, core config, and builder — along with the two `ActionSource` tiers (`LEGACY_BOOLEAN`, `HARDCODED_DEFAULT`) and the `deprecationWarnings()` list that existed only to carry v1 nags. After this commit the resolver is strictly two-tier (explicit `onXxx` > mode default), which is what the v2 design doc has always pointed at. Why now: we promised Phase 3 in DESIGN-v2.md and burned a full v1.9.x minor cycle warning about the removal so consumers had runway. Keeping the old surface around any longer just invites new code to grow against it and makes the mode-default story permanently fuzzy. Why this shape, not a softer one: - Reflective regression test (`legacyDslKnobsNoLongerExistInV2`) pins the absence of the accessors on every surface — extension, task, config, and builder — so a well-intentioned "restore the getter for back-compat" revert fails at build time instead of silently re-opening the v1 API. - Migration table in README/CHANGELOG explicitly calls out that `runAllIfNoMatches = false` is *not* a safe no-op delete in CI — the new zero-config `mode = "auto"` resolves to `ci`, and `ci` escalates `DISCOVERY_EMPTY` to `FULL_SUITE`. A v1 pipeline that set the flag specifically to suppress that branch would regress silently. Users must either pin `mode = "local"` or set both `onEmptyDiff = "skipped"` and `onDiscoveryEmpty = "skipped"`. - `--explain` sample output refreshed to match v2 reality (AUTO mode, `DISCOVERY_INCOMPLETE` row, no "pre-v2 defaults" string) so the README doesn't ship with a trace operators can no longer reproduce. - Architecture diagram (`docs/architecture.mmd` + regenerated SVG) trimmed to the two-tier ladder so the picture matches the code. The engine test `unmappedFileEscalationCanBeDisabled` now pins `mode(Mode.LOCAL)`. The migration it proves — opting out of the unmapped-file escalation via `onUnmappedFile = SELECTED` — causes a yaml-only diff to fall through to `DISCOVERY_EMPTY`, and under the new zero-config `mode = "auto"` that escalates to `FULL_SUITE` on any `CI=true` runner (GitHub Actions, GitLab, etc.). The test would have passed locally and failed on CI forever — the v2 behaviour change is correct, the test assertion is correct, but the test needs the same `mode = "local"` pin we tell v1 users to apply. This is the exact migration path the README now documents for `runAllOnNonJavaChange = false` users, so the test doubles as executable documentation. Test coverage follows the deletion: legacy-tier tests removed, priority tests rewritten against the two-tier resolver, engine/explain/log-format tests renamed away from the v1 vocabulary, and every test method that used to reach for the old knobs now uses `ignorePaths`, `onEmptyDiff`, `onDiscoveryEmpty`, or `onUnmappedFile` instead. Closes Phase 3 of DESIGN-v2.md. --- CHANGELOG.md | 70 ++++ README.md | 62 ++-- .../core/AffectedTestsEngine.java | 51 ++- .../io/affectedtests/core/config/Action.java | 11 +- .../core/config/ActionSource.java | 23 +- .../core/config/AffectedTestsConfig.java | 305 +++--------------- .../io/affectedtests/core/config/Mode.java | 23 +- .../affectedtests/core/config/Situation.java | 15 +- .../core/mapping/PathToClassMapper.java | 5 +- .../core/AffectedTestsEngineTest.java | 39 ++- .../core/config/AffectedTestsConfigTest.java | 268 +++------------ .../core/mapping/PathToClassMapperTest.java | 2 +- .../gradle/AffectedTestTask.java | 124 ++----- .../gradle/AffectedTestsExtension.java | 52 +-- .../gradle/AffectedTestsPlugin.java | 21 +- .../AffectedTestTaskExplainFormatTest.java | 38 +-- .../gradle/AffectedTestTaskLogFormatTest.java | 12 +- .../gradle/AffectedTestsPluginTest.java | 84 ++++- docs/DESIGN-v2.md | 21 +- docs/architecture.mmd | 6 +- docs/architecture.svg | 2 +- 21 files changed, 408 insertions(+), 826 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bae6e58..407f80c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,76 @@ adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Removed — v2.0 breaking release: legacy v1 knobs are gone + +The three v1 configuration knobs that were deprecated across the v1.9.x +line have been removed from the plugin entirely. This is the long-promised +v2.0 breaking release (Phase 3 of `docs/DESIGN-v2.md`). The v2 +situation/action DSL is now the only configuration surface, which keeps +`--explain` output honest, the priority-ladder resolver simple, and the +migration path explicit for operators who have been getting deprecation +warnings since v1.9.18. + +- **Removed `runAllIfNoMatches` from the Gradle DSL and + `AffectedTestsConfig`.** The v2 replacements + (`onEmptyDiff`, `onDiscoveryEmpty`) have been available since v1.9.18 + and cover the same behaviour with independent per-situation control. + Callers who set `runAllIfNoMatches = true` in v1 must now set + `onEmptyDiff = "FULL_SUITE"` and/or `onDiscoveryEmpty = "FULL_SUITE"` + depending on whether they want the safety net on empty diffs, + post-discovery misses, or both. +- **Removed `runAllOnNonJavaChange` from the Gradle DSL and + `AffectedTestsConfig`.** The v2 replacement is `onUnmappedFile`, which + has been available since v1.9.18. `runAllOnNonJavaChange = true` + maps to `onUnmappedFile = "FULL_SUITE"`; the explicit opt-out case + (`runAllOnNonJavaChange = false`) maps to `onUnmappedFile = "SELECTED"`. +- **Removed `excludePaths` — use `ignorePaths` instead.** `excludePaths` + was the pre-v1.9.18 name and has been an alias of `ignorePaths` since + v1.9.18. In v2 it is gone; callers must rename to `ignorePaths`. +- **Removed `ActionSource.LEGACY_BOOLEAN` and + `ActionSource.HARDCODED_DEFAULT`.** The priority ladder the resolver + walks is now a two-tier structure: explicit `onXxx` > mode default. + Zero-config installs always resolve to a concrete mode (`LOCAL` / `CI` + via `Mode.AUTO` detection, or `STRICT` when pinned) so there is no + longer a hardcoded-default fall-through to report. `--explain` output + drops the corresponding `(source: legacy boolean …)` and + `(source: pre-v2 hardcoded default)` phrases — anything still printing + those strings is now either `(source: explicit onXxx setting)` or + `(source: mode default)`. +- **Removed `AffectedTestsConfig.deprecationWarnings()` and the startup + warning loop.** With the legacy knobs gone, there is nothing left to + warn about. The Gradle task's "deprecation" log line disappears in + v2, which is itself a (small) behaviour change: integrations that + grep for `runAllIfNoMatches is deprecated` in CI logs must drop that + check. + +**Migration** — the mapping is mechanical and the full migration guide +lives in `docs/DESIGN-v2.md`, but the short version is: + +| v1 (removed in v2.0) | v2 replacement | +| -------------------------------------- | ------------------------------------------------ | +| `runAllIfNoMatches = true` | `onEmptyDiff = "FULL_SUITE"` + `onDiscoveryEmpty = "FULL_SUITE"` | +| `runAllIfNoMatches = false` | `mode = "local"` **or** explicit `onEmptyDiff = "SKIPPED"` + `onDiscoveryEmpty = "SKIPPED"`. Simply deleting the line is not equivalent — under the new zero-config `mode = "auto"` the `ci` profile escalates `DISCOVERY_EMPTY` to `FULL_SUITE`, so a v1 pipeline that used `false` to suppress the no-match full-suite will regress silently unless one of the two explicit paths is picked. | +| `runAllOnNonJavaChange = true` | `onUnmappedFile = "FULL_SUITE"` | +| `runAllOnNonJavaChange = false` | `onUnmappedFile = "SELECTED"` | +| `excludePaths = […]` | `ignorePaths = […]` | + +Regression coverage: + +- `AffectedTestsPluginTest.legacyDslKnobsNoLongerExistInV2` reflectively + pins the absence of the three legacy getters on + `AffectedTestsExtension` so a well-intentioned "restore the getter + for back-compat" revert fails the build instead of silently + re-opening the v1 API surface. +- `AffectedTestsConfigTest.actionSourceReflectsResolutionTierOrdering` + pins that the only two `ActionSource` values that survive in v2 are + `EXPLICIT` and `MODE_DEFAULT`, so a resolver refactor cannot + reintroduce the old tiers without being caught. +- `AffectedTestTaskExplainFormatTest.actionSourceSurfacesModeDefaultForZeroConfig` + pins the `--explain` trace wording for zero-config installs under + the new two-tier model, so the pre-v2 `(source: pre-v2 hardcoded + default)` string cannot sneak back into CI logs. + ### Added — discovery-incomplete signal + nested `gradlew` timeout (v1.9.22) The two deferred findings from the v1.9.21 reliability batch: both close diff --git a/README.md b/README.md index 64dc4a1..78f9263 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ plugins { ./gradlew affectedTest --explain ``` -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. +Prints the full decision trace — bucket counts, situation, action, and the tier of the priority ladder (explicit `onXxx` / mode default) 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 *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. @@ -38,7 +38,7 @@ Sample output: ``` === Affected Tests — decision trace (--explain) === Base ref: origin/master -Mode: unset (effective: n/a (pre-v2 defaults)) +Mode: AUTO (effective: LOCAL) Changed files: 3 Buckets: ignored 1 @@ -50,15 +50,16 @@ Buckets: production sample: src/main/java/com/example/Foo.java unmapped sample: build.gradle Situation: UNMAPPED_FILE -Action: FULL_SUITE (source: pre-v2 hardcoded default) -Outcome: FULL_SUITE — runAllOnNonJavaChange=true / onUnmappedFile=FULL_SUITE — non-Java or unmapped file in diff +Action: FULL_SUITE (source: mode default) +Outcome: FULL_SUITE — onUnmappedFile=FULL_SUITE — non-Java or unmapped file in diff Action matrix (situation → action [source]): - EMPTY_DIFF SKIPPED [pre-v2 hardcoded default] - ALL_FILES_IGNORED SKIPPED [pre-v2 hardcoded default] - ALL_FILES_OUT_OF_SCOPE SKIPPED [pre-v2 hardcoded default] - UNMAPPED_FILE FULL_SUITE [pre-v2 hardcoded default] - DISCOVERY_EMPTY SKIPPED [pre-v2 hardcoded default] - DISCOVERY_SUCCESS SELECTED [explicit onXxx setting] + EMPTY_DIFF SKIPPED [mode default] + ALL_FILES_IGNORED SKIPPED [mode default] + ALL_FILES_OUT_OF_SCOPE SKIPPED [mode default] + UNMAPPED_FILE FULL_SUITE [mode default] + DISCOVERY_INCOMPLETE SELECTED [mode default] + DISCOVERY_EMPTY SKIPPED [mode default] + DISCOVERY_SUCCESS SELECTED [explicit onXxx setting] === end --explain === ``` @@ -96,11 +97,11 @@ Every `affectedTest` run prints exactly one summary line in the form `Affected T ``` 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: FULL_SUITE (UNMAPPED_FILE) — 1 changed file(s); running full suite (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. +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. 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`: @@ -142,20 +143,20 @@ The preview caps at five FQNs per module; `--info` still surfaces the full per-F | `FULL_SUITE` | Run the entire test suite (no `--tests` filter). | | `SKIPPED` | Exit 0 without running tests. | -Every situation gets an independently-configurable action. The matrix is resolved in strict priority order: **explicit `onXxx`** setting → **legacy boolean** (`runAllIfNoMatches` / `runAllOnNonJavaChange`) → **`mode` profile default** → **pre-v2 hardcoded default**. So nothing you configure today silently regresses tomorrow. +Every situation gets an independently-configurable action. The matrix is resolved in strict priority order: **explicit `onXxx`** setting → **`mode` profile default**. Zero-config installs always resolve to a concrete mode via `Mode.AUTO` detection, so nothing you configure today silently regresses tomorrow. ### Mode profiles `mode` seeds the defaults for situations you haven't explicitly configured: -| Mode | `EMPTY_DIFF` | `ALL_FILES_IGNORED` | `ALL_FILES_OUT_OF_SCOPE` | `UNMAPPED_FILE` | `DISCOVERY_EMPTY` | -|---|---|---|---|---|---| -| `local` | SKIPPED | SKIPPED | SKIPPED | FULL_SUITE | SKIPPED | -| `ci` | SKIPPED | SKIPPED | SKIPPED | FULL_SUITE | **FULL_SUITE** | -| `strict` | FULL_SUITE | FULL_SUITE | SKIPPED | FULL_SUITE | FULL_SUITE | +| Mode | `EMPTY_DIFF` | `ALL_FILES_IGNORED` | `ALL_FILES_OUT_OF_SCOPE` | `UNMAPPED_FILE` | `DISCOVERY_EMPTY` | `DISCOVERY_INCOMPLETE` | +|---|---|---|---|---|---|---| +| `local` | SKIPPED | SKIPPED | SKIPPED | FULL_SUITE | SKIPPED | SELECTED | +| `ci` | SKIPPED | SKIPPED | SKIPPED | FULL_SUITE | **FULL_SUITE** | **FULL_SUITE** | +| `strict` | FULL_SUITE | FULL_SUITE | SKIPPED | FULL_SUITE | FULL_SUITE | FULL_SUITE | | `auto` | Detects `CI` / `GITHUB_ACTIONS` / `GITLAB_CI` / `JENKINS_HOME` and resolves to `ci` or `local`. | -Leaving `mode` unset keeps the pre-v2 zero-config behaviour (same as `local` plus the legacy `runAllOnNonJavaChange=true` safety net). +Leaving `mode` unset picks `auto`, which resolves to `local` or `ci` depending on the environment. The `UNMAPPED_FILE → FULL_SUITE` safety net is the default in every built-in mode, so a zero-config install still escalates on unmapped files without any DSL wiring. ## Configuration @@ -339,7 +340,7 @@ The `onUnmappedFile = "full_suite"` default follows the "run more, never run les ### 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: +**v2.0.0 removed the three v1 legacy knobs.** If any of these still appear in your `build.gradle`, Gradle configuration will fail with an unknown-property error before the `affectedTest` task runs: - `runAllIfNoMatches` - `runAllOnNonJavaChange` @@ -349,16 +350,16 @@ Existing configs keep working — **no pipeline breaks today**. But the legacy k | 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. | +| **v1.9.x and earlier** | Legacy knobs worked silently. No warnings. | +| **v1.10.x – v1.11.x** | Legacy knobs still worked. A per-run `WARNING: [affected-tests] '' is deprecated…` named each one and its replacement. Zero-config users saw nothing. | +| **v2.0.0** (this release) | **Legacy knobs removed.** `excludePaths`, `runAllIfNoMatches`, `runAllOnNonJavaChange` are unknown properties — Gradle configuration fails. Migrate using the table below. | #### 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. | +| `runAllIfNoMatches = false` (explicit) | **Set `mode = "local"`, or pin `onEmptyDiff = "skipped"` + `onDiscoveryEmpty = "skipped"` explicitly.** Do *not* just delete the line — in v2 the zero-config `mode = "auto"` resolves to `ci` in a CI runner, and `ci` escalates `DISCOVERY_EMPTY` to `FULL_SUITE`. A v1 pipeline that set `runAllIfNoMatches = false` specifically to prevent the discovery-empty branch from flipping to full suite will start running the full suite on every no-match MR unless one of these two knobs is pinned. | | `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. | @@ -411,7 +412,10 @@ Did you set runAllIfNoMatches? │ └─ 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) + └─ DO NOT just delete the line. In v2, zero-config AUTO-in-CI + escalates DISCOVERY_EMPTY to FULL_SUITE. Either: + · pin mode = "local", OR + · pin onEmptyDiff = "skipped" + onDiscoveryEmpty = "skipped" Did you set runAllOnNonJavaChange? ├─ No → nothing to do @@ -425,15 +429,15 @@ Did you set excludePaths? └─ excludePaths = [...] → rename to ignorePaths with the same list ``` -#### What the summary log tells you during migration +#### What the summary log tells you after 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: +Every `affectedTest` run prints the outcome + situation + the v2 knob that fired on one line, so CI dashboards can key off a stable vocabulary: ``` -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: FULL_SUITE (UNMAPPED_FILE) — 1 changed file(s); running full suite (onUnmappedFile=FULL_SUITE — non-Java or unmapped file in diff). ``` -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. +**Breaking-change note for grep-based alerting:** v1 summary lines carried both the v1 name (`runAllOnNonJavaChange=true`) and the v2 name (`onUnmappedFile=FULL_SUITE`) to ease migration. v2.0 drops the v1 vocabulary. Any CI alert rules keyed on `runAllIfNoMatches=true`, `runAllOnNonJavaChange=true`, or the `[affected-tests] '…' is deprecated` warning must be updated to the v2 knob names. ## Project Structure diff --git a/affected-tests-core/src/main/java/io/affectedtests/core/AffectedTestsEngine.java b/affected-tests-core/src/main/java/io/affectedtests/core/AffectedTestsEngine.java index cc5c52a..9557f53 100644 --- a/affected-tests-core/src/main/java/io/affectedtests/core/AffectedTestsEngine.java +++ b/affected-tests-core/src/main/java/io/affectedtests/core/AffectedTestsEngine.java @@ -49,19 +49,19 @@ public AffectedTestsEngine(AffectedTestsConfig config, Path projectDir) { /** * Why a result flipped to {@code runAll = true}, or {@link #NONE} when no - * escalation occurred. Preserved as a v1 back-compat surface so that - * existing Gradle-task callers keep receiving the same reason codes — - * the v2 engine now records a richer {@link Situation}/{@link Action} - * pair internally and derives the legacy code from them. + * escalation occurred. Derived one-for-one from the + * {@link Situation}/{@link Action} pair the engine resolved, and + * surfaced on the result record so Gradle-task log formatting and + * {@code --explain} can describe the escalation without re-deriving + * the mapping. */ public enum EscalationReason { /** No escalation — either a filtered selection or a plain "nothing to do" result. */ NONE, /** * Git produced an empty change set (no files differ between - * {@code baseRef} and the working tree) and {@code runAllIfNoMatches} - * was true. Derived from {@link Situation#EMPTY_DIFF} + - * {@link Action#FULL_SUITE}. + * {@code baseRef} and the working tree) and the action for + * {@link Situation#EMPTY_DIFF} resolved to {@link Action#FULL_SUITE}. */ RUN_ALL_ON_EMPTY_CHANGESET, /** @@ -80,8 +80,7 @@ public enum EscalationReason { /** * Every file in the diff matched {@link AffectedTestsConfig#ignorePaths()} * and the action for {@link Situation#ALL_FILES_IGNORED} resolved - * to {@link Action#FULL_SUITE}. v2-only — no legacy boolean - * produces this code. + * to {@link Action#FULL_SUITE}. */ RUN_ALL_ON_ALL_FILES_IGNORED, /** @@ -89,7 +88,7 @@ public enum EscalationReason { * {@link AffectedTestsConfig#outOfScopeTestDirs()} or * {@link AffectedTestsConfig#outOfScopeSourceDirs()} and the * action for {@link Situation#ALL_FILES_OUT_OF_SCOPE} resolved to - * {@link Action#FULL_SUITE}. v2-only. + * {@link Action#FULL_SUITE}. */ RUN_ALL_ON_ALL_FILES_OUT_OF_SCOPE, /** @@ -98,8 +97,7 @@ public enum EscalationReason { * so discovery may have under-reported its selection. The action * for {@link Situation#DISCOVERY_INCOMPLETE} resolved to * {@link Action#FULL_SUITE}, typically via {@link io.affectedtests.core.config.Mode#CI} - * or {@link io.affectedtests.core.config.Mode#STRICT} defaults - * or an explicit {@code runAllIfNoMatches=true}. v2-only. + * or {@link io.affectedtests.core.config.Mode#STRICT} defaults. */ RUN_ALL_ON_DISCOVERY_INCOMPLETE } @@ -163,9 +161,9 @@ public int total() { * @param action the resolved {@link Action} for * {@link #situation}; one of SELECTED, * FULL_SUITE, SKIPPED - * @param escalationReason legacy reason code kept in sync with - * {@link #situation}/{@link #action} for - * v1 callers; see {@link EscalationReason} + * @param escalationReason reason code derived from the + * {@link #situation}/{@link #action} + * pair; see {@link EscalationReason} */ public record AffectedTestsResult( Set testClassFqns, @@ -243,11 +241,11 @@ public AffectedTestsResult run() { // Pre-fix, this slid through to discovery with empty // {@code productionClasses} and {@code testClasses}, every // strategy returned empty, and the engine escalated via - // {@link Situation#DISCOVERY_EMPTY} — which under the default - // {@code runAllIfNoMatches=true} routed to {@code FULL_SUITE}. - // The result: a pure "docs + api-test" MR ran the whole unit - // suite despite the user having told the plugin "these dirs - // don't influence tests". Routing to {@link Situation#ALL_FILES_OUT_OF_SCOPE} + // {@link Situation#DISCOVERY_EMPTY} — which under the CI + // default routed to {@code FULL_SUITE}. The result: a pure + // "docs + api-test" MR ran the whole unit suite despite the + // user having told the plugin "these dirs don't influence + // tests". Routing to {@link Situation#ALL_FILES_OUT_OF_SCOPE} // instead lets {@code onAllFilesOutOfScope} (the stronger // operator signal when both signals disagree) decide, which // defaults to {@link Action#SKIPPED} in every built-in mode. @@ -272,10 +270,8 @@ public AffectedTestsResult run() { mapping.unmappedChangedFiles().size(), action, examples); // SELECTED here means "ignore the unmapped file, proceed with // discovery on whatever production/test files were in the - // diff" — this is the behaviour legacy - // {@code runAllOnNonJavaChange=false} callers expect, and the - // only way to express it in the v2 model without inventing a - // second fallthrough enum value. + // diff" — the opt-out for operators who don't want + // non-Java changes to escalate to a full suite. if (action != Action.SELECTED) { return emptyResult(Situation.UNMAPPED_FILE, action, changedFiles, mapping.productionClasses(), mapping.testClasses(), buckets); @@ -412,8 +408,7 @@ public AffectedTestsResult run() { *

When {@link Situation#UNMAPPED_FILE} resolves to * {@link Action#SELECTED} the engine deliberately does not * route through here — it continues into discovery so the diff's - * Java files still get analysed, matching the pre-v2 behaviour of - * {@code runAllOnNonJavaChange=false}. + * Java files still get analysed. */ private AffectedTestsResult resolveAmbiguous(Situation situation, Set changedFiles, @@ -455,11 +450,11 @@ private AffectedTestsResult emptyResult(Situation situation, Action action, skipped, situation, action, - legacyReason(situation, action) + escalationReason(situation, action) ); } - private static EscalationReason legacyReason(Situation situation, Action action) { + private static EscalationReason escalationReason(Situation situation, Action action) { if (action != Action.FULL_SUITE) return EscalationReason.NONE; return switch (situation) { case EMPTY_DIFF -> EscalationReason.RUN_ALL_ON_EMPTY_CHANGESET; diff --git a/affected-tests-core/src/main/java/io/affectedtests/core/config/Action.java b/affected-tests-core/src/main/java/io/affectedtests/core/config/Action.java index 2fafa8a..6057efe 100644 --- a/affected-tests-core/src/main/java/io/affectedtests/core/config/Action.java +++ b/affected-tests-core/src/main/java/io/affectedtests/core/config/Action.java @@ -17,12 +17,11 @@ * honestly (selection succeeded, set happened to be empty) rather than * claim the run was skipped. *

  • {@link #FULL_SUITE} — flip to running every test the project knows - * about. The legacy {@code runAllIfNoMatches} / {@code runAllOnNonJavaChange} - * booleans map onto this action via the shim in - * {@link AffectedTestsConfig.Builder#build()}.
    • - *
  • {@link #SKIPPED} — run no tests at all. Previously impossible to - * express without also disabling the plugin; the situation-specific - * knobs in v2 make it first-class.
    • + * about. The typical escalation target for "ambiguous" situations + * such as {@link Situation#UNMAPPED_FILE}. + *
  • {@link #SKIPPED} — run no tests at all. Expressed per-situation via + * the {@code onXxx} DSL so operators can silence branches that do + * not justify a full suite (e.g. {@code onAllFilesIgnored=SKIPPED}).
    • * */ public enum Action { diff --git a/affected-tests-core/src/main/java/io/affectedtests/core/config/ActionSource.java b/affected-tests-core/src/main/java/io/affectedtests/core/config/ActionSource.java index 232869c..2b695e0 100644 --- a/affected-tests-core/src/main/java/io/affectedtests/core/config/ActionSource.java +++ b/affected-tests-core/src/main/java/io/affectedtests/core/config/ActionSource.java @@ -20,23 +20,10 @@ public enum ActionSource { EXPLICIT, /** - * The caller set one of the legacy boolean flags - * ({@code runAllIfNoMatches}, {@code runAllOnNonJavaChange}) and the - * translation shim picked the action from it. + * The caller did not set an explicit action for this situation, and + * the resolved {@link Mode} default supplied the action. When no + * mode is configured, {@link Mode#AUTO} detects CI vs. local from + * the environment and applies the corresponding default table. */ - LEGACY_BOOLEAN, - - /** - * The caller set an explicit {@link Mode} but not the more-specific - * explicit or legacy setting, and the mode's default table supplied - * the action. - */ - MODE_DEFAULT, - - /** - * No caller setting at any tier applies; the pre-v2 hardcoded - * default was used. Zero-config installs observe this for every - * situation until they set a mode. - */ - HARDCODED_DEFAULT + MODE_DEFAULT } 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 2478bb2..1d81997 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 @@ -2,7 +2,6 @@ import io.affectedtests.core.util.LogSanitizer; -import java.util.ArrayList; import java.util.EnumMap; import java.util.List; import java.util.Map; @@ -14,31 +13,21 @@ * Immutable value object — use the {@link Builder} to construct. * *

    v2 situation-based config

    - * The legacy booleans {@code runAllIfNoMatches} and {@code runAllOnNonJavaChange} - * remain supported for back-compat and map onto the v2 - * {@link Situation} / {@link Action} model via a translation shim in - * {@link Builder#build()}: + * Every engine run produces exactly one {@link Situation}; each situation + * resolves to one {@link Action} ({@link Action#SELECTED}, + * {@link Action#FULL_SUITE}, or {@link Action#SKIPPED}) via the priority + * ladder in {@link #actionFor(Situation)}: * - *
      - *
    • {@code runAllIfNoMatches=true} → every "no affected tests" branch - * ({@link Situation#EMPTY_DIFF}, {@link Situation#ALL_FILES_IGNORED}, - * {@link Situation#ALL_FILES_OUT_OF_SCOPE}, - * {@link Situation#DISCOVERY_EMPTY}) escalates to {@link Action#FULL_SUITE}.
      • - *
    • {@code runAllIfNoMatches=false} → the same four branches resolve - * to {@link Action#SKIPPED} (i.e. run no tests), matching the pre-v2 - * "silent no-op" behaviour when discovery turned up nothing.
      • - *
    • {@code runAllOnNonJavaChange=true} → {@link Situation#UNMAPPED_FILE} - * resolves to {@link Action#FULL_SUITE}.
      • - *
    • {@code runAllOnNonJavaChange=false} → {@link Situation#UNMAPPED_FILE} - * resolves to {@link Action#SELECTED}, meaning "treat the unmapped file - * as if it hadn't been there and continue to discovery". That matches - * the pre-v2 "silent skip on YAML" behaviour exactly.
      • - *
    • An explicit {@code onXxx()} call always wins over the legacy boolean - * translation, and the legacy boolean translation always wins over the - * {@link Mode}-based defaults. Users upgrading can therefore adopt the - * new DSL branch-by-branch without having to delete their legacy - * config in a single step.
      • - *
    + *
      + *
    1. the caller's explicit {@code onXxx(Action)} setting, if any;
      2. + *
    2. otherwise the per-{@link Mode} default + * (CI / LOCAL / STRICT — AUTO auto-detects from the environment).
      3. + *
    + * + *

    v1's three legacy knobs ({@code runAllIfNoMatches}, + * {@code runAllOnNonJavaChange}, {@code excludePaths}) were removed in + * v2.0. Callers upgrading from v1 should map them to the v2 DSL per the + * "Migrating from v1" section of the plugin README. */ public final class AffectedTestsConfig { @@ -51,8 +40,6 @@ public final class AffectedTestsConfig { private final String baseRef; private final boolean includeUncommitted; private final boolean includeStaged; - private final boolean runAllIfNoMatches; - private final boolean runAllOnNonJavaChange; private final Set strategies; private final int transitiveDepth; private final List testSuffixes; @@ -68,7 +55,6 @@ 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; @@ -87,17 +73,9 @@ private AffectedTestsConfig(Builder builder) { // {@link Builder#gradlewTimeoutSeconds(long)}. this.gradlewTimeoutSeconds = builder.gradlewTimeoutSeconds; - // Resolve the paths-to-ignore list. Precedence matches the doc: - // explicit ignorePaths() > explicit excludePaths() > builder default. - List resolvedIgnore; - if (builder.ignorePaths != null) { - resolvedIgnore = List.copyOf(builder.ignorePaths); - } else if (builder.excludePaths != null) { - resolvedIgnore = List.copyOf(builder.excludePaths); - } else { - resolvedIgnore = List.copyOf(Builder.DEFAULT_IGNORE_PATHS); - } - this.ignorePaths = resolvedIgnore; + this.ignorePaths = builder.ignorePaths != null + ? List.copyOf(builder.ignorePaths) + : List.copyOf(Builder.DEFAULT_IGNORE_PATHS); this.outOfScopeTestDirs = List.copyOf( builder.outOfScopeTestDirs != null ? builder.outOfScopeTestDirs : List.of()); @@ -105,67 +83,11 @@ private AffectedTestsConfig(Builder builder) { builder.outOfScopeSourceDirs != null ? builder.outOfScopeSourceDirs : List.of()); this.mode = builder.mode != null ? builder.mode : Mode.AUTO; - this.effectiveMode = resolveEffectiveMode(builder.mode); - - // Keep the legacy getters literal — they return what the caller set, - // or the pre-v2 default if the caller never touched them. A - // "resolved view" that incorporated the new situation actions or - // mode detection would have been more accurate, but it would also - // flip the return value of {@code runAllIfNoMatches()} based on - // whether {@code CI} is set in the environment, which is exactly - // the kind of test-only determinism the pre-v2 API implicitly - // guaranteed. Engine code no longer consults these getters — - // see {@link #actionFor(Situation)}. - this.runAllIfNoMatches = - builder.runAllIfNoMatches != null ? builder.runAllIfNoMatches : false; - this.runAllOnNonJavaChange = - builder.runAllOnNonJavaChange != null ? builder.runAllOnNonJavaChange : true; + this.effectiveMode = resolveEffectiveMode(this.mode); 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. */ @@ -175,16 +97,14 @@ private record ResolvedActions( } /** - * Resolves the effective mode for action defaults. When the caller did - * not set a mode at all (null), we deliberately resolve to {@code null} - * here — the resolver downstream treats that as "fall through to the - * pre-v2 legacy-boolean defaults" instead of consulting a mode table. - * That keeps zero-config callers on the exact pre-v2 behaviour even - * when {@code $CI} happens to be set, which is what prevents the - * existing engine tests from going flaky on GitHub Actions runners. + * Resolves the configured {@link Mode} to the concrete profile whose + * defaults will be applied. {@link Mode#AUTO} probes the environment + * via {@link Builder#detectMode()}; every other mode passes through + * verbatim. Always returns one of {@link Mode#LOCAL}, + * {@link Mode#CI}, or {@link Mode#STRICT} — never {@code null}, + * never {@link Mode#AUTO}. */ private static Mode resolveEffectiveMode(Mode configured) { - if (configured == null) return null; if (configured == Mode.AUTO) return Builder.detectMode(); return configured; } @@ -193,65 +113,23 @@ private static Mode resolveEffectiveMode(Mode configured) { * Per-situation actions in strict priority order: *

      *
    1. the caller's explicit {@code on*} action,
      2. - *
    2. the translation of whichever legacy boolean that situation - * was historically driven by,
      3. - *
    3. the per-mode default (only when the caller set an explicit - * mode — {@code AUTO}/unset falls through to the final branch),
      4. - *
    4. the hard-coded pre-v2 default, kept identical to the legacy - * boolean defaults so zero-config callers continue to observe - * pre-v2 behaviour exactly.
      5. + *
    5. the per-mode default table (see {@link #defaultFor}).
      6. *
    + * + *

    {@link Situation#DISCOVERY_SUCCESS} is definitionally + * {@link Action#SELECTED} — there is no other sensible outcome when + * discovery returns tests, so the code fixes it rather than routing + * it through the resolver. */ private static ResolvedActions resolveSituationActions(Builder b, Mode effectiveMode) { - Action legacyNoMatches = (b.runAllIfNoMatches == null) - ? null - : (b.runAllIfNoMatches ? Action.FULL_SUITE : Action.SKIPPED); - // runAllOnNonJavaChange=false historically meant "ignore the unmapped - // file and proceed with whatever Java the diff touched" — that is - // {@code SELECTED} in the v2 model, not {@code SKIPPED}. The latter - // would have regressed every pre-v2 caller that opted out of the - // safety net expecting discovery to still run. - Action legacyNonJava = (b.runAllOnNonJavaChange == null) - ? null - : (b.runAllOnNonJavaChange ? Action.FULL_SUITE : Action.SELECTED); - EnumMap actions = new EnumMap<>(Situation.class); EnumMap sources = new EnumMap<>(Situation.class); - resolveInto(actions, sources, Situation.EMPTY_DIFF, - b.onEmptyDiff, legacyNoMatches, effectiveMode, Action.SKIPPED); - resolveInto(actions, sources, Situation.ALL_FILES_IGNORED, - b.onAllFilesIgnored, legacyNoMatches, effectiveMode, Action.SKIPPED); - // No legacy boolean maps to ALL_FILES_OUT_OF_SCOPE — the concept - // did not exist pre-v2, so there is nothing to translate and the - // hard-coded fallback is {@code SKIPPED}. - resolveInto(actions, sources, Situation.ALL_FILES_OUT_OF_SCOPE, - b.onAllFilesOutOfScope, null, effectiveMode, Action.SKIPPED); - resolveInto(actions, sources, Situation.UNMAPPED_FILE, - b.onUnmappedFile, legacyNonJava, effectiveMode, Action.FULL_SUITE); - resolveInto(actions, sources, Situation.DISCOVERY_EMPTY, - b.onDiscoveryEmpty, legacyNoMatches, effectiveMode, Action.SKIPPED); - // No legacy boolean maps to DISCOVERY_INCOMPLETE — the situation - // did not exist pre-v1.9.22, so there is nothing to translate - // and there is no pre-v2 behaviour to preserve. Wiring the - // {@code runAllIfNoMatches} shim in here would silently fight - // the CI / STRICT safety-net guarantee documented in the - // {@link Situation#DISCOVERY_INCOMPLETE} javadoc: a user who - // set {@code mode=CI} plus {@code runAllIfNoMatches=false} - // would get SKIPPED on parse failures, which is the opposite - // of what the doc promises. The hard-coded fallback is - // {@link Action#SELECTED}, matching the LOCAL-mode default - // and mirroring the {@link Situation#ALL_FILES_OUT_OF_SCOPE} - // wiring (another v2-only situation that passes {@code null} - // for the legacy argument). - resolveInto(actions, sources, Situation.DISCOVERY_INCOMPLETE, - b.onDiscoveryIncomplete, null, effectiveMode, Action.SELECTED); - // DISCOVERY_SUCCESS is definitionally SELECTED — there is no - // other sensible outcome when discovery returns tests. Making - // it configurable would let users set "discovery ran, found - // tests, now run nothing", which is never what anyone wants. - // It is reported as EXPLICIT in the source map because that's - // the honest answer — the code fixes it rather than choosing a - // default that someone could override. + resolveInto(actions, sources, Situation.EMPTY_DIFF, b.onEmptyDiff, effectiveMode); + resolveInto(actions, sources, Situation.ALL_FILES_IGNORED, b.onAllFilesIgnored, effectiveMode); + resolveInto(actions, sources, Situation.ALL_FILES_OUT_OF_SCOPE, b.onAllFilesOutOfScope, effectiveMode); + resolveInto(actions, sources, Situation.UNMAPPED_FILE, b.onUnmappedFile, effectiveMode); + resolveInto(actions, sources, Situation.DISCOVERY_EMPTY, b.onDiscoveryEmpty, effectiveMode); + resolveInto(actions, sources, Situation.DISCOVERY_INCOMPLETE, b.onDiscoveryIncomplete, effectiveMode); actions.put(Situation.DISCOVERY_SUCCESS, Action.SELECTED); sources.put(Situation.DISCOVERY_SUCCESS, ActionSource.EXPLICIT); return new ResolvedActions(Map.copyOf(actions), Map.copyOf(sources)); @@ -261,33 +139,18 @@ private static void resolveInto(EnumMap actions, EnumMap sources, Situation s, Action explicit, - Action legacy, - Mode effectiveMode, - Action preV2Default) { + Mode effectiveMode) { if (explicit != null) { actions.put(s, explicit); sources.put(s, ActionSource.EXPLICIT); return; } - if (legacy != null) { - actions.put(s, legacy); - sources.put(s, ActionSource.LEGACY_BOOLEAN); - return; - } - if (effectiveMode != null) { - actions.put(s, defaultFor(s, effectiveMode)); - sources.put(s, ActionSource.MODE_DEFAULT); - return; - } - actions.put(s, preV2Default); - sources.put(s, ActionSource.HARDCODED_DEFAULT); + actions.put(s, defaultFor(s, effectiveMode)); + sources.put(s, ActionSource.MODE_DEFAULT); } /** * Per-mode defaults (see {@link Mode} javadoc for the full table). - * Only invoked when the caller set an explicit mode — {@code AUTO} - * without any legacy override falls through to the pre-v2 defaults - * instead. */ private static Action defaultFor(Situation s, Mode effectiveMode) { return switch (effectiveMode) { @@ -325,21 +188,6 @@ private static Action defaultFor(Situation s, Mode effectiveMode) { public String baseRef() { return baseRef; } public boolean includeUncommitted() { return includeUncommitted; } public boolean includeStaged() { return includeStaged; } - public boolean runAllIfNoMatches() { return runAllIfNoMatches; } - - /** - * Whether to force a full test run whenever the change set contains any - * file that cannot be resolved to a Java class under the configured - * {@link #sourceDirs()} or {@link #testDirs()} — for example - * {@code application.yml}, {@code build.gradle}, a Liquibase changelog, - * or a logback config. Files matching {@link #ignorePaths()} are - * treated as an explicit opt-out and do not trigger the escalation. - * - *

    Default: {@code true} — "run more, never run less". - * - * @return the run-all-on-non-java-change property - */ - public boolean runAllOnNonJavaChange() { return runAllOnNonJavaChange; } public Set strategies() { return strategies; } public int transitiveDepth() { return transitiveDepth; } public List testSuffixes() { return testSuffixes; } @@ -382,17 +230,6 @@ private static Action defaultFor(Situation s, Mode effectiveMode) { */ public List ignorePaths() { return ignorePaths; } - /** - * Back-compat alias for {@link #ignorePaths()}. Returns the same list — - * v2 collapsed the two legacy names into a single effective list so - * downstream code never has to consult both. - * - * @return the effective ignore paths list (identical to {@link #ignorePaths()}) - * @deprecated use {@link #ignorePaths()} in new code; both return the same value. - */ - @Deprecated - public List excludePaths() { return ignorePaths; } - /** * Test source directories (e.g. {@code api-test/src/test/java}) that the * plugin must not resolve as in-scope tests. A diff consisting entirely @@ -428,32 +265,16 @@ private static Action defaultFor(Situation s, Mode effectiveMode) { /** * The mode after {@link Mode#AUTO} resolution. Always returns one of * {@link Mode#LOCAL}, {@link Mode#CI} or {@link Mode#STRICT} — never - * {@code null}. - * - *

    When the caller did not configure a mode at all, the internal - * situation-action resolver deliberately falls through to pre-v2 - * hardcoded defaults (preserving zero-config behaviour parity with - * the legacy API). The public getter still reports the mode that - * {@code AUTO} would have selected — either the value detected from - * the environment, or {@link Mode#LOCAL} when detection finds no - * CI markers — so callers reading this value get a concrete Mode - * that accurately describes the execution environment. + * {@code null}, never {@link Mode#AUTO}. * - * @return the resolved mode, never {@code null} + * @return the resolved mode */ - public Mode effectiveMode() { - // Honest fallback for the "caller passed nothing, we stayed on - // pre-v2 defaults" branch — resolve via the same AUTO-detection - // path the builder would have taken. Keeps the public contract - // non-null without destabilising the resolver, which reads the - // nullable internal field directly. - return effectiveMode != null ? effectiveMode : Builder.detectMode(); - } + public Mode effectiveMode() { return effectiveMode; } /** * The {@link Action} the engine will take for a given {@link Situation}. - * Produced by layering the explicit caller-set action (highest priority), - * then the legacy-boolean translation, then the mode default. + * Produced by layering the explicit caller-set action (highest priority) + * over the {@link Mode} default. * * @param situation the situation to resolve * @return the configured action for {@code situation} @@ -474,8 +295,7 @@ public Action actionFor(Situation situation) { /** * The {@link ActionSource} that picked the {@link Action} for a given * {@link Situation}. Used by {@code --explain} so operators can tell - * whether an outcome came from an explicit setting, a legacy boolean, - * a mode default, or the pre-v2 hardcoded baseline. + * whether an outcome came from an explicit setting or a mode default. * * @param situation the situation to resolve * @return the source tier that produced {@link #actionFor(Situation)} @@ -494,18 +314,6 @@ 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(); @@ -514,8 +322,8 @@ public static Builder builder() { public static final class Builder { /** - * Default list of paths that must never influence test selection — - * wider than the pre-v2 default ({@code ["**}{@code /generated/**"]}) + * Default list of paths that must never influence test selection. + * Broader than the pre-v2 default ({@code ["**}{@code /generated/**"]}) * so markdown-only PRs don't sneak past ignore rules on zero-config * installs. */ @@ -552,8 +360,6 @@ public static final class Builder { // want WIP to expand the diff opt in via {@code includeUncommitted(true)}. private boolean includeUncommitted = false; private boolean includeStaged = false; - private Boolean runAllIfNoMatches; - private Boolean runAllOnNonJavaChange; private Set strategies = Set.of(STRATEGY_NAMING, STRATEGY_USAGE, STRATEGY_IMPL, STRATEGY_TRANSITIVE); // 4 matches the v2 design: most real-world ctrl -> svc -> repo -> // mapper chains are 2-3 deep, so 4 leaves headroom without @@ -564,7 +370,6 @@ public static final class Builder { private List sourceDirs = List.of("src/main/java"); private List testDirs = List.of("src/test/java"); private List ignorePaths; - private List excludePaths; private List outOfScopeTestDirs; private List outOfScopeSourceDirs; private boolean includeImplementationTests = true; @@ -700,8 +505,6 @@ private static boolean containsControlChars(String value) { } public Builder includeUncommitted(boolean v) { this.includeUncommitted = v; return this; } public Builder includeStaged(boolean v) { this.includeStaged = v; return this; } - public Builder runAllIfNoMatches(boolean v) { this.runAllIfNoMatches = v; return this; } - public Builder runAllOnNonJavaChange(boolean v) { this.runAllOnNonJavaChange = v; return this; } public Builder strategies(Set v) { this.strategies = Objects.requireNonNull(v, "strategies must not be null"); return this; @@ -720,20 +523,6 @@ public Builder testDirs(List v) { return this; } - /** - * Back-compat alias for {@link #ignorePaths(List)}. If both are set, - * {@link #ignorePaths(List)} wins. - * - * @param v the exclude paths - * @return this builder - * @deprecated prefer {@link #ignorePaths(List)} for new code. - */ - @Deprecated - public Builder excludePaths(List v) { - this.excludePaths = Objects.requireNonNull(v, "excludePaths must not be null"); - return this; - } - /** Glob patterns for files the plugin must ignore entirely. */ public Builder ignorePaths(List v) { this.ignorePaths = Objects.requireNonNull(v, "ignorePaths must not be null"); diff --git a/affected-tests-core/src/main/java/io/affectedtests/core/config/Mode.java b/affected-tests-core/src/main/java/io/affectedtests/core/config/Mode.java index e66d79c..294dd27 100644 --- a/affected-tests-core/src/main/java/io/affectedtests/core/config/Mode.java +++ b/affected-tests-core/src/main/java/io/affectedtests/core/config/Mode.java @@ -8,24 +8,21 @@ * {@link #LOCAL} or {@link #CI} based on the usual CI env vars; every other * mode is an explicit opt-in and its defaults are applied verbatim. * - *

    Defaults per mode (used only when the caller has set neither the - * specific situation action nor the legacy boolean that would translate to - * it): + *

    Defaults per mode (used only when the caller has not set the + * specific {@code onXxx} situation action): * * * - * - * - * - * + * + * + * + * *
    Per-mode action defaults
    EMPTY_DIFFALL_IGNOREDALL_OUT_OF_SCOPEUNMAPPED_FILEDISCOVERY_EMPTY
    LOCALSKIPPEDSKIPPEDSKIPPEDFULL_SUITESKIPPED
    CISKIPPEDSKIPPEDSKIPPEDFULL_SUITEFULL_SUITE
    STRICTFULL_SUITEFULL_SUITESKIPPEDFULL_SUITEFULL_SUITE
    EMPTY_DIFFALL_IGNOREDALL_OUT_OF_SCOPEUNMAPPED_FILEDISCOVERY_EMPTYDISCOVERY_INCOMPLETE
    LOCALSKIPPEDSKIPPEDSKIPPEDFULL_SUITESKIPPEDSELECTED
    CISKIPPEDSKIPPEDSKIPPEDFULL_SUITEFULL_SUITEFULL_SUITE
    STRICTFULL_SUITEFULL_SUITESKIPPEDFULL_SUITEFULL_SUITEFULL_SUITE
    * - *

    The pre-v2 zero-config baseline was - * {@code runAllIfNoMatches=false}, {@code runAllOnNonJavaChange=true} - * — which lines up exactly with the {@link #LOCAL} column above. The - * {@link #CI} profile adds the {@code DISCOVERY_EMPTY=FULL_SUITE} - * safety net on top of that so zero-config users who land in a CI - * environment get the safer default without having to opt in. + *

    Zero-config installs land on {@link #AUTO} and therefore get either + * the LOCAL or CI column based on whether the usual CI env vars are + * set. {@link #STRICT} tightens the CI column further for users that + * want ambiguity to always escalate to a full suite. */ public enum Mode { /** Detect CI vs. local at build() time based on common CI env vars. */ diff --git a/affected-tests-core/src/main/java/io/affectedtests/core/config/Situation.java b/affected-tests-core/src/main/java/io/affectedtests/core/config/Situation.java index 8dfab7d..398b363 100644 --- a/affected-tests-core/src/main/java/io/affectedtests/core/config/Situation.java +++ b/affected-tests-core/src/main/java/io/affectedtests/core/config/Situation.java @@ -35,10 +35,9 @@ public enum Situation { UNMAPPED_FILE, /** - * Every file in the diff matched {@link AffectedTestsConfig#ignorePaths()} - * (or the legacy {@code excludePaths} shim). Distinct from - * {@link #ALL_FILES_OUT_OF_SCOPE} so users can treat "purely docs changes" - * differently from "purely api-test changes". + * Every file in the diff matched {@link AffectedTestsConfig#ignorePaths()}. + * Distinct from {@link #ALL_FILES_OUT_OF_SCOPE} so users can treat + * "purely docs changes" differently from "purely api-test changes". */ ALL_FILES_IGNORED, @@ -83,12 +82,8 @@ public enum Situation { *

    Defaults in the {@link AffectedTestsConfig} resolver are * conservative on purpose: {@link Mode#CI} and {@link Mode#STRICT} * escalate to {@link Action#FULL_SUITE}, {@link Mode#LOCAL} stays on - * {@link Action#SELECTED} (dev sees the WARN and decides), and the - * legacy {@code runAllIfNoMatches=true} shim also escalates — it is - * the closest existing boolean to "I don't trust a no-signal result". - * The hardcoded pre-v2 default is {@link Action#SELECTED} so - * zero-config callers upgrading from v1.9.21 do not experience a - * behaviour change. + * {@link Action#SELECTED} (dev sees the WARN and decides). Callers + * can override per-situation via {@code onDiscoveryIncomplete}. */ DISCOVERY_INCOMPLETE, 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 074256d..fb20c4e 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 @@ -238,8 +238,9 @@ public MappingResult mapChangedFiles(Set changedFiles) { * {@code indexOf(sourceDir)} would happily pick up * {@code "notsrc/main/java/Foo.java"} and classify it as production * {@code Foo}, which in turn would keep it out of the "unmapped" - * bucket that drives the {@code runAllOnNonJavaChange} safety net — - * exactly the silent-skip behaviour that flag exists to prevent. + * bucket that drives the {@link io.affectedtests.core.config.Situation#UNMAPPED_FILE} + * safety net — exactly the silent-skip behaviour that escalation + * exists to prevent. */ private String tryMapToClass(String filePath, java.util.List sourceDirs) { String normalized = filePath.replace('\\', '/'); diff --git a/affected-tests-core/src/test/java/io/affectedtests/core/AffectedTestsEngineTest.java b/affected-tests-core/src/test/java/io/affectedtests/core/AffectedTestsEngineTest.java index d2e5371..925bce9 100644 --- a/affected-tests-core/src/test/java/io/affectedtests/core/AffectedTestsEngineTest.java +++ b/affected-tests-core/src/test/java/io/affectedtests/core/AffectedTestsEngineTest.java @@ -93,8 +93,8 @@ void returnsEmptyWhenNoChanges() throws Exception { @Test void emptyChangesetWithRunAllIfNoMatchesReportsItsOwnReason() throws Exception { // Guards a subtle bug: before this test, an empty changeset with - // runAllIfNoMatches=true shared the RUN_ALL_IF_NO_MATCHES reason with - // the post-discovery empty branch, so the task logged + // the empty-diff escalation enabled shared the RUN_ALL_IF_NO_MATCHES + // reason with the post-discovery empty branch, so the task logged // "no affected tests discovered" even though discovery had never // actually run. The two branches must be distinguishable from the // outside, otherwise the lifecycle log actively lies about why we @@ -106,14 +106,14 @@ void emptyChangesetWithRunAllIfNoMatchesReportsItsOwnReason() throws Exception { .baseRef(head) .includeUncommitted(false) .includeStaged(false) - .runAllIfNoMatches(true) + .onEmptyDiff(Action.FULL_SUITE) .build(); AffectedTestsEngine engine = new AffectedTestsEngine(config, tempDir); AffectedTestsEngine.AffectedTestsResult result = engine.run(); assertTrue(result.runAll(), - "Empty changeset + runAllIfNoMatches=true must still flip runAll"); + "Empty changeset + onEmptyDiff=FULL_SUITE must still flip runAll"); assertTrue(result.changedFiles().isEmpty()); assertTrue(result.testClassFqns().isEmpty()); assertEquals(EscalationReason.RUN_ALL_ON_EMPTY_CHANGESET, result.escalationReason(), @@ -137,14 +137,14 @@ void runAllWhenNoMatchesAndFlagEnabled() throws Exception { .baseRef(base) .includeUncommitted(false) .includeStaged(false) - .runAllIfNoMatches(true) + .onDiscoveryEmpty(Action.FULL_SUITE) .transitiveDepth(0) .build(); AffectedTestsEngine engine = new AffectedTestsEngine(config, tempDir); AffectedTestsEngine.AffectedTestsResult result = engine.run(); - assertTrue(result.runAll(), "Should set runAll when flag is enabled and no tests match"); + assertTrue(result.runAll(), "Should set runAll when onDiscoveryEmpty=FULL_SUITE and no tests match"); assertEquals(EscalationReason.RUN_ALL_IF_NO_MATCHES, result.escalationReason(), "A discovery-empty runAll must report RUN_ALL_IF_NO_MATCHES so the task logs the right trigger"); } @@ -339,21 +339,33 @@ void excludedPathsDoNotForceRunAll() throws Exception { .baseRef(base) .includeUncommitted(false) .includeStaged(false) - .excludePaths(java.util.List.of("**/generated/**")) + .ignorePaths(java.util.List.of("**/generated/**")) .build(); AffectedTestsEngine engine = new AffectedTestsEngine(config, tempDir); AffectedTestsEngine.AffectedTestsResult result = engine.run(); assertFalse(result.runAll(), - "Excluded path should not trigger the non-Java safety escalation"); + "Ignored path should not trigger the non-Java safety escalation"); } } @Test - void runAllOnNonJavaChangeCanBeDisabled() throws Exception { + void unmappedFileEscalationCanBeDisabled() throws Exception { // Escape hatch: a user who wants the old "silent skip on YAML" - // behaviour can explicitly opt out of the safety net. + // behaviour can explicitly opt out of the safety net via the v2 + // onUnmappedFile setter. + // + // We pin mode(LOCAL) because once the unmapped-file branch is + // disabled, a yaml-only diff falls through to discovery with + // zero production classes and lands on DISCOVERY_EMPTY. Under + // the zero-config Mode.AUTO default that resolves to CI on any + // GitHub-Actions-style runner (env var `CI=true`), and the CI + // profile escalates DISCOVERY_EMPTY to FULL_SUITE — so an + // unpinned test would pass locally and fail on CI, which is + // exactly what happened on PR #34. This pin mirrors the v1→v2 + // migration path we document in README/CHANGELOG for users + // coming off `runAllOnNonJavaChange = false`. try (Git git = initRepoWithInitialCommit()) { String base = git.log().call().iterator().next().getName(); @@ -369,17 +381,18 @@ void runAllOnNonJavaChangeCanBeDisabled() throws Exception { .baseRef(base) .includeUncommitted(false) .includeStaged(false) - .runAllOnNonJavaChange(false) + .mode(Mode.LOCAL) + .onUnmappedFile(Action.SELECTED) .build(); AffectedTestsEngine engine = new AffectedTestsEngine(config, tempDir); AffectedTestsEngine.AffectedTestsResult result = engine.run(); assertFalse(result.runAll(), - "With runAllOnNonJavaChange=false the old silent-skip behaviour must be preserved"); + "With onUnmappedFile=SELECTED the old silent-skip behaviour must be preserved"); assertTrue(result.testClassFqns().isEmpty()); assertEquals(EscalationReason.NONE, result.escalationReason(), - "Opting out of the non-Java escalation must also clear its reason tag"); + "Opting out of the unmapped-file escalation must also clear its reason tag"); } } 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 2d49495..505312f 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 @@ -23,13 +23,6 @@ void defaultsAreApplied() { "Core builder default must be COMMITTED-ONLY as of the v1.9.14 → next-release flip"); assertFalse(config.includeStaged(), "Core builder default must be COMMITTED-ONLY as of the v1.9.14 → next-release flip"); - // Pre-v2 legacy defaults preserved 1:1 for zero-config callers — - // the getters below read the raw configured value (or the - // hardcoded pre-v2 default when unset), not the resolved - // per-situation action, so the assertions stay deterministic - // regardless of whether the test runs in CI or on a laptop. - assertFalse(config.runAllIfNoMatches()); - assertTrue(config.runAllOnNonJavaChange()); assertEquals(Set.of("naming", "usage", "impl", "transitive"), config.strategies()); // v2 raises the default from 2 to 4: real ctrl→svc→repo→mapper // chains sit 3-4 deep so 2 dropped coverage on zero-config. @@ -44,18 +37,20 @@ void defaultsAreApplied() { // should inspect {@link AffectedTestsConfig.Builder#DEFAULT_IGNORE_PATHS}. assertTrue(config.ignorePaths().contains("**/generated/**")); assertTrue(config.ignorePaths().contains("**/*.md")); - assertEquals(config.ignorePaths(), config.excludePaths(), - "excludePaths must alias ignorePaths in v2 — callers can't read two diverging lists"); assertTrue(config.includeImplementationTests()); assertEquals(List.of("Impl", "Default"), config.implementationNaming()); assertEquals(List.of(), config.outOfScopeTestDirs()); assertEquals(List.of(), config.outOfScopeSourceDirs()); + // Raw configured mode is AUTO — the resolved mode in + // {@link #effectiveMode()} is one of LOCAL / CI / STRICT depending + // on the environment (covered by + // {@link #effectiveModeIsAlwaysConcreteForZeroConfigCallers()}). assertEquals(Mode.AUTO, config.mode()); - // Zero-config callers get pre-v2 situation actions from the - // hard-coded defaults — NOT from mode detection — so the result - // is deterministic whether or not $CI is set. - assertEquals(Action.SKIPPED, config.actionFor(Situation.EMPTY_DIFF)); - assertEquals(Action.SKIPPED, config.actionFor(Situation.DISCOVERY_EMPTY)); + // Environment-independent action assertions — DISCOVERY_SUCCESS + // is hardcoded SELECTED; UNMAPPED_FILE is FULL_SUITE in every + // built-in mode. Mode-specific behaviour (DISCOVERY_EMPTY, + // EMPTY_DIFF) is covered by the per-mode tests below, which + // pin {@code mode(Mode.X)} explicitly to stay deterministic. assertEquals(Action.FULL_SUITE, config.actionFor(Situation.UNMAPPED_FILE)); assertEquals(Action.SELECTED, config.actionFor(Situation.DISCOVERY_SUCCESS)); } @@ -66,13 +61,12 @@ void customValuesArePreserved() { .baseRef("origin/main") .includeUncommitted(false) .includeStaged(false) - .runAllIfNoMatches(true) .strategies(Set.of("naming")) .transitiveDepth(0) .testSuffixes(List.of("Test")) .sourceDirs(List.of("src/main/java", "src/main/kotlin")) .testDirs(List.of("src/test/java", "src/test/kotlin")) - .excludePaths(List.of("**/generated/**", "**/*Dto.java")) + .ignorePaths(List.of("**/generated/**", "**/*Dto.java")) .includeImplementationTests(false) .implementationNaming(List.of("Impl", "Default")) .build(); @@ -80,13 +74,12 @@ void customValuesArePreserved() { assertEquals("origin/main", config.baseRef()); assertFalse(config.includeUncommitted()); assertFalse(config.includeStaged()); - assertTrue(config.runAllIfNoMatches()); assertEquals(Set.of("naming"), config.strategies()); assertEquals(0, config.transitiveDepth()); assertEquals(List.of("Test"), config.testSuffixes()); assertEquals(2, config.sourceDirs().size()); assertEquals(2, config.testDirs().size()); - assertEquals(2, config.excludePaths().size()); + assertEquals(2, config.ignorePaths().size()); assertFalse(config.includeImplementationTests()); assertEquals(2, config.implementationNaming().size()); } @@ -101,30 +94,24 @@ void transitiveDepthIsClamped() { } @Test - void explicitSituationActionWinsOverLegacyBooleans() { - // Explicit on* setter beats the legacy-boolean translation — a - // user migrating to v2 must be able to override a single branch - // without having to clear the legacy booleans first. - AffectedTestsConfig config = AffectedTestsConfig.builder() - .runAllIfNoMatches(true) - .onDiscoveryEmpty(Action.SKIPPED) - .build(); - assertEquals(Action.SKIPPED, config.actionFor(Situation.DISCOVERY_EMPTY)); - // Sibling situations driven by the same legacy boolean still - // follow the legacy translation. - assertEquals(Action.FULL_SUITE, config.actionFor(Situation.EMPTY_DIFF)); - } - - @Test - void legacyBooleanBeatsModeDefault() { - // Explicit legacy boolean beats the per-mode defaults so a CI - // user who set runAllIfNoMatches=false + mode=CI genuinely gets - // "skip" on empty diff instead of the CI safety-net full run. + void explicitSituationActionWinsOverModeDefault() { + // Explicit on* setter beats the per-mode default — the whole + // point of the priority ladder is that a caller who types + // `onDiscoveryEmpty` can rely on it sticking even when the + // resolved mode would otherwise flip that branch to FULL_SUITE. AffectedTestsConfig config = AffectedTestsConfig.builder() - .runAllIfNoMatches(false) .mode(Mode.CI) + .onDiscoveryEmpty(Action.SKIPPED) .build(); assertEquals(Action.SKIPPED, config.actionFor(Situation.DISCOVERY_EMPTY)); + assertEquals(ActionSource.EXPLICIT, + config.actionSourceFor(Situation.DISCOVERY_EMPTY)); + // Sibling situations not pinned explicitly still follow the + // mode default — explicit is a per-situation override, not a + // blanket one. + assertEquals(Action.SKIPPED, config.actionFor(Situation.EMPTY_DIFF)); + assertEquals(ActionSource.MODE_DEFAULT, + config.actionSourceFor(Situation.EMPTY_DIFF)); } @Test @@ -202,32 +189,6 @@ void explicitOnDiscoveryIncompleteWinsOverModeDefault() { assertEquals(Action.SKIPPED, config.actionFor(Situation.DISCOVERY_INCOMPLETE)); } - @Test - void legacyRunAllIfNoMatchesDoesNotOverrideDiscoveryIncomplete() { - // Regression for B6-#9 legacy-boolean wiring: DISCOVERY_INCOMPLETE - // is a v1.9.22-only situation, so wiring the pre-v2 - // runAllIfNoMatches shim into it would let a CI caller with - // runAllIfNoMatches=false silently opt out of the parse- - // failure safety net that the Situation javadoc and README - // promise. Since the legacy tier wins over mode defaults in - // {@link AffectedTestsConfig#resolveInto}, this test pins - // that the legacy argument is deliberately NOT wired for - // DISCOVERY_INCOMPLETE — CI + runAllIfNoMatches=false must - // still give FULL_SUITE on parse failures, the same as CI - // without the legacy flag. - AffectedTestsConfig config = AffectedTestsConfig.builder() - .mode(Mode.CI) - .runAllIfNoMatches(false) - .build(); - assertEquals(Action.FULL_SUITE, config.actionFor(Situation.DISCOVERY_INCOMPLETE), - "runAllIfNoMatches=false must NOT drag DISCOVERY_INCOMPLETE down to SKIPPED — " - + "this is a v2-only situation with no pre-v2 behaviour to preserve, " - + "so CI mode's safety-net FULL_SUITE must survive"); - assertEquals(ActionSource.MODE_DEFAULT, - config.actionSourceFor(Situation.DISCOVERY_INCOMPLETE), - "Source must report MODE_DEFAULT — the legacy boolean has no say"); - } - @Test void gradlewTimeoutDefaultsToZero() { // Zero means "no deadline" — the pre-v1.9.22 behaviour. @@ -262,56 +223,32 @@ void gradlewTimeoutRejectsNegativeValues() { "Error must name the offending setting so the user can find it; got: " + e.getMessage()); } - @Test - void runAllOnNonJavaChangeFalseTranslatesToSelected() { - // Pre-v2 callers that opted out of the safety net expected - // discovery to still run against whatever Java was in the diff. - // SKIPPED would regress that contract silently. - AffectedTestsConfig config = AffectedTestsConfig.builder() - .runAllOnNonJavaChange(false) - .build(); - assertEquals(Action.SELECTED, config.actionFor(Situation.UNMAPPED_FILE)); - } - @Test void actionSourceReflectsResolutionTierOrdering() { - // Zero-config → hardcoded pre-v2 default. This is the baseline - // every other tier overrides; pinning it here prevents a future - // refactor from silently bumping zero-config users to a mode - // default when they never set one. + // Zero-config → AUTO-resolved mode default. Pinning this + // prevents a future refactor from silently re-introducing the + // old HARDCODED_DEFAULT tier that v2.0 removed. AffectedTestsConfig baseline = AffectedTestsConfig.builder().build(); - assertEquals(ActionSource.HARDCODED_DEFAULT, + assertEquals(ActionSource.MODE_DEFAULT, baseline.actionSourceFor(Situation.EMPTY_DIFF)); - assertEquals(ActionSource.HARDCODED_DEFAULT, - baseline.actionSourceFor(Situation.UNMAPPED_FILE)); - // Setting mode should flip all unpinned situations to MODE_DEFAULT — - // the only escape hatch is an explicit onXxx or a legacy boolean. + // Setting mode keeps unpinned situations at MODE_DEFAULT — the + // only escape hatch left in v2.0 is an explicit onXxx setter. AffectedTestsConfig modeOnly = AffectedTestsConfig.builder() .mode(Mode.CI) .build(); assertEquals(ActionSource.MODE_DEFAULT, modeOnly.actionSourceFor(Situation.DISCOVERY_EMPTY)); - // Legacy boolean must win over mode, or the shim would lie about - // what the caller asked for. - AffectedTestsConfig legacyOverMode = AffectedTestsConfig.builder() - .mode(Mode.STRICT) - .runAllIfNoMatches(false) - .build(); - assertEquals(ActionSource.LEGACY_BOOLEAN, - legacyOverMode.actionSourceFor(Situation.DISCOVERY_EMPTY)); - - // Explicit onXxx must win over both legacy and mode — this is the + // Explicit onXxx must win over the mode default — this is the // only tier that's guaranteed to survive future default changes // and the --explain flag has to be honest about that. - AffectedTestsConfig explicitOverEverything = AffectedTestsConfig.builder() + AffectedTestsConfig explicitOverMode = AffectedTestsConfig.builder() .mode(Mode.STRICT) - .runAllIfNoMatches(false) - .onDiscoveryEmpty(Action.FULL_SUITE) + .onDiscoveryEmpty(Action.SKIPPED) .build(); assertEquals(ActionSource.EXPLICIT, - explicitOverEverything.actionSourceFor(Situation.DISCOVERY_EMPTY)); + explicitOverMode.actionSourceFor(Situation.DISCOVERY_EMPTY)); // DISCOVERY_SUCCESS is hardcoded SELECTED regardless of // configuration — that contract is reported as EXPLICIT because @@ -320,22 +257,6 @@ void actionSourceReflectsResolutionTierOrdering() { baseline.actionSourceFor(Situation.DISCOVERY_SUCCESS)); } - @Test - void ignorePathsAliasesExcludePaths() { - AffectedTestsConfig byExclude = AffectedTestsConfig.builder() - .excludePaths(List.of("**/*.gen.java")) - .build(); - assertEquals(List.of("**/*.gen.java"), byExclude.ignorePaths()); - assertEquals(byExclude.ignorePaths(), byExclude.excludePaths()); - - // When both are set the new name wins. - AffectedTestsConfig both = AffectedTestsConfig.builder() - .excludePaths(List.of("old")) - .ignorePaths(List.of("new")) - .build(); - assertEquals(List.of("new"), both.ignorePaths()); - } - @Test void baseRefRejectsBlank() { assertThrows(IllegalArgumentException.class, () -> @@ -473,7 +394,7 @@ void builderRejectsNullCollections() { assertThrows(NullPointerException.class, () -> AffectedTestsConfig.builder().testDirs(null).build()); assertThrows(NullPointerException.class, () -> - AffectedTestsConfig.builder().excludePaths(null).build()); + AffectedTestsConfig.builder().ignorePaths(null).build()); assertThrows(NullPointerException.class, () -> AffectedTestsConfig.builder().implementationNaming(null).build()); } @@ -485,122 +406,7 @@ void configIsImmutable() { assertThrows(UnsupportedOperationException.class, () -> config.testSuffixes().add("Spec")); 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); + config.ignorePaths().add("foo")); } @Test 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 1945e07..79bb39d 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 @@ -231,7 +231,7 @@ void excludedFilesAreNotReportedAsUnmapped() { // Explicit opt-out must stay silent — an excluded file is neither // mapped nor flagged as unmapped. AffectedTestsConfig excludeConfig = AffectedTestsConfig.builder() - .excludePaths(java.util.List.of("**/generated/**")) + .ignorePaths(java.util.List.of("**/generated/**")) .build(); PathToClassMapper excludeMapper = new PathToClassMapper(excludeConfig); 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 953b0ac..38d7198 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 @@ -94,42 +94,6 @@ public AffectedTestTask() { @Input public abstract Property getIncludeStaged(); - /** - * Whether to run the full test suite when no affected tests are found. - * v2 back-compat — translated into {@link Situation#EMPTY_DIFF}, - * {@link Situation#ALL_FILES_IGNORED}, - * {@link Situation#ALL_FILES_OUT_OF_SCOPE}, and - * {@link Situation#DISCOVERY_EMPTY} actions by the core config - * builder when set. Unset means "let the v2 resolver pick defaults". - * Default: unset (matching pre-v2 {@code false} once translated). - * - *

    Marked {@link org.gradle.api.tasks.Optional @Optional} because - * the extension no longer installs a convention — the Gradle task - * must be free to leave the property unset when the user has not - * overridden the legacy boolean. - * - * @return the run-all-if-no-matches property - */ - @Input - @org.gradle.api.tasks.Optional - public abstract Property getRunAllIfNoMatches(); - - /** - * Whether to force a full test run when the change set contains any - * file that cannot be resolved to a Java class under the configured - * source/test directories. v2 back-compat — translates into - * {@link Situation#UNMAPPED_FILE}'s action. - * - *

    Marked {@link org.gradle.api.tasks.Optional @Optional} because - * the extension no longer installs a convention; leaving it unset - * is what lets the v2 resolver reach its own defaults. - * - * @return the run-all-on-non-java-change property - */ - @Input - @org.gradle.api.tasks.Optional - public abstract Property getRunAllOnNonJavaChange(); - /** * Discovery strategies to use for finding affected tests. * Valid values: {@code "naming"}, {@code "usage"}, {@code "impl"}, {@code "transitive"}. @@ -183,18 +147,6 @@ public AffectedTestTask() { @Input public abstract ListProperty getTestDirs(); - /** - * v2 back-compat alias for {@link #getIgnorePaths()}. When neither - * is set, the core config's default ignore-path list applies. - * - * @return the exclude paths list property - * @deprecated prefer {@link #getIgnorePaths()}. - */ - @Input - @org.gradle.api.tasks.Optional - @Deprecated - public abstract ListProperty getExcludePaths(); - /** * Glob patterns for files that must never influence test selection. * Optional — when unset, the core config's default list applies. @@ -246,8 +198,8 @@ public AffectedTestTask() { /** * Execution profile name — one of {@code "auto"}, {@code "local"}, - * {@code "ci"}, {@code "strict"}. Unset leaves defaults in - * pre-v2 mode. + * {@code "ci"}, {@code "strict"}. Unset resolves to {@code "auto"} + * in the core config, which detects CI from common env vars. * * @return the mode property */ @@ -366,16 +318,6 @@ 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(); @@ -431,9 +373,9 @@ public void runAffectedTests() { /** * Assembles the immutable core config from the task's Gradle - * properties. Legacy booleans and the new situation/mode knobs are - * all optional at this layer; the core builder handles precedence - * (explicit > legacy-boolean > mode > pre-v2 default). + * properties. All situation/mode knobs are optional at this layer; + * the core builder handles precedence via the v2 two-tier ladder + * (explicit {@code onXxx} > mode default). */ private AffectedTestsConfig buildConfig() { AffectedTestsConfig.Builder builder = AffectedTestsConfig.builder() @@ -448,16 +390,8 @@ private AffectedTestsConfig buildConfig() { .includeImplementationTests(getIncludeImplementationTests().get()) .implementationNaming(getImplementationNaming().get()); - if (getRunAllIfNoMatches().isPresent()) { - builder.runAllIfNoMatches(getRunAllIfNoMatches().get()); - } - if (getRunAllOnNonJavaChange().isPresent()) { - builder.runAllOnNonJavaChange(getRunAllOnNonJavaChange().get()); - } if (getIgnorePaths().isPresent() && !getIgnorePaths().get().isEmpty()) { builder.ignorePaths(getIgnorePaths().get()); - } else if (getExcludePaths().isPresent() && !getExcludePaths().get().isEmpty()) { - builder.excludePaths(getExcludePaths().get()); } if (getOutOfScopeTestDirs().isPresent() && !getOutOfScopeTestDirs().get().isEmpty()) { builder.outOfScopeTestDirs(getOutOfScopeTestDirs().get()); @@ -1014,17 +948,13 @@ public Object[] args() { */ static String describeEscalation(EscalationReason reason) { Objects.requireNonNull(reason, "reason"); - // Phrases deliberately keep the legacy flag names ("runAllIfNoMatches=true", - // "runAllOnNonJavaChange=true") alongside the new situation-based name so - // existing CI greps stay matched. Removing either side would require a - // coordinated pipeline migration we do not want to force in Phase 1. return switch (reason) { case RUN_ALL_ON_NON_JAVA_CHANGE -> - "runAllOnNonJavaChange=true / onUnmappedFile=FULL_SUITE — non-Java or unmapped file in diff"; + "onUnmappedFile=FULL_SUITE — non-Java or unmapped file in diff"; case RUN_ALL_ON_EMPTY_CHANGESET -> - "runAllIfNoMatches=true / onEmptyDiff=FULL_SUITE — no changed files detected"; + "onEmptyDiff=FULL_SUITE — no changed files detected"; case RUN_ALL_IF_NO_MATCHES -> - "runAllIfNoMatches=true / onDiscoveryEmpty=FULL_SUITE — no affected tests discovered"; + "onDiscoveryEmpty=FULL_SUITE — no affected tests discovered"; case RUN_ALL_ON_ALL_FILES_IGNORED -> "onAllFilesIgnored=FULL_SUITE — every changed file matched ignorePaths"; case RUN_ALL_ON_ALL_FILES_OUT_OF_SCOPE -> @@ -1099,8 +1029,8 @@ static List renderLifecycleDispatchPreview(String taskPath, List * *

    Every section names the source of the decision so an operator * can see at a glance whether the action came from an explicit - * {@code onXxx} setting, a legacy boolean, the mode default table, - * or the pre-v2 hardcoded baseline. + * {@code onXxx} setting or the mode default table (the v2 two-tier + * resolver has no other sources). * *

    Package-private so {@code AffectedTestTaskExplainFormatTest} * can assert the format without spinning up Gradle. @@ -1109,11 +1039,13 @@ static List renderExplainTrace(AffectedTestsConfig config, AffectedTests List lines = new ArrayList<>(); lines.add("=== Affected Tests — decision trace (--explain) ==="); lines.add("Base ref: " + config.baseRef()); - String configuredMode = config.mode() == null ? "unset" : config.mode().name(); - // effectiveMode() is always non-null (see its Javadoc) — zero-config - // callers get the AUTO-detected value, identical to what an explicit - // `mode = "auto"` would have resolved to. - lines.add("Mode: " + configuredMode + // config.mode() is guaranteed non-null in v2 (defaults to AUTO when + // the user doesn't set it), so we don't branch on null here — doing + // so would only re-introduce the pre-v2 "unset" rendering we + // deliberately dropped. effectiveMode() is also always non-null + // (zero-config callers get the AUTO-detected value, identical to + // what an explicit `mode = "auto"` would have resolved to). + lines.add("Mode: " + config.mode().name() + " (effective: " + config.effectiveMode().name() + ")"); lines.add("Changed files: " + result.changedFiles().size()); @@ -1288,10 +1220,8 @@ private static List situationOrder() { private static String describeSource(ActionSource source) { return switch (source) { - case EXPLICIT -> "explicit onXxx setting"; - case LEGACY_BOOLEAN -> "legacy boolean (runAllIfNoMatches / runAllOnNonJavaChange)"; - case MODE_DEFAULT -> "mode default"; - case HARDCODED_DEFAULT -> "pre-v2 hardcoded default"; + case EXPLICIT -> "explicit onXxx setting"; + case MODE_DEFAULT -> "mode default"; }; } @@ -1311,22 +1241,16 @@ private static String describeSource(ActionSource source) { * *

    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. + * across runs with different selection sizes. */ 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. + // The "running full suite (reason)" phrase is the single + // place the reason string shows up in the summary line. + // Reason phrase is sourced from describeEscalation to + // avoid duplicating the vocabulary across two files. return new LogLine( prefix + "{} changed file(s); running full suite ({}).", new Object[] { 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 17f348c..df76d46 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 @@ -15,8 +15,7 @@ * // diff while iterating on tests locally. * includeUncommitted = false * includeStaged = false - * // v2: per-situation actions (replaces runAllIfNoMatches / runAllOnNonJavaChange). - * // See README.md "Migrating from v1 config" for the full table. + * // v2: per-situation actions. See README.md for the full table. * mode = "ci" * onEmptyDiff = "full_suite" * onAllFilesOutOfScope = "skipped" @@ -63,28 +62,6 @@ public abstract class AffectedTestsExtension { */ public abstract Property getIncludeStaged(); - /** - * Run full test suite if no affected tests are found. - * Default: {@code false}. - * - * @return the run-all-if-no-matches property - */ - public abstract Property getRunAllIfNoMatches(); - - /** - * Force a full test run whenever the change set contains any file that - * cannot be resolved to a Java class under {@link #getSourceDirs()} or - * {@link #getTestDirs()} — for example {@code application.yml}, - * {@code build.gradle}, a Liquibase changelog, or a logback config. - * Files matching {@link #getExcludePaths()} are treated as an explicit - * opt-out and do not trigger the escalation. - * - *

    Default: {@code true} — "run more, never run less". - * - * @return the run-all-on-non-java-change property - */ - public abstract Property getRunAllOnNonJavaChange(); - /** * Strategies to use for test discovery. Valid values: * {@code "naming"}, {@code "usage"}, {@code "impl"}, {@code "transitive"}. @@ -132,19 +109,6 @@ public abstract class AffectedTestsExtension { */ public abstract ListProperty getTestDirs(); - /** - * Glob patterns for files to exclude from analysis. - * v2 back-compat alias for {@link #getIgnorePaths()}. Default is an - * empty Gradle property — when unset, the v2 {@code ignorePaths} - * default applies (a broader list than the pre-v2 single-entry - * {@code ["**}{@code /generated/**"]} list). - * - * @return the exclude paths list property - * @deprecated prefer {@link #getIgnorePaths()} in new configs. - */ - @Deprecated - public abstract ListProperty getExcludePaths(); - /** * Glob patterns for files that must never influence test selection — * for purely documentation, build metadata, or generated artifacts. @@ -200,13 +164,11 @@ public abstract class AffectedTestsExtension { /** * Execution profile ({@code "auto"}, {@code "local"}, {@code "ci"}, * or {@code "strict"}). Controls per-situation default actions - * when neither the explicit {@code onXxx} setting nor the legacy - * boolean translation has picked one. + * when the explicit {@code onXxx} setting is not set. * - *

    Default: unset — which preserves the pre-v2 defaults for - * zero-config users. Setting it to {@code "auto"} is the - * recommended migration: it detects CI via common env vars and - * picks the CI defaults there, and the LOCAL defaults otherwise. + *

    Default: unset — which resolves to {@link io.affectedtests.core.config.Mode#AUTO} + * on the core config. {@code AUTO} detects CI via common env vars + * and picks the CI defaults there, and the LOCAL defaults otherwise. * * @return the mode property */ @@ -266,9 +228,7 @@ public abstract class AffectedTestsExtension { * *

    Unset falls through to the {@link #getMode() mode} default * (CI and STRICT escalate to {@code full_suite}; LOCAL keeps the - * partial selection). The legacy {@code runAllIfNoMatches=true} shim - * also maps to {@code full_suite} here because it is the closest - * existing signal for "I don't trust a silent no-signal result". + * partial selection). * * @return the on-discovery-incomplete property */ diff --git a/affected-tests-gradle/src/main/java/io/affectedtests/gradle/AffectedTestsPlugin.java b/affected-tests-gradle/src/main/java/io/affectedtests/gradle/AffectedTestsPlugin.java index 02c8c6d..1e80a64 100644 --- a/affected-tests-gradle/src/main/java/io/affectedtests/gradle/AffectedTestsPlugin.java +++ b/affected-tests-gradle/src/main/java/io/affectedtests/gradle/AffectedTestsPlugin.java @@ -36,23 +36,17 @@ public void apply(Project project) { // never silently expands the diff boundary. extension.getIncludeUncommitted().convention(false); extension.getIncludeStaged().convention(false); - // No conventions for the two legacy booleans — leaving them - // unset is the v2 signal that the caller has not overridden the - // defaults, which lets the core config resolver apply - // mode-based defaults instead of always shimming the pre-v2 - // boolean translation. Zero-config users still get pre-v2 - // behaviour because the core builder's hardcoded fallbacks - // match the old convention values 1:1. extension.getStrategies().convention(List.of("naming", "usage", "impl", "transitive")); extension.getTransitiveDepth().convention(4); extension.getTestSuffixes().convention(List.of("Test", "IT", "ITTest", "IntegrationTest")); extension.getSourceDirs().convention(List.of("src/main/java")); extension.getTestDirs().convention(List.of("src/test/java")); - // No convention for excludePaths / ignorePaths: an empty Gradle - // provider is how we signal "let the core config builder pick - // the v2 default list" (which is broader than the pre-v2 - // single-entry default). Setting a convention here would pin - // the pre-v2 narrow default even on zero-config installs. + // No convention for ignorePaths: an empty Gradle provider is how + // we signal "let the core config builder pick the default list". + // Setting a convention here would stop callers who want to + // explicitly wipe the default list with an empty list, and + // would also pin the list shape to this file rather than to + // the core {@code AffectedTestsConfig.Builder.DEFAULT_IGNORE_PATHS}. extension.getIncludeImplementationTests().convention(true); extension.getImplementationNaming().convention(List.of("Impl", "Default")); @@ -66,14 +60,11 @@ public void apply(Project project) { task.getBaseRef().set(extension.getBaseRef()); task.getIncludeUncommitted().set(extension.getIncludeUncommitted()); task.getIncludeStaged().set(extension.getIncludeStaged()); - task.getRunAllIfNoMatches().set(extension.getRunAllIfNoMatches()); - task.getRunAllOnNonJavaChange().set(extension.getRunAllOnNonJavaChange()); task.getStrategies().set(extension.getStrategies()); task.getTransitiveDepth().set(extension.getTransitiveDepth()); task.getTestSuffixes().set(extension.getTestSuffixes()); task.getSourceDirs().set(extension.getSourceDirs()); task.getTestDirs().set(extension.getTestDirs()); - task.getExcludePaths().set(extension.getExcludePaths()); task.getIgnorePaths().set(extension.getIgnorePaths()); task.getOutOfScopeTestDirs().set(extension.getOutOfScopeTestDirs()); task.getOutOfScopeSourceDirs().set(extension.getOutOfScopeSourceDirs()); 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 c5c843b..3cce494 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 @@ -118,27 +118,6 @@ void bucketSampleIsTruncatedWithRemainderCount() { + "masquerade as a 10-file diff in the trace"); } - @Test - void actionSourceSurfacesLegacyBooleanWhenOnlyLegacyFlagIsSet() { - AffectedTestsConfig config = AffectedTestsConfig.builder() - .runAllIfNoMatches(false) - .build(); - AffectedTestsResult result = new AffectedTestsResult( - Set.of(), Map.of(), - Set.of("README.md"), Set.of(), Set.of(), - new Buckets(Set.of("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)); - - assertTrue(trace.contains("Action: SKIPPED (source: legacy boolean"), - "Setting only the legacy boolean must surface as LEGACY_BOOLEAN in the trace, " - + "not silently show up as a mode default or explicit setting"); - } - @Test void actionSourceSurfacesExplicitSettingWhenOnXxxWins() { AffectedTestsConfig config = AffectedTestsConfig.builder() @@ -156,7 +135,7 @@ void actionSourceSurfacesExplicitSettingWhenOnXxxWins() { String trace = joined(AffectedTestTask.renderExplainTrace(config, result)); assertTrue(trace.contains("source: explicit onXxx setting"), - "Explicit setting must surface distinctly from legacy/mode/hardcoded sources — " + "Explicit setting must surface distinctly from the mode-default source — " + "otherwise the operator can't tell what survives a future default change"); } @@ -186,7 +165,12 @@ void actionSourceSurfacesModeDefaultWhenOnlyModeIsSet() { } @Test - void actionSourceSurfacesHardcodedDefaultForZeroConfig() { + void actionSourceSurfacesModeDefaultForZeroConfig() { + // v2.0 removed the pre-v2 hardcoded-default tier — zero-config + // installs now resolve straight to a mode default (AUTO → LOCAL + // or CI based on env detection). The --explain trace must name + // that honestly so operators know the resolved action can shift + // between LOCAL and CI based on where the build runs. AffectedTestsConfig config = AffectedTestsConfig.builder().build(); AffectedTestsResult result = new AffectedTestsResult( Set.of(), Map.of(), @@ -199,10 +183,10 @@ void actionSourceSurfacesHardcodedDefaultForZeroConfig() { String trace = joined(AffectedTestTask.renderExplainTrace(config, result)); - assertTrue(trace.contains("source: pre-v2 hardcoded default"), - "Zero-config installs must see the pre-v2 label — so operators know upgrading " - + "the plugin's defaults could silently change their behaviour if they " - + "do not pin a mode or onXxx setting"); + assertTrue(trace.contains("source: mode default"), + "Zero-config installs must surface MODE_DEFAULT in v2.0 — the pre-v2 " + + "hardcoded-default tier has been removed and every action now " + + "resolves through a concrete mode"); } @Test 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 0a20343..facfb93 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 @@ -223,16 +223,16 @@ void nonJavaEscalationNamesTheRealTrigger() { assertTrue(summary.contains("running full suite"), "A runAll summary must tell the reader we're running the full suite — not the " + "non-runAll count-based line that would say '0 production class(es), 0 test class(es) affected'"); - assertTrue(summary.contains("runAllOnNonJavaChange=true"), + assertTrue(summary.contains("onUnmappedFile=FULL_SUITE"), "The real trigger must appear in the log or the message defeats its own purpose"); assertTrue(summary.contains("non-Java or unmapped"), - "Render must explain what runAllOnNonJavaChange actually detected, not just name the flag"); + "Render must explain what onUnmappedFile actually detected, not just name the knob"); } @Test void emptyChangesetEscalationNamesItsOwnTrigger() { // Guards the bug the prior amendment still carried: empty changeset + - // runAllIfNoMatches shared the "no affected tests discovered" phrase + // onEmptyDiff=FULL_SUITE shared the "no affected tests discovered" phrase // with the post-discovery empty branch, even though discovery had // never actually run. The two branches must produce distinct phrases. AffectedTestsResult result = new AffectedTestsResult( @@ -251,8 +251,8 @@ void emptyChangesetEscalationNamesItsOwnTrigger() { String summary = render(AffectedTestTask.renderSummary(result)); assertTrue(summary.contains("running full suite")); - assertTrue(summary.contains("runAllIfNoMatches=true"), - "Empty-changeset escalation is driven by runAllIfNoMatches, so the flag name " + assertTrue(summary.contains("onEmptyDiff=FULL_SUITE"), + "Empty-changeset escalation is driven by onEmptyDiff, so the knob name " + "must surface in the log the same way it does in the config DSL"); assertTrue(summary.contains("no changed files"), "Phrase must say why we escalated — nothing had changed — instead of claiming " @@ -277,7 +277,7 @@ void postDiscoveryEmptyEscalationDistinguishesItselfFromEmptyChangeset() { String summary = render(AffectedTestTask.renderSummary(result)); assertTrue(summary.contains("running full suite")); - assertTrue(summary.contains("runAllIfNoMatches=true")); + assertTrue(summary.contains("onDiscoveryEmpty=FULL_SUITE")); assertTrue(summary.contains("no affected tests discovered"), "Post-discovery empty must say discovery ran and returned nothing, so the " + "reader can distinguish this case from the empty-changeset one"); diff --git a/affected-tests-gradle/src/test/java/io/affectedtests/gradle/AffectedTestsPluginTest.java b/affected-tests-gradle/src/test/java/io/affectedtests/gradle/AffectedTestsPluginTest.java index 5d0d7e6..b85e672 100644 --- a/affected-tests-gradle/src/test/java/io/affectedtests/gradle/AffectedTestsPluginTest.java +++ b/affected-tests-gradle/src/test/java/io/affectedtests/gradle/AffectedTestsPluginTest.java @@ -1,5 +1,6 @@ package io.affectedtests.gradle; +import io.affectedtests.core.config.AffectedTestsConfig; import org.gradle.api.Project; import org.gradle.testfixtures.ProjectBuilder; import org.junit.jupiter.api.Test; @@ -45,15 +46,6 @@ void extensionHasDefaults() { "Default must be COMMITTED-ONLY so local runs match CI — WIP inclusion is an explicit opt-in"); assertFalse(ext.getIncludeStaged().get(), "Default must be COMMITTED-ONLY so local runs match CI — staged-index inclusion is an explicit opt-in"); - // v2: no convention for the legacy booleans — leaving them unset - // is the signal the core builder uses to fall through to - // mode-based defaults. Zero-config users still observe pre-v2 - // behaviour because the builder's hard-coded fallbacks match the - // old convention values 1:1 (see AffectedTestsConfigTest). - assertFalse(ext.getRunAllIfNoMatches().isPresent(), - "v2 must not install a convention for runAllIfNoMatches; the core config resolver does the translation"); - assertFalse(ext.getRunAllOnNonJavaChange().isPresent(), - "v2 must not install a convention for runAllOnNonJavaChange; the core config resolver does the translation"); assertEquals(4, ext.getStrategies().get().size()); assertTrue(ext.getStrategies().get().contains("transitive")); // v2 raises the default transitive depth from 2 to 4 — real-world @@ -92,15 +84,15 @@ void taskInputsAreWiredFromExtension() { .getByType(AffectedTestsExtension.class); ext.getBaseRef().set("origin/main"); ext.getTransitiveDepth().set(0); - ext.getRunAllOnNonJavaChange().set(false); + ext.getOnUnmappedFile().set("SELECTED"); // Verify task picks up the change AffectedTestTask task = (AffectedTestTask) project.getTasks().findByName("affectedTest"); assertNotNull(task); assertEquals("origin/main", task.getBaseRef().get()); assertEquals(0, task.getTransitiveDepth().get()); - assertFalse(task.getRunAllOnNonJavaChange().get(), - "Task input must reflect the extension's runAllOnNonJavaChange override"); + assertEquals("SELECTED", task.getOnUnmappedFile().get(), + "Task input must reflect the extension's onUnmappedFile override"); } @Test @@ -145,4 +137,72 @@ void gradlewTimeoutAndDiscoveryIncompleteAreUnconfiguredByDefault() { assertFalse(ext.getGradlewTimeoutSeconds().isPresent(), "gradlewTimeoutSeconds must have no convention so the task branch between watchdog/exec paths stays correct"); } + + @Test + void legacyDslKnobsNoLongerExistInV2() { + // v2.0 breaking change: the three v1 knobs (`runAllIfNoMatches`, + // `runAllOnNonJavaChange`, `excludePaths`) were removed entirely + // after being deprecated across the v1.9.x line. This test locks + // in their absence so a well-intentioned "restore the getter for + // back-compat" revert gets caught at build time instead of + // silently re-opening the v1 API surface and the deprecation + // warnings it came with. + // + // We check reflectively rather than calling the accessors + // directly (which would turn into a compile error the moment + // someone re-adds them, masking the intent of the test). The + // contract is: none of the pre-v2 accessors exist anywhere on + // the public surface in v2+ — not on the extension, not on the + // task, not on the config, not on the builder. + java.util.List forbiddenAccessors = java.util.List.of( + "getRunAllIfNoMatches", + "getRunAllOnNonJavaChange", + "getExcludePaths" + ); + assertAllAbsent(AffectedTestsExtension.class, forbiddenAccessors, + "extension"); + assertAllAbsent(AffectedTestTask.class, forbiddenAccessors, + "task"); + + // On the core config type the legacy accessors were unprefixed + // (`runAllIfNoMatches()`, not `getRunAllIfNoMatches()`), and + // `deprecationWarnings()` was its own list we also deleted — if + // any of them come back the two-tier resolver docs are + // immediately out of sync with reality. + java.util.List forbiddenConfigAccessors = java.util.List.of( + "runAllIfNoMatches", + "runAllOnNonJavaChange", + "excludePaths", + "deprecationWarnings" + ); + assertAllAbsent(AffectedTestsConfig.class, forbiddenConfigAccessors, + "config"); + + // And the three Builder setters that fed those accessors. We + // pin them separately because a partial revert that restores + // the builder-side without the getter-side would be just as + // bad — it would silently accept the v1 DSL in Groovy while + // doing nothing with it. + java.util.List forbiddenBuilderSetters = java.util.List.of( + "runAllIfNoMatches", + "runAllOnNonJavaChange", + "excludePaths" + ); + assertAllAbsent(AffectedTestsConfig.Builder.class, forbiddenBuilderSetters, + "config builder"); + } + + private static void assertAllAbsent(Class type, + java.util.List forbiddenNames, + String surfaceLabel) { + for (String name : forbiddenNames) { + boolean stillDeclared = java.util.Arrays.stream(type.getMethods()) + .anyMatch(m -> m.getName().equals(name)); + assertFalse(stillDeclared, + type.getSimpleName() + "." + name + "() must stay removed in v2 — " + + "bringing it back on the " + surfaceLabel + " surface reopens " + + "the v1 back-compat path and breaks the migration documented " + + "in the CHANGELOG"); + } + } } diff --git a/docs/DESIGN-v2.md b/docs/DESIGN-v2.md index 7a87ba6..8bbd9b3 100644 --- a/docs/DESIGN-v2.md +++ b/docs/DESIGN-v2.md @@ -457,12 +457,21 @@ directly instead of working around the tool. - Docs flip all examples to new config. - README adds a migration section. -### Phase 3 — Breaking (next major, e.g. v2.0.0) - -- Remove legacy `runAllIfNoMatches` / `runAllOnNonJavaChange` / `excludePaths`. -- Rename `runAllOnNonJavaChange`'s successor if we choose a better name - (e.g. `onUnmappedFile` — already the Phase 1 name, so this collapses - naturally). +### Phase 3 — Breaking (v2.0.0) ✅ done + +- ✅ Removed legacy `runAllIfNoMatches` / `runAllOnNonJavaChange` / + `excludePaths` from `AffectedTestsExtension`, `AffectedTestsConfig`, + and the resolver priority ladder. +- ✅ Removed `ActionSource.LEGACY_BOOLEAN` and + `ActionSource.HARDCODED_DEFAULT`. The resolver is now a two-tier + structure (explicit `onXxx` > mode default); `Mode.AUTO` always + resolves to a concrete mode at `effectiveMode()`, so there is no + hardcoded-default fall-through to report. +- ✅ Removed the `AffectedTestsConfig.deprecationWarnings()` list and + the startup warning loop the Gradle task used to surface them. +- ✅ Regression test (`legacyDslKnobsNoLongerExistInV2`) reflectively + pins the absence of the three removed getters so a back-compat + revert is caught at build time. --- diff --git a/docs/architecture.mmd b/docs/architecture.mmd index 6e211d7..38315bf 100644 --- a/docs/architecture.mmd +++ b/docs/architecture.mmd @@ -45,10 +45,8 @@ flowchart TD Resolve{{"Resolve Action
    (priority order)"}}:::core Resolve --> Explicit["1 · explicit onXxx setting"]:::priority - Explicit --> Legacy["2 · legacy boolean
    (runAllIfNoMatches / runAllOnNonJavaChange)"]:::priority - Legacy --> ModeDefault["3 · mode default
    (auto · local · ci · strict)"]:::priority - ModeDefault --> Hardcoded["4 · pre-v2 hardcoded default"]:::priority - Hardcoded --> Action{Action}:::core + Explicit --> ModeDefault["2 · mode default
    (auto · local · ci · strict)"]:::priority + ModeDefault --> Action{Action}:::core Action -->|SELECTED| Router[Group FQNs by
    owning Gradle subproject]:::core Action -->|FULL_SUITE| FullSuite["gradle test
    full suite · no --tests filter"]:::io diff --git a/docs/architecture.svg b/docs/architecture.svg index 4263f97..b4b135b 100644 --- a/docs/architecture.svg +++ b/docs/architecture.svg @@ -1 +1 @@ -

    no

    yes

    matches
    ignorePaths

    under outOfScope
    TestDirs / SourceDirs

    .java under
    sourceDirs

    .java under
    testDirs

    everything else
    yml · gradle · migrations

    all files ignored

    all files out of scope

    unmapped non-empty

    only in-scope .java

    yes

    no

    SELECTED

    FULL_SUITE

    SKIPPED

    git diff baseRef..HEAD
    + uncommitted + staged

    Any changed
    files?

    Situation
    EMPTY_DIFF

    PathToClassMapper
    buckets each path

    ignored bucket
    *.md · LICENSE · generated

    out-of-scope bucket
    api-test · gatling

    production bucket
    changed class FQNs

    test bucket
    directly changed test FQNs

    unmapped bucket

    Classify situation

    Situation
    ALL_FILES_IGNORED

    Situation
    ALL_FILES_OUT_OF_SCOPE

    Situation
    UNMAPPED_FILE

    AffectedTestsEngine
    4 strategies · merged

    naming
    Foo → FooTest / FooIT

    usage
    JavaParser: imports + type refs

    impl
    subtypes of changed interfaces
    (Impl / Default prefixes)

    transitive
    reverse-dep walk · depth 4

    Union of
    affected test FQNs

    Union empty?

    Situation
    DISCOVERY_EMPTY

    Situation
    DISCOVERY_SUCCESS

    Resolve Action
    (priority order)

    1 · explicit onXxx setting

    2 · legacy boolean
    (runAllIfNoMatches / runAllOnNonJavaChange)

    3 · mode default
    (auto · local · ci · strict)

    4 · pre-v2 hardcoded default

    Action

    Group FQNs by
    owning Gradle subproject

    gradle test
    full suite · no --tests filter

    Exit 0 · skip tests

    :moduleA:test --tests FooTest
    :moduleB:test --tests BarTest

     \ No newline at end of file +

    no

    yes

    matches
    ignorePaths

    under outOfScope
    TestDirs / SourceDirs

    .java under
    sourceDirs

    .java under
    testDirs

    everything else
    yml · gradle · migrations

    all files ignored

    all files out of scope

    unmapped non-empty

    only in-scope .java

    yes

    no

    SELECTED

    FULL_SUITE

    SKIPPED

    git diff baseRef..HEAD
    + uncommitted + staged

    Any changed
    files?

    Situation
    EMPTY_DIFF

    PathToClassMapper
    buckets each path

    ignored bucket
    *.md · LICENSE · generated

    out-of-scope bucket
    api-test · gatling

    production bucket
    changed class FQNs

    test bucket
    directly changed test FQNs

    unmapped bucket

    Classify situation

    Situation
    ALL_FILES_IGNORED

    Situation
    ALL_FILES_OUT_OF_SCOPE

    Situation
    UNMAPPED_FILE

    AffectedTestsEngine
    4 strategies · merged

    naming
    Foo → FooTest / FooIT

    usage
    JavaParser: imports + type refs

    impl
    subtypes of changed interfaces
    (Impl / Default prefixes)

    transitive
    reverse-dep walk · depth 4

    Union of
    affected test FQNs

    Union empty?

    Situation
    DISCOVERY_EMPTY

    Situation
    DISCOVERY_SUCCESS

    Resolve Action
    (priority order)

    1 · explicit onXxx setting

    2 · mode default
    (auto · local · ci · strict)

    Action

    Group FQNs by
    owning Gradle subproject

    gradle test
    full suite · no --tests filter

    Exit 0 · skip tests

    :moduleA:test --tests FooTest
    :moduleB:test --tests BarTest

     \ No newline at end of file