Re: [PATCH 06/22] mailmap tests: modernize syntax & test idioms

Denton Liu [off-list ref] writes:

On Tue, Jan 12, 2021 at 09:17:50PM +0100, Ævar Arnfjörð Bjarmason wrote:

quoted

@@ -480,7 +545,7 @@ test_expect_success 'log.mailmap=false disables mailmap' '
 	Author: nick1 <bugs@company.xx>
 	Author: $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>
 	EOF
-	git -c log.mailmap=False log | grep Author >actual &&
+	git -c log.mailmap=false log | grep Author >actual &&

While you're doing test cleanup, here's another suggestion: we should
break all these pipes where git is in the upstream of a pipe. The return
code of a pipe comes from the last thing run which means if git outputs
correctly but then somehow fails after, we won't detect the failure.

In general, I've stopped my crusade against these because it seems like
it's more noise than it's worth in most cases but in this case, since
we're exercising mailmap codepaths that aren't tested in other test
cases, this pipe could plausibly hide a failure that isn't seen
anywhere else.

Yeah, I agree with your assessment that it does make sense to make
sure this "log" does not crash silently.

I find it unlikely for "log" to crash while giving an expected
Author line to its output stream, though.

Can you make your suggestion into a follow-up patch to be applied on
top of the series?

Thanks.