trial doesn't start the reactor before calling the first test method

[exarkun]

trial should try as hard as it can to run all test methods in a consistent manner. The inconsistency between whether the reactor has been started or not should go away.

[exarkun]

Ooops, slight mischaracterization. It runs all tests up until the one after the first to return a Deferred (maybe an unfired Deferred?) without starting the reactor.

[jml]

TestCase explicitly doesn't run the reactor for tests that fire synchronously. From what I gather, this was a conscious decision.

You think the reactor should be run for all tests? With the current implementation of _wait, that'd be a little bit weird, because the reactor would only start after the test has.

jml

[glyph]

IMHO:

• The global reactor should start when trial starts.
• The global reactor should end when trial ends.
• _wait should go away.

The test framework should ask the test before it runs, "do you want the reactor to be running". If the answer is "yes" then you get a new behavior, where the reactor is running around setUp, the test method, and tearDown. If you say "no", you get the current behavior, and a deprecation warning if you return a Deferred.

This doesn't necessarily mean that the runner actually needs to interrogate the TestCase; it could simply be a new TestCase subclass with a different run(). Such a run() would implicitly prohibit the use of, i.e. the global reactor's iterate() method, too.

However, I don't think that this behavior can be changed for the existing twisted.trial.unittest.TestCase without breaking the behavior that a lot of tests depend upon.

jml - does this sound like a good plan?

[glyph]

Aah, I left a bit out.

The plan I am suggesting does not involve actually starting the reactor when trial starts and stopping it when it stops. I'm saying that we could have a new TestCase class which would have a run for compatibility with stdlib unittest, zope testrunner, and the current run infrastructure, but start and stop the reactor in a more consistent way for test methods. However, such a class would be much more amenable to implementing a different method for invoking the test method asynchronously while the reactor was already running.

More importantly by making it a subclass of something different, we'd have a nice, explicit division between the old and new behaviors, and it would be easy for a reader of some test code to understand its requirements of the reactor's run state.

[jml]

Hi Glyph,

Your post confused me a little so I'll say what I think as clearly and concretely as I can:

• The trial command line tool can start the global reactor if it pleases.
• There should be a base TestCase-like class for tests that need to do asynchronous operations.
  • There must be a way of running this class using the .run(result) synchronous interface.
  • One way to do this is to implement run on the class itself.
• Twisted should also have a base test case for synchronous tests of the Twisted test suite.

Now, if I understand correctly, you are suggesting a new base TestCase which implements a synchronous interface for asynchronous ops that doesn't suffer from the inflexibilty of our current _wait / _Janitor combo. That sounds good enough to me.

So far I think I've just been agreeing with you.

Two questions:

• How do you suggest this is implemented?
• Are you proposing that there also be a way to share the reactor between tests?

I am ambivalent about this proposition. As much as possible, the reactor should be treated like any other test fixture. The way we share it should be the way we share any other resource between tests. Not that Trial has a blessed way of doing it at the moment. In any case, I consider this vastly less important than improving our reactor isolation between tests.

[glyph]

Replying to jml:

Hi Glyph, Your post confused me a little so I'll say what I think as clearly and concretely as I can: * The trial command line tool can start the global reactor if it pleases. * There should be a base TestCase-like class for tests that need to do asynchronous operations. * There must be a way of running this class using the .run(result) synchronous interface. * One way to do this is to implement run on the class itself. * Twisted should also have a base test case for synchronous tests of the Twisted test suite. Now, if I understand correctly, you are suggesting a new base TestCase which implements a synchronous interface for asynchronous ops that doesn't suffer from the inflexibilty of our current _wait / _Janitor combo. That sounds good enough to me. So far I think I've just been agreeing with you.

Indeed.

Two questions: * How do you suggest this is implemented?

I'm not clear on the level of dependency between crud in the Runner and crud in TestCase.run(); if (as I'm hoping) there isn't very much, then all I'm suggesting is a new TestCase which does something cleaner and has different, and much simpler, semantics as far as when the reactor is running; i.e. the reactor should always (somehow - and the rambling in my comment was mostly a way of saying "and I don't care how") be running when setUp, tearDown, and the test method are called, and crashed/restarted/cleaned up after that.

If there is some stuff still in the runner which makes this simple proposal more difficult, then I would propose having it explicitly know about both kinds of test cases and make whatever allowances are necessary.

This will have to be a sssslllooowwwwwww deprecation process, but if we can address other subtleties

* Are you proposing that there also be a way to share the reactor between tests?

No. At least, I'm not talking about increasing the level of sharing. The same clean-up should be performed, connections forcibly closed, etc. However, until there is no such thing as "the global reactor" (and I don't think we can ever completely eliminate this concept, given the semantics of receiving an event and propagating a call stack in Python) then the tests must continue to share it or resort to some pretty terrifying hacks to make 'from twisted.internet import reactor' look like it works. So I'm also not proposing any terrifying hacks to preclude sharing the reactor as much as it already is. I'd like to make the hacks less terrifying, in general: for example, I'd like to get rid of as much of _deprecateReactor as we can.

I am ambivalent about this proposition. As much as possible, the reactor should be treated like any other test fixture. The way we share it should be the way we share any other resource between tests. Not that Trial has a blessed way of doing it at the moment. In any case, I consider this vastly less important than improving our reactor isolation between tests.

I'd also really like to encourage test authors to isolate their tests (and their code, in general) from the global reactor. However, I also believe it's helpful to be able to have some kinds of tests use it, sometimes. For example, one usage I talked to a guy about one time was a suite of tests which would request certain data from a bank of servers which were all supposed to be online and responding. These tests are not really "unit" tests, but trial is useful as an integration testing tool and we shouldn't cripple it for that. However, while these tests use the reactor, they don't really share it, at least not more than they share resources like "the filesystem" or "memory"; the tests should still have no interaction with each other, and don't need to run in parallel.

Here's a wacky idea, tell me if you like it: perhaps there shouldn't be two test classes, one for "old" and one for "new", but three: two new ones, one which runs the reactor and one which doesn't. The one which doesn't could support Deferreds but complain or fail if the Deferred wasn't fired by the time the test was done. It could therefore run under a "real" reactor but it would complain if you actually tried to use it by accident. This would provide encouragement for the simple default case to not rely on the reactor; when you do start relying on the reactor, it is an explicit decision, "this test is going to do some I/O, I want a real honest-to-god event loop". It could reside in a module documented to not be for unit testing :). We could push this even further to provide another utility class, a test case which provided a verified-fake reactor and did all the appropriate introspection and cleanup logic on the fake reactor rather than the real one; this reactor could then be passed to a sufficiently well-written SUT.

None of this is necessary to clean up the interactions described in the description of this ticket between existing tests and the reactor, but perhaps worth considering if we are going to deprecate the old API and provide an ostensibly better way of doing things.

[glyph]

Replying to glyph:

Ahem.

This will have to be a sssslllooowwwwwww deprecation process, but if we can address other subtleties

... in the semantics of reactor integration, perhaps we won't have to do another one.

[jml]

Replying to glyph:

Replying to jml:

Two questions: * How do you suggest this is implemented?

I'm not clear on the level of dependency between crud in the Runner and crud in TestCase.run(); if (as I'm hoping) there isn't very much,

There's not very much at all. Three cheers for separation of concerns!

then all I'm suggesting is a new TestCase which does something cleaner and has different, and much simpler, semantics as far as when the reactor is running; i.e. the reactor should always (somehow - and the rambling in my comment was mostly a way of saying "and I don't care how") be running when setUp, tearDown, and the test method are called, and crashed/restarted/cleaned up after that.

OK. So it *will* use something like _wait. It will simply involve less Satan.

This will have to be a sssslllooowwwwwww deprecation process, but if we can address other subtleties in the semantics of reactor integration, perhaps we won't have to do another one.

* Are you proposing that there also be a way to share the reactor between tests?

No. At least, I'm not talking about increasing the level of sharing. The same clean-up should be performed, connections forcibly closed, etc.

Good. I was afraid you were saying the opposite. :)

However, until there is no such thing as "the global reactor" (and I don't think we can ever completely eliminate this concept, given the semantics of receiving an event and propagating a call stack in Python)

I think I'd rather assume a thing is possible until proven otherwise.

then the tests must continue to share it or resort to some pretty terrifying hacks to make '`from twisted.internet import reactor`' look like it works.

Yes.

So I'm also not proposing any terrifying hacks to preclude sharing the reactor as much as it already is. I'd like to make the hacks less terrifying, in general: for example, I'd like to get rid of as much of _deprecateReactor as we can.

There's a ticket that Jean-Paul filed that this reminds me of... I'll look it up and post in a follow-up.

I am ambivalent about this proposition. As much as possible, the reactor should be treated like any other test fixture. The way we share it should be the way we share any other resource between tests. Not that Trial has a blessed way of doing it at the moment. In any case, I consider this vastly less important than improving our reactor isolation between tests.

I'd also really like to encourage test authors to isolate their tests (and their code, in general) from the global reactor. However, I also believe it's helpful to be able to have some kinds of tests use it, sometimes.

I think you've missed my point. I'm didn't say that tests should be isolated from the global reactor, I said that tests should be isolated from other tests that touch a reactor.

It's definitely extremely useful to be able to use it sometimes. I wrote a test today that launched an TCP server. Tests should be able to use a reactor, and that this reactor should be the real deal, and entirely isolated from everything else.

Here's a wacky idea, tell me if you like it: perhaps there shouldn't be two test classes, one for "old" and one for "new", but three: two new ones, one which runs the reactor and one which doesn't.

This isn't wacky. This idea has been pushed by spiv for years, and I've largely been agreeing with him.

I even tried to implement it once, but got bogged down because my approach wasn't incremental enough.

The one which doesn't could support Deferreds but complain or fail if the Deferred wasn't fired by the time the test was done. It could therefore run under a "real" reactor but it would complain if you actually tried to use it by accident. This would provide encouragement for the simple default case to not rely on the reactor; when you do start relying on the reactor, it is an explicit decision, "this test is going to do some I/O, I want a real honest-to-god event loop". It could reside in a module documented to not be for unit testing :).

+1 with options on the details.

Related to this, getting even more off-topic, I think it might be useful to have TestCase subclasses for apps that use Twisted that are distinct from the TestCase classes used for the Twisted project.

It'll be easier to chase that idea up when I am encumbered by use cases.

We could push this even further to provide another utility class, a test case which provided a verified-fake reactor and did all the appropriate introspection and cleanup logic on the fake reactor rather than the real one; this reactor could then be passed to a sufficiently well-written SUT.

Well, this would be trivial, once we had a fake reactor and tests that parametrized the reactor.

None of this is necessary to clean up the interactions described in the description of this ticket between existing tests and the reactor, but perhaps worth considering if we are going to deprecate the old API and provide an ostensibly better way of doing things.

I think doing these things will make resolving this ticket easier.

This discussion is one of the more helpful ones I've had about Trial in a long time. I'm glad to see that we are both thinking along similar lines.

But-I-still-wish-this-was-on-mailing-list'ly yrs, jml

[exarkun]

To resolve this ticket, trial just needs to start the reactor before running code from the first test case. The rest of the stuff discussed here is probably great, but it's way beyond the fix for this. I'm not suggesting anything about the factoring of the implementation of this feature, I just require that the reactor has already started by the time the first setUp or test_foo is called on a twisted.trial.unittest.TestCase.

This problem causes warnings in our own test suite. It'd be great to have it resolved.

This ticket is also a duplicate of #1614. Lots more discussion has happened here than has happened there, so I'm going to close that one as a duplicate, even though it's older.

[exarkun]

#2219 was also a duplicate of this. I attached a code snippet there which shows three behaviors which change if the reactor starts before test code runs.

[exarkun]

Relating to the discussion about TestCase-refactoring and the like, #5853.