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
