Re: [PATCH 4/4] t5509: add basic tests for hideRefs

[Eric Sunshine <sunshine@sunshineco.com>]

On Mon, Nov 2, 2015 at 1:25 AM, Lukas Fleischer <lfleischer@lfos.de> wrote:
> On Sun, 01 Nov 2015 at 22:13:51, Eric Sunshine wrote:
>> On Sun, Nov 1, 2015 at 2:34 PM, Lukas Fleischer <lfleischer@lfos.de> wrote:
>> > +       cd pushee &&
>> > +       test_config transfer.hideRefs refs/tags &&
>> > +       GIT_NAMESPACE=namespace git ls-remote "ext::git %s ." >actual &&
>> > +       printf "$commit1    refs/heads/master\n" >expected &&
>> > +       test_cmp expected actual &&
>> > +       cd ..
>>
>> If any of the commands above "cd .." fail, then "cd .." will never be
>> invoked, thus subsequent tests will fail since they won't be executed
>> in the expected directory. The typical way to handle this is to place
>> the "cd foo" and remaining test body in a subshell, and drop "cd .."
>> altogether. When the subshell exits (via success or failure), the
>> working directory will be restored automatically.
>>
>>     test_expect_success '...' '
>>         (
>>             cd pushee &&
>>             test_config ... &&
>>             ...
>>         )
>>     '
>> [...]
>
> I chose the `cd ..` approach because test_config does not work from a
> subshell. However, searching the Git log for "test_config", I found
> 1a9a23e (t7610: don't use test_config in a subshell, 2015-09-05) and
> da568b6 (t7800: don't use test_config in a subshell, 2015-09-05) which
> suggest to use the -C switch. The test cases now look like this:
>
>     test_expect_success '[...]' '
>         test_config -C pushee transfer.hideRefs [...] &&
>         (
>             cd pushee &&
>             [...]
>         )
>     '

Yes, that can work, although for these simple cases, it might be more
straightforward to use the git -c option to set the config variable
just for the duration of the one git command. For instance:

test_expect_success 'hide namespaced refs with transfer.hideRefs' '
    (
        cd pushee &&
        GIT_NAMESPACE=namespace \
            git -c transfer.hideRefs=refs/tags \
            ls-remote "ext::git %s ." >actual &&
        printf "$commit1\trefs/heads/master\n" >expected &&
        test_cmp expected actual &&
    )
'

In fact, these test are so simple, that you don't really need the 'cd'
at all. You could just use -C (along with -c):

test_expect_success 'hide namespaced refs with transfer.hideRefs' '
    GIT_NAMESPACE=namespace \
        git -C pushee -c transfer.hideRefs=refs/tags \
        ls-remote "ext::git %s ." >actual &&
    printf "$commit1\trefs/heads/master\n" >expected &&
    test_cmp expected actual &&
'