e2e/reviewstanding_test.go

  1package e2e
  2
  3import (
  4	"os"
  5	"path/filepath"
  6	"strings"
  7	"testing"
  8)
  9
 10// TestReviewsCountOnlyFromWriters: a merge gate is a repository's own
 11// rule, so only people the repository trusts can decide it.
 12//
 13// mr review resolves with CanRead and applies no further check, and
 14// reviewGates counted every fresh verdict. On a public repository that
 15// let anyone with an account satisfy require-approvals — defeating
 16// four-eyes review by having two accounts, which on an open-registration
 17// instance means defeating it outright — and equally let them block a
 18// merge the owner wanted (#147).
 19//
 20// Reviewing stays open to everyone: an outside opinion on a public change
 21// is worth having. It just does not decide the gate.
 22func TestReviewsCountOnlyFromWriters(t *testing.T) {
 23	inst := startInstance(t)
 24	ownerKey := inst.newKey(t, "owner")
 25	writerKey := inst.newKey(t, "writer")
 26	strangerKey := inst.newKey(t, "stranger")
 27	inst.admin(t, "admin", "user", "create", "owner", "--key", ownerKey+".pub",
 28		"--email", "owner@example.test", "--verified")
 29	inst.admin(t, "admin", "user", "create", "writer", "--key", writerKey+".pub")
 30	inst.admin(t, "admin", "user", "create", "stranger", "--key", strangerKey+".pub")
 31
 32	// Public, so the stranger can read it and open a review at all.
 33	if _, errOut, code := inst.ssh(t, ownerKey, "", "repo", "create", "owner/pub"); code != 0 {
 34		t.Fatalf("repo create: %s", errOut)
 35	}
 36	if _, _, code := inst.ssh(t, ownerKey, "", "repo", "access", "grant", "owner/pub", "writer", "write"); code != 0 {
 37		t.Fatal("grant failed")
 38	}
 39	if _, _, code := inst.ssh(t, ownerKey, "", "repo", "settings", "require-approvals", "owner/pub", "1"); code != 0 {
 40		t.Fatal("require-approvals failed")
 41	}
 42
 43	env := inst.gitEnv(ownerKey)
 44	work := t.TempDir()
 45	mustGit(t, work, env, "clone", inst.sshURL("owner/pub"), "w")
 46	dir := filepath.Join(work, "w")
 47	os.WriteFile(filepath.Join(dir, "a.txt"), []byte("a\n"), 0o644)
 48	mustGit(t, dir, env, "checkout", "-q", "-b", "main")
 49	mustGit(t, dir, env, "add", ".")
 50	mustGit(t, dir, env, "commit", "-q", "-m", "base")
 51	mustGit(t, dir, env, "push", "-q", "origin", "main")
 52	newMR := func(branch string) {
 53		t.Helper()
 54		mustGit(t, dir, env, "checkout", "-q", "-b", branch, "main")
 55		mustGit(t, dir, env, "commit", "-q", "--allow-empty", "-m", branch)
 56		mustGit(t, dir, env, "push", "-q", "origin", branch)
 57		if _, errOut, code := inst.ssh(t, ownerKey, "", "mr", "create", "owner/pub",
 58			"--source", branch, "--target", "main", "--title", "'"+branch+"'"); code != 0 {
 59			t.Fatalf("mr create %s: %s", branch, errOut)
 60		}
 61	}
 62
 63	// !1 — a stranger's approval must not satisfy the gate.
 64	newMR("feat1")
 65	if _, errOut, code := inst.ssh(t, strangerKey, "", "mr", "review", "owner/pub", "1", "--approve"); code != 0 {
 66		t.Fatalf("a stranger should still be able to review a public MR: %s", errOut)
 67	}
 68	_, errOut, code := inst.ssh(t, ownerKey, "", "mr", "merge", "owner/pub", "1")
 69	if code != 4 || !strings.Contains(errOut, "fresh approval") {
 70		t.Fatalf("a stranger's approval satisfied require-approvals: exit %d, %s", code, errOut)
 71	}
 72	// A writer's approval does.
 73	if _, errOut, code := inst.ssh(t, writerKey, "", "mr", "review", "owner/pub", "1", "--approve"); code != 0 {
 74		t.Fatalf("writer review: %s", errOut)
 75	}
 76	if _, errOut, code := inst.ssh(t, ownerKey, "", "mr", "merge", "owner/pub", "1"); code != 0 {
 77		t.Fatalf("a writer's approval did not satisfy the gate: %s", errOut)
 78	}
 79
 80	// !2 — a stranger's objection must not block the owner.
 81	newMR("feat2")
 82	if _, _, code := inst.ssh(t, writerKey, "", "mr", "review", "owner/pub", "2", "--approve"); code != 0 {
 83		t.Fatal("writer approve failed")
 84	}
 85	if _, errOut, code := inst.ssh(t, strangerKey, "", "mr", "review", "owner/pub", "2", "--request-changes"); code != 0 {
 86		t.Fatalf("stranger request-changes: %s", errOut)
 87	}
 88	if _, errOut, code := inst.ssh(t, ownerKey, "", "mr", "merge", "owner/pub", "2"); code != 0 {
 89		t.Fatalf("a stranger blocked the owner's merge: %s", errOut)
 90	}
 91
 92	// !3 — a writer's objection still blocks, or the gate means nothing.
 93	newMR("feat3")
 94	if _, _, code := inst.ssh(t, writerKey, "", "mr", "review", "owner/pub", "3", "--request-changes"); code != 0 {
 95		t.Fatal("writer request-changes failed")
 96	}
 97	_, errOut, code = inst.ssh(t, ownerKey, "", "mr", "merge", "owner/pub", "3")
 98	if code != 4 || !strings.Contains(errOut, "writer requested changes") {
 99		t.Fatalf("a writer's objection did not block: exit %d, %s", code, errOut)
100	}
101
102	// The review is still recorded and visible either way — it is the
103	// gate that ignores it, not the conversation.
104	out, _, _ := inst.ssh(t, ownerKey, "", "mr", "show", "owner/pub", "2", "--json")
105	if !strings.Contains(out, "stranger") {
106		t.Fatalf("the stranger's review was discarded rather than recorded:\n%s", out)
107	}
108	if !strings.Contains(out, `"counts":false`) {
109		t.Fatalf("mr show does not say the review is not counted:\n%s", out)
110	}
111}