Subject: Re: [PATCH 05/11] t: convert tests to not access symrefs via the filesystem

From: Junio C Hamano <[email-masked]>
Date: Fri, 20 Oct 2023 12:52:36 -0700

Patrick Steinhardt <[email-masked]> writes:

> @@ -164,9 +164,9 @@ test_expect_success 'rev-parse skips symref pointing to broken name' '
>  test_expect_success 'for-each-ref emits warnings for broken names' '
>  	test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
>  	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
> -	printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
> +	test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref &&

I am of two minds here.  While it certainly smells nicer because we
can test ref backends other than the files backend with this change,
we are forcing all ref backends to support creating a symbolic ref
with invalid name, because otherwise "test-tool" would not be able
to do this.  Newer more database-oriented ref backends should be
allowed to implement their file format in which it is imposssible to
store such a bad name, but this makes it impossible.

I guess it is OK, because we would introduce some new prerequisite
(i.e. REF_BACKEND_ALLOWS_BROKEN_REFS) to skip this test on certain
ref backend where making invalid refs is impossible.

Other kind of changes in this patch, e.g., ...

> @@ -315,7 +325,9 @@ test_expect_success 'defaulted HEAD uses remote branch if available' '
>  	git -c init.defaultBranch=branchwithstuff -c protocol.version=2 \
>  		clone "file://$(pwd)/file_unborn_parent" \
>  		file_unborn_child 2>stderr &&
> -	grep "refs/heads/branchwithstuff" file_unborn_child/.git/HEAD &&
> +	echo "refs/heads/branchwithstuff" >expect &&
> +	git -C file_unborn_child symbolic-ref HEAD >actual &&
> +	test_cmp expect actual &&
>  	test_path_is_file file_unborn_child/stuff.t &&
>  	! grep "warning:" stderr
>  '
> diff --git a/t/t9133-git-svn-nested-git-repo.sh b/t/t9133-git-svn-nested-git-repo.sh
> index d8d536269cf..8ca24670acb 100755
> --- a/t/t9133-git-svn-nested-git-repo.sh
> +++ b/t/t9133-git-svn-nested-git-repo.sh
> @@ -11,7 +11,7 @@ test_expect_success 'setup repo with a git repo inside it' '
>  	(
>  		cd s &&
>  		git init &&
> -		test -f .git/HEAD &&
> +		git symbolic-ref HEAD &&
>  		> .git/a &&
>  		echo a > a &&
>  		svn_cmd add .git a &&

... all looked sensible, though.

Thanks.