test: add comprehensive unit tests for qlty-history crate #2380
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
❌ 2 blocking issues (9 total)
|
![qltysh[bot]](https://avatars.githubusercontent.com/in/890766?s=60&v=4){kind=small}
| fn test_compile_regexes_valid() { | ||
| let options = GitIndexOptions { | ||
| skip_paths: Some(r"\.git/.*".to_string()), | ||
| skip_commits_with_messages: Some(r"WIP|TODO".to_string()), |
|
|
||
| let messages_regex = skip_messages_regex.unwrap(); | ||
| assert!(messages_regex.is_match("WIP: work in progress")); | ||
| assert!(messages_regex.is_match("TODO: finish this")); |
Comment on lines
+228
to
+442
| fn parse_git_show_output( | ||
| &self, | ||
| lines: std::str::Lines, | ||
| commit: &mut Commit, | ||
| commit_diff: &mut CommitDiff, | ||
| skip_paths_regex: &Option<Regex>, | ||
| commit_num: usize, | ||
| ) -> Result<()> { | ||
| let mut current_file: Option<String> = None; | ||
| let mut line_change = LineChange { | ||
| sign: 0, | ||
| line_number_old: 0, | ||
| line_number_new: 0, | ||
| hunk_num: 0, | ||
| hunk_start_line_number_old: 0, | ||
| hunk_start_line_number_new: 0, | ||
| hunk_lines_added: 0, | ||
| hunk_lines_deleted: 0, | ||
| hunk_context: String::new(), | ||
| line: String::new(), | ||
| indent: 0, | ||
| line_type: LineType::Empty, | ||
| prev_commit_hash: String::new(), | ||
| prev_author: String::new(), | ||
| prev_time: None, | ||
| }; | ||
|
|
||
| let mut diff_size = 0; | ||
|
|
||
| for line in lines { | ||
| if line.starts_with(':') { | ||
| let parts: Vec<&str> = line.split_whitespace().collect(); | ||
| if parts.len() >= 5 { | ||
| let change_type_char = parts[4].chars().next().unwrap_or('?'); | ||
| let (change_type, path, old_path) = match change_type_char { | ||
| 'A' => { | ||
| commit.files_added += 1; | ||
| ( | ||
| FileChangeType::Add, | ||
| parts.get(5).unwrap_or(&"").to_string(), | ||
| String::new(), | ||
| ) | ||
| } | ||
| 'D' => { | ||
| commit.files_deleted += 1; | ||
| ( | ||
| FileChangeType::Delete, | ||
| parts.get(5).unwrap_or(&"").to_string(), | ||
| String::new(), | ||
| ) | ||
| } | ||
| 'M' => { | ||
| commit.files_modified += 1; | ||
| ( | ||
| FileChangeType::Modify, | ||
| parts.get(5).unwrap_or(&"").to_string(), | ||
| String::new(), | ||
| ) | ||
| } | ||
| 'R' => { | ||
| commit.files_renamed += 1; | ||
| let old = parts.get(5).unwrap_or(&"").to_string(); | ||
| let new = parts.get(6).unwrap_or(&"").to_string(); | ||
| (FileChangeType::Rename, new, old) | ||
| } | ||
| 'C' => { | ||
| let old = parts.get(5).unwrap_or(&"").to_string(); | ||
| let new = parts.get(6).unwrap_or(&"").to_string(); | ||
| (FileChangeType::Copy, new, old) | ||
| } | ||
| 'T' => ( | ||
| FileChangeType::Type, | ||
| parts.get(5).unwrap_or(&"").to_string(), | ||
| String::new(), | ||
| ), | ||
| _ => continue, | ||
| }; | ||
|
|
||
| if let Some(ref regex) = skip_paths_regex { | ||
| if regex.is_match(&path) { | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| let file_extension = Path::new(&path) | ||
| .extension() | ||
| .and_then(|ext| ext.to_str()) | ||
| .unwrap_or("") | ||
| .to_string(); | ||
|
|
||
| let file_change = FileChange { | ||
| change_type, | ||
| path: path.clone(), | ||
| old_path, | ||
| file_extension, | ||
| lines_added: 0, | ||
| lines_deleted: 0, | ||
| hunks_added: 0, | ||
| hunks_removed: 0, | ||
| hunks_changed: 0, | ||
| }; | ||
|
|
||
| commit_diff.insert( | ||
| path, | ||
| FileDiff { | ||
| file_change, | ||
| line_changes: Vec::new(), | ||
| }, | ||
| ); | ||
| } | ||
| } else if line.starts_with("--- ") { | ||
| if let Some(path) = line.strip_prefix("--- a/") { | ||
| current_file = Some(path.to_string()); | ||
| } else if line == "--- /dev/null" { | ||
| current_file = None; | ||
| } | ||
| } else if line.starts_with("+++ ") { | ||
| if current_file.is_none() { | ||
| if let Some(path) = line.strip_prefix("+++ b/") { | ||
| current_file = Some(path.to_string()); | ||
| } | ||
| } | ||
| } else if line.starts_with("@@ ") { | ||
| if let Some(ref file_path) = current_file { | ||
| if let Some(file_diff) = commit_diff.get_mut(file_path) { | ||
| let parts: Vec<&str> = line.split_whitespace().collect(); | ||
| if parts.len() >= 4 { | ||
| let old_range = parts[1].strip_prefix('-').unwrap_or(parts[1]); | ||
| let new_range = parts[2].strip_prefix('+').unwrap_or(parts[2]); | ||
|
|
||
| let (old_start, old_lines) = parse_range(old_range); | ||
| let (new_start, new_lines) = parse_range(new_range); | ||
|
|
||
| line_change.hunk_num += 1; | ||
| line_change.hunk_start_line_number_old = old_start; | ||
| line_change.hunk_start_line_number_new = new_start.max(1); | ||
| line_change.hunk_lines_added = new_lines; | ||
| line_change.hunk_lines_deleted = old_lines; | ||
| line_change.line_number_old = old_start; | ||
| line_change.line_number_new = new_start.max(1); | ||
|
|
||
| if parts.len() > 4 { | ||
| line_change.hunk_context = parts[4..].join(" "); | ||
| } | ||
|
|
||
| if old_lines > 0 && new_lines > 0 { | ||
| commit.hunks_changed += 1; | ||
| file_diff.file_change.hunks_changed += 1; | ||
| } else if old_lines > 0 { | ||
| commit.hunks_removed += 1; | ||
| file_diff.file_change.hunks_removed += 1; | ||
| } else if new_lines > 0 { | ||
| commit.hunks_added += 1; | ||
| file_diff.file_change.hunks_added += 1; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } else if line.starts_with('-') && !line.starts_with("---") { | ||
| diff_size += 1; | ||
| if let Some(ref file_path) = current_file { | ||
| if let Some(file_diff) = commit_diff.get_mut(file_path) { | ||
| commit.lines_deleted += 1; | ||
| file_diff.file_change.lines_deleted += 1; | ||
|
|
||
| let line_content = &line[1..]; | ||
| let (clean_line, indent, line_type) = | ||
| LineChange::classify_line(line_content); | ||
|
|
||
| let mut lc = line_change.clone(); | ||
| lc.sign = -1; | ||
| lc.line = clean_line; | ||
| lc.indent = indent; | ||
| lc.line_type = line_type; | ||
|
|
||
| file_diff.line_changes.push(lc); | ||
| line_change.line_number_old += 1; | ||
| } | ||
| } | ||
| } else if line.starts_with('+') && !line.starts_with("+++") { | ||
| diff_size += 1; | ||
| if let Some(ref file_path) = current_file { | ||
| if let Some(file_diff) = commit_diff.get_mut(file_path) { | ||
| commit.lines_added += 1; | ||
| file_diff.file_change.lines_added += 1; | ||
|
|
||
| let line_content = &line[1..]; | ||
| let (clean_line, indent, line_type) = | ||
| LineChange::classify_line(line_content); | ||
|
|
||
| let mut lc = line_change.clone(); | ||
| lc.sign = 1; | ||
| lc.line = clean_line; | ||
| lc.indent = indent; | ||
| lc.line_type = line_type; | ||
|
|
||
| file_diff.line_changes.push(lc); | ||
| line_change.line_number_new += 1; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if self.options.diff_size_limit > 0 | ||
| && commit_num != 0 | ||
| && diff_size > self.options.diff_size_limit | ||
| { | ||
| debug!( | ||
| "Skipping commit {} with diff size {} > {}", | ||
| commit.hash, diff_size, self.options.diff_size_limit | ||
| ); | ||
| return Ok(()); | ||
| } | ||
| } | ||
|
|
||
| Ok(()) |
Comment on lines
+307
to
+308
| if regex.is_match(&path) { | ||
| continue; |
Comment on lines
+354
to
+382
| if parts.len() >= 4 { | ||
| let old_range = parts[1].strip_prefix('-').unwrap_or(parts[1]); | ||
| let new_range = parts[2].strip_prefix('+').unwrap_or(parts[2]); | ||
|
|
||
| let (old_start, old_lines) = parse_range(old_range); | ||
| let (new_start, new_lines) = parse_range(new_range); | ||
|
|
||
| line_change.hunk_num += 1; | ||
| line_change.hunk_start_line_number_old = old_start; | ||
| line_change.hunk_start_line_number_new = new_start.max(1); | ||
| line_change.hunk_lines_added = new_lines; | ||
| line_change.hunk_lines_deleted = old_lines; | ||
| line_change.line_number_old = old_start; | ||
| line_change.line_number_new = new_start.max(1); | ||
|
|
||
| if parts.len() > 4 { | ||
| line_change.hunk_context = parts[4..].join(" "); | ||
| } | ||
|
|
||
| if old_lines > 0 && new_lines > 0 { | ||
| commit.hunks_changed += 1; | ||
| file_diff.file_change.hunks_changed += 1; | ||
| } else if old_lines > 0 { | ||
| commit.hunks_removed += 1; | ||
| file_diff.file_change.hunks_removed += 1; | ||
| } else if new_lines > 0 { | ||
| commit.hunks_added += 1; | ||
| file_diff.file_change.hunks_added += 1; | ||
| } |
Comment on lines
+445
to
+496
| fn update_snapshot( | ||
| &self, | ||
| snapshot: &mut Snapshot, | ||
| commit: &Commit, | ||
| file_changes: &mut CommitDiff, | ||
| ) -> Result<()> { | ||
| for (path, file_diff) in file_changes.iter_mut() { | ||
| if !file_diff.file_change.old_path.is_empty() && file_diff.file_change.old_path != *path | ||
| { | ||
| if let Some(old_blame) = snapshot.remove(&file_diff.file_change.old_path) { | ||
| snapshot.insert(path.clone(), old_blame); | ||
| } | ||
| } | ||
|
|
||
| let file_blame = snapshot.entry(path.clone()).or_default(); | ||
| let mut deleted_lines: HashMap<u32, Commit> = HashMap::new(); | ||
|
|
||
| for line_change in &mut file_diff.line_changes { | ||
| if line_change.sign == -1 { | ||
| if let Some(prev_commit) = file_blame.find(line_change.line_number_old as usize) | ||
| { | ||
| if prev_commit.time <= commit.time { | ||
| line_change.prev_commit_hash = prev_commit.hash.clone(); | ||
| line_change.prev_author = prev_commit.author.clone(); | ||
| line_change.prev_time = Some(prev_commit.time); | ||
| deleted_lines.insert(line_change.line_number_old, prev_commit.clone()); | ||
| } | ||
| } | ||
| } else if line_change.sign == 1 { | ||
| let this_line_in_prev = line_change.hunk_start_line_number_old | ||
| + (line_change.line_number_new - line_change.hunk_start_line_number_new); | ||
|
|
||
| if let Some(prev_commit) = deleted_lines.get(&this_line_in_prev) { | ||
| if prev_commit.time <= commit.time { | ||
| line_change.prev_commit_hash = prev_commit.hash.clone(); | ||
| line_change.prev_author = prev_commit.author.clone(); | ||
| line_change.prev_time = Some(prev_commit.time); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| for line_change in &file_diff.line_changes { | ||
| if line_change.sign == -1 { | ||
| file_blame.remove_line(line_change.line_number_new as usize); | ||
| } else if line_change.sign == 1 { | ||
| file_blame.add_line(line_change.line_number_new as usize, commit.clone()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Ok(()) |
Comment on lines
+466
to
+470
| if prev_commit.time <= commit.time { | ||
| line_change.prev_commit_hash = prev_commit.hash.clone(); | ||
| line_change.prev_author = prev_commit.author.clone(); | ||
| line_change.prev_time = Some(prev_commit.time); | ||
| deleted_lines.insert(line_change.line_number_old, prev_commit.clone()); |
Comment on lines
+478
to
+481
| if prev_commit.time <= commit.time { | ||
| line_change.prev_commit_hash = prev_commit.hash.clone(); | ||
| line_change.prev_author = prev_commit.author.clone(); | ||
| line_change.prev_time = Some(prev_commit.time); |
|
Diff Coverage for ubuntu-latest: The code coverage on the diff in this pull request is 55.7%. Total Coverage for ubuntu-latest: This PR will decrease coverage by 0.71%. File Coverage Changes
🛟 Help
|
|
Diff Coverage for macos-15: The code coverage on the diff in this pull request is 55.7%. Total Coverage for macos-15: This PR will decrease coverage by 0.72%. File Coverage Changes
🛟 Help
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds comprehensive unit tests for the
qlty-historycrate, improving test coverage and code reliability for the git indexing functionality.Changes
Added 38 unit tests covering core functionality across multiple modules:
TSV Writer Module (12 tests)
LineChange Module (11 tests)
GitIndexOptions Module (8 tests)
FileChange Module (4 tests)
Commit Module (3 tests)
LineChange Struct (2 additional tests)
Test Approach
All tests focus on pure logic without external dependencies:
Verification
qlty fmtcargo checkNext Steps
These unit tests provide a solid foundation for the
qlty-historycrate and will help ensure reliability as the codebase evolves.