From: Elijah Newren <newren@gmail.com>
Date: Mon, 06 Aug 2018 16:09:56 GMT
Subject: Re: [PATCH 2/3] t7406: avoid having git commands upstream of a pipe
Message-ID: <CABPp-BEJn3fj8y_K27MOX48t9W9DRS-OOGuJGhq+zuHsMz3SrA@mail.gmail.com>
In-Reply-To: <20180806154814.24585-1-szeder.dev@gmail.com>

On Mon, Aug 6, 2018 at 8:48 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> > @@ -922,7 +928,7 @@ test_expect_success 'submodule update clone shallow submodule' '
> >               sed -e "s#url = ../#url = file://$pwd/#" <.gitmodules >.gitmodules.tmp &&
> >               mv -f .gitmodules.tmp .gitmodules &&
> >               git submodule update --init --depth=$commit_count &&
> > -             test 1 = $(git -C submodule log --oneline | wc -l)
> > +             test 1 = $(git -C submodule rev-list --count HEAD)
> >       )
> >  '
> >
> > @@ -938,7 +944,7 @@ test_expect_success 'submodule update clone shallow submodule outside of depth'
> >               test_i18ngrep "Direct fetching of that commit failed." actual &&
> >               git -C ../submodule config uploadpack.allowReachableSHA1InWant true &&
> >               git submodule update --init --depth=1 >actual &&
> > -             test 1 = $(git -C submodule log --oneline | wc -l)
> > +             test 1 = $(git -C submodule rev-list --count HEAD)
> >       )
> >  '
>
> These two hunks don't have the desired effect, because command
> substitutions used like this will hide the exit code anyway.  I'd
> suggest
>
>   git -C submodule log --oneline >out &&
>   test_line_count = 1 out
>
> instead, with the additional benefit of a nice error message on
> failure.

Ah, good point...and good suggestion.  I'll wait for further feedback
then resend with this change.