How To Write Unit Tests

Boilerplate

package_test.go

// Copyright 2016 Canonical Ltd.
// Licensed under the AGPLv3, see LICENCE file for details.

package whatever_test

import (
    "testing"

    gc "gopkg.in/check.v1"
)

func TestPackage(t *testing.T) {
    gc.TestingT(t)
}

Always start with the above. It contains the single Test func that's exercised by the golang test infrastructure; its only job is to immediately hand control onto the check framework, which we import as gc by convention because we use it a lot.

Every package should have one of these, and it should almost always look exactly like the above, substituting only the string "whatever". Exceptions will be covered later.

something_test.go

// Copyright 2016 Canonical Ltd.
// Licensed under the AGPLv3, see LICENCE file for details.

package whatever_test

import (
    "github.com/juju/testing"
    gc "gopkg.in/check.v1"
)

type SomethingSuite struct {
    testing.IsolationSuite
}

var _ = gc.Suite(&SomethingSuite{})

func (*SomethingSuite) TestFatal(c *gc.C) {
    c.Fatalf("not done")
}

Before you continue, go test and check you see the failure you expect; then let's unpack what's going on here.

package whatever_test

Note that the test is defined outside the whatever package; this helps to keep us honest re: testing behaviour not implementation, and focusing our efforts on the externally observable features of the SUT (which are, after all, the only features anyone else cares about).

type SomethingSuite struct {
    testing.IsolationSuite
}

This bit just defines a type; nothing about the name is special in any way, but it's nice to export the parts that you intend to register with gc just so that humans can benefit from the visual clue.

It embeds IsolationSuite, which does the bare minimum context creation for your tests: it clears os.Env to avoid accidental dependencies; it sets up log capture in the tests; and it exposes an AddCleanup func which you probably shouldn't use because it's taking advantage of the suite type's statefulness and we shouldn't really be doing that. More on this later.

var _ = gc.Suite(&SomethingSuite{})

This line is the magic: it creates a test suite and hands it over to gc. Note that we're passing a pointer: even if stateful test suites aren't ideal, (1) we need it for IsolationSuite and (2) suite values only run the test cases with value receivers, potentially silently skipping a lot of tests. (It's happened before.)

Please always put this invocation immediately after the suite definition, rather than tucking them away in a separate file, or grouping them in a var block: it's easy to mess this up, and having a clear define-then-register pattern helps make correctness obvious -- suite definitions or registrations on their own are definitely wrong, and registration-before-definition is just needlessly backwards.

func (*SomethingSuite) TestFatal(c *gc.C) {
    c.Fatalf("not done")
}

Finally, just about everything else on the suite should look like this.

• exported name, starting with Test and accepting a *gc.C, is what causes it to be run by gc (but only because we called gc.Suite to register it, remember)
• pointer receiver, just always use pointers to suites for the reasons discussed above
• anonymous receiver, clear statement that it won't be depending on any context not initialised directly in this method (I wave my hands re: inconsistency with IsolationSuite; would observe that they're at least aligned in purpose, and that practicality still beats purity)
• trivial failing implementation so we get a spot of verification that the tests really are running

To emphasise the "just about everything else": the suite embeds IsolationSuite, but has no other fields; and does not implement SetUpSuite, SetUpTest, TearDownTest or TearDownSuite. Those methods, exposed because they're defined on the embed, will be called at appropriate times by the gc framework, and will manage the features described above; but direct use of those methods in your own code encourages an uncomfortable conflation of a test suite (a set of related test cases) with a test fixture (a baseline state against which you can run tests repeatably).

Non-boilerplate

Personally, I like to keep a failing "not done" test around for the duration of my work, and delete it only when I can't think of anything else to test; and to liberally add similar failing test cases, differing only by name, whenever I do think of something else I should test. That's actually often a good start: you'll have some idea of the properties you want, so drop them in as aides-memoire...

func (*SomethingSuite) TestOneThing(c *gc.C) {
    c.Fatalf("not done")
}

func (*SomethingSuite) TestAnotherThing(c *gc.C) {
    c.Fatalf("not done")
}

func (*SomethingSuite) TestYetAThirdThing(c *gc.C) {
    c.Fatalf("not done")
}

However, it's clear at this point that the pedagogical value of toy examples is negligible, and we need to look at some actual scenarios you might want to test, and how you'd go about them.

Testing Validation

Data structure validation is nice and simple, and commensurately easy to test, but nonetheless instructive. We'll go with a non-trivial config struct, let's say for driving some sort of worker:

type Facade interface {
    // some methods that talk to the api server
    // not important right now
}

// Config holds configuration and dependencies for Worker.
type Config struct {
    Facade   Facade
    Identity string
    Clock    clock.Clock
    MinDelay time.Duration
    MaxDelay time.Duration
}

// Validate returns an error if the Config cannot drive a Worker.
func (config Config) Validate() error {
    if config.Facade == nil {
        return errors.NotValidf("nil Facade")
    }
    if config.Identity == "" {
        return errors.NotValidf("empty Identity")
    }
    if config.Clock == nil {
        return errors.NotValidf("nil Clock")
    }
    if config.MinDelay <= 0 {
        return errors.NotValidf("non-positive MinDelay")
    }
    if config.MaxDelay <= 0 {
        return errors.NotValidf("non-positive MaxDelay")
    }
    if config.MinDelay < config.MaxDelay {
        return errors.NotValidf("MinDelay greater than MaxDelay")
    }
    return nil
}

...which is nice and clean and clear and correct by inspection, right? Do we even really need a test?

...yes, we do, because there's a bug in that code. You probably spotted it because you're awesome, of course, but still: it's super easy to make mistakes in even trivial logic, and it's absolutely worth checking it works.

So: what do you do? First of all, you notice that there are a whole bunch of error conditions you'll want to validate, so you'll probably want a boilerplate suite just for the config:

type ConfigSuite struct {
    testing.IsolationSuite
}

var _ = gc.Suite(&ConfigSuite{})

...and there's a nice obvious first test:

func (*ConfigSuite) TestValid(c *gc.C) {
    config := sut.Config{
        Facade:   struct{ sut.Facade }{},
        Identity: "some-id",
        Clock:    struct{ clock.Clock }{},
        MinDelay: time.Second,
        MaxDelay: time.Minute,
    }

    err := config.Validate()
    c.Check(err, jc.ErrorIsNil)
}

...which sets up the absolute minimal valid config it possibly can. The anonymous structs that satsify Facade and Clock, in particular, are taking advantage of the hidden-nil trap, such that the test will fail violently if Validate oversteps its bounds and actually tries to do anything with the values, which would certainly be inappropriate.

(Panicking is inelegant, but as long as we do it on the main goroutine it's not the end of the world, and it's not entirely unreasonable when our assumptions (Validate doesn't do anything) are broken. You could make an academic case for creating doubles with semi-functional method implementations that just fail the test, but the cost/benefit is way off.)

You could write all the other tests like this too:

func (*ConfigSuite) TestMissingFacade(c *gc.C) {
    config := sut.Config{
        Facade:   nil,
        Identity: "some-id",
        Clock:    struct{ clock.Clock }{},
        MinDelay: time.Second,
        MaxDelay: time.Minute,
    }

    err := config.Validate()
    c.Check(err, jc.Satisfies, errors.IsNotValid)
    c.Check(err, gc.ErrorMatches, "nil Facade not valid")
}

...but that rapidly becomes boring: we need some sort of test fixture. In particular, we need to reuse that minimal-valid config; and it will rapidly become apparent that we're duplicating a lot of the error checking, so we end up extracting two pieces:

func minimalConfig() sut.Config {
    return sut.Config{
        Facade:   struct{ sut.Facade }{},
        Identity: "some-id",
        Clock:    struct{ clock.Clock }{},
        MinDelay: time.Second,
        MaxDelay: time.Minute,
    }
}

func checkInvalid(c *gc.C, config sut.Config, match string) {
    err := config.Validate()
    c.Check(err, jc.Satisfies, errors.IsNotValid)
    c.Check(err, gc.ErrorMatches, match)
}

...which then get used as follows:

func (*ConfigSuite) TestValid(c *gc.C) {
    config := minimalConfig()

    err := config.Validate()
    c.Check(err, jc.ErrorIsNil)
}

func (*ConfigSuite) TestMissingFacade(c *gc.C) {
    config := minimalConfig()
    config.Facade = nil

    checkInvalid(c, config, "nil Facade not valid")
}

// ...

func (*ConfigSuite) TestMismatchedDelay(c *gc.C) {
    config := minimalConfig()
    config.MinDelay = time.Minute
    config.MaxDelay = time.Second

    checkInvalid(c, config, "MinDelay greater than MaxDelay not valid")
}

...and are super-readable and -updatable and so on. They might even seem too simple, but again, they're not without value: they really do catch bugs, IME, and they're not hard to write: you should be able to bash those out in a few minutes.

(Local forces may push you to make the config-test fixture more formal: both funcs on a type, somehow, perhaps? but it's honestly so lightweight that I don't feel particularly inclined to bother. It's good to know that it is a fixture, but there's no need to make a big deal out of it. We'll see more interesting fixtures later, anyway.)

Testing Workers

A worker should always be nicely self-contained, so it's quite a good place to start; and it'll involve concurrency, which will bite us if we screw up, and help us develop good habits.

In fact, we'll look at several workers of increasing complexity. For reference, they'll all have the same basic structure, differing only in the existence of the occasional runtime field (generally one or more channels; sometimes a map or something; and preferably nothing more, because anything that demands any of its own setup or configuration should be supplied from outside in proper IoC style, rather than adding unnecessary responsibilities to this type).

func New(config Config) (*Worker, error) {
    if err := config.Validate(); err != nil {
        return nil, errors.Trace(err)
    }
    worker := &Worker{
        config: config,
        // runtime state fields?
    }
    err := catacomb.Invoke(catacomb.Plan{
        Site: &worker.catacomb,
        Work: worker.loop,
    })
    if err != nil {
        return nil, error.Trace(err)
    }
    return worker, nil
}

type Worker struct {
    catacomb catacomb.Catacomb
    config   Config
    // runtime state fields?
}

// Kill is part of the worker.Worker interface.
func (w *Worker) Kill() {
    w.catacomb.Kill(nil)
}

// Wait is part of the worker.Worker interface.
func (w *Worker) Wait() error {
    return w.catacomb.Wait()
}

func (w *Worker) loop() error {
    // implementation-specific...

The workers we consider will otherwise vary only in their config structs; we'll assume they've been done reasonably sanely, and move on; perhaps pausing only to tweak the aforementioned checkInvalid:

func checkInvalid(c *gc.C, config sut.Config, match string) {
    check := func(err error) {
        c.Check(err, jc.Satisfies, errors.IsNotValid)
        c.Check(err, gc.ErrorMatches, match)
    }
    
    err := config.Validate(err)
    check(err)

    worker, err := sut.New(config)
    check(err)
    if !c.Check(worker, gc.IsNil) {
         workertest.CheckKill(worker)
    }
}

...so that we always know the constructor validates the config properly and can just forget that concern in the upcoming tests.

Time-based workers

Consider the following worker, which exists to Ping once on creation, and subsequently at regular intervals:

type Facade interface {
    Ping() error
}

type Config struct {
    Facade Facade
    Clock  clock.Clock
    Period time.Duration
}

// ...

func (w *Worker) loop() error {
    var delay time.Duration
    for {
        select {
        case <-w.catacomb.Dying():
            return w.catacomb.ErrDying()
        case <-w.config.Clock.After(delay):
            if err := w.config.Facade.Ping(); err != nil {
                return errors.Annotate(err, "ping failed")
            }
        }
        delay = w.config.Period
    }
}

...and then consider everything that might go wrong with it. Not much, one might think?

Sadly, there are a few things that can rarely go wrong (e.g. if the implementation gets messed up) -- like your worker refusing to die -- that are so irritating (test deadlocks for 10 mins before go test gives up on you) that you basically always need to address them early. You can quite reasonably write a single test for that behaviour alone:

func (*WorkerTest) TestKill(c *gc.C) {
    config := // hmm. we'll come back to this

    worker, err := sut.New(config)
    c.Assert(err, jc.ErrorIsNil)
    workertest.CleanKill(c, worker)
}

...but you'll also want the same verification in every test you run:

func (*WorkerTest) TestSomethingFancy(c *gc.C) {
    config := // soon, I promise

    worker, err := sut.New(config)
    c.Assert(err, jc.ErrorIsNil)
    defer workertest.CleanKill(c, worker)

    // more testing here...
}

...and, seriously, you need a harness for these tests. This is the point at which you start a fixture_test.go or util_test.go or mock_test.go or some helpful variant, containing a type designed to get all the muck out of the way so you can focus on what's actually happening.

type Fixture struct {
    errs []error
}

func NewFixture(errs ...error) Fixture {
    return fixture{errs}
}

type FixtureTest func(*sut.Worker, clock *testing.Clock)

func (fix Fixture) Run(c *gc.C, test FixtureTest) *testing.Stub {
    stub := &testing.Stub{}
    stub.SetErrors(fix.errs...)
    clock := testing.NewClock(time.Now())

    config := sut.Config{ // here it is
        Facade: newMockFacade(stub),
        Clock:  clock,
        Period: time.Minute,
    }

    worker, err := sut.New(config)
    c.Assert(err, jc.ErrorIsNil)
    defer workertest.CheckKill(c, worker)

    test(worker, clock)
    return stub
}

...and the mock can just look like this:

func newMockFacade(stub *testing.Stub) *mockFacade {
    return &mockFacade{stub}
}

type mockFacade struct {
    stub *testing.Stub
}

func (mock *mockFacade) Ping() error {
    mock.stub.AddCall("Ping")
    return mock.stub.NextErr()
}

...letting you write tests that look like this:

func (s *WorkerSuite) TestPingError(c *gc.C) {
    fix := NewFixture(errors.New("ouch!"))
    stub := fix.Run(c, func(worker *sut.Worker, _ *testing.Clock) {
        err := workertest.CheckKilled(c, worker)
        c.Check(err, gc.ErrorMatches, "ping failed: ouch!")
    })
    stub.CheckCallNames(c, "Ping")
}

...and that may not seem that impressive, but it's all the scaffolding you need for a wide variety of tests of the worker's behaviour. When the SUT gets complex, there's space to grow both the fixture (with new fields to specialize collaborator behaviour) and the FixtureTest (e.g. by accepting a suitable Context instead of *testing.Clock (which would then be accessible via the context).

In particular, you can test the timing behaviour in detail.

func (s *WorkerSuite) TestWaitsForSecondPing(c *gc.C) {
    fix := NewFixture()
    stub := fix.Run(c, func(worker *sut.Worker, clock *testing.Clock) {
        waitAlarms(c, clock, 2)
        clock.Advance(time.Minute - time.Nanosecond)
        workertest.CheckAlive(c, worker)
    })
    stub.CheckCallNames(c, "Ping")
}

func (s *WorkerSuite) TestFiresSecondPing(c *gc.C) {
    fix := NewFixture()
    stub := fix.Run(c, func(worker *sut.Worker, clock *testing.Clock) {
        waitAlarms(c, clock, 2)
        clock.Advance(time.Minute)
        workertest.CheckAlive(c, worker)
    })
    stub.CheckCallNames(c, "Ping", "Ping")
}

Both the above use a func like this:

// waitAlarms is used to synchronise a whitebox test that understands
// the SUT's expected timing behaviour. Every call to After, NewTimer,
// or timer.Reset will cause a value to be sent on the clock's Alarms
// channel; you can often usefully use it to count loop iterations.
//
// This func has already been written several times, someone should
// move it alongside testing.Clock.
func waitAlarms(c *gc.C, clock *testing.Clock, count int) {
    timeout := time.After(coretesting.LongWait)
    for i := 0; i < count; i++ {
        select {
        case <-clock.Alarms():
        case <-timeout:
            c.Fatalf("expected %d alarms set, only saw %d before timeout", count, i)
        }
    }
}

...and the exact same techniques remain applicable across timing-dependent workers.

Watch-based workers

Consider the following worker:

type Facade interface {
    Watch() (watcher.NotifyWatcher, error)
    Get() (int, error)
}

type Substrate interface {
    Store(int) error
}

type Config struct {
    Facade    Facade
    Substrate Substrate
}

// ...

func (w *Worker) loop() error {
    watch, err := w.config.Facade.Watch()
    if err != nil {
        return errors.Annotate(err, "facade Watch failed")
    }
    if err := w.catacomb.Add(watch); err != nil {
        return errors.Trace(err)
    }

    for {
        select {
        case <-w.catacomb.Dying():
            return w.catacomb.ErrDying()
        case _, ok := <-watch.Changes:
	if !ok {
                return errors.New("watcher closed channel")
            }
            value, err := w.config.Facade.Get()
            if err != nil {
                return errors.Annotate(err, "facade Get failed")
            }
            if err := w.config.Substrate.Store(value); err != nil {
                return errors.Annotate(err, "substrate Store failed")
            }
        }
    }
}

It's a bit more complicated than the previous one: it watches for changes in a value, and records them somewhere. It's still only interacting with two external components, but the interactions are potentially more complex. And, most annoyingly, we have a watcher to deal with.

As before, there are a bunch of common mistakes we don't want to make. We might not have a clear idea of everything we need but we can make a good start on a fixture without knowing anything other than the config:

func (fix Fixture) Run(c *gc.C, test FixtureTest) *testing.Stub {
    stub := &testing.Stub{}
    stub.SetErrors(fix.errs...)

    config := sut.Config{
        Facade:    newMockFacade(stub),
        Substrate: newMockSubstrate(stub),
    }

    worker, err := sut.New(config)
    c.Assert(err, jc.ErrorIsNil)
    defer workertest.CheckKill(c, worker)

    test(worker)
    return stub
}

...which of course pushes you into implementing your mock types. The substrate is very easy:

func newMockSubstrate(stub *testing.Stub) *mockSubstrate {
    return &mockSubstrate{stub: stub}
}

type mockSubstrate struct {
    stub *testing.Stub
}

func (mock *mockSubstrate) Store(value int) error {
    mock.stub.AddCall("Store", value)
    return mock.stub.NextErr()
}

...but the facade -- which creates a watcher -- is a little bit tricker. An easy and common mistake is to dive straight into stubbing out a watcher (or, indeed, any sort of worker):

func (mock *mockWatcher) Kill() {        // BAD WRONG CODE SAMPLE
    mock.stub.AddCall("Kill")
}

func (mock *mockWatcher) Wait() error {  // BAD WRONG CODE SAMPLE
    mock.stub.AddCall("Wait")
    return mock.stub.NextErr()
}

...but all this will do is mess up your tests: observe, simply, that there's no way to control how long the watcher will Wait for. In short, you're not actually implementing a Worker, you're just implementing something with the same methods; and at great risk of doubling down, with kill channels and error injections and all manner of incomprehensible yuck.

So: you need a real worker. Happily, because the watcher interfaces don't muck up their event streams with lifetime concerns, it's actually really easy to create a canned watcher with correct lifetime behaviour; configurable error behaviour; and canned behaviour that's quite good enough to exercise the SUT:

func newMockWatcher(stub *testing.Stub) *mockWatcher {
    const count = 3
    changes := make(chan struct{}, count)
    for i := 0; i < count; i++ {
        changes <- struct{}{}
    }
    waitErr := stub.NextErr()
    return &mockWatcher{
        Worker:  workertest.NewErrorWorker(waitErr),
        changes: changes,
    }
}

type mockWatcher struct {
    worker.Worker
    changes <-chan struct{}
}

func (mock *mockWatcher) Changes() <-chan struct{} {
    return mock.changes
}

This can be invoked quite easily by a simple facade implementation, which can again work quite happily with canned data:

func newMockFacade(stub *testing.Stub) *mockFacade {
    return &mockFacade{
        stub: stub,
        gets: []int{123, 456, 789},
    }
}

type mockFacade struct {
    stub *testing.Stub
    gets []int
}

func (mock *mockFacade) Get() (int, error) {
    mock.stub.AddCall("Get")
    if err := mock.stub.NextErr(); err != nil {
        return -99, err
    }
    next := mock.gets[0]
    mock.gets = mock.gets[1:]
    return next, nil
}

func (mock *mockFacade) Watch() (watcher.NotifyWatcher, error) {
    mock.stub.AddCall("Watch")
    if err := mock.stub.NextErr(); err != nil {
        return nil, err
    }
    return newMockWatcher(mock.stub), nil
}

...and at this point we should stop and consider some of the obvious deficiencies in the above code.

It's Not Configurable At All!

Right; it's only as configurable as I think it needs to be right now. It can and will surely evolve, but it's hard to know in advance exactly how that will happen. Often it's neater to drop the Fixture constructor, and create a bunch of fields that define different features of the SUT's environment and interactions; the only reason I don't go there to begin with is because, often, canned data is quite good enough: the ability to control the error stream gives you enough power to exercise an awful lot of behaviour.

It'll Panic On The Wrong Goroutine!

...dammit, you're right. We should be checking len(mock.gets) before accessing it, and -- I suppose -- returning some suitably distinctive error. (Or, plausibly, by making a *gc.C somehow accessible to the *mockFacade -- there's something ugly about that, but it seems superior by all objective measures.)

Don't You Already Know You Should Test Closed Channels?

Honestly, I usually can't be bothered. We never really did this ever, when it was much riskier: now that we've separated lifetime concerns from change events, we only have the:

if !ok {
    return errors.New("watcher closed channel")
}

...handling to worry about, and that's arguably simple enough -- despite its presence in a more complex context -- that we can skip the tests so long as we're mindful about the code.

If decided you needed to, though, it's easily addressed via fixture config.

A Bad SUT Could Leak Workers!

Yeah, it could. This is a judgment call akin to the closed-channel case: when the lifetime-handling code is simple and standard enough, I often convince myself it doesn't need further testing. When the situation is any hairier, it's worth tracking the workers with a bit more care. That demands a bit more infrastructure, though, and it shouldn't be written until it's needed.

You Can't Kill The Watcher From Outside!

Same situation. Trivial lifetime-handling code probably doesn't demand close attention; as it gets more complex, there's more reason to pay the infrastructure cost to examine the interactions in detail.

Please Can We See Some Tests?

Yeah; I think that digression was worthwhile, but let's see what we can do with the infrastructure we've got. The full happy case, in particular, is very easy indeed, because we already know the canned data. (You could make a strong case that the expected happy-path calls should be accessible via a Fixture method. Try it out, see if it works well -- the counterargument is that the explicitness is too good to lose, even if it does weigh down this test more than we'd like.)

func (*WorkerTest) TestRuns(c *gc.C) {
    fix := newFixture()
    stub := fix.Run(c, func(worker *sut.Worker) {
        workertest.CheckAlive(c, worker)
        workertest.CleanKill(c, worker)
    })
    stub.CheckCalls(c, []testing.StubCall{{
        FuncName: "Watch",
    }, {
        FuncName: "Get",
    }, {
        FuncName: "Store",
        Args:     []interface{}{123},
    }, {
        FuncName: "Get",
    }, {
        FuncName: "Store",
        Args:     []interface{}{456},
    }, {
        FuncName: "Get",
    }, {
        FuncName: "Store",
        Args:     []interface{}{789},
    }})
}

...and when we need to worry about errors, we just need to remember a Watch call consumes two (one for the result, one for when the watcher's Killed).

func (*WorkerTest) TestWatchError(c *gc.C) {
    fix := newFixture(errors.New("zap"))
    stub := fix.Run(c, func(worker *sut.Worker) {
        err := workertest.CheckKilled(c, worker)
        c.Check(err, gc.ErrorMatches, "facade Watch failed: zap")
    })
    stub.CheckCallNames(c, "Watch")
}

func (*WorkerTest) TestWatcherError(c *gc.C) {
    fix := newFixture(nil, errors.New("arrgh"))
    stub := fix.Run(c, func(worker *sut.Worker) {
        workertest.CheckAlive(c, worker)
        err := workertest.CheckKill(c, worker)
        c.Check(err, gc.ErrorMatches, "arrgh")
    })
    stub.CheckCallNames(c, "Watch", "Get", "Store", "Get", "Store", "Get", "Store")
}

...and then you can test Get and Store errors quite happily:

func (*WorkerTest) TestGetError(c *gc.C) {
    fix := newFixture(nil, nil, errors.New("pow"))
    stub := fix.Run(c, func(worker *sut.Worker) {
        err := workertest.CheckKilled(c, worker)
        c.Check(err, gc.ErrorMatches, "facade Get failed: pow")
    })
    stub.CheckCallNames(c, "Watch", "Get")
}

func (*WorkerTest) TestStoreError(c *gc.C) {
    fix := newFixture(nil, nil, nil, errors.New("bof"))
    stub := fix.Run(c, func(worker *sut.Worker) {
        err := workertest.CheckKilled(c, worker)
        c.Check(err, gc.ErrorMatches, "substrate Store failed: bof")
    })
    stub.CheckCallNames(c, "Watch", "Get", "Store")
}

...and, on reflection, those 5 tests already cover the main things we need to worry about, assuming we've got proper config validation tests elsewhere as described above.

...Is This Really Proportionate?

We've spent, what, 100 lines on test infrastructure to write 60ish lines of tests, to validate 80ish lines of implementation. Is it worth it?

In my opinion: yes! Absolutely! The techniques described above are literally no more or less than good coding practices, applied to the domain of testing this specific component. It is way too common for people to implicitly or explicitly relegate testing to a second-class citizen; avoiding obvious refactorings, allowing promiscuous repetition, embracing globals, summoning betentacled monstrosities from beyond the beyond, &c, all justified by a "well it's just test code".

Tests are important. That's why we write them. Of course test quality is important; it might even be more important than code quality. (Good tests can give you some confidence in the operation of bad code; you can check what it does, even if you don't understand why. But good code supported by bad tests has little to protect it from rot and thoughtless modification, and rarely stays good for long.)

And the tradeoffs aren't that bad, even in service of this deliberately trivial example. Once you start dealing with specific error values that need special handling, and/or timing considerations, or interactions any more complex than watch/get/store, the benefits of having some consistent management (and built-in deadlock protection, and so on) become overwhelming.