Re: [PATCH 2/3] t9902: test multiple removals via completion.commands

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Fri, Mar 01, 2019 at 01:22:52PM -0500, Eric Sunshine wrote:
> On Fri, Mar 1, 2019 at 12:35 PM Todd Zullinger <tmz@pobox.com> wrote:
> > 6532f3740b ("completion: allow to customize the completable command
> > list", 2018-05-20) added the completion.commands config variable.
> > Multiple commands may be added or removed, separated by a space.
> >
> > Demonstrate the failure of multiple removals.
> >
> > Signed-off-by: Todd Zullinger <tmz@pobox.com>
> > ---
> > diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> > @@ -1483,6 +1483,14 @@ test_expect_success 'git --help completion' '
> > +test_expect_failure 'completion.commands removes multiple commands' '
> > +       echo cherry-pick >expected &&
> > +       test_config_global completion.commands "-cherry -mergetool" &&
> > +       git --list-cmds=list-mainporcelain,list-complete,config |
> > +               grep ^cherry >actual &&
> > +       test_cmp expected actual
> > +'
> 
> We normally avoid placing a Git command upstream of a pipe. Instead,
> the Git command output would be redirected to a file and then the file
> grep'd.

Indeed.

> However, in this case, you might consider simplifying the test
> like this:
> 
>     test_expect_failure 'completion.commands removes multiple commands' '
>         test_config_global completion.commands "-cherry -mergetool" &&
>         git --list-cmds=list-mainporcelain,list-complete,config >actual &&
>         grep cherry-pick actual

This wouldn't check what we want.  The point is that the command
'cherry' is not listed, so it should be '! grep cherry$ actual'.

Furthermore, I think it would be prudent to check that 'mergetool' is
not listed, either.