e2e/reviewloop_test.go
205 lines · 8694 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 a reviewer finds out, and the only way: there is
91 // no "request review from <user>", so nothing is pushed to someone
92 // who is neither an owner nor already in the thread. Writing this
93 // test is what surfaced that — see #145.
94 if got := inbox(t, inst, reviewerKey); strings.Contains(got, "ready for review") {
95 t.Fatalf("a reviewer is notified after all; #145 and this comment are stale:\n%s", got)
96 }
97
98 // The author cannot approve their own work past the gate.
99 if _, _, code := inst.ssh(t, authorKey, "", "mr", "review", "author/lib", "1", "--approve"); code != 0 {
100 t.Fatal("self review refused outright; it should be recorded and not counted")
101 }
102 _, errOut, code := inst.ssh(t, authorKey, "", "mr", "merge", "author/lib", "1")
103 if code != 4 || !strings.Contains(errOut, "requires 1 fresh approval") {
104 t.Fatalf("author's own approval counted: exit %d, %s", code, errOut)
105 }
106
107 // A review thread from the reviewer blocks on require-resolved.
108 out, errOut, code := inst.ssh(t, reviewerKey, "", "mr", "diff-comment", "author/lib", "1",
109 "--path", "lib.go", "--line", "3", "--message", "'name it better'", "--json")
110 if code != 0 {
111 t.Fatalf("diff-comment: %s", errOut)
112 }
113 var thread struct {
114 Data struct {
115 Thread int64 `json:"thread"`
116 } `json:"data"`
117 }
118 json.Unmarshal([]byte(out), &thread)
119 if !strings.Contains(inbox(t, inst, authorKey), "commented on lib.go") {
120 t.Fatalf("author not told about the review thread:\n%s", inbox(t, inst, authorKey))
121 }
122
123 if _, _, code := inst.ssh(t, reviewerKey, "", "mr", "review", "author/lib", "1", "--approve"); code != 0 {
124 t.Fatal("reviewer approve failed")
125 }
126 _, errOut, code = inst.ssh(t, authorKey, "", "mr", "merge", "author/lib", "1")
127 if code != 4 || !strings.Contains(errOut, "threads resolved") {
128 t.Fatalf("open thread did not block the merge: exit %d, %s", code, errOut)
129 }
130
131 // Resolve, and every gate is satisfied at once: an owner approved, the
132 // count is met, the thread is closed.
133 if _, errOut, code := inst.ssh(t, reviewerKey, "", "mr", "resolve", "author/lib", "1",
134 fmt.Sprint(thread.Data.Thread)); code != 0 {
135 t.Fatalf("resolve: %s", errOut)
136 }
137 if _, errOut, code := inst.ssh(t, authorKey, "", "mr", "merge", "author/lib", "1"); code != 0 {
138 t.Fatalf("fully reviewed MR refused: %s", errOut)
139 }
140 if !strings.Contains(inbox(t, inst, reviewerKey), "merged !1") {
141 t.Fatalf("reviewer not told about the merge:\n%s", inbox(t, inst, reviewerKey))
142 }
143
144 // A CODEOWNERS gate with no owner approval still refuses, on a second
145 // MR, so the pass above was the approval and not the gate being off.
146 mustGit(t, dir, env, "checkout", "-q", "-b", "feat2", "main")
147 os.WriteFile(filepath.Join(dir, "other.go"), []byte("package lib\n"), 0o644)
148 mustGit(t, dir, env, "add", ".")
149 mustGit(t, dir, env, "commit", "-q", "-m", "another")
150 mustGit(t, dir, env, "push", "-q", "origin", "feat2")
151 if _, errOut, code := inst.ssh(t, authorKey, "", "mr", "create", "author/lib",
152 "--source", "feat2", "--target", "main", "--title", "'another'"); code != 0 {
153 t.Fatalf("mr create: %s", errOut)
154 }
155 // helper's approval meets require-approvals but owns none of the
156 // changed files, so what refuses this merge is CODEOWNERS alone —
157 // the gate that used to be reachable only through the approval count
158 // (#99), now on its own toggle (#142).
159 if _, _, code := inst.ssh(t, helperKey, "", "mr", "review", "author/lib", "2", "--approve"); code != 0 {
160 t.Fatal("helper approve failed")
161 }
162 _, errOut, code = inst.ssh(t, authorKey, "", "mr", "merge", "author/lib", "2")
163 if code != 4 || !strings.Contains(errOut, "CODEOWNERS") {
164 t.Fatalf("CODEOWNERS gate did not fire on the second MR: exit %d, %s", code, errOut)
165 }
166 // And the owner's approval clears it.
167 if _, _, code := inst.ssh(t, reviewerKey, "", "mr", "review", "author/lib", "2", "--approve"); code != 0 {
168 t.Fatal("reviewer approve failed")
169 }
170 if _, errOut, code := inst.ssh(t, authorKey, "", "mr", "merge", "author/lib", "2"); code != 0 {
171 t.Fatalf("owner-approved MR refused: %s", errOut)
172 }
173}
174
175// reviewQueue returns the MR numbers waiting on this account.
176func reviewQueue(t *testing.T, inst *instance, key string) []int64 {
177 t.Helper()
178 out, errOut, code := inst.ssh(t, key, "", "dashboard", "--json")
179 if code != 0 {
180 t.Fatalf("dashboard: %s", errOut)
181 }
182 var env struct {
183 Data struct {
184 Reviews []struct {
185 Number int64 `json:"number"`
186 } `json:"review_queue"`
187 } `json:"data"`
188 }
189 json.Unmarshal([]byte(out), &env)
190 var ns []int64
191 for _, r := range env.Data.Reviews {
192 ns = append(ns, r.Number)
193 }
194 return ns
195}
196
197// inbox returns this account's notifications as raw JSON.
198func inbox(t *testing.T, inst *instance, key string) string {
199 t.Helper()
200 out, errOut, code := inst.ssh(t, key, "", "notifications", "list", "--all", "--json")
201 if code != 0 {
202 t.Fatalf("notifications list: %s", errOut)
203 }
204 return out
205}