test_threads.ReactorThreadsTestCase is very fragile

While debugging with threads, and looking at #1115, I got some problems with test_threads.ReactorThreadsTestCase: 2 test methods can't be run alone (testCallInThread and testWakerOverflow), because reactor doesn't seem to be initialized like it should be.

Change History (12)

OK I attach a patch correcting the problem. I identify the blocking part in reactor._initThreadPool, because the start of the threadpool is delayed until reactor starts, which seems to happen too late for the tests using a threading.Event.

So starting the pool in the tests resolves the problem, even though it may not be the cleanest way.

both because it is shorter and because it avoids the potentially error-prone hasattr call.

The two tests which do deferToThread(time.sleep, 0) now should probably have a brief comment explaining the purpose of that, since I imagine it looks pointless to someone who isn't pretty familiar with how threading and reactor startup interact and how trial drives the reactor.

Added docstrings and the test_suggestThreadPoolSize improvement are nice. Go ahead and merge once the above stuff is taken care of.

Okay, as long as I have another chance to complain, I found a couple other things I think we can improve :)

threadpoolShutdownID should be documented, at least with a comment. Maybe it should also be private.

The shutdown system event trigger it is associated with should also reset threadpoolShutdownID back to None

Since that will require a new method, trial might as well call that method to stop the threadpool and make sure the rest of the reactor's state is consistent, instead of poking the threadpool directly. This still isn't ideal, since the new method won't be part of any interface the reactor actually declares, but it's slightly better than poking attributes directly.

On IRC we talked about having do_cleanThreads fire a shutdown event instead of poking the threadpool directly. This seems like a good idea but in ten minutes of fiddling I haven't actually gotten it to work. If you want to work on the idea, feel free, otherwise it can probably be done in another branch.