Skip to content

test(core): drop duplicate gitdiff untracked count case - #5468

Merged
wenshao merged 1 commit into
QwenLM:mainfrom
tt-a1i:test/gitdiff-drop-duplicate-untracked-count
Jun 20, 2026
Merged

test(core): drop duplicate gitdiff untracked count case#5468
wenshao merged 1 commit into
QwenLM:mainfrom
tt-a1i:test/gitdiff-drop-duplicate-untracked-count

Conversation

@tt-a1i

@tt-a1i tt-a1i commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • remove the duplicate fetchGitDiff untracked count case that repeats coverage from the stronger line-count aggregation test
  • keep the slow-path regression test that verifies >MAX_FILES untracked line counts

Fixes #5467

Test plan

  • npm --workspace packages/core run test -- src/utils/gitDiff.test.ts
  • npx prettier --check packages/core/src/utils/gitDiff.test.ts
  • npx eslint packages/core/src/utils/gitDiff.test.ts
  • git diff --check

AI Assistance Disclosure

I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.

@wenshao

wenshao commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator

✅ Verification — dropping the duplicate test loses no coverage

This PR removes a single test from gitDiff.test.ts (counts untracked files in filesCount even after the per-file map is full), claiming it duplicates an existing case. I confirmed that by running the real test suite + mutation testing in the worktree (Node v22, vitest 3.2.4, head c8a07748). Posting as a merge reference.

TL;DR

  • Suite is green before and after: main 59 → PR 58 tests, i.e. exactly the one removed case, no other change.
  • The removed test's assertions are a strict subset of the retained test aggregates untracked line counts into linesAdded even when the per-file map is full of tracked entries (gitDiff.test.ts:1226) — same setup, same assertions, plus an extra linesAdded check.
  • Mutation testing proves the retained test still catches every behavior the removed one guarded.

1) Coverage equivalence (static)

Both tests use the identical setup — fill the per-file map with MAX_FILES tracked modifications, then add untracked files — and assert:

Assertion Removed test Retained aggregates… (line 1226)
untracked counted after map full filesCount === MAX_FILES + 3 filesCount === MAX_FILES + 5
per-file map stays capped perFileStats.size === MAX_FILES perFileStats.size === MAX_FILES ✅ (identical)
line totals linesAdded === … (extra)

The removed case adds nothing the retained one doesn't already assert (only 3 vs 5 untracked files — not a distinct scenario).

2) Mutation testing (the real proof — runs the PR-state suite with the removed test gone)

I injected two faults into gitDiff.ts and ran the PR test file (the duplicate already deleted) to confirm a retained test still fails:

Mutation Source change Result on PR suite
A — break "count untracked after map full" stats.filesCount += untrackedCount+= 0 (line 229) 4 retained tests fail, incl. aggregates…expected 50 to be 55
B — break per-file cap for untracked-after-full remainingSlots = Math.max(0, MAX_FILES - size) → unbounded (line 251) aggregates… fails → expected 55 to be 50 + slow-path test

Each assertion the removed test made is independently caught by a retained test. The direct duplicate aggregates… catches both mutations — exactly as the removed test would have. After restoring the source, the suite is green again (58/58).

3) Live run (tmux)

✓ fetchGitDiff untracked counting > aggregates untracked line counts into linesAdded even when the per-file map is full of tracked entries
✓ fetchGitDiff untracked counting > line-counts every untracked file in the slow path, not just the first MAX_FILES
  Test Files  1 passed (1)
        Tests  58 passed (58)

Verdict

The removed test is a genuine duplicate, the behavior it guarded stays fully covered by the retained aggregates… test (verified by mutation, not just by reading), and the suite is green. LGTM — good to merge. 👍

🇨🇳 中文版(点击展开)

✅ 验证 —— 删除这个重复测试不会丢失任何覆盖

本 PR 从 gitDiff.test.ts 删除了一个测试(counts untracked files in filesCount even after the per-file map is full),理由是它与已有用例重复。我在 worktree 里通过**运行真实测试套件 + 变异测试(mutation testing)**确认了这一点(Node v22,vitest 3.2.4,head c8a07748)。作为 merge 参考。

结论速览

  • 删除前后套件都是绿的:main 59 → PR 58 个测试,恰好少了被删的那一个,没有别的变化。
  • 被删测试的断言是保留测试 aggregates untracked line counts into linesAdded even when the per-file map is full of tracked entriesgitDiff.test.ts:1226)的严格子集 —— 相同的构造、相同的断言,还多了一个 linesAdded 检查。
  • 变异测试证明:被删测试所保护的每一个行为,保留测试依然能抓到。

1)覆盖等价性(静态)

两个测试用的是完全相同的构造 —— 先用 MAX_FILES 个 tracked 改动把 per-file map 填满,再加 untracked 文件 —— 并断言:

断言 被删测试 保留的 aggregates…(行 1226)
map 满了之后 untracked 仍被计数 filesCount === MAX_FILES + 3 filesCount === MAX_FILES + 5
per-file map 保持封顶 perFileStats.size === MAX_FILES perFileStats.size === MAX_FILES ✅(完全一致)
行数合计 —— linesAdded === …(额外)

被删用例没有任何保留用例没断言到的东西(只是 untracked 文件数 3 vs 5 —— 并非不同场景)。

2)变异测试(真正的证明 —— 在已删除该测试的 PR 状态下运行套件)

我往 gitDiff.ts 注入了两个 fault,然后运行 PR 版测试文件(重复测试已删),确认仍有保留测试失败:

变异 源码改动 在 PR 套件上的结果
A —— 破坏"map 满后仍计数 untracked" stats.filesCount += untrackedCount+= 0(行 229) 4 个保留测试失败,含 aggregates…expected 50 to be 55
B —— 破坏 untracked-after-full 的 per-file 封顶 remainingSlots = Math.max(0, MAX_FILES - size) → 不封顶(行 251) aggregates… 失败 → expected 55 to be 50 + 慢路径测试

被删测试做出的每一个断言,都有保留测试独立抓到。那个直接重复的 aggregates… 测试两个变异都抓到了 —— 与被删测试本会做的完全一致。恢复源码后,套件重新全绿(58/58)。

3)实时运行(tmux)

✓ fetchGitDiff untracked counting > aggregates untracked line counts into linesAdded even when the per-file map is full of tracked entries
✓ fetchGitDiff untracked counting > line-counts every untracked file in the slow path, not just the first MAX_FILES
  Test Files  1 passed (1)
        Tests  58 passed (58)

结论

被删测试确为重复用例,它所保护的行为仍被保留的 aggregates… 测试完整覆盖(用变异测试证明,而不仅是肉眼比对),套件全绿。LGTM —— 可以合并。 👍

@wenshao
wenshao marked this pull request as ready for review June 20, 2026 10:29
@wenshao

wenshao commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. The removed test counts untracked files in filesCount even after the per-file map is full is fully covered by the adjacent aggregates untracked line counts into linesAdded even when the per-file map is full of tracked entries test, which asserts the same filesCount and perFileStats.size invariants plus linesAdded — a strict superset. LGTM! ✅

— DeepSeek/deepseek-v4-pro via Qwen Code /review

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR, @tt-a1i!

Template: headings deviate from the PR template — "Summary" instead of "What this PR does", "Test plan" instead of "Reviewer Test Plan", and "Why it's needed", "Risk & Scope", and "Tested on" are absent. The test commands are clear enough to verify the claim, so not blocking on this, but worth aligning with the template for future PRs.

On direction: straightforward — removing a genuinely duplicate test case is squarely in scope for test health. Nothing to question here.

On approach: the scope is minimal (1 file, −22 lines, 0 additions). The removed test counts untracked files in filesCount even after the per-file map is full asserts filesCount and perFileStats.size after filling the map with MAX_FILES tracked entries — both assertions are a strict subset of the retained aggregates untracked line counts… test (which adds a linesAdded check on top of the same setup). The only difference is 3 vs 5 untracked files, which isn't a distinct scenario. No unrelated changes, no scope creep.

Moving on to code review and testing. 🔍

中文说明

感谢 @tt-a1i 的 PR!

模板:标题与 PR 模板 有偏差 —— "Summary" 而非 "What this PR does","Test plan" 而非 "Reviewer Test Plan","Why it's needed"、"Risk & Scope"、"Tested on" 均未出现。测试命令清晰可验证,不作为阻塞项,但建议后续 PR 对齐模板。

方向:明确 —— 删除重复测试用例属于测试健康维护,完全在范围内。

方案:范围最小(1 个文件,−22 行,0 新增)。被删测试 counts untracked files in filesCount even after the per-file map is full 断言 filesCountperFileStats.size,均为保留测试 aggregates untracked line counts… 的严格子集(后者额外检查了 linesAdded)。唯一差异是 3 vs 5 个 untracked 文件,不构成不同场景。无无关改动,无范围蔓延。

进入代码审查和测试。🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code review

I wrote down my independent proposal before reading the diff: the counts untracked files in filesCount even after the per-file map is full test is a strict assertion-subset of aggregates untracked line counts into linesAdded even when the per-file map is full of tracked entries — same setup (fill per-file map with MAX_FILES tracked entries, add untracked files), same filesCount and perFileStats.size assertions, minus the linesAdded check. Removing it is the right call.

The PR matches my proposal exactly. The 22-line deletion is clean — no leftover references, no orphaned helpers, no dead imports. The it() block boundaries are respected, so no adjacent-test formatting damage. No AGENTS.md concerns.

Real-scenario testing

Ran npx vitest run src/utils/gitDiff.test.ts from packages/core on both main (before) and the PR patch (after).

Before (main — 59 tests including the duplicate)

 RUN  v3.2.4 /home/runner/work/qwen-code/qwen-code/packages/core
      Coverage enabled with v8

 ✓ src/utils/gitDiff.test.ts (59 tests) 1233ms

 Test Files  1 passed (1)
      Tests  59 passed (59)
   Start at  10:34:02
   Duration  5.29s (transform 161ms, setup 23ms, collect 192ms, tests 1.23s, environment 0ms, prepare 80ms)

After (this PR — 58 tests, duplicate removed)

 RUN  v3.2.4 /home/runner/work/qwen-code/qwen-code/packages/core
      Coverage enabled with v8

 ✓ src/utils/gitDiff.test.ts (58 tests) 1187ms

 Test Files  1 passed (1)
      Tests  58 passed (58)
   Start at  10:34:53
   Duration  5.29s (transform 158ms, setup 23ms, collect 188ms, tests 1.19s, environment 0ms, prepare 79ms)

Exactly 59 → 58 (one test removed), all green. The retained aggregates untracked line counts… test still exercises the same code paths the deleted test guarded.

中文说明

代码审查

在读取 diff 之前,我先写下了独立方案:counts untracked files in filesCount even after the per-file map is full 测试的断言是 aggregates untracked line counts into linesAdded even when the per-file map is full of tracked entries 的严格子集 —— 相同的构造(用 MAX_FILES 个 tracked 条目填满 per-file map,再添加 untracked 文件),相同的 filesCountperFileStats.size 断言,缺少 linesAdded 检查。删除是正确选择。

PR 与我的方案完全一致。22 行删除干净利落 —— 无残留引用、无孤立辅助函数、无死导入。it() 块边界完整,相邻测试格式无损伤。无 AGENTS.md 违反。

实际测试

packages/core 下分别运行 main(before)和 PR 补丁(after)的 npx vitest run src/utils/gitDiff.test.ts

Before(main — 59 个测试,含重复用例)

 RUN  v3.2.4 /home/runner/work/qwen-code/qwen-code/packages/core
      Coverage enabled with v8

 ✓ src/utils/gitDiff.test.ts (59 tests) 1233ms

 Test Files  1 passed (1)
      Tests  59 passed (59)
   Start at  10:34:02
   Duration  5.29s (transform 161ms, setup 23ms, collect 192ms, tests 1.23s, environment 0ms, prepare 80ms)

After(本 PR — 58 个测试,已删除重复用例)

 RUN  v3.2.4 /home/runner/work/qwen-code/qwen-code/packages/core
      Coverage enabled with v8

 ✓ src/utils/gitDiff.test.ts (58 tests) 1187ms

 Test Files  1 passed (1)
      Tests  58 passed (58)
   Start at  10:34:53
   Duration  5.29s (transform 158ms, setup 23ms, collect 188ms, tests 1.19s, environment 0ms, prepare 79ms)

恰好 59 → 58(删除一个测试),全绿。保留的 aggregates untracked line counts… 测试仍然覆盖了被删测试所保护的相同代码路径。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Stepping back: this is as clean as a test-deletion PR gets. My independent proposal before reading the diff was exactly what the PR does — remove the test whose assertions are a strict subset of the retained aggregates… case. The before/after confirms it: 59 → 58, all green, no orphaned code.

The template deviation noted in Stage 1 is minor and doesn't affect reviewability. The PR body clearly states what's being removed, why (duplicate coverage), and how to verify (test commands). That's sufficient.

Nothing in this PR needs rethinking. The deletion is correct, the motivation is sound, and the suite is healthier for it.

Approving. ✅

中文说明

回顾全局:这是一个尽可能干净的测试删除 PR。我在读 diff 之前写的独立方案与 PR 完全一致 —— 删除那个断言为保留 aggregates… 用例严格子集的测试。before/after 确认了这一点:59 → 58,全绿,无孤立代码。

Stage 1 提到的模板偏差较小,不影响可审查性。PR body 清楚说明了删除什么、为什么(重复覆盖)、如何验证(测试命令)。足够了。

没有需要重新考虑的地方。删除正确,动机合理,测试套件因此更健康。

批准。✅

Qwen Code · qwen3.7-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

@wenshao
wenshao merged commit c5d2b1e into QwenLM:main Jun 20, 2026
33 checks passed

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. LGTM! ✅

— DeepSeek/deepseek-v4-pro via Qwen Code /review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Duplicate gitDiff untracked counting test can time out on Windows

3 participants