[Twisted-Python] Re: Hanging test cases (Was: Evangelism notes...)

[Jp Calderone]

On 06 May 2005 20:19:45 -0400, David Bolen <db3l at fitlinxx.com> wrote:
>Bob Ippolito <bob at redivi.com> writes:
>
>> (1) Reactors can only be (meaningfully/predictably/etc) iterated if
>> Twisted rules the universe AND the implementation of that reactor is
>> amenable to that feature.  This is not a tautology.
>> (2) Reactors need to fire various startup/shutdown events.  Reactors
>> shouldn't be doing ANYTHING unless they are in a running state.
>>
>> The current deferredResult/deferredError breaks both of these
>> conditions.
>> (1) It iterates the reactor (which is a historically public, but
>> conceptually broken interface)
>> (2) It iterates the reactor in a STOPPED state.  The reactor is never
>> "running" during these tests.  Startup/Shutdown does not happen!
>
>This seems like an internal implementation issue to me - when a
>reactor is "running" (I've called run()), it's basically stuck in a
>loop doing runUntilCurrent and then doIteration.  That's precisely
>what iterate does.
>

  It's an implementation detail, yes, but not an internal one.  For the reactor to operate properly, it *must* be started up and shut down.  That's simply a requirement of the interface.  It's not even a particularly unusual one: many libraries require you call some initialization function before proceeding to other APIs they provide.

  With recent versions of Twisted, many reactors don't actually depend terribly heavily on being started up, so you may be able to call iterate() without calling run() first and see something resembling correct behavior.  However, you should note that future versions of Twisted may introduce new startup requirements which break programs which assume reactors do not need an explicit startup event.  Additionally, third-party reactors may depend on the startup event now, thus breaking when used with code that only uses iterate().

  The shutdown event is much more important these days.  People who have noticed Twisted programs and tests which hang at shutdown can attest to this.

> [snip]
>
>I guess that's true of any nested use of deferredResult too, but we
>don't nest our deferredResult calls - no real need since any
>deferrable is directly wrapped with the deferredResult call, and
>deferredResult is only used in the tests, so what they are calling is
>always production code that is written properly with callbacks and
>what not.

  This is one of the cases, yes.

>
>> [snip]
>
>I still don't necessarily see that (the last sentence).  We use
>multiple deferrable operations in single tests, but never more than
>one at a time (e.g., no recursive or nested uses).  But certainly more
>than a single deferrable operation within a single test.
>
>I would, however, agree that I'd prefer even more the ability to
>completely start/stop a reactor during the course of a test, but would
>still like to be able to iterate it manually during the test to
>provide a natural blocking flow to the test.

  Your application code all deals with Deferred callbacks.  Why is it a stretch to have your test code do the same?

>
>But in my current scenario, the majority of my components under test
>are having interfaces tested that are not impacted by startup/shutdown
>(and we don't use any services, in the Twisted sense, for example).

  That you know of.  With current releases of Twisted.

> [snip]
>
>Yeah, that's a tough problem, although one that would also simplify
>the fact that we often run the tests under the unittest GUI, and
>occasionally have to fight cross-test pollution from the reactor
>persisting across test runs, which is a wart from the current
>restrictions.

  This is one reason you should use trial, even though it is broken.  Trial will be fixed by Twisted developers who (at least occassionally) have a firm grasp of how it can correctly interact with the reactor, so by using it you benefit from this understanding.  By using unittest, you are forced to get all the things right that trial will someday get right, or accept a broken test harness.

  If you have feature requirements that trial does not satisfy, make some feature requests :)  We are quite aware that trial is not yet feature complete and will welcome suggestions for features that make it a more useful tool.

> [snip]
> 
>That's what I meant by saying that the integration into generators is
>slick (and goes a good way to linearizing what is normally a callback
>chain), but still isn't quite as simple as the interface provided by
>the deferred{Result,Error} functions.

  I don't think anyone is trying to suggest that it is simpler.  However, it has the advantage of being correct.  Given the choice between simplicity and correctness, I am inclined towards correctness.

>
>> Well, let's say your database thing is a service, that maintains some
>> kind of ephemeral state that's required in some way.  If
>> deferredResult were properly written, this ephemeral state would hit
>> the bit bucket on each deferredResult, probably breaking your code
>> even though the test are "correct".
>
>I might be getting lost on the "properly written" part, but if I were
>testing a component that did have state triggered during reactor
>startup/shutdown (which is what I think you're referring to), that
>test would likely be using direct calls to the component to trigger
>the startup/shutdown actions as part of the test setup/teardown, but
>without using the reactor.

  You can definitely do this.  The problem is that the reactor may have internal setup and tear down of which you are not aware and which will cause problems if skipped.

> [snip]
>
>I don't know - maybe my use case is just limited enough (non trial, no
>nesting, etc...) that I don't see any true exposures through
>deferred{Result,Error} while I'm getting benefits.

  It may be the case that none of the bugs in trial synchronous result utilities cause problems for you.  If they don't, I can understand how it would not be a big priority for you to move away from them (after all, I'm sure you have plenty of things that *are* causing you problems now that you would much rather spend time on).  I recommend simply keeping this conversation in mind if and when you run into problems with your use of deferredResult and deferredError in the future and that you not let such problems take you by surprise.  You can rest relatively easily in the knowledge that deferredResult and friends will not disappear from Twisted overnight.  Broken as they are, they are clearly part of a public API and so will remain long enough to satisfy backwards compatibility requirements.

  Jp