Re: [PATCH v2] t6026-merge-attr: don't fail if sleep exits early

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 10, 2016, 22:55 UTC
Message-ID: <xmqq1syj54j9.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20161110223522.4b35ojaz5nhk4sll@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I do think the test would be a lot more obvious if it confirmed at the
> end of the test that the process was still running, as opposed to
> relying on test_when_finished to check it.

I agree that "check that the process is still running" is a wrong thing to do in the first place. What would it mean if the process is no longer running? It is a timing-dependent bug in the test; after all we failed to produce the condition that could trigger a bug that we are guarding against. And "let's do '|| :'" is sweeping the bug (not in the code we are testing, but in the test that will fail to notice a bug we are preparing against) under the rug. So I agree with Dscho that we should do that first.

If we ensure that the process is still running, then such a check is a good belt-and-suspenders way to catch a breakage in the mechanism we choose to ensure it. So probably we can require that the kill in the "when finished" part to actually send a signal to a process that is still running.

Is there an equivalent to pause(2) available to shell scripts? I really hate a single "sleep 3600" or anything with a magic number.