"Leon Michalak via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: Leon Michalak <leonmichalak6@xxxxxxxxx>
>
> Use the modern "test_config" test utility instead of manual"git config"
> as the former provides clean up on test completion.
Here "completion" is not "all the tests in the script are done", but
"each of the test_expect_{success,failure} piece that uses test_config".
Drop "modern". It was invented 14 years ago (the same can be said
for test_grep which was called test_i18ngrep and had an extra
purpose, which was invented in the same year).
This conversion, unlike "test_grep" needs to be done a bit
carefully. The fact the configuration is removed after the test
piece "test_config" was used means any tests after the test that was
originally using "git config" needs to be inspected to make sure
it was *not* relying on the value that was left by the previous
test piece.
For example...
> test_expect_success 'diff.context honored by "log"' '
> git log -1 -p >output &&
> test_grep ! firstline output &&
> - git config diff.context 8 &&
> + test_config diff.context 8 &&
> git log -1 -p >output &&
> test_grep "^ firstline" output
> '
... the test piece after this one may have assumed (wrongly! The
assumption does not hold if this test failed before reaching "git
config") that diff.context is still set to 8 but that is no longer
the case. But that one is OK because ...
> test_expect_success 'The -U option overrides diff.context' '
> - git config diff.context 8 &&
> + test_config diff.context 8 &&
> git log -U4 -1 >output &&
> test_grep ! "^ firstline" output
> '
... it was setting it for itself. The same can be said with other
tests (not quoted in this reply). They all set things up for
themselves, which is a good hygiene.
That means the last one in the patch needs to be inspected
carefully. We do not know from the post-context of the patch what
the next test used to expect.
> test_expect_success '-U0 is valid, so is diff.context=0' '
> - git config diff.context 0 &&
> + test_config diff.context 0 &&
> git diff >output &&
> test_grep "^-ADDED" output &&
> test_grep "^+MODIFIED" output
The one that comes after this one is about giving an explicit
command line option -U<n>. It should not be affected by a
diff.context conffiguration variable that is lower-precedence
so we should be OK.
Thanks.
