test(frontend): a render harness, because the null-is-unknown rule lived untested - #117
Conversation
…ved untested
frontend/ had no test runner at all -- no harness, no CI check beyond the
release build. The rule that matters most in this app now lives there: a
platform with NO reading must render as an em dash, never 0.00. Showing
zero reports a loss that did not happen, and it does so most
convincingly to the user whose collector is broken, who is precisely the
person who must not be told everything is fine.
The Go side proves null survives to JSON, with negative controls.
Nothing proved the render.
THE BARRIER, and the first step the bead named: main.ts does
`const root = document.querySelector("#app")!` at module scope and
imports the Wails runtime, so importing it from Node needs a DOM, a CSS
loader and Wails stubs. Nothing in it could be tested.
So the pure render functions move to frontend/src/render/ -- no DOM, no
globals, no import-time side effects. renderFleetSection, relativeTime,
escapeHtml and formatBalance are moved VERBATIM; main.ts imports them
back. The built bundle goes 51.21 kB -> 51.26 kB, which is the extraction
being behaviour-neutral.
scripts/fleet_render_check.mjs then drives the REAL functions against
constructed state and asserts on the HTML they return -- 31 checks
covering the null rule and its mirror (a MEASURED zero must still print
as money), shared-platform marking, escaping of the server URL and slugs
into an innerHTML sink, freshness phrasing, and null vs empty lists.
It deliberately does NOT assert on source text. This repo and its sibling
have both been bitten by tests that matched their own prose, where a
check that a file CONTAINS a guard passed against a build where the guard
was unreachable.
Proven by control: turning the null guard into `?? 0` fails "an unknown
platform does NOT render a zero amount", and dropping escapeHtml from the
server URL fails "the server URL is escaped".
CI gains a frontend job -- typecheck, render checks, vite build. Before
this the frontend was built ONLY during a release, so a broken one
reached main and was found at tag time: the same shape as the docs site
building only on deploy, where the gate ran after the decision it was
meant to inform.
Refs: CashPilot-Desktop-tft
📝 WalkthroughWalkthroughThe frontend now uses pure rendering and formatting modules. A browser-free Node.js harness validates their output. CI runs typechecking, render checks, and the Vite production build. ChangesFrontend render testing
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/scripts/fleet_render_check.mjs`:
- Around line 77-85: Update the currency assertions in the relevant checks
around the fleet rendering tests to avoid hard-coded dot-decimal matching.
Compare rendered values against the generated formatBalance(amount, currency)
result, or use the locale-appropriate decimal separator, including the
assertions at the measured-zero, known-amount, and line-184 cases.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 36efd942-8e7f-45f1-aaf4-3f7cdde743e0
📒 Files selected for processing (9)
.github/workflows/ci.yml.gitignoreCLAUDE.mdfrontend/package.jsonfrontend/scripts/fleet_render_check.mjsfrontend/src/main.tsfrontend/src/render/fleet.tsfrontend/src/render/format.tsfrontend/tsconfig.harness.json
| check("a MEASURED zero renders as money, not a dash", /0\.00/.test(html), html.slice(0, 400)); | ||
| check("a measured zero is not called unknown", !html.includes("nothing collected yet")); | ||
| } | ||
|
|
||
| { | ||
| const html = renderFleetSection( | ||
| view({ platforms: [{ slug: "grass", usd: 4.25, shared: false }], totalUsd: 4.25 }), | ||
| ); | ||
| check("a known amount is rendered", /4\.25/.test(html), html.slice(0, 400)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does Intl.NumberFormat(undefined, options) use the host environment's default locale in ECMAScript?
💡 Result:
Yes, when the locales argument is undefined (or omitted) in the Intl.NumberFormat constructor, the ECMAScript Internationalization API (ECMA-402) uses the host environment's default locale [1][2]. In the ECMAScript specification, when Intl.NumberFormat is initialized without a specified locale, the runtime performs locale negotiation and defaults to the implementation's configured default locale [2]. This allows the formatting behavior to adapt automatically to the user's or system's language preferences if the application does not explicitly override them [3][2]. You can verify the locale being used by a specific Intl.NumberFormat instance by calling the resolvedOptions() method, which returns an object containing the locale property reflecting the actual locale used for formatting [4]. It is important to note that the specific "default locale" is determined by the host environment (such as a web browser or Node.js) [2][5]. For example, in environments with limited internationalization support or specific configurations (like some Node.js builds), the default might fall back to "en-US" if additional locale data is not available or configured [5].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/NumberFormat/NumberFormat
- 2: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl
- 3: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Internationalization
- 4: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/NumberFormat/resolvedOptions
- 5: NumberFormat is always en-US nodejs/node#8818
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'fleet_render_check\.mjs$' . || true
echo "== relevant snippets =="
if [ -f frontend/scripts/fleet_render_check.mjs ]; then
wc -l frontend/scripts/fleet_render_check.mjs
sed -n '1,240p' frontend/scripts/fleet_render_check.mjs | nl -ba
fi
echo "== search formatBalance usages =="
rg -n "function formatBalance|const formatBalance|formatBalance\\(" frontend/scripts frontend -g '*.mjs' -g '*.js' -g '*.ts' -g '*.tsx' || true
echo "== determine exact import and formatBalance definition locations =="
python3 - <<'PY'
from pathlib import Path
p=Path('frontend/scripts/fleet_render_check.mjs')
if p.exists():
text=p.read_text()
for i,line in enumerate(text.splitlines(),1):
if 'formatBalance' in line:
print(i,line)
PYRepository: GeiserX/CashPilot-Desktop
Length of output: 349
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant snippets with line numbers =="
awk '{printf "%6d\t%s\n", NR, $0}' frontend/scripts/fleet_render_check.mjs | sed -n '1,240p'
echo "== search formatBalance usages/definition =="
rg -n "function formatBalance|const formatBalance|formatBalance\\(" frontend/scripts frontend -g '*.mjs' -g '*.js' -g '*.ts' -g '*.tsx' || true
echo "== exact formatBalance lines in check file =="
python3 - <<'PY'
from pathlib import Path
p=Path('frontend/scripts/fleet_render_check.mjs')
text=p.read_text().splitlines()
for i,line in enumerate(text,1):
if 'formatBalance' in line:
print(i, line)
PYRepository: GeiserX/CashPilot-Desktop
Length of output: 12839
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '== format.ts implementation =='
awk '{printf "%6d\t%s\n", NR, $0}' frontend/src/render/format.ts | sed -n '1,140p'
printf '%s\n' '== inspect harness format.js if present =='
if [ -f frontend/.harness-build/render/format.js ]; then
awk '{printf "%6d\t%s\n", NR, $0}' frontend/.harness-build/render/format.js | sed -n '1,140p'
else
echo "frontend/.harness-build/render/format.js not present"
fi
printf '%s\n' '== behavioral probe for Intl.NumberFormat locales if Node Intl is available =='
node - <<'JS'
console.log("process.version=", process.version);
try {
const fmtUSD = new Intl.NumberFormat("en-US", { style: "currency", currency: "USD", maximumFractionDigits: 2 });
const fmtDE = new Intl.NumberFormat("de-DE", { style: "currency", currency: "USD", maximumFractionDigits: 2 });
const fmtUndefinedUSD = new Intl.NumberFormat(undefined, { style: "currency", currency: "USD", maximumFractionDigits: 2 });
console.log("en-US USD 0:", fmtUSD.format(0));
console.log("de-DE USD 0:", fmtDE.format(0));
console.log("undefined USD expected 0:", fmtUndefinedUSD.format(0), fmtUndefinedUSD.resolvedOptions().locale);
} catch (e) {
console.log("Intl probe failed:", e && e.message);
}
JSRepository: GeiserX/CashPilot-Desktop
Length of output: 3509
Make currency-value assertions locale-independent.
formatBalance(undefined) allows Intl.NumberFormat to use the host default locale, which may render 0.00 as 0,00. Update the assertions at lines 77, 85, and 184 to compare against the generated formatBalance(amount, currency) output or the expected decimal separator for that amount.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/scripts/fleet_render_check.mjs` around lines 77 - 85, Update the
currency assertions in the relevant checks around the fleet rendering tests to
avoid hard-coded dot-decimal matching. Compare rendered values against the
generated formatBalance(amount, currency) result, or use the locale-appropriate
decimal separator, including the assertions at the measured-zero, known-amount,
and line-184 cases.
Closes
CashPilot-Desktop-tft.The gap
frontend/had no test runner at all — no harness, no CI check beyond the release build. And the rule that matters most in this app now lives there:Showing zero reports a loss that did not happen — most convincingly to the user whose collector is broken, who is precisely the person who must not be told everything is fine.
The Go side proves
nullsurvives to JSON, with negative controls. Nothing proved the render.The barrier, and the first step the bead named
main.tsdoesconst root = document.querySelector("#app")!at module scope and imports the Wails runtime, so importing it from Node needs a DOM, a CSS loader and Wails stubs. Nothing in it could be tested.So the pure render functions move to
frontend/src/render/— no DOM, no globals, no import-time side effects.renderFleetSection,relativeTime,escapeHtmlandformatBalanceare moved verbatim;main.tsimports them back.The built bundle goes 51.21 kB → 51.26 kB, which is the extraction being behaviour-neutral.
The harness
scripts/fleet_render_check.mjsdrives the real functions against constructed state and asserts on the HTML they return — 31 checks:0must still print as money, because a guard that renders everything as a dash is as useless as no guardinnerHTMLsink""rather thanInvalid Datenullvs empty lists — the Go side sendsnull, not[], for "none"It deliberately does not assert on source text. This repo and its sibling have both been bitten by tests that matched their own prose, where a check that a file contains a guard passed against a build where the guard was unreachable. The bead says so explicitly and it is worth repeating in the harness itself.
Modelled on CashPilot's own browser-free harnesses (
currency_check.mjs,fleet_staleness_check.mjs, …), which are wired into CI the same way.Verification
?? 0escapeHtmlfrom the server URLFull CI sequence run on a real Linux container:
tsc --noEmitclean,npm test31/31,vite buildsucceeds. Go untouched —build,vet,test ./...all green.CI gains a frontend job
Typecheck → render checks →
vite build, on every PR.Before this the frontend was built only during a release, so a broken one reached main and was found at tag time. That is the same shape as the docs site building only on deploy: the gate ran after the decision it was meant to inform.
Summary by CodeRabbit
Bug Fixes
Tests