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

[David Bolen]

Bob Ippolito <bob at redivi.com> writes:

with respect to deferredResult/deferredError:

> Sorry, but Jp is right.  They have no place in any code.  Their
> existence is a mistake and should be corrected ASAP.  They severely
> violate the reactor interface.

Hmm, then perhaps it's the reactor interface that could use some
improving rather than removing this high level functionality?  Or is
everyone just saying that the implementation (versus the concept) of
the current functions is done wrong.  Or maybe I'm just missing what
major mess having this sort of interface is exposing.

Given Glyph's recent response, I might also clarify that while our
heaviest use of these functions are within unit tests, we aren't using
trial, so any linkage between poor behavior of these implementations
and trial is not something I'm referring to or worry about breaking.

What I do think is that there is a very good and practical reason to
be able to unwind deferrable operations into a blocking structure for
a wide variety of test conditions, and losing that capability would
make writing tested Twisted code (at least for our situation) much
more fragile and error prone (the tests themselves).  And that's not
quite the same as the newer waitForDeferred stuff in 2.0 that
integrates with generators.  While slick, it still doesn't work as
well for easily maintainable tests in many of our situations.

If there's a problem with the current implementation, or with how the
current implementation may abuse the public reactor interface, then
I'm all for cleaning it up, fixing it or making it part of the reactor
itself in some way, as long as the public mechanism remains.  I mean,
after all, at some point the reactor itself really is just sitting
there iterating itself, so I find little reason not to permit some
access to hook into that mechanism when appropriate.

For example, in our system we may very heavy use of our own components
all of which implement their entire API via deferrable interfaces.
Interacting with multiple such components during a test is thus a
multi-stage deferrable process.  Having these blocking calls permits a
test method to say something like:

    self.user = deferredResult(self.umgr.user())
    self.user.email = 'test at dom.ain'
    deferredResult(self.umgr.saveUser(self.user))

in order to test being able to save a new user object into the system.
Part of the test is that the calls themselves should succeed, so the
natural result of deferredResult raising an exception on an errback is
just right.  And while clearly readable, the test still tests that the
deferrable aspect of the interfaces will work properly (e.g, with
deferredResult generating the exception if it doesn't get passed a
deferred).

Sure, I could write the user() and saveUser() calls with a callback
chain, and then I'd have to nest the call to saveUser based on the
user callback, probably something like:

    def fail(failure):
        self.fail(failure)

    def gotUser(user):
        self.user = user
        self.user.email = 'test at dom.ain'
        return self.umgr.saveUser(self.user)

    d = self.umgr.user()
    d.addCallback(gotUser)
    d.addErrback(fail)

but I see no advantage (*in the context of such a test*) to doing it
this way, and I see a real loss of clarity and maintainability for the
test itself.

Since each component in our system has a deferrable interface, any
test which is going to interact with multiple components would quickly
evolve a fairly deep callback system in the test, without much benefit
that I can see.

I'm all for fixing an implementation or cleaning up an interface if
it's being misused, but I certainly find value in the particular
external interface that deferred{Result,Error} deliver - so much so
that we would (and in fact did) implement them locally before finding
that they already existed.

-- David