Merge pull request #220 from BaaRai/fix/db-test-isolation

test(db): make profile and company-scope tests repeatable on one database

db/company_scope_test.go

@@ -2,6 +2,7 @@ package db
 
 import (
 	"fmt"
+	"math/rand/v2"
 	"testing"
 	"time"
 )
@@ -331,19 +332,36 @@ func TestCompanyScopeAttribution(t *testing.T) {
 		d.Close()
 		t.Fatalf("startMax: %v", err)
 	}
+	// Remove everything the test creates, or the next pass on the same database
+	// finds this company's scope and attributes its assets to it. Assets first
+	// (they reference the company), then the company; its scope rows cascade.
+	var cid int64
 	t.Cleanup(func() {
-		_, _ = d.Exec(`DELETE FROM assets WHERE id > $1`, startMax)
-		d.Close()
+		defer d.Close()
+		if _, err := d.Exec(`DELETE FROM assets WHERE id > $1`, startMax); err != nil {
+			t.Errorf("cleanup assets: %v", err)
+		}
+		if cid != 0 {
+			if _, err := d.Exec(`DELETE FROM companies WHERE id = $1`, cid); err != nil {
+				t.Errorf("cleanup company %d: %v", cid, err)
+			}
+		}
 	})
 
-	uniq := startMax + 1
-	root := fmt.Sprintf("scopetest%d.com", uniq)
+	// Unique per run: MAX(assets.id) comes back to the same value after cleanup,
+	// so it cannot tell one pass from the next. The in-scope /24 is picked per
+	// run from 198.18.0.0/15 so a scope left by an interrupted run cannot claim
+	// this run's IP.
+	uniq := fmt.Sprintf("%d%d", time.Now().UnixNano(), rand.Uint32())
+	n := rand.IntN(512)
+	cidr := fmt.Sprintf("198.%d.%d.0/24", 18+n/256, n%256)
+	root := fmt.Sprintf("scopetest%s.com", uniq)
 	sub := "api." + root
-	ipIn := "198.51.100.9"
+	ipIn := fmt.Sprintf("198.%d.%d.9", 18+n/256, n%256)
 	ipOut := "203.0.113.9"
-	outDomain := fmt.Sprintf("other%d.net", uniq)
+	outDomain := fmt.Sprintf("other%s.net", uniq)
 
-	cid, _, err := cs.UpsertCompany(fmt.Sprintf("ScopeCo %d", uniq), "")
+	cid, _, err = cs.UpsertCompany(fmt.Sprintf("ScopeCo %s", uniq), "")
 	if err != nil {
 		t.Fatalf("UpsertCompany: %v", err)
 	}
@@ -359,7 +377,7 @@ func TestCompanyScopeAttribution(t *testing.T) {
 		t.Fatalf("pre-scope asset should be unattributed, got %v", *preCompanyID)
 	}
 
-	cs.AddScope(cid, []string{root, "198.51.100.0/24"}, "unit test")
+	cs.AddScope(cid, []string{root, cidr}, "unit test")
 
 	mustCid := func(id int64, want int64, label string) {
 		var cID *int64

db/config_test.go

@@ -4,7 +4,9 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"testing"
+	"time"
 )
 
 // TestPoolProfilesOrder pins the failover chain query: keyless profiles can't
@@ -17,17 +19,22 @@ func TestPoolProfilesOrder(t *testing.T) {
 	if err != nil {
 		t.Skipf("postgres unavailable (%v) — skipping", err)
 	}
-	defer d.Close()
+	// Close via Cleanup, registered FIRST so it runs LAST: a plain defer would
+	// close the pool before the row-deleting cleanups below run.
+	t.Cleanup(func() { d.Close() })
 
+	// llm_profiles.name is unique, so a fixed name collides with a row left on
+	// the same database by an earlier run.
+	sfx := fmt.Sprintf("-%d", time.Now().UnixNano())
 	mk := func(name string, priority int, exclude bool, key string) int64 {
 		id, err := d.SaveProfile(&LLMProfile{
-			Name: name, Format: "openai", Model: "m", APIKey: key,
+			Name: name + sfx, Format: "openai", Model: "m", APIKey: key,
 			Priority: priority, PoolExclude: exclude,
 		})
 		if err != nil {
 			t.Fatal(err)
 		}
-		t.Cleanup(func() { d.Exec(`DELETE FROM llm_profiles WHERE id=$1`, id) })
+		t.Cleanup(func() { deleteTestProfile(t, d, id) })
 		return id
 	}
 	lo := mk("t-pool-lo", 1, false, "k1")
@@ -42,11 +49,11 @@ func TestPoolProfilesOrder(t *testing.T) {
 	var got []int64
 	for _, p := range chain {
 		switch p.Name {
-		case "t-pool-lo", "t-pool-hi":
+		case "t-pool-lo" + sfx, "t-pool-hi" + sfx:
 			got = append(got, p.ID)
-		case "t-pool-excluded":
+		case "t-pool-excluded" + sfx:
 			t.Fatal("pool_exclude profile entered the failover chain")
-		case "t-pool-nokey":
+		case "t-pool-nokey" + sfx:
 			t.Fatal("keyless profile entered the failover chain")
 		}
 	}
@@ -55,6 +62,15 @@ func TestPoolProfilesOrder(t *testing.T) {
 	}
 }
 
+// deleteTestProfile removes a profile a test created and reports a failed delete
+// instead of silently leaving the row for the next run.
+func deleteTestProfile(t *testing.T, d *DB, id int64) {
+	t.Helper()
+	if _, err := d.Exec(`DELETE FROM llm_profiles WHERE id=$1`, id); err != nil {
+		t.Errorf("cleanup profile %d: %v", id, err)
+	}
+}
+
 func TestDeleteProfileContextHonorsCancellation(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel()

db/llmretry_test.go

@@ -1,6 +1,7 @@
 package db
 
 import (
+	"fmt"
 	"testing"
 	"time"
 )
@@ -14,7 +15,8 @@ func TestProfileRetryRoundTrip(t *testing.T) {
 	if err != nil {
 		t.Skipf("postgres unavailable (%v) — skipping", err)
 	}
-	defer d.Close()
+	// Registered FIRST so it runs LAST, after the row cleanup below.
+	t.Cleanup(func() { d.Close() })
 
 	want := RetryOverride{
 		Connect: RetryRule{Attempts: 5, IntervalMS: 2000},
@@ -22,12 +24,12 @@ func TestProfileRetryRoundTrip(t *testing.T) {
 		Stream:  RetryRule{IntervalMS: 250},
 	}
 	id, err := d.SaveProfile(&LLMProfile{
-		Name: "t-retry-roundtrip", Format: "openai", Model: "m", APIKey: "k", Retry: want,
+		Name: fmt.Sprintf("t-retry-roundtrip-%d", time.Now().UnixNano()), Format: "openai", Model: "m", APIKey: "k", Retry: want,
 	})
 	if err != nil {
 		t.Fatal(err)
 	}
-	t.Cleanup(func() { d.Exec(`DELETE FROM llm_profiles WHERE id=$1`, id) })
+	t.Cleanup(func() { deleteTestProfile(t, d, id) })
 
 	got, err := d.ProfileByID(id)
 	if err != nil || got == nil {
@@ -71,16 +73,17 @@ func TestProfileRetryClampedOnSave(t *testing.T) {
 	if err != nil {
 		t.Skipf("postgres unavailable (%v) — skipping", err)
 	}
-	defer d.Close()
+	// Registered FIRST so it runs LAST, after the row cleanup below.
+	t.Cleanup(func() { d.Close() })
 
 	id, err := d.SaveProfile(&LLMProfile{
-		Name: "t-retry-clamp", Format: "openai", Model: "m", APIKey: "k",
+		Name: fmt.Sprintf("t-retry-clamp-%d", time.Now().UnixNano()), Format: "openai", Model: "m", APIKey: "k",
 		Retry: RetryOverride{Connect: RetryRule{Attempts: -99, IntervalMS: -5}},
 	})
 	if err != nil {
 		t.Fatal(err)
 	}
-	t.Cleanup(func() { d.Exec(`DELETE FROM llm_profiles WHERE id=$1`, id) })
+	t.Cleanup(func() { deleteTestProfile(t, d, id) })
 	got, err := d.ProfileByID(id)
 	if err != nil || got == nil {
 		t.Fatalf("ProfileByID: %v", err)
@@ -97,17 +100,22 @@ func TestLLMRetryPolicyRoundTrip(t *testing.T) {
 	if err != nil {
 		t.Skipf("postgres unavailable (%v) — skipping", err)
 	}
-	defer d.Close()
+	// Registered FIRST so it runs LAST, after the restore below.
+	t.Cleanup(func() { d.Close() })
 
 	before, hadBefore, err := d.GetSetting(settingLLMRetryPolicy)
 	if err != nil {
 		t.Fatal(err)
 	}
 	t.Cleanup(func() {
+		var err error
 		if hadBefore {
-			d.SetSetting(settingLLMRetryPolicy, before)
+			err = d.SetSetting(settingLLMRetryPolicy, before)
 		} else {
-			d.Exec(`DELETE FROM settings WHERE key=$1`, settingLLMRetryPolicy)
+			_, err = d.Exec(`DELETE FROM settings WHERE key=$1`, settingLLMRetryPolicy)
+		}
+		if err != nil {
+			t.Errorf("restore %s: %v", settingLLMRetryPolicy, err)
 		}
 	})