From 92bc455b1b8e07c10e422a56f1def97bc98d80d9 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Fri, 4 Sep 2026 14:34:47 +0700 Subject: [PATCH] =?UTF-8?q?Features:=20#323=20=E2=80=94=20the=20reload=20c?= =?UTF-8?q?lassifier=20proves=20its=20own=20coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- 11-Features.md | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/11-Features.md b/11-Features.md index 4389560..1ff51e6 100644 --- a/11-Features.md +++ b/11-Features.md @@ -3940,3 +3940,37 @@ shared mutable map instance, so every read site must compare it by reference bef `peek`, `ack` and `deliverCallback` all do, and `own` clears a stale tombstone with the two-argument `remove` so it can never delete a real map. Both halves of the fix are separately load-bearing — reverting either one alone fails `AmqpReplyInboxReleaseRaceTest`. + +## The reload classifier proves its own coverage instead of claiming it + +**What.** A test enumerates every record component of a worker profile by reflection, changes each +one in turn, and asserts the reload classifier actually notices. A component that is neither +compared nor on a small, pinned exclusion list fails the build by name. The test prints its own +denominator on every run: + +``` +ConfigRef.sameLaunchSettings coverage — 26 Profile components total, 23 compared, 3 excluded ([weight, maxLoad, credentialId]) +``` + +**On.** Always on — it is a unit test, so it runs in every build. + +**Why it exists.** `sameLaunchSettings` decides whether a changed profile key is reported to the +operator as *deferred* (needs a restart). Its javadoc said it "compares every component the launcher +reads at spawn". It did not: `ideProjectDir`, `ideOpenCommand` and `autoCompactWindow` were all +missing, and `worktreeGroup` was missing from the sibling top-level check. A reload of any of them +returned `applied = true` with an empty deferred list — a bare `config reloaded` — while the daemon +kept the old value. The surrounding code already calls that "the worst outcome a reload can produce, +because the operator has no reason to doubt it". The list was a hand-maintained second copy of "what +the launcher reads at spawn", and it drifted. On its first run the new test found a fifth gap nobody +had reported: the `profile` component itself. + +**One thing to know for maintenance.** The exclusion list is this mechanism's own escape hatch, and +it is pinned for a reason. Measured while verifying the fix: moving `autoCompactWindow` and +`ideOpenCommand` *out* of the comparison and *into* the exclusion set left the whole suite green — +the loop simply skipped them and the denominator still balanced. That is the cheapest way to silence +a failing coverage test, and it silently restores the original bug. The test now asserts the +exclusion set equals exactly `{weight, maxLoad, credentialId}`, so growing it takes a visible, +deliberate edit. A component belongs there only if it is read live off the config supplier, never +because adding it makes the build pass. The general rule: when a ticket asks for a checker rather +than a fix, mutate the checker too, and ask what the cheapest way to pass it without doing the work +would be.