Subject: Re: [PATCH v2] refspec: make @ a synonym of HEAD

From: Junio C Hamano <[email-masked]>
Date: Tue, 24 Nov 2020 16:36:59 -0800

Felipe Contreras <[email-masked]> writes:

> diff --git a/t/t5511-refspec.sh b/t/t5511-refspec.sh
> index f541f30bc2..f808649de4 100755
> --- a/t/t5511-refspec.sh
> +++ b/t/t5511-refspec.sh
> @@ -58,6 +58,8 @@ test_refspec fetch 'HEAD~4:refs/remotes/frotz/new'		invalid
>  
>  test_refspec push 'HEAD'
>  test_refspec fetch 'HEAD'
> +test_refspec push '@'
> +test_refspec fetch '@'

OK.

> +test_expect_success 'push @ with non-existent, incomplete dest' '
> +
> +	mk_test testrepo &&
> +	git checkout master &&
> +	git push testrepo @:branch &&
> +	check_push_result testrepo $the_commit heads/branch
> +
> +'
> +
>  test_expect_success 'push with config remote.*.push = HEAD' '
>  
>  	mk_test testrepo heads/local &&
> @@ -501,6 +510,15 @@ test_expect_success 'push with config remote.*.push = HEAD' '
>  	check_push_result testrepo $the_first_commit heads/local
>  '
>  
> +test_expect_success 'push with @' '
> +
> +	mk_test testrepo heads/master &&
> +	git checkout master &&
> +	git push testrepo @ &&
> +	check_push_result testrepo $the_commit heads/master
> +
> +'

This is OK, but shouldn't this be placed before the tests with
various configuration?  Something along the lines of the attached,
but with the body of the loop properly reindented, would also give
us a better test coverage at the same time.

 t/t5516-fetch-push.sh | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git c/t/t5516-fetch-push.sh w/t/t5516-fetch-push.sh
index d11382f769..0b015a8d60 100755
--- c/t/t5516-fetch-push.sh
+++ w/t/t5516-fetch-push.sh
@@ -436,24 +436,27 @@ test_expect_success 'push ref expression with non-existent, incomplete dest' '
 
 '
 
-test_expect_success 'push with HEAD' '
+for HEAD in HEAD @
+do
+
+test_expect_success "push with $HEAD" '
 
 	mk_test testrepo heads/master &&
 	git checkout master &&
-	git push testrepo HEAD &&
+	git push testrepo $HEAD &&
 	check_push_result testrepo $the_commit heads/master
 
 '
 
-test_expect_success 'push with HEAD nonexisting at remote' '
+test_expect_success "push with $HEAD nonexisting at remote" '
 
 	mk_test testrepo heads/master &&
 	git checkout -b local master &&
-	git push testrepo HEAD &&
+	git push testrepo $HEAD &&
 	check_push_result testrepo $the_commit heads/local
 '
 
-test_expect_success 'push with +HEAD' '
+test_expect_success "push with +$HEAD" '
 
 	mk_test testrepo heads/master &&
 	git checkout master &&
@@ -464,25 +467,27 @@ test_expect_success 'push with +HEAD' '
 	check_push_result testrepo $the_commit heads/local &&
 
 	# Without force rewinding should fail
-	git reset --hard HEAD^ &&
-	test_must_fail git push testrepo HEAD &&
+	git reset --hard $HEAD^ &&
+	test_must_fail git push testrepo $HEAD &&
 	check_push_result testrepo $the_commit heads/local &&
 
 	# With force rewinding should succeed
-	git push testrepo +HEAD &&
+	git push testrepo +$HEAD &&
 	check_push_result testrepo $the_first_commit heads/local
 
 '
 
-test_expect_success 'push HEAD with non-existent, incomplete dest' '
+test_expect_success "push $HEAD with non-existent, incomplete dest" '
 
 	mk_test testrepo &&
 	git checkout master &&
-	git push testrepo HEAD:branch &&
+	git push testrepo $HEAD:branch &&
 	check_push_result testrepo $the_commit heads/branch
 
 '
 
+done
+
 test_expect_success 'push with config remote.*.push = HEAD' '
 
 	mk_test testrepo heads/local &&