diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 72f5c80..529896c 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,10 +1,8 @@ name: Release -# Publishing to npm does not happen here. Releases are published from GitLab, -# which is the source of truth and mirrors to GitHub; this workflow only -# records the GitHub Release for a tag. Do not add an `npm publish` step or an -# NPM_TOKEN secret to this repository — see the release beads for the -# GitLab-side pipeline. +# This workflow only records the GitHub Release for a tag. Publishing to npm +# happens from GitLab, so no `npm publish` step or NPM_TOKEN secret belongs in +# this repository. on: push: tags: @@ -46,8 +44,7 @@ jobs: echo "has_notes=true" >> "$GITHUB_OUTPUT" fi - # Skipped when the release already exists so that re-running the - # workflow does not fail on work that already succeeded. + # Skipped when the release exists, so re-runs are idempotent. - name: Create GitHub Release env: GH_TOKEN: ${{ github.token }} diff --git a/CHANGELOG.md b/CHANGELOG.md index 0989b31..8998198 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **sync**: `--auto-commit` now recognises its own translation output rather than only the files a given run wrote, which fixes two related problems. A translation left on disk by an earlier refused run was classed as an unrelated modification, so auto-commit refused forever and the user had to commit it by hand; it is now committed once the genuinely unrelated changes are dealt with. And in `--watch` mode, where the same path runs once per trigger, a trigger that translated nothing skipped the checks entirely and reported success while a commit was still owed. Staging is also driven by what is actually dirty, so a rewrite that produced identical bytes no longer attempts an empty commit. Ownership is derived from the lockfile's tracked source files, with each file matched to its own bucket so one bucket's `target_path_pattern` cannot claim another's output. - **sync**: Re-running `deepl sync --auto-commit` after a refusal no longer reports success without committing. A refused run still writes its translations to disk, so the retry found nothing to translate; the preflight was skipped in that case and the command exited 0 with no commit made, the translation still uncommitted and the tree still dirty. The repo-state checks now run whenever `--auto-commit` is requested, so the refusal is reported identically on every attempt. Staging and committing are still skipped when there is nothing to commit. Two tests covered this path but could not see the defect: their assertions ran only inside a `catch` the retry never entered, which also hid an assertion that could never have matched (it looked for "detached HEAD" against a message reading "HEAD is detached"). - **write**: `deepl write` now runs from a published install. `diff` is imported at the top of the write command but was declared only under `devDependencies`, which consumers do not install, so every command that loaded the module failed with `Cannot find package 'diff'` before producing output. `--help` did not surface it because the module loads lazily. - **init/write/sync**: `@inquirer/prompts` is declared as a dependency. It is imported at runtime by `init`, `write --interactive` and `sync init` while only the unused `inquirer` was declared, so it resolved through npm's hoisting: under a strict layout (pnpm, `--install-strategy=nested`) those commands failed with `ERR_MODULE_NOT_FOUND`, and under npm the prompt that reads the API key bound to whatever major another dependent happened to hoist. diff --git a/docs/SYNC.md b/docs/SYNC.md index 99c683e..d9b91ad 100644 --- a/docs/SYNC.md +++ b/docs/SYNC.md @@ -860,7 +860,7 @@ The `--auto-commit` flag stages translated files and the lock file (when updated The `.deepl-sync.lock` file is only staged when this sync run actually wrote an updated lockfile; a no-op sync (all locales current) will not stage or commit. -**Combined with `--watch`.** `deepl sync --watch --auto-commit` commits on **every successful sync cycle**, not only the initial pre-watch sync. Each commit is gated by the same preflight checks listed above (clean tree, on a branch, not mid-rebase/merge/cherry-pick, files actually written). If preflight fails for a given cycle, that cycle throws and the watcher logs the error; subsequent cycles retry normally once the tree is clean again. +**Combined with `--watch`.** `deepl sync --watch --auto-commit` commits on **every successful sync cycle**, not only the initial pre-watch sync. Each commit is gated by the same preflight checks listed above (clean tree, on a branch, not mid-rebase/merge/cherry-pick). If preflight fails for a given cycle, that cycle throws and the watcher logs the error; subsequent cycles report the same refusal until the tree is clean again, at which point the pending translations are committed — including any left behind by an earlier refused cycle. A cycle with nothing to commit is silent. ### GitLab CI diff --git a/src/cli/commands/sync-command.ts b/src/cli/commands/sync-command.ts index 19d5f2c..e25da13 100644 --- a/src/cli/commands/sync-command.ts +++ b/src/cli/commands/sync-command.ts @@ -384,7 +384,7 @@ export class SyncCommand { backupTracker, }); this.displayResult(result, 'text'); - if (options.autoCommit && result.success && !result.dryRun && !result.driftDetected && result.fileResults.length > 0) { + if (options.autoCommit && result.success && !result.dryRun && !result.driftDetected) { await this.autoCommitTranslations(result, activeConfig); } }, @@ -426,6 +426,46 @@ export class SyncCommand { }); } + /** + * Every relative path this config's buckets can write, mapped to its locale. + * Source files come from the lockfile and are matched to their own bucket, so + * one bucket's target_path_pattern cannot claim another bucket's output. + */ + private async resolveOwnedTargetPaths( + config: ResolvedSyncConfig, + ): Promise> { + const pathMod = await import('path'); + const { minimatch } = await import('minimatch'); + const { SyncLockManager } = await import('../../sync/sync-lock.js'); + const { resolveTargetPath } = await import('../../sync/sync-utils.js'); + + const owned = new Map(); + const lockManager = new SyncLockManager(pathMod.join(config.projectRoot, LOCK_FILE_NAME)); + const lockFile = await lockManager.read(); + const sourcePaths = Object.keys(lockFile.entries); + + for (const bucketConfig of Object.values(config.buckets)) { + for (const sourcePath of sourcePaths) { + if (!bucketConfig.include.some(pattern => minimatch(sourcePath, pattern))) continue; + for (const locale of config.target_locales) { + try { + const targetPath = resolveTargetPath( + sourcePath, + config.source_locale, + locale, + bucketConfig.target_path_pattern, + ); + owned.set(targetPath, locale); + } catch { + // No derivable target: the path does not carry the source locale. + } + } + } + } + + return owned; + } + private async autoCommitTranslations(result: SyncResult, config: ResolvedSyncConfig): Promise { const { execFile } = await import('child_process'); const { promisify } = await import('util'); @@ -445,16 +485,16 @@ export class SyncCommand { .filter(r => r.written) .map(r => r.file); - // Preflight: refuse to auto-commit in unsafe repo states. We want to avoid - // bundling unrelated work into the chore(i18n) commit and to avoid - // committing onto the wrong ref. + // Preflight: refuse to auto-commit in unsafe repo states, so unrelated work + // is never bundled into the chore(i18n) commit and nothing lands on the + // wrong ref. The checks run even when this sync wrote nothing, because a + // refusal leaves translations on disk that are still owed a commit. // - // These run even when this sync wrote nothing. A refused run still leaves - // its translations on disk, so the retry finds nothing to translate — and - // skipping the checks there reported success while no commit had been made - // and the tree was still dirty. - const expectedStaged = new Set(writtenFiles); - if (result.lockUpdated) expectedStaged.add(LOCK_FILE_NAME); + // Anything this config can write counts as expected, not only what this run + // wrote: a translation from an earlier refusal belongs to the next commit + // rather than being an unrelated modification. + const ownedPaths = await this.resolveOwnedTargetPaths(config); + const expectedStaged = new Set([...writtenFiles, ...ownedPaths.keys(), LOCK_FILE_NAME]); // 1. In-progress rebase/merge/cherry-pick const gitDir = (await execFileAsync('git', ['rev-parse', '--git-dir'], { cwd })).stdout.trim(); @@ -488,6 +528,7 @@ export class SyncCommand { // that aren't part of this sync run). const { stdout: porcelain } = await execFileAsync('git', ['status', '--porcelain', '-z'], { cwd }); const unrelated: string[] = []; + const dirtyOwned: string[] = []; if (porcelain.length > 0) { const parts = porcelain.split('\0').filter(Boolean); for (const part of parts) { @@ -498,7 +539,10 @@ export class SyncCommand { const statusCode = part.slice(0, 2); const filePath = part.slice(3); if (!filePath) continue; - if (expectedStaged.has(filePath)) continue; + if (expectedStaged.has(filePath)) { + dirtyOwned.push(filePath); + continue; + } // Untracked files with the exact target path are already filtered by // the expectedStaged check; everything else is unrelated. unrelated.push(`${filePath} (${statusCode.trim() || '??'})`); @@ -512,19 +556,22 @@ export class SyncCommand { ); } - if (writtenFiles.length === 0) return; - - const filesToStage: string[] = [...writtenFiles]; - if (result.lockUpdated && fsMod.existsSync(pathMod.join(cwd, LOCK_FILE_NAME))) { - filesToStage.push(LOCK_FILE_NAME); - } + // Dirt, not write records: a rewrite producing identical bytes has nothing + // to commit, while a translation awaiting a commit does. + const filesToStage = dirtyOwned; + if (filesToStage.length === 0) return; for (const file of filesToStage) { await execFileAsync('git', ['add', file], { cwd }); } - const locales = [...new Set(result.fileResults.map(r => r.locale))].join(', '); - const msg = `chore(i18n): sync translations for ${locales}`; + const localeOf = new Map(ownedPaths); + for (const fileResult of result.fileResults) localeOf.set(fileResult.file, fileResult.locale); + const locales = [...new Set(filesToStage.map(file => localeOf.get(file)).filter(Boolean))].join(', '); + + const msg = locales + ? `chore(i18n): sync translations for ${locales}` + : 'chore(i18n): sync translation lockfile'; await execFileAsync('git', ['commit', '-m', msg], { cwd }); Logger.info(`Auto-committed ${filesToStage.length} file(s): ${msg}`); } diff --git a/src/storage/config.ts b/src/storage/config.ts index 5118ae8..345d111 100644 --- a/src/storage/config.ts +++ b/src/storage/config.ts @@ -252,11 +252,10 @@ export class ConfigService { * Save configuration to disk */ private save(): void { - // The temp name is unpredictable and created exclusively: at a fixed path a - // pre-planted symlink would redirect this file — which holds the API key in - // plaintext — to a path of the planter's choosing, and the rename would - // then leave config.json as that symlink for every later write. `wx` fails - // rather than following anything already there. + // Unpredictable name, exclusive create. This file holds the API key in + // plaintext, and at a guessable path a planted symlink would redirect the + // write and survive the rename as config.json. `wx` fails instead of + // following whatever is already there. const tmpPath = `${this.configPath}.tmp.${process.pid}.${randomBytes(6).toString('hex')}`; try { const dir = path.dirname(this.configPath); diff --git a/src/sync/sync-instructions.ts b/src/sync/sync-instructions.ts index 4035fff..f3e457a 100644 --- a/src/sync/sync-instructions.ts +++ b/src/sync/sync-instructions.ts @@ -93,7 +93,7 @@ export function mergeInstructions( } // --------------------------------------------------------------------------- -// Phase 4 foundations: length-aware instructions +// Length-aware instructions // --------------------------------------------------------------------------- // Industry-standard approximations (IBM, W3C localization guidelines). diff --git a/tests/e2e/cli-http-options.e2e.test.ts b/tests/e2e/cli-http-options.e2e.test.ts index ee294dc..4cad014 100644 --- a/tests/e2e/cli-http-options.e2e.test.ts +++ b/tests/e2e/cli-http-options.e2e.test.ts @@ -1,7 +1,7 @@ /** * E2E Tests for the --timeout / --max-retries global options. - * Uses a server that accepts requests and never answers, so the client - * aborts locally — the case that used to re-submit a billable POST. + * Uses a server that accepts requests and never answers, so the client aborts + * locally — where retrying would re-submit a billable POST. */ import { spawn, ChildProcess } from 'child_process'; diff --git a/tests/e2e/cli-no-input.e2e.test.ts b/tests/e2e/cli-no-input.e2e.test.ts index 1b019fd..657ea16 100644 --- a/tests/e2e/cli-no-input.e2e.test.ts +++ b/tests/e2e/cli-no-input.e2e.test.ts @@ -36,9 +36,8 @@ describe('--no-input E2E', () => { } }); - // Without --no-input the wizard used to start prompting, then die at EOF - // with exit 1 and a Node unsettled-top-level-await warning. Docker without - // -it, CI, and piped invocations all take this path. + // The path taken by `docker run` without -it, by CI, and by piped + // invocations: the wizard must refuse rather than prompt into an EOF. it('should exit with code 6 when stdin is not a terminal', () => { expect.assertions(3); try { diff --git a/tests/hermetic-deepl.ts b/tests/hermetic-deepl.ts index ee7a18e..8861475 100644 --- a/tests/hermetic-deepl.ts +++ b/tests/hermetic-deepl.ts @@ -28,12 +28,11 @@ export default function installHermeticDeepl(): void { /** * Removes real credentials and the real config directory from the environment - * the whole suite inherits. + * the suite inherits. * - * nock cannot intercept across a process boundary, so any spawned CLI that - * inherits a working DEEPL_API_KEY reaches the live API: doing so spent real - * quota and wrote real responses into the developer's cache. Suites that need - * a key pass one explicitly, which still overrides this. + * nock cannot intercept across a process boundary, so a spawned CLI holding a + * working DEEPL_API_KEY reaches the live API and the real cache. Suites that + * need a key pass one explicitly, which still overrides this. */ export function isolateCredentialEnvironment(): void { delete process.env['DEEPL_API_KEY']; diff --git a/tests/integration/sync-auto-commit.integration.test.ts b/tests/integration/sync-auto-commit.integration.test.ts index 8581ea1..6961fd9 100644 --- a/tests/integration/sync-auto-commit.integration.test.ts +++ b/tests/integration/sync-auto-commit.integration.test.ts @@ -221,9 +221,8 @@ describeIfGit('SyncCommand auto-commit preflight', () => { } }); - // A refused run leaves its translations on disk, so the retry has nothing new - // to translate. The preflight used to be skipped in that case, and the retry - // reported success with no commit made and the tree still dirty. + // A refusal leaves translations on disk, so a retry has nothing new to + // translate — and must still refuse rather than report success. it('retry after a refusal refuses again instead of reporting success', async () => { expect.assertions(4); seedRepo(tmpDir); @@ -234,8 +233,7 @@ describeIfGit('SyncCommand auto-commit preflight', () => { const command = new SyncCommand(syncService); await expect(command.run({ autoCommit: true })).rejects.toThrow(ValidationError); - // The first run wrote the translation before refusing, so nothing is left - // to translate on the retry. + // Written before the refusal, so the retry finds nothing to translate. expect(fs.existsSync(path.join(tmpDir, 'locales', 'de.json'))).toBe(true); await expect(command.run({ autoCommit: true })).rejects.toThrow(/Refusing to auto-commit/i); @@ -244,6 +242,54 @@ describeIfGit('SyncCommand auto-commit preflight', () => { expect(git(tmpDir, ['log', '-1', '--pretty=%s']).trim()).toBe('initial'); }); + // A translation awaiting a commit is this sync's own output, not an unrelated + // modification, so it is committed once the tree is otherwise clean. + it('commits a translation left behind by an earlier refusal once the tree is otherwise clean', async () => { + expect.assertions(4); + seedRepo(tmpDir); + fs.writeFileSync(path.join(tmpDir, 'README.md'), '# changed\n', 'utf-8'); + const scope = seedTranslateMock(); + scope.persist(); + + const command = new SyncCommand(syncService); + await expect(command.run({ autoCommit: true })).rejects.toThrow(ValidationError); + expect(fs.existsSync(path.join(tmpDir, 'locales', 'de.json'))).toBe(true); + + // Deal with the unrelated change; the leftover translation remains. + git(tmpDir, ['checkout', '--', 'README.md']); + + await command.run({ autoCommit: true }); + + nock.cleanAll(); + expect(git(tmpDir, ['log', '-1', '--pretty=%s']).trim()).toContain('chore(i18n)'); + expect(git(tmpDir, ['status', '--porcelain']).trim()).toBe(''); + }); + + // Watch mode runs this path once per trigger: each must refuse while a commit + // is owed, and stay silent once nothing is. + it('refuses consistently across repeated triggers and stays quiet when there is nothing to commit', async () => { + expect.assertions(4); + seedRepo(tmpDir); + fs.writeFileSync(path.join(tmpDir, 'src.ts'), 'export {};\n', 'utf-8'); + const scope = seedTranslateMock(); + scope.persist(); + + const command = new SyncCommand(syncService); + await expect(command.run({ autoCommit: true })).rejects.toThrow(/Refusing to auto-commit/i); + await expect(command.run({ autoCommit: true })).rejects.toThrow(/Refusing to auto-commit/i); + + // Nothing left to commit: neither throws nor commits. + fs.unlinkSync(path.join(tmpDir, 'src.ts')); + git(tmpDir, ['add', '-A']); + git(tmpDir, ['commit', '-q', '-m', 'tidy']); + + await command.run({ autoCommit: true }); + + nock.cleanAll(); + expect(git(tmpDir, ['log', '-1', '--pretty=%s']).trim()).toBe('tidy'); + expect(git(tmpDir, ['status', '--porcelain']).trim()).toBe(''); + }); + it('clean repo, no lockfile update: auto-commit does not fail staging a nonexistent lockfile', async () => { seedRepo(tmpDir); const scope = seedTranslateMock(); diff --git a/tests/require-build.ts b/tests/require-build.ts index 65aa452..33cd3a1 100644 --- a/tests/require-build.ts +++ b/tests/require-build.ts @@ -1,9 +1,8 @@ /** - * Fails fast with an actionable message when tests run without a current - * build: dist/ is gitignored and `npm test` does not build, yet all three - * test tiers execute dist/cli/index.js. A missing build otherwise surfaces - * as hundreds of unrelated failures, and a stale one silently tests the - * previously built CLI rather than the current source. + * Fails fast when tests run without a current build. dist/ is gitignored and + * `npm test` does not build, yet all three tiers execute dist/cli/index.js: a + * missing build surfaces as hundreds of unrelated failures, and a stale one + * silently tests different source than the tree holds. */ import * as fs from 'fs'; diff --git a/tests/unit/cli/register-sync-init.test.ts b/tests/unit/cli/register-sync-init.test.ts index c1f6635..6e827d5 100644 --- a/tests/unit/cli/register-sync-init.test.ts +++ b/tests/unit/cli/register-sync-init.test.ts @@ -1,10 +1,9 @@ /** * Unit tests for `deepl sync init`'s flag vocabulary. * - * --source-locale / --target-locales are the only accepted spellings. The - * --source-lang / --target-langs aliases were removed at the 2.0 cut per the - * documented deprecation policy, so they must now be rejected outright rather - * than silently accepted. + * --source-locale / --target-locales are the only accepted spellings; the + * --source-lang / --target-langs aliases are rejected outright rather than + * silently accepted. */ import * as fs from 'fs'; diff --git a/tests/unit/config-service.test.ts b/tests/unit/config-service.test.ts index 8767b30..7f2c3b3 100644 --- a/tests/unit/config-service.test.ts +++ b/tests/unit/config-service.test.ts @@ -301,7 +301,7 @@ describe('ConfigService', () => { }); }); - describe('config directory permissions (Issue deepl-cli-99a)', () => { + describe('config directory permissions', () => { it('should create config directory with mode 0o700', () => { const uniqueDir = path.join(os.tmpdir(), `deepl-perm-test-${Date.now()}`); const configPath = path.join(uniqueDir, 'config.json'); diff --git a/tests/unit/docs/documented-surface.test.ts b/tests/unit/docs/documented-surface.test.ts index 24a1945..0872d9a 100644 --- a/tests/unit/docs/documented-surface.test.ts +++ b/tests/unit/docs/documented-surface.test.ts @@ -1,10 +1,7 @@ /** * Checks every `deepl …` invocation in the user-facing docs against the CLI's - * real surface, reported by the hidden `_describe` command. - * - * The docs were accurate at the 2.0.0 cut, but only because someone checked by - * hand. This makes a documented command or flag that no longer exists a test - * failure instead of something a reader discovers. + * real surface, reported by the hidden `_describe` command, so a documented + * command or flag that stops existing fails here rather than reaching a reader. */ import { execFileSync } from 'child_process'; diff --git a/tests/unit/formats-crlf.test.ts b/tests/unit/formats-crlf.test.ts index 8b4fad6..413ec27 100644 --- a/tests/unit/formats-crlf.test.ts +++ b/tests/unit/formats-crlf.test.ts @@ -1,11 +1,10 @@ /** * Tests that line-based parsers treat CRLF input the same as LF. * - * Every line-based parser split on '\n', leaving a trailing '\r' on each - * line, which then defeated the `$`-anchored patterns used to recognise - * entries. A Windows-authored resource file therefore extracted zero entries - * and `deepl sync status` reported 0% coverage with exit 0 — the file was - * invisible to the tool, and reconstruct would rewrite it from nothing. + * Splitting on '\n' alone leaves a trailing '\r' that defeats the `$`-anchored + * entry patterns, so a Windows-authored resource file extracts zero entries: + * `deepl sync status` reports 0% coverage with exit 0 and reconstruct rewrites + * the file from nothing. */ import { PoFormatParser } from '../../src/formats/po'; diff --git a/tests/unit/http-client.test.ts b/tests/unit/http-client.test.ts index c791569..b00c259 100644 --- a/tests/unit/http-client.test.ts +++ b/tests/unit/http-client.test.ts @@ -224,7 +224,7 @@ describe('HttpClient', () => { }); }); - describe('retry error wrapping (W6)', () => { + describe('retry error wrapping', () => { it('should throw NetworkError after retry exhaustion on 500', async () => { nock(baseUrl) .get('/v2/test') @@ -245,7 +245,7 @@ describe('HttpClient', () => { }); }); - describe('5xx error mapping (W7)', () => { + describe('5xx error mapping', () => { it('should map 500 to NetworkError with status in message', async () => { nock(baseUrl) .get('/v2/test') diff --git a/tests/unit/icu-surrounding-text.test.ts b/tests/unit/icu-surrounding-text.test.ts index 5847e33..0f086da 100644 --- a/tests/unit/icu-surrounding-text.test.ts +++ b/tests/unit/icu-surrounding-text.test.ts @@ -1,15 +1,14 @@ /** * Tests ICU preservation for messages with text around the ICU block. * - * ICU_DETECT_RE was `^`-anchored, so any text before the block meant the - * string was not recognised as ICU at all and the raw ICU syntax was sent to - * the MT engine as prose. Verified against the live DeepL API: + * Detection must not be `^`-anchored: text before the block would leave the + * string unrecognised as ICU, sending raw ICU syntax to the MT engine as prose. + * The engine then translates the keyword and selectors — * * in You have {count, plural, one {# item} other {# items}} in your cart. * out Sie haben {count, Plural, ein {# item} weiteres {# items}} in Ihrem Warenkorb. * - * The keyword and both selectors were translated, leaving a message with no - * valid format type and no `other` branch — structurally invalid ICU. + * leaving a message with no valid format type and no `other` branch. */ import { parseIcu } from '../../src/utils/icu-preservation'; diff --git a/tests/unit/services/batch-translation.test.ts b/tests/unit/services/batch-translation.test.ts index b37172b..d86cd5c 100644 --- a/tests/unit/services/batch-translation.test.ts +++ b/tests/unit/services/batch-translation.test.ts @@ -138,10 +138,6 @@ describe('BatchTranslationService', () => { expect(results.failed[0]?.error).toContain('Translation failed'); }); - // Note: We don't test p-limit's concurrency behavior as it's a well-tested - // third-party library. Concurrency limiting should be verified through manual - // testing: `deepl translate ./large-dir --concurrency 3` - it('should filter out unsupported file types', async () => { const files = [ path.join(testDir, 'file1.txt'), diff --git a/tests/unit/sync/sync-command.test.ts b/tests/unit/sync/sync-command.test.ts index c7568f9..60f7d3a 100644 --- a/tests/unit/sync/sync-command.test.ts +++ b/tests/unit/sync/sync-command.test.ts @@ -20,6 +20,9 @@ jest.mock('child_process', () => ({ execFile: mockExecFile, })); +/** NUL-separated "XY path" entries returned for `git status --porcelain -z`. */ +let mockPorcelain = ''; + const mockExistsSync = jest.fn((_p: string | URL) => false); jest.mock('fs', () => { const actual = jest.requireActual('fs'); @@ -603,9 +606,16 @@ describe('SyncCommand', () => { if (Array.isArray(args) && args[0] === 'rev-parse' && args[1] === '--git-dir') { stdout = '.git'; } + // Staging follows the dirty set, so a scenario that writes files must + // report them here — git cannot show a clean tree after a write. + if (Array.isArray(args) && args[0] === 'status') { + stdout = mockPorcelain; + } if (cb) cb(null, { stdout, stderr: '' }); }, ); + // Clean by default; tests expecting a commit declare their own dirt. + mockPorcelain = ''; mockExistsSync.mockReset(); mockExistsSync.mockImplementation((p: string | URL) => { // Lockfile exists by default so autoCommit stages it when lockUpdated=true. @@ -614,6 +624,7 @@ describe('SyncCommand', () => { }); it('should call git add and git commit when autoCommit=true and files were written', async () => { + mockPorcelain = ' M locales/de.json\0 M .deepl-sync.lock\0'; const result = makeResult({ fileResults: [writtenFile], lockUpdated: true, @@ -638,6 +649,8 @@ describe('SyncCommand', () => { }); it('should not stage .deepl-sync.lock when lockUpdated is false', async () => { + // Untouched lockfile is not dirty, so it must not be staged. + mockPorcelain = ' M locales/de.json\0'; const result = makeResult({ fileResults: [writtenFile], lockUpdated: false, @@ -655,6 +668,7 @@ describe('SyncCommand', () => { }); it('should pass cwd=config.projectRoot to every git invocation', async () => { + mockPorcelain = ' M locales/de.json\0 M .deepl-sync.lock\0'; const result = makeResult({ fileResults: [writtenFile], lockUpdated: true }); const mockService = createMockSyncService(result); const command = new SyncCommand(mockService); @@ -673,6 +687,7 @@ describe('SyncCommand', () => { }); it('should stage multiple written files and include all locales in commit message', async () => { + mockPorcelain = ' M locales/de.json\0 M locales/fr.json\0 M .deepl-sync.lock\0'; const writtenFileFr: SyncFileResult = { file: 'locales/fr.json', locale: 'fr', @@ -758,9 +773,8 @@ describe('SyncCommand', () => { expect(commitCalls).toHaveLength(0); }); - // The preflight runs even with nothing to commit, so that a retry after a - // refusal reports the refusal again rather than succeeding silently. Only - // staging and committing are skipped. + // Preflight runs even with nothing to commit; only staging and committing + // are skipped. it('should run the preflight but not commit when fileResults is empty', async () => { const result = makeResult({ fileResults: [] }); const mockService = createMockSyncService(result); @@ -804,6 +818,7 @@ describe('SyncCommand', () => { }); it('should log success message after committing', async () => { + mockPorcelain = ' M locales/de.json\0 M .deepl-sync.lock\0'; const result = makeResult({ fileResults: [writtenFile], lockUpdated: true }); const mockService = createMockSyncService(result); const command = new SyncCommand(mockService); @@ -954,6 +969,11 @@ describe('SyncCommand', () => { if (Array.isArray(args) && args[0] === 'rev-parse' && args[1] === '--git-dir') { stdout = '.git'; } + // Staging follows the dirty set, so a scenario that writes files must + // report them here — git cannot show a clean tree after a write. + if (Array.isArray(args) && args[0] === 'status') { + stdout = mockPorcelain; + } if (cb) cb(null, { stdout, stderr: '' }); }, ); @@ -972,6 +992,7 @@ describe('SyncCommand', () => { return mockWatcher; }); + mockPorcelain = ' M locales/de.json\0 M .deepl-sync.lock\0'; const result = makeResult({ fileResults: [ { file: 'locales/de.json', locale: 'de', translated: 5, skipped: 0, failed: 0, written: true }, diff --git a/tests/unit/voice-stream-session.test.ts b/tests/unit/voice-stream-session.test.ts index cb8e783..0c6ea32 100644 --- a/tests/unit/voice-stream-session.test.ts +++ b/tests/unit/voice-stream-session.test.ts @@ -416,7 +416,7 @@ describe('VoiceStreamSession', () => { mockClient.createWebSocket.mockImplementation(() => { const mockWs = new EventEmitter(); // Never open for writing, so the chunk pump parks waiting for a - // usable socket — the state that used to leak the generator. + // usable socket — the state in which the generator must not leak. mockWs.readyState = 3; mockWs.send = jest.fn(); mockWs.close = jest.fn();