Skip to content

test: add unit tests for diff-view pure functions - #26

Merged
aakashrajput merged 2 commits into
KlaatAI:mainfrom
siddhanttiwari19:test/diff-view-unit-tests
Jul 19, 2026
Merged

test: add unit tests for diff-view pure functions#26
aakashrajput merged 2 commits into
KlaatAI:mainfrom
siddhanttiwari19:test/diff-view-unit-tests

Conversation

@siddhanttiwari19

@siddhanttiwari19 siddhanttiwari19 commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds unit tests for all five pure diff-building functions in src/screens/diff-view.ts,
following the conventions in apply-patch.test.ts (bun:test, describe/test/expect).

Coverage

  • buildEditDiff: prefix/suffix trimming, ctx=1 context window, MAX_LINES (24) cap, no-change, empty input, gutter line numbers on/off
  • buildMultiEditDiff: blank separator between hunks with correct content partitioning, combined cap
  • buildWriteDiff: all-added-lines sign, numbering from 1, empty file, large file cap
  • buildPatchDiff: add/delete/update ops, moveTo rename header, multi-op separator, empty input, cap
  • lineOf: exact match, first-line fallback, no-match (undefined), line 1, empty haystack
  • diffStat: add/del counts, zero counts, write-only diff, context lines excluded, empty array

How I verified it

  • bun test — 31/31 new tests pass, 73/73 across the full suite, 0 failures
  • bun run typecheck — no type errors
  • bun run build — clean build (0.34 MB, 70 modules)

Refs #4

@github-actions

Copy link
Copy Markdown
Contributor

🤖 KlaatAI Review Bot (powered by Klaatu, advisory only — a maintainer makes the real call)

Issue match
Strong match for issue #4. The PR covers all four functions the issue explicitly called out (buildEditDiff, buildWriteDiff, lineOf, diffStat) plus the unrequested-but-related buildMultiEditDiff. Edge cases the issue listed — prefix/suffix trimming, ctx=1 context window, MAX_LINES cap (24), all-added numbering from 1, exact match / first-line fallback / no-match, add/del counting — are all present. Gap: buildPatchDiff is listed in the issue's "What" section as one of the pure functions needing coverage and is not tested here.

Test coverage
Behavior covered per function is concrete and well-named — the MAX_LINES overflow marker text, the startLine gutter behavior, the blank-separator hunk delimiter, and the lineOf fallback are all asserted. Missing: any test for buildPatchDiff (the apply_patch diff builder). Also missing: the "no-change" case for buildEditDiff only asserts no +/- signs but doesn't verify that context lines are also absent or what the output looks like — a no-op diff returning context-only would still pass. The buildMultiEditDiff cap test uses <= 25 while the sibling tests use === 25, so a regression that drops the cap silently could still pass.

Correctness concerns

  • src/screens/diff-view.test.ts:46 — the overflow-marker regex /more line/ couples the test to the exact English string in the source; a translation/rewording would silently break this assertion.
  • src/screens/diff-view.test.ts:170 (diffStat empty array) and the no-change test rely on the same sign !== " " filter pattern — fine, but worth confirming context lines (sign === " ") are never counted by diffStat; the existing tests don't directly assert that.
  • buildMultiEditDiff hunk-separator test only checks one blank line exists somewhere, not that it sits between the two hunks or that +/- lines from hunk 1 don't bleed into hunk 2.

Verdict
Flag to human reviewer: missing buildPatchDiff coverage is the main gap against issue #4's acceptance criteria; otherwise ready to merge.

This is an automated review to help triage faster, not a gate. Nothing here blocks merging.

@aakashrajput

Copy link
Copy Markdown
Member

@siddhanttiwari19 can you please link the issue in this pr.

@siddhanttiwari19

Copy link
Copy Markdown
Contributor Author

Closes #4 — linked in the PR description. Also pushed an updated commit that adds buildPatchDiff coverage and addresses all the bot's feedback (fixed no-change assertion, ===25 cap, separator partitioning, context lines in diffStat). 73/73 tests passing. @aakashrajput

@aakashrajput

Copy link
Copy Markdown
Member

@siddhanttiwari19
Thanks a lot for your contribution! 🎉

Really appreciate you taking the time to improve KlaatCode. Your contribution helps make the project better for everyone.

If you haven't already, we'd love it if you could ⭐ star the repository and follow https://github.com/KlaatAI on GitHub to stay updated with new features and releases.

Feel free to keep using KlaatAI for your projects, and don't hesitate to open more issues, suggest ideas, or submit future PRs—we're always happy to have contributors like you in the community.

Looking forward to seeing your next contribution. Happy coding! 🚀

@aakashrajput
aakashrajput merged commit 17ea7cf into KlaatAI:main Jul 19, 2026
2 checks passed
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.

2 participants