Subject: Re: [PATCH 11/18] t1407: require REFFILES for for_each_reflog test

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Wed, 21 Apr 2021 08:23:23 +0200

On Mon, Apr 19 2021, Han-Wen Nienhuys via GitGitGadget wrote:

> From: Han-Wen Nienhuys <[email-masked]>
>
> It tries to setup a reflog by catting to .git/logs/
>
> Signed-off-by: Han-Wen Nienhuys <[email-masked]>
> ---
>  t/t1407-worktree-ref-store.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh
> index d3fe77751122..27b57f248a94 100755
> --- a/t/t1407-worktree-ref-store.sh
> +++ b/t/t1407-worktree-ref-store.sh
> @@ -52,7 +52,7 @@ test_expect_success 'create_symref(FOO, refs/heads/main)' '
>  	test_cmp expected actual
>  '
>  
> -test_expect_success 'for_each_reflog()' '
> +test_expect_success REFFILES 'for_each_reflog()' '
>  	echo $ZERO_OID > .git/logs/PSEUDO-MAIN &&
>  	mkdir -p     .git/logs/refs/bisect &&
>  	echo $ZERO_OID > .git/logs/refs/bisect/random &&

Hrm, so already the first use of REFFILES has me questioning the need
for it.

I mean obviously this depends on ref-files in the strict sense.

But it seems to me that there's two classes of those issues:

 A. Test where we inherently depend 100% on "reffiles", e.g. the later
    tests of "empty directories" in .git/refs/, presumably there's no
    equivalent of those in reftable.

 B. Tests that are really wanting to test some specific type of ref
    corruption, that could (or not!) happen under reftable, but just
    uses ref files for the setup now.

I think (but am not sure) that this is the latter case. I.e. the
distiniction I noted in [1].

Just skimming the context I wonder which of these can/can't happen under
reftable:

 * We have a PSEUDO-MAIN ref
 * It's set to $ZERO_OID
 * We have a $ZERO_OID in a refs/bisect/random

Etc., the test is just a setup for a call to refs_for_each_reflog().

If I'm wrong and it really is a case of "A" then presumably that has
implications down to refs/iterator.c and beyond.

I don't think we need to exhaustively dig into every one of these for
some initial pass at getting the test suite running, but again re [1] I
worry that if we over-skip we'll end up not marking the distinction
between "A" and "B" above, and thus will have an end-state of losing
test coverage.

1. https://lore.kernel.org/git/[email-masked]/