[Maintainer] i18n: a new locale no longer breaks release-check

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.

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,15 +793,36 @@ 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',
   })
   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"')

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<void> {
           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)