Subject: Re: [PATCH 2/4] fsck: check rev-index checksums

From: Derrick Stolee <[email-masked]>
Date: Tue, 18 Apr 2023 10:57:15 -0400

On 4/18/2023 10:51 AM, Taylor Blau wrote:
> On Tue, Apr 18, 2023 at 10:27:57AM -0400, Derrick Stolee wrote:
>>>> +test_expect_success 'fsck catches invalid checksum' '
>>>> +	revfile=$(ls corrupt/.git/objects/pack/pack-*.rev) &&
>>>
>>> Would this test be tighter if we introduced a sub-shell and cd'd into
>>> "corrupt" here?
>>
>> corrupt_rev_and_verify does the subshell thing. Why should we do that
>> here in the test?
> 
> I was thinking that it might be more concise if you moved the subshell
> to the test and out of corrupt_rev_and_verify. In addition to making
> corrupt_rev_and_verify work in other instances where the repository
> isn't required to be in a directory named "corrupt", I think it
> simplifies the result.

I don't think there is a good reason to allow using a different repo
name. This is the only test that requires doing anything but calling
corrupt_rev_and_verify with different parameters, so I think this
makes the test script at the end of the series noisier.

Thanks,
-Stolee