From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mon, 24 Mar 2025 16:16:22 GMT
Subject: Re: [PATCH 13/20] t: refactor tests depending on Perl for textconv scripts
Message-ID: <80c2e84e-6603-4e96-8934-8219c55d4d36@gmail.com>
In-Reply-To: <20250320-b4-pks-t-perlless-v1-13-b1eefe27ac55@pks.im>

Hi Patrick

On 20/03/2025 09:35, Patrick Steinhardt wrote:
> We have a couple of tests that depend on Perl for textconv scripts.
> Refactor these tests to instead be implemented via shell utilities so
> that we can drop a couple of PERL_TEST_HELPERS prerequisites.
> 
> Note that not all of the conversions are a one-to-one equivalent to the
> previous textconv scripts. But that's not really needed in the first
> place: we only care that the textconv script does something, and that
> can be verified trivially without having a full-blown invocation of
> hexdump. So at times, the implementation of the textconv scripts is
> reduced to their bare minimum.
 > > -cat >hexdump <<'EOF'
> -#!/bin/sh
> -"$PERL_PATH" -e '$/ = undef; $_ = <>; s/./ord($&)/ge; print $_' < "$1"
> -EOF
> -chmod +x hexdump
> -
>   test_expect_success 'setup binary file with history' '
> +	write_script hexdump <<-\EOF &&
> +	tr "\000\001" "01" <"$1"
I guess it is fine just to handle the characters we expect at the moment 
(is that what the second paragraph of the commit message is referring 
to?), but the script it was more tolerant of future changes to the test 
data. We could always just use 'test-tool hexdump' here like the tests 
below but it is probably not worth a re-roll on its own.

Overall this series looks like a useful improvement.

Thanks

Phillip

> +	EOF
>   	test_commit --printf one file "\\0\\n" &&
>   	test_commit --printf --append two file "\\01\\n"
>   '
> diff --git a/t/t4031-diff-rewrite-binary.sh b/t/t4031-diff-rewrite-binary.sh
> index cbe50b15772..15e012ccc7c 100755
> --- a/t/t4031-diff-rewrite-binary.sh
> +++ b/t/t4031-diff-rewrite-binary.sh
> @@ -57,24 +57,19 @@ test_expect_success 'diff --stat counts binary rewrite as 0 lines' '
>   	grep " rewrite file" diff
>   '
>   
> -{
> -	echo "#!$SHELL_PATH"
> -	cat <<'EOF'
> -"$PERL_PATH" -e '$/ = undef; $_ = <>; s/./ord($&)/ge; print $_' < "$1"
> -EOF
> -} >dump
> -chmod +x dump
> -
>   test_expect_success 'setup textconv' '
> +	write_script dump <<-\EOF &&
> +	test-tool hexdump <"$1"
> +	EOF
>   	echo file diff=foo >.gitattributes &&
>   	git config diff.foo.textconv "\"$(pwd)\""/dump
>   '
>   
> -test_expect_success PERL_TEST_HELPERS 'rewrite diff respects textconv' '
> +test_expect_success 'rewrite diff respects textconv' '
>   	git diff -B >diff &&
> -	grep "dissimilarity index" diff &&
> -	grep "^-61" diff &&
> -	grep "^-0" diff
> +	test_grep "dissimilarity index" diff &&
> +	test_grep "^-3d 0a 00" diff &&
> +	test_grep "^+3d 0a 01" diff
>   '
>   
>   test_done
> diff --git a/t/t7815-grep-binary.sh b/t/t7815-grep-binary.sh
> index b2730d200c8..3bd91da9707 100755
> --- a/t/t7815-grep-binary.sh
> +++ b/t/t7815-grep-binary.sh
> @@ -4,12 +4,6 @@ test_description='git grep in binary files'
>   
>   . ./test-lib.sh
>   
> -if ! test_have_prereq PERL_TEST_HELPERS
> -then
> -	skip_all='skipping grep binary tests; Perl not available'
> -	test_done
> -fi
> -
>   test_expect_success 'setup' "
>   	echo 'binaryQfileQm[*]cQ*æQð' | q_to_nul >a &&
>   	git add a &&
> @@ -120,13 +114,10 @@ test_expect_success 'grep respects not-binary diff attribute' '
>   	test_cmp expect actual
>   '
>   
> -cat >nul_to_q_textconv <<'EOF'
> -#!/bin/sh
> -"$PERL_PATH" -pe 'y/\000/Q/' < "$1"
> -EOF
> -chmod +x nul_to_q_textconv
> -
>   test_expect_success 'setup textconv filters' '
> +	write_script nul_to_q_textconv <<-\EOF &&
> +	tr "\000" "Q" <"$1"
> +	EOF
>   	echo a diff=foo >.gitattributes &&
>   	git config diff.foo.textconv "\"$(pwd)\""/nul_to_q_textconv
>   '
>