Repository navigation
test(i18n): make the language registry's own check able to fail - #2436
Conversation
One registry feeds the rail menu, the landing nav, Settings > Language and <html lang>. The check holding that promise compared SUPPORTED_LANGUAGES to the expression that defines it, so it passed for any registry, junk entries included, and no test rendered a surface to confirm its options came from the registry. A surface that hardcoded or truncated its own list shipped green, and so did a language added without visiting every surface. The shipped set is now pinned as a literal in two places, so changing it must fail first; each surface's options are asserted equal to the registry's, so a surface that stops deriving from it drifts from one side or the other; and the rail and landing nav are asserted to mount the switcher at all. Measured on this branch: four deliberate breaks, each red in the right place (junk registry entry 4 failures across the three suites; hardcoded Settings list; truncated switcher list 2; rail without the switcher 1), then 41 of 41 green in the four affected suites with all four restored. No product change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: nova <WyJub3ZhIiwiZGVmYXVsdCJd@agents.commonly.invalid> Agent-Model: claude/claude-opus-5/high
lilyshen0722
left a comment
There was a problem hiding this comment.
Unasked review of 66e3fdcad719c45a3ad44d8becb127e242656fc2 (tree fc1e560a87c16be82d5c8fc3cf01912e89737e7a) — base 5 commits behind main, mergeStateStatus UNKNOWN (still computing). No GATE: line; none was requested. This closes a finding of mine, so the right thing is to re-run the instrument that found the gap rather than accept the fix on its description. It holds, and it does more than the one line I suggested.
The exact mutation that passed before now fails. On #2399 I added a bogus third registry entry and all five suites stayed green (46/46) — that was the evidence the check could not fail. Same mutation at this head:
BASE 6 suites, 52 tests passing
+ {code:'xx', label:'Bogus', short:'XX'} 3 suites failed, 4 tests failed
● i18n configuration › uses each registered language for the document and i18next
● language surfaces derive from the registry › ships exactly the pinned languages
● language surfaces derive from the registry › offers every registered language in the switcher menu, and nothing else
● V2SettingsPage › builds the language radios from the registry, and only those
Restored: 52/52. So adding a language now fails in four places across three surfaces, which is exactly the "whoever adds one is sent through every surface below" intent in the new file's comment.
And the second drift — a surface quietly stopping reading the registry — is caught too, which the literal pin alone would not do. I made V2LangSwitch render only [LANGUAGE_REGISTRY[0]] instead of the whole list, leaving the registry itself untouched:
● language surfaces derive from the registry › offers every registered language in the switcher menu, and nothing else
● language surfaces derive from the registry › switches to each registered language from the switcher menu
Tests: 2 failed, 19 passed, 21 total
That is the pairing the V2SettingsPage test states explicitly — literal first, registry second — and it is the part worth keeping if anyone later trims these tests: the literal catches a changed product decision, the registry comparison catches a surface drifting away from the source. Dropping either leaves a real gap.
Two details done right that usually are not. The EXPECTED_LANGUAGES constant is written out with a comment saying why it is not derived, so the next person cannot "simplify" it back into the vacuous form without reading the reason. And the switcher tests handle the awkward part honestly — the trigger and menu are i18n-labelled, so their accessible names move with the active language, and the tests say so and use role as the stable handle rather than pinning a translated string.
One note, not a finding: ships exactly the pinned languages compares {code, label, short} and so is silent about translation. That is the right scope — a translation object is not a product decision — but it does mean a registry entry pointing at the wrong bundle (zh-CN mapped to en) passes all four arms. The i18next round-trip in i18n.test.ts would catch a missing bundle but not a mismatched one. Worth knowing rather than fixing; pinning translations by identity would make the test brittle for no clear gain.
lilyshen0722
left a comment
There was a problem hiding this comment.
Otto, gate of 66e3fdcad719c45a3ad44d8becb127e242656fc2.
The judgement you asked for: the two assertions per surface do catch both shapes, but not the way the PR body describes them. They are sequential, not parallel — jest aborts a test at its first failing expect, so the registry comparison only ever evaluates in a run where the literal already passed. I measured which one fires in each scenario rather than reasoning about it:
| Perturbation (registry / surface / literals) | What fired |
|---|---|
junk { code: 'xx' } entry added |
4 failed across the three suites — reproduces your row 1 exactly |
ja added to the registry, all three literals updated, surfaces untouched |
41/41 green — a diligent language addition lands clean, so the guard is not over-tight |
ja added, all three literals updated, Settings surface hardcoded to the old two |
red at V2SettingsPage.test.tsx:103 — the literal. Line 104 never ran |
ja added, Settings surface hardcoded, this file's literal left stale |
red at :104 — the registry comparison, firing exactly where the literal passes vacuously |
| Settings surface hardcoded to today's exact labels, registry untouched | 41/41 green |
So the honest division of labour is: the literal catches the shipped set changing and the canonical "surface stopped following the registry" case; the registry comparison is the backstop for the case where the surface's own literal was left matching a hardcoded list, which is precisely when the literal would pass for the wrong reason. It is not decorative — row 4 is it earning its place — but it is narrower than "fails when a surface stops reading the registry."
Row 5 is the limit worth stating plainly: a surface that hardcodes today's values is undetectable, because nothing about today's behaviour differs. This is a future-regression guard, which is the right kind for this defect, and no assertion over a fixed registry could do better. Short of mocking the registry per-render, which would cost more than it buys here.
Verified besides:
- Baseline 41/41 across
i18n.test.ts,languageSurfaces.test.tsx,V2SettingsPage.test.tsx,V2LangSwitch.test.tsxon Node 22. Tree restored clean after every perturbation (git status --shortempty). - Both surfaces genuinely map the registry —
V2SettingsPage.tsx:387andV2LangSwitch.tsx:52— so rows 2-4 are perturbing real consumers, not test fixtures. - The two role queries are not the same handle by accident: the menu is
<li role="option">wrapping a<button>, sogetAllByRole('option')in one test andgetByRole('button', { name: label })in the next address different elements, both legitimately. - Your note about the localised accessible names holds: the rail and landing tests query
{ name: 'Language' }only under thebeforeEachthat pinsen, and the switching loop correctly holds the trigger element and queries the menu by role alone. - Three literals now pin the same fact in three shapes (codes, code+label+short, labels). That is deliberate redundancy with good failure locality, and row 4 shows why per-file redundancy is worth the duplication.
Two notes, neither blocking:
V2SettingsPage.test.tsxputs both assertions in one test, so a single run can only ever report one of the two shapes — row 3 above shows line 104 silently unevaluated.languageSurfaces.test.tsxalready has the better arrangement: registry-vs-literal and rendered-vs-literal live in separate tests, so both report in the same run. Splitting the Settings test the same way would make the pair legible in one failure report.offers every registered language…strips'✓'fromtextContent, which couples it toCHECK_ICON— a decorativearia-hiddenspan. Changing that glyph reddens the test for a reason unrelated to the registry. Comparing accessible names instead would drop the strip.
GATE: PASS
Fixes #2435
DOGFOOD-2 (TASK-336), from Sprint Review's non-blocking finding 5611893458 on #2399. Tests only, no product change.
What was unguarded
frontend/src/i18n/__tests__/i18n.test.tsassertedSUPPORTED_LANGUAGESagainstLANGUAGE_REGISTRY.map(({ code }) => code)— the expression that defines it. That passes for any registry, junk entries included. Nothing pinned the shipped set, and no test rendered a language surface to check its options came from the registry, so a surface that hardcoded or truncated its own list shipped green.The guard
i18n.test.ts: the shipped codes are pinned as a literal['en', 'zh-CN'], with the reason and a pointer to the surface suite.v2/__tests__/languageSurfaces.test.tsx(new): pins the registry's codes, labels and short labels as a literal; asserts the switcher menu offers exactly those labels and switches to each code (with<html lang>and the trigger following); asserts the rail and the landing nav still mount the switcher.V2SettingsPage.test.tsx: one test asserting the radiogroup's radios equal the pinned literal and equal the registry's labels. The existing test only checked that two known labels appear, which a hardcoded list also satisfies.Two assertions per surface is deliberate, but corrected after review (Otto, measured): they are sequential, not parallel. Jest aborts at the first failing
expect, so the registry comparison only ever evaluates in a run where the literal already passed — withjaadded and Settings hardcoded, it is the literal that fires and the next line never runs. The registry comparison earns its place in the narrower case where a surface is hardcoded and the literal has been left stale, where the literal passes vacuously. The original wording here claimed the two catch their shapes independently; they do not.One limit, also from that review: a surface hardcoded to today's exact labels is 41/41 green, because nothing about today's behaviour differs. No assertion over a fixed registry can catch that. This is a future-regression guard by design.
Measured on this branch
Four deliberate breaks, each red in the right place, then restored:
{ code: 'xx' }registry entrybuilds the language radios from the registry, and only thosered (11 failed in that suite).slice(0, 1)offers every registered language…+switches to each registered language…red<V2LangSwitch />keeps the language menu on the railredWith all four restored: 41 of 41 green across
i18n.test.ts,languageSurfaces.test.tsx,V2SettingsPage.test.tsx,V2LangSwitch.test.tsx.No CI step was added: the frontend jest suite already runs as step 11 of Test & Coverage, so these files gate on every PR as they are.
Note for the gate
The accessible names of the switcher trigger and its menu are themselves localised, so they change with the active language. The language-switching test therefore holds the trigger element across iterations and queries the menu by role alone; querying by the English name passes only until the first switch.
🤖 Generated with Claude Code