Re: [PATCH 1/2] t: add tests for ref tombstone scenarios

[Patrick Steinhardt <ps@pks.im>]

On Mon, Jul 06, 2026 at 01:35:55PM +0000, Kristofer Karlsson via GitGitGadget wrote:
> diff --git a/t/perf/p1401-ref-store-tombstones.sh b/t/perf/p1401-ref-store-tombstones.sh
> new file mode 100755
> index 0000000000..e40a6dcbf4
> --- /dev/null
> +++ b/t/perf/p1401-ref-store-tombstones.sh
> @@ -0,0 +1,44 @@
> +#!/bin/sh
> +
> +test_description="Tests performance of ref operations with many tombstones"
> +
> +. ./perf-lib.sh
> +
> +test_expect_success "setup" '
> +	git init --ref-format=reftable repo &&
> +	blob=$(echo foo | git -C repo hash-object -w --stdin) &&
> +	for i in $(test_seq 8000)
> +	do
> +		printf "create refs/tags/tag-%d %s\n" "$i" "$blob" ||
> +		return 1
> +	done >repo/input &&
> +	git -C repo update-ref --stdin <repo/input &&
> +	git -C repo for-each-ref --format="delete %(refname)" |
> +	git -C repo update-ref --stdin
> +'
> +
> +test_perf "recreate refs after mass delete" '
> +	git -C repo update-ref --stdin <repo/input &&
> +	git -C repo for-each-ref --format="delete %(refname)" |
> +	git -C repo update-ref --stdin
> +'

You're not only benchmarking the reference recreation, but also their
deletion. If I'm not misreading things, then you can queue cleanups via
`test_when_finished`, and these calls will not be measured.

> +test_expect_success "setup asymmetric" '
> +	for i in $(test_seq 8000)
> +	do
> +		printf "create refs/tags/old-%d %s\n" "$i" "$blob" ||
> +		return 1
> +	done >repo/input-old &&
> +	sed "s/old-/new-/" <repo/input-old >repo/input-new &&
> +	git -C repo update-ref --stdin <repo/input-old &&
> +	git -C repo for-each-ref --format="delete %(refname)" |
> +	git -C repo update-ref --stdin
> +'

Would it make sense to use separate repositories? Otherwise, state from
the preceding benchmark(s) will impact subsequent ones.

> diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
> index e19e036898..4b7cfe38e4 100755
> --- a/t/t0610-reftable-basics.sh
> +++ b/t/t0610-reftable-basics.sh
> @@ -1163,4 +1163,26 @@ test_expect_success 'writes do not persist peeled value for invalid tags' '
>  	)
>  '
>  
> +test_expect_success 'delete and re-create refs with tombstones' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	test_commit -C repo A &&
> +	A=$(git -C repo rev-parse HEAD) &&
> +	cat >input <<-EOF &&
> +	create refs/tags/a $A
> +	create refs/tags/b $A
> +	create refs/tags/c $A
> +	EOF
> +	git -C repo update-ref --stdin <input &&
> +
> +	# delete all tags, leaving tombstones
> +	git -C repo for-each-ref --format="delete %(refname)" refs/tags/ |
> +	git -C repo update-ref --stdin &&
> +
> +	# re-create the same refs and verify they are visible
> +	git -C repo update-ref --stdin <input &&
> +	git -C repo tag -l >actual &&
> +	test_line_count = 3 actual
> +'

I wonder whether this test really adds any value. We probably have lots
of tests already that test creation/deletion of references.

Patrick