Re: [PATCH] test: accept death by SIGPIPE as a valid failure mode

> On 05 Nov 2015, at 08:47, Jeff King <peff@peff.net> wrote:
> 
> On Fri, Oct 30, 2015 at 02:22:14PM -0700, Junio C Hamano wrote:
> 
>> On a local host, the object/history transport code often talks over
>> pipe with the other side.  The other side may notice some (expected)
>> failure, send the error message either to our process or to the
>> standard error and hung up.  In such codepaths, if timing were not
>> unfortunate, our side would receive the report of (expected) failure
>> from the other side over the pipe and die().  Otherwise, our side
>> may still be trying to talk to it and would die with a SIGPIPE.
>> 
>> This was observed as an intermittent breakage in t5516 by a few
>> people.
>> 
>> In the real-life scenario, either mode of death exits with a
>> non-zero status, and the user would learn that the command failed.
>> The test_must_fail helper should also know that dying with SIGPIPE
>> is one of the valid failure modes when we are expecting the tested
>> operation to notice problem and fail.
> 
> Sorry for the slow review; before commenting I wanted to dig into
> whether this SIGPIPE ambiguity was avoidable in the first place.
> 
> I think the answer is "probably not". We do call write_or_die() pretty
> consistently in the network-aware programs. So we could ignore SIGPIPE,
> and then we would catch EPIPE (of course, we convert that into SIGPIPE
> in many places, but we do not have to do so). But since the SIGPIPE
> behavior is global, that carries the risk of us failing to check a write
> against some other descriptor. It's probably not worth it.
> 
> Teaching the tests to handle both cases seems like a reasonable
> workaround. Changing test_must_fail covers a lot of cases; I wondered if
> there are other tests that would not want to silently cover up a SIGPIPE
> death. But I could not really think of a plausible reason.
> 
> So I think your patch is the best thing to do.
> 
> -Peff