Re: [PATCH v2 11/12] t5524: test --log=1 limits shortlog length

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Hannes,

On 2015-05-07 21:12, Johannes Sixt wrote:
> Am 07.05.2015 um 19:06 schrieb Paul Tan:
>
>> On Fri, May 8, 2015 at 12:28 AM, Johannes Schindelin
>> <johannes.schindelin@xxxxxx> wrote:
>>
>>> 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

That link leads to a patch that changes `! grep` to a `test_must_fail grep` and is not contested, at least not in the thread visible on GMane. Would you have a link with a more convincing argument for me?

Thank you,
Johannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]