Web lists-archives.com

Re: [PATCH v2 2/2] tests: add test for separate author and committer idents




On Sat, Jan 26, 2019 at 3:53 AM Ævar Arnfjörð Bjarmason
<avarab@xxxxxxxxx> wrote:
> Which, looking at this again, you'd only want if a previous test in the
> file was leaking its state. That's not the case, so this isn't needed
> and you can just apply this on top:
>
>      test_expect_success \
>             'author and committer config settings override user config settings' '
>     -       sane_unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL &&
>     -       sane_unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL &&
>             git config user.name user &&
>             git config user.email user@xxxxxxxxxxx &&
>             git config author.name author &&

Aside from future-proofing against a test being inserted before this
one which does set those environment variables, these invocations of
sane_unset() serve the additional purpose of documenting the interplay
of configuration and environment, and further indicate to readers that
the test author took this into consideration (rather than merely
slapping together the test without thought). As a reviewer and reader
of the test, I appreciate the additional context the sane_unset()
calls provide, thus think it makes sense to retain them.