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

The test helpers lived in ordinary `testing.go` files inside the config, database, middleware and session packages, so they were built into the binary and the shared `test-support` lint rule could not see them. The four files are gone: the session's helpers move into its own `_test.go` file, and the rest into `configtest`, `databasetest` and `middlewaretest`, which the `depguard` deny list now names, so a non-test file importing them fails lint. The test-support packages build through the production constructors.

Judgement call: the session, the middleware and the webhook database manager now take the plain logger they log through, which the application wiring provides.
Judgement call: two idle-expiry tests move the stored timestamps back instead of advancing a fake clock.

Model: opus-5-5

internal/database/databasetest/databasetest.go

@@ -0,0 +1,55 @@
+// Package databasetest builds a WebhookDBManager for tests in other
+// packages.
+package databasetest
+
+import (
+	"log/slog"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"go.uber.org/fx/fxtest"
+	"sneak.berlin/go/webhooker/internal/config"
+	"sneak.berlin/go/webhooker/internal/database"
+)
+
+// NewWebhookDBManager creates a WebhookDBManager backed by the given
+// data directory, logging at DEBUG to standard error.
+func NewWebhookDBManager(
+	t *testing.T, dataDir string,
+) *database.WebhookDBManager {
+	t.Helper()
+
+	return NewWebhookDBManagerWithLogger(
+		t,
+		dataDir,
+		slog.New(slog.NewTextHandler(
+			os.Stderr,
+			&slog.HandlerOptions{Level: slog.LevelDebug},
+		)),
+	)
+}
+
+// NewWebhookDBManagerWithLogger is NewWebhookDBManager with the
+// logger supplied by the caller. The per-webhook databases this manager
+// opens hand that logger to gormlog, so a test that needs to see the SQL
+// the service emits can capture it.
+//
+// It is built through database.NewWebhookDBManager on a lifecycle that
+// is never started, so nothing closes its databases but the caller.
+func NewWebhookDBManagerWithLogger(
+	t *testing.T, dataDir string, log *slog.Logger,
+) *database.WebhookDBManager {
+	t.Helper()
+
+	mgr, err := database.NewWebhookDBManager(
+		fxtest.NewLifecycle(t),
+		database.WebhookDBManagerParams{
+			Config: &config.Config{DataDir: dataDir},
+			Logger: log,
+		},
+	)
+	require.NoError(t, err)
+
+	return mgr
+}

internal/database/event_db_isolation_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/stretchr/testify/require"
 	_ "modernc.org/sqlite"
 	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/database/databasetest"
 )
 
 // testDataDirPerm is the mode the test data directory is created
@@ -133,7 +134,7 @@ func TestOpenPurgesLeakedTargetRows(t *testing.T) {
 	// Create the file the way the application does, so the targets
 	// table has exactly the shape AutoMigrate gives it, then write
 	// a leaked row into it the way the association upsert did.
-	initial := database.NewTestWebhookDBManager(dataDir)
+	initial := databasetest.NewWebhookDBManager(t, dataDir)
 
 	_, err := initial.GetDB(webhookID)
 	require.NoError(t, err)
@@ -156,7 +157,7 @@ func TestOpenPurgesLeakedTargetRows(t *testing.T) {
 	clearEventDBSweptMarker(t, seed)
 	require.NoError(t, seed.Close())
 
-	mgr := database.NewTestWebhookDBManager(dataDir)
+	mgr := databasetest.NewWebhookDBManager(t, dataDir)
 
 	_, err = mgr.GetDB(webhookID)
 	require.NoError(t, err)
@@ -172,7 +173,7 @@ func TestOpenPurgesLeakedTargetRows(t *testing.T) {
 
 	// Idempotent: a second open leaves it at zero and does not
 	// error.
-	again := database.NewTestWebhookDBManager(dataDir)
+	again := databasetest.NewWebhookDBManager(t, dataDir)
 
 	_, err = again.GetDB(webhookID)
 	require.NoError(t, err)
@@ -195,7 +196,7 @@ func TestOpenPurgeRemovesCredentialBytes(t *testing.T) {
 	webhookID := uuid.New().String()
 	credential := "T00000000/B00000000/" + uuid.New().String()
 
-	initial := database.NewTestWebhookDBManager(dataDir)
+	initial := databasetest.NewWebhookDBManager(t, dataDir)
 
 	_, err := initial.GetDB(webhookID)
 	require.NoError(t, err)
@@ -230,7 +231,7 @@ func TestOpenPurgeRemovesCredentialBytes(t *testing.T) {
 		"seeded credential is not in the file, so this test proves nothing",
 	)
 
-	mgr := database.NewTestWebhookDBManager(dataDir)
+	mgr := databasetest.NewWebhookDBManager(t, dataDir)
 
 	_, err = mgr.GetDB(webhookID)
 	require.NoError(t, err)
@@ -258,7 +259,7 @@ func TestOpenRevacuumsAfterIncompleteSweep(t *testing.T) {
 	webhookID := uuid.New().String()
 	credential := "T00000000/B00000000/" + uuid.New().String()
 
-	initial := database.NewTestWebhookDBManager(dataDir)
+	initial := databasetest.NewWebhookDBManager(t, dataDir)
 
 	_, err := initial.GetDB(webhookID)
 	require.NoError(t, err)
@@ -298,7 +299,7 @@ func TestOpenRevacuumsAfterIncompleteSweep(t *testing.T) {
 			"test proves nothing",
 	)
 
-	mgr := database.NewTestWebhookDBManager(dataDir)
+	mgr := databasetest.NewWebhookDBManager(t, dataDir)
 
 	_, err = mgr.GetDB(webhookID)
 	require.NoError(t, err)
@@ -325,7 +326,7 @@ func TestOpenSkipsSweptDatabase(t *testing.T) {
 	dataDir := eventDBDataDir(t)
 	webhookID := uuid.New().String()
 
-	mgr := database.NewTestWebhookDBManager(dataDir)
+	mgr := databasetest.NewWebhookDBManager(t, dataDir)
 
 	_, err := mgr.GetDB(webhookID)
 	require.NoError(t, err)
@@ -347,7 +348,7 @@ func TestOpenSkipsSweptDatabase(t *testing.T) {
 	require.NoError(t, err)
 	require.NoError(t, marked.Close())
 
-	again := database.NewTestWebhookDBManager(dataDir)
+	again := databasetest.NewWebhookDBManager(t, dataDir)
 
 	_, err = again.GetDB(webhookID)
 	require.NoError(t, err)
@@ -378,7 +379,7 @@ func TestOpenSucceedsWithoutTargetsTable(t *testing.T) {
 	require.NoError(t, err)
 	require.NoError(t, seed.Close())
 
-	mgr := database.NewTestWebhookDBManager(dataDir)
+	mgr := databasetest.NewWebhookDBManager(t, dataDir)
 
 	db, err := mgr.GetDB(webhookID)
 	require.NoError(t, err)
@@ -396,7 +397,7 @@ func TestEventDBCreateOmitsAssociations(t *testing.T) {
 	dataDir := eventDBDataDir(t)
 	webhookID := uuid.New().String()
 
-	mgr := database.NewTestWebhookDBManager(dataDir)
+	mgr := databasetest.NewWebhookDBManager(t, dataDir)
 
 	db, err := mgr.GetDB(webhookID)
 	require.NoError(t, err)

internal/database/retention_test.go

@@ -51,7 +51,7 @@ func setupRetentionTest(t *testing.T) *retentionTestEnv {
 
 	mgr, err := database.NewWebhookDBManager(
 		lc,
-		database.WebhookDBManagerParams{Config: cfg, Logger: l},
+		database.WebhookDBManagerParams{Config: cfg, Logger: l.Get()},
 	)
 	require.NoError(t, err)
 

internal/database/testing.go

@@ -1,47 +0,0 @@
-package database
-
-import (
-	"log/slog"
-	"os"
-
-	"gorm.io/gorm"
-)
-
-// NewTestDatabase creates a Database wrapper around a pre-opened *gorm.DB.
-// Intended for use in tests that need a *database.Database without the
-// full fx lifecycle. The caller is responsible for closing the underlying
-// sql.DB connection.
-func NewTestDatabase(db *gorm.DB) *Database {
-	return &Database{
-		db: db,
-		log: slog.New(slog.NewTextHandler(
-			os.Stderr,
-			&slog.HandlerOptions{Level: slog.LevelDebug},
-		)),
-	}
-}
-
-// NewTestWebhookDBManager creates a WebhookDBManager backed by the given
-// data directory. Intended for use in tests without the fx lifecycle.
-func NewTestWebhookDBManager(dataDir string) *WebhookDBManager {
-	return NewTestWebhookDBManagerWithLogger(
-		dataDir,
-		slog.New(slog.NewTextHandler(
-			os.Stderr,
-			&slog.HandlerOptions{Level: slog.LevelDebug},
-		)),
-	)
-}
-
-// NewTestWebhookDBManagerWithLogger is NewTestWebhookDBManager with the
-// logger supplied by the caller. The per-webhook databases this manager
-// opens hand that logger to gormlog, so a test that needs to see the SQL
-// the service emits can capture it.
-func NewTestWebhookDBManagerWithLogger(
-	dataDir string, log *slog.Logger,
-) *WebhookDBManager {
-	return &WebhookDBManager{
-		dataDir: dataDir,
-		log:     log,
-	}
-}

internal/database/webhook_db_manager.go

@@ -15,7 +15,6 @@ import (
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/datadir"
 	"sneak.berlin/go/webhooker/internal/gormlog"
-	"sneak.berlin/go/webhooker/internal/logger"
 )
 
 // WebhookDBManagerParams holds the fx dependencies for
@@ -24,7 +23,7 @@ type WebhookDBManagerParams struct {
 	fx.In
 
 	Config *config.Config
-	Logger *logger.Logger
+	Logger *slog.Logger
 }
 
 // errInvalidCachedDBType indicates a type assertion failure
@@ -70,7 +69,7 @@ func NewWebhookDBManager(
 ) (*WebhookDBManager, error) {
 	m := &WebhookDBManager{
 		dataDir: params.Config.DataDir,
-		log:     params.Logger.Get(),
+		log:     params.Logger,
 	}
 
 	// Create data directory if it doesn't exist. datadir.DirPerm is the

internal/database/webhook_db_manager_test.go

@@ -18,6 +18,7 @@ import (
 	"gorm.io/gorm"
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/database/databasetest"
 	"sneak.berlin/go/webhooker/internal/globals"
 	"sneak.berlin/go/webhooker/internal/logger"
 )
@@ -50,7 +51,7 @@ func setupTestWebhookDBManager(
 		lc,
 		database.WebhookDBManagerParams{
 			Config: cfg,
-			Logger: l,
+			Logger: l.Get(),
 		},
 	)
 	require.NoError(t, err)
@@ -117,7 +118,8 @@ func TestWebhookDBManager_ConcurrentFirstTouchOpensOnce(t *testing.T) {
 
 	var logs bytes.Buffer
 
-	mgr := database.NewTestWebhookDBManagerWithLogger(
+	mgr := databasetest.NewWebhookDBManagerWithLogger(
+		t,
 		t.TempDir(),
 		slog.New(slog.NewTextHandler(&logs, nil)),
 	)
@@ -307,7 +309,8 @@ func TestWebhookDBManager_LostDatabaseIsLogged(t *testing.T) {
 
 		var logs bytes.Buffer
 
-		mgr := database.NewTestWebhookDBManagerWithLogger(
+		mgr := databasetest.NewWebhookDBManagerWithLogger(
+			t,
 			t.TempDir(),
 			slog.New(slog.NewTextHandler(&logs, nil)),
 		)