openstack team mailing list archive

[Sandy Walsh <sandy.walsh@xxxxxxxxxxxxx>]

Thanks Soren, I see what you're doing now and it makes perfect sense. It'll be a nice helper class.

My only snipe would be that mox is generic to any library and this fake only gives the benefit to db operations. We have to remember "It's a db operation, so I have to do this. It's another method call so I need to do that"

How much effort would it be to make it into a better/more generic mox library?

-S

________________________________________
From: Soren Hansen [soren@xxxxxxxxxxx]
Sent: Tuesday, November 22, 2011 7:38 PM
To: Sandy Walsh
Cc: openstack@xxxxxxxxxxxxxxxxxxx
Subject: Re: [Openstack] [nova-testing] Efforts for Essex

2011/11/22 Sandy Walsh <sandy.walsh@xxxxxxxxxxxxx>:
> I suspect the problem is coming in with our definition of "unit
> tests". I don't think a unit test should be calling out of the method
> being tested at all. So anything beyond stubbing out the methods
> within the method being tested seems like noise to me. What you're
> describing sounds more like integration tests.

If I'm testing a method that includes a call to the db api, the strategy
with which I choose to replace that call with a double does not change
whether the test is a unit test or not.

I'm simply replacing this:

def test_something(self):
        self.mox.StubOutWithMock(db, 'instance_get')
    db.instance_get(mox.IgnoreArg(), mox.IgnoreArg()
        ).AndReturn({'name': 'this or that',
                     'instance_type_id': 42})

    exercise_the_routine_that_will_eventually_do_an_instance_get()
    verify_that_the_system_is_now_in_the_desired_state()

or this:

def test_something(self):
    def fake_instance_get(context, instance_uuid):
        return {'name': 'this or that',
                'instance_type_id': 42}

    self.stubs.Set(nova.db, 'instance_get_by_uuid', fake_instance_get)

    exercise_the_routine_that_will_eventually_do_an_instance_get()
    verify_that_the_system_is_now_in_the_desired_state()

with this:

def test_something(self):
        ctxt = _get_context()
        db.instance_create(ctxt, {'name': 'this or that',
                              'instance_type_id': 42})

    exercise_the_routine_that_will_eventually_do_an_instance_get()
    verify_that_the_system_is_now_in_the_desired_state()

Not only is this -- to my eye -- much more readable, but because the
fake db driver has been proven (by the db test suite) to give responses
that are exactly like what the real db driver would return, we have
better confidence in the output of the test. E.g. if the real db driver
always sets a particular attribute to a particular default value, it's
remarkably easy to forget to follow suit in an ad-hoc mock, and it's
even easier to forget to update the countless ad-hoc mocks later on, if
such a new attribute is added. This may or may not affect the tested
code's behaviour, but if that was easy to see/predict, we wouldn't need
tests to begin with :)

Over the course of this thread, I've heard many people raise concerns
about whether we'd really be testing the fake or testing the thing that
depends on the fake. I just don't get that at all. Surely a fake DB
driver that is proven to be true to its real counterpart should make us
*more* sure that we're testing our code correctly than an ad-hoc mock
whose correctness is very difficult to verify?

> I thought the motive of your thread was to create
> fast/small/readable/non-brittle/maintainable tests.

The motive was to gather testing related goals, action items, thoughts,
complaints, whatever. It just so happens that a lot of people (myself
included) think that speeding up the test suite and categorising tests
into "true unit tests" and "everything else" are important things to
look at.

> Integration tests, while important, make this goal difficult.

I agree. I'm very happy that there's a lot of people doing a lot of work
on the integration test suite so that I can focus more on unit tests. As
I think I've mentioned before, unit tests are really all we can expect
people to run.

> So, if we're both talking about real unit tests, I don't seen the
> benefit of the fake.

Please elaborate (with my above comments in mind).

> As for my example of 123 vs "abc", that was a bad example. Let me
> rephrase ... in one test I may want to have an environment that has no
> pre-existing instances in the db. In another test I may want to have
> an environment with a hundred instances.
>
> I'd like to understand how configuring the fake for both of these
> scenarios will be any easier than just having a stub. It seems like an
> unnecessary abstraction.

First of all, the DB is blown away between each individual test, so we
don't have to worry about its initial state.

In the first scenario, I'd do nothing. I have a clean slate, so I'm good
to go. In the second scenario, I'd just do 100 calls to
db.instance_create.  With the mock approach, I'd write a custom
instance_get with a 100 if/elif clauses, returning whatever makes sense
for the given instance_id. Mind you, the objects that I return from my
mock may or may not be valid Instance objects. I can only hope that they
are close enough.

--
Soren Hansen        | http://linux2go.dk/
Ubuntu Developer    | http://www.ubuntu.com/
OpenStack Developer | http://www.openstack.org/