From: Johannes Sixt <j6t@kdbg.org>
Date: Thu, 07 May 2015 19:12:58 GMT
Subject: Re: [PATCH v2 11/12] t5524: test --log=1 limits shortlog length
Message-ID: <554BB93A.3090000@kdbg.org>
In-Reply-To: <CACRoPnRfqQMRJ5N=oL84SMw=FC=Eg-Co-De_9E9cpWCaaSZAxw@mail.gmail.com>

Am 07.05.2015 um 19:06 schrieb Paul Tan:
> Hi Dscho,
>
> On Fri, May 8, 2015 at 12:28 AM, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
>> Hi Paul,
>>
>> On 2015-05-07 10:44, Paul Tan wrote:
>>> @@ -32,4 +35,18 @@ test_expect_success pull '
>>>   )
>>>   '
>>>
>>> +test_expect_failure '--log=1 limits shortlog length' '
>>> +(
>>> +     cd cloned &&
>>> +     git reset --hard HEAD^ &&
>>> +     test `cat afile` = original &&
>>> +     test `cat bfile` = added &&
>>> +     git pull --log &&
>>> +     git log -3 &&
>>> +     git cat-file commit HEAD >result &&
>>> +     grep Dollar result &&
>>> +     ! grep "second commit" result
>>> +)
>>
>> I think it might be better to use `test_must_fail` here, just for
>> consistency (the `!` operator would also pass if `grep` itself could not
>> be executed correctly, quite academic, I know, given that `grep` is
>> exercised plenty of times by the test suite, but still...)
>>
>> What do you think?
>
> Yep, it's definitely better. Sometimes I forget about the existence of
> some test utility functions :-/.

Nope, it's not better. test_must_fail is explicitly only for git 
invocations. We do not expect 'grep' to segfault or something.

Cf. eg. 
http://thread.gmane.org/gmane.comp.version-control.git/258725/focus=258752

-- Hannes