-
-
Notifications
You must be signed in to change notification settings - Fork 408
Update to imara-diff 0.2 in diff slider test
#2288
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
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 |
|---|---|---|
| @@ -1,11 +1,11 @@ | ||
| use gix_diff::blob::intern::TokenSource; | ||
| use gix_diff::blob::unified_diff::ContextSize; | ||
| use gix_diff::blob::{Algorithm, UnifiedDiff}; | ||
| use gix_object::bstr::ByteSlice; | ||
| use gix_testtools::bstr::{BString, ByteVec}; | ||
| use pretty_assertions::StrComparison; | ||
|
|
||
| #[test] | ||
| fn baseline() -> gix_testtools::Result { | ||
| fn baseline_v1() -> gix_testtools::Result { | ||
| use gix_diff::blob::{unified_diff::ContextSize, UnifiedDiff}; | ||
|
|
||
| let worktree_path = gix_testtools::scripted_fixture_read_only_standalone("make_diff_for_sliders_repo.sh")?; | ||
| let asset_dir = worktree_path.join("assets"); | ||
|
|
||
|
|
@@ -15,30 +15,16 @@ fn baseline() -> gix_testtools::Result { | |
|
|
||
| for entry in dir { | ||
| let entry = entry?; | ||
| let file_name = entry.file_name().into_string().expect("to be string"); | ||
|
|
||
| if !file_name.ends_with(".baseline") { | ||
| let Some(baseline::DirEntry { | ||
| file_name, | ||
| algorithm, | ||
| old_data, | ||
| new_data, | ||
| }) = baseline::parse_dir_entry(&asset_dir, &entry.file_name())? | ||
| else { | ||
| continue; | ||
| } | ||
|
|
||
| let parts: Vec<_> = file_name.split('.').collect(); | ||
| let [name, algorithm, ..] = parts[..] else { | ||
| unreachable!() | ||
| }; | ||
| let algorithm = match algorithm { | ||
| "myers" => Algorithm::Myers, | ||
| "histogram" => Algorithm::Histogram, | ||
| _ => unreachable!(), | ||
| }; | ||
|
|
||
| let parts: Vec<_> = name.split('-').collect(); | ||
| let [old_blob_id, new_blob_id] = parts[..] else { | ||
| unreachable!(); | ||
| }; | ||
|
|
||
| let old_data = std::fs::read(asset_dir.join(format!("{old_blob_id}.blob")))?; | ||
| let new_data = std::fs::read(asset_dir.join(format!("{new_blob_id}.blob")))?; | ||
|
|
||
| let interner = gix_diff::blob::intern::InternedInput::new( | ||
| tokens_for_diffing(old_data.as_slice()), | ||
| tokens_for_diffing(new_data.as_slice()), | ||
|
|
@@ -70,17 +56,74 @@ fn baseline() -> gix_testtools::Result { | |
| }) | ||
| .to_string(); | ||
|
|
||
| let baseline = baseline | ||
| .fold(BString::default(), |mut acc, diff_hunk| { | ||
| acc.push_str(diff_hunk.header.to_string().as_str()); | ||
| acc.push(b'\n'); | ||
| let baseline = baseline.fold_to_unidiff().to_string(); | ||
| let actual_matches_baseline = actual == baseline; | ||
| diffs.push((actual, baseline, actual_matches_baseline, file_name)); | ||
| } | ||
|
|
||
| acc.extend_from_slice(&diff_hunk.lines); | ||
| if diffs.is_empty() { | ||
| eprintln!("Slider baseline isn't setup - look at ./gix-diff/tests/README.md for instructions"); | ||
| } | ||
|
|
||
| acc | ||
| }) | ||
| assert_diffs(&diffs); | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| fn tokens_for_diffing(data: &[u8]) -> impl gix_diff::blob::intern::TokenSource<Token = &[u8]> { | ||
| gix_diff::blob::sources::byte_lines(data) | ||
| } | ||
|
|
||
| #[test] | ||
| fn baseline_v2() -> gix_testtools::Result { | ||
| use gix_diff::blob::v2::{Algorithm, BasicLineDiffPrinter, Diff, InternedInput, UnifiedDiffConfig}; | ||
|
|
||
| let worktree_path = gix_testtools::scripted_fixture_read_only_standalone("make_diff_for_sliders_repo.sh")?; | ||
| let asset_dir = worktree_path.join("assets"); | ||
|
|
||
| let dir = std::fs::read_dir(&worktree_path)?; | ||
|
|
||
| let mut diffs = Vec::new(); | ||
|
|
||
| for entry in dir { | ||
| let entry = entry?; | ||
| let Some(baseline::DirEntry { | ||
| file_name, | ||
| algorithm, | ||
| old_data, | ||
| new_data, | ||
| }) = baseline::parse_dir_entry(&asset_dir, &entry.file_name())? | ||
| else { | ||
| continue; | ||
| }; | ||
|
|
||
| let input = InternedInput::new( | ||
| old_data.to_str().expect("BUG: we don't have non-ascii here"), | ||
| new_data.to_str().expect("BUG: we don't have non-ascii here"), | ||
| ); | ||
| let algorithm = match algorithm { | ||
| gix_diff::blob::Algorithm::Myers => Algorithm::Myers, | ||
| gix_diff::blob::Algorithm::Histogram => Algorithm::Histogram, | ||
| gix_diff::blob::Algorithm::MyersMinimal => Algorithm::MyersMinimal, | ||
| }; | ||
|
|
||
| let mut diff = Diff::compute(algorithm, &input); | ||
| diff.postprocess_lines(&input); | ||
|
Comment on lines
+110
to
+111
|
||
|
|
||
| let actual = diff | ||
| .unified_diff( | ||
| &BasicLineDiffPrinter(&input.interner), | ||
| UnifiedDiffConfig::default(), | ||
| &input, | ||
| ) | ||
| .to_string(); | ||
|
|
||
| let baseline_path = worktree_path.join(&file_name); | ||
| let baseline = std::fs::read(baseline_path)?; | ||
| let baseline = baseline::Baseline::new(&baseline); | ||
|
|
||
| let baseline = baseline.fold_to_unidiff().to_string(); | ||
|
|
||
| let actual_matches_baseline = actual == baseline; | ||
| diffs.push((actual, baseline, actual_matches_baseline, file_name)); | ||
| } | ||
|
|
@@ -89,6 +132,11 @@ fn baseline() -> gix_testtools::Result { | |
| eprintln!("Slider baseline isn't setup - look at ./gix-diff/tests/README.md for instructions"); | ||
| } | ||
|
|
||
| assert_diffs(&diffs); | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn assert_diffs(diffs: &[(String, String, bool, String)]) { | ||
| let total_diffs = diffs.len(); | ||
| let matching_diffs = diffs | ||
| .iter() | ||
|
|
@@ -115,20 +163,16 @@ fn baseline() -> gix_testtools::Result { | |
| ) | ||
| } | ||
| ); | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| fn tokens_for_diffing(data: &[u8]) -> impl TokenSource<Token = &[u8]> { | ||
| gix_diff::blob::sources::byte_lines(data) | ||
| } | ||
|
|
||
| mod baseline { | ||
| use gix_object::bstr::ByteSlice; | ||
| use std::iter::Peekable; | ||
|
|
||
| use gix_diff::blob::unified_diff::{ConsumeHunk, HunkHeader}; | ||
| use gix_diff::blob::Algorithm; | ||
| use gix_object::bstr::{self, BString}; | ||
| use gix_object::bstr::{ByteSlice, ByteVec}; | ||
| use std::ffi::OsStr; | ||
| use std::iter::Peekable; | ||
| use std::path::Path; | ||
|
|
||
| static START_OF_HEADER: &[u8; 4] = b"@@ -"; | ||
|
|
||
|
|
@@ -193,6 +237,20 @@ mod baseline { | |
| } | ||
| } | ||
|
|
||
| impl Baseline<'_> { | ||
| /// Converts all baseline [`DiffHunk`]s into a single unified diff format string. | ||
| pub fn fold_to_unidiff(self) -> BString { | ||
| self.fold(BString::default(), |mut acc, diff_hunk| { | ||
| acc.push_str(diff_hunk.header.to_string().as_str()); | ||
| acc.push(b'\n'); | ||
|
|
||
| acc.extend_from_slice(&diff_hunk.lines); | ||
|
|
||
| acc | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| impl Iterator for Baseline<'_> { | ||
| type Item = DiffHunk; | ||
|
|
||
|
|
@@ -285,4 +343,45 @@ mod baseline { | |
| .parse::<u32>() | ||
| .expect("to be a number") | ||
| } | ||
|
|
||
| pub struct DirEntry { | ||
| pub file_name: String, | ||
| pub algorithm: Algorithm, | ||
| pub old_data: Vec<u8>, | ||
| pub new_data: Vec<u8>, | ||
| } | ||
|
|
||
| /// Returns `None` if the file isn't a baseline entry. | ||
| pub fn parse_dir_entry(asset_dir: &Path, file_name: &OsStr) -> std::io::Result<Option<DirEntry>> { | ||
| let file_name = file_name.to_str().expect("ascii filename").to_owned(); | ||
|
|
||
| if !file_name.ends_with(".baseline") { | ||
| return Ok(None); | ||
| } | ||
|
|
||
| let parts: Vec<_> = file_name.split('.').collect(); | ||
| let [name, algorithm, ..] = parts[..] else { | ||
| unreachable!("BUG: Need file named '<name>.<algorithm>'") | ||
| }; | ||
| let algorithm = match algorithm { | ||
| "myers" => Algorithm::Myers, | ||
| "histogram" => Algorithm::Histogram, | ||
| other => unreachable!("BUG: '{other}' is not a supported algorithm"), | ||
| }; | ||
|
|
||
| let parts: Vec<_> = name.split('-').collect(); | ||
| let [old_blob_id, new_blob_id] = parts[..] else { | ||
| unreachable!("BUG: name part of filename must be '<old_blob_id>-<new_blob_id>'"); | ||
| }; | ||
|
|
||
| let old_data = std::fs::read(asset_dir.join(format!("{old_blob_id}.blob")))?; | ||
| let new_data = std::fs::read(asset_dir.join(format!("{new_blob_id}.blob")))?; | ||
| Ok(DirEntry { | ||
| file_name, | ||
| algorithm, | ||
| old_data, | ||
| new_data, | ||
| } | ||
| .into()) | ||
| } | ||
| } | ||
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 algorithm mapping is duplicated between parse_dir_entry (lines 138-142) and this section. Since parse_dir_entry already returns the Algorithm enum from the v1 API, this remapping to v2 Algorithm could be extracted into a helper function to avoid duplication and make the relationship between v1 and v2 algorithms clearer.
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.
I don't think so.