Re: [RFC] add test cases for the --repo option to git push

Junio C Hamano venit, vidit, dixit 25.02.2009 22:58:

Michael J Gruber [off-list ref] writes:

quoted

@@ -419,6 +419,41 @@ test_expect_success 'push with config remote.*.push = HEAD' '
 git config --remove-section remote.there
 git config --remove-section branch.master
 
+test_expect_success 'push with --repo=repourl from non-tracking branch' '
+
+	mk_test heads/master &&
+	git push --repo=testrepo &&
+	check_push_result $the_commit heads/master
+'
+
+# set up fake remote config
+test_expect_success 'push with --repo=remoterepo from non-tracking branch' '
+
+	mk_test heads/master &&
+	git config remote.testremote.url testrepo &&
+	git push --repo=testremote &&
+	check_push_result $the_commit heads/master
+'
+
+# set up fake tracking info; testrepo exists, origin does not.
+test_expect_failure 'push with --repo=repo from tracking branch with bad config' '
+
+	mk_test heads/master &&
+	git config branch.master.remote origin &&
+	test_must_fail git push --repo=testrepo
+'

At this point, you have:

	branch.master.remote = origin
        remote.testremote.url = testrepo

and nothing else related to push.  And you are asking a "git push but
instead of origin please default to testrepo".

In response to that request, in order to figure out what refs to push by
default, remote.testremote.push instead of remote.origin.push will be
consulted, and we won't even have to notice nor complain the missing
remote.origin.  remote.testremote.push is missing, so the push defaults to
the "matching ref" behaviour.

I do not understand why you expect the above to fail.

First of all: I define good/bad as matching the documentation. Current
behaviour does not match the doc: "git push--repo=repo" is equivalent to
"git push repo" with the current code. One could simply document it as
such, of course. I took the other approach: Test for documented
behaviour and adjust the code.

Now for the test above:
I expect:
git push looks whether the branch tracks a remote.
It finds "origin" due to branch.master.remote=origin
It looks for a push refspec.
It finds none thus uses the default.
It pushes to origin with the default refspec and fails because there is
no remote origin.
This is why I have test_must_fail

I get with current behaviour:
git takes the --repo=testrepo argument instead of the tracking info.
push succeeds accordingly.
This is why I have test_expect_failure.

After the fix in PATCH 2/2, the behaviour matches exactly what I
described under expect. Note that I did not change the handling of
tracking info etc. at all in the PATCH, I only changed the treatment of
the --repo option and produced the expected behaviour.

quoted

+test_expect_failure 'push with --repo=repo from tracking branch with good config' '
+
+	mk_test heads/master &&
+	git config branch.master.remote testrepo &&
+	git push --repo=origin &&
+	check_push_result $the_commit heads/master
+'

Likewise.

Uhm, yes, likewise ;)
Expect:
git push looks whether the branch tracks a remote.
It finds "testrepo" due to branch.master.remote=testrepo
It looks for a push refspec.
It finds none thus uses the default.
It pushes to testrepo with the default refspec and succeeds (exists, has
matching refs).
For extra safety, I check that the push produced the correct ref on the
remote side.
This is why I expect it to succeed.

I get with current behaviour:
git takes the --repo=origin argument instead of the tracking info.
push fails because there is no remote origin.
This is why I have test_expect_failure.

Note that according to the doc, "git push --repo=origin" is equivalent
to "git push", but this test shows that it is not currently.

After PATCH 2/2 the test succeeds as expected.

I think the "good/bad config" labels given to your two test scripts are
swapped and there is no bug or misfeature here.

Well, there's a discrepancy between doc and behaviour, as reported by
Jay. The proposed patch brings them in line the hard way
(fixing/adjusting the code).

Michael