Re: [PATCH v2 4/6] refs: demonstrate excessive execution of the reference-transaction hook

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 8, 2022, 01:31 UTC
Message-ID: <xmqqiluv3tev.fsf@gitster.g>
In-Reply-To: <b52e59cdac75e6c4530cb39f7dcb41bb327f50e2.1641556319.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> Add tests which demonstate which demonstrates that we're executing the

You demonstrate too often, which may be the point of the test, but looks wrong.

I actually think this should be done as part of the fix to the code itself, which presumably is a single-liner to tell the "skip when running delete in packed-refs backend". IOW, just fix the code and test how the externally observable behaviour of the code should be in new tests, in the same commit.

Show 73 quoted lines

> diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
> index 6c941027a8..0567fbdf0b 100755
> --- a/t/t1416-ref-transaction-hooks.sh
> +++ b/t/t1416-ref-transaction-hooks.sh
> @@ -136,4 +136,68 @@ test_expect_success 'interleaving hook calls succeed' '
>  	test_cmp expect target-repo.git/actual
>  '
>  
> +test_expect_success 'hook does not get called on packing refs' '
> +	# Pack references first such that we are in a known state.
> +	git pack-refs --all &&
> +
> +	write_script .git/hooks/reference-transaction <<-\EOF &&
> +		echo "$@" >>actual
> +		cat >>actual
> +	EOF
> +	rm -f actual &&
> +
> +	git update-ref refs/heads/unpacked-ref $POST_OID &&
> +	git pack-refs --all &&
> +
> +	# We only expect a single hook invocation, which is the call to
> +	# git-update-ref(1). But currently, packing refs will also trigger the
> +	# hook.
> +	cat >expect <<-EOF &&
> +		prepared
> +		$ZERO_OID $POST_OID refs/heads/unpacked-ref
> +		committed
> +		$ZERO_OID $POST_OID refs/heads/unpacked-ref
> +		prepared
> +		$ZERO_OID $POST_OID refs/heads/unpacked-ref
> +		committed
> +		$ZERO_OID $POST_OID refs/heads/unpacked-ref
> +		prepared
> +		$POST_OID $ZERO_OID refs/heads/unpacked-ref
> +		committed
> +		$POST_OID $ZERO_OID refs/heads/unpacked-ref
> +	EOF
> +
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'deleting packed ref calls hook once' '
> +	# Create a reference and pack it.
> +	git update-ref refs/heads/to-be-deleted $POST_OID &&
> +	git pack-refs --all &&
> +
> +	write_script .git/hooks/reference-transaction <<-\EOF &&
> +		echo "$@" >>actual
> +		cat >>actual
> +	EOF
> +	rm -f actual &&
> +
> +	git update-ref -d refs/heads/to-be-deleted $POST_OID &&
> +
> +	# We only expect a single hook invocation, which is the logical
> +	# deletion. But currently, we see two interleaving transactions, once
> +	# for deleting the loose refs and once for deleting the packed ref.
> +	cat >expect <<-EOF &&
> +		prepared
> +		$ZERO_OID $ZERO_OID refs/heads/to-be-deleted
> +		prepared
> +		$POST_OID $ZERO_OID refs/heads/to-be-deleted
> +		committed
> +		$ZERO_OID $ZERO_OID refs/heads/to-be-deleted
> +		committed
> +		$POST_OID $ZERO_OID refs/heads/to-be-deleted
> +	EOF
> +
> +	test_cmp expect actual
> +'
> +
>  test_done