consensus on using separate *_test packages

[Gustavo Niemeyer]

On Fri, Sep 12, 2014 at 3:16 PM, Nate Finch <nate.finch at canonical.com> wrote:
> In the thread Eric pointed to, Brad Fitzpatrick (one of the core Go
> developers) says they prefer to keep tests in the same package unless forced
> to have them in a different package to avoid circular dependencies.  I like
> that.

Brad is a great guy, but defending a position because someone says so
is not good reasoning, no matter who that is. I've provided a
different point of view on that same thread, with reasoning, and had
no counterarguments.

> I have always thought that export_test was an anti-pattern that should only
> be used as a last resort.  The main problem I have with export_test is that
> it makes your tests lie.  Your tests call foo.Create, but package foo
> doesn't actually export Create, it has a non-exported create method, that
> happens to get exported in the export_test file.  This makes your tests more
> confusing than if you just called "create" from an internal test.  That's
> even aside from the point of the busywork it creates from having to write
> the file in the first place.

On the counter side, it greatly encourages you to keep your tests
isolated from internal details, and forces you to make it very
explicit when you're using non-public interfaces in your test, via a
well known convention. Both are great real benefits, that easily
outweigh the theoretical "lie" described.

I'm not saying you should never use internal tests, though, but rather
agreeing with people that posted before you on this thread.

> One argument for export_test is that it gives you a canonical place to go
> look for all the internal stuff that is getting used in tests... but I don't
> actually think that's very valuable.  I'm not actually sure why it would be
> important to see what internal functions are getting exported for use during
> tests.

The second part of that same paragraph answers the question with a
counter example:

>  And in theory, if you're writing extensive unit tests, almost all
> the internal methods would get exported there... so it becomes just a huge
> boilerplate file for no good reason.  If you really want to see what
> functions are getting exercised during tests, use code coverage, it's built
> into the go tool.

This clearly describes one of the important reasons for the pattern.
If every internal function is imported in tests, and assuming good
code-writing practices for when to use a new function, it means the
test is extremely tightly bound to the implementation.

> I agree with Gustavo's point that tests can be good examples of how to use a
> package.  And, in fact, Go supports example functions that are run like
> tests, but also get displayed in generated docs.  These example tests can
> exist in the package_test package to make them very accurate representations
> of how to use the package.

That was just one side I mentioned, in a long list of reasons, and in
practice examples are pretty much always written well after the code
is ready. How many public functions do you have in the juju code base?
 How many of them are covered by those examples you mention?  How many
of them are covered in tests?

> Most tests that are written to test the functionality of a package are not
> actually good examples of how to use the package.  Most tests are just
> isolating one part of the logic and testing that.  No one using the package
> for its intended purpose would ever do that.  This is doubly true of unit
> tests, where you may just be testing the input and output of a single
> internal function.

That's very far from being true. My own tests exercise the logic I'm
writing, with the API I designed and documented, and they reflect how
people use that code. I commonly even copy & paste snippets out of my
own tests into mailing lists as examples of API usage. Perhaps you
write some sort of test that I'm not used to.

> I think our current trend of doing most tests in an external package has
> significantly contributed to the poor quality of our tests.  Because we're
> running from outside the package, we generally only have access to the
> exported API of the package, so we try to "make it work" by mocking out
> large portions of the code in order to be able to call the external
> function, rather than writing real unit tests that just test one small
> portion of the internal functionality of the package.

This insight fails to account for the fact that in practice "exported
API" is precisely about the API that was made public in one particular
isolated abstraction. Several of these "public APIs" are in fact very
internal details to the implementation of juju, but that make sense in
isolation. When you're testing these via their public API, it means
you're actually exercising and focusing on the promises that were made
to the outside implementation of juju itself, which are the most
valuable guarantees to encode into test form. Developers should be
free to refactor the implementation as necessary to improve clarity,
performance, memory consumption, etc, as long as they keep those
specific guarantees.

Now, again, I'm not saying purely internal tests are not useful. I
have those myself in a few cases. I'm just saying this is a damn good
thing to have as a baseline.


gustavo @ http://niemeyer.net