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

Re: [PATCH] t5570: use explicit push refspec

From
Clemens Buchacher <drizzd@aon.at>
Date
Apr 15, 2012, 19:52 UTC
Message-ID
<20120415195231.GB1960@ecki>
In-Reply-To
<7v7gxg2461.fsf@alter.siamese.dyndns.org>
On Sun, Apr 15, 2012 at 12:20:06PM -0700, Junio C Hamano wrote:
Show 22 quoted lines
> Clemens Buchacher <drizzd@aon.at> writes:
> 
> > On Sat, Apr 14, 2012 at 04:40:01PM -0700, Junio C Hamano wrote:
> >> Clemens Buchacher <drizzd@aon.at> writes:
> >> 
> >> > The default mode for push without arguments will change. Some warnings
> >> > are about to be enabled for such use, which causes some t5570 tests to
> >> > fail because they do not expect this output. Fix this by passing an
> >> > explicit refspec to git push.
> >> 
> >> I wonder if a better fix is to configure "push.default = matching" in the
> >> test repository.  Otherwise wouldn't the result of the push change once
> >> the default changes?
> >
> > The push.default option matters only if a refspec is not specified. By
> > adding a refspec, push.default should not matter any more. Unless that
> > is going to change as well?
> 
> No, I was thinking more about testing cases where there is no refspec on
> the command line, which we used to test, but with your patch we no longer
> do.  In other words, your fix not just squelches the advice message and
> make them pass, but it changes the way the command behaves, no?

It does exactly the same thing it did before, since there is only one local branch. The goal of the test is not to check behavior of git push without arguments. Since that is subject to change, and is causing causing the test to fail already for no good reason, I think it better to use the more explicit version of the command.

> Besides, that way you do not have to swap the parameters to test_remote_error
> so we do not have scratch our heads wondering why we have changes to test
> vectors that run clones and fetches.

In my opinion, this change is also an improvement in itself, since now we can more easily pass extra arguments to test_remote_error. Maybe the scratching of heads can be alleviated by amending the commit message like so?

-->o--
Subject: [PATCH] t5570: use explicit push refspec

The default mode for push without arguments will change. Some warnings are about to be enabled for such use, which causes some t5570 tests to fail because they do not expect this output.

Fix this by passing an explicit refspec to git push. To that end, change the calling conventions of test_remote_error in order to accomodate extra command arguments.

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---
 t/t5570-git-daemon.sh |   30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
index 7cbc999..a3a4e47 100755
--- a/t/t5570-git-daemon.sh
+++ b/t/t5570-git-daemon.sh
@@ -103,14 +103,12 @@ test_remote_error()
 		esac
 	done
 
-	if test $# -ne 3
-	then
-		error "invalid number of arguments"
-	fi
-
+	msg=$1
+	shift
 	cmd=$1
-	repo=$2
-	msg=$3
+	shift
+	repo=$1
+	shift || error "invalid number of arguments"
 
 	if test -x "$GIT_DAEMON_DOCUMENT_ROOT_PATH/$repo"
 	then
@@ -122,7 +120,7 @@ test_remote_error()
 		fi
 	fi
 
-	test_must_fail git "$cmd" "$GIT_DAEMON_URL/$repo" 2>output &&
+	test_must_fail git "$cmd" "$GIT_DAEMON_URL/$repo" "$@" 2>output &&
 	echo "fatal: remote error: $msg: /$repo" >expect &&
 	test_cmp expect output
 	ret=$?
@@ -131,18 +129,18 @@ test_remote_error()
 }
 
 msg="access denied or repository not exported"
-test_expect_success 'clone non-existent' "test_remote_error    clone nowhere.git '$msg'"
-test_expect_success 'push disabled'      "test_remote_error    push  repo.git    '$msg'"
-test_expect_success 'read access denied' "test_remote_error -x fetch repo.git    '$msg'"
-test_expect_success 'not exported'       "test_remote_error -n fetch repo.git    '$msg'"
+test_expect_success 'clone non-existent' "test_remote_error    '$msg' clone nowhere.git    "
+test_expect_success 'push disabled'      "test_remote_error    '$msg' push  repo.git master"
+test_expect_success 'read access denied' "test_remote_error -x '$msg' fetch repo.git       "
+test_expect_success 'not exported'       "test_remote_error -n '$msg' fetch repo.git       "
 
 stop_git_daemon
 start_git_daemon --informative-errors
 
-test_expect_success 'clone non-existent' "test_remote_error    clone nowhere.git 'no such repository'"
-test_expect_success 'push disabled'      "test_remote_error    push  repo.git    'service not enabled'"
-test_expect_success 'read access denied' "test_remote_error -x fetch repo.git    'no such repository'"
-test_expect_success 'not exported'       "test_remote_error -n fetch repo.git    'repository not exported'"
+test_expect_success 'clone non-existent' "test_remote_error    'no such repository'      clone nowhere.git    "
+test_expect_success 'push disabled'      "test_remote_error    'service not enabled'     push  repo.git master"
+test_expect_success 'read access denied' "test_remote_error -x 'no such repository'      fetch repo.git       "
+test_expect_success 'not exported'       "test_remote_error -n 'repository not exported' fetch repo.git       "
 
 stop_git_daemon
 test_done
-- 
1.7.9.6
Previous: Junio C HamanoNext: Junio C Hamano
Message 6 of 18 in “git-daemon wrapper to wait until daemon is ready”
  1. git-daemon wrapper to wait until daemon is readyClemens Buchacher, Apr 14, 2012
  2. t5570: use explicit push refspecClemens Buchacher, Apr 14, 2012
  3. Junio C HamanoApr 14, 2012
  4. Clemens BuchacherApr 15, 2012
  5. Junio C HamanoApr 15, 2012
  6. Clemens BuchacherApr 15, 2012
  7. Junio C HamanoApr 15, 2012
  8. Ben WaltonApr 14, 2012
  9. Clemens BuchacherApr 14, 2012
  10. git-daemon wrapper to wait until daemon is readyClemens Buchacher, Apr 14, 2012
  11. Johannes SixtApr 14, 2012
  12. Clemens BuchacherApr 14, 2012
  13. git-daemon wrapper to wait until daemon is readyClemens Buchacher, Apr 15, 2012
  14. Zbigniew Jędrzejewski-SzmekApr 16, 2012
  15. Junio C HamanoApr 19, 2012
  16. Johannes SixtApr 15, 2012
  17. Clemens BuchacherApr 15, 2012
  18. Johannes SixtApr 15, 2012

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.