test(i18n): follow-ups of the review of PR #435
- renderCategoryLabel.test.ts passes markedT itself, its assertions in the marked form (`t(balance.category.cash[defaultValue=cash])`): the second fake `t` of the repository is gone. - i18n-mock-guard: a fake `t` is also caught as a binding (`t = (`, `t = vi.fn(`) or as a `t: vi.fn(` property, and a file that mocks react-i18next must call `i18nMock(`, comments dropped before the search. Each new rule has its own case in the guard's tests; the binding rule found renderCategoryLabel.test.ts before its migration. - docs/architecture.md: the "only fake t" sentence is scoped to test files and says the guard only refuses the forms it recognises. Refs #389 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
le king fu
parent
a1c31a1da7
commit
ac3a981f64
3 changed files with 57 additions and 23 deletions
|
|
@ -619,7 +619,7 @@ Utilisateurs : `useOnboardingChecklist.wiring.test.ts`, `UsersSettingsPage.test.
|
|||
|
||||
### Faux `t` marqué — `src/test/i18nMock.ts` (#389)
|
||||
|
||||
**Jamais de `t` brut dans un test.** Un faux `t` qui rend la clé telle quelle cache un appel à `t` oublié : un composant qui affiche `license.removeKey.confirmValid` au lieu du texte traduit passe toute la suite. Le seul faux `t` du dépôt est `markedT` : il rend `t(clé)`, ou `t(clé[a=1,b=2])` avec des options, dans l'ordre des options. Motif unique, en tête du fichier de test :
|
||||
**Jamais de `t` brut dans un test.** Un faux `t` qui rend la clé telle quelle cache un appel à `t` oublié : un composant qui affiche `license.removeKey.confirmValid` au lieu du texte traduit passe toute la suite. Dans les fichiers de test, le seul faux `t` est `markedT` (`src/test/i18nMock.ts`), servi par la fabrique `i18nMock` ou passé tel quel à une fonction qui prend `t` en paramètre (par exemple `renderCategoryLabel`) : il rend `t(clé)`, ou `t(clé[a=1,b=2])` avec des options, dans l'ordre des options. La garde ci-dessous ne refuse que les formes qu'elle sait reconnaître : un faux `t` écrit autrement lui échapperait. Motif unique, en tête du fichier de test :
|
||||
|
||||
```ts
|
||||
vi.mock("react-i18next", async () => (await import("../test/i18nMock")).i18nMock());
|
||||
|
|
@ -629,7 +629,7 @@ Options de la fabrique : `language` (« fr » par défaut ; une fonction est rel
|
|||
|
||||
Les assertions qui nomment un texte affiché s'écrivent sous la forme marquée (`toContain(">t(dashboard.title)<")`, `toEqual(["t(onboarding.state.done)"])`) : elles rougissent sur une chaîne qui a sauté `t`. Une assertion par sous-chaîne sur une clé nue reste valide, la clé figurant dans la marque, mais ne prouve pas l'appel à `t`. Dans une expression régulière, échapper les parenthèses et crochets de la marque.
|
||||
|
||||
Garde : `src/__integration__/i18n-mock-guard.test.ts` lit tous les `*.test.ts(x)` de `src/` et échoue sur une propriété `useTranslation:` ou `t: (` (ancrées sur une frontière de mot) hors de `src/test/i18nMock.ts`, ou sur un `vi.mock("react-i18next", …)` qui ne passe pas par `i18nMock`.
|
||||
Garde : `src/__integration__/i18n-mock-guard.test.ts` lit tous les `*.test.ts(x)` de `src/` et échoue, hors de `src/test/i18nMock.ts`, sur une propriété `useTranslation:`, sur un faux `t` en propriété (`t: (`, `t: vi.fn(`) ou en liaison (`t = (`, `t = vi.fn(`), tous ancrés sur une frontière de mot, ou sur un `vi.mock("react-i18next", …)` sans appel à `i18nMock(` (le nom seul, dans un commentaire par exemple, ne compte pas : les commentaires sont retirés avant la recherche).
|
||||
|
||||
## CI/CD
|
||||
|
||||
|
|
|
|||
|
|
@ -7,13 +7,17 @@
|
|||
* does, and fails on any test file (`*.test.ts`, `*.test.tsx`) other than the
|
||||
* module itself that:
|
||||
* 1. declares a `useTranslation:` property (a hand-written `react-i18next` module);
|
||||
* 2. declares a `t: (` property (a hand-written fake `t`);
|
||||
* 3. mocks `react-i18next` without going through `i18nMock`.
|
||||
* 2. declares a hand-written fake `t`: a `t: (` or `t: vi.fn(` property, or a
|
||||
* `t = (` / `t = vi.fn(` binding (`const t = (key) => key`, passed to a
|
||||
* function that takes `t`);
|
||||
* 3. mocks `react-i18next` without an actual call to `i18nMock(` (the name
|
||||
* alone, in a comment for instance, is not enough: comments are dropped
|
||||
* before the search).
|
||||
*
|
||||
* Keywords rather than an analysis of the factory: less fragile. The matches
|
||||
* are anchored on a word boundary, so `useLicenseContext: () =>` or
|
||||
* `get: (target, key)` are not a `t: (`. This file names the patterns only
|
||||
* inside regular expressions and excludes itself.
|
||||
* are anchored on a word boundary, so `useLicenseContext: () =>`,
|
||||
* `get: (target, key)`, `const at = (` or `t === (` are not a fake `t`. This
|
||||
* file names the patterns only inside regular expressions and excludes itself.
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from "vitest";
|
||||
|
|
@ -25,8 +29,14 @@ const MODULE = resolve(SRC, "test", "i18nMock.ts");
|
|||
const SELF = resolve(import.meta.dirname, "i18n-mock-guard.test.ts");
|
||||
|
||||
const LOCAL_MODULE = /\buseTranslation\s*:/;
|
||||
const LOCAL_T = /\bt\s*:\s*\(/;
|
||||
const LOCAL_T = /\bt\s*[:=]\s*(?:\(|vi\.fn\()/;
|
||||
const MOCKS_I18NEXT = /vi\.mock\(\s*["']react-i18next["']/;
|
||||
const CALLS_I18NMOCK = /\bi18nMock\s*\(/;
|
||||
|
||||
/** The source without its comments, so a mention of `i18nMock(` in one does not count as a call. */
|
||||
function withoutComments(source: string): string {
|
||||
return source.replace(/\/\*[\s\S]*?\*\//g, "").replace(/(^|[^:])\/\/.*$/gm, "$1");
|
||||
}
|
||||
|
||||
/** The offending lines of one source, as `path:line: text`. */
|
||||
function offences(path: string, source: string): string[] {
|
||||
|
|
@ -36,7 +46,7 @@ function offences(path: string, source: string): string[] {
|
|||
.flatMap((line, i) =>
|
||||
LOCAL_MODULE.test(line) || LOCAL_T.test(line) ? [`${name}:${i + 1}: ${line.trim()}`] : [],
|
||||
);
|
||||
if (MOCKS_I18NEXT.test(source) && !source.includes("i18nMock")) {
|
||||
if (MOCKS_I18NEXT.test(source) && !CALLS_I18NMOCK.test(withoutComments(source))) {
|
||||
found.push(`${name}: mocks react-i18next without i18nMock`);
|
||||
}
|
||||
return found;
|
||||
|
|
@ -66,10 +76,35 @@ describe("i18n mock guard (#389)", () => {
|
|||
'vi.mock("react-i18next", async () => (await import("../test/i18nMock")).i18nMock());',
|
||||
"const context = { useLicenseContext: () => ctx, get: (target: object) => target };",
|
||||
"await view.removeKey({ confirm, t: markedT });",
|
||||
"const at = (n: number) => n; const format = (v: string) => v;",
|
||||
"if (t === (other as unknown)) return; const run = (t) => t;",
|
||||
"const t = markedT; const set = { t: markedT };",
|
||||
].join("\n");
|
||||
expect(offences(resolve(SRC, "x.test.tsx"), clean)).toEqual([]);
|
||||
});
|
||||
|
||||
it("recognises a fake t bound to a name or built with vi.fn", () => {
|
||||
const x = resolve(SRC, "x.test.ts");
|
||||
expect(offences(x, "const t = (key: string) => key;")).toHaveLength(1);
|
||||
expect(offences(x, "let t = (key: string, o?: object) => key;")).toHaveLength(1);
|
||||
expect(offences(x, " t = (key: string) => key;")).toHaveLength(1);
|
||||
expect(offences(x, "const t = vi.fn((key: string) => key);")).toHaveLength(1);
|
||||
expect(offences(x, " useT: () => ({ t: vi.fn((k: string) => k) }),")).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("requires an actual call to i18nMock(, not its name in a comment", () => {
|
||||
const x = resolve(SRC, "x.test.tsx");
|
||||
const inComment = [
|
||||
"// Not i18nMock(): a hand-written module.",
|
||||
"/* i18nMock( would be the pattern */",
|
||||
'vi.mock("react-i18next", () => fake);',
|
||||
].join("\n");
|
||||
expect(offences(x, inComment)).toEqual(["x.test.tsx: mocks react-i18next without i18nMock"]);
|
||||
expect(offences(x, '// i18nMock is the rule\nvi.mock("react-i18next", () => fake);')).toHaveLength(1);
|
||||
const called = 'vi.mock("react-i18next", async () => (await import("../test/i18nMock")).i18nMock({ language: "en" }));';
|
||||
expect(offences(x, called)).toEqual([]);
|
||||
});
|
||||
|
||||
it("finds no other fake t than the one of src/test/i18nMock.ts", () => {
|
||||
const found = TEST_FILES.flatMap((path) => offences(path, readFileSync(path, "utf-8")));
|
||||
expect(found, "use i18nMock from src/test/i18nMock.ts").toEqual([]);
|
||||
|
|
|
|||
|
|
@ -1,17 +1,16 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import { markedT as t } from "../test/i18nMock";
|
||||
import {
|
||||
renderCategoryLabel,
|
||||
renderCategoryLabelFromCategory,
|
||||
renderCategoryLabelFromAccount,
|
||||
} from "./renderCategoryLabel";
|
||||
|
||||
// Fake `t`: echoes the key prefixed, but honours defaultValue for unknown keys
|
||||
// the way react-i18next does. We model "known" keys explicitly.
|
||||
const known: Record<string, string> = {
|
||||
"balance.category.cash": "Liquidités",
|
||||
};
|
||||
const t = (key: string, options?: { defaultValue?: string }) =>
|
||||
known[key] ?? options?.defaultValue ?? key;
|
||||
// `t` is the marked one of src/test/i18nMock.ts (#389): a translation reads
|
||||
// `t(key[defaultValue=…])`, so a label that skipped `t` fails. Falling back to
|
||||
// `defaultValue` for an unknown key is i18next's job; the helper's part is to
|
||||
// pass the raw key as that default, which the mark shows.
|
||||
const CASH = "t(balance.category.cash[defaultValue=cash])";
|
||||
|
||||
describe("renderCategoryLabel", () => {
|
||||
it("prefers a non-empty custom_label over the i18n translation", () => {
|
||||
|
|
@ -23,25 +22,25 @@ describe("renderCategoryLabel", () => {
|
|||
it("trims custom_label and treats whitespace-only as absent", () => {
|
||||
expect(
|
||||
renderCategoryLabel(" ", "balance.category.cash", "cash", t)
|
||||
).toBe("Liquidités");
|
||||
).toBe(CASH);
|
||||
});
|
||||
|
||||
it("falls back to the i18n translation when custom_label is null", () => {
|
||||
expect(
|
||||
renderCategoryLabel(null, "balance.category.cash", "cash", t)
|
||||
).toBe("Liquidités");
|
||||
).toBe(CASH);
|
||||
});
|
||||
|
||||
it("falls back to the i18n translation when custom_label is undefined", () => {
|
||||
expect(
|
||||
renderCategoryLabel(undefined, "balance.category.cash", "cash", t)
|
||||
).toBe("Liquidités");
|
||||
).toBe(CASH);
|
||||
});
|
||||
|
||||
it("falls back to the raw key when no translation exists", () => {
|
||||
it("passes the raw key as the default i18next falls back to", () => {
|
||||
expect(
|
||||
renderCategoryLabel(null, "balance.category.unknown", "unknown", t)
|
||||
).toBe("unknown");
|
||||
).toBe("t(balance.category.unknown[defaultValue=unknown])");
|
||||
});
|
||||
|
||||
it("returns a trimmed custom_label (leading/trailing spaces removed)", () => {
|
||||
|
|
@ -58,7 +57,7 @@ describe("renderCategoryLabelFromCategory", () => {
|
|||
{ custom_label: null, i18n_key: "balance.category.cash", key: "cash" },
|
||||
t
|
||||
)
|
||||
).toBe("Liquidités");
|
||||
).toBe(CASH);
|
||||
expect(
|
||||
renderCategoryLabelFromCategory(
|
||||
{ custom_label: "X", i18n_key: "balance.category.cash", key: "cash" },
|
||||
|
|
@ -79,7 +78,7 @@ describe("renderCategoryLabelFromAccount", () => {
|
|||
},
|
||||
t
|
||||
)
|
||||
).toBe("Liquidités");
|
||||
).toBe(CASH);
|
||||
expect(
|
||||
renderCategoryLabelFromAccount(
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in a new issue