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	t.Parallel()
 24	inst := startInstance(t)
 25	ownerKey := inst.newKey(t, "owner")
 26	writerKey := inst.newKey(t, "writer")
 27	strangerKey := inst.newKey(t, "stranger")
 28	inst.admin(t, "admin", "user", "create", "owner", "--key", ownerKey+".pub",
 29		"--email", "owner@example.test", "--verified")
 30	inst.admin(t, "admin", "user", "create", "writer", "--key", writerKey+".pub")
 31	inst.admin(t, "admin", "user", "create", "stranger", "--key", strangerKey+".pub")
 32
 33	// Public, so the stranger can read it and open a review at all.
 34	if _, errOut, code := inst.ssh(t, ownerKey, "", "repo", "create", "owner/pub"); code != 0 {
 35		t.Fatalf("repo create: %s", errOut)
 36	}
 37	if _, _, code := inst.ssh(t, ownerKey, "", "repo", "access", "grant", "owner/pub", "writer", "write"); code != 0 {
 38		t.Fatal("grant failed")
 39	}
 40	if _, _, code := inst.ssh(t, ownerKey, "", "repo", "settings", "require-approvals", "owner/pub", "1"); code != 0 {
 41		t.Fatal("require-approvals failed")
 42	}
 43
 44	env := inst.gitEnv(ownerKey)
 45	work := t.TempDir()
 46	mustGit(t, work, env, "clone", inst.sshURL("owner/pub"), "w")
 47	dir := filepath.Join(work, "w")
 48	os.WriteFile(filepath.Join(dir, "a.txt"), []byte("a\n"), 0o644)
 49	mustGit(t, dir, env, "checkout", "-q", "-b", "main")
 50	mustGit(t, dir, env, "add", ".")
 51	mustGit(t, dir, env, "commit", "-q", "-m", "base")
 52	mustGit(t, dir, env, "push", "-q", "origin", "main")
 53	newMR := func(branch string) {
 54		t.Helper()
 55		mustGit(t, dir, env, "checkout", "-q", "-b", branch, "main")
 56		mustGit(t, dir, env, "commit", "-q", "--allow-empty", "-m", branch)
 57		mustGit(t, dir, env, "push", "-q", "origin", branch)
 58		if _, errOut, code := inst.ssh(t, ownerKey, "", "mr", "create", "owner/pub",
 59			"--source", branch, "--target", "main", "--title", "'"+branch+"'"); code != 0 {
 60			t.Fatalf("mr create %s: %s", branch, errOut)
 61		}
 62	}
 63
 64	// !1 — a stranger's approval must not satisfy the gate.
 65	newMR("feat1")
 66	if _, errOut, code := inst.ssh(t, strangerKey, "", "mr", "review", "owner/pub", "1", "--approve"); code != 0 {
 67		t.Fatalf("a stranger should still be able to review a public MR: %s", errOut)
 68	}
 69	_, errOut, code := inst.ssh(t, ownerKey, "", "mr", "merge", "owner/pub", "1")
 70	if code != 4 || !strings.Contains(errOut, "fresh approval") {
 71		t.Fatalf("a stranger's approval satisfied require-approvals: exit %d, %s", code, errOut)
 72	}
 73	// A writer's approval does.
 74	if _, errOut, code := inst.ssh(t, writerKey, "", "mr", "review", "owner/pub", "1", "--approve"); code != 0 {
 75		t.Fatalf("writer review: %s", errOut)
 76	}
 77	if _, errOut, code := inst.ssh(t, ownerKey, "", "mr", "merge", "owner/pub", "1"); code != 0 {
 78		t.Fatalf("a writer's approval did not satisfy the gate: %s", errOut)
 79	}
 80
 81	// !2 — a stranger's objection must not block the owner.
 82	newMR("feat2")
 83	if _, _, code := inst.ssh(t, writerKey, "", "mr", "review", "owner/pub", "2", "--approve"); code != 0 {
 84		t.Fatal("writer approve failed")
 85	}
 86	if _, errOut, code := inst.ssh(t, strangerKey, "", "mr", "review", "owner/pub", "2", "--request-changes"); code != 0 {
 87		t.Fatalf("stranger request-changes: %s", errOut)
 88	}
 89	if _, errOut, code := inst.ssh(t, ownerKey, "", "mr", "merge", "owner/pub", "2"); code != 0 {
 90		t.Fatalf("a stranger blocked the owner's merge: %s", errOut)
 91	}
 92
 93	// !3 — a writer's objection still blocks, or the gate means nothing.
 94	newMR("feat3")
 95	if _, _, code := inst.ssh(t, writerKey, "", "mr", "review", "owner/pub", "3", "--request-changes"); code != 0 {
 96		t.Fatal("writer request-changes failed")
 97	}
 98	_, errOut, code = inst.ssh(t, ownerKey, "", "mr", "merge", "owner/pub", "3")
 99	if code != 4 || !strings.Contains(errOut, "writer requested changes") {
100		t.Fatalf("a writer's objection did not block: exit %d, %s", code, errOut)
101	}
102
103	// The review is still recorded and visible either way — it is the
104	// gate that ignores it, not the conversation.
105	out, _, _ := inst.ssh(t, ownerKey, "", "mr", "show", "owner/pub", "2", "--json")
106	if !strings.Contains(out, "stranger") {
107		t.Fatalf("the stranger's review was discarded rather than recorded:\n%s", out)
108	}
109	if !strings.Contains(out, `"counts":false`) {
110		t.Fatalf("mr show does not say the review is not counted:\n%s", out)
111	}
112}