Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.
vole
Clone

Read only, no credentials:

https://rodent.software/vole.git

Write over SSH with a registered key:

git clone ssh://git@rodent.software:2222/vole.git

A free and open-source forge that lives in your Git repo.

Code

395 commit(s) on main

Compare History
update readme Keenan Nemetz · 57m ago 32290114
history blame raw 31.3 KiB
1package metadata
2
3// A review thread lives in the same tree as the request it discusses, so the
4// parser is also the boundary that decides what a comment file may look like.
5// These tests hold the two halves: a comment is accepted exactly when it has an
6// author and a timestamp, and anything else in the subtree is refused with the
7// path named.
8
9import (
10 "strings"
11 "testing"
12 "time"
13
14 "rodent.software/vole/core/git"
15)
16
17const (
18 reqIDA = "01M5JR2JK29KKMAA1T9MDNTRB1" // the request
19 reqIDB = "01M5JR2JK29KKMAA1T9MDNTRB2" // a comment
20 reqIDC = "01M5JR2JK29KKMAA1T9MDNTRB3" // the author's account, and what the writer last saw
21 reqIDD = "01M5JR2JK29KKMAA1T9MDNTRB4" // an approver who is not the author
22)
23
24func requestBody(head string) string {
25 return "---\n" +
26 `title: "Land the retry"` + "\n" +
27 "state: open\n" +
28 "author: keenan\n" +
29 "base: main\n" +
30 "head: " + head + "\n" +
31 "created: 2026-10-22T22:00:00Z\n" +
32 "---\n" +
33 "why this change\n"
34}
35
36const (
37 headSHA = "0123456789abcdef0123456789abcdef01234567"
38 sideSHA = "89abcdef0123456789abcdef0123456789abcdef"
39)
40
41func TestParseReadsTheDiscussion(t *testing.T) {
42 t.Parallel()
43 list, err := ParseRequests(map[string]string{
44 ChangesDir + "/" + reqIDA + "/change.md": requestBody(headSHA),
45 ChangesDir + "/" + reqIDA + "/comments/" + reqIDB + ".md": "---\nauthor: wilbur\ncreated: 2026-10-22T23:00:00Z\n---\nOne question.\n",
46 ChangesDir + "/" + reqIDA + "/approvals/wilbur.md": "---\nbase: main\nhead: " + headSHA + "\ncreated: 2026-10-22T23:10:00Z\n---\n",
47 ChangesDir + "/" + reqIDA + "/comments/01M5JR2JK29KKMAA1T9MDNTRBZ.md": "---\nauthor: keenan\ncreated: 2026-10-22T23:05:00Z\n---\nEarlier.\n",
48 })
49 if err != nil {
50 t.Fatalf("unexpected error: %v", err)
51 }
52 if len(list) != 1 {
53 t.Fatalf("requests = %d, want 1", len(list))
54 }
55 req := list[0]
56 if len(req.Comments) != 2 {
57 t.Fatalf("comments = %d, want 2", len(req.Comments))
58 }
59 // Sorted by id, which is time-ordered, so a thread reads in the order it was
60 // written rather than in whatever order the tree was walked.
61 if req.Comments[0].Author != "wilbur" || req.Comments[1].Author != "keenan" {
62 t.Fatalf("comment order: %s then %s", req.Comments[0].Author, req.Comments[1].Author)
63 }
64 if strings.TrimSpace(req.Comments[0].Body) != "One question." {
65 t.Fatalf("body = %q", req.Comments[0].Body)
66 }
67 if len(req.Approvals) != 1 || req.Approvals[0].Principal != "wilbur" {
68 t.Fatalf("approvals = %+v", req.Approvals)
69 }
70}
71
72func TestCommentNeedsAnAuthorAndATime(t *testing.T) {
73 t.Parallel()
74 cases := []struct{ name, body, want string }{
75 {"no author", "---\ncreated: 2026-10-22T23:00:00Z\n---\ntext\n", "author is required"},
76 {"no created", "---\nauthor: wilbur\n---\ntext\n", "created is required"},
77 {"bad created", "---\nauthor: wilbur\ncreated: yesterday\n---\ntext\n", "created is required and must be RFC3339"},
78 {"no frontmatter", "just text\n", "a comment is frontmatter with author and created"},
79 }
80 for _, c := range cases {
81 _, err := ParseRequests(map[string]string{
82 ChangesDir + "/" + reqIDA + "/change.md": requestBody(headSHA),
83 ChangesDir + "/" + reqIDA + "/comments/" + reqIDB + ".md": c.body,
84 })
85 ve, ok := err.(*ValidationError)
86 if !ok {
87 t.Fatalf("%s: accepted: %v", c.name, err)
88 }
89 joined := strings.Join(ve.Problems, "; ")
90 if !strings.Contains(joined, "comments/"+reqIDB+".md") || !strings.Contains(joined, c.want) {
91 t.Fatalf("%s: unhelpful refusal: %v", c.name, ve.Problems)
92 }
93 }
94}
95
96// TestUnknownFileInARequestIsRefused: the subtree is a closed format, so a review
97// note saved under the wrong name is a rejection with both shapes named, not a
98// file the web silently ignores.
99func TestUnknownFileInARequestIsRefused(t *testing.T) {
100 t.Parallel()
101 _, err := ParseRequests(map[string]string{
102 ChangesDir + "/" + reqIDA + "/change.md": requestBody(headSHA),
103 ChangesDir + "/" + reqIDA + "/review.md": "---\nauthor: wilbur\ncreated: 2026-10-22T23:00:00Z\n---\nnote\n",
104 })
105 ve, ok := err.(*ValidationError)
106 if !ok {
107 t.Fatalf("accepted: %v", err)
108 }
109 joined := strings.Join(ve.Problems, "; ")
110 for _, want := range []string{"review.md", "change.md", "approvals/", "comments/"} {
111 if !strings.Contains(joined, want) {
112 t.Fatalf("refusal does not mention %q: %v", want, ve.Problems)
113 }
114 }
115}
116
117func TestCommentPaths(t *testing.T) {
118 t.Parallel()
119 path := RequestCommentPath(reqIDA, reqIDB)
120 if path != "changes/"+reqIDA+"/comments/"+reqIDB+".md" {
121 t.Fatalf("path = %q", path)
122 }
123 if !IsRequestCommentPath(path) {
124 t.Fatal("a comment path was not recognised")
125 }
126 for _, other := range []string{
127 "changes/" + reqIDA + "/change.md",
128 "changes/" + reqIDA + "/approvals/" + reqIDB + ".md",
129 "issues/" + reqIDA + "/comments/" + reqIDB + ".md",
130 } {
131 if IsRequestCommentPath(other) {
132 t.Fatalf("%q was mistaken for a request comment", other)
133 }
134 }
135}
136
137func TestRepliesAllowAnyCommentAsParent(t *testing.T) {
138 t.Parallel()
139 const (
140 rootID = "01M5JR2JK29KKMAA1T9MDNTRC1"
141 replyID = "01M5JR2JK29KKMAA1T9MDNTRC2"
142 deeperID = "01M5JR2JK29KKMAA1T9MDNTRC3"
143 )
144 requestPath := ChangesDir + "/" + reqIDA + "/change.md"
145 commentPath := func(id string) string {
146 return ChangesDir + "/" + reqIDA + "/comments/" + id + ".md"
147 }
148 root := "---\nauthor: wilbur\ncreated: 2026-10-22T23:00:00Z\n---\nRoot.\n"
149 reply := "---\nauthor: keenan\ncreated: 2026-10-22T23:01:00Z\nreply_to: " + rootID + "\n---\nReply.\n"
150 deeper := "---\nauthor: wilbur\ncreated: 2026-10-22T23:02:00Z\nreply_to: " + replyID + "\n---\nDeeper.\n"
151
152 list, err := ParseRequests(map[string]string{
153 requestPath: requestBody(headSHA),
154 commentPath(rootID): root,
155 commentPath(replyID): reply,
156 commentPath(deeperID): deeper,
157 })
158 if err != nil {
159 t.Fatalf("nested replies: %v", err)
160 }
161 comments := list[0].Comments
162 if len(comments) != 3 || comments[1].ReplyTo != rootID || comments[2].ReplyTo != replyID {
163 t.Fatalf("replies were not read: %+v", comments)
164 }
165
166 raw, err := RenderComment(&Comment{
167 ID: replyID, Author: "keenan", Created: time.Date(2026, 10, 22, 23, 1, 0, 0, time.UTC),
168 Body: "Reply.\n", ReplyTo: rootID,
169 })
170 if err != nil || !strings.Contains(string(raw), "reply_to: "+rootID) {
171 t.Fatalf("render reply: %v\n%s", err, raw)
172 }
173
174 // A missing parent is not a parse error: the UI synthesizes a tombstone for
175 // it, which also keeps older metadata readable.
176 _, err = ParseRequests(map[string]string{
177 requestPath: requestBody(headSHA),
178 commentPath(replyID): "---\nauthor: keenan\ncreated: 2026-10-22T23:01:00Z\nreply_to: 01M5JR2JK29KKMAA1T9MDNTRZZ\n---\nOrphan.\n",
179 })
180 if err != nil {
181 t.Fatalf("orphan reply was rejected: %v", err)
182 }
183}
184
185// A merged request is a record of a landing, so the file has to name the commit
186// that landed. Without this, "merged" would be a claim no commit backs, and a
187// reopened request could keep a stale one.
188func TestMergedStateNamesTheCommitThatLanded(t *testing.T) {
189 t.Parallel()
190 base := map[string]string{
191 ChangesDir + "/" + reqIDA + "/change.md": "",
192 }
193 merged := func(extra string) map[string]string {
194 m := map[string]string{}
195 for k := range base {
196 m[k] = "---\n" +
197 `title: "Land the retry"` + "\n" +
198 "state: merged\n" +
199 "author: keenan\n" +
200 "base: main\n" +
201 "head: " + headSHA + "\n" +
202 "created: 2026-10-22T22:00:00Z\n" +
203 extra +
204 "---\n" +
205 "why this change\n"
206 }
207 return m
208 }
209
210 verified := "merged_head: " + sideSHA + "\nmerged_by: wilbur\nmerged_at: 2026-10-23T09:00:00Z\n"
211 if _, err := ParseRequests(merged(verified)); err != nil {
212 t.Fatalf("a merged request with a commit was refused: %v", err)
213 }
214 // merged_base records what the base pointed at before the landing, which is what
215 // a merged request is diffed against. It is optional on read so records written
216 // before it existed still load, but the value has to be a commit.
217 withBase := merged(verified + "merged_base: " + headSHA + "\n")
218 list, err := ParseRequests(withBase)
219 if err != nil {
220 t.Fatalf("a merged request naming its base was refused: %v", err)
221 }
222 if list[0].MergedBase != headSHA {
223 t.Fatalf("merged_base = %q, want %q", list[0].MergedBase, headSHA)
224 }
225 for name, extra := range map[string]string{
226 "no merged_head": "merged_by: wilbur\nmerged_at: 2026-10-23T09:00:00Z\n",
227 "short merged_head": "merged_head: " + headSHA[:12] + "\nmerged_by: wilbur\nmerged_at: 2026-10-23T09:00:00Z\n",
228 "short merged_base": verified + "merged_base: " + headSHA[:12] + "\n",
229 "no merged_by": "merged_head: " + sideSHA + "\nmerged_at: 2026-10-23T09:00:00Z\n",
230 "no merged_at": "merged_head: " + sideSHA + "\nmerged_by: wilbur\n",
231 "merged_at is not TS": "merged_head: " + sideSHA + "\nmerged_by: wilbur\nmerged_at: yesterday\n",
232 } {
233 if _, err := ParseRequests(merged(extra)); err == nil {
234 t.Errorf("a merged request with %s was accepted", name)
235 }
236 }
237 // A request that is not merged may not carry the record either.
238 for field, line := range map[string]string{
239 "merged_head": "merged_head: " + sideSHA + "\n",
240 "merged_base": "merged_base: " + sideSHA + "\n",
241 } {
242 pending := map[string]string{
243 ChangesDir + "/" + reqIDA + "/change.md": "---\n" +
244 `title: "Land the retry"` + "\n" +
245 "state: open\n" +
246 "author: keenan\n" +
247 "base: main\n" +
248 "head: " + headSHA + "\n" +
249 "created: 2026-10-22T22:00:00Z\n" +
250 line +
251 "---\n",
252 }
253 if _, err := ParseRequests(pending); err == nil {
254 t.Errorf("an open request with a %s was accepted", field)
255 }
256 }
257}
258
259// A request may record the branch its head came from. It is provenance, not
260// authorisation: the commit is what the gate matches, so a branch that has moved on
261// must not carry the request's approval to the new tip. The field is optional so
262// records written before it existed still load, and it must be a branch name with a
263// commit behind it.
264func TestHeadRefIsAHeadingNotAnApproval(t *testing.T) {
265 t.Parallel()
266 body := func(extra string) map[string]string {
267 return map[string]string{
268 ChangesDir + "/" + reqIDA + "/change.md": "---\n" +
269 `title: "Land the retry"` + "\n" +
270 "state: open\n" +
271 "author: keenan\n" +
272 "base: main\n" +
273 "head: " + headSHA + "\n" +
274 extra +
275 "created: 2026-10-22T22:00:00Z\n" +
276 "---\n",
277 }
278 }
279 list, err := ParseRequests(body("head_ref: contrib/keenan/fix\n"))
280 if err != nil {
281 t.Fatalf("a request naming its branch was refused: %v", err)
282 }
283 if list[0].HeadRef != "contrib/keenan/fix" {
284 t.Fatalf("head_ref = %q, want contrib/keenan/fix", list[0].HeadRef)
285 }
286 for name, extra := range map[string]string{
287 "a full ref": "head_ref: refs/heads/main\n",
288 "a space": "head_ref: two words\n",
289 } {
290 if _, err := ParseRequests(body(extra)); err == nil {
291 t.Errorf("head_ref with %s was accepted", name)
292 }
293 }
294 noHead := map[string]string{
295 ChangesDir + "/" + reqIDA + "/change.md": "---\n" +
296 `title: "Land the retry"` + "\n" +
297 "state: open\n" +
298 "author: keenan\n" +
299 "base: main\n" +
300 "head_ref: main\n" +
301 "created: 2026-10-22T22:00:00Z\n" +
302 "---\n",
303 }
304 if _, err := ParseRequests(noHead); err == nil {
305 t.Error("a branch with no head commit was accepted")
306 }
307
308 // The writer round-trips the field, and refuses what the reader would refuse.
309 now := time.Date(2026, 10, 22, 22, 0, 0, 0, time.UTC)
310 req, err := NewRequest("Land it", "", "keenan", "main", headSHA, now)
311 if err != nil {
312 t.Fatal(err)
313 }
314 req.HeadRef = "contrib/keenan/fix"
315 raw, err := renderRequest(req)
316 if err != nil {
317 t.Fatal(err)
318 }
319 again, err := ParseRequests(map[string]string{ChangesDir + "/" + req.ID + "/change.md": string(raw)})
320 if err != nil {
321 t.Fatalf("the writer produced a request the reader refuses: %v", err)
322 }
323 if again[0].HeadRef != req.HeadRef {
324 t.Fatalf("round trip head_ref = %q, want %q", again[0].HeadRef, req.HeadRef)
325 }
326 bad, err := NewRequest("Land it", "", "keenan", "main", headSHA, now)
327 if err != nil {
328 t.Fatal(err)
329 }
330 bad.HeadRef = "refs/heads/main"
331 if err := validateRequestForWrite(bad); err == nil {
332 t.Error("the writer accepted a head_ref that is not a branch name")
333 }
334
335 // The gate matches the commit. A branch name on the request authorises nothing.
336 reqs := []*Request{{
337 ID: reqIDA, Title: "Land it", Author: "keenan", Base: "main",
338 Head: headSHA, HeadRef: "contrib/keenan/fix", State: "open",
339 Approvals: []Approval{{Principal: "wilbur", Base: "main", Head: headSHA}},
340 }}
341 m, err := FindApprovedMatch(nil, reqs, ApprovalRequirement{Branch: "main", NewTip: headSHA, Required: 1})
342 if err != nil || m == nil {
343 t.Fatalf("the approved head did not match: %v, %v", m, err)
344 }
345 m, err = FindApprovedMatch(nil, reqs, ApprovalRequirement{Branch: "main", NewTip: sideSHA, Required: 1})
346 if err != nil || m != nil {
347 t.Fatalf("a branch name authorised %s: %v, %v", sideSHA, m, err)
348 }
349}
350
351// "merged" is not settable through the generic state writer: it has to travel with
352// the commit, so MarkMerged is the only way in.
353func TestMergedIsNotAGenericState(t *testing.T) {
354 t.Parallel()
355 var g *git.Repo
356 if _, err := SetRequestState(g, reqIDA, "merged", "keenan"); err == nil {
357 t.Fatal("SetRequestState accepted merged")
358 }
359}
360
361// A request may name the issues it closes. The file is the record, so the ids have to
362// be full ones — a prefix that collides is exactly the mistake this format refuses.
363func TestClosesIsAFullIdList(t *testing.T) {
364 t.Parallel()
365 body := func(closes string) map[string]string {
366 return map[string]string{
367 ChangesDir + "/" + reqIDA + "/change.md": "---\n" +
368 `title: "Land the retry"` + "\n" +
369 "state: open\n" +
370 "author: keenan\n" +
371 "base: main\n" +
372 "head: " + headSHA + "\n" +
373 closes +
374 "created: 2026-10-22T22:00:00Z\n" +
375 "---\n",
376 }
377 }
378 const issueA = "01M5JR2JK29KKMAA1T9MDNTRB5"
379 const issueB = "01M5JR2JK29KKMAA1T9MDNTRB6"
380
381 list, err := ParseRequests(body("closes:\n - " + issueB + "\n - " + issueA + "\n - " + issueA + "\n"))
382 if err != nil {
383 t.Fatalf("unexpected error: %v", err)
384 }
385 // Sorted and deduplicated, so two writers that link the same issues render the
386 // same file.
387 if got := strings.Join(list[0].Closes, ","); got != issueA+","+issueB {
388 t.Fatalf("closes = %s, want %s,%s", got, issueA, issueB)
389 }
390 for _, bad := range []string{" - 01M5JR2\n", " - not-an-id\n"} {
391 if _, err := ParseRequests(body("closes:\n" + bad)); err == nil {
392 t.Fatalf("closes entry %q was accepted", strings.TrimSpace(bad))
393 }
394 }
395}
396
397// The mark and the closures are one commit: a merge that cannot write both writes
398// neither, and an issue somebody else already closed is left alone.
399func TestMarkMergedClosesLinkedIssues(t *testing.T) {
400 t.Parallel()
401 root := t.TempDir()
402 if err := git.InitBare(root, "main"); err != nil {
403 t.Fatal(err)
404 }
405 g := git.Open(root)
406 tree, err := g.MkTree(nil)
407 if err != nil {
408 t.Fatal(err)
409 }
410 seed, err := g.CommitTree(tree, "", "seed")
411 if err != nil {
412 t.Fatal(err)
413 }
414 if err := g.UpdateRef(Ref, seed); err != nil {
415 t.Fatal(err)
416 }
417 now := time.Date(2026, 10, 27, 12, 0, 0, 0, time.UTC)
418 linked, err := NewIssue("Linked", "", "keenan", nil, nil, now)
419 if err != nil {
420 t.Fatal(err)
421 }
422 if err := CreateIssue(g, linked, "keenan"); err != nil {
423 t.Fatal(err)
424 }
425 shut, err := NewIssue("Already closed", "", "keenan", nil, nil, now)
426 if err != nil {
427 t.Fatal(err)
428 }
429 if err := CreateIssue(g, shut, "keenan"); err != nil {
430 t.Fatal(err)
431 }
432 if _, err := SetIssueState(g, shut.ID, "closed", "keenan"); err != nil {
433 t.Fatal(err)
434 }
435 req, err := NewRequest("Land it", "", "keenan", "main", headSHA, now)
436 if err != nil {
437 t.Fatal(err)
438 }
439 req.Closes = []string{linked.ID, shut.ID}
440 if err := CreateRequest(g, req, "keenan"); err != nil {
441 t.Fatal(err)
442 }
443 // A link to an issue that does not exist is refused when the request is written.
444 typo, err := NewRequest("Typo", "", "keenan", "main", headSHA, now)
445 if err != nil {
446 t.Fatal(err)
447 }
448 typo.Closes = []string{"01M5JR2JK29KKMAA1T9MDNTRBZ"}
449 if err := CreateRequest(g, typo, "keenan"); err == nil {
450 t.Fatal("a request closing a non-existent issue was written")
451 }
452
453 got, closed, err := MarkMerged(g, req.ID, sideSHA, headSHA, headSHA, "reviewer", now)
454 if err != nil {
455 t.Fatalf("mark: %v", err)
456 }
457 if got.State != "merged" || got.MergedHead != headSHA || got.MergedBase != sideSHA {
458 t.Fatalf("request = %q/%q/%q", got.State, got.MergedBase, got.MergedHead)
459 }
460 if len(closed) != 1 || closed[0] != linked.ID {
461 t.Fatalf("closed = %v, want [%s] — the already-closed issue is not rewritten", closed, linked.ID)
462 }
463 issues, err := LoadIssues(g, Ref)
464 if err != nil {
465 t.Fatal(err)
466 }
467 for _, iss := range issues {
468 if iss.ID == linked.ID && iss.State != "closed" {
469 t.Fatalf("linked issue = %q, want closed", iss.State)
470 }
471 }
472 // Marking again is a no-op, so a retried landing does not re-close anything.
473 // A retry that cannot name the base keeps the one the record already has.
474 if _, again, err := MarkMerged(g, req.ID, "", headSHA, headSHA, "reviewer", now); err != nil || len(again) != 0 {
475 t.Fatalf("second mark = %v, %v", again, err)
476 }
477 if again, err := GetRequest(g, req.ID); err != nil {
478 t.Fatal(err)
479 } else if again.MergedBase != sideSHA {
480 t.Fatalf("a retry without a base dropped it: merged_base = %q, want %q", again.MergedBase, sideSHA)
481 }
482}
483
484// A request's head is what the review and the approvals name, so it may only move
485// while the request is open: a merged request's head is the record of what landed,
486// and a closed one is not going to land at all. The refusal lives in the writer so
487// the CLI and the browser cannot disagree about it.
488func TestSetRequestHeadMovesOnlyAnOpenRequest(t *testing.T) {
489 t.Parallel()
490 root := t.TempDir()
491 if err := git.InitBare(root, "main"); err != nil {
492 t.Fatal(err)
493 }
494 g := git.Open(root)
495 now := time.Date(2026, 10, 27, 12, 0, 0, 0, time.UTC)
496 seed, err := g.CommitTree(mustTree(t, g), "", "seed")
497 if err != nil {
498 t.Fatal(err)
499 }
500 if err := g.UpdateRef(Ref, seed); err != nil {
501 t.Fatal(err)
502 }
503 req, err := NewRequest("Land it", "", "keenan", "main", headSHA, now)
504 if err != nil {
505 t.Fatal(err)
506 }
507 if err := CreateRequest(g, req, "keenan"); err != nil {
508 t.Fatal(err)
509 }
510
511 got, err := SetRequestHead(g, req.ID, sideSHA, "feature", "keenan")
512 if err != nil || got.Head != sideSHA || got.HeadRef != "feature" {
513 t.Fatalf("open head move = %s on %q, %v; want %s on feature", got.Head, got.HeadRef, err, sideSHA)
514 }
515
516 // Closed: the head is history, not a setting.
517 if _, err := SetRequestState(g, req.ID, "closed", "keenan"); err != nil {
518 t.Fatal(err)
519 }
520 if _, err := SetRequestHead(g, req.ID, headSHA, "", "keenan"); err == nil {
521 t.Fatal("a closed request's head moved")
522 }
523
524 // Merged: the head names what landed, so it is settled.
525 if _, err := SetRequestState(g, req.ID, "open", "keenan"); err != nil {
526 t.Fatal(err)
527 }
528 if _, _, err := MarkMerged(g, req.ID, "", sideSHA, sideSHA, "reviewer", now); err != nil {
529 t.Fatal(err)
530 }
531 if _, err := SetRequestHead(g, req.ID, headSHA, "", "keenan"); err == nil {
532 t.Fatal("a merged request's head moved")
533 }
534}
535
536// Retargeting a stacked request changes the base and therefore the diff, so every
537// approval that named the old base stops counting. A base may contain a slash, which
538// is what a stacked request on another feature branch usually needs.
539func TestSetRequestBaseRetargetsAStackedRequest(t *testing.T) {
540 t.Parallel()
541 g := newRequestRepo(t)
542 now := time.Date(2026, 10, 27, 12, 0, 0, 0, time.UTC)
543 r, err := NewRequest("Stacked", "", "keenan", "main", headSHA, now)
544 if err != nil {
545 t.Fatal(err)
546 }
547 if err := CreateRequest(g, r, "keenan"); err != nil {
548 t.Fatal(err)
549 }
550 if _, err := ApproveRequest(g, r.ID, "wilbur", "main", headSHA, now, "wilbur"); err != nil {
551 t.Fatal(err)
552 }
553
554 got, err := SetRequestBase(g, r.ID, "feature/parent", "keenan")
555 if err != nil || got.Base != "feature/parent" {
556 t.Fatalf("retarget = %q, %v; want feature/parent", got.Base, err)
557 }
558 if got.CurrentApprovals() != 0 {
559 t.Fatalf("an approval of the old base still counts: %+v", got.Approvals)
560 }
561 if len(got.Approvals) != 1 || got.Approvals[0].Base != "main" {
562 t.Fatalf("the approval was not left as history: %+v", got.Approvals)
563 }
564
565 // Repeating the same base is not an edit, so the metadata branch does not move.
566 before, err := g.RevParse(Ref)
567 if err != nil {
568 t.Fatal(err)
569 }
570 if _, err := SetRequestBase(g, r.ID, "feature/parent", "keenan"); err != nil {
571 t.Fatal(err)
572 }
573 after, err := g.RevParse(Ref)
574 if err != nil {
575 t.Fatal(err)
576 }
577 if after != before {
578 t.Fatalf("a no-op retarget moved %s: %s -> %s", Ref, before, after)
579 }
580
581 // Closed and merged requests have settled their base.
582 if _, err := SetRequestState(g, r.ID, "closed", "keenan"); err != nil {
583 t.Fatal(err)
584 }
585 if _, err := SetRequestBase(g, r.ID, "main", "keenan"); err == nil {
586 t.Fatal("a closed request's base moved")
587 }
588 if _, err := SetRequestState(g, r.ID, "open", "keenan"); err != nil {
589 t.Fatal(err)
590 }
591 if _, _, err := MarkMerged(g, r.ID, "", headSHA, headSHA, "reviewer", now); err != nil {
592 t.Fatal(err)
593 }
594 if _, err := SetRequestBase(g, r.ID, "main", "keenan"); err == nil {
595 t.Fatal("a merged request's base moved")
596 }
597}
598
599// The request page posts base and head together, so the writer moves both in one
600// metadata commit. Either field changing makes approvals stale; setting both to what
601// they already are is not a commit.
602func TestSetRequestTargetMovesBaseAndFollowedHead(t *testing.T) {
603 t.Parallel()
604 g := newRequestRepo(t)
605 now := time.Date(2026, 10, 27, 12, 0, 0, 0, time.UTC)
606 r, err := NewRequest("Stacked", "", "keenan", "main", headSHA, now)
607 if err != nil {
608 t.Fatal(err)
609 }
610 r.HeadRef = "feature"
611 if err := CreateRequest(g, r, "keenan"); err != nil {
612 t.Fatal(err)
613 }
614 if _, err := ApproveRequest(g, r.ID, "wilbur", "main", headSHA, now, "wilbur"); err != nil {
615 t.Fatal(err)
616 }
617
618 got, err := SetRequestTarget(g, r.ID, "next", sideSHA, "other", "keenan")
619 if err != nil {
620 t.Fatal(err)
621 }
622 if got.Base != "next" || got.Head != sideSHA || got.HeadRef != "other" {
623 t.Fatalf("target = %s <- %s at %s", got.Base, got.Head, got.HeadRef)
624 }
625 if got.CurrentApprovals() != 0 {
626 t.Fatalf("an approval of the old target still counts: %+v", got.Approvals)
627 }
628
629 // Repeating the same target is not an edit, so the metadata branch does not move.
630 before, err := g.RevParse(Ref)
631 if err != nil {
632 t.Fatal(err)
633 }
634 if _, err := SetRequestTarget(g, r.ID, "next", sideSHA, "other", "keenan"); err != nil {
635 t.Fatal(err)
636 }
637 after, err := g.RevParse(Ref)
638 if err != nil {
639 t.Fatal(err)
640 }
641 if after != before {
642 t.Fatalf("a no-op target move changed %s: %s -> %s", Ref, before, after)
643 }
644
645 if _, err := SetRequestState(g, r.ID, "closed", "keenan"); err != nil {
646 t.Fatal(err)
647 }
648 if _, err := SetRequestTarget(g, r.ID, "main", headSHA, "feature", "keenan"); err == nil {
649 t.Fatal("a closed request's target moved")
650 }
651}
652
653// A request opened from a branch follows it: a push moves the head to the new tip,
654// the approval of the old tip stops counting, and a pinned or closed request is left
655// alone. A sync that changes nothing does not grow the metadata log.
656func TestSyncBranchHeadFollowsOpenRequests(t *testing.T) {
657 t.Parallel()
658 g := newRequestRepo(t)
659 now := time.Date(2026, 10, 27, 12, 0, 0, 0, time.UTC)
660
661 mk := func(title, headRef, head string) *Request {
662 r, err := NewRequest(title, "", "keenan", "main", head, now)
663 if err != nil {
664 t.Fatal(err)
665 }
666 r.HeadRef = headRef
667 if err := CreateRequest(g, r, "keenan"); err != nil {
668 t.Fatal(err)
669 }
670 return r
671 }
672 followed := mk("Followed", "feature", headSHA)
673 pinned := mk("Pinned", "", headSHA)
674 closed := mk("Closed", "feature", headSHA)
675 if _, err := SetRequestState(g, closed.ID, "closed", "keenan"); err != nil {
676 t.Fatal(err)
677 }
678 if _, err := ApproveRequest(g, followed.ID, "wilbur", "main", headSHA, now, "wilbur"); err != nil {
679 t.Fatal(err)
680 }
681
682 n, err := SyncBranchHead(g, "feature", sideSHA, "keenan")
683 if err != nil || n != 1 {
684 t.Fatalf("sync = %d, %v; want 1", n, err)
685 }
686 got, err := GetRequest(g, followed.ID)
687 if err != nil {
688 t.Fatal(err)
689 }
690 if got.Head != sideSHA || got.HeadRef != "feature" {
691 t.Fatalf("followed = %s on %q, want %s on feature", got.Head, got.HeadRef, sideSHA)
692 }
693 if got.CurrentApprovals() != 0 {
694 t.Fatalf("the approval of the old head still counts: %+v", got.Approvals)
695 }
696 if len(got.Approvals) != 1 || got.Approvals[0].Head != headSHA {
697 t.Fatalf("the approval was not left as history: %+v", got.Approvals)
698 }
699 for _, want := range []*Request{pinned, closed} {
700 cur, err := GetRequest(g, want.ID)
701 if err != nil {
702 t.Fatal(err)
703 }
704 if cur.Head != headSHA {
705 t.Fatalf("%s moved to %s", cur.Title, cur.Head)
706 }
707 }
708
709 // A ref that already names the head is not a change, so it must not add a commit.
710 before, err := g.RevParse(Ref)
711 if err != nil {
712 t.Fatal(err)
713 }
714 if n, err := SyncBranchHead(g, "feature", sideSHA, "keenan"); err != nil || n != 0 {
715 t.Fatalf("second sync = %d, %v", n, err)
716 }
717 after, err := g.RevParse(Ref)
718 if err != nil {
719 t.Fatal(err)
720 }
721 if after != before {
722 t.Fatalf("a no-op sync moved %s: %s -> %s", Ref, before, after)
723 }
724 if n, err := SyncBranchHead(g, "other", sideSHA, "keenan"); err != nil || n != 0 {
725 t.Fatalf("unrelated branch sync = %d, %v", n, err)
726 }
727}
728
729// The head can move between reading a request and approving it, because a push to
730// the followed branch moves it, and the base can move when a stacked request is
731// retargeted. The approval must name the target the approver saw, so the writer
732// refuses a moved base or head instead of approving the new one.
733func TestApproveRequestRefusesAMovedTarget(t *testing.T) {
734 t.Parallel()
735 g := newRequestRepo(t)
736 now := time.Date(2026, 10, 27, 12, 0, 0, 0, time.UTC)
737 r, err := NewRequest("Land it", "", "keenan", "main", headSHA, now)
738 if err != nil {
739 t.Fatal(err)
740 }
741 r.HeadRef = "feature"
742 if err := CreateRequest(g, r, "keenan"); err != nil {
743 t.Fatal(err)
744 }
745
746 if _, err := ApproveRequest(g, r.ID, "wilbur", "main", sideSHA, now, "wilbur"); err == nil ||
747 !strings.Contains(err.Error(), "moved from") ||
748 !strings.Contains(err.Error(), "review the new head") {
749 t.Fatalf("a moved head was approved: %v", err)
750 }
751 got, err := GetRequest(g, r.ID)
752 if err != nil {
753 t.Fatal(err)
754 }
755 if len(got.Approvals) != 0 {
756 t.Fatalf("a refused approval was written: %+v", got.Approvals)
757 }
758
759 // Retargeting is the other way the review target can move under an approver.
760 if _, err := SetRequestBase(g, r.ID, "next", "keenan"); err != nil {
761 t.Fatal(err)
762 }
763 if _, err := ApproveRequest(g, r.ID, "wilbur", "main", headSHA, now, "wilbur"); err == nil ||
764 !strings.Contains(err.Error(), "moved base from main to next") {
765 t.Fatalf("a moved base was approved: %v", err)
766 }
767 if _, err := SetRequestBase(g, r.ID, "main", "keenan"); err != nil {
768 t.Fatal(err)
769 }
770 if _, err := ApproveRequest(g, r.ID, "wilbur", "main", headSHA, now, "wilbur"); err != nil {
771 t.Fatal(err)
772 }
773 got, err = GetRequest(g, r.ID)
774 if err != nil {
775 t.Fatal(err)
776 }
777 if len(got.Approvals) != 1 || got.Approvals[0].Head != headSHA {
778 t.Fatalf("approval = %+v, want head %s", got.Approvals, headSHA)
779 }
780}
781
782// A merged record names what landed. If the followed branch moves after the landing
783// was prepared, head is frozen to the reviewed commit rather than following the
784// branch past it.
785func TestMarkMergedFreezesTheReviewedHead(t *testing.T) {
786 t.Parallel()
787 g := newRequestRepo(t)
788 now := time.Date(2026, 10, 27, 12, 0, 0, 0, time.UTC)
789 r, err := NewRequest("Land it", "", "keenan", "main", headSHA, now)
790 if err != nil {
791 t.Fatal(err)
792 }
793 r.HeadRef = "feature"
794 if err := CreateRequest(g, r, "keenan"); err != nil {
795 t.Fatal(err)
796 }
797 if _, err := SyncBranchHead(g, "feature", sideSHA, "keenan"); err != nil {
798 t.Fatal(err)
799 }
800 got, _, err := MarkMerged(g, r.ID, "", headSHA, headSHA, "reviewer", now)
801 if err != nil {
802 t.Fatal(err)
803 }
804 if got.Head != headSHA || got.MergedHead != headSHA {
805 t.Fatalf("merged record = head %s, merged %s; want both %s", got.Head, got.MergedHead, headSHA)
806 }
807}
808
809func newRequestRepo(t *testing.T) *git.Repo {
810 t.Helper()
811 root := t.TempDir()
812 if err := git.InitBare(root, "main"); err != nil {
813 t.Fatal(err)
814 }
815 g := git.Open(root)
816 seed, err := g.CommitTree(mustTree(t, g), "", "seed")
817 if err != nil {
818 t.Fatal(err)
819 }
820 if err := g.UpdateRef(Ref, seed); err != nil {
821 t.Fatal(err)
822 }
823 return g
824}
825
826// A link is often discovered after a request is opened, so it can be set later. The
827// same rules apply: a full id, an issue that exists, and only while the request can
828// still land.
829func TestSetRequestClosesEditsAnOpenRequest(t *testing.T) {
830 t.Parallel()
831 root := t.TempDir()
832 if err := git.InitBare(root, "main"); err != nil {
833 t.Fatal(err)
834 }
835 g := git.Open(root)
836 now := time.Date(2026, 10, 27, 12, 0, 0, 0, time.UTC)
837 seed, err := g.CommitTree(mustTree(t, g), "", "seed")
838 if err != nil {
839 t.Fatal(err)
840 }
841 if err := g.UpdateRef(Ref, seed); err != nil {
842 t.Fatal(err)
843 }
844 iss, err := NewIssue("Later", "", "keenan", nil, nil, now)
845 if err != nil {
846 t.Fatal(err)
847 }
848 if err := CreateIssue(g, iss, "keenan"); err != nil {
849 t.Fatal(err)
850 }
851 req, err := NewRequest("Land it", "", "keenan", "main", headSHA, now)
852 if err != nil {
853 t.Fatal(err)
854 }
855 if err := CreateRequest(g, req, "keenan"); err != nil {
856 t.Fatal(err)
857 }
858
859 got, err := SetRequestCloses(g, req.ID, []string{iss.ID}, "keenan")
860 if err != nil {
861 t.Fatalf("link: %v", err)
862 }
863 if len(got.Closes) != 1 || got.Closes[0] != iss.ID {
864 t.Fatalf("closes = %v, want [%s]", got.Closes, iss.ID)
865 }
866 // A second call with the same set is not an edit, so the branch does not move.
867 if _, err := SetRequestCloses(g, req.ID, []string{iss.ID}, "keenan"); err != nil {
868 t.Fatalf("re-link: %v", err)
869 }
870 for _, bad := range [][]string{{"01M5JR2JK29KKMAA1T9MDNTRBZ"}, {"short"}} {
871 if _, err := SetRequestCloses(g, req.ID, bad, "keenan"); err == nil {
872 t.Fatalf("closes %v was accepted", bad)
873 }
874 }
875 // Once merged, the list is a record of what happened, not a setting.
876 if _, _, err := MarkMerged(g, req.ID, sideSHA, headSHA, headSHA, "reviewer", now); err != nil {
877 t.Fatal(err)
878 }
879 if _, err := SetRequestCloses(g, req.ID, nil, "keenan"); err == nil {
880 t.Fatal("a merged request's links were changed")
881 }
882}
883
884// The prose is not what a review names, so correcting it does not start the
885// review over and is not limited to an open request. The writer still validates the
886// record it is about to store.
887func TestSetRequestDetailsEditsTitleAndBody(t *testing.T) {
888 t.Parallel()
889 g := newRequestRepo(t)
890 now := time.Date(2026, 10, 27, 12, 0, 0, 0, time.UTC)
891 req, err := NewRequest("First draft", "why\n", "keenan", "main", headSHA, now)
892 if err != nil {
893 t.Fatal(err)
894 }
895 if err := CreateRequest(g, req, "keenan"); err != nil {
896 t.Fatal(err)
897 }
898
899 got, err := SetRequestDetails(g, req.ID, " Better title ", "clearer why\n", "keenan")
900 if err != nil {
901 t.Fatalf("edit: %v", err)
902 }
903 if got.Title != "Better title" || got.Description != "clearer why\n" {
904 t.Fatalf("edited request = %q / %q", got.Title, got.Description)
905 }
906 stored, err := GetRequest(g, req.ID)
907 if err != nil {
908 t.Fatal(err)
909 }
910 if stored.Title != "Better title" || strings.TrimSpace(stored.Description) != "clearer why" {
911 t.Fatalf("stored request = %q / %q", stored.Title, stored.Description)
912 }
913
914 // An edit that changes nothing is not a commit.
915 tip, err := g.RevParse(Ref)
916 if err != nil {
917 t.Fatal(err)
918 }
919 if _, err := SetRequestDetails(g, req.ID, "Better title", "clearer why\n", "keenan"); err != nil {
920 t.Fatalf("repeat edit: %v", err)
921 }
922 if next, _ := g.RevParse(Ref); next != tip {
923 t.Fatal("an edit that changed nothing moved the branch")
924 }
925
926 if _, err := SetRequestDetails(g, req.ID, " ", "clearer why\n", "keenan"); err == nil {
927 t.Fatal("a blank title was accepted")
928 }
929}
930
931func mustTree(t *testing.T, g *git.Repo) string {
932 t.Helper()
933 tree, err := g.MkTree(nil)
934 if err != nil {
935 t.Fatal(err)
936 }
937 return tree
938}
939