On 9 September 2014 12:02, Raphaël Badin <raphael.badin@xxxxxxxxxxxxx> wrote:
[...]
I wondering if we couldn't have something more systematic and have
this done, for all the available cluster RPC methods, in the base
test class (and for every cluster created in the tests). Of course
you'll have to configure mock objects if you're expecting a
particular response from one of the RPC methods. But if all you need
is the code not to blow up because a cluster RPC method has been
called as a side-effect of what you're actually testing, this will
be taken care of.
If we get to the point where we're stubbing out more than 2 or 3 RPC
calls in a test then we're not writing a unit test any more. The
desire for a does-all-the-things mock cluster might be a
manifestation of writing overly broad tests, or of not writing small
enough units of testable code. Equally, having a does-all-the-things
mock makes it easier to write overly broad tests and overly large
units of code.
I disagree. We're in a situation where the RPC commands are so deep in
the code base that it's unavoidable that they are called as
side-effects of things that are being unit-tested.
We should seek to make them less deep, which is fairly hand-wavy, or
write code such that we can, when testing, detach it from the n-levels
of indirection between it and the RPC calls (or whatever side-effect it
is that is causing us bother). Otherwise we're not unit testing the code
we're purporting to.
Fwiw, I think signals in model code can contribute to this: tests
always have to deal with side-effects from signals, which now might
include RPC. I liked the way that we dealt with needing side-effects
as a result of model changes in Launchpad: enforce use of a mutator
method and forbid use of the model attribute from view/controller
code. We could test the mutator, its use was explicit, but we could
bypass it when setting up tests.
fwiw, Celery was dealing quite elegantly with this (see
src/maastesting/celery.py). The CeleryFixture would allow the tasks to
be run (synchronously) and collect a record of all the tasks being run
in a test.
CeleryFixture is not the whole story though: there's also DHCP_CONNECT
and DNS_CONNECT, there to stop tests from doing real DHCP and DNS stuff.
During the RPC work I've taken out many patches to Omshell which do
something similar. There are specialised configurations for Celery and
Django so that tasks, and Celery and Django themselves, can even be
functional during tests. It all works... but is it elegant?
The RPC fixtures allow you to go from nothing to fully functional in a
few lines of code, no external configuration needed. The thing you must
do is fill in the stubs. Then even help you to do so correctly, because
calls and responses are validated against the call's schema. It looks
like a lot of work now, but don't forget how much work has gone into
making MAAS testable as it is. Over time we can extract more commonality
from what we have, and refactor, and consolidate.
