On Thu, Feb 12, 2015 at 2:53 AM, Eric Snow <[email protected]> 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. > Sounds good. The less patching the better. 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. > Looks okay in principal, though I'm not sure how much benefit it would add to the existing, tailored fakes in the codebase. It seems a little bit weird that error results are encapsulated, but not non-error results. 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 > [email protected] > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/juju-dev >
-- Juju-dev mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
