Keep test helpers out of the shipped binary (closes #506)

The four testing.go files in config, database, middleware and session
are gone. ClearEnvForTest moves to internal/config/configtest as
ClearEnv; the webhook database manager helpers move to
internal/database/databasetest as NewWebhookDBManager and
NewWebhookDBManagerWithLogger, and the middleware's NewForTest to
internal/middleware/middlewaretest as New, both now built through the
production constructors. Tests that wrapped an open main database use
database.Open. The session helpers move into the session package's
export_test.go; the middleware tests build their session through
session.New and age its timestamps instead of using a fake clock.

The session, middleware and webhook database manager now take the
*slog.Logger they log through, so tests in other packages can give
them their own.

Model: opus-5-5

internal/middleware/accesslog_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/middleware"
+	"sneak.berlin/go/webhooker/internal/middleware/middlewaretest"
 )
 
 // floodRequests is the number of distinct invented paths each flood
@@ -83,7 +84,7 @@ func capturingMiddleware(t *testing.T) (*middleware.Middleware, *bytes.Buffer) {
 		TrustedProxies: trustedProxies("192.0.2.1/32"),
 	}
 
-	return middleware.NewForTest(log, cfg, nil), buf
+	return middlewaretest.New(t, log, cfg, nil), buf
 }
 
 // capturingTextMiddleware is capturingMiddleware for the other handler
@@ -107,7 +108,7 @@ func capturingTextMiddleware(
 		TrustedProxies: trustedProxies("192.0.2.1/32"),
 	}
 
-	return middleware.NewForTest(log, cfg, nil), buf
+	return middlewaretest.New(t, log, cfg, nil), buf
 }
 
 // accessLogRouter mirrors the production route shapes that an

internal/middleware/clientip_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/middleware"
+	"sneak.berlin/go/webhooker/internal/middleware/middlewaretest"
 )
 
 const (
@@ -133,8 +134,8 @@ func clientLogLines(
 		TrustedProxies:    trustedProxies(trustedProxyCIDR),
 	}
 
-	m := middleware.NewForTest(
-		log, cfg, newTestSessionManager(cfg, log, nil),
+	m := middlewaretest.New(
+		t, log, cfg, newTestSessionManager(t, cfg),
 	)
 	handler := m.Logging()(site.build(m))
 

internal/middleware/logbound_test.go

@@ -41,6 +41,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/middleware"
+	"sneak.berlin/go/webhooker/internal/middleware/middlewaretest"
 )
 
 // bodyLimitBytes is the MaxBodySize cap these tests install. Any
@@ -155,9 +156,9 @@ func capturingBoundMiddleware(
 		ReceiverRateLimit: receiverLimitPerMinute,
 	}
 
-	sess := newTestSessionManager(cfg, log, nil)
+	sess := newTestSessionManager(t, cfg)
 
-	return middleware.NewForTest(log, cfg, sess), buf
+	return middlewaretest.New(t, log, cfg, sess), buf
 }
 
 // unreachable is a next-handler that fails the test if the middleware

internal/middleware/metrics.go

@@ -151,7 +151,7 @@ var _ httpmetrics.Recorder = boundedLabelRecorder{}
 
 // Metrics returns middleware that records Prometheus HTTP metrics
 // with the Middleware's one recorder, which New builds on the registry
-// the /metrics route serves and NewForTest on a registry of its own.
+// it is given: in the application, the one the /metrics route serves.
 // Every call reuses that recorder, so any number of routers can
 // install it.
 func (s *Middleware) Metrics() func(http.Handler) http.Handler {

internal/middleware/metrics_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/middleware"
+	"sneak.berlin/go/webhooker/internal/middleware/middlewaretest"
 )
 
 const (
@@ -70,8 +71,8 @@ func metricsTestRouter(
 		Environment:       "prod",
 		ReceiverRateLimit: receiverLimit,
 	}
-	m := middleware.NewForTest(
-		log, cfg, newTestSessionManager(cfg, log, nil),
+	m := middlewaretest.New(
+		t, log, cfg, newTestSessionManager(t, cfg),
 	)
 
 	reg := prometheus.NewRegistry()
@@ -455,11 +456,11 @@ func TestMetrics_StatusAndSizeStillRecorded(t *testing.T) {
 	)
 }
 
-// TestMetrics_WorksOnNewForTestMiddleware pins that a Middleware built
-// by NewForTest has a recorder of its own: its Metrics() serves a
-// request instead of panicking, and a second one does not collide
-// with the first.
-func TestMetrics_WorksOnNewForTestMiddleware(t *testing.T) {
+// TestMetrics_WorksOnMiddlewaretestNew pins that a Middleware built
+// by middlewaretest.New has a recorder of its own: its Metrics()
+// serves a request instead of panicking, and a second one does not
+// collide with the first.
+func TestMetrics_WorksOnMiddlewaretestNew(t *testing.T) {
 	t.Parallel()
 
 	log := slog.New(slog.DiscardHandler)
@@ -469,7 +470,7 @@ func TestMetrics_WorksOnNewForTestMiddleware(t *testing.T) {
 	})
 
 	for range 2 {
-		h := middleware.NewForTest(log, cfg, nil).Metrics()(ok)
+		h := middlewaretest.New(t, log, cfg, nil).Metrics()(ok)
 
 		req := httptest.NewRequestWithContext(
 			t.Context(), http.MethodGet, okRoute, nil,

internal/middleware/middleware.go

@@ -22,7 +22,6 @@ import (
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/globals"
 	"sneak.berlin/go/webhooker/internal/logfield"
-	"sneak.berlin/go/webhooker/internal/logger"
 	"sneak.berlin/go/webhooker/internal/session"
 )
 
@@ -155,7 +154,7 @@ const (
 type MiddlewareParams struct {
 	fx.In
 
-	Logger   *logger.Logger
+	Logger   *slog.Logger
 	Globals  *globals.Globals
 	Config   *config.Config
 	Session  *session.Session
@@ -169,12 +168,10 @@ type Middleware struct {
 	params  *MiddlewareParams
 	session *session.Session
 
-	// metricsRecorder records the inbound HTTP metrics. New builds
-	// it on the registry /metrics serves, NewForTest on a registry
-	// of its own. Either way it is built once per Middleware and
-	// Metrics reuses it, because building it registers its
-	// collectors, and a second registration on the same registry
-	// panics.
+	// metricsRecorder records the inbound HTTP metrics on
+	// params.Registry. It is built once per Middleware and Metrics
+	// reuses it, because building it registers its collectors, and a
+	// second registration on the same registry panics.
 	metricsRecorder httpmetrics.Recorder
 
 	// loginGuard counts failed credential verifications and bounds
@@ -193,7 +190,7 @@ func New(
 ) (*Middleware, error) {
 	s := new(Middleware)
 	s.params = &params
-	s.log = params.Logger.Get()
+	s.log = params.Logger
 	s.session = params.Session
 	s.metricsRecorder = prommetrics.NewRecorder(
 		prommetrics.Config{Registry: params.Registry},

internal/middleware/middleware_test.go

@@ -14,36 +14,34 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.uber.org/fx/fxtest"
 	"sneak.berlin/go/webhooker/internal/config"
+	"sneak.berlin/go/webhooker/internal/database"
 	"sneak.berlin/go/webhooker/internal/middleware"
+	"sneak.berlin/go/webhooker/internal/middleware/middlewaretest"
 	"sneak.berlin/go/webhooker/internal/session"
 )
 
 const testKeySize = 32
 
 // testMiddleware creates a Middleware with minimal dependencies
-// for testing. It uses a real session.Session backed by an
-// in-memory cookie store.
+// for testing. It uses a real session.Session.
 func testMiddleware(
 	t *testing.T,
 	env string,
 ) (*middleware.Middleware, *session.Session) {
 	t.Helper()
 
-	m, s, _ := testMiddlewareWithSessionClock(t, env, 0, nil)
-
-	return m, s
+	return testMiddlewareWithIdleTimeout(t, env, 0)
 }
 
-// testMiddlewareWithSessionClock is testMiddleware with a
-// configurable session idle timeout and a manually advanced clock,
-// for the session-expiry tests. A nil clock uses the real one.
-func testMiddlewareWithSessionClock(
+// testMiddlewareWithIdleTimeout is testMiddleware with a
+// configurable session idle timeout, for the session-expiry tests.
+func testMiddlewareWithIdleTimeout(
 	t *testing.T,
 	env string,
 	idleTimeout time.Duration,
-	clock *fakeClock,
-) (*middleware.Middleware, *session.Session, *fakeClock) {
+) (*middleware.Middleware, *session.Session) {
 	t.Helper()
 
 	log := slog.New(slog.NewTextHandler(
@@ -56,59 +54,44 @@ func testMiddlewareWithSessionClock(
 		SessionIdleTimeout: idleTimeout,
 	}
 
-	sessManager := newTestSessionManager(cfg, log, clock)
+	sessManager := newTestSessionManager(t, cfg)
 
-	m := middleware.NewForTest(log, cfg, sessManager)
+	m := middlewaretest.New(t, log, cfg, sessManager)
 
-	return m, sessManager, clock
+	return m, sessManager
 }
 
 // newTestSessionManager builds the real session.Session the
-// middleware tests run against: an in-memory cookie store with a
-// known key, and optionally a manually advanced clock.
+// middleware tests run against, through session.New, with its key
+// in a main database of its own.
 func newTestSessionManager(
+	t *testing.T,
 	cfg *config.Config,
-	log *slog.Logger,
-	clock *fakeClock,
 ) *session.Session {
-	key := make([]byte, testKeySize)
+	t.Helper()
 
-	for i := range key {
-		key[i] = byte(i)
-	}
+	discard := slog.New(slog.DiscardHandler)
 
-	store := session.NewStore(key)
+	db, err := database.Open(t.TempDir(), discard)
+	require.NoError(t, err)
 
-	var now func() time.Time
+	t.Cleanup(func() { _ = db.Close() })
 
-	if clock != nil {
-		now = clock.Now
-	}
+	lc := fxtest.NewLifecycle(t)
 
-	return session.NewForTest(store, cfg, log, key, now)
-}
+	sessManager, err := session.New(lc, session.Params{
+		Config:   cfg,
+		Database: db,
+		Logger:   discard,
+	})
+	require.NoError(t, err)
 
-// fakeClock is a manually advanced clock, so session expiry can be
-// tested without sleeping.
-type fakeClock struct {
-	t time.Time
-}
+	// The start hook reads the key from db and builds the cookie
+	// store.
+	lc.RequireStart()
+	t.Cleanup(lc.RequireStop)
 
-func (c *fakeClock) Now() time.Time {
-	return c.t
-}
-
-func (c *fakeClock) Advance(d time.Duration) {
-	c.t = c.t.Add(d)
-}
-
-// newFakeClock returns a clock started at a fixed instant.
-func newFakeClock() *fakeClock {
-	return &fakeClock{
-		t: time.Date(
-			2026, time.January, 2, 3, 4, 5, 0, time.UTC,
-		),
-	}
+	return sessManager
 }
 
 // --- Logging Middleware Tests ---
@@ -583,6 +566,40 @@ func sessionCookies(
 	return out
 }
 
+// aged re-issues the session cookie in cookies with both of its
+// timestamps moved back by d: the cookie as it stands once d has
+// passed, so session expiry can be tested without sleeping.
+func aged(
+	t *testing.T,
+	sessManager *session.Session,
+	cookies []*http.Cookie,
+	d time.Duration,
+) []*http.Cookie {
+	t.Helper()
+
+	req := httptest.NewRequestWithContext(
+		context.Background(), http.MethodGet, "/", nil)
+
+	for _, c := range cookies {
+		req.AddCookie(c)
+	}
+
+	sess, err := sessManager.Get(req)
+	require.NoError(t, err)
+
+	for _, key := range []string{session.CreatedAtKey, session.LastSeenKey} {
+		at, ok := sess.Values[key].(int64)
+		require.True(t, ok, "the session has no %s", key)
+
+		sess.Values[key] = at - int64(d/time.Second)
+	}
+
+	w := httptest.NewRecorder()
+	require.NoError(t, sessManager.Save(req, w, sess))
+
+	return sessionCookies(w)
+}
+
 func TestRequireAuth_IdleExpiredSession_RedirectsToLogin(
 	t *testing.T,
 ) {
@@ -590,13 +607,11 @@ func TestRequireAuth_IdleExpiredSession_RedirectsToLogin(
 
 	idle := time.Hour
 
-	m, sessManager, clock := testMiddlewareWithSessionClock(
-		t, config.EnvironmentDev, idle, newFakeClock(),
+	m, sessManager := testMiddlewareWithIdleTimeout(
+		t, config.EnvironmentDev, idle,
 	)
 
-	cookies := loginCookies(t, sessManager)
-
-	clock.Advance(idle)
+	cookies := aged(t, sessManager, loginCookies(t, sessManager), idle)
 
 	called, w := runAuthed(t, m, cookies)
 
@@ -621,14 +636,12 @@ func TestRequireAuth_RefreshesIdleDeadlineOnActivity(
 
 	idle := time.Hour
 
-	m, sessManager, clock := testMiddlewareWithSessionClock(
-		t, config.EnvironmentDev, idle, newFakeClock(),
+	m, sessManager := testMiddlewareWithIdleTimeout(
+		t, config.EnvironmentDev, idle,
 	)
 
-	cookies := loginCookies(t, sessManager)
-
 	// Activity halfway through the idle window.
-	clock.Advance(idle / 2)
+	cookies := aged(t, sessManager, loginCookies(t, sessManager), idle/2)
 
 	called, w := runAuthed(t, m, cookies)
 	require.True(t, called, "handler should run while valid")
@@ -640,16 +653,22 @@ func TestRequireAuth_RefreshesIdleDeadlineOnActivity(
 	)
 
 	// Past the original deadline. The refreshed cookie is still
-	// good; the original one is not.
-	clock.Advance(idle - time.Second)
+	// good; the original one is not. A minute short of the idle
+	// window leaves room for the real clock, which the session
+	// reads, to tick on while the test runs.
+	later := idle - time.Minute
 
-	calledRefreshed, _ := runAuthed(t, m, refreshed)
+	calledRefreshed, _ := runAuthed(
+		t, m, aged(t, sessManager, refreshed, later),
+	)
 	assert.True(
 		t, calledRefreshed,
 		"refreshed session should outlive the original deadline",
 	)
 
-	calledStale, staleW := runAuthed(t, m, cookies)
+	calledStale, staleW := runAuthed(
+		t, m, aged(t, sessManager, cookies, later),
+	)
 	assert.False(
 		t, calledStale,
 		"the pre-refresh cookie carries the old idle deadline",
@@ -662,8 +681,8 @@ func TestRequireAuth_UnauthenticatedRequestDoesNotRefresh(
 ) {
 	t.Parallel()
 
-	m, sessManager, _ := testMiddlewareWithSessionClock(
-		t, config.EnvironmentDev, time.Hour, newFakeClock(),
+	m, sessManager := testMiddlewareWithIdleTimeout(
+		t, config.EnvironmentDev, time.Hour,
 	)
 
 	// A session cookie that exists but was never authenticated.
@@ -924,12 +943,9 @@ func metricsAuthMiddleware(
 		MetricsPassword: "secret",
 	}
 
-	key := make([]byte, testKeySize)
-	store := session.NewStore(key)
-
-	sessManager := session.NewForTest(store, cfg, log, key, nil)
-
-	return middleware.NewForTest(log, cfg, sessManager)
+	return middlewaretest.New(
+		t, log, cfg, newTestSessionManager(t, cfg),
+	)
 }
 
 // runMetricsAuthRequest sends a GET /metrics request with the

internal/middleware/middlewaretest/middlewaretest.go

@@ -0,0 +1,42 @@
+// Package middlewaretest builds a Middleware for tests in other
+// packages.
+package middlewaretest
+
+import (
+	"log/slog"
+	"testing"
+
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/fx/fxtest"
+	"sneak.berlin/go/webhooker/internal/config"
+	"sneak.berlin/go/webhooker/internal/middleware"
+	"sneak.berlin/go/webhooker/internal/session"
+)
+
+// New builds a Middleware through middleware.New, on a
+// lifecycle that is never started.
+//
+// Its metrics recorder writes to a fresh registry of its own, so
+// Metrics() works on it and two of them never collide.
+func New(
+	t *testing.T,
+	log *slog.Logger,
+	cfg *config.Config,
+	sess *session.Session,
+) *middleware.Middleware {
+	t.Helper()
+
+	m, err := middleware.New(
+		fxtest.NewLifecycle(t),
+		middleware.MiddlewareParams{
+			Logger:   log,
+			Config:   cfg,
+			Session:  sess,
+			Registry: prometheus.NewRegistry(),
+		},
+	)
+	require.NoError(t, err)
+
+	return m
+}

internal/middleware/ratelimit_test.go

@@ -18,6 +18,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/middleware"
+	"sneak.berlin/go/webhooker/internal/middleware/middlewaretest"
 )
 
 func TestPostRateLimit_AllowsGET(t *testing.T) {
@@ -198,7 +199,7 @@ func rateLimitMiddleware(
 		&slog.HandlerOptions{Level: slog.LevelDebug},
 	))
 
-	return middleware.NewForTest(log, cfg, nil)
+	return middlewaretest.New(t, log, cfg, nil)
 }
 
 // trustedProxies parses CIDR strings for a test Config.

internal/middleware/testing.go

@@ -1,32 +0,0 @@
-package middleware
-
-import (
-	"log/slog"
-
-	"github.com/prometheus/client_golang/prometheus"
-	prommetrics "github.com/slok/go-http-metrics/metrics/prometheus"
-	"sneak.berlin/go/webhooker/internal/config"
-	"sneak.berlin/go/webhooker/internal/session"
-)
-
-// NewForTest creates a Middleware with the minimum dependencies
-// needed for testing. This bypasses the fx lifecycle.
-//
-// Its metrics recorder writes to a fresh registry of its own, so
-// Metrics() works on it and two of them never collide.
-func NewForTest(
-	log *slog.Logger,
-	cfg *config.Config,
-	sess *session.Session,
-) *Middleware {
-	return &Middleware{
-		log: log,
-		params: &MiddlewareParams{
-			Config: cfg,
-		},
-		session: sess,
-		metricsRecorder: prommetrics.NewRecorder(
-			prommetrics.Config{Registry: prometheus.NewRegistry()},
-		),
-	}
-}