Feedback on a base "fake" type in the testing repo

[John Meinel]

High level thoughts:

   1. Using an interface{} instead of patching function pointers sounds
   like a great thing
   2. Writing a provider via faking out rather than implementing a double
   means that it is really hard to run the cross-provider interface tests
   (environs/jujutest), which means we run into what we have with Azure, where
   *some* of the functions return a different error than the one we expect
   (see the recent 'juju restore' regression). Our interface tests aren't
   amazing, but if we can push more on that, then we can avoid having
   implementations leak through their abstractions.
   3. I do realize that time-to-something-working is a lot faster with a
   Fake implementation, but ongoing support and maintenance might pay off
   more. (I love how quickly the GCE provider was available, I'm concerned if
   we know that it actually acts like EC2/Openstack when it comes to edge
   cases.)

John
=:->


On Wed, Feb 11, 2015 at 10:53 PM, Eric Snow <eric.snow at canonical.com> wrote:

> tl;dr Using fakes for testing works well so I wrote a base fake type. [1]
>
> While working on the GCE provider, Wayne and I started taking a
> different approach to unit testing than the usual 1. expose an
> external dependency as an unexported var; 2.export it in
> export_test.go; 3. patch it out in tests. [2]  Instead we wrote an
> interface for the provider's low-level API and implemented the
> provider relative to that. [3]  Then in tests we used a fake
> implementation of that interface instead of the concrete one.  Instead
> of making the actual API requests, the fake simply tracked method
> calls and controlled return values.
>
> Using the fake had a number of benefits:
>
> 1. our tests were very focused on what code gets exercised, meaning we
> don't take responsibility for what amounts to testing our dependencies
> at the same time as the code at hand.
> 2. the tests ran faster since they aren't making HTTP requests (even
> if just to localhost).
> 3. the provider code isn't cluttered up with
> vars-only-there-to-be-patched-out. [2]
> 4. the test code isn't cluttered with many separate s.PatchValue calls. [2]
> 5. setting up the faked return values was cleaner.
> 6. checking which methods were called (in order) along with the
> arguments, was easier and cleaner.
>
> In addition to all that, taking the fake approach required that we
> encapsulate our low-level dependencies in a single interface.  This
> was good because it forced us to spell out those dependencies.  That
> helped us write the provider better.  The low-level interface also
> made the code more maintainable since it makes the boundary layers
> explicit, and formats it in a concise display (the interface
> definition).
>
> So I've taken the base fake type from the GCE patch and pulled it into
> patch against the testing repo. [1]  I've made some adjustments based
> on use cases I've had in subsequent patches.  Nate has the bright idea
> of getting some feedback from the team before landing anything "since
> it's the kind of thing that'll start popping up everywhere, and I want
> to make sure it passes muster with others."
>
> I'm not suggesting that fakes are the solution to all testing problems
> so let's please avoid that discussion. :)  Neither am I proposing that
> any existing tests should be converted to use fakes.  Nor am I
> suggesting the need for any policy recommending the use of fakes.  As
> well, we still need to make sure we have enough "full stack" test
> coverage to be confident about integration between layers.  Relatedly,
> the concrete implementation of low-level interfaces needs adequate
> test coverage to ensure the underlying APIs (remote, etc.) continue to
> match expected behavior.  So yeah, neither fakes nor isolated unit
> tests meet all our testing needs.  I just think the base fake type
> I've written will help facilitate a cleaner approach where it's
> appropriate.
>
> Thoughts?
>
> -eric
>
> [1] https://github.com/juju/testing/pull/48
> [2] We were still feeling out the approach so we still took the
> var-and-patch in some cases.
> [3] See "gceConnection" in http://reviews.vapour.ws/r/771/diff/#46.
>
> --
> Juju-dev mailing list
> Juju-dev at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/juju-dev/attachments/20150213/503b4f94/attachment.html>