| Message ID | cba766f224b0d2b4fd952b11bef8068c07dfcf88.1696969994.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | bloom: changed-path Bloom filters v2 (& sundries) | expand |
On Tue, Oct 10, 2023 at 04:33:42PM -0400, Taylor Blau wrote: > From: Jonathan Tan <jonathantanmy@google.com> > > Subsequent commits will teach Git another version of changed path > filter that has different behavior with paths that contain at least > one character with its high bit set, so test the existing behavior as > a baseline. > > Signed-off-by: Jonathan Tan <jonathantanmy@google.com> > Signed-off-by: Junio C Hamano <gitster@pobox.com> > Signed-off-by: Taylor Blau <me@ttaylorr.com> > Signed-off-by: Junio C Hamano <gitster@pobox.com> > Signed-off-by: Taylor Blau <me@ttaylorr.com> Nit: the signoffs are still funny here. > --- > t/t4216-log-bloom.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 52 insertions(+) > > diff --git a/t/t4216-log-bloom.sh b/t/t4216-log-bloom.sh > index f49a8f2fbf..da67c40134 100755 > --- a/t/t4216-log-bloom.sh > +++ b/t/t4216-log-bloom.sh > @@ -484,4 +484,56 @@ test_expect_success 'merge graph layers with incompatible Bloom settings' ' > ! grep "disabling Bloom filters" err > ' > > +get_first_changed_path_filter () { > + test-tool read-graph bloom-filters >filters.dat && > + head -n 1 filters.dat > +} > + > +# chosen to be the same under all Unicode normalization forms > +CENT=$(printf "\302\242") > + > +test_expect_success 'set up repo with high bit path, version 1 changed-path' ' > + git init highbit1 && > + test_commit -C highbit1 c1 "$CENT" && > + git -C highbit1 commit-graph write --reachable --changed-paths > +' > + > +test_expect_success 'setup check value of version 1 changed-path' ' > + ( > + cd highbit1 && > + echo "52a9" >expect && > + get_first_changed_path_filter >actual && > + test_cmp expect actual > + ) > +' > + > +# expect will not match actual if char is unsigned by default. Write the test > +# in this way, so that a user running this test script can still see if the two > +# files match. (It will appear as an ordinary success if they match, and a skip > +# if not.) > +if test_cmp highbit1/expect highbit1/actual > +then > + test_set_prereq SIGNED_CHAR_BY_DEFAULT > +fi > +test_expect_success SIGNED_CHAR_BY_DEFAULT 'check value of version 1 changed-path' ' > + # Only the prereq matters for this test. > + true > +' Doesn't this mean that the preceding test where we `test_cmp expect actual` can fail on some platforms depending on the signedness of `char`? Patrick > +test_expect_success 'setup make another commit' ' > + # "git log" does not use Bloom filters for root commits - see how, in > + # revision.c, rev_compare_tree() (the only code path that eventually calls > + # get_bloom_filter()) is only called by try_to_simplify_commit() when the commit > + # has one parent. Therefore, make another commit so that we perform the tests on > + # a non-root commit. > + test_commit -C highbit1 anotherc1 "another$CENT" > +' > + > +test_expect_success 'version 1 changed-path used when version 1 requested' ' > + ( > + cd highbit1 && > + test_bloom_filters_used "-- another$CENT" > + ) > +' > + > test_done > -- > 2.42.0.342.g8bb3a896ee >
On Tue, Oct 17, 2023 at 10:45:13AM +0200, Patrick Steinhardt wrote: > > +test_expect_success 'setup check value of version 1 changed-path' ' > > + ( > > + cd highbit1 && > > + echo "52a9" >expect && > > + get_first_changed_path_filter >actual && > > + test_cmp expect actual > > + ) > > +' > > + > > +# expect will not match actual if char is unsigned by default. Write the test > > +# in this way, so that a user running this test script can still see if the two > > +# files match. (It will appear as an ordinary success if they match, and a skip > > +# if not.) > > +if test_cmp highbit1/expect highbit1/actual > > +then > > + test_set_prereq SIGNED_CHAR_BY_DEFAULT > > +fi > > +test_expect_success SIGNED_CHAR_BY_DEFAULT 'check value of version 1 changed-path' ' > > + # Only the prereq matters for this test. > > + true > > +' > > Doesn't this mean that the preceding test where we `test_cmp expect > actual` can fail on some platforms depending on the signedness of > `char`? Great catch, I am surprised this slipped by in earlier rounds. This should do the trick, since we don't actually care about conditioning that test's passing on test_cmp coming up clean. We check that in the if statement you pointed out here, so: --- 8< --- diff --git a/t/t4216-log-bloom.sh b/t/t4216-log-bloom.sh index 114672e904..400dce2193 100755 --- a/t/t4216-log-bloom.sh +++ b/t/t4216-log-bloom.sh @@ -502,8 +502,7 @@ test_expect_success 'setup check value of version 1 changed-path' ' ( cd highbit1 && echo "52a9" >expect && - get_first_changed_path_filter >actual && - test_cmp expect actual + get_first_changed_path_filter >actual ) ' --- >8 --- Thanks, Taylor
diff --git a/t/t4216-log-bloom.sh b/t/t4216-log-bloom.sh index f49a8f2fbf..da67c40134 100755 --- a/t/t4216-log-bloom.sh +++ b/t/t4216-log-bloom.sh @@ -484,4 +484,56 @@ test_expect_success 'merge graph layers with incompatible Bloom settings' ' ! grep "disabling Bloom filters" err ' +get_first_changed_path_filter () { + test-tool read-graph bloom-filters >filters.dat && + head -n 1 filters.dat +} + +# chosen to be the same under all Unicode normalization forms +CENT=$(printf "\302\242") + +test_expect_success 'set up repo with high bit path, version 1 changed-path' ' + git init highbit1 && + test_commit -C highbit1 c1 "$CENT" && + git -C highbit1 commit-graph write --reachable --changed-paths +' + +test_expect_success 'setup check value of version 1 changed-path' ' + ( + cd highbit1 && + echo "52a9" >expect && + get_first_changed_path_filter >actual && + test_cmp expect actual + ) +' + +# expect will not match actual if char is unsigned by default. Write the test +# in this way, so that a user running this test script can still see if the two +# files match. (It will appear as an ordinary success if they match, and a skip +# if not.) +if test_cmp highbit1/expect highbit1/actual +then + test_set_prereq SIGNED_CHAR_BY_DEFAULT +fi +test_expect_success SIGNED_CHAR_BY_DEFAULT 'check value of version 1 changed-path' ' + # Only the prereq matters for this test. + true +' + +test_expect_success 'setup make another commit' ' + # "git log" does not use Bloom filters for root commits - see how, in + # revision.c, rev_compare_tree() (the only code path that eventually calls + # get_bloom_filter()) is only called by try_to_simplify_commit() when the commit + # has one parent. Therefore, make another commit so that we perform the tests on + # a non-root commit. + test_commit -C highbit1 anotherc1 "another$CENT" +' + +test_expect_success 'version 1 changed-path used when version 1 requested' ' + ( + cd highbit1 && + test_bloom_filters_used "-- another$CENT" + ) +' + test_done