Re: [PATCH] t3070: make chain lint tester happy

From: Jeff King <peff@peff.net>
Date: Mar 25, 2023, 08:51 UTC
Message-ID: <20230325085127.GA26684@coredump.intra.peff.net>
In-Reply-To: <20230325084107.GB3738217@coredump.intra.peff.net>

On Sat, Mar 25, 2023 at 04:41:08AM -0400, Jeff King wrote:

Show 6 quoted lines

> Right, the chainlint.pl one is much more thorough. I just wondered if
> there were any cases we were worried about it missing, that the internal
> one catches. We found one in this thread, but as discussed, it is not a
> problem (presumably chainlint.pl catches a "real" case where an
> earlier line is hidden by the "&", but I wouldn't mind seeing it
> complain here as a matter of style/future-proofing).

Hmm, actually chainlint.pl does not seem to catch this:

-- >8 --
test_expect_success 'ok, first line cannot break &&-chain' '
	true &
	pid=$!
'

test_expect_success 'not ok, failure is lost' '
	false &&
	true &
	pid=$!
'
-- >8 --

It's a little funny, because we actually background the whole "false && true" chain. So if you did "wait $pid" at the end, you would see the failure. But the test in this thread doesn't actually do that (it depends on kill after 2 seconds not finding the pid). Plus in general this seems like an accident that we should be flagging.

-Peff