fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

#337: extend ConfigRefTopLevelReportingCoverageTest to DEFERRED_KEYS #347

Closed
agent wants to merge 0 commits from worker/fleetd-337-5ec7d4-21 into main
pull from: worker/fleetd-337-5ec7d4-21
merge into: fleet:main
Conversation 0 Commits - Files Changed -
agent commented 2026-09-04 11:07:40 +02:00
Member
Copy Link
Copy Source

fleetd #337 — DEFERRED_KEYS reporting coverage

ConfigRefTopLevelReportingCoverageTest (added by #333) proved every COLD_KEYS/SPLIT_KEYS
member has a real comparison behind it in ConfigRef. It deliberately did not cover
DEFERRED_TOP_LEVEL_KEYS — 11 of the 22 top-level FleetConfig keys, the largest bucket. This
ticket closes that gap.

Re-derived denominator by mutation, not by trusting the issue's list

I did not take the issue's list of "deferred keys with no individual behavioural test" as fact.
I re-derived it myself: for each of the 11 DEFERRED_KEYS members, I dropped that key's
comparison branch out of ConfigRef.changedDeferredKeys (wrapped the if condition in
false && (...)), ran mvn test unpiped, recorded pass/fail, then restored.

Real measured result — 6 of 11 uncovered by any existing test:

Key Dropping its branch... Covered?
lifecycle fails ConfigRefTest.aDeferredChangeIsAppliedAndReported + aSplitChangeAndADeferredChangeCoexist covered
leadHeartbeat 1355 green UNCOVERED
guard 1355 green (already established by the lead) UNCOVERED
worktreeRoot 1355 green UNCOVERED
worktreeGroup fails changingWorktreeGroupIsReportedAsDeferred covered
primary fails changingPrimaryIsReportedAsDeferred covered
configReload fails changingConfigReloadIsReportedAsDeferred covered
spawnReadyTimeoutMs/spawnReadyPollMs (combined branch) 1355 green UNCOVERED
quarantineCooldownSeconds 1355 green UNCOVERED
profiles (added/removed) fails addingAProfileIsReportedAsDeferred covered
profiles (launch settings) fails 4 launch-settings tests covered

This contradicts the issue's own guessed list in two ways the mutation proved and a reading
did not:

  • lifecycle was on the issue's list of suspects — it is actually covered.
  • worktreeRoot was not on the issue's list at all — it is genuinely uncovered.

Mechanism chosen

Extended ConfigRefTopLevelReportingCoverageTest to DEFERRED_KEYS the same way it already
covers COLD_KEYS/SPLIT_KEYS: mutate one top-level component at a time on a reflection-built
FleetConfig pair and call the real ConfigRef.changedDeferredKeys to prove each key is actually
reported.

Every DEFERRED_KEYS component turned out to be a scalar or a simple record (Guard,
Lifecycle, Primary, LeadHeartbeat, ConfigReload) — unlike fleet.leaders in #333, no
exclusion set was needed. BASE/ALT now give every top-level component a real, distinct
value (previously all 11 deferred fields were null on both sides, which is why mutate(key)
never produced an actual difference for any of them). profiles is exercised via the
added/removed comparison only (a single added profile is enough to prove "profiles" has a branch
behind it); the launch-settings comparison already has its own hand-written ConfigRefTest cases.

Also promoted DEFERRED_TOP_LEVEL_KEYS into ConfigRef.DEFERRED_KEYS (package-private,
alongside COLD_KEYS/SPLIT_KEYS), replacing the test-side copy in
ConfigRefTopLevelCoverageTest — the same reason COLD_KEYS/SPLIT_KEYS are production
constants rather than test copies: two lists supposed to describe the same method are exactly the
shape that silently drifts apart. Checked no test relied on the two copies being able to differ
(grepped for other uses — none). Made changedDeferredKeys package-private to match
changedColdKeys/changedSplitKeys, needed for the reflective test to call it directly.

Invariants honored: ConfigRefTest's existing per-key tests are untouched (added coverage, not a
replacement); no reload behavior changed (test-only diff, plus one method's visibility and a set's
home); ConfigRefProfileCoverageTest untouched.

Mutation proof — the one-way direction that matters

Dropped guard's branch from changedDeferredKeys again, this time with the fix in place,
and ran the new test:

ConfigRef.changedDeferredKeys reporting coverage — 11 DEFERRED_KEYS, 10 verified
[ERROR] Tests run: 3, Failures: 1, Errors: 0, Skipped: 0 <<< FAILURE! -- in ConfigRefTopLevelReportingCoverageTest
[ERROR] ConfigRefTopLevelReportingCoverageTest.everyDeferredKeyIsActuallyReportedByChangedDeferredKeys
[ERROR]   ...these keys are in ConfigRef.DEFERRED_KEYS but mutating them alone produces no matching
  entry from changedDeferredKeys...: [guard] ==> expected: <[]> but was: <[guard]>

ConfigRefTest stayed fully green through the same mutation — confirming guard really had no
hand-written test catching it before this PR. Restored immediately after, verified with a fresh
mvn clean install.

Build

cd fleetd && mvn clean install, unpiped, final run:

Tests run: 1356, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

(1356 = 1355 on main at eee4d57 + 1 new test method.)

Shape survey — not fixed, just reported (per brief)

Other places in this repo treating a Set of names as coverage, found while working this ticket:

  • FleetConfig.KNOWN_TOP_LEVEL_KEYS (FleetConfig.java:1494) — used only to warn about unknown
    YAML keys; a companion test asserts every entry is documented in fleetd.example.yaml. Lower
    risk than ConfigRef's sets since it's a warn-only list, not a "this key's reload works" claim.
  • FleetConfig.FLEET_POOL_KEYS (FleetConfig.java:1527) — the fleet: child block names
    (leaders/architects/developers/reviewers); did not check what consumes it or whether
    membership is proven behaviourally.
  • ConfigRef.LAUNCH_SETTINGS_EXCLUDED (already has full reflective coverage via
    ConfigRefProfileCoverageTest — not a gap, just the same general shape).

Not investigated further, per scope.

Ticket: fleetd #337

## fleetd #337 — DEFERRED_KEYS reporting coverage `ConfigRefTopLevelReportingCoverageTest` (added by #333) proved every `COLD_KEYS`/`SPLIT_KEYS` member has a real comparison behind it in `ConfigRef`. It deliberately did not cover `DEFERRED_TOP_LEVEL_KEYS` — 11 of the 22 top-level `FleetConfig` keys, the largest bucket. This ticket closes that gap. ### Re-derived denominator by mutation, not by trusting the issue's list I did not take the issue's list of "deferred keys with no individual behavioural test" as fact. I re-derived it myself: for each of the 11 `DEFERRED_KEYS` members, I dropped that key's comparison branch out of `ConfigRef.changedDeferredKeys` (wrapped the `if` condition in `false && (...)`), ran `mvn test` **unpiped**, recorded pass/fail, then restored. Real measured result — **6 of 11 uncovered by any existing test**: | Key | Dropping its branch... | Covered? | |---|---|---| | `lifecycle` | fails `ConfigRefTest.aDeferredChangeIsAppliedAndReported` + `aSplitChangeAndADeferredChangeCoexist` | **covered** | | `leadHeartbeat` | 1355 green | **UNCOVERED** | | `guard` | 1355 green (already established by the lead) | **UNCOVERED** | | `worktreeRoot` | 1355 green | **UNCOVERED** | | `worktreeGroup` | fails `changingWorktreeGroupIsReportedAsDeferred` | covered | | `primary` | fails `changingPrimaryIsReportedAsDeferred` | covered | | `configReload` | fails `changingConfigReloadIsReportedAsDeferred` | covered | | `spawnReadyTimeoutMs`/`spawnReadyPollMs` (combined branch) | 1355 green | **UNCOVERED** | | `quarantineCooldownSeconds` | 1355 green | **UNCOVERED** | | `profiles` (added/removed) | fails `addingAProfileIsReportedAsDeferred` | covered | | `profiles` (launch settings) | fails 4 launch-settings tests | covered | This **contradicts the issue's own guessed list** in two ways the mutation proved and a reading did not: - `lifecycle` was on the issue's list of suspects — it is actually covered. - `worktreeRoot` was **not** on the issue's list at all — it is genuinely uncovered. ### Mechanism chosen Extended `ConfigRefTopLevelReportingCoverageTest` to `DEFERRED_KEYS` the same way it already covers `COLD_KEYS`/`SPLIT_KEYS`: mutate one top-level component at a time on a reflection-built `FleetConfig` pair and call the real `ConfigRef.changedDeferredKeys` to prove each key is actually reported. Every `DEFERRED_KEYS` component turned out to be a scalar or a simple record (`Guard`, `Lifecycle`, `Primary`, `LeadHeartbeat`, `ConfigReload`) — unlike `fleet.leaders` in #333, **no exclusion set was needed**. `BASE`/`ALT` now give every top-level component a real, distinct value (previously all 11 deferred fields were `null` on both sides, which is why `mutate(key)` never produced an actual difference for any of them). `profiles` is exercised via the added/removed comparison only (a single added profile is enough to prove "profiles" has a branch behind it); the launch-settings comparison already has its own hand-written `ConfigRefTest` cases. Also **promoted `DEFERRED_TOP_LEVEL_KEYS` into `ConfigRef.DEFERRED_KEYS`** (package-private, alongside `COLD_KEYS`/`SPLIT_KEYS`), replacing the test-side copy in `ConfigRefTopLevelCoverageTest` — the same reason `COLD_KEYS`/`SPLIT_KEYS` are production constants rather than test copies: two lists supposed to describe the same method are exactly the shape that silently drifts apart. Checked no test relied on the two copies being able to differ (grepped for other uses — none). Made `changedDeferredKeys` package-private to match `changedColdKeys`/`changedSplitKeys`, needed for the reflective test to call it directly. Invariants honored: `ConfigRefTest`'s existing per-key tests are untouched (added coverage, not a replacement); no reload behavior changed (test-only diff, plus one method's visibility and a set's home); `ConfigRefProfileCoverageTest` untouched. ### Mutation proof — the one-way direction that matters Dropped `guard`'s branch from `changedDeferredKeys` again, this time **with the fix in place**, and ran the new test: ``` ConfigRef.changedDeferredKeys reporting coverage — 11 DEFERRED_KEYS, 10 verified [ERROR] Tests run: 3, Failures: 1, Errors: 0, Skipped: 0 <<< FAILURE! -- in ConfigRefTopLevelReportingCoverageTest [ERROR] ConfigRefTopLevelReportingCoverageTest.everyDeferredKeyIsActuallyReportedByChangedDeferredKeys [ERROR] ...these keys are in ConfigRef.DEFERRED_KEYS but mutating them alone produces no matching entry from changedDeferredKeys...: [guard] ==> expected: <[]> but was: <[guard]> ``` `ConfigRefTest` stayed fully green through the same mutation — confirming `guard` really had no hand-written test catching it before this PR. Restored immediately after, verified with a fresh `mvn clean install`. ### Build `cd fleetd && mvn clean install`, unpiped, final run: ``` Tests run: 1356, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` (1356 = 1355 on `main` at `eee4d57` + 1 new test method.) ### Shape survey — not fixed, just reported (per brief) Other places in this repo treating a `Set` of names as coverage, found while working this ticket: - `FleetConfig.KNOWN_TOP_LEVEL_KEYS` (`FleetConfig.java:1494`) — used only to warn about unknown YAML keys; a companion test asserts every entry is documented in `fleetd.example.yaml`. Lower risk than `ConfigRef`'s sets since it's a warn-only list, not a "this key's reload works" claim. - `FleetConfig.FLEET_POOL_KEYS` (`FleetConfig.java:1527`) — the `fleet:` child block names (`leaders`/`architects`/`developers`/`reviewers`); did not check what consumes it or whether membership is proven behaviourally. - `ConfigRef.LAUNCH_SETTINGS_EXCLUDED` (already has full reflective coverage via `ConfigRefProfileCoverageTest` — not a gap, just the same general shape). Not investigated further, per scope. Ticket: fleetd #337
agent added 1 commit 2026-09-04 11:07:41 +02:00
#337: extend ConfigRefTopLevelReportingCoverageTest to DEFERRED_KEYS
CI / contract (pull_request) Successful in 1m23s
Details
CI / build (pull_request) Successful in 1m27s
Details
d703ce1313 
ConfigRefTopLevelReportingCoverageTest (added by #333) proved every COLD_KEYS
and SPLIT_KEYS member has a real comparison behind it, but left
DEFERRED_TOP_LEVEL_KEYS unexercised. Re-measured by mutation (drop each
key's branch from changedDeferredKeys, run the suite, restore): 6 of the 11
deferred keys had no behavioural test naming them — guard, leadHeartbeat,
worktreeRoot, spawnReadyTimeoutMs, spawnReadyPollMs, quarantineCooldownSeconds
— which corrects the issue's own guessed list in two ways: lifecycle is
actually covered (ConfigRefTest.aDeferredChangeIsAppliedAndReported), and
worktreeRoot was missing from the issue's list entirely.

Promoted the test-side DEFERRED_TOP_LEVEL_KEYS copy into ConfigRef.DEFERRED_KEYS
(package-private, alongside COLD_KEYS/SPLIT_KEYS) so the reflective test reads
the same set changedDeferredKeys is compared against, and made
changedDeferredKeys package-private so the test can call it directly. Every
DEFERRED_KEYS component turned out to be a scalar or a simple record, so no
exclusion set was needed.

Mutation proof: dropping guard's branch from changedDeferredKeys leaves the
whole suite green except the new
everyDeferredKeyIsActuallyReportedByChangedDeferredKeys test, which fails
naming guard exactly.
ltms closed this pull request 2026-09-04 11:19:50 +02:00
Some checks are pending
CI / contract (pull_request) Successful in 1m23s
Details
CI / build (pull_request) Successful in 1m27s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#347
Delete Branch "worker/fleetd-337-5ec7d4-21"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?