Keep each test database when the pool replaces its connection (closes #209)

SQLite deletes a shared in-memory database when its last connection
closes. A query cancelled while it runs makes the pool close its
connection, so a test could lose all its tables and rows at random.

The new package internal/dbtest has NewURL. It returns the URL of a
new in-memory database with a random name and holds one connection to
it open until the test ends. The db, handlers, ircserver, server and
service tests take their database URL from it. The test-support lint
rule keeps the package out of the server binary.

A new test cancels a query on such a database, checks with a TEMP
table that the connection was replaced, and checks that a row is
still there.

Model: opus-5-5

.golangci.yml

@@ -61,6 +61,10 @@ linters:
               desc: >-
                 Test-support code belongs in test files and in packages whose
                 directory name ends in test, not in the shipped binary.
+            - pkg: sneak.berlin/go/neoirc/internal/dbtest
+              desc: >-
+                Test-support code belongs in test files and in packages whose
+                directory name ends in test, not in the shipped binary.
     # Only decisions already recorded in the Go package defaults are
     # listed here. Every entry matches the module path exactly.
     gomodguard_v2:

README.md

@@ -3342,6 +3342,7 @@ neoirc/
 │   │   └── schema/
 │   │       ├── 000.sql     # schema_migrations bootstrap
 │   │       └── 001_initial.sql
+│   ├── dbtest/             # Gives each test its own database; only tests import it
 │   ├── globals/            # Application-wide metadata
 │   │   └── globals.go
 │   ├── handlers/           # HTTP request handlers

internal/db/db_test.go

@@ -12,6 +12,7 @@ import (
 	"go.uber.org/fx/fxtest"
 	"sneak.berlin/go/neoirc/internal/config"
 	"sneak.berlin/go/neoirc/internal/db"
+	"sneak.berlin/go/neoirc/internal/dbtest"
 	"sneak.berlin/go/neoirc/internal/globals"
 	"sneak.berlin/go/neoirc/internal/logger"
 )
@@ -155,6 +156,52 @@ func TestForeignKeysOnAfterCancelledQuery(t *testing.T) {
 	}
 }
 
+// TestNewURLKeepsDataAfterCancelledQuery cancels a query on a test
+// database while it runs, which makes the pool close its only
+// connection and open a new one. A TEMP table lasts only as long as
+// its connection, so its absence shows the connection was replaced.
+// The database must still have its tables and rows.
+func TestNewURLKeepsDataAfterCancelledQuery(t *testing.T) {
+	t.Parallel()
+
+	database, err := startDatabase(t, dbtest.NewURL(t))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = database.GetOrCreateChannel(t.Context(), "#test")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = database.GetDB().ExecContext(
+		t.Context(), "CREATE TEMP TABLE first_connection (x)",
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ctx, cancel := context.WithTimeout(t.Context(), endlessQueryTimeout)
+	defer cancel()
+
+	_, err = database.GetDB().ExecContext(ctx, endlessQuery)
+	if !errors.Is(err, context.DeadlineExceeded) {
+		t.Fatalf("endless query: err = %v", err)
+	}
+
+	tempTables := queryInt(
+		t, database, "SELECT COUNT(*) FROM sqlite_temp_schema",
+	)
+	if tempTables != 0 {
+		t.Fatal("the pool kept its first connection")
+	}
+
+	channels := queryInt(t, database, "SELECT COUNT(*) FROM channels")
+	if channels != 1 {
+		t.Errorf("channels = %d, want 1", channels)
+	}
+}
+
 // TestDBURLSettingForeignKeysFailsStartup checks that the database
 // refuses to start when DBURL sets foreign_keys itself, however it
 // is spelled.

internal/db/export_test.go

@@ -5,21 +5,16 @@ import (
 	"database/sql"
 	"fmt"
 	"log/slog"
-	"sync/atomic"
+	"testing"
+
+	"sneak.berlin/go/neoirc/internal/dbtest"
 )
 
-//nolint:gochecknoglobals // test counter
-var testDBCounter atomic.Int64
-
 // NewTestDatabase creates an in-memory database for testing.
-func NewTestDatabase() (*Database, error) {
+func NewTestDatabase(tb testing.TB) (*Database, error) {
-	counter := testDBCounter.Add(1)
+	tb.Helper()
 
-	dsn := fmt.Sprintf(
+	dsn := dbtest.NewURL(tb) + "&_pragma=foreign_keys(1)"
-		"file:testdb%d?mode=memory"+
-			"&cache=shared&_pragma=foreign_keys(1)",
-		counter,
-	)
 
 	conn, err := sql.Open("sqlite", dsn)
 	if err != nil {

internal/db/queries_test.go

@@ -13,7 +13,7 @@ import (
 func setupTestDB(t *testing.T) *db.Database {
 	t.Helper()
 
-	database, err := db.NewTestDatabase()
+	database, err := db.NewTestDatabase(t)
 	if err != nil {
 		t.Fatal(err)
 	}

internal/dbtest/dbtest.go

@@ -0,0 +1,39 @@
+// Package dbtest gives each test a database of its own. Only tests
+// may import it.
+package dbtest
+
+import (
+	"crypto/rand"
+	"database/sql"
+	"testing"
+
+	_ "modernc.org/sqlite" // registers the sqlite driver
+)
+
+// NewURL returns the URL of a new, empty in-memory database for one
+// test. SQLite deletes an in-memory database when its last connection
+// closes. A pool closes a connection when a query on it is cancelled
+// while it runs. So this function holds a connection of its own open
+// until the test ends.
+func NewURL(tb testing.TB) string {
+	tb.Helper()
+
+	// A random name, so that no other test can share the database.
+	dbURL := "file:" + rand.Text() + "?mode=memory&cache=shared"
+
+	pool, err := sql.Open("sqlite", dbURL)
+	if err != nil {
+		tb.Fatalf("open test database: %v", err)
+	}
+
+	tb.Cleanup(func() { _ = pool.Close() })
+
+	conn, err := pool.Conn(tb.Context())
+	if err != nil {
+		tb.Fatalf("connect to test database: %v", err)
+	}
+
+	tb.Cleanup(func() { _ = conn.Close() })
+
+	return dbURL
+}

internal/handlers/api_test.go

@@ -3,7 +3,6 @@ package handlers_test
 import (
 	"bytes"
 	"context"
-	"crypto/rand"
 	"database/sql"
 	"encoding/json"
 	"errors"
@@ -28,6 +27,7 @@ import (
 	api "sneak.berlin/go/neoirc/internal/cli/api"
 	"sneak.berlin/go/neoirc/internal/config"
 	"sneak.berlin/go/neoirc/internal/db"
+	"sneak.berlin/go/neoirc/internal/dbtest"
 	"sneak.berlin/go/neoirc/internal/globals"
 	"sneak.berlin/go/neoirc/internal/handlers"
 	"sneak.berlin/go/neoirc/internal/hashcash"
@@ -164,13 +164,7 @@ func newTestServerWith(
 	viperMu.Lock()
 	defer viperMu.Unlock()
 
-	// A random name, so that no other test can share it, not
+	dbURL := dbtest.NewURL(t)
-	// even one that has finished but still holds its database
-	// open.
-	dbURL := fmt.Sprintf(
-		"file:%s?mode=memory&cache=shared",
-		rand.Text(),
-	)
 	copyTemplateDB(t, dbURL)
 
 	var (

internal/ircserver/server_test.go

@@ -3,7 +3,6 @@ package ircserver_test
 import (
 	"bufio"
 	"context"
-	"crypto/rand"
 	"database/sql"
 	"errors"
 	"fmt"
@@ -20,6 +19,7 @@ import (
 	"sneak.berlin/go/neoirc/internal/broker"
 	"sneak.berlin/go/neoirc/internal/config"
 	"sneak.berlin/go/neoirc/internal/db"
+	"sneak.berlin/go/neoirc/internal/dbtest"
 	"sneak.berlin/go/neoirc/internal/globals"
 	"sneak.berlin/go/neoirc/internal/ircserver"
 	"sneak.berlin/go/neoirc/internal/logger"
@@ -107,14 +107,7 @@ func newTestEnvWithResolver(
 ) *testEnv {
 	t.Helper()
 
-	// A random name, so that no other test environment, not
+	conn, err := sql.Open("sqlite", dbtest.NewURL(t))
-	// even an earlier run of the same test, can share it.
-	dsn := fmt.Sprintf(
-		"file:%s?mode=memory&cache=shared",
-		rand.Text(),
-	)
-
-	conn, err := sql.Open("sqlite", dsn)
 	if err != nil {
 		t.Fatalf("open db: %v", err)
 	}

internal/server/server_test.go

@@ -5,7 +5,6 @@
 package server_test
 
 import (
-	"crypto/rand"
 	"fmt"
 	"net"
 	"net/http"
@@ -19,6 +18,7 @@ import (
 	"sneak.berlin/go/neoirc/internal/broker"
 	"sneak.berlin/go/neoirc/internal/config"
 	"sneak.berlin/go/neoirc/internal/db"
+	"sneak.berlin/go/neoirc/internal/dbtest"
 	"sneak.berlin/go/neoirc/internal/globals"
 	"sneak.berlin/go/neoirc/internal/handlers"
 	"sneak.berlin/go/neoirc/internal/healthcheck"
@@ -241,10 +241,7 @@ func newApp(
 			stats.New,
 		),
 		fx.Decorate(func(cfg *config.Config) *config.Config {
-			// A random name, so that no other test can share it.
+			cfg.DBURL = dbtest.NewURL(t)
-			cfg.DBURL = fmt.Sprintf(
-				"file:%s?mode=memory&cache=shared", rand.Text(),
-			)
 			cfg.Port = port
 
 			return cfg

internal/service/service_test.go

@@ -6,7 +6,6 @@ package service_test
 
 import (
 	"context"
-	"crypto/rand"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -23,6 +22,7 @@ import (
 	"sneak.berlin/go/neoirc/internal/broker"
 	"sneak.berlin/go/neoirc/internal/config"
 	"sneak.berlin/go/neoirc/internal/db"
+	"sneak.berlin/go/neoirc/internal/dbtest"
 	"sneak.berlin/go/neoirc/internal/globals"
 	"sneak.berlin/go/neoirc/internal/hashcash"
 	"sneak.berlin/go/neoirc/internal/logger"
@@ -46,13 +46,7 @@ type testEnv struct {
 func newTestEnv(t *testing.T) *testEnv {
 	t.Helper()
 
-	// A random name, so that no other test can share it, not
+	dbURL := dbtest.NewURL(t)
-	// even one that has finished but still holds its database
-	// open.
-	dbURL := fmt.Sprintf(
-		"file:%s?mode=memory&cache=shared",
-		rand.Text(),
-	)
 
 	var (
 		database *db.Database