On Tue, Apr 18, 2023 at 10:57:15AM -0400, Derrick Stolee wrote: > 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. No worries. I was thinking that it might be convenient in the future if we wanted to corrupt a .rev file in a different repository, but that's absolutely a bridge we can cross if/when we get to it. Thanks, Taylor
