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

Junio C Hamano venit, vidit, dixit 26.02.2009 18:09:

Michael J Gruber [off-list ref] writes:

quoted

First of all: I define good/bad as matching the documentation.

Ok, I was primarily working from this:

commit bcc785f611dc6084be75999a3b6bafcc950e21d6
Author: Linus Torvalds [off-list ref]
Date:   Mon Oct 30 08:28:59 2006 -0800

[snip]

You will see that:

 (1) bf07cc5 (i.e. J6t's documentation) passes your tests;

Hmm, I don't see that, we must be doing something differently, see below.

 (2) somewhere between that and v1.6.2-rc2, there is a regression to make
     your test fail.

if the above conjecture is true, and we may want to fix that regression to
match the documentation.

On the other hand, if bf07cc5 does not pass your tests, it means that the
documentation update was the cause of the confusion, and it is not the
behaviour that needs to be fixed.

Sorry, but I do not have time today to look into this.  Could you help?

I'll try and provide the necessary info below, but I think there is a
decision to be made about the desired order of priorities for "repo
argument", "repo option (--repo=)" and "tracking info config
(branch.$branch.remote=)".

Current code: argument > option > track config > 'origin'
Current doc: argument > track config > option > 'origin'

I'd say the documented version is more natural, meaning --repo=$repo is
fully equivalent to changing "origin" to "$repo".

Now, here's if you want to know the details:

When Linus introduced "--repo" there was no heeding of the tracking info
at all:

bcc785f (git push: add verbose flag and allow overriding of default
target repository, 2006-10-30) Linus introduces --repo.

5751f49 (Move remote parsing into a library file out of builtin-push.,
2007-05-12) Daniel Barkalow introduces the use of branch.$branch.remote
info for the repo if git push has no other repo arguments nor --repo
options.

378c483 (Use parseopts in builtin-push, 2007-11-04) Daniel switches
git-push to parseopts, without changing the behaviour.

bf07cc5 (git-push.txt: Describe --repo option in more detail,
2008-10-07) J6t writes the current version of the doc.

Using a trimmed down ./t5516-fetch-push.sh from my patch (and a copy of
current test-lib), all 4 of them give the same results:

*   ok 1: setup
*   ok 2: push with --repo=repourl from non-tracking branch
*   ok 3: push with --repo=remoterepo from non-tracking branch
* FAIL 4: 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

* FAIL 5: 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

[testrepo exists, origin does not]

Note that bcc785f was not even supposed to heed the value of
branch.master.remote, so it's clear they fail 4&5. 2&3 only test whether
--repo is considered if there's no other info, so it's clear they pass
(versions without bcc785f would fail).

Cheers,
Michael