Author SHA1 Message Date
clawbot 7674efd2b1 Rewrite AGENTS.md sentence by sentence
check / check (push) Waiting to run
Rewrite the agent rules from scratch, with one idea per sentence and
every file name and command in backticks. Drop the rule that models
embed `models.Base`, which no package has. Say that the linter keeps
tests in external test packages, and that an `export_test.go` file
gives them access to unexported code. Limit the rules on doc comments
and blank imports to files outside the tests. State magic numbers as
a preference, and correct the blank-line rule for `return`. State the
branch workflow: pull requests target `next` and only the owner
merges `next` into `main`. Record the three snake_case JSON tags the
HTTP API keeps.

Part of #190.

Model: fable-5-1 (writing); opus-5-5 (rework)
2026-10-27 13:56:57 +00:00
clawbot ecf47c37f5 Start test servers on a port the system picks (closes #211)
check / check (push) Canceled after 0s
The internal/server and internal/ircserver tests picked a port by
opening a listener, reading its port and closing it. They then started
a server on that number. Another process could take the port in
between, and a test failed with "address already in use".

The IRC server tests now start the server on 127.0.0.1:0 and read the
port back from its listener. The HTTP server tests start the app with
port 0. The HTTP server now keeps the listener it binds, so the stop
test sends its long poll to the port the app was given.

Model: opus-5-5
2026-10-27 13:34:45 +01:00
clawbot d444831f41 Keep each test database when the pool replaces its connection (closes #209)
check / check (push) Canceled after 0s
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
2026-10-27 12:26:48 +01:00
clawbot 53009cf9a0 Keep foreign keys on for every database connection (closes #207)
check / check (push) Canceled after 0s
SQLite keeps the foreign keys setting per connection, and the server
turned it on once with a PRAGMA. A query cancelled while it runs makes
the pool replace its only connection, and the new one ran without
foreign keys until a restart.

connect now adds `_pragma=foreign_keys(1)` to the database URL. The
driver runs it on every connection it opens. The server refuses to
start when DBURL sets foreign_keys itself, because the driver sorts
the pragmas and that one could win.

One test cancels a query and checks, with a TEMP table, that the
connection was replaced and that foreign keys are on. Another checks
the startup error for several spellings.

Model: opus-5-5
2026-10-27 11:14:01 +01:00
14 changed files with 377 additions and 146 deletions
+4
View File
@@ -61,6 +61,10 @@ linters:
desc: >- desc: >-
Test-support code belongs in test files and in packages whose Test-support code belongs in test files and in packages whose
directory name ends in test, not in the shipped binary. 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 # Only decisions already recorded in the Go package defaults are
# listed here. Every entry matches the module path exactly. # listed here. Every entry matches the module path exactly.
gomodguard_v2: gomodguard_v2:
+56 -38
View File
@@ -1,55 +1,73 @@
# AGENTS.md — Rules for AI Agents # Rules for coding agents
## Before Every Commit This file holds the rules for coding agents working in this repository.
`CONVENTIONS.md` defines the architecture. `REPO_POLICIES.md` defines the
repository standards. Both are copies of shared documents. Do not edit them
here.
1. **Format**: `make fmt` ## Before every commit
2. **Check**: `make check` — the tests and the linter, which run in Docker, and
the format check: all passing, zero issues
Never run `go test`, `golangci-lint` or `gofmt` directly; use the `make` 1. Run `make fmt`.
targets. 2. Run `make check`. It runs the tests and the linter in Docker, then the format
check. All three must pass with no findings.
No commit lands on main with lint errors, test failures, or formatting issues. Never run `go test`, `golangci-lint` or `gofmt` directly. Use the `make` targets
instead, such as `make test` and `make lint`.
## Code Style ## Branches and pull requests
- Follow CONVENTIONS.md exactly — it defines the architecture - Never commit to `main` or `next` directly. Every change is a pull request from
- All exported types, functions, methods, and vars must have doc comments a feature branch into `next`.
- All packages must have a package doc comment - Only the owner merges `next` into `main`.
- JSON tags use camelCase (not snake_case) - A pull request that closes an issue has the title `Description (closes #N)`.
- No magic numbers — use named constants - One logical change per commit.
- No inline error handling (`if err := ...; err != nil`) — use plain assignment - Commit messages are in the imperative mood and short.
- Blank line before every `return` statement
- Blank line separating embedded fields from regular fields in structs
- Blank imports must have a justification comment
## Git Workflow ## Code style
- **Never commit directly to main** — all changes go on feature branches - Follow `CONVENTIONS.md`.
- Merge to main only when ready and passing all checks - Outside test files, every package has a package doc comment.
- PR titles: `Description (closes #N)` when closing an issue - Outside test files, every exported type, function and variable has a doc
- One logical change per commit comment.
- Commit messages: imperative mood, concise - JSON tags are camelCase. The HTTP API's `last_id`, `pow_token` and
`hashcash_bits` stay as they are, each with a `//nolint:tagliatelle` comment.
- Prefer named constants to magic numbers. The `mnd` linter checks for them
outside test files.
- No inline error handling such as `if err := f(); err != nil`. Assign the error
on its own line, then test it.
- A `return` that follows another statement in its block has a blank line before
it.
- A blank line between the embedded fields of a struct and its other fields.
- Outside test files, every blank import has a comment saying why it is there.
## Architecture ## Architecture
- Dependency injection via Uber fx — all components are fx providers - The server uses `go.uber.org/fx` for dependency injection.
- `internal/` for all application packages — one package per concern `cmd/neoircd/main.go` lists its `fx` providers, such as `db.New` and
- Handlers return `http.HandlerFunc` using the closure pattern `handlers.New`.
- Request/response structs defined inside handler closures - Application packages live under `internal/`, one package per concern.
- Models embed `models.Base` for database access to relation methods - `pkg/irc` holds the IRC command names and numeric reply codes.
- Database migrations in `internal/db/schema/` — embedded via `go:embed` - The `web` package embeds the built web client.
- SQLite by default (pure Go driver: modernc.org/sqlite) - Each exported `Handle…` method of `Handlers` returns an `http.HandlerFunc`.
- A request struct is defined inside the method that decodes it.
- The server uses the database only through the methods of `db.Database`.
- The database migrations live in `internal/db/schema/` and are embedded with
`go:embed`.
- The database is SQLite, through the pure Go driver `modernc.org/sqlite`.
## Testing ## Testing
- Table-driven tests preferred - Prefer table-driven tests.
- Use `testing.T` helpers, not assertion libraries - Use `testing.T` helpers, not assertion libraries.
- Test files live next to the code they test - A test file lives next to the code it tests and ends in `_test.go`.
- `_test.go` suffix, same package for white-box testing - The linter requires each test file to be in the package's external test
package, such as `handlers_test`. It allows `export_test.go` and files ending
in `_internal_test.go` in the package itself.
- An `export_test.go` file gives the external tests access to the package's
unexported code.
## Dependencies ## Dependencies
- Required libraries are listed in CONVENTIONS.md §1 - The required libraries are listed in `CONVENTIONS.md` §1.
- No new dependencies without justification - No new dependency without a reason given in the pull request.
- Pure Go preferred (no CGO) - Prefer dependencies written in pure Go, which build without cgo.
+2 -1
View File
@@ -2611,7 +2611,7 @@ listener.
| Variable | Type | Default | Description | | Variable | Type | Default | Description |
| ---------------------- | ------ | -------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ---------------------- | ------ | -------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `PORT` | int | `8080` | HTTP listen port | | `PORT` | int | `8080` | HTTP listen port |
| `DBURL` | string | `file:///var/lib/neoirc/state.db?_pragma=journal_mode(WAL)&_pragma=busy_timeout(5000)` | SQLite connection string. For file-based: `file:///path/to/db.db?_pragma=journal_mode(WAL)&_pragma=busy_timeout(5000)`. For in-memory (testing): `file::memory:?cache=shared`. See [SQLite Considerations](#sqlite-considerations). | | `DBURL` | string | `file:///var/lib/neoirc/state.db?_pragma=journal_mode(WAL)&_pragma=busy_timeout(5000)` | SQLite connection string. For file-based: `file:///path/to/db.db?_pragma=journal_mode(WAL)&_pragma=busy_timeout(5000)`. For in-memory (testing): `file::memory:?cache=shared`. It must not set `foreign_keys`. See [SQLite Considerations](#sqlite-considerations). |
| `DEBUG` | bool | `false` | Switch logging to debug level | | `DEBUG` | bool | `false` | Switch logging to debug level |
| `MESSAGE_MAX_AGE` | string | `720h` | Maximum age of messages as a Go duration string (e.g. `720h`, `24h`). Messages older than this are pruned. Default is 30 days. | | `MESSAGE_MAX_AGE` | string | `720h` | Maximum age of messages as a Go duration string (e.g. `720h`, `24h`). Messages older than this are pruned. Default is 30 days. |
| `SESSION_IDLE_TIMEOUT` | string | `720h` | Idle timeout as a Go duration string (e.g. `720h`, `24h`). Clients with no authenticated HTTP API request or IRC listener command for this long are removed, and a session left with none is expired and its nick released (see [Data Lifecycle](#data-lifecycle)). Default is 30 days. | | `SESSION_IDLE_TIMEOUT` | string | `720h` | Idle timeout as a Go duration string (e.g. `720h`, `24h`). Clients with no authenticated HTTP API request or IRC listener command for this long are removed, and a session left with none is expired and its nick released (see [Data Lifecycle](#data-lifecycle)). Default is 30 days. |
@@ -3342,6 +3342,7 @@ neoirc/
│ │ └── schema/ │ │ └── schema/
│ │ ├── 000.sql # schema_migrations bootstrap │ │ ├── 000.sql # schema_migrations bootstrap
│ │ └── 001_initial.sql │ │ └── 001_initial.sql
│ ├── dbtest/ # Gives each test its own database; only tests import it
│ ├── globals/ # Application-wide metadata │ ├── globals/ # Application-wide metadata
│ │ └── globals.go │ │ └── globals.go
│ ├── handlers/ # HTTP request handlers │ ├── handlers/ # HTTP request handlers
+33 -7
View File
@@ -5,9 +5,11 @@ import (
"context" "context"
"database/sql" "database/sql"
"embed" "embed"
"errors"
"fmt" "fmt"
"io/fs" "io/fs"
"log/slog" "log/slog"
"net/url"
"sort" "sort"
"strconv" "strconv"
"strings" "strings"
@@ -22,6 +24,10 @@ import (
const minMigrationParts = 2 const minMigrationParts = 2
var errForeignKeysInDBURL = errors.New(
"DBURL must not set foreign_keys, which the server always turns on",
)
// SchemaFiles contains embedded SQL migration files. // SchemaFiles contains embedded SQL migration files.
// //
//go:embed schema/*.sql //go:embed schema/*.sql
@@ -90,6 +96,33 @@ func (database *Database) connect(ctx context.Context) error {
dbURL = config.DefaultDBURL dbURL = config.DefaultDBURL
} }
// SQLite keeps the foreign keys setting per connection. The driver
// runs each _pragma parameter on every connection it opens. So the
// one added below also covers the connection the pool opens after
// a cancelled query breaks the old one. A foreign_keys pragma in
// DBURL could undo it, because the driver sorts the pragmas.
_, query, hasQuery := strings.Cut(dbURL, "?")
urlParams, err := url.ParseQuery(query)
if err != nil {
return fmt.Errorf("parse DBURL: %w", err)
}
for _, pragma := range urlParams["_pragma"] {
if strings.Contains(strings.ToLower(pragma), "foreign_keys") {
return fmt.Errorf(
"%w: _pragma=%s", errForeignKeysInDBURL, pragma,
)
}
}
separator := "?"
if hasQuery {
separator = "&"
}
dbURL += separator + "_pragma=foreign_keys(1)"
database.log.Info( database.log.Info(
"connecting to database", "url", dbURL, "connecting to database", "url", dbURL,
) )
@@ -109,13 +142,6 @@ func (database *Database) connect(ctx context.Context) error {
database.conn = conn database.conn = conn
database.log.Info("database connected") database.log.Info("database connected")
_, err = database.conn.ExecContext(
ctx, "PRAGMA foreign_keys = ON",
)
if err != nil {
return fmt.Errorf("enable foreign keys: %w", err)
}
return database.runMigrations(ctx) return database.runMigrations(ctx)
} }
+203
View File
@@ -1,19 +1,34 @@
package db_test package db_test
import ( import (
"context"
"errors"
"net/url"
"path/filepath" "path/filepath"
"strings" "strings"
"testing" "testing"
"time"
"go.uber.org/fx/fxtest" "go.uber.org/fx/fxtest"
"sneak.berlin/go/neoirc/internal/config" "sneak.berlin/go/neoirc/internal/config"
"sneak.berlin/go/neoirc/internal/db" "sneak.berlin/go/neoirc/internal/db"
"sneak.berlin/go/neoirc/internal/dbtest"
"sneak.berlin/go/neoirc/internal/globals" "sneak.berlin/go/neoirc/internal/globals"
"sneak.berlin/go/neoirc/internal/logger" "sneak.berlin/go/neoirc/internal/logger"
) )
const wantBusyTimeoutMS = 5000 const wantBusyTimeoutMS = 5000
const (
// endlessQuery counts the rows of a recursive query that never
// stops, so it runs until its context ends.
endlessQuery = `WITH RECURSIVE n(x) AS
(SELECT 1 UNION ALL SELECT x + 1 FROM n)
SELECT COUNT(*) FROM n`
endlessQueryTimeout = 100 * time.Millisecond
)
// TestDefaultDBURLSetsWALAndBusyTimeout opens a file database // TestDefaultDBURLSetsWALAndBusyTimeout opens a file database
// with the default URL's parameters the way the server does, // with the default URL's parameters the way the server does,
// and checks the journal mode and busy timeout it ends up with. // and checks the journal mode and busy timeout it ends up with.
@@ -73,3 +88,191 @@ func TestDefaultDBURLSetsWALAndBusyTimeout(t *testing.T) {
) )
} }
} }
// TestForeignKeysOnAfterCancelledQuery cancels a query 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 test then
// checks that the file database keeps its tables and that the new
// connection has foreign keys on.
func TestForeignKeysOnAfterCancelledQuery(t *testing.T) {
t.Parallel()
_, defaultParams, _ := strings.Cut(config.DefaultDBURL, "?")
urlParams := map[string]string{
"default parameters": "?" + defaultParams,
"no parameters": "",
}
for name, params := range urlParams {
t.Run(name, func(t *testing.T) {
t.Parallel()
dbURL := "file://" +
filepath.Join(t.TempDir(), "state.db") + params
database, err := startDatabase(t, dbURL)
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")
}
migrations := queryInt(
t, database, "SELECT COUNT(*) FROM schema_migrations",
)
if migrations == 0 {
t.Error("schema_migrations is empty")
}
foreignKeys := queryInt(t, database, "PRAGMA foreign_keys")
if foreignKeys != 1 {
t.Errorf("foreign_keys = %d, want 1", foreignKeys)
}
})
}
}
// 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.
func TestDBURLSettingForeignKeysFailsStartup(t *testing.T) {
t.Parallel()
pragmas := []string{
"foreign_keys(0)",
"foreign_keys(off)",
"foreign_keys=0",
"FOREIGN_KEYS(FALSE)",
" Foreign_Keys = 1",
}
for _, pragma := range pragmas {
t.Run(pragma, func(t *testing.T) {
t.Parallel()
dbURL := "file://" +
filepath.Join(t.TempDir(), "state.db") +
"?_pragma=" + url.QueryEscape(pragma)
_, err := startDatabase(t, dbURL)
if err == nil {
t.Fatal("the database started")
}
if !strings.Contains(err.Error(), "DBURL") {
t.Errorf("error %q does not name DBURL", err)
}
})
}
}
// startDatabase starts a Database for dbURL the way the server does.
// It returns the error that stopped it from starting, if any.
func startDatabase(t *testing.T, dbURL string) (*db.Database, error) {
t.Helper()
lifecycle := fxtest.NewLifecycle(t)
log, err := logger.New(lifecycle, logger.Params{
Globals: &globals.Globals{},
})
if err != nil {
t.Fatal(err)
}
database, err := db.New(lifecycle, db.Params{
Logger: log,
Config: &config.Config{DBURL: dbURL},
})
if err != nil {
t.Fatal(err)
}
t.Cleanup(lifecycle.RequireStop)
return database, lifecycle.Start(t.Context())
}
// queryInt runs a query that returns a single number.
func queryInt(t *testing.T, database *db.Database, query string) int {
t.Helper()
var result int
err := database.GetDB().QueryRowContext(t.Context(), query).Scan(&result)
if err != nil {
t.Fatal(err)
}
return result
}
+6 -11
View File
@@ -5,21 +5,16 @@ import (
"database/sql" "database/sql"
"fmt" "fmt"
"log/slog" "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. // 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) conn, err := sql.Open("sqlite", dsn)
if err != nil { if err != nil {
+1 -1
View File
@@ -13,7 +13,7 @@ import (
func setupTestDB(t *testing.T) *db.Database { func setupTestDB(t *testing.T) *db.Database {
t.Helper() t.Helper()
database, err := db.NewTestDatabase() database, err := db.NewTestDatabase(t)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
+39
View File
@@ -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
}
+2 -8
View File
@@ -3,7 +3,6 @@ package handlers_test
import ( import (
"bytes" "bytes"
"context" "context"
"crypto/rand"
"database/sql" "database/sql"
"encoding/json" "encoding/json"
"errors" "errors"
@@ -28,6 +27,7 @@ import (
api "sneak.berlin/go/neoirc/internal/cli/api" api "sneak.berlin/go/neoirc/internal/cli/api"
"sneak.berlin/go/neoirc/internal/config" "sneak.berlin/go/neoirc/internal/config"
"sneak.berlin/go/neoirc/internal/db" "sneak.berlin/go/neoirc/internal/db"
"sneak.berlin/go/neoirc/internal/dbtest"
"sneak.berlin/go/neoirc/internal/globals" "sneak.berlin/go/neoirc/internal/globals"
"sneak.berlin/go/neoirc/internal/handlers" "sneak.berlin/go/neoirc/internal/handlers"
"sneak.berlin/go/neoirc/internal/hashcash" "sneak.berlin/go/neoirc/internal/hashcash"
@@ -164,13 +164,7 @@ func newTestServerWith(
viperMu.Lock() viperMu.Lock()
defer viperMu.Unlock() 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) copyTemplateDB(t, dbURL)
var ( var (
+5 -24
View File
@@ -3,7 +3,6 @@ package ircserver_test
import ( import (
"bufio" "bufio"
"context" "context"
"crypto/rand"
"database/sql" "database/sql"
"errors" "errors"
"fmt" "fmt"
@@ -20,6 +19,7 @@ import (
"sneak.berlin/go/neoirc/internal/broker" "sneak.berlin/go/neoirc/internal/broker"
"sneak.berlin/go/neoirc/internal/config" "sneak.berlin/go/neoirc/internal/config"
"sneak.berlin/go/neoirc/internal/db" "sneak.berlin/go/neoirc/internal/db"
"sneak.berlin/go/neoirc/internal/dbtest"
"sneak.berlin/go/neoirc/internal/globals" "sneak.berlin/go/neoirc/internal/globals"
"sneak.berlin/go/neoirc/internal/ircserver" "sneak.berlin/go/neoirc/internal/ircserver"
"sneak.berlin/go/neoirc/internal/logger" "sneak.berlin/go/neoirc/internal/logger"
@@ -107,14 +107,7 @@ func newTestEnvWithResolver(
) *testEnv { ) *testEnv {
t.Helper() 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 { if err != nil {
t.Fatalf("open db: %v", err) t.Fatalf("open db: %v", err)
} }
@@ -144,20 +137,6 @@ func newTestEnvWithResolver(
brk := broker.New() brk := broker.New()
var listenConfig net.ListenConfig
listener, err := listenConfig.Listen(t.Context(), "tcp", "127.0.0.1:0")
if err != nil {
t.Fatalf("listen: %v", err)
}
addr := listener.Addr().String()
err = listener.Close()
if err != nil {
t.Fatalf("close listener: %v", err)
}
log := slog.New(slog.NewTextHandler( log := slog.New(slog.NewTextHandler(
os.Stderr, os.Stderr,
&slog.HandlerOptions{Level: slog.LevelError}, //nolint:exhaustruct &slog.HandlerOptions{Level: slog.LevelError}, //nolint:exhaustruct
@@ -167,7 +146,9 @@ func newTestEnvWithResolver(
log, cfg, database, brk, interval, deadline, resolve, log, cfg, database, brk, interval, deadline, resolve,
) )
err = srv.Start(addr) // Port 0 makes the system pick a free port. dial reads it
// back from the server's listener.
err = srv.Start("127.0.0.1:0")
if err != nil { if err != nil {
t.Fatalf("start irc server: %v", err) t.Fatalf("start irc server: %v", err)
} }
+6
View File
@@ -15,6 +15,12 @@ func NewTestSPAServer() *Server {
return srv return srv
} }
// Listener returns the listener the start hook bound for the
// HTTP server.
func (srv *Server) Listener() net.Listener {
return srv.listener
}
// ServeUntilShutdown runs serveUntilShutdown, which serves the // ServeUntilShutdown runs serveUntilShutdown, which serves the
// HTTP server on listener, and returns when it does. // HTTP server on listener, and returns when it does.
func (srv *Server) ServeUntilShutdown(listener net.Listener) { func (srv *Server) ServeUntilShutdown(listener net.Listener) {
+3
View File
@@ -50,6 +50,7 @@ type Server struct {
sentryEnabled bool sentryEnabled bool
log *slog.Logger log *slog.Logger
httpServer *http.Server httpServer *http.Server
listener net.Listener
router *chi.Mux router *chi.Mux
params Params params Params
mw *middleware.Middleware mw *middleware.Middleware
@@ -179,6 +180,8 @@ func (srv *Server) startHTTP(ctx context.Context) error {
return fmt.Errorf("http listen: %w", err) return fmt.Errorf("http listen: %w", err)
} }
srv.listener = listener
srv.httpServer = &http.Server{ //nolint:exhaustruct // optional fields srv.httpServer = &http.Server{ //nolint:exhaustruct // optional fields
Addr: listenAddr, Addr: listenAddr,
ReadTimeout: httpReadTimeout, ReadTimeout: httpReadTimeout,
+15 -48
View File
@@ -5,7 +5,6 @@
package server_test package server_test
import ( import (
"crypto/rand"
"fmt" "fmt"
"net" "net"
"net/http" "net/http"
@@ -19,6 +18,7 @@ import (
"sneak.berlin/go/neoirc/internal/broker" "sneak.berlin/go/neoirc/internal/broker"
"sneak.berlin/go/neoirc/internal/config" "sneak.berlin/go/neoirc/internal/config"
"sneak.berlin/go/neoirc/internal/db" "sneak.berlin/go/neoirc/internal/db"
"sneak.berlin/go/neoirc/internal/dbtest"
"sneak.berlin/go/neoirc/internal/globals" "sneak.berlin/go/neoirc/internal/globals"
"sneak.berlin/go/neoirc/internal/handlers" "sneak.berlin/go/neoirc/internal/handlers"
"sneak.berlin/go/neoirc/internal/healthcheck" "sneak.berlin/go/neoirc/internal/healthcheck"
@@ -45,10 +45,9 @@ const (
// for a long poll the HTTP server is serving to finish, and // for a long poll the HTTP server is serving to finish, and
// that the poll gets its response. // that the poll gets its response.
func TestStopWaitsForRequestInProgress(t *testing.T) { func TestStopWaitsForRequestInProgress(t *testing.T) {
port := freePort(t) var srv *server.Server
addr := net.JoinHostPort("127.0.0.1", strconv.Itoa(port))
app, brk, database := startApp(t, port) app, brk, database := startApp(t, fx.Populate(&srv))
sessionID, _, token, err := database.CreateSession( sessionID, _, token, err := database.CreateSession(
t.Context(), "poller", "poller", "", "", t.Context(), "poller", "poller", "", "",
@@ -57,9 +56,12 @@ func TestStopWaitsForRequestInProgress(t *testing.T) {
t.Fatalf("create session: %v", err) t.Fatalf("create session: %v", err)
} }
waitUntil(t, "the HTTP server to listen", func() bool { tcpAddr, ok := srv.Listener().Addr().(*net.TCPAddr)
return canConnect(t, addr) if !ok {
}) t.Fatalf("listener address %v is not TCP", srv.Listener().Addr())
}
addr := net.JoinHostPort("127.0.0.1", strconv.Itoa(tcpAddr.Port))
statuses := make(chan int, 1) statuses := make(chan int, 1)
@@ -145,7 +147,7 @@ func TestStartFailsWhenHTTPPortIsTaken(t *testing.T) {
func TestServeFailureStopsApp(t *testing.T) { func TestServeFailureStopsApp(t *testing.T) {
var srv *server.Server var srv *server.Server
app, _, _ := startApp(t, freePort(t), fx.Populate(&srv)) app, _, _ := startApp(t, fx.Populate(&srv))
shutdowns := app.Wait() shutdowns := app.Wait()
@@ -168,7 +170,7 @@ func TestServeFailureStopsApp(t *testing.T) {
func TestServeAfterStopDoesNotStopApp(t *testing.T) { func TestServeAfterStopDoesNotStopApp(t *testing.T) {
var srv *server.Server var srv *server.Server
app, _, _ := startApp(t, freePort(t), fx.Populate(&srv)) app, _, _ := startApp(t, fx.Populate(&srv))
shutdowns := app.Wait() shutdowns := app.Wait()
@@ -186,16 +188,15 @@ func TestServeAfterStopDoesNotStopApp(t *testing.T) {
} }
} }
// startApp starts the app newApp builds, with the HTTP server // startApp starts the app newApp builds, with the HTTP port
// listening on port. // set to 0, so the system gives the server a free port.
func startApp( func startApp(
t *testing.T, t *testing.T,
port int,
options ...fx.Option, options ...fx.Option,
) (*fxtest.App, *broker.Broker, *db.Database) { ) (*fxtest.App, *broker.Broker, *db.Database) {
t.Helper() t.Helper()
app, brk, database := newApp(t, port, options...) app, brk, database := newApp(t, 0, options...)
app.RequireStart() app.RequireStart()
@@ -241,10 +242,7 @@ func newApp(
stats.New, stats.New,
), ),
fx.Decorate(func(cfg *config.Config) *config.Config { 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 cfg.Port = port
return cfg return cfg
@@ -257,21 +255,6 @@ func newApp(
return app, brk, database return app, brk, database
} }
// freePort returns a TCP port that nothing was listening on a
// moment ago.
func freePort(t *testing.T) int {
t.Helper()
listener := closedListener(t)
tcpAddr, ok := listener.Addr().(*net.TCPAddr)
if !ok {
t.Fatalf("listener address %v is not TCP", listener.Addr())
}
return tcpAddr.Port
}
// closedListener returns a TCP listener that has already been // closedListener returns a TCP listener that has already been
// closed, so that accepting a connection on it fails. // closed, so that accepting a connection on it fails.
func closedListener(t *testing.T) net.Listener { func closedListener(t *testing.T) net.Listener {
@@ -294,22 +277,6 @@ func closedListener(t *testing.T) net.Listener {
return listener return listener
} }
// canConnect reports whether addr accepts a TCP connection.
func canConnect(t *testing.T, addr string) bool {
t.Helper()
var dialer net.Dialer
conn, err := dialer.DialContext(t.Context(), "tcp", addr)
if err != nil {
return false
}
_ = conn.Close()
return true
}
// waitUntil calls done every checkStep until it returns true, // waitUntil calls done every checkStep until it returns true,
// and fails the test if it does not within testTimeout. // and fails the test if it does not within testTimeout.
func waitUntil(t *testing.T, what string, done func() bool) { func waitUntil(t *testing.T, what string, done func() bool) {
+2 -8
View File
@@ -6,7 +6,6 @@ package service_test
import ( import (
"context" "context"
"crypto/rand"
"encoding/json" "encoding/json"
"errors" "errors"
"fmt" "fmt"
@@ -23,6 +22,7 @@ import (
"sneak.berlin/go/neoirc/internal/broker" "sneak.berlin/go/neoirc/internal/broker"
"sneak.berlin/go/neoirc/internal/config" "sneak.berlin/go/neoirc/internal/config"
"sneak.berlin/go/neoirc/internal/db" "sneak.berlin/go/neoirc/internal/db"
"sneak.berlin/go/neoirc/internal/dbtest"
"sneak.berlin/go/neoirc/internal/globals" "sneak.berlin/go/neoirc/internal/globals"
"sneak.berlin/go/neoirc/internal/hashcash" "sneak.berlin/go/neoirc/internal/hashcash"
"sneak.berlin/go/neoirc/internal/logger" "sneak.berlin/go/neoirc/internal/logger"
@@ -46,13 +46,7 @@ type testEnv struct {
func newTestEnv(t *testing.T) *testEnv { func newTestEnv(t *testing.T) *testEnv {
t.Helper() 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 ( var (
database *db.Database database *db.Database