From: René Scharfe <l.s.r@web.de>
Date: Fri, 20 Nov 2015 20:49:58 GMT
Subject: Re: [PATCH 1/2] t1450: add tests for NUL in headers of commits and tags
Message-ID: <564F8776.1050706@web.de>
In-Reply-To: <20151120111454.GB11198@sigill.intra.peff.net>

Am 20.11.2015 um 12:14 schrieb Jeff King:
> On Thu, Nov 19, 2015 at 09:54:54PM +0100, René Scharfe wrote:
>
>>>> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
>>>> @@ -176,6 +176,18 @@ test_expect_success 'integer overflow in timestamps is reported' '
>>>> +test_expect_success 'commit with NUL in header' '
>>>> +       git cat-file commit HEAD >basis &&
>>>> +       sed "s/author ./author Q/" <basis | q_to_nul >commit-NUL-header &&
>>>> +       new=$(git hash-object -t commit -w --stdin <commit-NUL-header) &&
>>>> +       test_when_finished "remove_object $new" &&
>>>> +       git update-ref refs/heads/bogus "$new" &&
>>>> +       test_when_finished "git update-ref -d refs/heads/bogus" &&
>>>> +       test_must_fail git fsck 2>out &&
>>>> +       cat out &&
>>>
>>> What is the purpose of this 'cat'?
>>
>> It shows the full error message when the test is run with --debug, which is
>> convenient when the following grep doesn't match.  The same is done in most
>> tests in that file.
>
> I'm slightly negative on such a construct, just because it wastes a
> process in the case where we are not in --verbose mode. I don't mind it
> in this patch in the spirit of consistency within t1450, but I think we
> should probably avoid spreading it.

This practice is not used that much (yet).  We can contain it by
providing an alternative.  I'll send patches for adding a helper
function similar to test_must_be_empty for that.

René