Features: #323 — the reload classifier proves its own coverage
+34
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user