Re: [PATCH] t: make many tests depend less on the refs being files

Hi,

of course I messed up and did not fix the Cc: list... now fixed ;-)


On Mon, 21 May 2018, Johannes Schindelin wrote:

Hi Chris,

On Mon, 21 May 2018, Christian Couder wrote:

quoted

From: David Turner <redacted>

I vaguely remember that Dave suggested using a different email address
these days...

*clicketyclick*

ah, yes:
https://public-inbox.org/git/1474935093-26757-3-git-send-email-dturner@twosigma.com/

So maybe update it here, too, to 

	From: David Turner [off-list ref]

?

quoted

So that they work under alternate ref storage backends.

This will be really needed when such alternate ref storage backends are
developed. But this could already help by making clear to readers that
some tests do not depend on which ref backend is used.

This patch just takes care of many low hanging fruits. It does not try
to completely solves the issue.

Signed-off-by: David Turner <redacted>
Signed-off-by: Christian Couder <redacted>
---

This is great. I am all for making the tests better ;-)

quoted

diff --git a/t/lib-t6000.sh b/t/lib-t6000.sh
index 3f2d873fec..b8567cdf94 100644
--- a/t/lib-t6000.sh
+++ b/t/lib-t6000.sh

@@ -4,11 +4,10 @@ mkdir -p .git/refs/tags
 
 >sed.script
 
-# Answer the sha1 has associated with the tag. The tag must exist in .git/refs/tags
+# Answer the sha1 has associated with the tag. The tag must exist under refs/tags
 tag () {
 	_tag=$1
-	test -f ".git/refs/tags/$_tag" || error "tag: \"$_tag\" does not exist"
-	cat ".git/refs/tags/$_tag"
+	git rev-parse --verify "refs/tags/$_tag" || error "tag: \"$_tag\" does not exist"

Line longer than 80 columns...

quoted

diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 0680dec808..886a9e3b72 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh

@@ -30,7 +30,7 @@ add () {
 	test_tick &&
 	commit=$(echo "$text" | git commit-tree $tree $parents) &&
 	eval "$name=$commit; export $name" &&
-	echo $commit > .git/refs/heads/$branch &&
+	git update-ref refs/heads/$branch $commit &&

I think we have to be careful here to quote both "refs/heads/$branch" and
"$commit" here, as a bug that introduces spaces into $commit or $branch
would have been caught earlier, but not now, unless we quote.

This goes for all introduced `update-ref` calls.

Maybe even for some `git rev-parse --verify` calls.

quoted

diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 8f5c811dd7..c3b89ae783 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh

@@ -148,7 +148,7 @@ test_expect_success 'prompt - inside .git directory' '
 test_expect_success 'prompt - deep inside .git directory' '
 	printf " (GIT_DIR!)" >expected &&
 	(
-		cd .git/refs/heads &&
+		cd .git/objects &&
 		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"

-- 

This one looks wrong.

Upon cursory review, one might be tempted to assume that the file is now
written into .git/objects/ instead of .git/refs/heads/. And the patch
context provided in the email is not enough to see (gawd, I hate
mail-based patch review, it really takes all my Git tools away from me).
The trick is that `actual` points at an absolute path:

	#!/bin/sh
	#
	# Copyright (c) 2012 SZEDER Gábor
	#

	test_description='test git-specific bash prompt functions'

	. ./lib-bash.sh

	. "$GIT_BUILD_DIR/contrib/completion/git-prompt.sh"

	actual="$TRASH_DIRECTORY/actual"
	[...]

So the remaining question (which probably wants to be added to the commit
message together with a hint that `actual` points at an absolute path) is:
Why not `cd .git` instead?

Ciao,
Dscho