From c2a07ce69069d5ceae26c985cbc88767a6d0bb3c Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Mon, 21 Sep 2026 10:05:38 +0200 Subject: [PATCH] test: add tests for globals, healthcheck and logger (closes #110) Tests for the three packages that had none, written from outside each package, each able to fail on a plausible break: - globals: values set are read back through New, and New returns an independent copy. One sequential test function with a disclosed paralleltest suppression, because it changes shared package variables. - healthcheck: Check returns status "ok", the documented JSON fields, an RFC3339Nano timestamp, the maintenance flag from config in both states, and version and appname from globals. - logger: New gives a usable *slog.Logger, debug output is off by default and EnableDebugLogging turns it on. No production code changed. The terminal output format is not asserted. Model: opus-4-8 --- TODO.md | 2 + internal/globals/globals_test.go | 50 +++++++++ internal/healthcheck/healthcheck_test.go | 134 +++++++++++++++++++++++ internal/logger/logger_test.go | 66 +++++++++++ 4 files changed, 252 insertions(+) create mode 100644 internal/globals/globals_test.go create mode 100644 internal/healthcheck/healthcheck_test.go create mode 100644 internal/logger/logger_test.go diff --git a/TODO.md b/TODO.md index b00f161..18cd4e6 100644 --- a/TODO.md +++ b/TODO.md @@ -23,6 +23,8 @@ Rationale, Design, TODO, License, Author) if any are still missing. # Completed Steps +- 2026-09-21: added behavioural tests for `internal/globals`, + `internal/healthcheck`, and `internal/logger` (closes #110). - 2026-09-21: `go mod tidy` dropped the redundant `golang.org/x/sync` `// indirect` line so `script/bootstrap` leaves a clean tree (#132) - 2026-08-10: comment-only corrections to `script/bootstrap`, diff --git a/internal/globals/globals_test.go b/internal/globals/globals_test.go new file mode 100644 index 0000000..f44174e --- /dev/null +++ b/internal/globals/globals_test.go @@ -0,0 +1,50 @@ +package globals_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "sneak.berlin/go/dnswatcher/internal/globals" +) + +// TestGlobals exercises the package-level version and appname +// variables through their setters and read-back via New. These are +// shared package state, so the test mutates a global and must run +// sequentially; it cannot use t.Parallel(). +// +//nolint:paralleltest // mutates shared package-level globals, must run sequentially +func TestGlobals(t *testing.T) { + versions := []string{"v1.2.3", "dev", "", "v1.2.3-4-gabcdef"} + for _, want := range versions { + globals.SetVersion(want) + + g, err := globals.New(nil) + require.NoError(t, err) + assert.Equal(t, want, g.Version, + "New must surface the version set by SetVersion") + } + + names := []string{"dnswatcher", "other", ""} + for _, want := range names { + globals.SetAppname(want) + + g, err := globals.New(nil) + require.NoError(t, err) + assert.Equal(t, want, g.Appname, + "New must surface the appname set by SetAppname") + } + + // New returns a snapshot: a later SetVersion must not mutate a + // Globals handed out earlier. + globals.SetVersion("first") + + g, err := globals.New(nil) + require.NoError(t, err) + + globals.SetVersion("second") + assert.Equal(t, "first", g.Version, + "a Globals returned by New must not change when the "+ + "package variable is set again") +} diff --git a/internal/healthcheck/healthcheck_test.go b/internal/healthcheck/healthcheck_test.go new file mode 100644 index 0000000..b87c3d1 --- /dev/null +++ b/internal/healthcheck/healthcheck_test.go @@ -0,0 +1,134 @@ +package healthcheck_test + +import ( + "context" + "encoding/json" + "testing" + "time" + + "go.uber.org/fx" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "sneak.berlin/go/dnswatcher/internal/config" + "sneak.berlin/go/dnswatcher/internal/globals" + "sneak.berlin/go/dnswatcher/internal/healthcheck" + "sneak.berlin/go/dnswatcher/internal/logger" +) + +// recordingLifecycle is a minimal fx.Lifecycle that records the hooks +// appended to it, so healthcheck.New can be exercised through its real +// constructor without standing up a whole fx application. +type recordingLifecycle struct { + hooks []fx.Hook +} + +func (l *recordingLifecycle) Append(hook fx.Hook) { + l.hooks = append(l.hooks, hook) +} + +// newHealthcheck builds a Healthcheck through the real constructor and +// runs the registered OnStart hook so StartupTime is set the same way +// the fx lifecycle would set it. +func newHealthcheck( + t *testing.T, + maintenance bool, + version string, +) *healthcheck.Healthcheck { + t.Helper() + + g := &globals.Globals{Appname: "dnswatcher", Version: version} + + log, err := logger.New(nil, logger.Params{Globals: g}) + require.NoError(t, err) + + lifecycle := &recordingLifecycle{} + + hc, err := healthcheck.New(lifecycle, healthcheck.Params{ + Globals: g, + Config: &config.Config{MaintenanceMode: maintenance}, + Logger: log, + }) + require.NoError(t, err) + + require.Len(t, lifecycle.hooks, 1, + "New must register exactly one lifecycle hook") + require.NotNil(t, lifecycle.hooks[0].OnStart) + require.NoError(t, lifecycle.hooks[0].OnStart(context.Background())) + + return hc +} + +func TestCheckStatusAndPayloadShape(t *testing.T) { + t.Parallel() + + hc := newHealthcheck(t, false, "v9.9.9") + resp := hc.Check() + + assert.Equal(t, "ok", resp.Status) + + // The JSON shape and field names are part of the contract for the + // /health and /.well-known/healthcheck routes, so assert on the + // exact set of keys the response marshals to. + raw, err := json.Marshal(resp) + require.NoError(t, err) + + var fields map[string]json.RawMessage + require.NoError(t, json.Unmarshal(raw, &fields)) + + wantKeys := []string{ + "status", + "now", + "uptimeSeconds", + "uptimeHuman", + "version", + "appname", + "maintenanceMode", + } + assert.Len(t, fields, len(wantKeys), + "response must marshal to exactly the documented fields") + + for _, key := range wantKeys { + assert.Contains(t, fields, key, "missing JSON field %q", key) + } + + // The Now field is documented as RFC3339Nano; a change to the + // format constant should turn this red. + _, err = time.Parse(time.RFC3339Nano, resp.Now) + assert.NoError(t, err, "Now must be RFC3339Nano") +} + +func TestCheckMaintenanceModeReflectsConfig(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + maintenance bool + }{ + {"maintenance off", false}, + {"maintenance on", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + hc := newHealthcheck(t, tt.maintenance, "test") + resp := hc.Check() + assert.Equal(t, tt.maintenance, resp.Maintenance, + "maintenanceMode must mirror Config.MaintenanceMode") + }) + } +} + +func TestCheckSurfacesVersionAndAppname(t *testing.T) { + t.Parallel() + + hc := newHealthcheck(t, false, "surfaced-version-123") + resp := hc.Check() + + assert.Equal(t, "surfaced-version-123", resp.Version, + "version from globals must appear in the payload") + assert.Equal(t, "dnswatcher", resp.Appname) +} diff --git a/internal/logger/logger_test.go b/internal/logger/logger_test.go new file mode 100644 index 0000000..80ec8c9 --- /dev/null +++ b/internal/logger/logger_test.go @@ -0,0 +1,66 @@ +package logger_test + +import ( + "context" + "log/slog" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "sneak.berlin/go/dnswatcher/internal/globals" + "sneak.berlin/go/dnswatcher/internal/logger" +) + +func newTestLogger(t *testing.T) *logger.Logger { + t.Helper() + + g := &globals.Globals{Appname: "dnswatcher", Version: "test"} + + l, err := logger.New(nil, logger.Params{Globals: g}) + require.NoError(t, err) + + return l +} + +// TestNewReturnsUsableLogger checks that the constructor yields a +// working *slog.Logger. +func TestNewReturnsUsableLogger(t *testing.T) { + t.Parallel() + + l := newTestLogger(t) + require.NotNil(t, l.Get(), "Get must return a non-nil logger") +} + +// TestDefaultLevelExcludesDebug verifies the default configuration +// logs at info: debug records are suppressed, info records pass. +func TestDefaultLevelExcludesDebug(t *testing.T) { + t.Parallel() + + log := newTestLogger(t).Get() + ctx := context.Background() + + assert.False(t, log.Enabled(ctx, slog.LevelDebug), + "debug must be suppressed at the default level") + assert.True(t, log.Enabled(ctx, slog.LevelInfo), + "info must be enabled at the default level") +} + +// TestEnableDebugLoggingChangesLevel verifies the debug and non-debug +// configurations differ as intended: enabling debug makes debug +// records pass where they previously did not. +func TestEnableDebugLoggingChangesLevel(t *testing.T) { + t.Parallel() + + l := newTestLogger(t) + log := l.Get() + ctx := context.Background() + + require.False(t, log.Enabled(ctx, slog.LevelDebug), + "debug must start disabled") + + l.EnableDebugLogging() + + assert.True(t, log.Enabled(ctx, slog.LevelDebug), + "debug must be enabled after EnableDebugLogging") +}