e2e/reviewloop_test.go
206 lines · 8765 bytes
1package e2e
2
3import (
4 "encoding/json"
5 "fmt"
6 "os"
7 "path/filepath"
8 "strings"
9 "testing"
10)
11
12// TestTwoAccountReviewLoop drives the collaboration features end to end as
13// two distinct accounts: the author never approves their own work, and the
14// reviewer sees each step arrive.
15//
16// This is not the second human #139 asks for, and it is worth being exact
17// about why. The CODEOWNERS bug (#99) was a gate that never fired; a test
18// written from the same understanding as the code would have asserted the
19// same wrong thing. What this does close is the narrower gap that nothing
20// exercised the loop end to end at all — every feature had its own test
21// and none of them met.
22func TestTwoAccountReviewLoop(t *testing.T) {
23 inst := startInstance(t)
24 authorKey := inst.newKey(t, "author")
25 reviewerKey := inst.newKey(t, "reviewer")
26 // The author merges, and a merge commit carries their identity, so
27 // the account needs a verified address. The first merge below can
28 // fast-forward and would not have needed one; the second cannot.
29 inst.admin(t, "admin", "user", "create", "author", "--key", authorKey+".pub",
30 "--email", "author@example.test", "--verified")
31 inst.admin(t, "admin", "user", "create", "reviewer", "--key", reviewerKey+".pub")
32 // A third account with write access who owns nothing, so an approval
33 // from them satisfies the count and leaves CODEOWNERS unsatisfied.
34 helperKey := inst.newKey(t, "helper")
35 inst.admin(t, "admin", "user", "create", "helper", "--key", helperKey+".pub")
36
37 if _, errOut, code := inst.ssh(t, authorKey, "", "repo", "create", "author/lib"); code != 0 {
38 t.Fatalf("repo create: %s", errOut)
39 }
40 for _, who := range []string{"reviewer", "helper"} {
41 if _, _, code := inst.ssh(t, authorKey, "", "repo", "access", "grant", "author/lib", who, "write"); code != 0 {
42 t.Fatalf("grant %s failed", who)
43 }
44 }
45 // Every gate at once, which is how a repository that means it is set
46 // up, and which no single-feature test covers together.
47 for _, s := range [][]string{
48 {"repo", "settings", "require-approvals", "author/lib", "1"},
49 {"repo", "settings", "require-resolved", "author/lib", "on"},
50 {"repo", "settings", "require-codeowners", "author/lib", "on"},
51 } {
52 if _, errOut, code := inst.ssh(t, authorKey, "", s...); code != 0 {
53 t.Fatalf("%v: %s", s, errOut)
54 }
55 }
56
57 env := inst.gitEnv(authorKey)
58 work := t.TempDir()
59 mustGit(t, work, env, "clone", inst.sshURL("author/lib"), "w")
60 dir := filepath.Join(work, "w")
61 os.WriteFile(filepath.Join(dir, "CODEOWNERS"), []byte("*.go @reviewer\n"), 0o644)
62 os.WriteFile(filepath.Join(dir, "lib.go"), []byte("package lib\n"), 0o644)
63 mustGit(t, dir, env, "checkout", "-q", "-b", "main")
64 mustGit(t, dir, env, "add", ".")
65 mustGit(t, dir, env, "commit", "-q", "-m", "base")
66 mustGit(t, dir, env, "push", "-q", "origin", "main")
67
68 mustGit(t, dir, env, "checkout", "-q", "-b", "feat")
69 os.WriteFile(filepath.Join(dir, "lib.go"), []byte("package lib\n\nvar V = 1\n"), 0o644)
70 mustGit(t, dir, env, "add", ".")
71 mustGit(t, dir, env, "commit", "-q", "-m", "add V")
72 mustGit(t, dir, env, "push", "-q", "origin", "feat")
73
74 // Opened as a draft: the reviewer is not asked yet.
75 if _, errOut, code := inst.ssh(t, authorKey, "", "mr", "create", "author/lib",
76 "--source", "feat", "--target", "main", "--title", "'add V'", "--draft"); code != 0 {
77 t.Fatalf("mr create: %s", errOut)
78 }
79 if q := reviewQueue(t, inst, reviewerKey); len(q) != 0 {
80 t.Fatalf("a draft is waiting on the reviewer: %v", q)
81 }
82
83 // Ready. Now it is theirs, and it reaches their inbox.
84 if _, errOut, code := inst.ssh(t, authorKey, "", "mr", "ready", "author/lib", "1"); code != 0 {
85 t.Fatalf("mr ready: %s", errOut)
86 }
87 if q := reviewQueue(t, inst, reviewerKey); len(q) != 1 || q[0] != 1 {
88 t.Fatalf("review queue = %v, want !1", q)
89 }
90 // The queue is how this reviewer finds out: nobody ran "mr review
91 // request" for them, so being an owner or already in the thread is
92 // the only other way in, and this reviewer is neither. Writing this
93 // test is what surfaced the gap — see #145; TestMRReviewRequest covers
94 // the case where someone has been asked directly.
95 if got := inbox(t, inst, reviewerKey); strings.Contains(got, "ready for review") {
96 t.Fatalf("a reviewer is notified without being asked or involved:\n%s", got)
97 }
98
99 // The author cannot approve their own work past the gate.
100 if _, _, code := inst.ssh(t, authorKey, "", "mr", "review", "author/lib", "1", "--approve"); code != 0 {
101 t.Fatal("self review refused outright; it should be recorded and not counted")
102 }
103 _, errOut, code := inst.ssh(t, authorKey, "", "mr", "merge", "author/lib", "1")
104 if code != 4 || !strings.Contains(errOut, "requires 1 fresh approval") {
105 t.Fatalf("author's own approval counted: exit %d, %s", code, errOut)
106 }
107
108 // A review thread from the reviewer blocks on require-resolved.
109 out, errOut, code := inst.ssh(t, reviewerKey, "", "mr", "diff-comment", "author/lib", "1",
110 "--path", "lib.go", "--line", "3", "--message", "'name it better'", "--json")
111 if code != 0 {
112 t.Fatalf("diff-comment: %s", errOut)
113 }
114 var thread struct {
115 Data struct {
116 Thread int64 `json:"thread"`
117 } `json:"data"`
118 }
119 json.Unmarshal([]byte(out), &thread)
120 if !strings.Contains(inbox(t, inst, authorKey), "commented on lib.go") {
121 t.Fatalf("author not told about the review thread:\n%s", inbox(t, inst, authorKey))
122 }
123
124 if _, _, code := inst.ssh(t, reviewerKey, "", "mr", "review", "author/lib", "1", "--approve"); code != 0 {
125 t.Fatal("reviewer approve failed")
126 }
127 _, errOut, code = inst.ssh(t, authorKey, "", "mr", "merge", "author/lib", "1")
128 if code != 4 || !strings.Contains(errOut, "threads resolved") {
129 t.Fatalf("open thread did not block the merge: exit %d, %s", code, errOut)
130 }
131
132 // Resolve, and every gate is satisfied at once: an owner approved, the
133 // count is met, the thread is closed.
134 if _, errOut, code := inst.ssh(t, reviewerKey, "", "mr", "resolve", "author/lib", "1",
135 fmt.Sprint(thread.Data.Thread)); code != 0 {
136 t.Fatalf("resolve: %s", errOut)
137 }
138 if _, errOut, code := inst.ssh(t, authorKey, "", "mr", "merge", "author/lib", "1"); code != 0 {
139 t.Fatalf("fully reviewed MR refused: %s", errOut)
140 }
141 if !strings.Contains(inbox(t, inst, reviewerKey), "merged !1") {
142 t.Fatalf("reviewer not told about the merge:\n%s", inbox(t, inst, reviewerKey))
143 }
144
145 // A CODEOWNERS gate with no owner approval still refuses, on a second
146 // MR, so the pass above was the approval and not the gate being off.
147 mustGit(t, dir, env, "checkout", "-q", "-b", "feat2", "main")
148 os.WriteFile(filepath.Join(dir, "other.go"), []byte("package lib\n"), 0o644)
149 mustGit(t, dir, env, "add", ".")
150 mustGit(t, dir, env, "commit", "-q", "-m", "another")
151 mustGit(t, dir, env, "push", "-q", "origin", "feat2")
152 if _, errOut, code := inst.ssh(t, authorKey, "", "mr", "create", "author/lib",
153 "--source", "feat2", "--target", "main", "--title", "'another'"); code != 0 {
154 t.Fatalf("mr create: %s", errOut)
155 }
156 // helper's approval meets require-approvals but owns none of the
157 // changed files, so what refuses this merge is CODEOWNERS alone —
158 // the gate that used to be reachable only through the approval count
159 // (#99), now on its own toggle (#142).
160 if _, _, code := inst.ssh(t, helperKey, "", "mr", "review", "author/lib", "2", "--approve"); code != 0 {
161 t.Fatal("helper approve failed")
162 }
163 _, errOut, code = inst.ssh(t, authorKey, "", "mr", "merge", "author/lib", "2")
164 if code != 4 || !strings.Contains(errOut, "CODEOWNERS") {
165 t.Fatalf("CODEOWNERS gate did not fire on the second MR: exit %d, %s", code, errOut)
166 }
167 // And the owner's approval clears it.
168 if _, _, code := inst.ssh(t, reviewerKey, "", "mr", "review", "author/lib", "2", "--approve"); code != 0 {
169 t.Fatal("reviewer approve failed")
170 }
171 if _, errOut, code := inst.ssh(t, authorKey, "", "mr", "merge", "author/lib", "2"); code != 0 {
172 t.Fatalf("owner-approved MR refused: %s", errOut)
173 }
174}
175
176// reviewQueue returns the MR numbers waiting on this account.
177func reviewQueue(t *testing.T, inst *instance, key string) []int64 {
178 t.Helper()
179 out, errOut, code := inst.ssh(t, key, "", "dashboard", "--json")
180 if code != 0 {
181 t.Fatalf("dashboard: %s", errOut)
182 }
183 var env struct {
184 Data struct {
185 Reviews []struct {
186 Number int64 `json:"number"`
187 } `json:"review_queue"`
188 } `json:"data"`
189 }
190 json.Unmarshal([]byte(out), &env)
191 var ns []int64
192 for _, r := range env.Data.Reviews {
193 ns = append(ns, r.Number)
194 }
195 return ns
196}
197
198// inbox returns this account's notifications as raw JSON.
199func inbox(t *testing.T, inst *instance, key string) string {
200 t.Helper()
201 out, errOut, code := inst.ssh(t, key, "", "notifications", "list", "--all", "--json")
202 if code != 0 {
203 t.Fatalf("notifications list: %s", errOut)
204 }
205 return out
206}