Subject: Re: [PATCH v2 01/21] t4202: split testcase for invalid HEAD symref and HEAD hash

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Thu, 20 May 2021 17:03:20 +0200

On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:

> From: Han-Wen Nienhuys <[email-masked]>
>
> Reftable will prohibit invalid hashes at the storage level, but
> git-symbolic-ref can still create branches ending in ".lock".
>
> Signed-off-by: Han-Wen Nienhuys <[email-masked]>
> ---
>  t/t4202-log.sh | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> index 350cfa35936a..a8c5a00d012d 100755
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
> @@ -1834,14 +1834,18 @@ test_expect_success 'log --graph --no-walk is forbidden' '
>  	test_must_fail git log --graph --no-walk
>  '
>  
> -test_expect_success 'log diagnoses bogus HEAD' '
> +test_expect_success 'log diagnoses bogus HEAD hash' '
>  	git init empty &&
>  	test_must_fail git -C empty log 2>stderr &&
>  	test_i18ngrep does.not.have.any.commits stderr &&
>  	echo 1234abcd >empty/.git/refs/heads/main &&
>  	test_must_fail git -C empty log 2>stderr &&
> -	test_i18ngrep broken stderr &&
> -	echo "ref: refs/heads/invalid.lock" >empty/.git/HEAD &&
> +	test_i18ngrep broken stderr'
> +
> +test_expect_success 'log diagnoses bogus HEAD symref' '
> +	rm -rf empty &&
> +	git init empty &&
> +	git --git-dir empty/.git symbolic-ref HEAD refs/heads/invalid.lock &&
>  	test_must_fail git -C empty log 2>stderr &&
>  	test_i18ngrep broken stderr &&
>  	test_must_fail git -C empty log --default totally-bogus 2>stderr &&

When splitting up tests like:

    git init xyz &&
    [do with xyz]

Let's do:

    # test 1
    test_when_finished "rm -rf xyz" &&
    git init xyz &&
    [do with xyz]

    # test 2
    test_when_finished "rm -rf xyz" &&
    git init xyz &&
    [do with xyz]

Instead of, as here:

    # test 1
    git init xyz &&
    [do with xyz]

    # test 2
    rm -rf xyz &&
    git init xyz &&
    [do with xyz]

This isn't a logic error, but just a better idiom. Better to have the
thing creating stuff clean it up, than other tests being aware of their
order and (possibly redundantly, if we skip tests), clean up after
earlier tests.