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

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Sat, 26 Jan 2019 09:53:18 +0100

On Sat, Jan 26 2019, William Hubbs wrote:

> On Sat, Jan 26, 2019 at 12:05:08AM +0100, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Fri, Jan 25 2019, William Hubbs wrote:
>
> ...
>
>> > +	sane_unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL &&
>> > +	sane_unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL &&
>>
>> Fine, but FYI sets these variables for the rest of the test.
>
> I'm not quite sure what you mean by this. I want the environment
> variables to be *unset*. I don't want them to override anything in the
> config file for this test.
>
> Are you saying they will not be set for the test unless I set them,
> so I don't need the SANE_UNSET calls?

Sorry for not being clear. I just meant that unlike "test_config" the
"sane_unset" function won't reset the state at the end of the
"test_expect_success".

Right now it doesn't matter in practice since this is the last test
before "test_done", but as tests are added we tend to leak state between
them, which is why we use these "unset at the end" helper functions.

But unlike with config that doesn't matter in this case, since we want
these unset anyway.

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:

    diff --git a/t/t7517-per-repo-email.sh b/t/t7517-per-repo-email.sh
    index 06c7c0fb78..e5845b0b86 100755
    --- a/t/t7517-per-repo-email.sh
    +++ b/t/t7517-per-repo-email.sh
    @@ -87,8 +87,6 @@ test_expect_success REBASE_P \

     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 [email-masked] &&
            git config author.name author &&

You don't need to be paranoid and unset these, we already unset GIT_*
variables that aren't on a whitelist in test-lib.sh, see 'A call to
"unset"' there.