On 4/17/2023 6:24 PM, Taylor Blau wrote: > On Mon, Apr 17, 2023 at 04:21:39PM +0000, Derrick Stolee via GitGitGadget wrote: >> +test_expect_success 'set up rev-index corruption tests' ' > > s/set up/setup for easy `--run`-ing (e.g., ./t5325-*.sh --run=setup,fsck). Makes sense. >> +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? >> + orig_size=$(wc -c <$revfile) && > > I'm nitpicking, but we may want to use `test_file_size` here instead of > `wc -c`. The latter outnumbers the former in terms of number of uses, > but I think we consider `test_file_size` to be canonical these days. Makes sense. Thanks, -Stolee
