Re: [PATCH v3 0/6] Keep all info in command-list.txt in git binary

[Ramsay Jones <ramsay@ramsayjones.plus.com>]

On 22/04/18 16:22, Duy Nguyen wrote:
> On Sun, Apr 22, 2018 at 4:45 PM, Ramsay Jones
> <ramsay@ramsayjones.plus.com> wrote:
>>
>>
>> On 21/04/18 17:56, Duy Nguyen wrote:
>>> On Sat, Apr 21, 2018 at 06:54:08PM +0200, Nguyễn Thái Ngọc Duy wrote:
>>>> Changes:
>>>>
>>>> - remove the deprecated column in command-list.txt. My change break it
>>>>   anyway if anyone uses it.
>>>> - fix up failed tests that I marked in the RFC and kinda forgot about it.
>>>> - fix bashisms in generate-cmdlist.sh
>>>> - fix segfaul in "git help"
>>>
>>> Sorry I forgot the interdiff
>>>
>> [snip]
>>
>>> diff --git a/t/t0012-help.sh b/t/t0012-help.sh
>>> index fd2a7f27dc..53208ab20e 100755
>>> --- a/t/t0012-help.sh
>>> +++ b/t/t0012-help.sh
>>> @@ -25,6 +25,15 @@ test_expect_success "setup" '
>>>       EOF
>>>  '
>>>
>>> +# make sure to exercise these code paths, the output is a bit tricky
>>> +# to verify
>>> +test_expect_success 'basic help commands' '
>>> +     git help >/dev/null &&
>>> +     git help -a >/dev/null &&
>>> +     git help -g >/dev/null &&
>>> +     git help -av >/dev/null
>>> +'
>>> +
>> I think you need to try a little harder than this! ;-)
> 
> Yeah. I did think about grepping the output but decided not to because
> of gettext poison stuff and column output in "git help". If we do want
> to test this, how about I extend --list-cmds= option to take a few
> more parameters? --list-cmds=common would output all common commands,
> --list-cmds=<category> does the same for other command category. This
> way we can verify without worrying about text formatting, paging or
> translation.

Hmm, my immediate reaction would be to prefer my simple tests.
Yes, they are not exactly rigorous and they would be affected 
by changing the help formatting, but they are effective. ;-)

[I don't think the formatting would change that often, or at
all - whoever submits that patch would get to update the tests!]

What did you think about adding the BUG() checks?

ATB,
Ramsay Jones