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. > > 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.
