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

On Mon, Nov 2, 2015 at 1:25 AM, Lukas Fleischer [off-list ref] wrote:

On Sun, 01 Nov 2015 at 22:13:51, Eric Sunshine wrote:

quoted

On Sun, Nov 1, 2015 at 2:34 PM, Lukas Fleischer [off-list ref] wrote:

quoted

+       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 &&
'