Re: [PATCH] fsck: check skiplist for object in fsck_blob()

From: Ramsay Jones <ramsay@ramsayjones.plus.com>
Date: Jul 13, 2018, 20:08 UTC
Message-ID: <6e3ed7b0-105f-f527-65af-6fc217bc826d@ramsayjones.plus.com>
In-Reply-To: <20180713194609.GA16745@sigill.intra.peff.net>

On 13/07/18 20:46, Jeff King wrote:

Show 17 quoted lines

> On Fri, Jul 13, 2018 at 03:41:19PM -0400, Jeff King wrote:
> 
>>>   not ok 18 - push rejects corrupt .gitmodules (policy)
>>>   #	
>>>   #		rm -rf dst.git &&
>>>   #		git init --bare dst.git &&
>>>   #		git -C dst.git config transfer.fsckObjects true &&
>>>   #		git -C dst.git config fsck.gitmodulesParse error &&
>>>   #		test_must_fail git -C corrupt push ../dst.git HEAD 2>output &&
>>>   #		grep gitmodulesParse output &&
>>>   #		test_i18ngrep ! "bad config" output
>>
>> There are separate config slots for local fsck versus receiving objects.
>> So I think you need to be setting receive.fsck.gitmodulesParse.
> 
> I confirmed that s/fsck/receive.fsck/ in your patch makes the tests
> pass.

Doh! Thanks for catching my stupid mistake! I was rushing a bit just before going out (yes, I'm going to be late now!).

> I didn't bother adding extra push tests in the patch I just sent, since
> upgrading of config error classes is already covered elsewhere in t5504.

yeah, I like to 'test' by adding tests if I can (makes repeating the steps less effort ...). So, I was just 'showing my working', as it were.

> That said, I'm not opposed to adding more tests on top even if they are
> slightly redundant (well, not redundant if you're into black-box
> testing, but our current tests are usually written with an assumption of
> where the module boundaries are, and what is likely to be a problem).

I don't mind either way. I will let you and Junio decide.

Thanks!

ATB, Ramsay Jones