Fix bd-1: Prevent test database pollution

Tests were connecting to test daemon but daemon routed to production DB via
findDatabaseForCwd(). Fixed by ensuring tests use isolated .beads directories
and change working directory to tmpDir.

Changes:
- bench_test.go: Added .beads subdir, chdir, and client.dbPath to setupBenchServer
- bench_test.go: Set dbPath for goroutine clients in BenchmarkConcurrentAgents
- comments_test.go: Refactored to use setupTestServer
- version_test.go: Fixed 4 tests to use setupTestServerIsolated with proper isolation
- rpc_test.go: Added setupTestServerIsolated() helper for custom test setup

Verified: RPC test suite runs with no database pollution (151→151 issues)
Amp-Thread-ID: https://ampcode.com/threads/T-348b7ba8-4292-4ed3-b143-0ad07d226c21
Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
Steve Yegge
2025-10-22 00:42:50 -07:00
parent 78e8cb914c
commit 97e5df1180
5 changed files with 301 additions and 128 deletions

View File

@@ -49,7 +49,7 @@ func BenchmarkDirectCreate(b *testing.B) {
// BenchmarkDaemonCreate benchmarks RPC create operations
func BenchmarkDaemonCreate(b *testing.B) {
_, client, cleanup := setupBenchServer(b)
_, client, cleanup, _ := setupBenchServer(b)
defer cleanup()
b.ResetTimer()
@@ -107,7 +107,7 @@ func BenchmarkDirectUpdate(b *testing.B) {
// BenchmarkDaemonUpdate benchmarks RPC update operations
func BenchmarkDaemonUpdate(b *testing.B) {
_, client, cleanup := setupBenchServer(b)
_, client, cleanup, _ := setupBenchServer(b)
defer cleanup()
createArgs := &CreateArgs{
@@ -181,7 +181,7 @@ func BenchmarkDirectList(b *testing.B) {
// BenchmarkDaemonList benchmarks RPC list operations
func BenchmarkDaemonList(b *testing.B) {
_, client, cleanup := setupBenchServer(b)
_, client, cleanup, _ := setupBenchServer(b)
defer cleanup()
for i := 0; i < 100; i++ {
@@ -207,7 +207,7 @@ func BenchmarkDaemonList(b *testing.B) {
// BenchmarkDaemonLatency measures round-trip latency
func BenchmarkDaemonLatency(b *testing.B) {
_, client, cleanup := setupBenchServer(b)
_, client, cleanup, _ := setupBenchServer(b)
defer cleanup()
b.ResetTimer()
@@ -220,7 +220,7 @@ func BenchmarkDaemonLatency(b *testing.B) {
// BenchmarkConcurrentAgents benchmarks concurrent agent throughput
func BenchmarkConcurrentAgents(b *testing.B) {
server, _, cleanup := setupBenchServer(b)
server, _, cleanup, dbPath := setupBenchServer(b)
defer cleanup()
numAgents := 4
@@ -239,6 +239,9 @@ func BenchmarkConcurrentAgents(b *testing.B) {
}
defer client.Close()
// Set dbPath so client validates it's connected to the right daemon
client.dbPath = dbPath
for j := 0; j < opsPerAgent; j++ {
args := &CreateArgs{
Title: fmt.Sprintf("Issue %d", j),
@@ -260,14 +263,21 @@ func BenchmarkConcurrentAgents(b *testing.B) {
}
}
func setupBenchServer(b *testing.B) (*Server, *Client, func()) {
func setupBenchServer(b *testing.B) (*Server, *Client, func(), string) {
tmpDir, err := os.MkdirTemp("", "bd-rpc-bench-*")
if err != nil {
b.Fatalf("Failed to create temp dir: %v", err)
}
dbPath := filepath.Join(tmpDir, "test.db")
socketPath := filepath.Join(tmpDir, "bd.sock")
// Create .beads subdirectory so findDatabaseForCwd finds THIS database, not project's
beadsDir := filepath.Join(tmpDir, ".beads")
if err := os.MkdirAll(beadsDir, 0755); err != nil {
os.RemoveAll(tmpDir)
b.Fatalf("Failed to create .beads dir: %v", err)
}
dbPath := filepath.Join(beadsDir, "test.db")
socketPath := filepath.Join(beadsDir, "bd.sock")
store, err := sqlitestorage.New(dbPath)
if err != nil {
@@ -286,22 +296,44 @@ func setupBenchServer(b *testing.B) (*Server, *Client, func()) {
time.Sleep(100 * time.Millisecond)
client, err := TryConnect(socketPath)
// Change to tmpDir so client's os.Getwd() finds the test database
originalWd, err := os.Getwd()
if err != nil {
cancel()
server.Stop()
store.Close()
os.RemoveAll(tmpDir)
b.Fatalf("Failed to get working directory: %v", err)
}
if err := os.Chdir(tmpDir); err != nil {
cancel()
server.Stop()
store.Close()
os.RemoveAll(tmpDir)
b.Fatalf("Failed to change directory: %v", err)
}
client, err := TryConnect(socketPath)
if err != nil {
cancel()
server.Stop()
store.Close()
os.Chdir(originalWd)
os.RemoveAll(tmpDir)
b.Fatalf("Failed to connect client: %v", err)
}
// Set the client's dbPath to the test database so it doesn't route to the wrong DB
client.dbPath = dbPath
cleanup := func() {
client.Close()
cancel()
server.Stop()
store.Close()
os.Chdir(originalWd) // Restore original working directory
os.RemoveAll(tmpDir)
}
return server, client, cleanup
return server, client, cleanup, dbPath
}

View File

@@ -1,51 +1,15 @@
package rpc
import (
"context"
"encoding/json"
"path/filepath"
"testing"
"time"
sqlitestorage "github.com/steveyegge/beads/internal/storage/sqlite"
"github.com/steveyegge/beads/internal/types"
)
func TestCommentOperationsViaRPC(t *testing.T) {
tmpDir := t.TempDir()
dbPath := filepath.Join(tmpDir, "test.db")
socketPath := filepath.Join(tmpDir, "bd.sock")
store, err := sqlitestorage.New(dbPath)
if err != nil {
t.Fatalf("failed to create store: %v", err)
}
defer store.Close()
server := NewServer(socketPath, store)
ctx, cancel := context.WithCancel(context.Background())
serverErr := make(chan error, 1)
go func() {
serverErr <- server.Start(ctx)
}()
select {
case <-server.WaitReady():
case err := <-serverErr:
t.Fatalf("server failed to start: %v", err)
case <-time.After(2 * time.Second):
t.Fatal("timeout waiting for server to start")
}
client, err := TryConnect(socketPath)
if err != nil {
t.Fatalf("failed to connect to server: %v", err)
}
if client == nil {
t.Fatal("client is nil after successful connection")
}
defer client.Close()
_, client, cleanup := setupTestServer(t)
defer cleanup()
createResp, err := client.Create(&CreateArgs{
Title: "Comment test",
@@ -98,16 +62,4 @@ func TestCommentOperationsViaRPC(t *testing.T) {
if comments[0].Text != "first comment" {
t.Fatalf("expected comment text 'first comment', got %q", comments[0].Text)
}
if err := server.Stop(); err != nil {
t.Fatalf("failed to stop server: %v", err)
}
cancel()
select {
case err := <-serverErr:
if err != nil && err != context.Canceled {
t.Fatalf("server returned error: %v", err)
}
default:
}
}

View File

@@ -112,6 +112,36 @@ func setupTestServer(t *testing.T) (*Server, *Client, func()) {
return server, client, cleanup
}
// setupTestServerIsolated creates an isolated test server in a temp directory
// with .beads structure, but allows the caller to customize server/client setup.
// Returns tmpDir, beadsDir, dbPath, socketPath, and cleanup function.
// Caller must change to tmpDir if needed and set client.dbPath manually.
func setupTestServerIsolated(t *testing.T) (tmpDir, beadsDir, dbPath, socketPath string, cleanup func()) {
tmpDir, err := os.MkdirTemp("", "bd-rpc-test-*")
if err != nil {
t.Fatalf("Failed to create temp dir: %v", err)
}
// Create .beads subdirectory so findDatabaseForCwd finds THIS database, not project's
beadsDir = filepath.Join(tmpDir, ".beads")
if err := os.MkdirAll(beadsDir, 0755); err != nil {
os.RemoveAll(tmpDir)
t.Fatalf("Failed to create .beads dir: %v", err)
}
dbPath = filepath.Join(beadsDir, "test.db")
socketPath = filepath.Join(beadsDir, "bd.sock")
// Ensure socket doesn't exist from previous failed test
os.Remove(socketPath)
cleanup = func() {
os.RemoveAll(tmpDir)
}
return tmpDir, beadsDir, dbPath, socketPath, cleanup
}
func TestPing(t *testing.T) {
_, client, cleanup := setupTestServer(t)
defer cleanup()

View File

@@ -4,7 +4,6 @@ import (
"context"
"encoding/json"
"os"
"path/filepath"
"testing"
"time"
@@ -80,15 +79,9 @@ func TestVersionCompatibility(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Setup temp environment
tmpDir, err := os.MkdirTemp("", "bd-version-test-*")
if err != nil {
t.Fatalf("Failed to create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)
dbPath := filepath.Join(tmpDir, "test.db")
socketPath := filepath.Join(tmpDir, "bd.sock")
// Setup isolated test environment
tmpDir, _, dbPath, socketPath, cleanup := setupTestServerIsolated(t)
defer cleanup()
store, err := sqlitestorage.New(dbPath)
if err != nil {
@@ -118,6 +111,16 @@ func TestVersionCompatibility(t *testing.T) {
ClientVersion = tt.clientVersion
defer func() { ClientVersion = originalClientVersion }()
// Change to tmpDir so client's os.Getwd() finds the test database
originalWd, err := os.Getwd()
if err != nil {
t.Fatalf("Failed to get working directory: %v", err)
}
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("Failed to change directory: %v", err)
}
defer os.Chdir(originalWd)
client, err := TryConnect(socketPath)
if err != nil {
t.Fatalf("Failed to connect: %v", err)
@@ -127,6 +130,9 @@ func TestVersionCompatibility(t *testing.T) {
}
defer client.Close()
// Set dbPath so client validates it's connected to the right daemon
client.dbPath = dbPath
// Try to create an issue (this triggers version check)
args := &CreateArgs{
Title: "Version test issue",
@@ -166,14 +172,8 @@ func TestVersionCompatibility(t *testing.T) {
}
func TestHealthCheckIncludesVersionInfo(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "bd-health-version-*")
if err != nil {
t.Fatalf("Failed to create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)
dbPath := filepath.Join(tmpDir, "test.db")
socketPath := filepath.Join(tmpDir, "bd.sock")
tmpDir, _, dbPath, socketPath, cleanup := setupTestServerIsolated(t)
defer cleanup()
store, err := sqlitestorage.New(dbPath)
if err != nil {
@@ -196,12 +196,25 @@ func TestHealthCheckIncludesVersionInfo(t *testing.T) {
time.Sleep(100 * time.Millisecond)
// Change to tmpDir so client's os.Getwd() finds the test database
originalWd, err := os.Getwd()
if err != nil {
t.Fatalf("Failed to get working directory: %v", err)
}
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("Failed to change directory: %v", err)
}
defer os.Chdir(originalWd)
client, err := TryConnect(socketPath)
if err != nil {
t.Fatalf("Failed to connect: %v", err)
}
defer client.Close()
// Set dbPath so client validates it's connected to the right daemon
client.dbPath = dbPath
health, err := client.Health()
if err != nil {
t.Fatalf("Health check failed: %v", err)
@@ -223,14 +236,8 @@ func TestHealthCheckIncludesVersionInfo(t *testing.T) {
}
func TestIncompatibleVersionInHealth(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "bd-health-incompatible-*")
if err != nil {
t.Fatalf("Failed to create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)
dbPath := filepath.Join(tmpDir, "test.db")
socketPath := filepath.Join(tmpDir, "bd.sock")
tmpDir, _, dbPath, socketPath, cleanup := setupTestServerIsolated(t)
defer cleanup()
store, err := sqlitestorage.New(dbPath)
if err != nil {
@@ -253,12 +260,25 @@ func TestIncompatibleVersionInHealth(t *testing.T) {
time.Sleep(100 * time.Millisecond)
// Change to tmpDir so client's os.Getwd() finds the test database
originalWd, err := os.Getwd()
if err != nil {
t.Fatalf("Failed to get working directory: %v", err)
}
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("Failed to change directory: %v", err)
}
defer os.Chdir(originalWd)
client, err := TryConnect(socketPath)
if err != nil {
t.Fatalf("Failed to connect: %v", err)
}
defer client.Close()
// Set dbPath so client validates it's connected to the right daemon
client.dbPath = dbPath
// Health check should succeed but report incompatible
health, err := client.Health()
if err != nil {
@@ -336,14 +356,8 @@ func TestVersionCheckMessage(t *testing.T) {
}
func TestPingAndHealthBypassVersionCheck(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "bd-bypass-version-*")
if err != nil {
t.Fatalf("Failed to create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)
dbPath := filepath.Join(tmpDir, "test.db")
socketPath := filepath.Join(tmpDir, "bd.sock")
tmpDir, _, dbPath, socketPath, cleanup := setupTestServerIsolated(t)
defer cleanup()
store, err := sqlitestorage.New(dbPath)
if err != nil {
@@ -366,12 +380,25 @@ func TestPingAndHealthBypassVersionCheck(t *testing.T) {
time.Sleep(100 * time.Millisecond)
// Change to tmpDir so client's os.Getwd() finds the test database
originalWd, err := os.Getwd()
if err != nil {
t.Fatalf("Failed to get working directory: %v", err)
}
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("Failed to change directory: %v", err)
}
defer os.Chdir(originalWd)
client, err := TryConnect(socketPath)
if err != nil {
t.Fatalf("Failed to connect: %v", err)
}
defer client.Close()
// Set dbPath so client validates it's connected to the right daemon
client.dbPath = dbPath
// Ping should work despite version mismatch
if err := client.Ping(); err != nil {
t.Errorf("Ping should work despite version mismatch, got: %v", err)
@@ -404,14 +431,8 @@ func TestPingAndHealthBypassVersionCheck(t *testing.T) {
}
func TestMetricsOperation(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "bd-metrics-test-*")
if err != nil {
t.Fatalf("Failed to create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)
dbPath := filepath.Join(tmpDir, "test.db")
socketPath := filepath.Join(tmpDir, "bd.sock")
tmpDir, _, dbPath, socketPath, cleanup := setupTestServerIsolated(t)
defer cleanup()
store, err := sqlitestorage.New(dbPath)
if err != nil {
@@ -433,12 +454,25 @@ func TestMetricsOperation(t *testing.T) {
time.Sleep(100 * time.Millisecond)
// Change to tmpDir so client's os.Getwd() finds the test database
originalWd, err := os.Getwd()
if err != nil {
t.Fatalf("Failed to get working directory: %v", err)
}
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("Failed to change directory: %v", err)
}
defer os.Chdir(originalWd)
client, err := TryConnect(socketPath)
if err != nil {
t.Fatalf("Failed to connect: %v", err)
}
defer client.Close()
// Set dbPath so client validates it's connected to the right daemon
client.dbPath = dbPath
metrics, err := client.Metrics()
if err != nil {
t.Fatalf("Metrics call failed: %v", err)