-
-
Notifications
You must be signed in to change notification settings - Fork 1
D-039: test: cover None branch of classify_span_change #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,7 @@ mod helpers; | |||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| use std::path::PathBuf; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| use commitbee::classify_diff_span; | ||||||||||||||||||||||||||||
| use commitbee::config::Config; | ||||||||||||||||||||||||||||
| use commitbee::domain::{ | ||||||||||||||||||||||||||||
| ChangeStatus, CodeSymbol, CommitType, FileCategory, IntentKind, SymbolKind, | ||||||||||||||||||||||||||||
|
|
@@ -1439,6 +1440,57 @@ fn whitespace_detection_returns_none_when_span_has_no_changes() { | |||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // Direct assertions on the `None` branch of `classify_span_change` via the | ||||||||||||||||||||||||||||
| // public `classify_diff_span` wrapper re-exported from `lib.rs`. The function | ||||||||||||||||||||||||||||
| // returns `None` exactly when no added/removed lines fall inside the requested | ||||||||||||||||||||||||||||
| // (`old_start..=old_end`) / (`new_start..=new_end`) spans — see the | ||||||||||||||||||||||||||||
| // `added_in_span.is_empty() && removed_in_span.is_empty()` guard in | ||||||||||||||||||||||||||||
| // `src/services/context.rs`. These tests pin that contract so a future | ||||||||||||||||||||||||||||
| // refactor cannot silently convert the "no changes in span" case into a | ||||||||||||||||||||||||||||
| // false-positive `Some(true)` whitespace-only classification. | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| #[test] | ||||||||||||||||||||||||||||
| fn classify_span_change_returns_none_when_span_is_outside_hunk() { | ||||||||||||||||||||||||||||
| // Hunk touches lines 1-3 in both old and new files, but we query a | ||||||||||||||||||||||||||||
| // symbol span at lines 50-60. No +/- line lands inside the span, so | ||||||||||||||||||||||||||||
| // the function must short-circuit to `None` before the whitespace | ||||||||||||||||||||||||||||
| // comparison runs. | ||||||||||||||||||||||||||||
| let diff = "@@ -1,3 +1,3 @@\n fn other() {\n- old()\n+ new()\n }\n"; | ||||||||||||||||||||||||||||
| assert_eq!( | ||||||||||||||||||||||||||||
| classify_diff_span(diff, 50, 60, 50, 60), | ||||||||||||||||||||||||||||
| None, | ||||||||||||||||||||||||||||
| "span entirely outside the hunk must yield None" | ||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| #[test] | ||||||||||||||||||||||||||||
| fn classify_span_change_returns_none_for_empty_diff() { | ||||||||||||||||||||||||||||
| // Empty diff: the parse loop never enters a hunk, so both | ||||||||||||||||||||||||||||
| // `added_in_span` and `removed_in_span` stay empty and the function | ||||||||||||||||||||||||||||
| // hits the early-return `None`. | ||||||||||||||||||||||||||||
| assert_eq!( | ||||||||||||||||||||||||||||
| classify_diff_span("", 1, 10, 1, 10), | ||||||||||||||||||||||||||||
| None, | ||||||||||||||||||||||||||||
| "empty diff must yield None" | ||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| #[test] | ||||||||||||||||||||||||||||
| fn classify_span_change_returns_none_when_span_range_is_empty() { | ||||||||||||||||||||||||||||
| // Span with `new_start > new_end` (and likewise for `old`) never | ||||||||||||||||||||||||||||
| // matches any +/- line because `in_new_span`/`in_old_span` evaluate | ||||||||||||||||||||||||||||
| // `false` for every counter value. This is a degenerate but reachable | ||||||||||||||||||||||||||||
| // input from callers that derive span bounds from AST nodes whose | ||||||||||||||||||||||||||||
| // `end < start` (e.g. zero-length nodes), so the `None` short-circuit | ||||||||||||||||||||||||||||
| // must hold here too. | ||||||||||||||||||||||||||||
| let diff = "@@ -1,3 +1,3 @@\n fn f() {\n- old()\n+ new()\n }\n"; | ||||||||||||||||||||||||||||
| assert_eq!( | ||||||||||||||||||||||||||||
| classify_diff_span(diff, 100, 50, 100, 50), | ||||||||||||||||||||||||||||
|
Comment on lines
+1482
to
+1488
|
||||||||||||||||||||||||||||
| // `false` for every counter value. This is a degenerate but reachable | |
| // input from callers that derive span bounds from AST nodes whose | |
| // `end < start` (e.g. zero-length nodes), so the `None` short-circuit | |
| // must hold here too. | |
| let diff = "@@ -1,3 +1,3 @@\n fn f() {\n- old()\n+ new()\n }\n"; | |
| assert_eq!( | |
| classify_diff_span(diff, 100, 50, 100, 50), | |
| // `false` for every counter value. Keep the inverted bounds within the | |
| // hunk's 1-3 line range so this specifically validates the inverted-span | |
| // behavior rather than the separate "outside hunk" case covered above. | |
| let diff = "@@ -1,3 +1,3 @@\n fn f() {\n- old()\n+ new()\n }\n"; | |
| assert_eq!( | |
| classify_diff_span(diff, 3, 1, 3, 1), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment describing the
classify_span_changecontract lists the span order as(old_start..=old_end) / (new_start..=new_end), but the actual API (and implementation inContextBuilder::classify_span_change) takes/usesnew_start..=new_endfor added lines andold_start..=old_endfor removed lines. Please update the comment to match the parameter order to avoid misleading future readers.