In some places, we have exposed constants that should not normally be
public as public variables, purely for the purpose of testing.  The reason
we need to do that, is because packages can't use code that is only exposed
in _test files for other packages (i.e.  package foo has a export_test.go
that it uses to export wrappers for internal things to be tweaked during
testing.  However, package bar can't access the stuff in foo's
export_test.go, only foo can access that stuff.)

The problem with these public variables is that the only thing preventing
people from using them is a comment that says "this is only for testing,
don't modify during production".

If you're consuming the code and see this public name pop up in code
completion, you might not realize it's only for testing, and use it anyway.

I've actually seen and fixed a bug like this, where production code was
twiddling with one of these variables that it was not supposed to be using.

The fix I thought up was to instead make a function called MockPackage().
 This function does everything that is needed to mock out the package for
testing, and returns a function that undoes the mocking.

The reason I like this better that public variables is twofold:

1.) It's a lot more obvious that MockPackage is something that you
shouldn't be calling in production code... rather than some vaguely named
public variable that may or may not be one of these special "do not touch"
variables.  This is doubly true if MockPackage becomes a standard pattern.

2.) It abstracts away the mocking of this package from consumers of the
package, so that if the package changes and needs to modify how it mocks
out its functionality, you don't have to change any other code.

I used this pattern in a merge proposal, and Roger suggested we just do it
the old way, with a public variable... so I'd like more input from the team.

-Nate
-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev

Reply via email to