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

- From: Glen Choo \<hidden\>
- Date: 2021-09-29 22:57:02 
- List: git
- Page: https://ratatoskr.run/git/2021/09/7904044
- Thread: https://ratatoskr.run/git/2021/09/7902332/t.md

```text
Eric Sunshine [ref] 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.
```