steve/pansy
1
0
Fork 0
Code Issues 2 Pull Requests Actions Packages Projects Releases Wiki Activity

API-level tests for the /seed-lots route group #88

Merged
steve merged 3 commits from test/seed-lots-api into main 2026-07-22 02:53:13 +00:00
Conversation 6 Commits 3 Files Changed 1
+23 -4
1 changed files with 23 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 5bdaf21828 - Show all commits
internal/api/seed_lots_test.go
+23 -4
View File
2
 @@ -89,8 +89,12 @@ func TestSeedLotCrudAPI(t *testing.T) {
if n := len(decodeList(t, w.Body.Bytes())); n != 0 {
t.Errorf("filter by a plant with no lots returned %d, want 0", n)
}
if w := doJSON(t, r, http.MethodGet, "/api/v1/seed-lots?plantId=nope", nil, cookie); w.Code != http.StatusBadRequest {
t.Errorf("bad plantId = %d, want 400", w.Code)
// A bad plantId filter is 400, whether non-numeric or out of range — the
// handler rejects id < 1, not just unparseable strings.
for _, bad := range []string{"nope", "0", "-1"} {
if w := doJSON(t, r, http.MethodGet, "/api/v1/seed-lots?plantId="+bad, nil, cookie); w.Code != http.StatusBadRequest {
t.Errorf("plantId=%q filter = %d, want 400", bad, w.Code)
}
}
// PATCH with the current version.
@@ -156,10 +160,10 @@ func TestSeedLotRemainingIsDerivedAPI(t *testing.T) {
if w.Code != http.StatusCreated {
t.Fatalf("create object: status %d, body %s", w.Code, w.Body.String())
}
objID := int64(decodeMap(t, w.Body.Bytes())["id"].(float64))
oid := int64(decodeMap(t, w.Body.Bytes())["id"].(float64))
// Plant 12 of them against the lot.
w = doJSON(t, r, http.MethodPost, objectPath(objID)+"/plantings", map[string]any{
w = doJSON(t, r, http.MethodPost, objectPath(oid)+"/plantings", map[string]any{
"plantId": plantID, "xCm": 0, "yCm": 0, "radiusCm": 20, "count": 12, "seedLotId": lotID,
}, cookie)
if w.Code != http.StatusCreated {
@@ -174,6 +178,21 @@ func TestSeedLotRemainingIsDerivedAPI(t *testing.T) {
if rem, ok := got["remaining"].(float64); !ok || rem != 38 {
t.Errorf("remaining = %v, want 38 (50 bought - 12 planted): %s", got["remaining"], w.Body.String())
}
// Over-plant it: 45 more (57 total against 50 bought) drives remaining
// NEGATIVE. That's deliberate — the number is a derived truth about what
// you've committed, not a floor clamped at zero, and "you've planted more than
// you bought" is exactly the signal a gardener wants rather than a hidden -7.
w = doJSON(t, r, http.MethodPost, objectPath(oid)+"/plantings", map[string]any{
"plantId": plantID, "xCm": 40, "yCm": 40, "radiusCm": 20, "count": 45, "seedLotId": lotID,
}, cookie)
if w.Code != http.StatusCreated {
t.Fatalf("over-plant: status %d, body %s", w.Code, w.Body.String())
}
w = doJSON(t, r, http.MethodGet, seedLotPath(lotID), nil, cookie)
gitea-actions commented 2026-07-22 02:30:23 +00:00
Review
Copy Link
Copy Source

⚪ Missing status-code check before decoding response body after over-planting, masking the real error on failure

error-handling · flagged by 1 model

  • internal/api/seed_lots_test.go:192-195 — The final GET in TestSeedLotRemainingIsDerivedAPI decodes the response body without first checking w.Code == http.StatusOK, unlike every other GET/decode pair in this file (e.g. lines 173-176, 160-162). If this request fails (e.g. a 404/500 due to a regression), decodeMap (internal/api/gardens_test.go:26-34) will still unmarshal successfully since it only fails on invalid JSON, not missing keys — ["remaining"] will be absent, the assertion fal…

🪰 Gadfly · advisory

⚪ **Missing status-code check before decoding response body after over-planting, masking the real error on failure** _error-handling · flagged by 1 model_ - `internal/api/seed_lots_test.go:192-195` — The final GET in `TestSeedLotRemainingIsDerivedAPI` decodes the response body without first checking `w.Code == http.StatusOK`, unlike every other GET/decode pair in this file (e.g. lines 173-176, 160-162). If this request fails (e.g. a 404/500 due to a regression), `decodeMap` (internal/api/gardens_test.go:26-34) will still unmarshal successfully since it only fails on invalid JSON, not missing keys — `["remaining"]` will be absent, the assertion fal… <sub>🪰 Gadfly · advisory</sub>
if rem, ok := decodeMap(t, w.Body.Bytes())["remaining"].(float64); !ok || rem != -7 {
t.Errorf("remaining after over-planting = %v, want -7 (50 - 57)", rem)
}
}
// TestSeedLotsArePrivateAPI checks the ACL through the router.