On Sat, Jul 13, 2013 at 2:29 AM, Stefan Beller
<[email protected]> wrote:
> On 07/13/2013 02:35 AM, Eric Sunshine wrote:
>> Two tests in t4203-mailmap.sh set up the mapping <[email protected]> =>
>> <[email protected]> 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 protected]>
>> ---
>>
>> 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 protected]> => <[email protected]>
>> 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.
>>
>>  cat >expect <<\EOF
>> -Internal Guy (1):
>> +Internal Guy <[email protected]> (1):
>>        second
>>
>> -Repo Guy (1):
>> +Repo Guy <[email protected]> (1):
>>        initial
>>
>>  EOF
>> @@ -131,7 +131,7 @@ test_expect_success 'name entry after email entry, 
>> case-insensitive' '
>>       mkdir -p internal_mailmap &&
>>       echo "<[email protected]> <[email protected]>" >internal_mailmap/.mailmap 
>> &&
>>       echo "Internal Guy <[email protected]>" >>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)

With the re-roll of Junio's series which I just sent out [1], this
case-preserving problem is tested (and fixed), and email address
remapping is tested in general, so I think this submission should be
retired.

(It might still be worthwhile, as mentioned in the patch commentary,
to drop the confusing lines altogether from those two tests, but
that's an issue for another day.)

[1]: http://thread.gmane.org/gmane.comp.version-control.git/230425/

> So I think we need another yet test case there:
> commited:
>     Internal Guy <[email protected]>
>     Internal Guy <[email protected]>
>
> Having just one entry in the mailmap
>     Internal Guy <[email protected]> <[email protected]>
>
> should still work with the "shortlog -e"
>
>> -     git shortlog HEAD >actual &&
>> +     git shortlog -e HEAD >actual &&
>>       test_cmp expect actual
>>  '
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to