diff mbox series

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

Message ID 1ac120368c6cd995841c28bde7542e882ec7b04f.1697607222.git.ps@pks.im (mailing list archive)
State Superseded
Headers show
Series t: reduce direct disk access to data structures | expand

Commit Message

Patrick Steinhardt Oct. 18, 2023, 5:35 a.m. UTC 
Some of our tests access symbolic references via the filesystem
directly. While this works with the current files reference backend, it
this will break once we have a second reference backend in our codebase.

Refactor these tests to instead use git-symbolic-ref(1) or our
`ref-store` test tool. The latter is required in some cases where safety
checks of git-symbolic-ref(1) would otherwise reject writing a symbolic
reference.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t1400-update-ref.sh              |  8 ++++----
 t/t1430-bad-ref-name.sh            | 12 ++++++------
 t/t1450-fsck.sh                    |  4 ++--
 t/t3200-branch.sh                  |  9 ++++++---
 t/t4013-diff-various.sh            |  2 +-
 t/t4202-log.sh                     |  2 +-
 t/t5605-clone-local.sh             |  2 +-
 t/t5702-protocol-v2.sh             | 24 ++++++++++++++++++------
 t/t9133-git-svn-nested-git-repo.sh |  2 +-
 9 files changed, 40 insertions(+), 25 deletions(-)

Comments

Junio C Hamano Oct. 20, 2023, 7:52 p.m. UTC | #1 
Patrick Steinhardt <ps@pks.im> 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.
diff mbox series

Patch

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index cd24018ce99..5f505e2f353 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -221,15 +221,15 @@  test_expect_success 'delete symref without dereference when the referred ref is
 test_expect_success 'update-ref -d is not confused by self-reference' '
 	git symbolic-ref refs/heads/self refs/heads/self &&
 	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
-	test_path_is_file .git/refs/heads/self &&
+	git symbolic-ref --no-recurse refs/heads/self &&
 	test_must_fail git update-ref -d refs/heads/self &&
-	test_path_is_file .git/refs/heads/self
+	git symbolic-ref --no-recurse refs/heads/self
 '
 
 test_expect_success 'update-ref --no-deref -d can delete self-reference' '
 	git symbolic-ref refs/heads/self refs/heads/self &&
 	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
-	test_path_is_file .git/refs/heads/self &&
+	git symbolic-ref --no-recurse refs/heads/self &&
 	git update-ref --no-deref -d refs/heads/self &&
 	test_must_fail git show-ref --verify -q refs/heads/self
 '
@@ -239,7 +239,7 @@  test_expect_success 'update-ref --no-deref -d can delete reference to bad ref' '
 	test_when_finished "rm -f .git/refs/heads/bad" &&
 	git symbolic-ref refs/heads/ref-to-bad refs/heads/bad &&
 	test_when_finished "git update-ref -d refs/heads/ref-to-bad" &&
-	test_path_is_file .git/refs/heads/ref-to-bad &&
+	git symbolic-ref --no-recurse refs/heads/ref-to-bad &&
 	git update-ref --no-deref -d refs/heads/ref-to-bad &&
 	test_must_fail git show-ref --verify -q refs/heads/ref-to-bad
 '
diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
index 7b7d6953c62..5debb91f7b7 100755
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -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 &&
 	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
-	printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
+	test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/main &&
 	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
 	git for-each-ref >output 2>error &&
 	! grep -e "broken\.\.\.ref" output &&
@@ -257,7 +257,7 @@  test_expect_success 'update-ref -d can delete broken name through symref' '
 '
 
 test_expect_success 'update-ref --no-deref -d can delete symref with broken name' '
-	printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
+	test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/main &&
 	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
 	test_ref_exists refs/heads/broken...symref &&
 	git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
@@ -267,7 +267,7 @@  test_expect_success 'update-ref --no-deref -d can delete symref with broken name
 '
 
 test_expect_success 'branch -d can delete symref with broken name' '
-	printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
+	test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/main &&
 	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
 	test_ref_exists refs/heads/broken...symref &&
 	git branch -d broken...symref >output 2>error &&
@@ -277,7 +277,7 @@  test_expect_success 'branch -d can delete symref with broken name' '
 '
 
 test_expect_success 'update-ref --no-deref -d can delete dangling symref with broken name' '
-	printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
+	test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/idonotexist &&
 	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
 	test_ref_exists refs/heads/broken...symref &&
 	git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
@@ -287,7 +287,7 @@  test_expect_success 'update-ref --no-deref -d can delete dangling symref with br
 '
 
 test_expect_success 'branch -d can delete dangling symref with broken name' '
-	printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
+	test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/idonotexist &&
 	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
 	test_ref_exists refs/heads/broken...symref &&
 	git branch -d broken...symref >output 2>error &&
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 5cce24f1006..804a5594ddd 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -134,7 +134,7 @@  test_expect_success 'HEAD link pointing at a funny object' '
 test_expect_success 'HEAD link pointing at a funny place' '
 	saved_head=$(git rev-parse --verify HEAD) &&
 	test_when_finished "git update-ref --no-deref HEAD ${saved_head}" &&
-	echo "ref: refs/funny/place" >.git/HEAD &&
+	test-tool ref-store main create-symref HEAD refs/funny/place &&
 	# avoid corrupt/broken HEAD from interfering with repo discovery
 	test_must_fail env GIT_DIR=.git git fsck 2>out &&
 	test_i18ngrep "HEAD points to something strange" out
@@ -171,7 +171,7 @@  test_expect_success 'other worktree HEAD link pointing at missing object' '
 test_expect_success 'other worktree HEAD link pointing at a funny place' '
 	test_when_finished "rm -rf .git/worktrees other" &&
 	git worktree add other &&
-	echo "ref: refs/funny/place" >.git/worktrees/other/HEAD &&
+	git -C other symbolic-ref HEAD refs/funny/place &&
 	test_must_fail git fsck 2>out &&
 	test_i18ngrep "worktrees/other/HEAD points to something strange" out
 '
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index bde4f1485b7..874520e3f10 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -215,10 +215,13 @@  test_expect_success 'git branch -M should leave orphaned HEAD alone' '
 		cd orphan &&
 		test_commit initial &&
 		git checkout --orphan lonely &&
-		grep lonely .git/HEAD &&
+		git symbolic-ref HEAD >expect &&
+		echo refs/heads/lonely >actual &&
+		test_cmp expect actual &&
 		test_ref_missing refs/head/lonely &&
 		git branch -M main mistress &&
-		grep lonely .git/HEAD
+		git symbolic-ref HEAD >expect &&
+		test_cmp expect actual
 	)
 '
 
@@ -809,7 +812,7 @@  test_expect_success 'deleting a symref' '
 
 test_expect_success 'deleting a dangling symref' '
 	git symbolic-ref refs/heads/dangling-symref nowhere &&
-	test_path_is_file .git/refs/heads/dangling-symref &&
+	git symbolic-ref --no-recurse refs/heads/dangling-symref &&
 	echo "Deleted branch dangling-symref (was nowhere)." >expect &&
 	git branch -d dangling-symref >actual &&
 	test_ref_missing refs/heads/dangling-symref &&
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 5de1d190759..5abbea36b39 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -514,7 +514,7 @@  test_expect_success 'log -S requires an argument' '
 '
 
 test_expect_success 'diff --cached on unborn branch' '
-	echo ref: refs/heads/unborn >.git/HEAD &&
+	git symbolic-ref HEAD refs/heads/unborn &&
 	git diff --cached >result &&
 	process_diffs result >actual &&
 	process_diffs "$TEST_DIRECTORY/t4013/diff.diff_--cached" >expected &&
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index af4a123cd22..57b298a4e22 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -2265,7 +2265,7 @@  test_expect_success REFFILES 'log diagnoses bogus HEAD hash' '
 
 test_expect_success REFFILES 'log diagnoses bogus HEAD symref' '
 	git init empty &&
-	echo "ref: refs/heads/invalid.lock" > empty/.git/HEAD &&
+	test-tool -C empty ref-store main create-symref 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 &&
diff --git a/t/t5605-clone-local.sh b/t/t5605-clone-local.sh
index 946c5751885..bedd29d0550 100755
--- a/t/t5605-clone-local.sh
+++ b/t/t5605-clone-local.sh
@@ -65,7 +65,7 @@  test_expect_success 'Even without -l, local will make a hardlink' '
 '
 
 test_expect_success 'local clone of repo with nonexistent ref in HEAD' '
-	echo "ref: refs/heads/nonexistent" > a.git/HEAD &&
+	git -C a.git symbolic-ref HEAD refs/heads/nonexistent &&
 	git clone a d &&
 	(cd d &&
 	git fetch &&
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index 6af5c2062fd..dcc4cd95fe7 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -221,7 +221,9 @@  test_expect_success 'clone of empty repo propagates name of default branch' '
 	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= \
 	git -c init.defaultBranch=main -c protocol.version=2 \
 		clone "file://$(pwd)/file_empty_parent" file_empty_child &&
-	grep "refs/heads/mydefaultbranch" file_empty_child/.git/HEAD
+	echo refs/heads/mydefaultbranch >expect &&
+	git -C file_empty_child symbolic-ref HEAD >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success '...but not if explicitly forbidden by config' '
@@ -234,7 +236,9 @@  test_expect_success '...but not if explicitly forbidden by config' '
 	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= \
 	git -c init.defaultBranch=main -c protocol.version=2 \
 		clone "file://$(pwd)/file_empty_parent" file_empty_child &&
-	! grep "refs/heads/mydefaultbranch" file_empty_child/.git/HEAD
+	echo refs/heads/main >expect &&
+	git -C file_empty_child symbolic-ref HEAD >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'bare clone propagates empty default branch' '
@@ -247,7 +251,9 @@  test_expect_success 'bare clone propagates empty default branch' '
 	git -c init.defaultBranch=main -c protocol.version=2 \
 		clone --bare \
 		"file://$(pwd)/file_empty_parent" file_empty_child.git &&
-	grep "refs/heads/mydefaultbranch" file_empty_child.git/HEAD
+	echo "refs/heads/mydefaultbranch" >expect &&
+	git -C file_empty_child.git symbolic-ref HEAD >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'clone propagates unborn HEAD from non-empty repo' '
@@ -265,7 +271,9 @@  test_expect_success 'clone propagates unborn HEAD from non-empty repo' '
 	git -c init.defaultBranch=main -c protocol.version=2 \
 		clone "file://$(pwd)/file_unborn_parent" \
 		file_unborn_child 2>stderr &&
-	grep "refs/heads/mydefaultbranch" file_unborn_child/.git/HEAD &&
+	echo "refs/heads/mydefaultbranch" >expect &&
+	git -C file_unborn_child symbolic-ref HEAD >actual &&
+	test_cmp expect actual &&
 	grep "warning: remote HEAD refers to nonexistent ref" stderr
 '
 
@@ -295,7 +303,9 @@  test_expect_success 'bare clone propagates unborn HEAD from non-empty repo' '
 	git -c init.defaultBranch=main -c protocol.version=2 \
 		clone --bare "file://$(pwd)/file_unborn_parent" \
 		file_unborn_child.git 2>stderr &&
-	grep "refs/heads/mydefaultbranch" file_unborn_child.git/HEAD &&
+	echo "refs/heads/mydefaultbranch" >expect &&
+	git -C file_unborn_child.git symbolic-ref HEAD >actual &&
+	test_cmp expect actual &&
 	! grep "warning:" stderr
 '
 
@@ -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 &&