Subject: Re: [PATCH 08/10] test-lib-functions: add and use a "test_hook" wrapper

From: Junio C Hamano <[email-masked]>
Date: Wed, 02 Mar 2022 13:59:04 -0800

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

> @@ -430,7 +430,7 @@ test_expect_success 'status succeeds with sparse index' '
>  
>  		# This one modifies outside the sparse-checkout definition
>  		# and hence we expect to expand the sparse-index.
> -		write_script .git/hooks/fsmonitor-test <<-\EOF &&
> +		test_hook --clobber fsmonitor-test <<-\EOF &&
>  			printf "last_update_token\0"
>  			printf "dir1a/modified\0"
>  		EOF

These look almost trivial (the --setup part is somewhat trickier
than the rest), thanks to the use of write_script.

It would have been even cleaner if we jumped from "unindented
cat && chmod +x" directly to "use write_script" in one of the
earlier steps, which would have reduced the need for the last step
in the series.

> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 85385d2ede7..0bef5913100 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -551,6 +551,58 @@ write_script () {
>  	chmod +x "$1"
>  }
>  
> +# Usage: test_hook [options] <hook-name> <<-\EOF
> +#
> +#   -C <dir>:
> +#	Run all git commands in directory <dir>
> +#   --setup
> +#	Setup a hook for subsequent tests, i.e. don't remove it in a
> +#	"test_when_finished"
> +#   --clobber
> +#	Overwrite an existing <hook-name>, if it exists. Implies
> +#	--setup (i.e. the "test_when_finished" is assumed to have been
> +#	set up already).

OK.

We aspire to make everybody use test_hook, so the fact that we have
to clobber means somebody else made the right choice to either use
or not use --setup to create it, and we do not have to (re)arrange
it to be removed later, hence it makes perfect sense for --clobber
to imply --setup.

Looking good.

> +test_hook () {
> +	setup= &&
> +	clobber= &&
> +	indir= &&
> +	while test $# != 0
> +	do
> +		case "$1" in
> +		-C)
> +			indir="$2" &&
> +			shift
> +			;;
> +		--setup)
> +			setup=t
> +			;;
> +		--clobber)
> +			clobber=t
> +			;;
> +		-*)
> +			BUG "invalid argument: $1"
> +			;;
> +		*)
> +			break
> +			;;
> +		esac &&
> +		shift
> +	done &&
> +
> +	git_dir=$(git -C "$indir" rev-parse --absolute-git-dir) &&
> +	hook_dir="$git_dir/hooks" &&
> +	hook_file="$hook_dir/$1" &&
> +	if test -z "$clobber"
> +	then
> +		test_path_is_missing "$hook_file"
> +	fi &&
> +	if test -z "$setup$clobber"
> +	then
> +		test_when_finished "rm \"$hook_file\""
> +	fi &&
> +	write_script "$hook_file"
> +}
> +
>  # Use test_set_prereq to tell that a particular prerequisite is available.
>  # The prerequisite can later be checked for in two ways:
>  #