> On Jan 9, 2017, at 4:13 AM, Jean-Paul Calderone <exar...@twistedmatrix.com>
> wrote:
>
> On Mon, Jan 9, 2017 at 4:52 AM, Glyph Lefkowitz <gl...@twistedmatrix.com
> <mailto:gl...@twistedmatrix.com>> wrote:
> On Jan 8, 2017, at 4:34 PM, Jean-Paul Calderone <exar...@twistedmatrix.com
> <mailto:exar...@twistedmatrix.com>> wrote:
>>
>> Here's one example I know of off the top of my head,
>> <https://github.com/ScatterHQ/flocker/blob/master/flocker/restapi/testtools.py#L316
>>
>> <https://github.com/ScatterHQ/flocker/blob/master/flocker/restapi/testtools.py#L316>>.
>> This one isn't a from-scratch re-implementation of the implicit Request
>> interface but a Request subclass, instead. However, it still interacts with
>> a lot of important pieces of Request which aren't part of IRequest.
>
> Mark already addressed how he won't be breaking this use-case (which is
> hugely important, and core to the whole idea of a compatibility policy, so
> that is as it should be).
>
> However, this kind of test-mocking is, I think, ultimately done at the wrong
> layer. It's trying to override some very vaguely-specified internals in the
> middle of an implementation.
>
>
> Absolutely.
Yay! Glad to have some consensus on this.
> Really, twisted.web should provide its own testing tools, of course. But if
> you're going to implement something that does this sort of overriding, I
> think the idiom to follow would be treq.testing:
> https://github.com/twisted/treq/blob/fcf5deb976c955ca6ef6484f414d25839932940e/src/treq/testing.py
>
> <https://github.com/twisted/treq/blob/fcf5deb976c955ca6ef6484f414d25839932940e/src/treq/testing.py>,
> rather than any of the various implementations of DummyRequest (including
> more than a few I'm sure I've written).
>
> Though, note, the link I gave above was support code for something very
> similar to this treq code:
> https://github.com/ScatterHQ/flocker/blob/master/flocker/restapi/testtools.py#L460
>
> <https://github.com/ScatterHQ/flocker/blob/master/flocker/restapi/testtools.py#L460>
>
> To clarify your point a bit (I think):
>
> MemoryAgent (from Flocker) provides a testing IAgent by implementing an
> IRequest that does in-memory resource traversal to dispatch requests and
> generate responses.
> RequestTraversalAgent (from treq) provides a testing IAgent by implementing
> (using) iosim to do in-memory protocol/transport interacts to drive an
> in-memory HTTP conversation that runs all of the regular Twisted Web HTTP
> processing machinery.
> RequestTraversalAgent's approach is better because the protocol/transport
> interface is better defined. Because it's better defined,
> RequestTraversalAgent doesn't have to touch pieces that we might want to
> consider implementation details (whether they're _-prefixed or not). It also
> invokes a larger portion of the real implementation code making it more
> likely to be a realistic simulation of real-world use of the code.
100% agree.
> Having spelled this out, what occurs to me now is that RequestTraversalAgent
> is really just a step or so up the ladder and there's further to go.
I generally agree with this next bit, but I have my own spin. So, one point at
a time:
> For example, RequestTraversalAgent only needs to exist at all because `iosim`
> has a distinct interface. This distinct interface means you need a
> RequestTraversalAgent-like thing for each reactor-using thing for which you
> want to provide a test double.
I think that there are pieces of RequestTraversalAgent for which this is true;
the parts where it has to instantiate its own FakeTransport objects and
IPv4Addresses and whatever. This could be streamlined a lot more.
But at the higher level, each layer will want its own public API entrypoint, to
facilitate the orchestration and setup of domain-specific client/server objects
and reduce boilerplate for testing at a given level.
> If this adaption behavior were bundled up differently then I think we'd get a
> lot more in-memory test doubles for free (or closer to it - we'd be another
> rung up the ladder, at least).
This I definitely agree with. Each abstraction-level-specific fake should be a
paper-thin layer over the one below, and each abstraction layer should cover a
fair amount of ground for itself. For example, you can test Klein apps
trivially with treq.testing, without adding very much extra logic at all.
> That seems like it would be a big win given how few of these testing helpers
> Twisted has historically managed to provide (suggesting it's just too hard to
> do so given the current tools).
Here I think there are a few mitigating factors:
I don't actually think that it's that hard using current tools. It's pretty
easy, in fact, to build up from the bottom and layer subsequent things on top
of another.
The way I see the problem that we're facing is that we realized we should be
doing this many years after doing the actual implementation, and we realized
this several layers up. So we built mid-tier test mocks and then tried to
build on top of them, rather than heading straight for the bottom layer. This
has lead to awkwardness like needing to monkeypatch HostnameEndpoint to avoid
deferToThread calls.
So, even now, we have an anemic bottom layer. task.Clock is pretty good, but
MemoryReactor still leaves a lot to be desired.
Therefore, I don't think that we really need to build that many different
abstraction layers; if we really focus on nailing down (A)
MemoryReactor+iosim+something with addresses so 'connectTCP' can be hooked up
to 'listenTCP' when both are given the same address, and (B) the vast majority
of what folks want to be able to test is HTTP API type stuff, so integrating
treq.testing (treq is license-compatible, so we can just move the code if we
want) would get us a _long_ way there, and would popularize this style with a
lot of other developers who might then be motivated to help improve the more
obscure corners, like, say, DNS.
I've been slowly improving the testability of Twisted's core; the recent
name-resolver change made a LOT of things easier. We should be making
deterministicResolvingReactor and twisted.threads (along with MemoryWorker)
public, and a lot of the circuitous routes that higher-level code needed to
traverse to get to "no I/O" will just go away. I'm probably forgetting a few
other building blocks, but I've noticed that each additional one that I remove
makes the task more like exponentially than linearly easier.
> This is a set of ideas that I've been gradually arriving at over a long
> period of time, but it's probably high time for some public discussion by
> now, even if it's just everybody saying "yeah, that sounds good" :-).
>
> So, it pretty much sounds good, though with the above refinements. :)
Cool. I suspect I haven't even covered all of it with my refine-refinements
above, but at this point it seems clear we can all pull in the same direction.
I wonder if a themed sprint at some point in the next year might be worthwhile?
>
> On a related note, 'proto_helpers' is in a super awkward place.
> 'twisted.testing', anyone? :)
>
> Yes. I almost suggested this about a week ago when I was preparing to
> contribute some testing code but then realized I couldn't contribute the code
> after all. :(
Well, now it's just a matter of time :).
-g
_______________________________________________
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
