Jeff King <peff@xxxxxxxx> writes: > Ugh, I think this test is racy. I saw a failure via SIGPIPE on OS X in > CI, and running it locally with --stress confirmed. I think the problem > is in our method to trigger pack-objects to fail for the cruft pack. We > pass a bogus command line argument, so pack-objects exits more or less > immediately. But the parent git-repack process is trying to write to its > stdin (to name packs to keep/exclude). So that write may succeed or fail > based on how quickly the child dies. Yeah, I saw flaky failures myself during my integration tests. > Some options: > > - find a different way to convince repack to die. The most likely is > probably corrupting the cruft objects in some way such that > pack-objects dies, but not until it starts doing real work. > > - mark the test_must_fail with ok=sigpipe. In most case this is a > band-aid, but here we still test what we want. The .tmp cleanup > should kick in from a die() or from a signal death, so either is > sufficient for our purposes. > > - teach git-repack to ignore sigpipe. We don't actually check the > writes to pack-objects (since they're done by fprintf), but we'd > notice its failing exit code. And arguably this is improving the > user experience. Saying "pack-objects died with an error" is more > useful than a sigpipe. > > Thoughts? I agree that for this particular one the second one is a reasonable thing to do in the context of the test. The third one may actually be a better fix, exactly for the reason you state here. Thanks.
