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