git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
May 25, 2018, 08:48 UTC
Message-ID
<615f57ad-7591-128a-0c42-660312d34ca2@alum.mit.edu>
In-Reply-To
<20180523052517.4443-1-chriscool@tuxfamily.org>
On 05/23/2018 07:25 AM, Christian Couder wrote:
Show 21 quoted lines
> From: David Turner <dturner@twopensource.com>
> 
> Many tests are very focused on the file system representation of the
> loose and packed refs code. As there are plans to implement other
> ref storage systems, let's migrate these tests to a form that test
> the intent of the refs storage system instead of it internals.
> [...]
> 
> diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
> index 9e782a8122..a4ebb0b65f 100755
> --- a/t/t1401-symbolic-ref.sh
> +++ b/t/t1401-symbolic-ref.sh
> @@ -65,7 +65,7 @@ reset_to_sane
>  test_expect_success 'symbolic-ref fails to delete real ref' '
>  	echo "fatal: Cannot delete refs/heads/foo, not a symbolic ref" >expect &&
>  	test_must_fail git symbolic-ref -d refs/heads/foo >actual 2>&1 &&
> -	test_path_is_file .git/refs/heads/foo &&
> +	git rev-parse --verify refs/heads/foo &&
>  	test_cmp expect actual
>  '
>  reset_to_sane

Should t1401 be considered a backend-agnostic test, or is it needed to ensure that symbolic refs are written correctly in the files backend?

Show 163 quoted lines
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index c0ef946811..222dc2c377 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -234,34 +234,34 @@ test_expect_success 'git branch -M master2 master2 should work when master is ch
>  
>  test_expect_success 'git branch -v -d t should work' '
>  	git branch t &&
> -	test_path_is_file .git/refs/heads/t &&
> +	git rev-parse --verify refs/heads/t &&
>  	git branch -v -d t &&
> -	test_path_is_missing .git/refs/heads/t
> +	test_must_fail git rev-parse --verify refs/heads/t
>  '
>  
>  test_expect_success 'git branch -v -m t s should work' '
>  	git branch t &&
> -	test_path_is_file .git/refs/heads/t &&
> +	git rev-parse --verify refs/heads/t &&
>  	git branch -v -m t s &&
> -	test_path_is_missing .git/refs/heads/t &&
> -	test_path_is_file .git/refs/heads/s &&
> +	test_must_fail git rev-parse --verify refs/heads/t &&
> +	git rev-parse --verify refs/heads/s &&
>  	git branch -d s
>  '
>  
>  test_expect_success 'git branch -m -d t s should fail' '
>  	git branch t &&
> -	test_path_is_file .git/refs/heads/t &&
> +	git rev-parse refs/heads/t &&
>  	test_must_fail git branch -m -d t s &&
>  	git branch -d t &&
> -	test_path_is_missing .git/refs/heads/t
> +	test_must_fail git rev-parse refs/heads/t
>  '
>  
>  test_expect_success 'git branch --list -d t should fail' '
>  	git branch t &&
> -	test_path_is_file .git/refs/heads/t &&
> +	git rev-parse refs/heads/t &&
>  	test_must_fail git branch --list -d t &&
>  	git branch -d t &&
> -	test_path_is_missing .git/refs/heads/t
> +	test_must_fail git rev-parse refs/heads/t
>  '
>  
>  test_expect_success 'git branch --list -v with --abbrev' '
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index aefde7b172..1f871d3cca 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -726,7 +726,7 @@ test_expect_success 'store updates stash ref and reflog' '
>  	git reset --hard &&
>  	! grep quux bazzy &&
>  	git stash store -m quuxery $STASH_ID &&
> -	test $(cat .git/refs/stash) = $STASH_ID &&
> +	test $(git rev-parse stash) = $STASH_ID &&
>  	git reflog --format=%H stash| grep $STASH_ID &&
>  	git stash pop &&
>  	grep quux bazzy
> diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
> index 0680dec808..d4f435155f 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" &&
>  	eval ${branch}TIP=$commit
>  }
>  
> @@ -45,10 +45,10 @@ pull_to_client () {
>  
>  			case "$heads" in
>  			    *A*)
> -				    echo $ATIP > .git/refs/heads/A;;
> +				    git update-ref refs/heads/A "$ATIP";;
>  			esac &&
>  			case "$heads" in *B*)
> -			    echo $BTIP > .git/refs/heads/B;;
> +			    git update-ref refs/heads/B "$BTIP";;
>  			esac &&
>  			git symbolic-ref HEAD refs/heads/$(echo $heads \
>  				| sed -e "s/^\(.\).*$/\1/") &&
> @@ -92,8 +92,8 @@ test_expect_success 'setup' '
>  		cur=$(($cur+1))
>  	done &&
>  	add B1 $A1 &&
> -	echo $ATIP > .git/refs/heads/A &&
> -	echo $BTIP > .git/refs/heads/B &&
> +	git update-ref refs/heads/A "$ATIP" &&
> +	git update-ref refs/heads/B "$BTIP" &&
>  	git symbolic-ref HEAD refs/heads/B
>  '
>  
> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> index ae5a530a2d..e402aee6a2 100755
> --- a/t/t5510-fetch.sh
> +++ b/t/t5510-fetch.sh
> @@ -63,7 +63,7 @@ test_expect_success "fetch test" '
>  	git commit -a -m "updated by origin" &&
>  	cd two &&
>  	git fetch &&
> -	test -f .git/refs/heads/one &&
> +	git rev-parse --verify refs/heads/one &&
>  	mine=$(git rev-parse refs/heads/one) &&
>  	his=$(cd ../one && git rev-parse refs/heads/master) &&
>  	test "z$mine" = "z$his"
> @@ -73,8 +73,8 @@ test_expect_success "fetch test for-merge" '
>  	cd "$D" &&
>  	cd three &&
>  	git fetch &&
> -	test -f .git/refs/heads/two &&
> -	test -f .git/refs/heads/one &&
> +	git rev-parse --verify refs/heads/two &&
> +	git rev-parse --verify refs/heads/one &&
>  	master_in_two=$(cd ../two && git rev-parse master) &&
>  	one_in_two=$(cd ../two && git rev-parse one) &&
>  	{
> diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh
> index 31db7b5f91..aa2d360ce3 100755
> --- a/t/t6010-merge-base.sh
> +++ b/t/t6010-merge-base.sh
> @@ -34,7 +34,7 @@ doit () {
>  
>  	commit=$(echo $NAME | git commit-tree $T $PARENTS) &&
>  
> -	echo $commit >.git/refs/tags/$NAME &&
> +	git update-ref "refs/tags/$NAME" "$commit" &&
>  	echo $commit
>  }
>  
> diff --git a/t/t7201-co.sh b/t/t7201-co.sh
> index 76c223c967..ab9da61da3 100755
> --- a/t/t7201-co.sh
> +++ b/t/t7201-co.sh
> @@ -65,7 +65,7 @@ test_expect_success setup '
>  test_expect_success "checkout from non-existing branch" '
>  
>  	git checkout -b delete-me master &&
> -	rm .git/refs/heads/delete-me &&
> +	git update-ref -d --no-deref refs/heads/delete-me &&
>  	test refs/heads/delete-me = "$(git symbolic-ref HEAD)" &&
>  	git checkout master &&
>  	test refs/heads/master = "$(git symbolic-ref HEAD)"
> diff --git a/t/t9104-git-svn-follow-parent.sh b/t/t9104-git-svn-follow-parent.sh
> index a735fa3717..9c49b6c1fe 100755
> --- a/t/t9104-git-svn-follow-parent.sh
> +++ b/t/t9104-git-svn-follow-parent.sh
> @@ -215,7 +215,8 @@ test_expect_success "multi-fetch continues to work" "
>  	"
>  
>  test_expect_success "multi-fetch works off a 'clean' repository" '
> -	rm -r "$GIT_DIR/svn" "$GIT_DIR/refs/remotes" "$GIT_DIR/logs" &&
> +	rm -rf "$GIT_DIR/svn" "$GIT_DIR/refs/remotes" &&
> +	git reflog expire --all --expire=all &&
>  	mkdir "$GIT_DIR/svn" &&
>  	git svn multi-fetch
>  	'
> 
`rm -rf "$GIT_DIR/refs/remotes"` is not kosher. I think it can be written
    printf 'option no-deref\ndelete %s\n' $(git for-each-ref
--format='%(refname)' refs/remotes) | git update-ref --stdin

as long as the number of references doesn't exceed command-line limits. This will also take care of the reflogs. Another alternative would be to write it as a loop.

Michael
Previous: Junio C HamanoNext: Jeff King
Message 3 of 7 in “t: make many tests depend less on the refs being files”
  1. t: make many tests depend less on the refs being filesChristian Couder, May 23, 2018
  2. Junio C HamanoMay 23, 2018
  3. Michael HaggertyMay 25, 2018
  4. Jeff KingMay 25, 2018
  5. Michael HaggertyMay 25, 2018
  6. Christian CouderMay 25, 2018
  7. Christian CouderMay 25, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.