On 8/21/2024 11:27 AM, Eric Sunshine wrote: > On Wed, Aug 21, 2024 at 1:50 PM Josh Steadmon <steadmon@xxxxxxxxxx> 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. > Yea, I think I've been convinced. I'll remove these tests. >>> 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. > I tried to ensure we have test cases covering both --stdin and a combination. I'll double check this in a v3 and ensure test cases covering the behavior. Thanks for the feedback!
