D-037: test: cover ChangeStatus Deleted and Renamed variants#7
D-037: test: cover ChangeStatus Deleted and Renamed variants#7Sephyi merged 2 commits intodevelopmentfrom
Conversation
Per audit D-037, the test suite never constructed `ChangeStatus::Deleted`
or meaningfully exercised `ChangeStatus::Renamed` through the splitter or
context builder. This closes that gap:
tests/helpers.rs
- Extract `make_renamed_file_with_diff` so rename fixtures can carry a
non-empty diff body plus explicit add/delete counts. The original
`make_renamed_file` becomes a thin wrapper for existing callers.
tests/context.rs
- Extend `format_files_shows_renamed_marker` to assert the old path,
the `->` arrow, and the new path all appear in `file_breakdown` —
proving `old_path` is respected, not silently dropped.
- Add `format_files_mixed_statuses_show_all_markers`: one FileChange
per variant in a single StagedChanges, asserting every `[+]`/`[M]`/
`[-]`/`[R]` prefix and every `change_summary` count fires.
tests/splitter.rs
- `deleted_file_is_represented_in_splitter_output`: a deleted file
colocated with a sibling modification must land in exactly one group
(or the SingleCommit branch) — never dropped.
- `deleted_file_is_placed_into_a_splitter_group`: with a symbol-bearing
addition in an unrelated module, the deletion still lands in exactly
one split group.
- `renamed_file_grouped_by_new_path_module`: the splitter must key
module detection off the new path, not `old_path`.
- `renamed_file_is_placed_into_a_splitter_group_with_old_path_preserved`:
splitter output references the rename by its new path, and both
`old_path` and `rename_similarity` round-trip unchanged on the input.
Closes audit entry D-037 from #3.
There was a problem hiding this comment.
Pull request overview
This PR adds regression tests to ensure ChangeStatus::Deleted and ChangeStatus::Renamed are exercised in test fixtures, addressing audit item D-037 by covering splitter grouping and context formatting paths for these variants.
Changes:
- Add
CommitSplittertests that includeDeletedandRenamedFileChangefixtures and assert expected grouping/output inclusion. - Extend context formatting tests to verify deleted/renamed markers, including
old_path -> new_pathrendering and summary counts. - Enhance test helpers with a
make_renamed_file_with_diffconstructor to build rename fixtures with explicit diff bodies and add/delete stats.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tests/splitter.rs | Adds new splitter tests for Deleted/Renamed fixtures and grouping expectations. |
| tests/helpers.rs | Introduces make_renamed_file_with_diff and routes make_renamed_file through it. |
| tests/context.rs | Adds assertions and a new test to cover formatting/summary output for all ChangeStatus variants. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "src/services/llm/anthropic.rs", | ||
| ChangeStatus::Modified, | ||
| "@@ -0,0 +1,20 @@\n+pub fn brand_new_api() {}\n", | ||
| 20, |
| "@@ -0,0 +1,20 @@\n+pub fn brand_new_api() {}\n", | ||
| 20, |
| ChangeStatus::Deleted, | ||
| "@@ -1,10 +0,0 @@\n-pub fn retired() {}\n-pub fn also_retired() {}\n", | ||
| 0, | ||
| 10, |
| SplitSuggestion::SingleCommit => { | ||
| // Single-commit means all files fall into the one implicit group | ||
| // — the deleted file is still present in the input, so no further | ||
| // assertion is needed beyond verifying the input shape. | ||
| assert!(changes.files.iter().any(|f| f.path == deleted_path)); | ||
| } | ||
| } |
|
@copilot apply changes based on the comments in this thread |
…te guard - Fix deletions:10→2 in deleted_file_is_represented_in_splitter_output to match the two removed lines in the diff body; also fix hunk header - Replace SingleCommit fallback with panic so a regression that drops Deleted files from analysis would be caught (addresses reviewer comment) - Fix additions:20→1 in deleted_file_is_placed_into_a_splitter_group (diff has one added line, not 20) - Fix additions:20→1 in renamed_file_is_placed_into_a_splitter_group_with_old_path_preserved (same single-line diff mismatch) Agent-Logs-Url: https://github.com/Sephyi/commitbee/sessions/05eadcfc-8514-4ef1-8a44-c1cb63d1895e Co-authored-by: Sephyi <89100305+Sephyi@users.noreply.github.com>
Done in f9e547c. Four fixture inconsistencies addressed:
All 21 splitter tests pass. |
|
Thank you for your contribution! Before we can merge this PR, you need to sign the Contributor License Agreement. To sign, please reply with a comment containing exactly: I have read the CLA Document and I hereby sign the CLA. You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
Summary
test: cover ChangeStatus Deleted and Renamed variants.
Audit context
Closes audit entry D-037 from #3.
Verification
cargo fmt --checkcargo clippy --all-targets --all-features -- -D warningscargo test --all-targets