diff --git a/i18n/README.md b/i18n/README.md index 91f4bf245a..c88f972181 100644 --- a/i18n/README.md +++ b/i18n/README.md @@ -79,6 +79,9 @@ CI and releases allow drafts, never regressions: it falls back to `origin/main` if that is another commit with catalogs, then to the commit that introduced the catalogs. It never compares with a base that has no catalogs: with none available it fails and asks for `--base`. + A locale bundled after the base has no catalog there, and so no reviewed + entries to keep: the release job lists it as new and still scans its + compiled pack. It fails when a reviewed entry with the same English became a draft, or when a compiled pack contains any draft. Both jobs write the per-locale draft count, and the base used, to the job summary. diff --git a/scripts/i18n-literals.test.ts b/scripts/i18n-literals.test.ts index 029369db82..0fcd47a50d 100644 --- a/scripts/i18n-literals.test.ts +++ b/scripts/i18n-literals.test.ts @@ -1,5 +1,5 @@ -import { expect, test } from 'bun:test' -import { cpSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs' +import { afterEach, expect, test } from 'bun:test' +import { cpSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { compileMessageOrThrow } from '@lingui/message-utils/compileMessage' @@ -679,10 +679,16 @@ test('extraction never approves a newly filled translation by itself', () => { }) const LOCALES_DIR = 'packages/editor/src/i18n/locales' +const temporaryRoots: string[] = [] + +afterEach(() => { + for (const path of temporaryRoots.splice(0)) rmSync(path, { recursive: true, force: true }) +}) /** A copy of the real catalogs to run the release job against, edited per case. */ function catalogFixture(edit: (root: string) => void): { current: string; base: string } { const root = mkdtempSync(join(tmpdir(), 'pascal-i18n-')) + temporaryRoots.push(root) const base = join(root, 'base') const current = join(root, 'current') cpSync(LOCALES_DIR, base, { recursive: true }) @@ -715,6 +721,22 @@ function markDraft(root: string, id: string): void { writeFileSync(path, stringifyPo(po, { foldLength: 0 })) } +/** Compiles one of the locale's drafts into its shipped pack. */ +function shipDraft(root: string, locale: string): void { + const path = join(root, `${locale}/messages.ts`) + const draftId = parsePo(readFileSync(join(root, `${locale}/messages.po`), 'utf8')).items.find( + (item) => item.flags.fuzzy, + )?.msgid + if (!draftId) throw new Error(`fixture has no ${locale} draft`) + writeFileSync( + path, + readFileSync(path, 'utf8').replace( + 'JSON.parse("{', + `JSON.parse("{\\"${draftId}\\":[\\"Draft\\"],`, + ), + ) +} + test('the release job passes on unchanged catalogs', () => { expect(releaseCheckJob(catalogFixture(() => {})).exitCode).toBe(0) }) @@ -730,32 +752,37 @@ test('the release job fails when a reviewed entry becomes a draft with the same }) test('the release job fails when a compiled pack ships a draft', () => { - const { exitCode, output } = releaseCheckJob( - catalogFixture((root) => { - const path = join(root, 'fr/messages.ts') - const pack = readFileSync(path, 'utf8') - const draftId = parsePo(readFileSync(join(root, 'fr/messages.po'), 'utf8')).items.find( - (item) => item.flags.fuzzy, - )?.msgid - if (!draftId) throw new Error('fixture has no draft') - writeFileSync( - path, - pack.replace('JSON.parse("{', `JSON.parse("{\\"${draftId}\\":[\\"Brouillon\\"],`), - ) - }), - ) + const { exitCode, output } = releaseCheckJob(catalogFixture((root) => shipDraft(root, 'fr'))) expect(exitCode).toBe(1) expect(output).toMatch(/fr: compiled pack ships unreviewed "[^"]+"/) }) -/** A throwaway repository with the real catalogs, for the default base lookup. */ +test('a base directory without catalogs fails the release job', () => { + const fixture = catalogFixture(() => {}) + const base = join(fixture.base, 'missing') + const { exitCode, output } = releaseCheckJob({ ...fixture, base }) + expect(exitCode).toBe(1) + expect(output).toContain(`--base-dir ${base} has no bundled PO catalogs to compare with`) +}) + +/** A throwaway repository with the real catalogs, for the release job's base lookup. */ function releaseRepo(steps: (git: (...args: string[]) => void, root: string) => void): string { const root = mkdtempSync(join(tmpdir(), 'pascal-release-')) + temporaryRoots.push(root) + // A global signing setup would sign every fixture commit (slow, or a prompt) + // and make `git tag` demand a message for a signed annotated tag. + const config = [ + 'user.name=Test', + 'user.email=test@example.com', + 'commit.gpgSign=false', + 'tag.gpgSign=false', + ] const git = (...args: string[]) => { - const result = Bun.spawnSync( - ['git', '-c', 'user.name=Test', '-c', 'user.email=test@example.com', ...args], - { cwd: root, stdout: 'ignore', stderr: 'pipe' }, - ) + const result = Bun.spawnSync(['git', ...config.flatMap((entry) => ['-c', entry]), ...args], { + cwd: root, + stdout: 'ignore', + stderr: 'pipe', + }) if (result.exitCode !== 0) throw new Error(`git ${args.join(' ')}: ${result.stderr}`) } git('init', '-q', '-b', 'main') @@ -766,8 +793,8 @@ function releaseRepo(steps: (git: (...args: string[]) => void, root: string) => const addCatalogs = (root: string) => cpSync(LOCALES_DIR, join(root, LOCALES_DIR), { recursive: true }) -function defaultReleaseCheck(root: string) { - const result = Bun.spawnSync(['bun', 'scripts/i18n.ts', 'release-check'], { +function repoReleaseCheck(root: string, ...args: string[]) { + const result = Bun.spawnSync(['bun', 'scripts/i18n.ts', 'release-check', ...args], { env: { ...process.env, I18N_ROOT: root, GITHUB_STEP_SUMMARY: '' }, stdout: 'pipe', stderr: 'pipe', @@ -775,6 +802,27 @@ function defaultReleaseCheck(root: string) { return { exitCode: result.exitCode, output: `${result.stdout}${result.stderr}` } } +/** + * A `pre-i18n` commit without catalogs, a release tagged before `locale` was + * bundled, then the commit that bundles it. + */ +function releaseBeforeLocale(locale: string): string { + return releaseRepo((git, root) => { + writeFileSync(join(root, 'README.md'), 'before i18n\n') + git('add', '.') + git('commit', '-q', '-m', 'before i18n') + git('tag', 'pre-i18n') + addCatalogs(root) + rmSync(join(root, LOCALES_DIR, locale), { recursive: true }) + git('add', '.') + git('commit', '-q', '-m', 'release') + git('tag', '@pascal-app/editor@9.9.9') + addCatalogs(root) + git('add', '.') + git('commit', '-q', '-m', `bundle ${locale}`) + }) +} + test('the release job compares with the latest editor release tag and fails on a reviewed-to-draft change', () => { const root = releaseRepo((git, root) => { addCatalogs(root) @@ -784,7 +832,7 @@ test('the release job compares with the latest editor release tag and fails on a git('tag', 'v0.0.1') markDraft(join(root, LOCALES_DIR), 'editor.buildPanel.advanced') }) - const { exitCode, output } = defaultReleaseCheck(root) + const { exitCode, output } = repoReleaseCheck(root) expect(output).toContain('Comparing with @pascal-app/editor@9.9.9') expect(exitCode).toBe(1) expect(output).toContain( @@ -806,7 +854,7 @@ test('a release tag without catalogs falls back to the commit that introduced th git('commit', '-q', '-m', 'later work') markDraft(join(root, LOCALES_DIR), 'editor.buildPanel.advanced') }) - const { exitCode, output } = defaultReleaseCheck(root) + const { exitCode, output } = repoReleaseCheck(root) expect(output).toContain( 'the commit that introduced the catalogs (@pascal-app/editor@1.0.0 has no catalogs)', ) @@ -820,11 +868,41 @@ test('with no base that has catalogs the release job fails loudly', () => { git('add', '.') git('commit', '-q', '-m', 'catalogs at HEAD only') }) - const { exitCode, output } = defaultReleaseCheck(root) + const { exitCode, output } = repoReleaseCheck(root) expect(exitCode).toBe(1) expect(output).toContain('release-check: no base with bundled PO catalogs') }) +test('a locale bundled since the latest release tag is new there and does not block the release', () => { + const { exitCode, output } = repoReleaseCheck(releaseBeforeLocale('ja')) + expect(output).toContain('Comparing with @pascal-app/editor@9.9.9') + expect(output).toContain( + 'New locales since @pascal-app/editor@9.9.9, with no reviewed entries to keep: ja', + ) + expect(exitCode).toBe(0) +}) + +test('with a new locale, the release still compares the other locales and scans every pack', () => { + const root = releaseBeforeLocale('ja') + markDraft(join(root, LOCALES_DIR), 'editor.buildPanel.advanced') + shipDraft(join(root, LOCALES_DIR), 'ja') + const { exitCode, output } = repoReleaseCheck(root) + expect(output).toContain('Comparing with @pascal-app/editor@9.9.9') + expect(exitCode).toBe(1) + expect(output).toContain( + 'fr: "editor.buildPanel.advanced" is reviewed on the base branch with the same English and must stay reviewed', + ) + expect(output).toMatch(/ja: compiled pack ships unreviewed "[^"]+"/) +}) + +test('an explicit base may predate a locale but not the catalogs', () => { + const root = releaseBeforeLocale('ja') + expect(repoReleaseCheck(root, '--base', '@pascal-app/editor@9.9.9').exitCode).toBe(0) + const { exitCode, output } = repoReleaseCheck(root, '--base', 'pre-i18n') + expect(exitCode).toBe(1) + expect(output).toContain('--base pre-i18n has no bundled PO catalogs to compare with') +}) + test('JSX text and attributes decode the HTML references the codebase uses, strictly', () => { expect(renderJsxText('2 × 3 > 5')).toBe('2 × 3 > 5') expect(renderJsxText('it's "done"')).toBe('it\'s "done"') diff --git a/scripts/i18n.ts b/scripts/i18n.ts index 2aaf291072..27b225c65b 100644 --- a/scripts/i18n.ts +++ b/scripts/i18n.ts @@ -1050,8 +1050,12 @@ export function releaseCheck( /** The package whose release carries the bundled catalogs. */ const CATALOG_PACKAGE = 'editor' +/** + * Whether `revision` postdates the catalogs. One catalog is enough: a locale + * bundled since then has none there, and so no reviewed entries to keep. + */ function hasCatalogs(revision: string): boolean { - return TARGET_LOCALES.every( + return TARGET_LOCALES.some( (locale) => readAtRevision(revision, catalogPath(locale)) !== undefined, ) } @@ -1203,6 +1207,19 @@ async function main(): Promise { return existsSync(file) ? readFileSync(file, 'utf8') : undefined }) : readLocaleEntries((path) => readAtRevision(revision!, path)) + const added = TARGET_LOCALES.filter((locale) => !base[locale]) + if (baseDir && added.length === TARGET_LOCALES.length) { + console.error( + `[i18n] release-check: --base-dir ${baseDir} has no bundled PO catalogs to compare with`, + ) + process.exitCode = 1 + return + } + if (added.length > 0) { + console.log( + `New locales since ${baseDir ?? resolved?.label}, with no reviewed entries to keep: ${added.join(', ')}`, + ) + } const sources = readSourceMessages() const { errors, warnings } = releaseCheck(sources, base) reportWarnings(warnings)