| @@ -49,6 +49,7 @@ func (f *queueFixture) suggest(u store.User, path string, start, end int, lines |
| 49 | c := &Ctx{User: u, Scope: "full", Store: f.st, Stdout: &out, Stderr: &errOut, JSON: true, |
49 | c := &Ctx{User: u, Scope: "full", Store: f.st, Stdout: &out, Stderr: &errOut, JSON: true, |
| 50 | Stdin: strings.NewReader(body)} |
50 | Stdin: strings.NewReader(body)} |
| 51 | c.Cfg.Server.Root = f.root |
51 | c.Cfg.Server.Root = f.root |
| |
52 | c.Cfg.Limits.WriteRate = -1 |
| 52 | argv := []string{"mr", "diff-comment", f.repo.Path(), "1", "--path", path, |
53 | argv := []string{"mr", "diff-comment", f.repo.Path(), "1", "--path", path, |
| 53 | "--start-line", strconv.Itoa(start), "--line", strconv.Itoa(end), "--file", "-"} |
54 | "--start-line", strconv.Itoa(start), "--line", strconv.Itoa(end), "--file", "-"} |
| 54 | if code := Dispatch(c, argv); code != protocol.ExitOK { |
55 | if code := Dispatch(c, argv); code != protocol.ExitOK { |
| @@ -73,6 +74,19 @@ type threadJSON struct { |
| 73 | Suggestion *SuggestionOut `json:"suggestion"` |
74 | Suggestion *SuggestionOut `json:"suggestion"` |
| 74 | } |
75 | } |
| 75 | |
76 | |
| |
77 | // unlimited lifts the per-account write limit, which these tests would |
| |
78 | // otherwise spend for every test in the package that writes as uid 1. |
| |
79 | func unlimited(c *Ctx) { c.Cfg.Limits.WriteRate = -1 } |
| |
80 | |
| |
81 | func (f *queueFixture) mustWrite(u store.User, argv ...string) string { |
| |
82 | f.t.Helper() |
| |
83 | code, out, errOut := f.runWith(u, unlimited, argv...) |
| |
84 | if code != protocol.ExitOK { |
| |
85 | f.t.Fatalf("%v: exit %d, %s", argv, code, errOut) |
| |
86 | } |
| |
87 | return out |
| |
88 | } |
| |
89 | |
| 76 | func (f *queueFixture) threads(u store.User) []threadJSON { |
90 | func (f *queueFixture) threads(u store.User) []threadJSON { |
| 77 | f.t.Helper() |
91 | f.t.Helper() |
| 78 | out := f.mustRun(u, "mr", "threads", f.repo.Path(), "1", "--json") |
92 | out := f.mustRun(u, "mr", "threads", f.repo.Path(), "1", "--json") |
| @@ -168,6 +182,7 @@ func TestSuggestionRefusals(t *testing.T) { |
| 168 | c := &Ctx{User: f.alice, Scope: "full", Store: f.st, Stdout: &out, Stderr: &errOut, |
182 | c := &Ctx{User: f.alice, Scope: "full", Store: f.st, Stdout: &out, Stderr: &errOut, |
| 169 | Stdin: strings.NewReader(body)} |
183 | Stdin: strings.NewReader(body)} |
| 170 | c.Cfg.Server.Root = f.root |
184 | c.Cfg.Server.Root = f.root |
| |
185 | c.Cfg.Limits.WriteRate = -1 |
| 171 | return Dispatch(c, append([]string{"mr", "diff-comment", f.repo.Path(), "1", "--file", "-"}, extra...)), errOut.String() |
186 | return Dispatch(c, append([]string{"mr", "diff-comment", f.repo.Path(), "1", "--file", "-"}, extra...)), errOut.String() |
| 172 | } |
187 | } |
| 173 | block := "```suggestion\nx\n```\n" |
188 | block := "```suggestion\nx\n```\n" |
| @@ -190,3 +205,263 @@ func TestSuggestionRefusals(t *testing.T) { |
| 190 | } |
205 | } |
| 191 | } |
206 | } |
| 192 | } |
207 | } |
| |
208 | |
| |
209 | // verified gives u a verified primary address, which a commit needs. |
| |
210 | func (f *queueFixture) verified(u store.User) { |
| |
211 | f.t.Helper() |
| |
212 | if err := f.st.AddEmail(u.ID, u.Username+"@example.test", "admin", true); err != nil { |
| |
213 | f.t.Fatal(err) |
| |
214 | } |
| |
215 | } |
| |
216 | |
| |
217 | // applyOK applies thread as u and returns the new commit, checking what |
| |
218 | // every successful apply must have done: one commit on the old head by |
| |
219 | // u, naming the merge request and thread, the merge request moved to |
| |
220 | // it, and the thread resolved. |
| |
221 | func (f *queueFixture) applyOK(u store.User, thread string) string { |
| |
222 | f.t.Helper() |
| |
223 | old := strings.TrimSpace(f.git(f.dir, "rev-parse", "refs/heads/feature")) |
| |
224 | out := f.mustWrite(u, "mr", "apply-suggestion", f.repo.Path(), "1", thread, "--json") |
| |
225 | var env struct { |
| |
226 | Data struct { |
| |
227 | SHA string `json:"sha"` |
| |
228 | } `json:"data"` |
| |
229 | } |
| |
230 | json.Unmarshal([]byte(out), &env) |
| |
231 | sha := env.Data.SHA |
| |
232 | if tip := strings.TrimSpace(f.git(f.dir, "rev-parse", "refs/heads/feature")); tip != sha || sha == "" { |
| |
233 | f.t.Fatalf("feature = %s, apply reported %q", tip, sha) |
| |
234 | } |
| |
235 | if parent := strings.TrimSpace(f.git(f.dir, "rev-parse", sha+"^")); parent != old { |
| |
236 | f.t.Fatalf("parent = %s, want the old head %s", parent, old) |
| |
237 | } |
| |
238 | meta := f.git(f.dir, "log", "-1", "--format=%an <%ae>|%cn <%ce>|%B", sha) |
| |
239 | who := u.Username + " <" + u.Username + "@example.test>" |
| |
240 | if !strings.HasPrefix(meta, who+"|"+who+"|") || !strings.Contains(meta, "Thread "+thread+" on alice/app!1") { |
| |
241 | f.t.Fatalf("commit = %q", meta) |
| |
242 | } |
| |
243 | if mr := f.mr(); mr.HeadSHA != sha { |
| |
244 | f.t.Fatalf("MR head = %s, want %s", mr.HeadSHA, sha) |
| |
245 | } |
| |
246 | if head := strings.TrimSpace(f.git(f.dir, "rev-parse", mrHeadRef(1))); head != sha { |
| |
247 | f.t.Fatalf("MR head ref = %s, want %s", head, sha) |
| |
248 | } |
| |
249 | for _, th := range f.threads(u) { |
| |
250 | if strconv.Itoa(int(th.ID)) == thread && th.Resolved != u.Username { |
| |
251 | f.t.Fatalf("thread %s resolved by %q, want %s", thread, th.Resolved, u.Username) |
| |
252 | } |
| |
253 | } |
| |
254 | return sha |
| |
255 | } |
| |
256 | |
| |
257 | func (f *queueFixture) file(sha, path string) string { |
| |
258 | f.t.Helper() |
| |
259 | return f.git(f.dir, "show", sha+":"+path) |
| |
260 | } |
| |
261 | |
| |
262 | // Each shape of range: several lines to more, deletion, the last line of |
| |
263 | // a file with no final newline, and a CRLF file. |
| |
264 | func TestApplySuggestion(t *testing.T) { |
| |
265 | f := newSuggestFixture(t, nil) |
| |
266 | f.verified(f.alice) |
| |
267 | cases := []struct { |
| |
268 | path string |
| |
269 | start, end int |
| |
270 | lines []string |
| |
271 | want string |
| |
272 | }{ |
| |
273 | {"lib.txt", 2, 3, []string{"TWO", "THREE", "3.5"}, "one\nTWO\nTHREE\n3.5\nfour\nfive\n"}, |
| |
274 | {"lib.txt", 5, 6, nil, "one\nTWO\nTHREE\n3.5\n"}, |
| |
275 | {"tail.txt", 2, 2, []string{"LAST", "more"}, "x\nLAST\nmore"}, |
| |
276 | {"dos.txt", 2, 2, []string{"B", "B2"}, "a\r\nB\r\nB2\r\nc\r\n"}, |
| |
277 | } |
| |
278 | for _, c := range cases { |
| |
279 | id := f.suggest(f.alice, c.path, c.start, c.end, c.lines...) |
| |
280 | sha := f.applyOK(f.alice, id) |
| |
281 | if got := f.file(sha, c.path); got != c.want { |
| |
282 | t.Errorf("%s %d-%d: file = %q, want %q", c.path, c.start, c.end, got, c.want) |
| |
283 | } |
| |
284 | } |
| |
285 | } |
| |
286 | |
| |
287 | // A suggestion whose lines changed, or whose file is gone, is refused; |
| |
288 | // so is one on a repository requiring signed commits, with the command |
| |
289 | // that applies it locally. |
| |
290 | func TestApplySuggestionRefusals(t *testing.T) { |
| |
291 | f := newSuggestFixture(t, nil) |
| |
292 | f.verified(f.alice) |
| |
293 | stale := f.suggest(f.alice, "lib.txt", 1, 1, "ONE") |
| |
294 | gone := f.suggest(f.alice, "tail.txt", 1, 1, "X") |
| |
295 | plain := f.suggest(f.alice, "lib.txt", 2, 2, "two") |
| |
296 | f.write("lib.txt", "uno\ntwo\nthree\nfour\nfive\n") |
| |
297 | f.git(f.src, "rm", "-q", "tail.txt") |
| |
298 | f.git(f.src, "commit", "-q", "-am", "moved on") |
| |
299 | f.moveHead() |
| |
300 | cases := []struct { |
| |
301 | thread string |
| |
302 | code int |
| |
303 | want string |
| |
304 | }{ |
| |
305 | {stale, protocol.ExitUsage, "outdated: " + reasonChanged}, |
| |
306 | {gone, protocol.ExitUsage, reasonGone}, |
| |
307 | {plain, protocol.ExitUsage, "changes nothing"}, |
| |
308 | {"999", protocol.ExitNotFound, "no thread 999"}, |
| |
309 | } |
| |
310 | for _, c := range cases { |
| |
311 | code, _, errOut := f.runWith(f.alice, unlimited, "mr", "apply-suggestion", f.repo.Path(), "1", c.thread) |
| |
312 | if code != c.code || !strings.Contains(errOut, c.want) { |
| |
313 | t.Errorf("thread %s: exit %d %q, want %d with %q", c.thread, code, errOut, c.code, c.want) |
| |
314 | } |
| |
315 | } |
| |
316 | |
| |
317 | if _, err := f.st.UpdateRepoSettings(f.repo.ID, func(s *store.RepoSettings) { s.RequireSignedCommits = true }); err != nil { |
| |
318 | t.Fatal(err) |
| |
319 | } |
| |
320 | ok := f.suggest(f.alice, "lib.txt", 2, 2, "TWO") |
| |
321 | code, _, errOut := f.runWith(f.alice, unlimited, "mr", "apply-suggestion", f.repo.Path(), "1", ok) |
| |
322 | if code != protocol.ExitDenied || !strings.Contains(errOut, "gitbay mr apply-suggestion alice/app 1 "+ok) { |
| |
323 | t.Fatalf("signed repo: exit %d %q", code, errOut) |
| |
324 | } |
| |
325 | } |
| |
326 | |
| |
327 | // The update is held to the pre-receive ref policy a push is: a source |
| |
328 | // branch that is protected under require-mr refuses it. |
| |
329 | func TestApplySuggestionHonoursRefPolicy(t *testing.T) { |
| |
330 | f := newSuggestFixture(t, func(s *store.RepoSettings) { |
| |
331 | s.ProtectedBranches = []string{"feature"} |
| |
332 | s.RequireMR = true |
| |
333 | }) |
| |
334 | f.verified(f.alice) |
| |
335 | id := f.suggest(f.alice, "lib.txt", 1, 1, "ONE") |
| |
336 | before := strings.TrimSpace(f.git(f.dir, "rev-parse", "refs/heads/feature")) |
| |
337 | code, _, errOut := f.runWith(f.alice, unlimited, "mr", "apply-suggestion", f.repo.Path(), "1", id) |
| |
338 | if code != protocol.ExitDenied || !strings.Contains(errOut, "merge requests only") { |
| |
339 | t.Fatalf("exit %d %q, want the require-mr refusal", code, errOut) |
| |
340 | } |
| |
341 | if after := strings.TrimSpace(f.git(f.dir, "rev-parse", "refs/heads/feature")); after != before { |
| |
342 | t.Fatal("refused apply moved the branch") |
| |
343 | } |
| |
344 | } |
| |
345 | |
| |
346 | // Only the source branch's writers apply: a reader cannot. A thread in |
| |
347 | // an unsubmitted review is not applied, and to anyone but its author it |
| |
348 | // does not exist. |
| |
349 | func TestApplySuggestionNeedsWrite(t *testing.T) { |
| |
350 | f := newSuggestFixture(t, nil) |
| |
351 | carol := f.user("carol", "read") |
| |
352 | f.verified(carol) |
| |
353 | id := f.suggest(f.alice, "lib.txt", 1, 1, "ONE") |
| |
354 | code, _, errOut := f.runWith(carol, unlimited, "mr", "apply-suggestion", f.repo.Path(), "1", id) |
| |
355 | if code != protocol.ExitDenied || !strings.Contains(errOut, "only its writers") { |
| |
356 | t.Fatalf("reader: exit %d %q", code, errOut) |
| |
357 | } |
| |
358 | |
| |
359 | var out, stderr strings.Builder |
| |
360 | c := &Ctx{User: f.alice, Scope: "full", Store: f.st, Stdout: &out, Stderr: &stderr, JSON: true, |
| |
361 | Stdin: strings.NewReader("```suggestion\nONE\n```\n")} |
| |
362 | c.Cfg.Server.Root = f.root |
| |
363 | c.Cfg.Limits.WriteRate = -1 |
| |
364 | if code := Dispatch(c, []string{"mr", "diff-comment", f.repo.Path(), "1", "--path", "lib.txt", "--line", "1", "--pending", "--file", "-"}); code != protocol.ExitOK { |
| |
365 | t.Fatalf("pending diff-comment: %s", stderr.String()) |
| |
366 | } |
| |
367 | var env struct { |
| |
368 | Data struct { |
| |
369 | Thread int64 `json:"thread"` |
| |
370 | } `json:"data"` |
| |
371 | } |
| |
372 | json.Unmarshal([]byte(out.String()), &env) |
| |
373 | pending := strconv.Itoa(int(env.Data.Thread)) |
| |
374 | if code, _, errOut := f.runWith(f.alice, unlimited, "mr", "apply-suggestion", f.repo.Path(), "1", pending); code != protocol.ExitUsage || !strings.Contains(errOut, "unsubmitted") { |
| |
375 | t.Errorf("own pending thread: exit %d %q", code, errOut) |
| |
376 | } |
| |
377 | if code, _, _ := f.runWith(carol, unlimited, "mr", "apply-suggestion", f.repo.Path(), "1", pending); code != protocol.ExitNotFound { |
| |
378 | t.Errorf("someone else's pending thread: exit %d, want not found", code) |
| |
379 | } |
| |
380 | } |
| |
381 | |
| |
382 | // Applying is a push by the applier, so a queued merge sees it: a |
| |
383 | // writer's apply keeps the queue, and resolving the thread it came from |
| |
384 | // lets the queued merge land on the new head. |
| |
385 | func TestApplySuggestionReachesQueuedMerge(t *testing.T) { |
| |
386 | f := newSuggestFixture(t, func(s *store.RepoSettings) { s.RequireResolved = true }) |
| |
387 | f.verified(f.alice) |
| |
388 | id := f.suggest(f.alice, "lib.txt", 1, 1, "ONE") |
| |
389 | f.mustWrite(f.alice, "mr", "merge", f.repo.Path(), "1", "--when-ready") |
| |
390 | f.wantQueued("threads resolved") |
| |
391 | sha := f.applyOK(f.alice, id) |
| |
392 | f.wantMergedBy("alice") |
| |
393 | if main := strings.TrimSpace(f.git(f.dir, "rev-parse", "refs/heads/main")); main != sha { |
| |
394 | t.Fatalf("main = %s, want the applied commit %s", main, sha) |
| |
395 | } |
| |
396 | } |
| |
397 | |
| |
398 | // On a merge request from a fork the source branch is the fork's, so its |
| |
399 | // writers apply and the target's do not. The fork writer's apply is a |
| |
400 | // push by someone who cannot merge into the target, which dequeues a |
| |
401 | // merge queued there. |
| |
402 | func TestApplySuggestionFork(t *testing.T) { |
| |
403 | f := newSuggestFixture(t, nil) |
| |
404 | bobID, err := f.st.CreateUser("bob", false) |
| |
405 | if err != nil { |
| |
406 | t.Fatal(err) |
| |
407 | } |
| |
408 | bob, _ := f.st.UserByID(bobID) |
| |
409 | f.verified(f.alice) |
| |
410 | f.verified(bob) |
| |
411 | forkID, err := f.st.CreateRepo("user", bobID, "app", "public") |
| |
412 | if err != nil { |
| |
413 | t.Fatal(err) |
| |
414 | } |
| |
415 | fork, _ := f.st.RepoByID(forkID) |
| |
416 | forkDir := RepoDir(f.root, fork.OwnerName, fork.Name) |
| |
417 | f.git(f.root, "clone", "-q", "--bare", f.src, forkDir) |
| |
418 | f.git(f.dir, "update-ref", mrHeadRef(2), f.headSHA) |
| |
419 | if _, err := f.st.CreateMR(f.repo.ID, bobID, forkID, "feature", "main", "forked", "", f.headSHA, "md", false); err != nil { |
| |
420 | t.Fatal(err) |
| |
421 | } |
| |
422 | var out, errOut strings.Builder |
| |
423 | c := &Ctx{User: f.alice, Scope: "full", Store: f.st, Stdout: &out, Stderr: &errOut, JSON: true, |
| |
424 | Stdin: strings.NewReader("```suggestion\nONE\n```\n")} |
| |
425 | c.Cfg.Server.Root = f.root |
| |
426 | c.Cfg.Limits.WriteRate = -1 |
| |
427 | if code := Dispatch(c, []string{"mr", "diff-comment", f.repo.Path(), "2", "--path", "lib.txt", "--line", "1", "--file", "-"}); code != protocol.ExitOK { |
| |
428 | t.Fatalf("diff-comment: %s", errOut.String()) |
| |
429 | } |
| |
430 | var env struct { |
| |
431 | Data struct { |
| |
432 | Thread int64 `json:"thread"` |
| |
433 | } `json:"data"` |
| |
434 | } |
| |
435 | json.Unmarshal([]byte(out.String()), &env) |
| |
436 | thread := strconv.Itoa(int(env.Data.Thread)) |
| |
437 | |
| |
438 | code, _, stderr := f.runWith(f.alice, unlimited, "mr", "apply-suggestion", f.repo.Path(), "2", thread) |
| |
439 | if code != protocol.ExitDenied || !strings.Contains(stderr, "bob/app:feature") { |
| |
440 | t.Fatalf("target owner on a fork's branch: exit %d %q", code, stderr) |
| |
441 | } |
| |
442 | |
| |
443 | if _, err := f.st.UpdateRepoSettings(f.repo.ID, func(s *store.RepoSettings) { s.RequireApprovals = 1 }); err != nil { |
| |
444 | t.Fatal(err) |
| |
445 | } |
| |
446 | f.mustWrite(f.alice, "mr", "merge", f.repo.Path(), "2", "--when-ready", "--strategy", "merge") |
| |
447 | f.mustWrite(bob, "mr", "apply-suggestion", f.repo.Path(), "2", thread) |
| |
448 | tip := strings.TrimSpace(f.git(forkDir, "rev-parse", "refs/heads/feature")) |
| |
449 | if got := f.git(forkDir, "show", tip+":lib.txt"); !strings.HasPrefix(got, "ONE\ntwo\n") { |
| |
450 | t.Fatalf("fork's lib.txt = %q", got) |
| |
451 | } |
| |
452 | mr, _ := f.st.MRByNumber(f.repo.ID, 2) |
| |
453 | if mr.HeadSHA != tip { |
| |
454 | t.Fatalf("MR head = %s, want the fork's new tip %s", mr.HeadSHA, tip) |
| |
455 | } |
| |
456 | if mr.QueuedAt != "" || mr.State != "open" { |
| |
457 | t.Fatalf("queued merge after the fork writer's apply: state %s queued %q", mr.State, mr.QueuedAt) |
| |
458 | } |
| |
459 | cs, _ := f.st.ListMRComments(mr.ID) |
| |
460 | said := false |
| |
461 | for _, c := range cs { |
| |
462 | said = said || (c.Kind == "system" && strings.Contains(c.Body, "bob pushed and cannot merge")) |
| |
463 | } |
| |
464 | if !said { |
| |
465 | t.Fatalf("timeline does not say why the merge was dequeued: %+v", cs) |
| |
466 | } |
| |
467 | } |