Re: [PATCH v2 1/3] fsck: verify commit graph when implicitly enabled

quoted

Because I took the time to scan backward through this test script, I 
understand that `core.commitGraph` is already `true` by the time this 
test is reached, thus the new test title is accurate (for now). However, 
it would be a bit easier to reason about this and make it more robust by 
having the test itself guarantee that it is set to `true` by invoking 
`git config core.commitGraph true`.

quoted

test_config() unsets the config variable when the test completes, so I'm 
wondering if its use is in fact desirable here. I ask because, from a 
quick scan through the file, it appears that many tests in this script 
assume that `core.commitGraph` is `true`, so it's not clear that 
unsetting it at the end of this test is a good idea in general.

This is a good point. I made the original change in response to Taylor's
comment, but I think we both didn't consider that this script assumes
`core.commitGraph` is `true`. The rest of the tests pass, but only
because the default value is true and I'd rather not have tests rely on
a happy accident.

I said I would incorporate these suggestions, but I didn't propose what
changes I would actually make.

Given that each test depends on a global config value in the setup
phase, it might be simplest to read if we try to avoid anything that
touches that value. The easiest way to achieve this is to use git -c
core.commitGraph {true,false} for the {true,false} cases. Since there is
no -c equivalent for the unset case, I'll continue to use
test_unconfig() + test_when_finished() to temporarily unset the value.