Address Gadfly review on #12: helper naming + FK-backstop test
Build image / build-and-push (push) Successful in 5s
Build image / build-and-push (push) Successful in 5s
- Rename test helper intPtr → ptrInt to match the package's ptrBool sibling (2/5 reviewers flagged the naming inconsistency). - Add a store-level test that drives DeletePlant into the ON DELETE RESTRICT foreign key so the isForeignKeyViolation string-match (→ ErrPlantInUse) is pinned; a driver-message change now fails here, not as a prod 500. Skipped (advisory, non-blocking): the parseNullableInt/parseNullableString generic-helper refactor (cosmetic, touches working code) and the documented LIMIT 5000 backstop. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01JdQpdYYsTgtkJBxbcpAszi
This commit is contained in:
@@ -9,7 +9,7 @@ import (
|
|||||||
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
|
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
|
||||||
)
|
)
|
||||||
|
|
||||||
func intPtr(n int) *int { return &n }
|
func ptrInt(n int) *int { return &n }
|
||||||
|
|
||||||
// validPlant is a well-formed PlantInput for tests to tweak.
|
// validPlant is a well-formed PlantInput for tests to tweak.
|
||||||
func validPlant(name string) PlantInput {
|
func validPlant(name string) PlantInput {
|
||||||
@@ -53,7 +53,7 @@ func TestCreatePlantIsOwnedAndTrimmed(t *testing.T) {
|
|||||||
owner := seedUser(t, s, "[email protected]")
|
owner := seedUser(t, s, "[email protected]")
|
||||||
|
|
||||||
in := validPlant(" Purple basil ")
|
in := validPlant(" Purple basil ")
|
||||||
in.DaysToMaturity = intPtr(60)
|
in.DaysToMaturity = ptrInt(60)
|
||||||
p, err := s.CreatePlant(context.Background(), owner, in)
|
p, err := s.CreatePlant(context.Background(), owner, in)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("CreatePlant: %v", err)
|
t.Fatalf("CreatePlant: %v", err)
|
||||||
@@ -147,7 +147,7 @@ func TestUpdatePlantClearsDays(t *testing.T) {
|
|||||||
s := newTestService(t, openConfig())
|
s := newTestService(t, openConfig())
|
||||||
owner := seedUser(t, s, "[email protected]")
|
owner := seedUser(t, s, "[email protected]")
|
||||||
in := validPlant("Dill")
|
in := validPlant("Dill")
|
||||||
in.DaysToMaturity = intPtr(50)
|
in.DaysToMaturity = ptrInt(50)
|
||||||
p, _ := s.CreatePlant(context.Background(), owner, in)
|
p, _ := s.CreatePlant(context.Background(), owner, in)
|
||||||
|
|
||||||
// SetDays with nil clears; absent (SetDays=false) would leave it.
|
// SetDays with nil clears; absent (SetDays=false) would leave it.
|
||||||
@@ -191,7 +191,7 @@ func TestCreatePlantValidation(t *testing.T) {
|
|||||||
{Name: "x", Category: domain.CategoryHerb, SpacingCM: 10, Color: "nope", Icon: "🌿"}, // bad color
|
{Name: "x", Category: domain.CategoryHerb, SpacingCM: 10, Color: "nope", Icon: "🌿"}, // bad color
|
||||||
{Name: "x", Category: domain.CategoryHerb, SpacingCM: 10, Color: "#4a7c3f", Icon: ""}, // empty icon
|
{Name: "x", Category: domain.CategoryHerb, SpacingCM: 10, Color: "#4a7c3f", Icon: ""}, // empty icon
|
||||||
{Name: strings.Repeat("x", maxPlantNameLen+1), Category: domain.CategoryHerb, SpacingCM: 10, Color: "#4a7c3f", Icon: "🌿"}, // name too long
|
{Name: strings.Repeat("x", maxPlantNameLen+1), Category: domain.CategoryHerb, SpacingCM: 10, Color: "#4a7c3f", Icon: "🌿"}, // name too long
|
||||||
{Name: "x", Category: domain.CategoryHerb, SpacingCM: 10, Color: "#4a7c3f", Icon: "🌿", DaysToMaturity: intPtr(0)}, // days must be >= 1
|
{Name: "x", Category: domain.CategoryHerb, SpacingCM: 10, Color: "#4a7c3f", Icon: "🌿", DaysToMaturity: ptrInt(0)}, // days must be >= 1
|
||||||
}
|
}
|
||||||
for i, in := range bad {
|
for i, in := range bad {
|
||||||
if _, err := s.CreatePlant(context.Background(), owner, in); !errors.Is(err, domain.ErrInvalidInput) {
|
if _, err := s.CreatePlant(context.Background(), owner, in); !errors.Is(err, domain.ErrInvalidInput) {
|
||||||
@@ -202,7 +202,7 @@ func TestCreatePlantValidation(t *testing.T) {
|
|||||||
// A shorthand hex (#rgb) and a set days value pass.
|
// A shorthand hex (#rgb) and a set days value pass.
|
||||||
ok := validPlant("Thyme")
|
ok := validPlant("Thyme")
|
||||||
ok.Color = "#0a0"
|
ok.Color = "#0a0"
|
||||||
ok.DaysToMaturity = intPtr(90)
|
ok.DaysToMaturity = ptrInt(90)
|
||||||
if _, err := s.CreatePlant(context.Background(), owner, ok); err != nil {
|
if _, err := s.CreatePlant(context.Background(), owner, ok); err != nil {
|
||||||
t.Errorf("valid plant rejected: %v", err)
|
t.Errorf("valid plant rejected: %v", err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2,9 +2,12 @@ package store
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"errors"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
|
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
|
||||||
)
|
)
|
||||||
|
|
||||||
// openTestDB opens a fresh file-backed DB in a temp dir and migrates it.
|
// openTestDB opens a fresh file-backed DB in a temp dir and migrates it.
|
||||||
@@ -88,6 +91,44 @@ func TestForeignKeysEnforced(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestDeletePlantForeignKeyBackstop(t *testing.T) {
|
||||||
|
db := openTestDB(t)
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
// Seed a user → garden → object → custom plant, then a planting referencing
|
||||||
|
// the plant. DeletePlant must surface the ON DELETE RESTRICT FK as
|
||||||
|
// ErrPlantInUse (not a raw 500-worthy error) — this pins isForeignKeyViolation's
|
||||||
|
// string-match so a driver-message change is caught here rather than in prod.
|
||||||
|
// The service normally blocks this via a count pre-check; here we hit the
|
||||||
|
// store backstop directly (the lost-race path).
|
||||||
|
insert := func(q string, args ...any) int64 {
|
||||||
|
res, err := db.SQL().ExecContext(ctx, q, args...)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("seed exec %q: %v", q, err)
|
||||||
|
}
|
||||||
|
id, err := res.LastInsertId()
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("last insert id: %v", err)
|
||||||
|
}
|
||||||
|
return id
|
||||||
|
}
|
||||||
|
uid := insert(`INSERT INTO users (email, display_name) VALUES ('[email protected]', 'A')`)
|
||||||
|
gid := insert(`INSERT INTO gardens (owner_id, name, width_cm, height_cm) VALUES (?, 'G', 100, 100)`, uid)
|
||||||
|
oid := insert(`INSERT INTO garden_objects (garden_id, kind, x_cm, y_cm, width_cm, height_cm) VALUES (?, 'bed', 0, 0, 10, 10)`, gid)
|
||||||
|
pid := insert(`INSERT INTO plants (owner_id, name, category, spacing_cm, color, icon) VALUES (?, 'Basil', 'herb', 25, '#4a7c3f', '🌿')`, uid)
|
||||||
|
insert(`INSERT INTO plantings (object_id, plant_id, x_cm, y_cm, radius_cm) VALUES (?, ?, 0, 0, 10)`, oid, pid)
|
||||||
|
|
||||||
|
if err := db.DeletePlant(ctx, pid); !errors.Is(err, domain.ErrPlantInUse) {
|
||||||
|
t.Fatalf("DeletePlant on referenced plant = %v, want ErrPlantInUse", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Clearing the reference lets the delete succeed.
|
||||||
|
insert(`DELETE FROM plantings WHERE plant_id = ?`, pid)
|
||||||
|
if err := db.DeletePlant(ctx, pid); err != nil {
|
||||||
|
t.Errorf("DeletePlant after clearing reference: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestCheckConstraintRejectsBadEnum(t *testing.T) {
|
func TestCheckConstraintRejectsBadEnum(t *testing.T) {
|
||||||
db := openTestDB(t)
|
db := openTestDB(t)
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user