Subject: Re: [PATCH] t4203: fix checks for email address remapping

From: Stefan Beller <[email-masked]>
Date: Sat, 13 Jul 2013 08:29:32 +0200

On 07/13/2013 02:35 AM, Eric Sunshine wrote:
> Two tests in t4203-mailmap.sh set up the mapping <[email-masked]> =>
> <[email-masked]> 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 <[email-masked]>
> ---
> 
> 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 <[email-masked]> => <[email-masked]>
> 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 <[email-masked]> (1):
>        second
>  
> -Repo Guy (1):
> +Repo Guy <[email-masked]> (1):
>        initial
>  
>  EOF
> @@ -114,15 +114,15 @@ test_expect_success 'name entry after email entry' '
>  	mkdir -p internal_mailmap &&
>  	echo "<[email-masked]> <[email-masked]>" >internal_mailmap/.mailmap &&
>  	echo "Internal Guy <[email-masked]>" >>internal_mailmap/.mailmap &&
> -	git shortlog HEAD >actual &&
> +	git shortlog -e HEAD >actual &&
>  	test_cmp expect actual
>  '
>  
>  cat >expect <<\EOF
> -Internal Guy (1):
> +Internal Guy <[email-masked]> (1):
>        second
>  
> -Repo Guy (1):
> +Repo Guy <[email-masked]> (1):
>        initial
>  
>  EOF
> @@ -131,7 +131,7 @@ test_expect_success 'name entry after email entry, case-insensitive' '
>  	mkdir -p internal_mailmap &&
>  	echo "<[email-masked]> <[email-masked]>" >internal_mailmap/.mailmap &&
>  	echo "Internal Guy <[email-masked]>" >>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 <[email-masked]>
    Internal Guy <[email-masked]>

Having just one entry in the mailmap
    Internal Guy <[email-masked]> <[email-masked]>

should still work with the "shortlog -e"

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