health_check's smoke path has no unit coverage — every test short-circuits before it #53

New issue

Closed

opened 2026-08-17 22:20:35 +00:00 by forgejo-admin · 0 comments

forgejo-admin commented

2026-08-17 22:20:35 +00:00

Owner

Found while fixing #52. Separate defect, filing rather than growing that PR.

What

lunacycleHealthCheck gates its three public smoke checks on a secret:

const botPassword = secrets.smoketest_bot_password;
if (!botPassword) {
  logger.info('Skipping SPA/API/WS smoke checks: smoketest_bot_password not set');
} else {
  const botToken = await mintSmokeToken({ ... });
  if (botToken) {
    await runSpaSmoke({ ... });
    await runApiSmokeCheck({ ... });
    await runWsSmokeCheck({ ... });
  }
}

All 15 call sites in health-check.test.ts pass secrets: {}. Not one sets
smoketest_bot_password, so every test takes the skip branch. mintSmokeToken,
runSpaSmoke, runApiSmokeCheck and runWsSmokeCheck are never entered by any unit
test, and neither is the botToken falsy branch that suppresses the three checks after
a mint failure.

This was masked twice over: the directory was never typechecked (#52), and the tests
that do pass genuinely pass — they just never reach the code.

Why it matters

This is the branch that decides whether a deploy is healthy. It runs on every deploy,
on the 15m monitor, and via the deploy.completed subscriber. Its only proving ground
today is production. A regression in the mint/smoke path is invisible until it fires
against the live fleet, and its failure mode is a deploy that reports healthy when it
is not — or one that fails and blocks a release for a reason no test can reproduce.

Fix

The seams already exist, so this needs no refactor:

deps.loadChromium is an injectable override (documented as "Tests pass a stub here")
and nothing passes one.
mintSmokeToken goes through the injected fetch, so createMockFetcher can answer
the authentik token endpoint.

Worth covering, in rough priority:

smoketest_bot_password set + mint succeeds -> all three smoke checks run and their
results reach addCheck.
Mint fails -> exactly one failed check recorded, and the three smoke checks are
not run (the comment claims this explicitly; nothing verifies it).
smoketest_bot_password absent -> the skip is logged. This is the only branch with
coverage today, and only by accident.

Keep it focused — three tests, not a suite per helper.

Found while fixing #52. Separate defect, filing rather than growing that PR. ## What `lunacycleHealthCheck` gates its three public smoke checks on a secret: ```ts const botPassword = secrets.smoketest_bot_password; if (!botPassword) { logger.info('Skipping SPA/API/WS smoke checks: smoketest_bot_password not set'); } else { const botToken = await mintSmokeToken({ ... }); if (botToken) { await runSpaSmoke({ ... }); await runApiSmokeCheck({ ... }); await runWsSmokeCheck({ ... }); } } ``` All 15 call sites in `health-check.test.ts` pass `secrets: {}`. Not one sets `smoketest_bot_password`, so **every test takes the skip branch**. `mintSmokeToken`, `runSpaSmoke`, `runApiSmokeCheck` and `runWsSmokeCheck` are never entered by any unit test, and neither is the `botToken` falsy branch that suppresses the three checks after a mint failure. This was masked twice over: the directory was never typechecked (#52), and the tests that do pass genuinely pass — they just never reach the code. ## Why it matters This is the branch that decides whether a deploy is healthy. It runs on every deploy, on the 15m monitor, and via the `deploy.completed` subscriber. Its only proving ground today is production. A regression in the mint/smoke path is invisible until it fires against the live fleet, and its failure mode is a deploy that reports healthy when it is not — or one that fails and blocks a release for a reason no test can reproduce. ## Fix The seams already exist, so this needs no refactor: - `deps.loadChromium` is an injectable override (documented as "Tests pass a stub here") and nothing passes one. - `mintSmokeToken` goes through the injected `fetch`, so `createMockFetcher` can answer the authentik token endpoint. Worth covering, in rough priority: 1. `smoketest_bot_password` set + mint succeeds -> all three smoke checks run and their results reach `addCheck`. 2. Mint fails -> exactly one failed check recorded, and the three smoke checks are **not** run (the comment claims this explicitly; nothing verifies it). 3. `smoketest_bot_password` absent -> the skip is logged. This is the only branch with coverage today, and only by accident. Keep it focused — three tests, not a suite per helper.

forgejo-admin added the

labels

2026-08-17 22:20:35 +00:00

forgejo-admin referenced this issue from a commit

2026-08-17 22:21:02 +00:00

fix(ci): typecheck the celilo hook scripts, and fix the 18 errors behind it

forgejo-admin referenced this issue

2026-08-17 22:21:28 +00:00