From 7d0a796255020899177ecd2e7b4669a07bde16fa Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Tue, 23 Jun 2026 22:16:05 -0700 Subject: [PATCH] test: add test coverage for duplicate-path snapshot remapping - Add unit test verifying that duplicate-path snapshots do not remap to a retained fallback area. - Robustify E2E large diff freeze repro test by ensuring interval timers are always cleared in a finally block. - Standardize path utilities in large diff fixtures to ensure cross-platform compatibility. --- .../editor/combined-diff-entries.test.ts | 12 ++++++++++ tests/e2e/large-diff-freeze-repro.spec.ts | 23 ++++++++++++------- tests/e2e/large-diff-repro-fixtures.ts | 8 ++++--- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/src/renderer/src/components/editor/combined-diff-entries.test.ts b/src/renderer/src/components/editor/combined-diff-entries.test.ts index 4302c2bdfc6..c3ff00ecab0 100644 --- a/src/renderer/src/components/editor/combined-diff-entries.test.ts +++ b/src/renderer/src/components/editor/combined-diff-entries.test.ts @@ -145,6 +145,18 @@ describe('resolveCombinedUncommittedSnapshotEntries', () => { ) }) + it('does not remap duplicate-path snapshots to a retained fallback area', () => { + const snapshotEntries: GitStatusEntry[] = [ + { path: 'src/file.ts', status: 'modified', area: 'staged', added: 4 }, + { path: 'src/file.ts', status: 'modified', area: 'unstaged', added: 2 } + ] + const retained: GitStatusEntry[] = [{ path: 'src/file.ts', status: 'modified', area: 'staged' }] + + expect(resolveCombinedUncommittedSnapshotEntries(snapshotEntries, [], retained)).toEqual([ + { path: 'src/file.ts', status: 'modified', area: 'staged', added: 4 } + ]) + }) + it('does not let a retained stale area duplicate an original target area', () => { const snapshotEntries: GitStatusEntry[] = [ { path: 'src/file.ts', status: 'modified', area: 'unstaged', added: 2 }, diff --git a/tests/e2e/large-diff-freeze-repro.spec.ts b/tests/e2e/large-diff-freeze-repro.spec.ts index 3ad6bfd14ed..dc8292d5427 100644 --- a/tests/e2e/large-diff-freeze-repro.spec.ts +++ b/tests/e2e/large-diff-freeze-repro.spec.ts @@ -184,6 +184,8 @@ test.describe('Large diff freeze repro', () => { throw new Error(`staged locale fixture missing entries: ${missing.join(', ')}`) } + // Why: reproduce stale snapshot behavior by opening combined diffs + // as "unstaged" using entries captured from the staged status snapshot. const staleUnstagedEntries = entries.map((entry) => ({ ...entry, area: 'unstaged' })) const intervalMs = 50 const samples: number[] = [] @@ -202,17 +204,22 @@ test.describe('Large diff freeze repro', () => { let editorCount = 0 let fallbackCount = 0 - while (performance.now() - startedAt < 30_000) { - await new Promise((resolve) => window.setTimeout(resolve, 50)) - editorCount = document.querySelectorAll('.monaco-diff-editor').length - fallbackCount = document.querySelectorAll('[data-testid="large-diff-fallback"]').length - if (editorCount + fallbackCount >= Math.min(entries.length, 5)) { - await new Promise((resolve) => window.setTimeout(resolve, 1_000)) - break + try { + while (performance.now() - startedAt < 30_000) { + await new Promise((resolve) => window.setTimeout(resolve, 50)) + editorCount = document.querySelectorAll('.monaco-diff-editor').length + fallbackCount = document.querySelectorAll( + '[data-testid="large-diff-fallback"]' + ).length + if (editorCount + fallbackCount >= Math.min(entries.length, 5)) { + await new Promise((resolve) => window.setTimeout(resolve, 1_000)) + break + } } + } finally { + window.clearInterval(timer) } - window.clearInterval(timer) const classHits = Array.from( document.querySelectorAll( '.monaco-diff-editor .line-insert, .monaco-diff-editor .line-delete, .monaco-diff-editor .char-insert, .monaco-diff-editor .char-delete' diff --git a/tests/e2e/large-diff-repro-fixtures.ts b/tests/e2e/large-diff-repro-fixtures.ts index cbc93c6a585..fe1f803a46a 100644 --- a/tests/e2e/large-diff-repro-fixtures.ts +++ b/tests/e2e/large-diff-repro-fixtures.ts @@ -69,10 +69,12 @@ export function createIsolatedStagedLocaleDiffRepo(): IsolatedStagedLocaleDiffRe runGit(repoPath, ['config', 'user.name', 'E2E Test']) mkdirSync(path.join(repoPath, 'src', 'locales'), { recursive: true }) + const toAbsoluteFsPath = (relativePosixPath: string): string => + path.join(repoPath, ...relativePosixPath.split(path.posix.sep)) const relativePaths: string[] = [] for (let fileIndex = 0; fileIndex < 5; fileIndex += 1) { - const relativePath = `src/locales/locale-${fileIndex}.json` - const absolutePath = path.join(repoPath, ...relativePath.split('/')) + const relativePath = path.posix.join('src', 'locales', `locale-${fileIndex}.json`) + const absolutePath = toAbsoluteFsPath(relativePath) const original = buildLocaleLikeJson(fileIndex, 3600) writeFileSync(absolutePath, original) relativePaths.push(relativePath) @@ -81,7 +83,7 @@ export function createIsolatedStagedLocaleDiffRepo(): IsolatedStagedLocaleDiffRe runGit(repoPath, ['commit', '-m', 'Initial locale fixture']) for (let fileIndex = 0; fileIndex < relativePaths.length; fileIndex += 1) { - const absolutePath = path.join(repoPath, ...relativePaths[fileIndex].split('/')) + const absolutePath = toAbsoluteFsPath(relativePaths[fileIndex]) const original = buildLocaleLikeJson(fileIndex, 3600) writeFileSync(absolutePath, modifyLocaleLikeJson(original, fileIndex)) }