Defaulting tests internal to the package

[William Reade]

On Thu, Jan 21, 2016 at 9:55 PM, Nate Finch <nate.finch at canonical.com>
wrote:

> [reposting this to the wider juju-dev mailing list]
>
> So, I hit an interesting problem a while back.  I have some unit tests
> that need to be internal tests, thus they are in 'package foo'.  However,
> my code needs to use some testhelper functions that someone else wrote...
> which are in 'package foo_test'.  It's impossible to reference those, so I
> had to move those helpers to package foo, which then either requires that
> we make them exported (which is exactly like using export_test.go, which,
> in the juju-core Oakland sprint, we all agreed was bad), or all tests that
> use the helpers need to be in 'package foo'... which means I had to go
> change a bunch of files to be in package foo, and change the calls in those
> files from foo.SomeFunc() to just SomeFunc().
>
> Given the assumption that some tests at some point will make sense to be
> internal tests, and given it's likely that helper functions/types will want
> to be shared across suites - should we not just always make our tests in
> package foo, and avoid this whole mess in the first place?
>

This is a poor justification. The only reason to write internal tests is
when you *really can't* write those tests against the exported interface;
and the only legitimate reason to accept that situation is when you're
dealing with terrible legacy code and *really don't* have the time to
rewrite a whole package properly.

(A note, this happened again today - I wanted to add a unit test of a
> non-exported function to an existing test suite, and can't because the unit
> tests are in the foo_test package)
>

You're doing it wrong. Either export the system you want to test (and
inject it explicitly into clients) or write a fixture for the exported bit
that lets you demonstrate that the system is hooked up. Anything else is
mere cargo-culting and grants neither the verification nor the design
benefits that testing should deliver.

There seems to only be two concrete benefits to putting tests in package
> foo_test:
> 1. It avoids circular dependencies if your test needs to import something
> that imports the package you're testing.
> 2. It makes your tests read like normal usages of your package, i.e.
> calling foo.SomeFunc().
> The first is obviously non-negotiable when it comes up... but I think it's
> actually quite rare (and might indicate a mixture of concerns that warrants
> investigation).  The second is nice to have, but not really essential (if
> we want tests that are good examples, we can write example functions).
>

There are two main reasons to write tests:

1) Tests let you verify how the code you run in production will actually
act.
2) Tests are a second client for the interface you create, and thus apply
early pressure towards good/sane interface design.

Writing package-internal tests subverts both goals, and gives us a *tiny*
bit of short-term convenience in return. It's a terrible trade-off.

So, I propose we put all tests in package foo by default.  For those devs
> that want to only test the exported API of a package, you can still do
> that.  But this prevents problems where helper code can't be shared between
> the two packages without ugliness and/or dumb code churnm, and it means
> anyone can add unit tests for non-exported code without having to create a
> whole new file and testsuite.
>

I do not want anyone to add unit tests for non-exported code, because those
tests are almost completely worthless. It's not a matter of personal
preference; it's a matter of simple correctness. Keep your tests in
separate packages.

Cheers
William



>
> -Nate
>
> --
> 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/20160122/2748d719/attachment.html>