Denton Liu <liu.denton@xxxxxxxxx> writes: > On Tue, Jan 12, 2021 at 09:17:50PM +0100, Ævar Arnfjörð Bjarmason wrote: >> @@ -480,7 +545,7 @@ test_expect_success 'log.mailmap=false disables mailmap' ' >> Author: nick1 <bugs@xxxxxxxxxx> >> 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.
