From: Stefan Beller <stefanbeller@googlemail.com>
Date: Sat, 13 Jul 2013 06:29:32 GMT
Subject: Re: [PATCH] t4203: fix checks for email address remapping
Message-ID: <51E0F3CC.3040304@googlemail.com>
In-Reply-To: <1373675709-40486-1-git-send-email-sunshine@sunshineco.com>

On 07/13/2013 02:35 AM, Eric Sunshine wrote:
> Two tests in t4203-mailmap.sh set up the mapping <bugs@company.xx> =>
> <bugs@company.xy> in an apparent attempt to check that email address
> remapping works as expected (in addition to name remapping which is also
> tested).  To test the remapping, git-shortlog is invoked but the
> invocation lacks the -e option instructing it to show email addresses,
> hence the tests do not actually prove that address remapping succeeded.
> Fix this by instructing git-shortlog to output email addresses as well.
> 
> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
> 
> The very last git-shortlog "complex" test in the script does use -e and
> checks that email address remapping actually works, so it's not clear
> that this patch is needed. The <bugs@company.xx> => <bugs@company.xy>
> remapping done by the two tests touched by this patch, however, is
> misleading to the reader since it seems to imply that these two tests
> want to check address remapping as well. Perhaps a better change would
> be to remove the address remapping from these two tests.
> 
> 
>  t/t4203-mailmap.sh | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
> index 842b754..3cf64de 100755
> --- a/t/t4203-mailmap.sh
> +++ b/t/t4203-mailmap.sh
> @@ -102,10 +102,10 @@ test_expect_success 'mailmap.file non-existent' '
>  '
>  
>  cat >expect <<\EOF
> -Internal Guy (1):
> +Internal Guy <bugs@company.xy> (1):
>        second
>  
> -Repo Guy (1):
> +Repo Guy <author@example.com> (1):
>        initial
>  
>  EOF
> @@ -114,15 +114,15 @@ test_expect_success 'name entry after email entry' '
>  	mkdir -p internal_mailmap &&
>  	echo "<bugs@company.xy> <bugs@company.xx>" >internal_mailmap/.mailmap &&
>  	echo "Internal Guy <bugs@company.xx>" >>internal_mailmap/.mailmap &&
> -	git shortlog HEAD >actual &&
> +	git shortlog -e HEAD >actual &&
>  	test_cmp expect actual
>  '
>  
>  cat >expect <<\EOF
> -Internal Guy (1):
> +Internal Guy <bugs@company.xy> (1):
>        second
>  
> -Repo Guy (1):
> +Repo Guy <author@example.com> (1):
>        initial
>  
>  EOF
> @@ -131,7 +131,7 @@ test_expect_success 'name entry after email entry, case-insensitive' '
>  	mkdir -p internal_mailmap &&
>  	echo "<bugs@company.xy> <bugs@company.xx>" >internal_mailmap/.mailmap &&
>  	echo "Internal Guy <BUGS@Company.xx>" >>internal_mailmap/.mailmap &&+

So here it is capitalized email address (BUGS@), but at the expect file
it's still lower cased. I think this is a bug.
Junio was trying to fix it in 543f99173c2d2f648d8f846e24875150f7de03d3
(origin/jc/mailmap-case-insensitivity)
So I think we need another yet test case there:
commited:
    Internal Guy <BUGS@Company.xx>
    Internal Guy <bugs@company.xy>

Having just one entry in the mailmap
    Internal Guy <BUGS@Company.xx> <bugs@Company.xy>

should still work with the "shortlog -e"

> -	git shortlog HEAD >actual &&
> +	git shortlog -e HEAD >actual &&
>  	test_cmp expect actual
>  '
>  
>