Re: [PATCH v2 1/3] check-mailmap: accept "user@host" contacts

> On Wed, Aug 21, 2024 at 1:50 PM Josh Steadmon <steadmon@google.com> wrote:
>> On 2024.08.19 17:07, Jacob Keller wrote:
>>> diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
>>> @@ -73,11 +73,40 @@ test_expect_success 'check-mailmap --stdin arguments: mapping' '
>>>  test_expect_success 'check-mailmap bogus contact' '
>>> -     test_must_fail git check-mailmap bogus
>>> +     cat >expect <<-EOF &&
>>> +     <bogus>
>>> +     EOF
>>> +     git check-mailmap bogus >actual &&
>>> +     test_cmp expect actual
>>>  '
>>
>> I think I'd just remove this test case altogether, IIUC it' doesn't
>> provide any additional value vs. the "check-mailmap simple address: no
>> mapping" test below.
> 
> I had the same thought upon reading this.
> 

>>>  test_expect_success 'check-mailmap bogus contact --stdin' '
>>> -     test_must_fail git check-mailmap --stdin bogus </dev/null
>>> +     cat >expect <<-EOF &&
>>> +     <bogus>
>>> +     EOF
>>> +     cat >stdin <<-EOF &&
>>> +     bogus
>>> +     EOF
>>> +     git check-mailmap --stdin <stdin >actual &&
>>> +     test_cmp expect actual
>>> +'
>>
>> Similarly, I might change this to use a real address instead of "bogus",
>> as we're no longer checking for invalid input.>
> Ditto for this change, but even more so because this is a fairly
> significant semantic change. In particular, the documented and
> intended behavior of the command when --stdin is specified is that it
> will consume email addresses from *both* the command-line and from
> standard input, and I think the point of the original test was to
> verify that it still correctly recognized a bogus email address
> specified as an argument even when --stdin is requested. Given that
> understanding (assuming it's correct), then the original test was
> already perhaps somewhat iffy anyhow, but after this change, it is
> even less meaningful.
>