I get the feeling you misunderstand what I'm saying we should do.
glyph@divmod.com wrote:
On 6 Aug, 02:17 pm, andrew-twisted@puzzling.org wrote:
[...]
I don't think this is the right approach. The right approach is to fix Twisted to support multiple simultaneous reactors, so that your Twisted test runner that wants to do stuff with a reactor is isolated from the tests, and vice versa. The tests should then use a fresh reactor for each test. It's simple and robust.
Your suggested implementation reinforces the antipattern of "tests are special and need 'waitFor' or 'blockOn' because they can't be written otherwise". Then, of course, newbies ask why the tests can have this
Not at all. I never said that tests should be using techniques like 'waitFor'/'blockOn'.
I think the current API that test methods return Deferreds is just fine. I am not proposing that people writing tests should be starting and stopping reactors (unless of course they are unit testing the starting and stopping of reactors...). I am proposing that TwistedTestCase.run should be doing this internally.
but their protocol implementation (which really needs it, seriously, it's not like *any* other application using Twisted) can't. Aside from the fact that it might actually work / be tested, it is the same (as far as I'm concerned) as much of the brokenness that Trial has dealt with for quite some time.
I definitely am not suggesting that tests should be significantly different to any other code.
(Incidentally, there are times when multiple reactors would be useful outside of Trial.)
Much code within and without Twisted uses, and will continue for the forseeable future to use, "from twisted.internet import reactor" to access the reactor. One might hope that this usage would eventually be replaced by something better, but it's not clear if this (or an equivalent spelling) could ever be *completely* eliminated. I quite like Jim Fulton's suggestion for adding an ITransport.reactor attribute and using that in most places where the global import is currently used. However, even if we had a comprehensive somehow non-global way to get at a reactor available today, there would still be a *very* lengthy transition period to a new API. The question will remain what to do about that code.
Sure. We will be living with the concept of a default global reactor for a long time.
That doesn't mean it can't be made restartable.
My main objection here, though, is that I'd really like to be able to add nifty Twisted-using features to Trial, and it's basically impossible right now, due in large part to the fact that the reactor keeps starting and stopping. Creating a new reactor for each test is going to create confusing semantics for code written using established idioms, because either the framework is going to go to a lot of trouble to fool everything into using trial's idea of the reactor the tests should be using (which begs the question: how do you test trial itself, if it has a reference to the "real" reactor?), or it's going to require special hacks to get at the "real" reactor which still won't behave in an event- driven way if a test (shock, horror) actually does want to do some real I/O itself. Although I am *personally* focusing on how to write better and more isolated unit tests using trial, I know of a small number of people using it as an integration testing tool that does tons of I/O to external systems and I think that is an interesting use-case and should be better supported, not worse.
I don't understand this.
Having a separate reactor for Trial and for the system-under-test (SUT) makes Trial *more* testable.
Your point about “confusing semantics for code written using established idioms” is just the inevitable result of supporting a default global reactor. That's not Trial's fault, it's Twisted's. The way for Trial to cope with it gracefully is to add support for multiple simultaneous reactors to Twisted, and then make Trial use the non-global one for its own use, and let the tests keep using the global one, so that legacy code will be unaffected.
I do not see how doing large amounts of I/O in test methods will be made any worse by this design.
Perhaps the 'trial' tool itself is a misguided design though, and 'disttrial' will simply replace it in short order. If this is the case, then the tests are running in a subprocess anyway, and there's no reason to run any code in-process with the tests, except for things to gather metrics. In that case, the 'disttrial' tool itself is a real Twisted program, and the subprocess fakes just enough to get by:
http://twistedmatrix.com/trac/browser/branches/disttrial-1784/twisted/trial/...
That *particular* hack makes me cringe, but I think the overall architecture may satisfy us both better in the end.
Right, it's a hack that gives us multiple reactors by putting Trial and the SUT in separate processes, so that Trial and the SUT are using different reactors. I'm saying that if we supported multiple reactors in the same process we could and should use that feature to do the same thing: give Trial and the SUT different reactors.
(Whether or not you share the same reactor instance between multiple tests is a separate issue. I think experience has taught us that doing that is fragile, which is why I also advocate for a new reactor instance for each test, but that's really an orthogonal issue.)
-Andrew.