Re: [PATCH 5/7] t1405: remove unneeded cleanup step

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 14, 2024, 23:17 UTC
Message-ID: <xmqq4jeatz3n.fsf@gitster.g>
In-Reply-To: <25cf00c36f715edc6b4e86001a36a093f4c4b2e0.1707463221.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 19 quoted lines

> In 5e00514745 (t1405: explictly delete reflogs for reftable, 2022-01-31)
> we have added a test that explicitly deletes the reflog when not using
> the "files" backend. This was required because back then, the "reftable"
> backend didn't yet delete reflogs when deleting their corresponding
> branches, and thus subsequent tests would fail because some unexpected
> reflogs still exist.
>
> The "reftable" backend was eventually changed though so that it behaves
> the same as the "files" backend and deletes reflogs when deleting refs.
> This was done to make the "reftable" backend behave like the "files"
> backend as closely as possible so that it can act as a drop-in
> replacement.
>
> The cleanup-style test is thus not required anymore. Remove it.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  t/t1405-main-ref-store.sh | 6 ------
>  1 file changed, 6 deletions(-)

Again, makes sense.

This is a tangent, but artificial limitations we imposed on reftable to be more similar to files backend may be something we would want to reconsider once reftable hits mainline and people actively start using it. Not having to lose the reflog only because you removed a branch by mistake would be a powerful thing, as it would allow you to resurrect the branch as well as its log. Being able to have a branch 'foo', with work related to 'foo' kept inbranches 'foo/arc1' 'foo/arc2', etc., would be a very nice organizational tool.

But it is a good idea to start limited and later making it looser. These two limitations are something all users are already familiar with, so it is not as cripplingly bad as it smells anyway.

Thanks.

Show 18 quoted lines

>
> diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
> index 976bd71efb..1183232a72 100755
> --- a/t/t1405-main-ref-store.sh
> +++ b/t/t1405-main-ref-store.sh
> @@ -33,12 +33,6 @@ test_expect_success 'delete_refs(FOO, refs/tags/new-tag)' '
>  	test_must_fail git rev-parse refs/tags/new-tag --
>  '
>  
> -# In reftable, we keep the reflogs around for deleted refs.
> -test_expect_success !REFFILES 'delete-reflog(FOO, refs/tags/new-tag)' '
> -	$RUN delete-reflog FOO &&
> -	$RUN delete-reflog refs/tags/new-tag
> -'
> -
>  test_expect_success 'rename_refs(main, new-main)' '
>  	git rev-parse main >expected &&
>  	$RUN rename-ref refs/heads/main refs/heads/new-main &&