twisted.internet.test.test_sigchld way too aggressive in assuming no previous fd or SIGCHLD handler was installed

[itamar]

twisted.internet.test.test_sigchld.SetWakeupSIGCHLDTests.setUp assumes no one has previously set a SIGCHLD handler or registered a wakeup fd. If someone has, that is considered an error. However, any random test that uses the reactor will cause this registration; AFAICT this never caused problems before due to a specific sort order of running tests.

On Ubuntu 12.10:

$ trial -z 1 twisted.test.test_udp twisted.internet.test.test_sigchld

...

[ERROR]
Traceback (most recent call last):
  File "/home/itamarst/Devel/Twisted/trunk/twisted/internet/test/test_sigchld.py", line 58, in setUp
    raise RuntimeError("You used some signal APIs wrong!  Try again.")
exceptions.RuntimeError: You used some signal APIs wrong!  Try again.

twisted.internet.test.test_sigchld.SetWakeupSIGCHLDTests.test_uninstallHandler

[itamarst]

(In [36265]) Branching to 'sigchld-tests-6161'

[itamar]

I fixed this by just not raising a RuntimeError if a previous handler was installed; tearDown already has code to more or less restore the previous signal handler. I also changed the signal handler registered for SIGCHLD to have a name, so that the log message in this situation is more meaningful (the name of the function is logged).

 http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/sigchld-tests-6161 started.

[itamar]

This is, sadly, a dependency for the gireactor port (#6055).

[exarkun]

AFAICT this never caused problems before due to a specific sort order of running tests.

Actually it has shown up on buildbot a few times - we have random test order builders. So it'll be nice to have it fixed. :)

1. Maybe we don't need the warning either? It does seem perfectly true that a prior test may have legitimately installed some kind of signal handler. I don't remember why this check was added in the first place - perhaps only as a way to track down bugs in test_sigchld.py.
2. I'll bike shed on the name of doNothing a little bit. Perhaps noopSignalHandler?

That's all, please address these to your satisfaction and merge.

[itamar]

1. It seems kinda untidy to leave global state changes around as a result of tests. If these tests can restore it to where it was when they started (module siginterrupt), why not other tests? So keeping this as reminder seems OK to me.
2. Done.

[itamarst]

(In [36329]) Merge sigchld-tests-6161: Don't fail test_sigchld if SIGCHLD was previously installed.

Author: itamar Review: exarkun Fixes: #6161

The twisted.internet.test.test_sigchld tests were previously somewhat fragile, erroneously raising RuntimeError if a previous test had left a SIGCHLD handler lying around.