The live tests seeded a tenant per run and never took it back
Each live store test makes its own client - deliberately, so they can run in any order and so the isolation assertions have a real neighbour to be isolated from - and none of them removed it afterwards. The dev database had reached 242 abandoned tenants against the one real company. That is not untidy, it is a broken screen. The platform admin's Companies view lists every client, so the real company sat under pages of `walk1788761685056287000`, which is the first thing anyone opening tenant administration would see. dropTenant registers the cleanup against the CLIENT rather than each table: every foreign key onto clients is ON DELETE CASCADE, so one delete takes the sites, visitors, visits, face images, embeddings, cameras and agents with it. A per-table list would rot the first time a migration adds a table, and it would rot silently - the same shape as the leak it replaces. A failed cleanup calls t.Errorf rather than being ignored. A tenant left behind is precisely what this exists to prevent, and swallowing the error would let the leak come back with nothing to show for it. Verified against the live database: three consecutive runs of the store suite leave clients, sites and visits unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qiy5iKfz4L8S4vRaYPBdaU
This commit is contained in:
@@ -39,6 +39,38 @@ func liveStore(t *testing.T) *Store {
|
||||
return st
|
||||
}
|
||||
|
||||
// dropTenant removes a seeded tenant when the test that made it finishes.
|
||||
//
|
||||
// Without this these tests are a slow leak. Every one of them seeds its own
|
||||
// tenant - deliberately, so they can run in any order and so the isolation
|
||||
// assertions have a real neighbour - and none of them ever removed it. A dev
|
||||
// database reached 242 abandoned tenants against the single real one, which is
|
||||
// not merely untidy: the platform admin's Companies screen lists every client,
|
||||
// so the one real company was buried under pages of `walk1788761685056287000`.
|
||||
//
|
||||
// Registered against the CLIENT rather than each table because every foreign
|
||||
// key onto clients is ON DELETE CASCADE, so one delete takes the sites,
|
||||
// visitors, visits, face images, embeddings, cameras and agents with it. A
|
||||
// per-table list would rot the first time a migration adds a table, and it
|
||||
// would rot silently - which is the shape of the bug it is cleaning up after.
|
||||
//
|
||||
// t.Cleanup runs LIFO and liveStore registers st.Close before any seeding, so
|
||||
// the delete still has a live pool when it runs.
|
||||
func dropTenant(t *testing.T, st *Store, clientID string) {
|
||||
t.Helper()
|
||||
t.Cleanup(func() {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
||||
defer cancel()
|
||||
if _, err := st.pool.Exec(ctx,
|
||||
`DELETE FROM clients WHERE id = $1::uuid`, clientID); err != nil {
|
||||
// Reported rather than ignored: a tenant left behind is the very
|
||||
// thing this exists to prevent, and swallowing the error would let
|
||||
// the leak return with nothing to show for it.
|
||||
t.Errorf("cleanup tenant %s: %v", clientID, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// seedTenant builds a client, a site and n visits, and returns the client id.
|
||||
// Every test gets its own tenant so they can run in any order without a
|
||||
// truncate between them - and so the isolation assertions below have a real
|
||||
@@ -52,6 +84,7 @@ func seedTenant(t *testing.T, st *Store, name string, n int, withImages bool) (c
|
||||
if err != nil {
|
||||
t.Fatalf("seed client: %v", err)
|
||||
}
|
||||
dropTenant(t, st, clientID)
|
||||
err = st.pool.QueryRow(ctx, `
|
||||
INSERT INTO sites (client_id, name, slug) VALUES ($1::uuid, $2, $3)
|
||||
RETURNING id::text`, clientID, name+" Main", name+"-main").Scan(&siteID)
|
||||
|
||||
@@ -26,6 +26,7 @@ func seedAgentSite(t *testing.T, st *Store, name string) ingest.Site {
|
||||
name).Scan(&site.ClientID); err != nil {
|
||||
t.Fatalf("seed client: %v", err)
|
||||
}
|
||||
dropTenant(t, st, site.ClientID)
|
||||
if err := st.pool.QueryRow(ctx, `
|
||||
INSERT INTO sites (client_id, name, slug) VALUES ($1::uuid, $2, $3)
|
||||
RETURNING id::text`, site.ClientID, name, name).Scan(&site.SiteID); err != nil {
|
||||
|
||||
Reference in New Issue
Block a user