diff --git a/internal/tools/tbd_ship_test.go b/internal/tools/tbd_ship_test.go index 231ebcd..452fd6a 100644 --- a/internal/tools/tbd_ship_test.go +++ b/internal/tools/tbd_ship_test.go @@ -19,12 +19,23 @@ import ( // shipFake serves the whole tbd_ship flow; runsJSON is the workflow_runs array // for the head commit, letting each test drive the CI gate. func shipFake(t *testing.T, runsJSON string, merged, deleted *atomic.Bool) *httptest.Server { + t.Helper() + return shipFakeOpts(t, runsJSON, 0, http.StatusOK, merged, deleted) +} + +// shipFakeOpts adds protection (requiredApprovals>0 → protected) and a merge +// status code, so tests can drive the review-protected and conflict paths. +func shipFakeOpts(t *testing.T, runsJSON string, requiredApprovals, mergeStatus int, merged, deleted *atomic.Bool) *httptest.Server { t.Helper() return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { p := r.URL.Path w.Header().Set("Content-Type", "application/json") switch { case r.Method == http.MethodGet && strings.Contains(p, "/branch_protections/"): + if requiredApprovals > 0 { + _, _ = w.Write([]byte(`{"required_approvals":` + itoa(requiredApprovals) + `}`)) + return + } w.WriteHeader(http.StatusNotFound) // unprotected _, _ = w.Write([]byte(`{"message":"not found"}`)) case r.Method == http.MethodPost && strings.HasSuffix(p, "/branches"): @@ -43,7 +54,7 @@ func shipFake(t *testing.T, runsJSON string, merged, deleted *atomic.Bool) *http _, _ = w.Write([]byte(`{"total_count":0,"workflow_runs":` + runsJSON + `}`)) case r.Method == http.MethodPost && strings.HasSuffix(p, "/pulls/7/merge"): merged.Store(true) - w.WriteHeader(http.StatusOK) + w.WriteHeader(mergeStatus) _, _ = w.Write([]byte(`{}`)) case r.Method == http.MethodDelete && strings.Contains(p, "/branches/"): deleted.Store(true) @@ -109,6 +120,36 @@ func TestTBDShip_RedCI_FailsClosed(t *testing.T) { assert.False(t, merged.Load(), "merge must NOT be called when CI is red") } +func itoa(n int) string { b, _ := json.Marshal(n); return string(b) } + +// Green CI but the base branch requires review → fail closed (no auto-merge). +func TestTBDShip_ReviewProtected_FailsClosed(t *testing.T) { + var merged, deleted atomic.Bool + srv := shipFakeOpts(t, `[{"id":1,"status":"completed","conclusion":"success","head_sha":"abc"}]`, 1, http.StatusOK, &merged, &deleted) + defer srv.Close() + + res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x"}`) + + assert.Equal(t, false, res["merged"]) + assert.NotEmpty(t, res["reason"]) + assert.Contains(t, res["reason"], "review") + assert.False(t, merged.Load(), "must NOT merge a review-protected base") +} + +// Green CI but the merge is unclean (409) → fail closed, PR left open. +func TestTBDShip_MergeConflict_FailsClosed(t *testing.T) { + var merged, deleted atomic.Bool + srv := shipFakeOpts(t, `[{"id":1,"status":"completed","conclusion":"success","head_sha":"abc"}]`, 0, http.StatusConflict, &merged, &deleted) + defer srv.Close() + + res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x"}`) + + assert.Equal(t, false, res["merged"]) + assert.NotEmpty(t, res["reason"]) + assert.True(t, merged.Load(), "merge is attempted") + assert.False(t, deleted.Load(), "branch is NOT deleted on a failed merge") +} + func TestTBDShip_AllowlistRejects(t *testing.T) { tool := tools.NewTBDShip(gitea.NewClient("http://unused", ""), allowlist.New([]string{"mathias"})) _, err := tool.Call(context.Background(), json.RawMessage(`{"owner":"evil","repo":"r","path":"p","content":"c","message":"m"}`))