fix/discovery-incomplete-and-gradlew-timeout: surface parse failures as DISCOVERY_INCOMPLETE and deadline nested gradlew invocations#33
Merged
vedanthvdev merged 1 commit intomasterfrom Apr 22, 2026
Conversation
…as DISCOVERY_INCOMPLETE and deadline nested gradlew invocations Closes the two deferred findings from the v1.9.21 reliability batch. Before this release, a single mid-refactor syntax error in a scanned Java file would silently shrink the test selection: `JavaParsers.parseOrWarn` returned null, the dispatch map was built from whatever happened to parse, and the engine reported `DISCOVERY_SUCCESS` — exactly the scenario where the merge-gate safety net needs to pay out. `ProjectIndex` now counts parse failures at the cache boundary (de-duplicated across strategies) and `AffectedTestsEngine` routes through a new `Situation.DISCOVERY_INCOMPLETE` before the old empty/success branches. Mode defaults keep the existing safety-vs-speed asymmetry: `CI`/`STRICT` escalate to `FULL_SUITE`, `LOCAL` stays on `SELECTED` because the developer already saw the WARN in their terminal. The new `onDiscoveryIncomplete` DSL knob lets operators override per-project, and a matching `EscalationReason.RUN_ALL_ON_DISCOVERY_INCOMPLETE` feeds `--explain` so a full-suite run from a parse failure is distinguishable from one triggered by an empty diff or an unmapped file. The second half of the release deadlines the nested `./gradlew :module:test` invocation. Before this release, that child was launched via `ExecOperations` with no timeout surface at all — a hung test kept the CI worker busy until the outer CI-job deadline killed everything, which typically surfaces as "pipeline failed with no useful logs" hours later. The new `affectedTests.gradlewTimeoutSeconds` knob defaults to `0` (preserves the wait-forever behaviour for zero-config upgrades) and, when set, branches onto a `ProcessBuilder` watchdog that runs a polite `destroy()` → 10s grace → `destroyForcibly()` → 5s reap ladder and fails the build with a message that names the setting. Because `./gradlew` connects to a shared daemon JVM, killing only the wrapper would leave the actually-hung test JVM running as a grandchild; the watchdog snapshots `ProcessHandle.descendants()` before signalling and reaps every still-live descendant on both legs of the ladder — the initial cut only reaped on the forcible leg, which failed the regression test that now pins the behaviour because a SIGTERM-responsive wrapper (plain `sh`, most test runners) exits on `destroy()` and leaves its children re-parented to pid 1. Outer-build cancellation propagates through the same path and re-asserts the interrupt. Validation lives at the builder gate (rejects negative values, pins the zero default) and at runtime in a new `AffectedTestTaskTimeoutTest` that drives the watchdog against a hung `sleep 60` child and verifies the grandchild PID via `ProcessHandle` after the exception surfaces. The PR also addresses the tier-2 findings from the code-reviewer sweep on the initial cut: `situationOrder()` and the matrix-order regression test now include `DISCOVERY_INCOMPLETE` in the correct evaluation position, `describeSkipReason` branches on `Action` so `DISCOVERY_INCOMPLETE + SELECTED + empty` no longer renders the contradictory "SELECTED … onDiscoveryIncomplete=SKIPPED" summary, `Situation` javadoc is refreshed from "six" to "seven" situations with the new evaluation order, `DISCOVERY_INCOMPLETE` is explicitly unwired from the `runAllIfNoMatches` legacy boolean so `CI` mode cannot silently skip on parse failures, and the `AffectedTestsEngine.run()` tail no longer duplicates the result-construction block or drops the `=== Result` log header.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the two deferred reliability findings on the path to v2.0 (v1.9.22):
Situation.DISCOVERY_INCOMPLETEnow fires on any scanned Java file that fails to parse.ProjectIndexcounts parse failures at the cache boundary (de-duplicated across strategies) andAffectedTestsEngineroutes through the new situation before the old empty/success branches. Mode defaults:CI/STRICTescalate toFULL_SUITE,LOCALstays onSELECTED. New DSL knobonDiscoveryIncomplete; matchingEscalationReason.RUN_ALL_ON_DISCOVERY_INCOMPLETEin--explain.affectedTests.gradlewTimeoutSecondsdeadlines the nested./gradlewinvocation. Defaults to0(pre-v1.9.22 wait-forever behaviour). Positive values branch onto aProcessBuilderwatchdog with adestroy()→ 10s grace →destroyForcibly()→ 5s reap ladder. Descendants (shared Gradle daemons, test JVMs) are snapshotted before the first signal and reaped on both legs of the ladder — the initial cut only reaped on the forcible leg, which failed the new regression test because SIGTERM-responsive wrappers exit ondestroy()and leave children re-parented to pid 1.Tier-2 findings from the code-reviewer sweep are also addressed in this PR:
situationOrder()+ matrix regression test, action-awaredescribeSkipReason,Situationjavadoc refresh,DISCOVERY_INCOMPLETEunwired fromrunAllIfNoMatches, andAffectedTestsEngine.run()tail deduplicated (no more missing=== Resultheader).Test plan
./gradlew check— all core + gradle module tests green, including the newAffectedTestTaskTimeoutTestsuiteshutdownChild, confirmeddescendantsAreTerminatedNotOnlyTheWrapperflips red (PID survives), restored the fixparseFailureCount++inProjectIndex, confirmed theDISCOVERY_INCOMPLETEengine tests fail, restored the fixsecurity-servicewith the resulting v1.9.22 build to confirm the DSL knob and the watchdog behave as documented under a real merge-gate diff