Re: [PATCH v4 1/3] fetch: add test to make sure we stay backwards compatible

[Stefan Beller <sbeller@google.com>]

On Mon, Oct 16, 2017 at 6:57 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> The current implementation of submodules supports on-demand fetch if
> there is no .gitmodules entry for a submodule. Let's add a test to
> document this behavior.
>
> Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
> ---
>  t/t5526-fetch-submodules.sh | 42 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
> index 42251f7f3a..43a22f680f 100755
> --- a/t/t5526-fetch-submodules.sh
> +++ b/t/t5526-fetch-submodules.sh
> @@ -478,7 +478,47 @@ test_expect_success "don't fetch submodule when newly recorded commits are alrea
>                 git fetch >../actual.out 2>../actual.err
>         ) &&
>         ! test -s actual.out &&
> -       test_i18ncmp expect.err actual.err
> +       test_i18ncmp expect.err actual.err &&
> +       (
> +               cd submodule &&
> +               git checkout -q master
> +       )

For few instructions inside another repo, I tend to use the
-C option:

  git -C submodule checkout -q master

That saves a shell, which is noticeable cost on Windows I was told.
(also fewer lines to type).

Oh, I see, that is consistent with the rest of the file. Oh well.
(Otherwise I would have lobbied to even move it further up and
put it inside a test_when_finished "<cmd>"


> +'
> +
> +test_expect_success "'fetch.recurseSubmodules=on-demand' works also without .gitmodule entry" '
> +       (
> +               cd downstream &&
> +               git fetch --recurse-submodules
> +       ) &&

This is consistent with the rest of the file as well, so I shall
refrain from complaining. ;)

> +       add_upstream_commit &&
> +       head1=$(git rev-parse --short HEAD) &&
> +       git add submodule &&
> +       git rm .gitmodules &&
> +       git commit -m "new submodule without .gitmodules" &&
> +       printf "" >expect.out &&

This could be just

    : >expect.out

no need to invoke a function to print nothing.

> +       head2=$(git rev-parse --short HEAD) &&
> +       echo "From $pwd/." >expect.err.2 &&
> +       echo "   $head1..$head2  master     -> origin/master" >>expect.err.2 &&
> +       head -3 expect.err >>expect.err.2 &&
> +       (
> +               cd downstream &&
> +               rm .gitmodules &&
> +               git config fetch.recurseSubmodules on-demand &&
> +               # fake submodule configuration to avoid skipping submodule handling
> +               git config -f .gitmodules submodule.fake.path fake &&
> +               git config -f .gitmodules submodule.fake.url fakeurl &&
> +               git add .gitmodules &&
> +               git config --unset submodule.submodule.url &&
> +               git fetch >../actual.out 2>../actual.err &&
> +               # cleanup
> +               git config --unset fetch.recurseSubmodules &&
> +               git reset --hard
> +       ) &&
> +       test_i18ncmp expect.out actual.out &&
> +       test_i18ncmp expect.err.2 actual.err &&
> +       git checkout HEAD^ -- .gitmodules &&
> +       git add .gitmodules &&
> +       git commit -m "new submodule restored .gitmodules"
>  '

Thanks for writing this test.
With or without the nits addressed, this is

Reviewed-by: Stefan Beller <sbeller@google.com>