Re: [PATCH 1/2] t1450: add tests for NUL in headers of commits and tags
- From
René Scharfe <l.s.r@web.de>
- Date
- Nov 20, 2015, 20:49 UTC
- 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:
Show 24 quoted lines
> 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é