|
|
|
@@ -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
|
|
|
|
|
* <strong>not</strong> 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
|
|
|
|
|
* <strong>not</strong> 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.
|
|
|
|
|
*
|
|
|
|
|
* <p>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 <key>} 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 <key>} 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.
|
|
|
|
|
*
|
|
|
|
|
* <h2>What this deliberately does NOT cover</h2>
|
|
|
|
|
* {@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}.
|
|
|
|
|
* <h2>fleetd #337 — DEFERRED_KEYS was the gap left open here</h2>
|
|
|
|
|
* 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<String, Object> 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<String, Object> ALT = altValues();
|
|
|
|
|
|
|
|
|
|
private static Map<String, Object> 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<String, Object> values) {
|
|
|
|
|
Set<String> 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<String, String> 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<String> uncovered = new ArrayList<>();
|
|
|
|
|
for (String key : new TreeSet<>(ConfigRef.DEFERRED_KEYS)) {
|
|
|
|
|
FleetConfig mutated = mutate(key);
|
|
|
|
|
List<String> 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);
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
Dai Ha