Subject: Re: [PATCH 3/4] t7004: create separate tags for different tests

From: Junio C Hamano <[email-masked]>
Date: Wed, 22 Dec 2021 12:11:07 -0800

"Han-Wen Nienhuys via GitGitGadget" <[email-masked]> writes:

> From: Han-Wen Nienhuys <[email-masked]>
>
> Reftable intentionally keeps reflog data for deleted refs.
>
> This breaks tests that delete and recreate "refs/tags/tag_with_reflog" as traces
> of the deletion are left in reflog. To resolve this, use a differently named ref
> for each test case.
>
> Signed-off-by: Han-Wen Nienhuys <[email-masked]>
> ---

Makes sense.  

These tests are *not* trying to see if a ref is recreated, its logs
will reflect only the events after the last such recreation event.

The only thing they care about is that the logs record the events
after the ref was created the last time.

Hmmm, is it?  After spelling it out like the above to make sure I
understand the argument, I am not so sure.  If these tests are
surprised to see reflog entries from the previous life, it will
certainly surprise users and their tools if we suddenly start
showing them without being told.  Their expectation is that a
deletion and recreation is a life resetting event for a ref.

It feels as if we would need a new option in reflog traversal to
take advantage of the new capability reftable offers (i.e. "git
reflog --show-previous-life"), which, use of it is an error if the
backend does not support it.  An alternative is to document it, when
reftable integration happens, as a limitation of files-backend that
it cannot store or show the logs of the ref from its previous life.

I do not care too deeply about it either way right now, because this
patch allows us sidestep the design issue altogether in this test
script.  But we need to think about it when integrating the reftable
backend into refs API.

Thanks.

>  t/t7004-tag.sh | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 082be85dffc..301d1190482 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -94,10 +94,10 @@ test_expect_success 'creating a tag with --create-reflog should create reflog' '
>  	git log -1 \
>  		--format="format:tag: tagging %h (%s, %cd)%n" \
>  		--date=format:%Y-%m-%d >expected &&
> -	test_when_finished "git tag -d tag_with_reflog" &&
> -	git tag --create-reflog tag_with_reflog &&
> -	git reflog exists refs/tags/tag_with_reflog &&
> -	sed -e "s/^.*	//" .git/logs/refs/tags/tag_with_reflog >actual &&
> +	test_when_finished "git tag -d tag_with_reflog1" &&
> +	git tag --create-reflog tag_with_reflog1 &&
> +	git reflog exists refs/tags/tag_with_reflog1 &&
> +	sed -e "s/^.*	//" .git/logs/refs/tags/tag_with_reflog1 >actual &&
>  	test_cmp expected actual
>  '
>  
> @@ -105,10 +105,10 @@ test_expect_success 'annotated tag with --create-reflog has correct message' '
>  	git log -1 \
>  		--format="format:tag: tagging %h (%s, %cd)%n" \
>  		--date=format:%Y-%m-%d >expected &&
> -	test_when_finished "git tag -d tag_with_reflog" &&
> -	git tag -m "annotated tag" --create-reflog tag_with_reflog &&
> -	git reflog exists refs/tags/tag_with_reflog &&
> -	sed -e "s/^.*	//" .git/logs/refs/tags/tag_with_reflog >actual &&
> +	test_when_finished "git tag -d tag_with_reflog2" &&
> +	git tag -m "annotated tag" --create-reflog tag_with_reflog2 &&
> +	git reflog exists refs/tags/tag_with_reflog2 &&
> +	sed -e "s/^.*	//" .git/logs/refs/tags/tag_with_reflog2 >actual &&
>  	test_cmp expected actual
>  '
>  
> @@ -118,10 +118,10 @@ test_expect_success '--create-reflog does not create reflog on failure' '
>  '
>  
>  test_expect_success 'option core.logAllRefUpdates=always creates reflog' '
> -	test_when_finished "git tag -d tag_with_reflog" &&
> +	test_when_finished "git tag -d tag_with_reflog3" &&
>  	test_config core.logAllRefUpdates always &&
> -	git tag tag_with_reflog &&
> -	git reflog exists refs/tags/tag_with_reflog
> +	git tag tag_with_reflog3 &&
> +	git reflog exists refs/tags/tag_with_reflog3
>  '
>  
>  test_expect_success 'listing all tags if one exists should succeed' '