FleetHealthMonitor.coverage has no test at all, and it feeds fleet_list's healthCoverage field #426

Closed
opened 2026-09-10 06:50:29 +02:00 by ltms · 1 comment
Owner

Sibling of #407, different set. #407 is about the five reportXxx(cfg) invocations in Fleetd.main. This is about the coverage() reporters — where the method, its call sites and its output string are all unpinned.

Spotted by the #415 worker as an out-of-scope note. I checked it here and it is worse than reported.

Measured

$ grep -rln "FleetHealthMonitor.coverage" fleetd/src/test
  (no output)
$ grep -rn "detection-only\|healthCoverage" fleetd/src/test
  (no output)

Zero references in the whole test tree — not the method, not either of its output strings, not the field name it populates.

The grep is capable of a hit — the same search pattern against the sibling method finds one, so the empty result is evidence and not a broken command:

$ grep -rln "CompletionResolver.coverage" fleetd/src/test
  fleetd/src/test/java/dev/ltms/fleet/inject/CompletionResolverTest.java

The method is three lines (FleetHealthMonitor.java:377):

public static String coverage(boolean enabled, boolean notificationConfigured) {
    return !enabled ? "off" : notificationConfigured ? "full" : "detection-only";
}

It has two production call sites:

  • Fleetd.java:595 — a startup log line.
  • Fleetd.java:677 — inside the HealthCoverageSource lambda, which is what fleet_list returns as its healthCoverage field. That is an operator-visible surface: a live fleet_list on this daemon reports "healthCoverage":"detection-only" right now.

So all three of these ship a green build:

  1. Inverting the enabled test.
  2. Swapping "full" and "detection-only".
  3. Deleting either call site.

Why it is worth a ticket rather than a shrug

Three arguments and a two-branch ternary is not where a bug hides on its own. What makes this worth pinning is the pair of lessons the last two days produced:

  • #415 — the wording of a coverage line was false for one key, for a month, because the method could not know what unset meant. The wording is the product here; it is the only thing an operator sees.
  • #404 / #425 — a status field that does not read what the behaviour reads. healthCoverage is exactly that shape of field, and it currently has no test tying its value to the health monitor's real state.

There is also a measured precedent that the arguments are the risky part, not the method. On PR #423 for #415 I swapped the two UnsetMeaning arguments at their call sites in Fleetd.java, which recreates #415's original defect with the two keys exchanged, and ran the full suite:

[INFO] Tests run: 1506, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS

Zero failures. The coverage() method was thoroughly tested in both directions and the pairing of key to meaning was not tested at all. FleetHealthMonitor.coverage has the same shape with neither half tested.

What is wanted

  • A test for the method's three outputs: off / detection-only / full.
  • A test that ties fleet_list's healthCoverage field to the health configuration that produced it — so inverting the arguments at Fleetd.java:677 fails. This is the half that matters; the three-branch test is the easy one.
  • Read #407 before choosing a technique for the call-site half. It works through the same problem for the five reporters and lands on two options, preferring the one that adds no production code. If your answer here needs a seam, say why option 1 there does not apply.

Acceptance

  • Inverting enabled, or swapping "full" with "detection-only", fails a test.
  • Breaking the argument pairing at Fleetd.java:677 fails a test.
  • Report each mutation separately with the exact failing test name and assertion. If any one of them turns out to be pinned already by something my greps missed, say so plainly — that is a useful result and I would rather hear it than have it worked around.
  • No socket, no port bind, no spawn, and nothing written outside a @TempDir.

Out of scope

  • The five reportXxx invocations are #407. Do not fix them here.
  • CompletionResolver.coverage's call sites are being pinned on PR #423 for #415. Do not touch that file.
  • Do not change the three output strings. detection-only in particular is already reported by a live fleet_list and read by leads.
Sibling of #407, different set. #407 is about the five `reportXxx(cfg)` **invocations** in `Fleetd.main`. This is about the `coverage()` reporters — where the method, its call sites and its output string are all unpinned. Spotted by the #415 worker as an out-of-scope note. I checked it here and it is worse than reported. ## Measured ``` $ grep -rln "FleetHealthMonitor.coverage" fleetd/src/test (no output) $ grep -rn "detection-only\|healthCoverage" fleetd/src/test (no output) ``` Zero references in the whole test tree — not the method, not either of its output strings, not the field name it populates. **The grep is capable of a hit** — the same search pattern against the sibling method finds one, so the empty result is evidence and not a broken command: ``` $ grep -rln "CompletionResolver.coverage" fleetd/src/test fleetd/src/test/java/dev/ltms/fleet/inject/CompletionResolverTest.java ``` The method is three lines (`FleetHealthMonitor.java:377`): ```java public static String coverage(boolean enabled, boolean notificationConfigured) { return !enabled ? "off" : notificationConfigured ? "full" : "detection-only"; } ``` It has two production call sites: - `Fleetd.java:595` — a startup log line. - `Fleetd.java:677` — inside the `HealthCoverageSource` lambda, which is what `fleet_list` returns as its **`healthCoverage`** field. That is an operator-visible surface: a live `fleet_list` on this daemon reports `"healthCoverage":"detection-only"` right now. So all three of these ship a green build: 1. Inverting the `enabled` test. 2. Swapping `"full"` and `"detection-only"`. 3. Deleting either call site. ## Why it is worth a ticket rather than a shrug Three arguments and a two-branch ternary is not where a bug hides on its own. What makes this worth pinning is the pair of lessons the last two days produced: - **#415** — the *wording* of a coverage line was false for one key, for a month, because the method could not know what unset meant. The wording is the product here; it is the only thing an operator sees. - **#404 / #425** — a status field that does not read what the behaviour reads. `healthCoverage` is exactly that shape of field, and it currently has no test tying its value to the health monitor's real state. There is also a measured precedent that the *arguments* are the risky part, not the method. On PR #423 for #415 I swapped the two `UnsetMeaning` arguments at their call sites in `Fleetd.java`, which recreates #415's original defect with the two keys exchanged, and ran the full suite: ``` [INFO] Tests run: 1506, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS ``` Zero failures. The `coverage()` method was thoroughly tested in both directions and the pairing of key to meaning was not tested at all. `FleetHealthMonitor.coverage` has the same shape with **neither** half tested. ## What is wanted - A test for the method's three outputs: off / detection-only / full. - A test that ties `fleet_list`'s `healthCoverage` field to the health configuration that produced it — so inverting the arguments at `Fleetd.java:677` fails. This is the half that matters; the three-branch test is the easy one. - Read #407 before choosing a technique for the call-site half. It works through the same problem for the five reporters and lands on two options, preferring the one that adds no production code. If your answer here needs a seam, say why option 1 there does not apply. ## Acceptance - Inverting `enabled`, or swapping `"full"` with `"detection-only"`, fails a test. - Breaking the argument pairing at `Fleetd.java:677` fails a test. - Report each mutation separately with the exact failing test name and assertion. If any one of them turns out to be pinned already by something my greps missed, say so plainly — that is a useful result and I would rather hear it than have it worked around. - No socket, no port bind, no spawn, and nothing written outside a `@TempDir`. ## Out of scope - The five `reportXxx` invocations are #407. Do not fix them here. - `CompletionResolver.coverage`'s call sites are being pinned on PR #423 for #415. Do not touch that file. - Do not change the three output strings. `detection-only` in particular is already reported by a live `fleet_list` and read by leads.
Author
Owner

Closed by PR #542, merged after I verified it myself on the merged tree.

What I ran, and what it said

Check Result
mvn -B clean install on the merged tree exit 0 — Tests run: 1712, Failures: 0, Errors: 0, Skipped: 0
#459's javadoc reference gate on the merged tree exit 0, 0 reference errors

1712 = 1704 on cec3e19 + the 8 new tests. The PR body says 1701 → 1709; that is not a
disagreement. The branch base is f1640f5, which predates #537 (+3) and #459's merge.

Mutations — I re-applied two of them myself

A proof is about a revision, not a file, so I did not reuse the worker's runs.

Argument pairing swap at the HealthCoverageSource call site — exit 1, 1712 run, 3 failures:

  • disabledHealthReportsOff — expected: <off> but was: <detection-only>
  • enabledWithoutNotificationsReportsDetectionOnly
  • reloadedNotificationsStillChangeWhatFleetListReports

"full" / "detection-only" swap inside FleetHealthMonitor.coverage — exit 1, 5 failures.

Both killed. A kill is its own harness proof, so neither needed a separate false-assertion run.
Both restores were checked with shasum -a 256 against my own pristine copies, and both of my
shas matched the worker's independently: 939034d9… for Fleetd.java, 40e29b66… for
FleetHealthMonitor.java. The proof cell was run twice (against git show HEAD and against the
working tree) so a cell that reports "applied" in both states would have been visible. Green
control after restore: exit 0, 1712/0/0/0, clean git status.

The new production code

Fleetd.healthCoverageSource(ConfigRef) is a pure extraction of the inline lambda. The three
output strings at FleetHealthMonitor.java:378 are untouched.

I checked the precedent in the code rather than taking the PR's word: quarantineSource
(Fleetd.java:909) and capacitySource (Fleetd.java:1031) are both package-private static
factories, and both carry #415 javadoc about the same argument-pairing defect. So this follows an
established shape here, it does not invent one.

The worker also explained honestly why 5 of 8 tests stayed green under the pairing mutation: those
fixtures use symmetric booleans, where swapping two equal values gives the same answer. The
mixed-boolean fixtures are what actually exercise the pairing, and they caught it.

Not closed by this

The startup log line — the other coverage(...) call site, around Fleetd.java:619-625 — still
has no pairing test. The worker found it, flagged it, and left it out of scope. That was the right
call for this ticket, but it means the same defect shape is still unpinned at that one site.

Closed by PR #542, merged after I verified it myself on the merged tree. ## What I ran, and what it said | Check | Result | |---|---| | `mvn -B clean install` on the merged tree | exit 0 — `Tests run: 1712, Failures: 0, Errors: 0, Skipped: 0` | | #459's javadoc reference gate on the merged tree | exit 0, 0 reference errors | 1712 = 1704 on `cec3e19` + the 8 new tests. The PR body says 1701 → 1709; that is not a disagreement. The branch base is `f1640f5`, which predates #537 (+3) and #459's merge. ## Mutations — I re-applied two of them myself A proof is about a revision, not a file, so I did not reuse the worker's runs. **Argument pairing swap at the `HealthCoverageSource` call site** — exit 1, 1712 run, 3 failures: - `disabledHealthReportsOff` — `expected: <off> but was: <detection-only>` - `enabledWithoutNotificationsReportsDetectionOnly` - `reloadedNotificationsStillChangeWhatFleetListReports` **`"full"` / `"detection-only"` swap inside `FleetHealthMonitor.coverage`** — exit 1, 5 failures. Both killed. A kill is its own harness proof, so neither needed a separate false-assertion run. Both restores were checked with `shasum -a 256` against my own pristine copies, and both of my shas matched the worker's independently: `939034d9…` for `Fleetd.java`, `40e29b66…` for `FleetHealthMonitor.java`. The proof cell was run twice (against `git show HEAD` and against the working tree) so a cell that reports "applied" in both states would have been visible. Green control after restore: exit 0, `1712/0/0/0`, clean `git status`. ## The new production code `Fleetd.healthCoverageSource(ConfigRef)` is a pure extraction of the inline lambda. The three output strings at `FleetHealthMonitor.java:378` are untouched. I checked the precedent in the code rather than taking the PR's word: `quarantineSource` (`Fleetd.java:909`) and `capacitySource` (`Fleetd.java:1031`) are both package-private static factories, and both carry #415 javadoc about the same argument-pairing defect. So this follows an established shape here, it does not invent one. The worker also explained honestly why 5 of 8 tests stayed green under the pairing mutation: those fixtures use symmetric booleans, where swapping two equal values gives the same answer. The mixed-boolean fixtures are what actually exercise the pairing, and they caught it. ## Not closed by this The **startup log line** — the other `coverage(...)` call site, around `Fleetd.java:619-625` — still has no pairing test. The worker found it, flagged it, and left it out of scope. That was the right call for this ticket, but it means the same defect shape is still unpinned at that one site.
ltms closed this issue 2026-09-12 08:56:09 +02:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#426