Open Stack

On 23 September 2014 08:09, Jay Pipes <jaypipes at gmail.com> wrote:
> On 09/22/2014 03:36 PM, Robert Collins wrote:

>> I'm going to push on this a bit further. Mocking is fine in many
>> cases, but it encodes dependencies from within your objects into the
>> test suite: e.g. that X will call Y and then Z - and if thats what you
>> want to *test*, thats great, but it often isn't outside of the very
>> smallest unit tests - because it makes your tests fragile to change -
>> tests for a class Foo need to change when a parent class Bar alters,
>> or when a utility class Quux alters. And that coupling has a cost and
>> is a pain.
>
>
> It's a pain that is on purpose, though. The coupling of the unit test to the
> class relationship is precisely to cause pain when the interface changes. It
> is this pain that informs you that "oh, I changed an interface, so I need to
> change the unit test that verifies that interface".
>
> The problem we have in OpenStack right now is too many fake stub methods
> that hide too much, either using stuff like:
>
>  def fake_get_instance(*args, **kwargs):
>     ... blah
>
> or returning completely incorrect stuff (like dicts instead of objects, thus
> in some cases making our unit tests the *only* remaining place where certain
> things are passed as dicts and not objects).
>
> Also, for the record, when I refer to fakes, I'm mostly referring to stubbed
> out methods that really should instead be calls to
> mock.MagicMock.assert_called[_once]_with().
>
> I'm not referring to fake drivers. Those are a special kind of problem in
> OpenStack, but not related to the use of fake stub methods when mock should
> be used instead.
>
> The problem with some fake drivers in OpenStack code is demonstrated here:
>
> https://github.com/openstack/nova/blob/e7d4ede1317ca0401bf8b4d49d58d000c53b1ed2/nova/tests/image/fake.py#L36
>
> * The fixtures contain types that don't actually come back from Glance (for
> example size isn't a string and the datetime format is wrong for some
> versions of Glance API)
>
> * The fake download() method ends up only exposing part of the real
> download() method, and it gets the return arguments wrong for other parts
>
> * The fake download() method masks over the real driver's complexity in
> requiring the v2 Glance API (the locations attribute of the image) and thus
> introduces further mistakes into the unit tests that (used to) depend on
> this thing.
>
> * The fake stub_out_image_service() completely messes up the return values
> by returning the wrong parameters
>
> All of this is just the stubbed out fakes for the image service. All of the
> above introduced subtle bugs into the unit tests and produced a situation
> where the unit tests were erroneously passing when the RBD image backend
> patches (the ones that were reverted days after inclusion right before
> Icehouse was released, remember?) used the fake image service instead of
> actually verifying the interface that exists in the real driver.
>
> If instead of writing full fake drivers for some of these important parts of
> the code base, we instead just mocked out the specific methods that cross
> performance or dependency-sensitive boundaries, I think we'd all be a bit
> saner.

So that fake ImageService is a *not* a verified fake - as you say it
had many forms of skew vs the real service, and those were fatal for
us. Mocking is no safer than that fake ImageService though: because
mocks are tautologies, they are subject to exactly the same skew, just
worse. They are no use for ensuring that code works in reality, only
for checking that the locally provided definition works. Mocks that
record interactions with real objects and capture it for you are a lot
better, but we don't have any of those in OpenStack today.

-Rob

-- 
Robert Collins <rbtcollins at hp.com>
Distinguished Technologist
HP Converged Cloud