Re: [PATCH v2 2/3] fsck: verify multi-pack-index when implictly enabled

Eric Sunshine [off-list ref] writes:

quoted

  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.