From d703ce1313a527d0d835acb5dbca42e603c6ebb5 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Fri, 4 Sep 2026 16:06:09 +0700 Subject: [PATCH] #337: extend ConfigRefTopLevelReportingCoverageTest to DEFERRED_KEYS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../java/dev/ltms/fleet/config/ConfigRef.java | 42 +++- .../config/ConfigRefTopLevelCoverageTest.java | 28 +-- ...onfigRefTopLevelReportingCoverageTest.java | 185 ++++++++++++------ 3 files changed, 175 insertions(+), 80 deletions(-) diff --git a/fleetd/src/main/java/dev/ltms/fleet/config/ConfigRef.java b/fleetd/src/main/java/dev/ltms/fleet/config/ConfigRef.java index eab99c8..1c37bd1 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/config/ConfigRef.java +++ b/fleetd/src/main/java/dev/ltms/fleet/config/ConfigRef.java @@ -154,10 +154,13 @@ import java.util.function.Supplier; * {@link ConfigRefProfileCoverageTest} shape (one level up, over {@code FleetConfig} itself rather * than {@code FleetConfig.Profile}) proves this file's four classes exhaust the record's components * — see {@code ConfigRefTopLevelCoverageTest}. That test proves the record's shape is fully - * triaged; it does NOT prove a {@code SPLIT_KEYS}/{@code COLD_KEYS} member has any reporting code - * behind it at all — {@code ConfigRefTopLevelReportingCoverageTest} is what fleetd #333 added for - * that, after measuring that a {@code SPLIT_KEYS} entry with its reporting branch deleted passes - * both this file's own "kept in step" assert and {@code ConfigRefTopLevelCoverageTest} unchanged. + * triaged; it does NOT prove a {@code SPLIT_KEYS}/{@code COLD_KEYS}/{@code DEFERRED_KEYS} member has + * any reporting code behind it at all — {@code ConfigRefTopLevelReportingCoverageTest} is what + * fleetd #333 added for that, after measuring that a {@code SPLIT_KEYS} entry with its reporting + * branch deleted passes both this file's own "kept in step" assert and + * {@code ConfigRefTopLevelCoverageTest} unchanged. fleetd #337 extended it to {@code DEFERRED_KEYS} + * after measuring the same one-way gap there directly: dropping {@code guard}'s branch out of + * {@link #changedDeferredKeys} while {@code "guard"} stayed in the set left the whole suite green. * *

A cold change refuses the whole reload. Not the hot half applied and the cold * half warned about: that would leave the running daemon in a state matching no file on disk, which @@ -193,6 +196,25 @@ public final class ConfigRef implements Supplier { */ static final Set SPLIT_KEYS = Set.of("health", "coordinator", "fleet"); + /** + * Top-level keys {@link #changedDeferredKeys} compares — see the class doc's Deferred bullet. + * Promoted here from a test-side copy in {@code ConfigRefTopLevelCoverageTest} by fleetd #337, + * the same reason {@link #COLD_KEYS} and {@link #SPLIT_KEYS} live here rather than in a test: a + * second, hand-maintained copy of this set is exactly the kind of thing that silently drifts + * from the method it is supposed to describe. {@code spawnReadyTimeoutMs} and + * {@code spawnReadyPollMs} are compared together in one branch and reported under the combined + * label {@code "spawnReady*"}; {@code profiles} is compared twice over (added/removed names, + * then an existing profile's launch settings) — see {@link #changedDeferredKeys}. + * + *

Package-private (not {@code private}) so {@code ConfigRefTopLevelCoverageTest} and + * {@code ConfigRefTopLevelReportingCoverageTest} can both read it, the same way they already + * read {@link #COLD_KEYS} and {@link #SPLIT_KEYS}. + */ + static final Set DEFERRED_KEYS = Set.of( + "guard", "worktreeRoot", "worktreeGroup", "primary", "configReload", + "leadHeartbeat", "lifecycle", "spawnReadyTimeoutMs", "spawnReadyPollMs", + "quarantineCooldownSeconds", "profiles"); + private final Path path; private final AtomicReference current; @@ -350,8 +372,16 @@ public final class ConfigRef implements Supplier { return changed; } - /** Changed keys that were accepted but whose effect waits for a restart. */ - private static List changedDeferredKeys(FleetConfig old, FleetConfig fresh) { + /** + * Changed keys that were accepted but whose effect waits for a restart. + * + *

Package-private (not {@code private}) so {@code ConfigRefTopLevelReportingCoverageTest} + * can call it directly with a reflection-built {@code FleetConfig} pair, the same reason + * {@link #changedColdKeys} and {@link #changedSplitKeys} already are (fleetd #333, extended to + * this method by fleetd #337 — membership in {@link #DEFERRED_KEYS} proved nothing about this + * method on its own until then; see that test's class doc). + */ + static List changedDeferredKeys(FleetConfig old, FleetConfig fresh) { List changed = new ArrayList<>(); if (!Objects.equals(old.lifecycle(), fresh.lifecycle())) { changed.add("lifecycle"); diff --git a/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTopLevelCoverageTest.java b/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTopLevelCoverageTest.java index 90525fb..b89856a 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTopLevelCoverageTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTopLevelCoverageTest.java @@ -47,21 +47,21 @@ class ConfigRefTopLevelCoverageTest { private static final RecordComponent[] COMPONENTS = FleetConfig.class.getRecordComponents(); /** - * Top-level components whose change {@code ConfigRef.changedDeferredKeys} reads and reports on - * — verified by reading that method as of fleetd #330, not derived from this test. - * {@code spawnReadyTimeoutMs}/{@code spawnReadyPollMs} are compared together and reported under - * one combined label ({@code "spawnReady*"}); {@code profiles} is compared twice over — once for - * added/removed profile names, once for an existing profile's launch settings — and that second - * comparison excludes {@code weight}/{@code maxLoad}/{@code credentialId} as hot sub-fields, - * which is what {@link ConfigRefProfileCoverageTest} exists to keep honest at the sub-field - * level. {@code profiles} itself still belongs here, not in the hot-exclusion set below: most of - * a profile's fields are NOT read live, so citing "read live off the config supplier" for the - * whole top-level key would be false. + * Top-level components whose change {@code ConfigRef.changedDeferredKeys} reads and reports on. + * Fleetd #337 promoted this out of a hand-maintained copy here into {@link ConfigRef#DEFERRED_KEYS} + * itself, the same reason {@link ConfigRef#COLD_KEYS} and {@link ConfigRef#SPLIT_KEYS} are + * production constants rather than test-side copies: two lists that are supposed to describe the + * same method are exactly the shape that silently drifts apart. {@code spawnReadyTimeoutMs}/ + * {@code spawnReadyPollMs} are compared together and reported under one combined label + * ({@code "spawnReady*"}); {@code profiles} is compared twice over — once for added/removed + * profile names, once for an existing profile's launch settings — and that second comparison + * excludes {@code weight}/{@code maxLoad}/{@code credentialId} as hot sub-fields, which is what + * {@link ConfigRefProfileCoverageTest} exists to keep honest at the sub-field level. {@code + * profiles} itself still belongs here, not in the hot-exclusion set below: most of a profile's + * fields are NOT read live, so citing "read live off the config supplier" for the whole + * top-level key would be false. */ - private static final Set DEFERRED_TOP_LEVEL_KEYS = Set.of( - "guard", "worktreeRoot", "worktreeGroup", "primary", "configReload", - "leadHeartbeat", "lifecycle", "spawnReadyTimeoutMs", "spawnReadyPollMs", - "quarantineCooldownSeconds", "profiles"); + private static final Set DEFERRED_TOP_LEVEL_KEYS = ConfigRef.DEFERRED_KEYS; /** * The escape hatch: top-level components with no reload bookkeeping at all, because every read diff --git a/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTopLevelReportingCoverageTest.java b/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTopLevelReportingCoverageTest.java index 00b77a5..aea7bba 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTopLevelReportingCoverageTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTopLevelReportingCoverageTest.java @@ -16,61 +16,68 @@ import static org.junit.jupiter.api.Assertions.assertEquals; /** * fleetd #333, finding F2: {@link ConfigRefTopLevelCoverageTest} proves every {@link FleetConfig} - * top-level component sits in exactly one of {@link ConfigRef#COLD_KEYS}, {@code - * DEFERRED_TOP_LEVEL_KEYS}, {@link ConfigRef#SPLIT_KEYS} or the hot-excluded set. It does - * not prove that a key's membership in {@code COLD_KEYS} or {@code SPLIT_KEYS} - * corresponds to any actual comparison in {@link ConfigRef}: a key can sit in either set with no - * branch in {@code changedColdKeys}/{@code changedSplitKeys} checking it, and both - * {@link ConfigRefTopLevelCoverageTest} and the "kept in step" {@code assert} inside each of those - * methods stay green, because neither one reads the method body — the coverage test only reads - * set membership, and the assert only checks that reported entries are a SUBSET of the set, never - * that every set member produced a reported entry. + * top-level component sits in exactly one of {@link ConfigRef#COLD_KEYS}, {@link + * ConfigRef#DEFERRED_KEYS}, {@link ConfigRef#SPLIT_KEYS} or the hot-excluded set. It does + * not prove that a key's membership in one of the first three sets corresponds to + * any actual comparison in {@link ConfigRef}: a key can sit in a set with no branch in {@code + * changedColdKeys}/{@code changedSplitKeys}/{@code changedDeferredKeys} checking it, and both + * {@link ConfigRefTopLevelCoverageTest} and the "kept in step" {@code assert} inside the first two + * of those methods stay green, because neither one reads the method body — the coverage test only + * reads set membership, and the assert only checks that reported entries are a SUBSET of the set, + * never that every set member produced a reported entry. {@code changedDeferredKeys} does not even + * have a "kept in step" assert of its own. * *

Measured directly, live, while fixing fleetd #333: dropping the {@code coordinator} branch out * of {@code ConfigRef.changedSplitKeys} while leaving {@code "coordinator"} in * {@link ConfigRef#SPLIT_KEYS} left {@link ConfigRefTopLevelCoverageTest} and the in-method assert * both green — only a hand-written behavioural case in {@link ConfigRefTest} caught it, because it * happened to name that exact key. This is the {@link ConfigRefProfileCoverageTest} mechanism one - * level up, generalised over every {@code COLD_KEYS}/{@code SPLIT_KEYS} member rather than one - * hand-picked field: enumerate {@link FleetConfig}'s own record components by reflection, build "a - * config where only {@code } differs" for each cold/split key, and call the real - * {@link ConfigRef#changedColdKeys}/{@link ConfigRef#changedSplitKeys} methods (made - * package-private for exactly this, the same reason {@link ConfigRef#sameLaunchSettings} already - * is) to prove each one is actually reported — not assumed from a set literal. + * level up, generalised over every {@code COLD_KEYS}/{@code SPLIT_KEYS}/{@code DEFERRED_KEYS} + * member rather than one hand-picked field: enumerate {@link FleetConfig}'s own record components by + * reflection, build "a config where only {@code } differs" for each key, and call the real + * {@link ConfigRef#changedColdKeys}/{@link ConfigRef#changedSplitKeys}/ + * {@link ConfigRef#changedDeferredKeys} methods (all package-private for exactly this, the same + * reason {@link ConfigRef#sameLaunchSettings} already is) to prove each one is actually reported — + * not assumed from a set literal. * - *

What this deliberately does NOT cover

- * {@code DEFERRED_TOP_LEVEL_KEYS} is not exercised here. That bucket carries the identical - * one-way risk in principle — a key added to it with no matching branch in - * {@code changedDeferredKeys} would pass {@link ConfigRefTopLevelCoverageTest} exactly the way - * {@code coordinator} passed it above — but this test stays narrow to {@code COLD_KEYS} and - * {@code SPLIT_KEYS} for two reasons. First, that is where fleetd #333 actually found and measured - * the gap (F1 was a live instance of it). Second, most of {@code DEFERRED_TOP_LEVEL_KEYS} already - * carries an individual behavioural test in {@link ConfigRefTest} naming it by key — {@code - * worktreeGroup}, {@code primary}, {@code configReload}, {@code profiles}' launch settings / - * weight-maxLoad / exhaustedPattern / errorPattern / ideProjectDir — which is the same protection - * this class gives {@code COLD_KEYS}/{@code SPLIT_KEYS}, just written by hand per key instead of - * generated by reflection over the whole set. {@code guard}, {@code lifecycle}, - * {@code leadHeartbeat}, {@code spawnReadyTimeoutMs}/{@code spawnReadyPollMs} and - * {@code quarantineCooldownSeconds} do NOT have a dedicated behavioural test naming them, so the - * one-way gap fleetd's own memory notes ("pre-existing on COLD_KEYS and on the test's own - * DEFERRED_TOP_LEVEL_KEYS") is real and not fully closed by this class — extending this mechanism's - * {@code BASE}/{@code ALT} map to cover every top-level component and adding a third - * {@code everyDeferredKeyIsActuallyReportedByChangedDeferredKeys} test is the natural next step, left - * for whoever next finds a deferred key with the same shape as this ticket's {@code fleet.leaders}. + *

fleetd #337 — DEFERRED_KEYS was the gap left open here

+ * This class originally covered {@code COLD_KEYS} and {@code SPLIT_KEYS} only — {@code + * DEFERRED_TOP_LEVEL_KEYS} (now {@link ConfigRef#DEFERRED_KEYS}) carried the identical one-way risk + * in principle, unexercised. fleetd #337 measured the real consequence rather than assuming it from + * the shape of the gap: dropping {@code guard}'s comparison out of {@code changedDeferredKeys} while + * {@code "guard"} stayed in the set left all 1355 tests green — the same failure mode {@code + * coordinator} demonstrated for {@code SPLIT_KEYS} in fleetd #333, now confirmed for {@code + * DEFERRED_KEYS} too. Re-deriving the full list by mutation (drop each key's branch in turn, run the + * suite, restore) found six of the eleven {@code DEFERRED_KEYS} members with no behavioural test in + * {@link ConfigRefTest} naming them: {@code guard}, {@code leadHeartbeat}, {@code worktreeRoot}, + * {@code spawnReadyTimeoutMs}, {@code spawnReadyPollMs} and {@code quarantineCooldownSeconds}. That + * list corrects fleetd #333's own guess at it in two ways the mutation proved and a reading did not: + * {@code lifecycle} is NOT on it — {@code ConfigRefTest.aDeferredChangeIsAppliedAndReported} already + * names it, and dropping its branch fails that test — and {@code worktreeRoot} IS on it, which #333 + * never named at all. The other five {@code DEFERRED_KEYS} members ({@code lifecycle}, {@code + * worktreeGroup}, {@code primary}, {@code configReload}, {@code profiles}) already had a hand-written + * case each. {@link #everyDeferredKeyIsActuallyReportedByChangedDeferredKeys} below now covers all + * eleven the reflective way, so the six with no hand-written test are no longer silently unpinned — + * every {@code DEFERRED_KEYS} component turned out to be a scalar or a simple record, so, unlike + * {@code fleet.leaders} in fleetd #333, none needed an exclusion set: {@link #BASE}/{@link #ALT} give + * every top-level component (not only {@code COLD_KEYS}/{@code SPLIT_KEYS}) a real, distinct value. */ class ConfigRefTopLevelReportingCoverageTest { private static final RecordComponent[] COMPONENTS = FleetConfig.class.getRecordComponents(); /** - * One valid value per top-level {@link FleetConfig} component — "the a value". Components not - * exercised by either test below ({@code profiles}, {@code guard}, {@code worktreeRoot}, …) are - * left {@code null}/empty; {@link FleetConfig}'s compact constructor only normalizes - * {@code profiles}, so every other field accepts {@code null} unmutated. + * One valid value per top-level {@link FleetConfig} component — "the a value". fleetd #337 gave + * every {@code DEFERRED_KEYS} component a real value here too (previously left {@code null} on + * both sides, which meant {@code mutate(key)} produced no actual difference for any of them); + * only {@code placement}, {@code memberCredentials} and {@code memberLoginShell} — the + * hot-excluded set, never compared by any {@code changed*Keys} method — stay {@code null}. + * {@link FleetConfig}'s compact constructor only normalizes {@code profiles}, so every other + * field accepts whatever is put here unmutated. */ private static final Map BASE = baseValues(); - /** The same shape, each value distinct from {@link #BASE} — "the b value" — for COLD_KEYS/SPLIT_KEYS only. */ + /** The same shape, each value distinct from {@link #BASE} — "the b value". */ private static final Map ALT = altValues(); private static Map baseValues() { @@ -79,26 +86,26 @@ class ConfigRefTopLevelReportingCoverageTest { v.put("herdrSocket", "~/.config/herdr/a.sock"); v.put("memberHerdrSocket", "~/.config/herdr/member-a.sock"); v.put("profiles", Map.of()); - v.put("guard", null); - v.put("worktreeRoot", null); - v.put("lifecycle", null); - v.put("spawnReadyTimeoutMs", null); - v.put("spawnReadyPollMs", null); + v.put("guard", new FleetConfig.Guard(List.of("host-a"))); + v.put("worktreeRoot", "/wt/a"); + v.put("lifecycle", new FleetConfig.Lifecycle(300, 5, 30, false)); + v.put("spawnReadyTimeoutMs", 5000); + v.put("spawnReadyPollMs", 100); v.put("broker", new FleetConfig.Broker("amqp://a", null, 1)); - v.put("primary", null); + v.put("primary", new FleetConfig.Primary("term-a", 1, 1000)); v.put("fleet", new FleetConfig.Fleet( Map.of("opus", new FleetConfig.Leader("sonnet", "lead: opus-a", 1, null, 10, "claude", null, null, null)), Map.of(), Map.of(), Map.of(), Map.of(), "{role}: {profile} #{n}")); - v.put("leadHeartbeat", null); + v.put("leadHeartbeat", new FleetConfig.LeadHeartbeat(300, 60_000L, 3)); v.put("health", new FleetConfig.Health(true, 30, 600, null, null)); v.put("placement", null); v.put("auth", new FleetConfig.Auth("loopback-trust", null)); - v.put("configReload", null); - v.put("quarantineCooldownSeconds", null); + v.put("configReload", new FleetConfig.ConfigReload(true, 10)); + v.put("quarantineCooldownSeconds", 1800); v.put("memberCredentials", null); v.put("coordinator", new FleetConfig.Coordinator("amqp://coord-a", null, "self-a", 1)); - v.put("worktreeGroup", null); + v.put("worktreeGroup", "group-a"); v.put("memberLoginShell", null); assertNamesMatchComponents(v); return v; @@ -109,14 +116,18 @@ class ConfigRefTopLevelReportingCoverageTest { v.put("bind", new FleetConfig.Bind("127.0.0.2", 8766)); v.put("herdrSocket", "~/.config/herdr/b.sock"); v.put("memberHerdrSocket", "~/.config/herdr/member-b.sock"); - v.put("profiles", Map.of()); - v.put("guard", null); - v.put("worktreeRoot", null); - v.put("lifecycle", null); - v.put("spawnReadyTimeoutMs", null); - v.put("spawnReadyPollMs", null); + // A single added profile — enough to trip the "added/removed" comparison in + // ConfigRef.changedDeferredKeys, which is all this mechanism needs to prove "profiles" has + // a branch behind it; the launch-settings comparison already has its own hand-written cases + // in ConfigRefTest (changingAProfilesLaunchSettingsIsReportedAsDeferred and siblings). + v.put("profiles", Map.of("sonnet", minimalProfile("sonnet"))); + v.put("guard", new FleetConfig.Guard(List.of("host-b"))); + v.put("worktreeRoot", "/wt/b"); + v.put("lifecycle", new FleetConfig.Lifecycle(600, 10, 60, true)); + v.put("spawnReadyTimeoutMs", 10_000); + v.put("spawnReadyPollMs", 200); v.put("broker", new FleetConfig.Broker("amqp://b", null, 2)); - v.put("primary", null); + v.put("primary", new FleetConfig.Primary("term-b", 2, 2000)); // Differs from BASE.fleet only in fleet.leaders (a different tab for "opus") — the frozen // sub-field ConfigRef.changedSplitKeys actually compares. A Fleet that instead differed only // in tabLabel would correctly NOT be reported (see @@ -126,20 +137,27 @@ class ConfigRefTopLevelReportingCoverageTest { Map.of("opus", new FleetConfig.Leader("sonnet", "lead: opus-b", 1, null, 10, "claude", null, null, null)), Map.of(), Map.of(), Map.of(), Map.of(), "{role}: {profile} #{n}")); - v.put("leadHeartbeat", null); + v.put("leadHeartbeat", new FleetConfig.LeadHeartbeat(600, 120_000L, 5)); v.put("health", new FleetConfig.Health(false, 90, 900, null, null)); v.put("placement", null); v.put("auth", new FleetConfig.Auth("token", "TOKEN_ENV")); - v.put("configReload", null); - v.put("quarantineCooldownSeconds", null); + v.put("configReload", new FleetConfig.ConfigReload(false, 20)); + v.put("quarantineCooldownSeconds", 3600); v.put("memberCredentials", null); v.put("coordinator", new FleetConfig.Coordinator("amqp://coord-b", null, "self-b", 2)); - v.put("worktreeGroup", null); + v.put("worktreeGroup", "group-b"); v.put("memberLoginShell", null); assertNamesMatchComponents(v); return v; } + /** A minimal, otherwise-null {@link FleetConfig.Profile} — just enough to name one in a map. */ + private static FleetConfig.Profile minimalProfile(String name) { + return new FleetConfig.Profile(name, null, null, null, null, null, null, null, null, null, + null, null, null, null, null, null, null, null, null, null, null, null, null, null, + null, null); + } + private static void assertNamesMatchComponents(Map values) { Set names = new TreeSet<>(); for (RecordComponent rc : COMPONENTS) { @@ -216,4 +234,51 @@ class ConfigRefTopLevelReportingCoverageTest { + "entry from changedSplitKeys — a set entry with no comparison behind it, " + "exactly the fleetd #333 F2 shape: " + uncovered); } + + /** + * For most {@link ConfigRef#DEFERRED_KEYS} members, {@code changedDeferredKeys} reports the key + * name verbatim — the default this map assumes. Two entries don't: {@code spawnReadyTimeoutMs} + * and {@code spawnReadyPollMs} are compared together in one branch and reported under the + * combined label {@code "spawnReady*"} (see {@link ConfigRef#changedDeferredKeys}). {@code + * profiles} keeps the default: mutating it here only exercises the added/removed comparison + * (see {@link #altValues}), which reports {@code "profiles (added/removed: …)"} — starts with + * {@code "profiles"}, same as the default would expect. + */ + private static final Map DEFERRED_REPORT_PREFIX = Map.of( + "spawnReadyTimeoutMs", "spawnReady*", + "spawnReadyPollMs", "spawnReady*"); + + /** + * fleetd #337: the same mechanism applied to {@link ConfigRef#DEFERRED_KEYS}, closing the gap + * this class's own javadoc left open since fleetd #333. Mutate each deferred key in isolation + * and prove {@code changedDeferredKeys} actually names it (message starting with the key's + * expected report prefix — see {@link #DEFERRED_REPORT_PREFIX}), not just that {@code + * DEFERRED_KEYS} claims it does. This is the exact check that fails for {@code guard} the way + * {@code coordinator} failed {@link #everySplitKeyIsActuallyReportedByChangedSplitKeys} in + * fleetd #333 — verified live: dropping {@code guard}'s branch from {@code changedDeferredKeys} + * while {@code "guard"} stayed in {@code DEFERRED_KEYS} left the whole 1355-test suite green, + * and this test is what now catches it (it fails naming {@code guard} with that mutation in + * place). + */ + @Test + void everyDeferredKeyIsActuallyReportedByChangedDeferredKeys() throws ReflectiveOperationException { + FleetConfig base = configOf(BASE); + List uncovered = new ArrayList<>(); + for (String key : new TreeSet<>(ConfigRef.DEFERRED_KEYS)) { + FleetConfig mutated = mutate(key); + List deferred = ConfigRef.changedDeferredKeys(base, mutated); + String prefix = DEFERRED_REPORT_PREFIX.getOrDefault(key, key); + if (deferred.stream().noneMatch(s -> s.startsWith(prefix))) { + uncovered.add(key); + } + } + System.out.printf( + "ConfigRef.changedDeferredKeys reporting coverage — %d DEFERRED_KEYS, %d verified%n", + ConfigRef.DEFERRED_KEYS.size(), ConfigRef.DEFERRED_KEYS.size() - uncovered.size()); + assertEquals(List.of(), uncovered, + "these keys are in ConfigRef.DEFERRED_KEYS but mutating them alone produces no " + + "matching entry from changedDeferredKeys — a set entry with no comparison " + + "behind it, exactly the fleetd #333 F2 shape, confirmed here for " + + "DEFERRED_KEYS by fleetd #337: " + uncovered); + } }