phase: 12_nut_self_test_verdict
Final state verified. Report: **Verified (phase 12 — all 3 tasks already complete; this was the verification pass)** - Re-ran the wire capture live: `ups.test.result` = `No test initiated` (apc) / `Done and passed` (cyberpower) — matches `LIVE_SELF_TEST_WORDS` + both fixtures; `/api/summary` has **no** self-test row, `/api/ups/snapshot` reads `cyberpower ok | Done and passed`, `apc unknown | …run a battery test`; both screenshots read back correct. - Fixed a defect found in prior work while checking the "unrelated live noise" note: `NutMonitor.collect()` reused one upsd socket after a *partially-read* reply, so a 4 s stall produced bogus `expected 'BEGIN LIST VAR', got 'RW apc battery.charge.low "10"'` errors (same hazard on `INSTCMD`, where a late `OK` could be read as a different command's success). `NutClient` now marks a socket whose reply stopped early as `UNUSABLE`, naming the command that stalled; a complete `ERR …` line still leaves it usable (the per-UPS tolerance `collect()` depends on). +3 hermetic pins against the fake upsd; live re-check (09:33–09:44) now logs one line per stall, no cascade. **Tests / lint / coverage (all green)** - `bash .agents/validate.sh` → **exit 0**; `uv run pytest --cov --cov-fail-under=94` → **1172 passed, 11 skipped**, coverage **98.63 %** (floor 94); `uv run ruff check src tests agent` clean; `uv run pyright` 0 errors; `node --check` clean. Modified modules: `sources/nut.py` 95 %, `collectors.py` 97 % (no new line uncovered). - `uv run pytest -m live --live` → **8 passed / 3 failed**, all three the pre-existing `driveripper` agent-transport failures AGENTS.md records (verified still true: unit `inactive`, no installed file, nothing on `:9120`). - `./scripts/dev-server.sh status`: `nut ok`, `llm:* ok`, `host/disks:deskwork|gateway ok`; the two `driveripper` jobs DOWN. **Completion criteria** - Live capture recorded + asserts `ok` — met (re-confirmed from the wire today). - `/ups` renders `ok`, overview silent, screenshots present — met. - pytest / cov / ruff / pyright / validate — met (above). - No other `health.checks[]` behaviour changed — met (diff confined to the `test_result` branch; all prior nut/ups-contract tests unmodified). **Documentation**: README verified consistent (self-test wording, config table cross-checked against `config.py`, all documented commands/flags exist) — no changes needed beyond phase tasks' edit. AGENTS.md fixes: stale test count (1140 → 1172), `nvme0n1` power-on hours 19 158 → 19 160, gap 4 store-error claim corrected (last stored `store-error` 2026-10-18 06:33, none in the previous 24 h), new `collectors.py`/`sources/nut.py` socket-hygiene note, new gap-1 bullet on the upsd stall, plus live evidence appended to `data/logs/NOTES-self-test.md`. **Deviations**: made no live *changes* (read-only probes only) — `driveripper`'s agent install needs root (`ops` has no installer/visudo grant) and a battery test stays the operator's call; the upsd 4 s stall itself is upstream-side. `uv pip list --outdated` still advisory (certifi/fastapi/starlette) — dependencies untouched this phase. **Next pending phase**: `13_llm_page_and_decode_rate`.
This commit is contained in:
@@ -382,6 +382,171 @@ def test_snapshot_payloads_carry_their_own_severity_sections(data_client) -> Non
|
||||
assert isinstance(card["numbers"], dict) and "charge_pct" in card["numbers"]
|
||||
|
||||
|
||||
# ---- the self-test verdict on the two surfaces that were wrong (phase 12) --- #
|
||||
|
||||
#: The `ups.test.result` sentences as the hardware answers them. `Done and passed` is the
|
||||
#: CyberPower's verbatim word from the 2026-10-19 capture (`tests/test_nut.py
|
||||
#: ::LIVE_SELF_TEST_WORDS`); `FAILED` is what a driver says when a test ran and did not pass,
|
||||
#: and is the reason the check exists at all. `tests/test_nut.py` pins the *word map*; what is
|
||||
#: pinned here is the surface `TODO.md:3` names — a warning in the overview's Warnings panel —
|
||||
#: because a verdict that grades correctly inside `assess()` and still reaches the alert feed
|
||||
#: as a warning is the same bug wearing a different hat.
|
||||
SELF_TEST_PASS_WORD = "Done and passed"
|
||||
SELF_TEST_FAIL_WORD = "FAILED"
|
||||
|
||||
#: How `/api/summary` says the check out loud (`alerts._ups_alerts`): `"<check name>: <detail>"`.
|
||||
SELF_TEST_ALERT_TEXT = "Last self-test"
|
||||
|
||||
|
||||
def set_self_test_verdict(payload: dict[str, Any], status: str, detail: str) -> None:
|
||||
"""Rewrite every card's `Last self-test` check in the hub's own UPS payload.
|
||||
|
||||
Mutating the payload the job actually serves (instead of hand-building a fake one) is what
|
||||
makes this an end-to-end pin: `/api/ups/snapshot` and `alerts.build()` then read the *same*
|
||||
object, so the test cannot pass by having the two surfaces disagree about the verdict.
|
||||
"""
|
||||
graded = 0
|
||||
for card in payload["ups"]:
|
||||
for check in card["health"]["checks"]:
|
||||
if check["name"] == SELF_TEST_ALERT_TEXT:
|
||||
check["status"] = status
|
||||
check["detail"] = detail
|
||||
graded += 1
|
||||
assert graded, "no `Last self-test` check in the UPS payload — nothing was tested"
|
||||
|
||||
|
||||
@contextmanager
|
||||
def self_test_app(tmp_path: Path, monkeypatch: pytest.MonkeyPatch,
|
||||
) -> Iterator[tuple[TestClient, dict[str, Any]]]:
|
||||
"""A UPS app whose served payload cannot be replaced by a poll mid-assertion.
|
||||
|
||||
The simulated UPSes answer `PASSED`, so the pass direction would survive a race with the
|
||||
poll loop and the *fail* direction would not — a test that flakes on a poll is a test of
|
||||
timing. Hence `healthy_client`'s trick for the `nut` job: the collector hands back the very
|
||||
dict this harness holds, so the health record says `ok` truthfully (invariant 4) and the
|
||||
payload stays ours. The interval is shortened for the same reason — waiting a real
|
||||
`NUT_INTERVAL_SECONDS` per test would only make the suite slower at proving the same thing.
|
||||
"""
|
||||
with build_client(tmp_path, monkeypatch, simulate=True, nut_host="127.0.0.1",
|
||||
nut_interval=0.5) as test_client:
|
||||
from dashboards.poller import Result # noqa: PLC0415 - only this harness needs it
|
||||
|
||||
hub = test_client.app.state.ctx.hub # type: ignore[attr-defined]
|
||||
job = hub.jobs["nut"]
|
||||
wait_for_payload(test_client, "/api/ups/snapshot")
|
||||
payload = job.state.payload
|
||||
|
||||
async def collect() -> Result:
|
||||
return Result(payload=payload, samples=[])
|
||||
|
||||
job.fn = collect
|
||||
# let one poll of *our* collector land before the caller mutates the payload, so the
|
||||
# health record and the served bytes are both demonstrably ours
|
||||
baseline = job.state.attempts
|
||||
deadline = time.time() + 5.0
|
||||
while time.time() < deadline and job.state.attempts <= baseline:
|
||||
time.sleep(0.02)
|
||||
assert job.state.payload is payload and job.state.ok, (
|
||||
"the nut job is not serving the payload under test — the pin below would be fiction"
|
||||
)
|
||||
yield test_client, payload
|
||||
|
||||
|
||||
def self_test_alerts(alerts_list: list[dict[str, Any]]) -> list[dict[str, Any]]:
|
||||
return [item for item in alerts_list
|
||||
if SELF_TEST_ALERT_TEXT.lower() in item["message"].lower()
|
||||
or SELF_TEST_ALERT_TEXT.lower() in item["id"].lower()]
|
||||
|
||||
|
||||
def test_a_passed_self_test_is_quiet_on_the_overview_and_ok_on_the_ups_page(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""The TODO's sentence, end to end: `Done and passed` is not a warning anywhere.
|
||||
|
||||
Both halves of the fix are asserted on one payload: `/api/ups/snapshot` carries the `ok`
|
||||
entry the `/ups` check list renders verbatim, and `/api/summary` — the only call the
|
||||
overview makes — carries **no** row naming it. The simulated card still contributes its
|
||||
`NUT_SIMULATE` line, so the list is not empty and this cannot be a vacuous pass.
|
||||
"""
|
||||
with self_test_app(tmp_path, monkeypatch) as (test_client, payload):
|
||||
set_self_test_verdict(payload, "ok", SELF_TEST_PASS_WORD)
|
||||
cards = wait_for_payload(test_client, "/api/ups/snapshot")["ups"]
|
||||
for card in cards:
|
||||
check = next(c for c in card["health"]["checks"]
|
||||
if c["name"] == SELF_TEST_ALERT_TEXT)
|
||||
assert check["status"] == "ok", f"{card['name']} reports a pass as {check['status']}"
|
||||
assert check["detail"] == SELF_TEST_PASS_WORD, "the driver's word is the useful half"
|
||||
rows = self_test_alerts(test_client.get("/api/summary").json()["alerts"])
|
||||
assert rows == [], f"a passed battery test is on the overview as {rows}"
|
||||
assert any(item["source"] == "ups" for item in
|
||||
test_client.get("/api/summary").json()["alerts"]), (
|
||||
"the simulated-UPS line disappeared, so this assertion has nothing left to be quiet about"
|
||||
)
|
||||
|
||||
|
||||
def test_a_never_tested_battery_is_not_an_alert_either(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""The APC's factory `No test initiated` reaches the overview as silence, not amber.
|
||||
|
||||
This is the state the live hardware is actually in on one of the two UPSes, so the
|
||||
screenshot proves the absence of a warning for a *never-run* test while the payload pin
|
||||
above proves it for a passed one. `unknown` is not a verdict an operator can act on, and
|
||||
`alerts._RANK` admits only `fail` and `warn`.
|
||||
"""
|
||||
with self_test_app(tmp_path, monkeypatch) as (test_client, payload):
|
||||
set_self_test_verdict(payload, "unknown", nut.NEVER_TESTED_DETAIL)
|
||||
cards = wait_for_payload(test_client, "/api/ups/snapshot")["ups"]
|
||||
assert all(next(c for c in card["health"]["checks"]
|
||||
if c["name"] == SELF_TEST_ALERT_TEXT)["status"] == "unknown"
|
||||
for card in cards)
|
||||
assert self_test_alerts(test_client.get("/api/summary").json()["alerts"]) == []
|
||||
|
||||
|
||||
def test_a_failed_self_test_still_reaches_the_overview(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""The check keeps its reason for existing: a test that failed is a red line, once per UPS.
|
||||
|
||||
Without this direction the absence test above is satisfiable by deleting the check, which
|
||||
is the failure mode the phase's completion criterion calls out.
|
||||
"""
|
||||
with self_test_app(tmp_path, monkeypatch) as (test_client, payload):
|
||||
set_self_test_verdict(payload, "fail", SELF_TEST_FAIL_WORD)
|
||||
names = {card["name"] for card in
|
||||
wait_for_payload(test_client, "/api/ups/snapshot")["ups"]}
|
||||
assert names, "no UPS card to fail"
|
||||
rows = self_test_alerts(test_client.get("/api/summary").json()["alerts"])
|
||||
assert {row["source"] for row in rows} == names
|
||||
for row in rows:
|
||||
assert row["severity"] == "fail", f"a failed battery test reads {row['severity']}"
|
||||
assert row["href"] == "/ups", "the alert must link to the card that names the UPS"
|
||||
assert SELF_TEST_ALERT_TEXT in row["message"] and SELF_TEST_FAIL_WORD in row["message"]
|
||||
assert row["message"].count(SELF_TEST_ALERT_TEXT) == 1, (
|
||||
"the row names the check twice — see alerts._clean_error")
|
||||
assert all(row["id"] == f"{row['source']}:health:{SELF_TEST_ALERT_TEXT}" for row in rows)
|
||||
|
||||
|
||||
def test_the_ups_check_list_needs_no_new_markup_for_a_passed_verdict(client) -> None:
|
||||
"""Task 03 expected no page change, and this pins *why* there is none.
|
||||
|
||||
`page-ups.js` puts `check.status` straight on the `<li>`, and `dash.css` colours only the
|
||||
`.mark` glyph from the shared `--ok`/`--warn`/`--fail` tokens while the check's name and
|
||||
detail stay `--text`/`--text-muted` — so an `ok` row is quiet ink with a tick, and a verdict
|
||||
fixed in `assess()` is a verdict rendered. Nothing on either asset names the self-test
|
||||
check, which is the point: a special case added here would be a second opinion about the
|
||||
driver's word, and the API already made it.
|
||||
"""
|
||||
js = _code(client.get("/static/page-ups.js").text)
|
||||
assert '<li class="${check.status}">' in js, "the row no longer wears the payload verdict as its class"
|
||||
for verdict in ("ok", "warn", "fail", "unknown"):
|
||||
assert f"{verdict}: '" in js[js.index("const MARKS"):], f"MARKS lost the {verdict} glyph"
|
||||
rows = js[js.index("const health = item.health"):js.index("const commands = item.commands")]
|
||||
assert "self-test" not in rows.lower(), "the check list grew an opinion about one check by name"
|
||||
css = _code(_css(client))
|
||||
ok_rule = re.search(r"ul\.checks li\.ok \.mark \{([^}]*)\}", css)
|
||||
assert ok_rule and "var(--ok)" in ok_rule.group(1), "the ok tick is not the shared token"
|
||||
assert "ul.checks li.ok {" not in css, "the whole ok row is coloured — a pass is not a banner"
|
||||
assert "self-test" not in css.lower(), "the stylesheet special-cases one check by name"
|
||||
|
||||
|
||||
# ---- unconfigured: say so, and do not dial -------------------------------- #
|
||||
|
||||
def test_config_reports_every_source_as_off(client) -> None:
|
||||
|
||||
Reference in New Issue
Block a user