Defaulting tests internal to the package

[roger peppe]

On 22 January 2016 at 09:40, William Reade <william.reade at canonical.com> wrote:
> I do not want anyone to add unit tests for non-exported code, because those
> tests are almost completely worthless.

I'd beg to disagree with this. I think it very much depends what you are
testing. For me tests are about giving confidence that the code works.
Sometimes as part of the implementation of a package I'll write a
function with a well defined contract that doesn't justify being made
public because it's intimately related to the way that the package is
implemented. The function might be reasonably tricky, and it may be hard
to reach all its corner cases at arm's length through the public API.
It might not even be possible, but that doesn't mean that it's not
useful to test the function, as it may provide a foundation for more
varied future functionality.

In this kind of scenario, I think it makes good sense to write a unit test
for the unexported function. If you change the implementation, you might
throw away the function and its tests, and that's fine, but this approach,
in my experience, can give good confidence in some tricky situations with
significantly less effort than doing everything through the public API.

As always, there's a trade-off here - we want to maximise confidence
while minimising time and effort spent writing and maintaining the
code. It's always going to be a judgement call and flat assertions like
"those tests are almost completely worthless" are not that helpful IMHO.

I do agree that writing external tests *when reasonable* is the way,
and I think that export_test.go is a reasonable escape hatch for the
times when it's useful to write internal tests.

Nate, you seem to be arguing that because we can't have total separation
between external and internal APIs, then we should not have any separation
at all. I don't agree - I think that keeping as much as possible to
the external API, but having a few well-defined exported objects (in
export_test.go) works well, and reflects the nuanced situation that we
actually live in.