Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes: >> test_expect_success 'git-fsck incorrect offset' ' >> corrupt_midx_and_verify $MIDX_BYTE_OFFSET "\377" $objdir \ >> "incorrect object offset" \ >> - "git -c core.multipackindex=true fsck" >> + "git -c core.multipackindex=true fsck" && >> + test_must_fail git fsck && >> + git -c core.multipackindex=false fsck >> ' > > I guess the newly-added `test_must_fail git fsck` line is checking the > fallback case then `core.multipackindex` is not set. We could make this > check a bit more robust by _ensuring_ that it is unset rather than > relying upon whatever state the configuration is in by the time this > test is reached. Perhaps something like this: > > ... > "git -c core.multipackindex=true fsck" && > test_unconfig core.multipackindex && > test_must_fail git fsck && > git -c core.multipackindex=false fsck I think this extra robustness is worth it. I sometimes find that the tests are a bit too interdependent to read on their own, so this is a good step forward. > The indentation is a bit unusual. It aligns nicely and is, in some > sense, easy to read, but the two new lines are over-indented considering > that they are siblings of the corrupt_midx_and_verify() call. I agree. This was a typo from me, not a conscious choice.
