-
-
Notifications
You must be signed in to change notification settings - Fork 385
[WIP] Add script to generate tests cases for gix-diff
#2197
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: main
Are you sure you want to change the base?
[WIP] Add script to generate tests cases for gix-diff
#2197
Conversation
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.
Thanks so much for making this happen! Once the slider problem is solved for us and validated, it's such a major leap for the diff-quality in gitoxide
.
There is a major question I have that isn't mentioned inline: Is it easy to read/understand the output of the pretty assertion that compares both diffs using the internal hunk format? How does that fare in comparison to diffing unified diffs?
This obviates the need to create a git history which considerably simplifies the test as it can just direclty read files instead.
This makes visually interpreting differences between the two versions significantly easier.
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.
Thanks, this looks so much simpler now!
The diffs are also much easier to understand, yet I am wondering: would an actual unified diff be better to diff against another unified diff? That would retain diff lines, and I think the common term for it is 'interdiff'.
let mut blocks: Vec<String> = vec![format!( | ||
r#"#!/usr/bin/env bash | ||
# TODO: | ||
# `git diff --no-index` returns 1 when there's differences, but 1 is treated as an error by the |
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'd keep the set -eu…
and do git diff … || true
or something like that, assuming that it will keep working.
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.
Thanks for the suggestion, changed!
|
||
blocks.push(format!( | ||
r#"git diff --no-index "$ROOT/{asset_dir}/{old_blob_id}.commit" "$ROOT/{asset_dir}/{new_blob_id}.commit" > .git/{old_blob_id}-{new_blob_id}.baseline | ||
cp "$ROOT/{asset_dir}/{old_blob_id}.commit" assets/ |
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.
Are these .commit
s?
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.
Thanks for the suggestion, changed to .blob
.
This PR is about 75–80 % done (hopefully). It still needs comments and there’s also a couple of
TODO
comments that I want to go through. I already wanted to open it at this early stage, kind of as a sneak preview of the things I’m currently working on. :-)What’s the context and what are the goals of this PR?
We assume that most people would expect the output of
gix-diff
to be identical to that ofgit diff
. Therefore, we want an easy way to compare the output ofgix-diff
andgit diff
on a large corpus of diffs to make sure they match.This PR tries to make as much of that process scripted and as easily reproducible as possible.
There is a repository that contains “[t]ools for experimenting [with] diff "slider" heuristics”: diff-slider-tools. We can use
diff-slider-tools
to, for any git repository, generate a list of locations where a diff between two files is not unambiguous.This PR creates a tool that takes this a list of locations generated by
diff-slider-tools
and turns it into test cases that can be run as part ofgix-diff
’s test suite.This enables us to, whenever we want, run large-scale tests comparing
gix-diff
togix diff
, hopefully uncovering any edge case we might have missed in our slider heuristic and making sure we conform to our users’ expectations.Usage
create-diff-cases
to create the script ingix-diff/tests/fixtures/
. The script which will be calledmake_diff_for_sliders_repo.sh
.cargo test sliders -- --nocapture
insidegix-diff/tests
to run the actual tests.Implementation details
The preamble to
make_diff_for_sliders_repo.sh
comes from a similar script ingix-diff
: https://github.com/cruessler/gitoxide/blob/f0ceafa62ff519484e016789c50fa49a79819a80/gix/tests/fixtures/make_diff_repos.sh#L1-L4.The repo created by
make_diff_for_sliders_repo.sh
follows a few conventions:The test runs assertions for each pair of commits. It reads the baseline the comes from
git diff
and compares it to the output ofgix-diff
.Open questions