test(ui): guard the interpolated placeholders that the language packs require - #171
Merged
Merged
Conversation
… require The four key-resolution tests added in #170 check that every key exists, but nothing checked that a call site actually passes the variables the text needs. ui/js/i18n.js leaves the placeholder verbatim when a variable is missing, so T("cnt.calls") renders as {n} 次 / {n} instead of 1,234 次 / 1,234 - a defect that fails no existing assertion. Adds one assertion class to src/i18n_pack.rs plus its negative control: - every T("key", { ... }) call site in ui/js/app.js supplies all placeholders that either pack uses for that key (487 sites, 0 violations today) - no value bound by a static data-i18n* attribute contains a placeholder - those are written by applyStatic() with innerHTML and have no argument channel (305 keys, 0 today) The call-site scanner reuses the module's existing T("literal") rule rather than reimplementing it (a "smarter" draft counted 517 sites instead of 520, i.e. a different set than every_t_literal_resolves). Tests only: no production code, no new dependency, no language-pack or ui/ change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The four key-resolution tests added in #170 check that every key exists, but nothing checked that a call site actually passes the variables the text needs.
ui/js/i18n.jsleaves the placeholder verbatim when a variable is missing:so
T("cnt.calls")renders as{n} 次/{n}instead of1,234 次/1,234. That defect fails no existing assertion — the key exists, both packs have it, and every count is unchanged — so today only a human eye can catch it.This adds one more assertion to the same module (
src/i18n_pack.rs), plus its negative control:T("key", { … })call site inui/js/app.jssupplies all placeholders that either pack's text for that key uses (487 call sites, 0 violations today);data-i18n*attribute contains a placeholder — those are written byapplyStatic()viainnerHTMLand have no argument channel at all, so such text would be wrong permanently (305 bound keys, 0 today).The call-site scanner reuses the module's existing
T("literal")recognition rule instead of reimplementing it: an earlier draft with a "smarter" rule found 517 call sites instead of 520, i.e. it counted a different set thanevery_t_literal_resolves. The tests now share one primitive, and the call-site count is asserted against the existingT_LITERAL_COUNTcontrol.Related Issue
None. This is a test-only change (CONTRIBUTING sanctions the
testcommit type and requires#[cfg(test)]tests for new functionality), and the repository has no open issue.Changes
ui/file or behaviour. Adds 2 tests to the existing#[cfg(test)] mod i18n_pack(still compiled only in test builds).Tests
cargo test全部通过 — 155 passed (was 153)cargo fmt --check通过The new tests are not merely green on today's tree — they are proven able to fail. Injecting (a) a dropped
{ n: … }argument at a real call site, (b) a renamed variable ({ count: … }), and (c) a placeholder on a statically bound key each turned the new assertion red with a message naming the offending key. The UI files were restored byte-for-byte afterwards (md5 verified againstgit show HEAD:…).Checklist
test/i18n-placeholder-gate. CONTRIBUTING listsfeat/ fix/ docs/ refactor/but does not listtest/, whiletestis an allowed commit type and nothing but tests changed; the same deviation was taken and disclosed in test(ui): guard the language-pack invariants that were previously unchecked #170.test(ui): …src/i18n_pack.rs