mirror of
https://github.com/hanzoai/base.git
synced 2026-08-06 23:02:27 +00:00
tests: stop asserting numbers and strings that were never ours to pin
deploy / deploy (push) Failing after 4s
deploy / deploy (push) Failing after 4s
Every failure in this repo came from an expectation that was correct when
written and had no way to stay correct.
Collection counts (apis x6, core x3). `27` was hand-derived in a comment —
"17 framework/system + demo collections plus the 10 CRM ... collections" — and
the delete-event counts were a second hand-derivation, `9 + 10 = 19`. The
calendar-booking migration made them 31 and 23 and both files had been failing
since. Now read from the app: the total from CollectionQuery, and the deletes as
"all non-system, less the one the payload updates". A count of EVERYTHING is
invalidated by every migration that adds anything, so it cannot be a literal.
`SQL logic error` (apis x2). SQLite now says `near "invalid": syntax error`.
That assertion pinned a THIRD-PARTY string; the tests around it already check
status 400, `"data":{}`, our own `Raw error:` prefix, and — for the rollback
case — that the table was not created. Those are ours and they are the point.
IAM mock paths (iam x12). The mock still served `/api/get-users` and
`/api/add-user` after the client moved to `/v1/iam/*`. The client had a stale
doc comment saying `/api/add-user` too. Both fixed; the four tests were failing
with a bare `404 page not found`, which names nothing.
And one that was NOT a stale test. tools/search generates SQL calling acos, cos,
sin, radians and sqrt. The pure-Go backend (CGO_ENABLED=0) has them and that is
what the Dockerfile ships, so production is fine — but the cgo backend only gets
them behind csqlite `sqlite_math_functions`, so a cgo build silently has a
SMALLER SQL surface than the code written against it, and nothing says so until
a geoDistance filter returns "no such function: acos" from an endpoint that
works in prod. core/sqlite_math_required.go makes that combination a build
error naming the fix. Two build modes that answer differently is the bug.
Verified: full suite green as shipped (CGO_ENABLED=0), core twice.
network/ does not import core, so the cgo CI attack suite is unaffected.
TestNotifyWatcher_CollectionsUpdate is a pre-existing timing flake — passed 3/3
alone and on both full re-runs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -13,9 +13,16 @@ import (
|
||||
func TestCollectionsImport(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// 17 framework/system + demo collections plus the 10 CRM connect/messaging/
|
||||
// calendar collections added by migration 1780500000_connect_messaging_calendar.
|
||||
totalCollections := 27
|
||||
// Derived, not counted by hand: every migration that adds a collection
|
||||
// changes both of these, and the literals they replace (27 and 19) had gone
|
||||
// stale behind the calendar-booking collections.
|
||||
totalCollections, nonSystemCollections := collectionCounts(t)
|
||||
|
||||
// The create/update/delete payload updates ONE existing collection and
|
||||
// creates one that did not exist. System collections are never deleted, and
|
||||
// every other non-system collection is absent from the payload, so it goes.
|
||||
// Hence: all non-system, less the single one the payload updates.
|
||||
deleted := nonSystemCollections - 1
|
||||
|
||||
scenarios := []tests.ApiScenario{
|
||||
{
|
||||
@@ -289,15 +296,13 @@ func TestCollectionsImport(t *testing.T) {
|
||||
"OnCollectionUpdateExecute": 1,
|
||||
"OnCollectionAfterUpdateSuccess": 1,
|
||||
// ---
|
||||
// 9 original non-system collections + the 10 CRM connect/messaging/
|
||||
// calendar collections (migration 1780500000) are all "missing" from
|
||||
// this import payload and get deleted.
|
||||
"OnModelDelete": 19,
|
||||
"OnModelAfterDeleteSuccess": 19,
|
||||
"OnModelDeleteExecute": 19,
|
||||
"OnCollectionDelete": 19,
|
||||
"OnCollectionDeleteExecute": 19,
|
||||
"OnCollectionAfterDeleteSuccess": 19,
|
||||
// Every non-system collection absent from the payload is deleted.
|
||||
"OnModelDelete": deleted,
|
||||
"OnModelAfterDeleteSuccess": deleted,
|
||||
"OnModelDeleteExecute": deleted,
|
||||
"OnCollectionDelete": deleted,
|
||||
"OnCollectionDeleteExecute": deleted,
|
||||
"OnCollectionAfterDeleteSuccess": deleted,
|
||||
},
|
||||
AfterTestFunc: func(t testing.TB, app *tests.TestApp, res *http.Response) {
|
||||
collections := []*core.Collection{}
|
||||
|
||||
+49
-2
@@ -4,6 +4,7 @@ import (
|
||||
"net/http"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -13,9 +14,55 @@ import (
|
||||
"github.com/hanzoai/base/tools/list"
|
||||
)
|
||||
|
||||
// collectionCount is how many collections a freshly bootstrapped test app has.
|
||||
//
|
||||
// It used to be written as the literal 27 in two assertions. That is a number
|
||||
// every migration changes — it had already drifted to 31, and both scenarios
|
||||
// had been failing ever since the calendar-booking collections landed. A stale
|
||||
// literal in a passing-by-default position is a test that stops testing.
|
||||
//
|
||||
// Deriving it keeps the assertion that matters — the list endpoint reports
|
||||
// EVERY collection, dropping or filtering none — without re-arming a trap that
|
||||
// fires on the next migration.
|
||||
func collectionCount(t *testing.T) int {
|
||||
total, _ := collectionCounts(t)
|
||||
return total
|
||||
}
|
||||
|
||||
// collectionCounts reports how many collections a freshly bootstrapped test app
|
||||
// has, and how many of those are NOT system collections.
|
||||
//
|
||||
// Callers need both because the numbers that used to be hardcoded were derived
|
||||
// from each other by hand, in comments: "17 framework/system + demo collections
|
||||
// plus the 10 CRM ... collections" for the total, and "9 original non-system
|
||||
// collections + the 10 CRM ... collections are all missing from this import
|
||||
// payload and get deleted" for the delete-event counts. Arithmetic over the
|
||||
// migration list is wrong the moment a migration lands, and it was.
|
||||
func collectionCounts(t *testing.T) (total, nonSystem int) {
|
||||
t.Helper()
|
||||
app, err := tests.NewTestApp()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer app.Cleanup()
|
||||
|
||||
var cols []*core.Collection
|
||||
if err := app.CollectionQuery().All(&cols); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, c := range cols {
|
||||
if !c.System {
|
||||
nonSystem++
|
||||
}
|
||||
}
|
||||
return len(cols), nonSystem
|
||||
}
|
||||
|
||||
func TestCollectionsList(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
totalItems := `"totalItems":` + strconv.Itoa(collectionCount(t))
|
||||
|
||||
scenarios := []tests.ApiScenario{
|
||||
{
|
||||
Name: "unauthorized",
|
||||
@@ -47,7 +94,7 @@ func TestCollectionsList(t *testing.T) {
|
||||
ExpectedContent: []string{
|
||||
`"page":1`,
|
||||
`"perPage":30`,
|
||||
`"totalItems":27`,
|
||||
totalItems,
|
||||
`"items":[{`,
|
||||
`"name":"` + core.CollectionNameSuperusers + `"`,
|
||||
`"name":"users"`,
|
||||
@@ -81,7 +128,7 @@ func TestCollectionsList(t *testing.T) {
|
||||
ExpectedContent: []string{
|
||||
`"page":2`,
|
||||
`"perPage":2`,
|
||||
`"totalItems":27`,
|
||||
totalItems,
|
||||
`"items":[{`,
|
||||
},
|
||||
ExpectedEvents: map[string]int{
|
||||
|
||||
+12
-2
@@ -145,8 +145,13 @@ func TestSQLRun(t *testing.T) {
|
||||
ExpectedStatus: 400,
|
||||
ExpectedContent: []string{
|
||||
`"data":{}`,
|
||||
// "Raw error:" is OUR wording — that the driver's complaint was
|
||||
// surfaced rather than swallowed. What follows it is the
|
||||
// driver's, and pinning that here asserted a third-party string:
|
||||
// this read `SQL logic error` until SQLite started saying
|
||||
// `near "invalid": syntax error`, and the test broke without
|
||||
// anything in this repo changing.
|
||||
`Raw error:`,
|
||||
`SQL logic error`,
|
||||
},
|
||||
ExpectedEvents: map[string]int{"*": 0},
|
||||
},
|
||||
@@ -246,8 +251,13 @@ func TestSQLRun(t *testing.T) {
|
||||
ExpectedStatus: 400,
|
||||
ExpectedContent: []string{
|
||||
`"data":{}`,
|
||||
// "Raw error:" is OUR wording — that the driver's complaint was
|
||||
// surfaced rather than swallowed. What follows it is the
|
||||
// driver's, and pinning that here asserted a third-party string:
|
||||
// this read `SQL logic error` until SQLite started saying
|
||||
// `near "invalid": syntax error`, and the test broke without
|
||||
// anything in this repo changing.
|
||||
`Raw error:`,
|
||||
`SQL logic error`,
|
||||
},
|
||||
ExpectedEvents: map[string]int{"*": 0},
|
||||
},
|
||||
|
||||
@@ -11,10 +11,10 @@ import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/hanzoai/dbx"
|
||||
"github.com/hanzoai/base/core"
|
||||
"github.com/hanzoai/base/tests"
|
||||
"github.com/hanzoai/base/tools/list"
|
||||
"github.com/hanzoai/dbx"
|
||||
)
|
||||
|
||||
func TestCollectionQuery(t *testing.T) {
|
||||
@@ -78,13 +78,28 @@ func TestFindAllCollections(t *testing.T) {
|
||||
app, _ := tests.NewTestApp()
|
||||
defer app.Cleanup()
|
||||
|
||||
// How many collections exist, asked through a DIFFERENT path than the one
|
||||
// under test. Written as the literal 27 until the calendar-booking
|
||||
// migration took it to 31 and these three rows started failing — a count of
|
||||
// "everything" is invalidated by every migration that adds anything.
|
||||
//
|
||||
// Reading it from CollectionQuery keeps the assertion real: the three rows
|
||||
// below say that a nil filter, an empty filter and a blank filter all mean
|
||||
// "no filter", and that FindAllCollections then returns every collection the
|
||||
// store holds. Comparing FindAllCollections to itself would say nothing.
|
||||
var stored []*core.Collection
|
||||
if err := app.CollectionQuery().All(&stored); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
total := len(stored)
|
||||
|
||||
scenarios := []struct {
|
||||
collectionTypes []string
|
||||
expectTotal int
|
||||
}{
|
||||
{nil, 27},
|
||||
{[]string{}, 27},
|
||||
{[]string{""}, 27},
|
||||
{nil, total},
|
||||
{[]string{}, total},
|
||||
{[]string{""}, total},
|
||||
{[]string{"unknown"}, 0},
|
||||
{[]string{"unknown", core.CollectionTypeAuth}, 4},
|
||||
{[]string{core.CollectionTypeAuth, core.CollectionTypeView}, 7},
|
||||
|
||||
@@ -0,0 +1,31 @@
|
||||
//go:build cgo && !sqlite_math_functions
|
||||
|
||||
package core
|
||||
|
||||
// Base's search layer generates SQL that calls acos, cos, sin, radians and
|
||||
// sqrt — see the geoDistance token function in tools/search. Those are SQLite
|
||||
// math functions, and SQLite only has them when compiled with
|
||||
// SQLITE_ENABLE_MATH_FUNCTIONS.
|
||||
//
|
||||
// The pure-Go backend (CGO_ENABLED=0, modernc) always has them, and that is
|
||||
// what the Dockerfile ships, so production is fine. The cgo backend gets them
|
||||
// only behind csqlite's `sqlite_math_functions` build tag — so a cgo build
|
||||
// silently produces a binary whose SQL surface is SMALLER than the one the
|
||||
// code above it writes against. Nothing complains until someone filters by
|
||||
// geoDistance and gets "no such function: acos" from an endpoint that works in
|
||||
// production.
|
||||
//
|
||||
// Two build modes that answer differently is the bug; this makes them answer
|
||||
// the same or not build at all. Fix by adding the tag:
|
||||
//
|
||||
// go build -tags sqlite_math_functions ./...
|
||||
// go test -tags sqlite_math_functions ./...
|
||||
//
|
||||
// or by building the way the product ships, CGO_ENABLED=0.
|
||||
//
|
||||
// Deliberately a compile error rather than a runtime check: a capability the
|
||||
// query builder assumes is not something to discover from a customer's failed
|
||||
// search.
|
||||
func init() {
|
||||
_ = cgoBuildNeedsSQLiteMathFunctions
|
||||
}
|
||||
+12
-12
@@ -79,7 +79,7 @@ func TestLookupByAttribute_Hit(t *testing.T) {
|
||||
"16125551234": true,
|
||||
"6125551234": true,
|
||||
}
|
||||
f.setHandler("/api/get-users", func(w http.ResponseWriter, r *http.Request) {
|
||||
f.setHandler("/v1/iam/get-users", func(w http.ResponseWriter, r *http.Request) {
|
||||
if got := r.URL.Query().Get("field"); got != "phone" {
|
||||
t.Errorf("field: got %q want phone", got)
|
||||
}
|
||||
@@ -108,7 +108,7 @@ func TestLookupByAttribute_Hit(t *testing.T) {
|
||||
|
||||
func TestLookupByAttribute_Miss(t *testing.T) {
|
||||
f := newFakeIAM(t)
|
||||
f.setHandler("/api/get-users", func(w http.ResponseWriter, r *http.Request) {
|
||||
f.setHandler("/v1/iam/get-users", func(w http.ResponseWriter, r *http.Request) {
|
||||
writeOK(w, []map[string]any{})
|
||||
})
|
||||
|
||||
@@ -129,7 +129,7 @@ func TestLookupByAttribute_PhoneNormalization(t *testing.T) {
|
||||
// The user actually exists under the raw US-national form ("6125551234"),
|
||||
// so the first two probes miss and the third hits.
|
||||
f := newFakeIAM(t)
|
||||
f.setHandler("/api/get-users", func(w http.ResponseWriter, r *http.Request) {
|
||||
f.setHandler("/v1/iam/get-users", func(w http.ResponseWriter, r *http.Request) {
|
||||
switch r.URL.Query().Get("value") {
|
||||
case "6125551234":
|
||||
writeOK(w, []map[string]any{
|
||||
@@ -150,7 +150,7 @@ func TestLookupByAttribute_PhoneNormalization(t *testing.T) {
|
||||
if len(out) != 1 || out[0].ID != "u-7" {
|
||||
t.Fatalf("expected normalization to recover u-7, got %+v", out)
|
||||
}
|
||||
if got := f.callCount("/api/get-users"); got != 3 {
|
||||
if got := f.callCount("/v1/iam/get-users"); got != 3 {
|
||||
t.Fatalf("expected 3 probes (raw, no-plus, no-US), got %d", got)
|
||||
}
|
||||
}
|
||||
@@ -165,7 +165,7 @@ func TestLookupByAttribute_RequiresAdminCreds(t *testing.T) {
|
||||
|
||||
func TestLookupByAttribute_IAMErrorPropagates(t *testing.T) {
|
||||
f := newFakeIAM(t)
|
||||
f.setHandler("/api/get-users", func(w http.ResponseWriter, r *http.Request) {
|
||||
f.setHandler("/v1/iam/get-users", func(w http.ResponseWriter, r *http.Request) {
|
||||
writeErr(w, "field not supported")
|
||||
})
|
||||
|
||||
@@ -185,7 +185,7 @@ func TestLookupByAttribute_IAMErrorPropagates(t *testing.T) {
|
||||
func TestEnsureUser_Create(t *testing.T) {
|
||||
f := newFakeIAM(t)
|
||||
var addUserCalls, getUserCalls int64
|
||||
f.setHandler("/api/add-user", func(w http.ResponseWriter, r *http.Request) {
|
||||
f.setHandler("/v1/iam/add-user", func(w http.ResponseWriter, r *http.Request) {
|
||||
atomic.AddInt64(&addUserCalls, 1)
|
||||
body, _ := io.ReadAll(r.Body)
|
||||
var payload map[string]any
|
||||
@@ -198,7 +198,7 @@ func TestEnsureUser_Create(t *testing.T) {
|
||||
}
|
||||
writeOK(w, "Affected")
|
||||
})
|
||||
f.setHandler("/api/get-user", func(w http.ResponseWriter, r *http.Request) {
|
||||
f.setHandler("/v1/iam/get-user", func(w http.ResponseWriter, r *http.Request) {
|
||||
atomic.AddInt64(&getUserCalls, 1)
|
||||
writeOK(w, map[string]any{
|
||||
"id": "new-id",
|
||||
@@ -237,11 +237,11 @@ func TestEnsureUser_Idempotent_AlreadyExists(t *testing.T) {
|
||||
// idempotent-replay and resolve the user via GET /api/get-user.
|
||||
f := newFakeIAM(t)
|
||||
var addUserCalls int64
|
||||
f.setHandler("/api/add-user", func(w http.ResponseWriter, r *http.Request) {
|
||||
f.setHandler("/v1/iam/add-user", func(w http.ResponseWriter, r *http.Request) {
|
||||
atomic.AddInt64(&addUserCalls, 1)
|
||||
writeErr(w, "user:Email already exists")
|
||||
})
|
||||
f.setHandler("/api/get-user", func(w http.ResponseWriter, r *http.Request) {
|
||||
f.setHandler("/v1/iam/get-user", func(w http.ResponseWriter, r *http.Request) {
|
||||
if got := r.URL.Query().Get("email"); got != "dup@x.com" {
|
||||
t.Errorf("email: got %q want dup@x.com", got)
|
||||
}
|
||||
@@ -269,11 +269,11 @@ func TestEnsureUser_Idempotent_HTTP409(t *testing.T) {
|
||||
// Some IAM versions / proxies may return HTTP 409 directly. EnsureUser
|
||||
// handles both signals.
|
||||
f := newFakeIAM(t)
|
||||
f.setHandler("/api/add-user", func(w http.ResponseWriter, r *http.Request) {
|
||||
f.setHandler("/v1/iam/add-user", func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusConflict)
|
||||
_, _ = w.Write([]byte(`{"status":"error","msg":"already exists"}`))
|
||||
})
|
||||
f.setHandler("/api/get-user", func(w http.ResponseWriter, r *http.Request) {
|
||||
f.setHandler("/v1/iam/get-user", func(w http.ResponseWriter, r *http.Request) {
|
||||
writeOK(w, map[string]any{
|
||||
"id": "existing-id",
|
||||
"name": "dup",
|
||||
@@ -298,7 +298,7 @@ func TestEnsureUser_PropagatesNonExistsError(t *testing.T) {
|
||||
// Errors that are NOT "already exists" must propagate to the caller —
|
||||
// not get silently swallowed by the idempotent-replay path.
|
||||
f := newFakeIAM(t)
|
||||
f.setHandler("/api/add-user", func(w http.ResponseWriter, r *http.Request) {
|
||||
f.setHandler("/v1/iam/add-user", func(w http.ResponseWriter, r *http.Request) {
|
||||
writeErr(w, "organization not found")
|
||||
})
|
||||
|
||||
|
||||
@@ -544,7 +544,7 @@ func (c *IAMClient) LookupByAttribute(ctx context.Context, attr, value, org stri
|
||||
// EnsureUser idempotently provisions an IAM user matching spec. If the user
|
||||
// already exists (matched by email within spec.Owner), the existing user is
|
||||
// returned without modification. Otherwise the user is created via
|
||||
// POST /api/add-user and the new user is fetched and returned.
|
||||
// POST /v1/iam/add-user and the new user is fetched and returned.
|
||||
//
|
||||
// EnsureUser treats both HTTP 409 and IAM's status:"error" + "already exists"
|
||||
// envelope as the idempotent-replay path — IAM responds with HTTP 200 in
|
||||
|
||||
Reference in New Issue
Block a user