Ralf Thielow <ralf.thie...@gmail.com> writes:

> - add tests for the refspec installed by the clone command

Thanks.

> +test_expect_success 'refspec contains all branches by default' '
> +     git clone "file://$PWD" dir_all &&

There have been numerous "on windows which should we use, $PWD or
$(pwd)?" gotchas.  In this particular case, wouldn't the tests work
equally well with "." to avoid the uneasiness factor?

> +     echo "+refs/heads/*:refs/remotes/origin/*" > expected &&
> +     git --git-dir=dir_all/.git config --get remote.origin.fetch > actual &&
> +     test_cmp expected actual

I am a bit torn with this one.

We'd want to make sure a single clone from a repository with two
branches will not result in a repository where the next fetch will
not pull in the other branch, and the value of the
"remote.origin.fetch" is an implementation detail that happen to be
what affects the outcome.  The test is not checking the expected
outcome in a direct way (imagine how this test will break when we do
another change similar to the migration from .git/remotes/origin to
the remote.origin.fetch variables).

I'll let it pass for now, though.

> +test_expect_success 'no refspec is written if remotes HEAD is detached' '
> +     git checkout two^ &&
> +     git clone --single-branch "file://$PWD" dir_detached &&
> +     rm expected && touch expected &&

If earlier tests failed, there may not be any expected file and the
"rm expected" will fail.  You can just say

        >expected &&

instead.  "touch" is a way to update the timestamp of the file; do
not use it when you want to make sure an empty file exists.

> +     git --git-dir=dir_detached/.git config --get remote.origin.fetch > 
> actual
> +     test_cmp expected actual
> +'

I'd feel better if the test were like this instead:

        git checkout two^ &&
        git clone --single-branch . dir_detached &&
        (
                cd dir_detached &&
                git fetch &&
                git for-each-ref refs/remotes/origin >actual
        ) &&
        >expect &&
        test_cmp expect actual

That is what I would call "testing the desired results in the most
direct way".

Perhaps like this?

-- >8 -- t5709-clone-refspec.sh -- >8 --
#!/bin/sh

test_description='test refspec written by clone-command'
. ./test-lib.sh

test_expect_success 'setup' '
        # Make two branches, "master" and "side"
        echo one >file &&
        git add file &&
        git commit -m one &&
        echo two >file &&
        git commit -a -m two &&
        git tag two &&
        echo three >file &&
        git commit -a -m three &&
        git checkout -b side &&
        echo four >file &&
        git commit -a -m four &&
        git checkout master &&

        # default clone
        git clone . dir_all &&

        # default --single that follows HEAD=master
        git clone --single-branch . dir_master &&

        # default --single that follows HEAD=side
        git checkout side &&
        git clone --single-branch . dir_side &&

        # explicit --single that follows side
        git checkout master &&
        git clone --single-branch --branch side . dir_side2 &&

        # --single that does not know what branch to follow
        git checkout two^ &&
        git clone --single-branch . dir_detached &&

        # advance both "master" and "side" branches
        git checkout side &&
        echo five >file &&
        git commit -a -m five &&
        git checkout master &&
        echo six >file &&
        git commit -a -m six
'

test_expect_success 'by default all branches will be kept updated' '
        (
                cd dir_all && git fetch &&
                git for-each-ref refs/remotes/origin |
                sed -e "/HEAD$/d" \
                    -e "s|/remotes/origin/|/heads/|" >../actual
        ) &&
        # follow both master and side
        git for-each-ref refs/heads >expect &&
        test_cmp expect actual
'

test_expect_success '--single-branch while HEAD pointing at master' '
        (
                cd dir_master && git fetch &&
                git for-each-ref refs/remotes/origin |
                sed -e "/HEAD$/d" \
                    -e "s|/remotes/origin/|/heads/|" >../actual
        ) &&
        # only follow master
        git for-each-ref refs/heads/master >expect &&
        test_cmp expect actual
'

test_expect_success '--single-branch while HEAD pointing at side' '
        (
                cd dir_side && git fetch &&
                git for-each-ref refs/remotes/origin |
                sed -e "/HEAD$/d" \
                    -e "s|/remotes/origin/|/heads/|" >../actual
        ) &&
        # only follow side
        git for-each-ref refs/heads/side >expect &&
        test_cmp expect actual
'

test_expect_success '--single-branch with explicit --branch side' '
        (
                cd dir_side2 && git fetch &&
                git for-each-ref refs/remotes/origin |
                sed -e "/HEAD$/d" \
                    -e "s|/remotes/origin/|/heads/|" >../actual
        ) &&
        # only follow side
        git for-each-ref refs/heads/side >expect &&
        test_cmp expect actual
'

test_expect_success '--single-branch with detached' '
        (
                cd dir_detached && git fetch &&
                git for-each-ref refs/remotes/origin |
                sed -e "/HEAD$/d" \
                    -e "s|/remotes/origin/|/heads/|" >../actual
        )
        # nothing
        >expect &&
        test_cmp expect actual
'

test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to