Defaulting tests internal to the package

[William Reade]

On Fri, Jan 22, 2016 at 5:17 PM, Nate Finch <nate.finch at canonical.com>
wrote:

> I'm glad to hear Roger's opinion about testing internal code... that's
> exactly how I feel.  True unit tests of small bits of code are easy to
> write, easy to read and understand, and give you confidence that your code
> is doing what you think it'll do in a wide variety of cases.  If the unit
> test fails, it's generally incredibly clear where the fault in the code
> lies, and it's easy to fix either the code or the test.  In 6 months, you
> or someone else can go back to the tests, easily understand what they are
> testing, and modify them trivially when making a bug fix or improvement.
>

Tests for unexported utility functions can give you great confidence in the
operation of those unexported utility functions... but those tests will
continue to pass even if you change the package so that they're never
actually used, and that renders those tests' diagnostic power pretty much
worthless. And, to make things worse, that sort of test is very frequently
used as a justification for skipping tests of the same functionality
against the exported interface.


> While you certainly *can* write code that tests all the corner cases of
> some small utility function through the external API... those tests will
> almost always be incredibly opaque and hard to understand, and generally
> much more complicated than they would be if you could just test the utility
> function by itself.  This is often a problem I have with our current
> tests... it's hard to see what is actually being tested, and even harder to
> verify that the test itself is correct and complete.  When a test fails,
> you often have no clue where the actual problem lies, because the test
> traverses so much code.
>

I think you're misdiagnosing the problems with our current tests. Most of
our rubbish "unit" tests are rubbish because they're not unit tests at all:
they're full-stack tests that are utterly and inextricably bound up with
the implementation details of *all* the layers underneath, and depend
critically upon assumptions about how those other layers work. And, yes,
one of the worst things about those tests is how hard it is to diagnose
what's actually happened when they fail.

But that's really *not* a good reason to add *more* implicit assumptions
about how various components are connected together, which is exactly what
you're doing when you write tests for unexported implementation details. If
your utility function tests fail, but the package tests don't, all it means
is that you've written bad package tests; or, if your package tests *are*
complete and correct, an isolated utility-test failure just means you've
wasted time writing tests for functionality that delivers no observable
value.

Of course, I definitely think you *also* need tests of the exported API of
> a package... but small unit tests are still incredibly valuable.
>

"Small unit tests" are tremendously valuable, I agree. And if your tests
for the exported API are *not* small unit tests, you're Doing It Wrong. But
unit tests for unexported functions are *by definition* tests for
implementation details, not for behaviour, and as such have *negative*
value; and not just because they fossilise the code, or even just because
they give you false confidence, but because they remove any pressure to
export a good interface.

To put it another way: if it's hard to write tests to exercise X internal
functionality, either that functionality is not important... or, worse, it
*is* important but your interface design is making it hard to observe.
That's not a reason to add internal tests: it's a reason to *fix your
interface*. (And *usually* in a juju context that just means "stop screwing
around with globals and make your dependencies explicit". Turns out that
inversion of control is actually a pretty good idea...)

(Maybe it would help if we had a concrete example? What were the tests that
you felt you needed to pull in-package? It may be that you were originally
talking about a hairy-enough situation that it's worth relaxing the
principles in play here -- even bad tests are often better than *no* tests
-- but you were also advocating that *all* tests should be in-package, and
I really don't think that's a justifiable default.)

Cheers
William
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/juju-dev/attachments/20160122/c2d63ffc/attachment-0001.html>