Subject: Re: [PATCH v3 08/17] t4216: test changed path filters with high bit paths

From: Patrick Steinhardt <[email-masked]>
Date: Tue, 17 Oct 2023 10:45:13 +0200

On Tue, Oct 10, 2023 at 04:33:42PM -0400, Taylor Blau wrote:
> From: Jonathan Tan <[email-masked]>
> 
> 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 <[email-masked]>
> Signed-off-by: Junio C Hamano <[email-masked]>
> Signed-off-by: Taylor Blau <[email-masked]>
> Signed-off-by: Junio C Hamano <[email-masked]>
> Signed-off-by: Taylor Blau <[email-masked]>

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
>