sneak/chat
1
0
Fork 0
Code Issues 5 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

fix: use in-memory SQLite for handler tests to fix CI timeout #93

Merged
sneak merged 1 commits from fix/test-timeout-90 into main 2026-03-25 20:11:34 +01:00
Conversation 1 Commits 1 Files Changed 1 +18 -67
clawbot commented 2026-03-25 02:35:17 +01:00
Collaborator
Copy Link

Summary

Fixes the CI build failure caused by the internal/handlers test package exceeding the 30-second per-package timeout on the x86_64 CI runner.

Root Cause

Each of the 104 handler tests was creating a file-backed SQLite database in a temp directory with WAL journaling. On slower CI runners (x86_64 ubuntu-latest), the cumulative filesystem I/O overhead for 104 DB create + migrate + teardown cycles pushed the package well past the 30s timeout.

Fix

  1. In-memory SQLite — Switch test databases from file:<tmpdir>/test.db?_journal_mode=WAL&_busy_timeout=5000 to file:test_<ptr>?mode=memory&cache=shared. Each test still gets its own isolated database (unique name per *testing.T pointer), but without filesystem I/O.

  2. Consolidated test server constructors — Merged the duplicate newTestServer() and newTestServerWithOper() setup code into a shared newTestServerWith() helper, removing ~50 lines of duplication.

Results

Environment Before After
ARM native (no race) ~4.5s ~2.0s
ARM native (with race) ~11.5s ~8.7s
Docker ARM (with race+cover) ~20.4s ~10.0s

The Docker ARM time is the closest proxy for CI. With the ~2x overhead of x86_64 emulation on CI, the estimated CI time is ~20s — well within the 30s timeout.

What This Does NOT Change

  • No test assertions modified
  • No tests skipped or removed
  • No linter config changes
  • No Makefile changes
  • No CI config changes
  • All 104 handler tests still run with full isolation

closes #90

## Summary Fixes the CI build failure caused by the `internal/handlers` test package exceeding the 30-second per-package timeout on the x86_64 CI runner. ## Root Cause Each of the 104 handler tests was creating a **file-backed SQLite database** in a temp directory with WAL journaling. On slower CI runners (x86_64 ubuntu-latest), the cumulative filesystem I/O overhead for 104 DB create + migrate + teardown cycles pushed the package well past the 30s timeout. ## Fix 1. **In-memory SQLite** — Switch test databases from `file:<tmpdir>/test.db?_journal_mode=WAL&_busy_timeout=5000` to `file:test_<ptr>?mode=memory&cache=shared`. Each test still gets its own isolated database (unique name per `*testing.T` pointer), but without filesystem I/O. 2. **Consolidated test server constructors** — Merged the duplicate `newTestServer()` and `newTestServerWithOper()` setup code into a shared `newTestServerWith()` helper, removing ~50 lines of duplication. ## Results | Environment | Before | After | |---|---|---| | ARM native (no race) | ~4.5s | ~2.0s | | ARM native (with race) | ~11.5s | ~8.7s | | Docker ARM (with race+cover) | **~20.4s** | **~10.0s** | The Docker ARM time is the closest proxy for CI. With the ~2x overhead of x86_64 emulation on CI, the estimated CI time is ~20s — well within the 30s timeout. ## What This Does NOT Change - No test assertions modified - No tests skipped or removed - No linter config changes - No Makefile changes - No CI config changes - All 104 handler tests still run with full isolation closes https://git.eeqj.de/sneak/chat/issues/90
clawbot added 1 commit 2026-03-25 02:35:17 +01:00
fix: use in-memory SQLite for handler tests to fix CI timeout
All checks were successful
check / check (push) Successful in 1m5s
Details
13b1d083cf 
Switch handler test databases from file-backed SQLite (with
WAL mode and temp dir) to in-memory SQLite. This eliminates
filesystem I/O overhead per test, cutting handler test time
roughly in half.

Before: ~20s in Docker (ARM), exceeding 30s timeout on CI
After:  ~10s in Docker (ARM), well within 30s timeout

Also consolidate the two test server constructors
(newTestServer and newTestServerWithOper) into a shared
newTestServerWith helper, removing ~50 lines of duplicate
setup code.

closes #90
clawbot added the needs-review label 2026-03-25 02:35:44 +01:00
clawbot reviewed 2026-03-25 02:39:17 +01:00
clawbot left a comment
Author
Collaborator
Copy Link

Review: PASS ✅

PR #93 — fix: use in-memory SQLite for handler tests to fix CI timeout
Closes #90 — critical: main branch build broken

Checklist

  • No test assertions modified or weakened — All 104 func Test* functions preserved. All assertion calls (t.Fatal, t.Fatalf, t.Error, require.*) unchanged. The only removed fmt.Errorf was in the deleted duplicate constructor, not a test assertion.
  • No tests skipped or removed — 104 test functions on both main and this branch.
  • No linter/CI/Makefile config changes — Only file changed is internal/handlers/api_test.go. Makefile, .golangci.yml, .gitea/workflows/, and Dockerfile are untouched.
  • In-memory SQLite provides test isolation — Each test gets a unique DB via fmt.Sprintf("file:test_%p?mode=memory&cache=shared", t) where %p is the unique *testing.T pointer. No cross-test contamination possible.
  • docker build . passes with 30s timeout — Build succeeds. Timeout confirmed at go test -timeout 30s in Makefile (unchanged).
  • Handler tests actually faster — internal/handlers completed in 10.385s in Docker (ARM), well under the 30s timeout. Down from ~20s before.
  • No scope creep — Single file change. Constructor consolidation (newTestServerWith) is a clean deduplication that removes ~50 lines of copy-paste without changing behavior.

Behavioral equivalence verified

  • newTestServer(t) → newTestServerWith(t, 0, "", "") — matches old behavior (HashcashBits=0, empty oper creds = Go zero values)
  • newTestServerWithOper(t) → newTestServerWith(t, 0, testOperName, testOperPassword) — matches old behavior exactly

Build output (key lines)

ok  git.eeqj.de/sneak/neoirc/internal/handlers  10.385s  coverage: 70.0%
make fmt-check: PASS
make lint: 0 issues
docker build: SUCCESS

Clean fix. Addresses all three requirements from #90: confirmed timeout was the issue, timeout stays at 30s, tests complete well under 15s.

## Review: PASS ✅ **PR [#93](https://git.eeqj.de/sneak/chat/pulls/93)** — fix: use in-memory SQLite for handler tests to fix CI timeout **Closes [#90](https://git.eeqj.de/sneak/chat/issues/90)** — critical: main branch build broken ### Checklist - [x] **No test assertions modified or weakened** — All 104 `func Test*` functions preserved. All assertion calls (`t.Fatal`, `t.Fatalf`, `t.Error`, `require.*`) unchanged. The only removed `fmt.Errorf` was in the deleted duplicate constructor, not a test assertion. - [x] **No tests skipped or removed** — 104 test functions on both `main` and this branch. - [x] **No linter/CI/Makefile config changes** — Only file changed is `internal/handlers/api_test.go`. Makefile, `.golangci.yml`, `.gitea/workflows/`, and Dockerfile are untouched. - [x] **In-memory SQLite provides test isolation** — Each test gets a unique DB via `fmt.Sprintf("file:test_%p?mode=memory&cache=shared", t)` where `%p` is the unique `*testing.T` pointer. No cross-test contamination possible. - [x] **`docker build .` passes with 30s timeout** — Build succeeds. Timeout confirmed at `go test -timeout 30s` in Makefile (unchanged). - [x] **Handler tests actually faster** — `internal/handlers` completed in **10.385s** in Docker (ARM), well under the 30s timeout. Down from ~20s before. - [x] **No scope creep** — Single file change. Constructor consolidation (`newTestServerWith`) is a clean deduplication that removes ~50 lines of copy-paste without changing behavior. ### Behavioral equivalence verified - `newTestServer(t)` → `newTestServerWith(t, 0, "", "")` — matches old behavior (HashcashBits=0, empty oper creds = Go zero values) - `newTestServerWithOper(t)` → `newTestServerWith(t, 0, testOperName, testOperPassword)` — matches old behavior exactly ### Build output (key lines) ``` ok git.eeqj.de/sneak/neoirc/internal/handlers 10.385s coverage: 70.0% make fmt-check: PASS make lint: 0 issues docker build: SUCCESS ``` Clean fix. Addresses all three requirements from [#90](https://git.eeqj.de/sneak/chat/issues/90): confirmed timeout was the issue, timeout stays at 30s, tests complete well under 15s.
clawbot added merge-ready and removed needs-review labels 2026-03-25 02:39:43 +01:00
sneak was assigned by clawbot 2026-03-25 02:39:43 +01:00
sneak merged commit e62962d192 into main 2026-03-25 20:11:34 +01:00
sneak deleted branch fix/test-timeout-90 2026-03-25 20:11:34 +01:00
Sign in to join this conversation.
Reviewers
No Reviewers
clawbot
Labels
Clear labels
merge-ready
All checks pass, reviewed, rebased, ready for merge
 needs-checks
make check does not pass cleanly
 needs-rebase
PR has merge conflicts or is behind main
 needs-review
Code review not yet done
 needs-review
needs-rework
Code review found issues to fix
No Label merge-ready
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees sneak
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/chat#93
Delete Branch "fix/test-timeout-90"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?