Report number of non-matching diffs in slider test #2276
Conversation
cruessler
changed the title
In addition to the first non-matching diff, the test now also reports the fixture it came from and the number of total as well as non-matching diffs. This makes the slider test more suitable as a benchmark for comparing different slider heuristics.
cruessler
force-pushed
the
run-tests-in-parallel
branch
from
9747a5e to
614d93d
Compare
Byron
left a comment
Byron
left a comment
There was a problem hiding this comment.
Thanks a lot, this is also what I had in mind when thinking about making the test more suitable. And it can always be extended from here, learning everything it needs to.
I have a few comments, but am happy to merge if this is exactly what you want.
gix-diff/tests/diff/blob/slider.rs
Outdated
| assert!( | ||
| matching_diffs == total_diffs, |
There was a problem hiding this comment.
Would this not be more readable with assert_eq!?
gix-diff/tests/diff/blob/slider.rs
Outdated
|
|
||
| assert!( | ||
| matching_diffs == total_diffs, | ||
| "assertion failed: total diffs {} == matching diffs {}\n\n{}", |
There was a problem hiding this comment.
I think the assertion failure will come across by merit of assert_eq, so the text can skip assertion failed:.
|
Thanks for the suggestions, they make a lot of sense! I also added the ratio of matching diffs as a percentage, to make the comparison between two sets of diffs even easier in case they contain a different number of diffs. |
|
Wonderful 🙏! |
2 participants
This PR improves the test reporting for the diff slider test.
In addition to the first non-matching diff, the test now also reports the fixture it came from and the number of total as well as non-matching diffs. This makes the slider test more suitable as a benchmark for comparing different slider heuristics.
This is a follow-up to #2197.
Comparison of new test report (above) vs. old test report (below):