test: escape placeholder values in the translate() mock, matching real behaviour - #136
Open
moodyjmz wants to merge 1 commit into
Open
test: escape placeholder values in the translate() mock, matching real behaviour#136moodyjmz wants to merge 1 commit into
moodyjmz wants to merge 1 commit into
Conversation
…l behaviour Real translate() escapes every placeholder value by default (escape-html), but the shared vitest.setup.ts mock did naive substitution with no escaping — so any spec asserting on translated, placeholder-interpolated text passed regardless of whether the real string would come out double-escaped in a context that doesn't expect it (an ARIA attribute, for one). Found reviewing #125, where this exact gap hid a real bug. Pin the mock's contract with a small dedicated spec rather than relying on each feature spec to happen to cover it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
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
1 participant
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.
Real
translate()escapes every placeholder value by default (escape-html), but the sharedvitest.setup.tsmock did naive substitution with no escaping — so any spec asserting on translated, placeholder-interpolated text passed regardless of whether the real string would come out double-escaped in a context that doesn't expect it (an ARIA attribute, for one).Found reviewing #125, where this exact gap hid a real bug (an owner display name containing
&/</>/quotes producing a mangled accessible name — invisible to the test because the mock never escaped it).Adds
escape-html+@types/escape-htmlas devDependencies (same versions@nextcloud/l10nitself uses), and a small dedicated spec (src/test-utils/translateMock.spec.ts) pinning the mock's contract, rather than relying on each feature spec to happen to cover it.Verified:
npm run test:unit(85/85 passing, up from 83 — no existing test relied on the unescaped behavior),npm run lint,npm run stylelint,npm run build(byte-identical output, confirming this is test-only).🤖 AI (if applicable)