Compare two references — branches, tags or commits — of any git repository
on this instance, and review the diff file by file.
diff --git a/web/templates/layout.html b/web/templates/layout.html
index 1a53b79f089260a7e1cf993aaefbf716592dbb4c..28ee73a13b7c3e093ed55046e6f064ee5dda796e 100644
--- a/web/templates/layout.html
+++ b/web/templates/layout.html
@@ -1,3 +1,16 @@
+{{/*
+ The SourceHut chrome. The brand, the service switcher, the login block and the
+ environment banner are NOT rendered here: they come from sr-ht-ecore's shared
+ partials ("srht-env-banner", "srht-nav"), which every custom service on the
+ instance draws from one copy. The dot is this package's viewData, which embeds
+ chrome.Page, so the fields those partials read promote into it.
+
+ What is left here is the document, the diff-status colours below (compare's
+ own, and nobody else's), and three seams: "head" and "scripts" are blocks, so
+ a page that needs neither still renders; "content" is a {{template}} and must
+ stay one — a block would give every page an empty default, which is the chrome
+ around a hole served 200 for the page that forgot to define it.
+*/}}
re-requests the page
+ it is on, which is a page load per page load. New refuses to start
+ without a stylesheet, so in a deployed binary this is always taken. */}}
+ {{if .StyleHref}}
{{end}}
{{block "head" .}}{{end}}
- {{if .ShowBanner}}
-
{{template "content" .}}
diff --git a/web/web_test.go b/web/web_test.go
index a162fe94359ef76906590e8b884122865b91087e..3b8117b9e16ee7f9cfb6204d252e54b31f96b5e3 100644
--- a/web/web_test.go
+++ b/web/web_test.go
@@ -16,6 +16,8 @@ "testing"
"github.com/fernet/fernet-go"
"github.com/go-chi/chi/v5"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
"github.com/vaughan0/go-ini"
"sourcecraft.dev/bigbes/sr-ht-core/config"
"sourcecraft.dev/bigbes/sr-ht-core/crypto"
@@ -99,15 +101,11 @@ "GIT_COMMITTER_NAME=Alice", "GIT_COMMITTER_EMAIL=alice@example.com",
"GIT_AUTHOR_DATE="+date, "GIT_COMMITTER_DATE="+date,
)
out, err := cmd.CombinedOutput()
- if err != nil {
- t.Fatalf("git %s: %v\n%s", strings.Join(args, " "), err, out)
- }
+ require.NoErrorf(t, err, "git %s:\n%s", strings.Join(args, " "), out)
return string(out)
}
write := func(name, data string) {
- if err := os.WriteFile(filepath.Join(work, name), []byte(data), 0o644); err != nil {
- t.Fatal(err)
- }
+ require.NoError(t, os.WriteFile(filepath.Join(work, name), []byte(data), 0o644))
}
d1, d2, d3 := "2024-01-01T00:00:00Z", "2024-01-02T00:00:00Z", "2024-01-03T00:00:00Z"
@@ -126,9 +124,7 @@ git(d3, "add", "feature.txt")
git(d3, "commit", "-m", "add feature.txt")
git(d3, "checkout", "main")
- if err := os.MkdirAll(filepath.Join(root, "~alice"), 0o755); err != nil {
- t.Fatal(err)
- }
+ require.NoError(t, os.MkdirAll(filepath.Join(root, "~alice"), 0o755))
bare := filepath.Join(root, "~alice", "demo")
git(d3, "clone", "--bare", work, bare)
@@ -140,9 +136,7 @@ func runGit(t *testing.T, dir string, args ...string) string {
t.Helper()
cmd := exec.Command("git", append([]string{"-C", dir}, args...)...)
out, err := cmd.CombinedOutput()
- if err != nil {
- t.Fatalf("git %s: %v\n%s", strings.Join(args, " "), err, out)
- }
+ require.NoErrorf(t, err, "git %s:\n%s", strings.Join(args, " "), out)
return string(out)
}
@@ -160,18 +154,12 @@ "webhooks": ini.Section{"private-key": testConf.Section("webhooks")["private-key"]},
"compare.sr.ht": ini.Section{"origin": "https://compare.example"},
"meta.sr.ht": ini.Section{"origin": "https://meta.example"},
"git.sr.ht": ini.Section{"origin": "https://git.example", "repos": root},
- // Extra service sections to exercise nav ordering/exclusions.
- "todo.sr.ht": ini.Section{"origin": "https://todo.example"},
- "builds.sr.ht": ini.Section{"origin": "https://builds.example"},
- "lists.sr.ht": ini.Section{"origin": "https://lists.example"},
- "paste.sr.ht": ini.Section{"origin": "https://paste.example"},
- "pages.sr.ht": ini.Section{"origin": "https://pages.example"},
- "hub.sr.ht": ini.Section{"origin": "https://hub.example"},
+ "todo.sr.ht": ini.Section{"origin": "https://todo.example"},
+ "hub.sr.ht": ini.Section{"origin": "https://hub.example"},
}
srv, err := New(conf, az)
- if err != nil {
- t.Fatalf("New: %v", err)
- }
+ require.NoError(t, err, "New")
+
r := chi.NewRouter()
r.Use(config.Middleware(conf, "compare.sr.ht"))
r.Use(authz.Middleware())
@@ -215,40 +203,27 @@ root, _ := gitFixture(t)
h := testServer(t, root, demoAuthorizer())
rec := get(t, h, "/~alice/demo/compare/main...feature", "")
- if rec.Code != http.StatusOK {
- t.Fatalf("status = %d, want 200\n%s", rec.Code, rec.Body.String())
- }
+ require.Equalf(t, http.StatusOK, rec.Code, "body:\n%s", rec.Body.String())
+
body := rec.Body.String()
- if !strings.Contains(body, `id="compare-data"`) {
- t.Fatal("missing compare-data script")
- }
- if !strings.Contains(body, `data-diff-wrap`) {
- t.Fatal("missing long-line wrapping control")
- }
- if !strings.Contains(body, `src="/static/`+bundleName(t)+`"`) {
- t.Fatal("missing hashed bundle script tag")
- }
+ assert.Contains(t, body, `id="compare-data"`, "missing compare-data script")
+ assert.Contains(t, body, `data-diff-wrap`, "missing long-line wrapping control")
+ assert.Contains(t, body, `src="/static/`+bundleName(t)+`"`, "missing hashed bundle script tag")
cd := extractCompareData(t, body)
- if cd.Mode != "compare" {
- t.Fatalf("mode = %q, want compare", cd.Mode)
- }
- if cd.Spec.Base != "main" || cd.Spec.Head != "feature" || !cd.Spec.ThreeDot {
- t.Fatalf("spec = %+v, want base=main head=feature threeDot=true", cd.Spec)
- }
- // feature adds feature.txt relative to the merge base (c1).
+ assert.Equal(t, "compare", cd.Mode)
+ assert.Equal(t, jsonSpec{Base: "main", Head: "feature", ThreeDot: true}, cd.Spec)
+
+ // feature adds feature.txt relative to the merge base (c1), and paths carry
+ // no a/ or b/ diff prefix.
found := false
for _, f := range cd.Files {
if f.Path == "feature.txt" {
found = true
- if strings.HasPrefix(f.Path, "a/") || strings.HasPrefix(f.Path, "b/") {
- t.Fatalf("file path has diff prefix: %q", f.Path)
- }
}
+ assert.NotRegexp(t, `^[ab]/`, f.Path, "file path has a diff prefix")
}
- if !found {
- t.Fatalf("feature.txt not in files: %+v", cd.Files)
- }
+ assert.Truef(t, found, "feature.txt not in files: %+v", cd.Files)
}
func TestTwoDotVsThreeDot(t *testing.T) {
@@ -256,13 +231,10 @@ root, _ := gitFixture(t)
h := testServer(t, root, demoAuthorizer())
two := extractCompareData(t, get(t, h, "/~alice/demo/compare/main..feature", "").Body.String())
- if two.Spec.ThreeDot {
- t.Fatal("main..feature parsed as three-dot")
- }
+ assert.False(t, two.Spec.ThreeDot, "main..feature parsed as three-dot")
+
three := extractCompareData(t, get(t, h, "/~alice/demo/compare/main...feature", "").Body.String())
- if !three.Spec.ThreeDot {
- t.Fatal("main...feature parsed as two-dot")
- }
+ assert.True(t, three.Spec.ThreeDot, "main...feature parsed as two-dot")
}
func TestComparePatchRoute(t *testing.T) {
@@ -270,15 +242,10 @@ root, _ := gitFixture(t)
h := testServer(t, root, demoAuthorizer())
rec := get(t, h, "/~alice/demo/compare/main...feature.patch", "")
- if rec.Code != http.StatusOK {
- t.Fatalf("status = %d, want 200", rec.Code)
- }
- if ct := rec.Header().Get("Content-Type"); !strings.HasPrefix(ct, "text/plain") {
- t.Fatalf("content-type = %q, want text/plain", ct)
- }
- if !strings.Contains(rec.Body.String(), "diff --git") {
- t.Fatalf("patch body missing diff header:\n%s", rec.Body.String())
- }
+ require.Equal(t, http.StatusOK, rec.Code)
+ assert.True(t, strings.HasPrefix(rec.Header().Get("Content-Type"), "text/plain"),
+ "content-type = %q, want text/plain", rec.Header().Get("Content-Type"))
+ assert.Contains(t, rec.Body.String(), "diff --git", "patch body missing diff header")
}
func TestCommitPage(t *testing.T) {
@@ -286,161 +253,155 @@ root, mainSHA := gitFixture(t)
h := testServer(t, root, demoAuthorizer())
rec := get(t, h, "/~alice/demo/commit/"+mainSHA, "")
- if rec.Code != http.StatusOK {
- t.Fatalf("status = %d, want 200\n%s", rec.Code, rec.Body.String())
- }
- if !strings.Contains(rec.Body.String(), `data-diff-wrap`) {
- t.Fatal("missing long-line wrapping control")
- }
+ require.Equalf(t, http.StatusOK, rec.Code, "body:\n%s", rec.Body.String())
+ assert.Contains(t, rec.Body.String(), `data-diff-wrap`, "missing long-line wrapping control")
+
cd := extractCompareData(t, rec.Body.String())
- if cd.Mode != "commit" {
- t.Fatalf("mode = %q, want commit", cd.Mode)
- }
+ assert.Equal(t, "commit", cd.Mode)
// c2 modifies a.txt and adds b.txt.
- if len(cd.Files) == 0 {
- t.Fatal("commit page has no files")
- }
+ assert.NotEmpty(t, cd.Files, "commit page has no files")
}
func TestCommitPatchRoute(t *testing.T) {
root, mainSHA := gitFixture(t)
h := testServer(t, root, demoAuthorizer())
+
rec := get(t, h, "/~alice/demo/commit/"+mainSHA+".patch", "")
- if rec.Code != http.StatusOK {
- t.Fatalf("status = %d, want 200", rec.Code)
- }
- if !strings.Contains(rec.Body.String(), "diff --git") {
- t.Fatal("commit patch missing diff header")
- }
+ require.Equal(t, http.StatusOK, rec.Code)
+ assert.Contains(t, rec.Body.String(), "diff --git", "commit patch missing diff header")
}
func TestUnknownRepoIs404(t *testing.T) {
root, _ := gitFixture(t)
h := testServer(t, root, demoAuthorizer())
+
rec := get(t, h, "/~alice/nope/compare/main...feature", "")
- if rec.Code != http.StatusNotFound {
- t.Fatalf("status = %d, want 404", rec.Code)
- }
+ assert.Equal(t, http.StatusNotFound, rec.Code)
}
func TestPrivateRepoInvisibleIs404(t *testing.T) {
// Authorizer reports the repo as not-found (visibility hidden) even though
// the bare repo exists on disk.
root, _ := gitFixture(t)
- az := &stubAuthorizer{repos: map[string]authz.RepoInfo{}}
- h := testServer(t, root, az)
+ h := testServer(t, root, &stubAuthorizer{repos: map[string]authz.RepoInfo{}})
+
rec := get(t, h, "/~alice/demo", "")
- if rec.Code != http.StatusNotFound {
- t.Fatalf("status = %d, want 404", rec.Code)
- }
+ assert.Equal(t, http.StatusNotFound, rec.Code)
}
func TestAuthorizerTransportErrorIs500(t *testing.T) {
root, _ := gitFixture(t)
- az := &stubAuthorizer{err: errors.New("graphql unreachable")}
- h := testServer(t, root, az)
+ h := testServer(t, root, &stubAuthorizer{err: errors.New("graphql unreachable")})
+
rec := get(t, h, "/~alice/demo", "")
- if rec.Code != http.StatusInternalServerError {
- t.Fatalf("status = %d, want 500 (transport error must not be 404)", rec.Code)
- }
+ assert.Equal(t, http.StatusInternalServerError, rec.Code,
+ "a transport error must not be reported as 404")
}
func TestBadRefIs400(t *testing.T) {
root, _ := gitFixture(t)
h := testServer(t, root, demoAuthorizer())
+
rec := get(t, h, "/~alice/demo/compare/..bad", "")
- if rec.Code != http.StatusBadRequest {
- t.Fatalf("status = %d, want 400", rec.Code)
- }
+ assert.Equal(t, http.StatusBadRequest, rec.Code)
}
func TestIndexAnonymous(t *testing.T) {
root, _ := gitFixture(t)
h := testServer(t, root, demoAuthorizer())
+
rec := get(t, h, "/", "")
+ require.Equal(t, http.StatusOK, rec.Code)
+
body := rec.Body.String()
- if rec.Code != http.StatusOK {
- t.Fatalf("status = %d", rec.Code)
- }
- if !strings.Contains(body, `action="/jump"`) {
- t.Fatal("anonymous index missing jump form")
- }
- if !strings.Contains(body, "return_to=") {
- t.Fatal("login URL missing return_to")
- }
+ assert.Contains(t, body, `action="/jump"`, "anonymous index missing jump form")
+ assert.Contains(t, body, "return_to=", "login URL missing return_to")
}
func TestIndexLoggedIn(t *testing.T) {
root, _ := gitFixture(t)
h := testServer(t, root, demoAuthorizer())
+
rec := get(t, h, "/", "bigbes")
+ require.Equal(t, http.StatusOK, rec.Code)
+
body := rec.Body.String()
- if rec.Code != http.StatusOK {
- t.Fatalf("status = %d", rec.Code)
- }
- if !strings.Contains(body, "/~bigbes/demo") {
- t.Fatal("logged-in index missing repo link from MyRepos")
- }
- if !strings.Contains(body, `
private`) {
- t.Fatal("logged-in index missing visibility label on the event card")
- }
+ assert.Contains(t, body, "/~bigbes/demo", "logged-in index missing repo link from MyRepos")
+ assert.Contains(t, body, `
private`,
+ "logged-in index missing the visibility label on the event card")
}
-func TestNavExclusionsAndActive(t *testing.T) {
+// TestChromeIsRendered checks that the layout really draws the shared partials
+// of sr-ht-ecore — the brand's red service label and the login block. What the
+// switcher contains and how it is ordered is ecore's business and ecore's test;
+// this one only asserts that this service's layout invokes the chrome at all,
+// which is the wiring a bad merge here would break.
+func TestChromeIsRendered(t *testing.T) {
root, _ := gitFixture(t)
h := testServer(t, root, demoAuthorizer())
- // Nav switcher only renders for a logged-in viewer.
- body := get(t, h, "/", "bigbes").Body.String()
- if !strings.Contains(body, "https://git.example") || !strings.Contains(body, "https://todo.example") {
- t.Fatal("nav missing expected services")
- }
- if strings.Contains(body, "https://paste.example") || strings.Contains(body, "https://pages.example") {
- t.Fatal("nav must exclude paste/pages")
- }
- // hub is the brand, never a switcher item.
- nav := body[strings.Index(body, `
")]
- if strings.Contains(nav, "hub.example") {
- t.Fatal("hub must not appear in the switcher list")
- }
- if !strings.Contains(nav, `nav-item active`) {
- t.Fatal("compare should be the active nav item")
+ anon := get(t, h, "/", "").Body.String()
+ assert.Contains(t, anon, `
compare`,
+ "the brand's service label is missing")
+ assert.Contains(t, anon, "Log in", "an anonymous viewer must be offered the login")
+ // The environment is "development" in the test config, so the banner shows.
+ assert.Contains(t, anon, "DEVELOPMENT ENVIRONMENT", "missing the non-production banner")
+
+ viewer := get(t, h, "/", "bigbes").Body.String()
+ assert.Contains(t, viewer, "Logged in as", "the login block does not name the viewer")
+ assert.Contains(t, viewer, "https://hub.example/~bigbes",
+ "the profile link should prefer hub's ~username page")
+ assert.Contains(t, viewer, "https://todo.example", "the switcher is missing a sibling service")
+}
+
+// TestDiffPagesAreFullBleed pins the one chrome decision compare makes for
+// itself: the diff views ask for the full window, because a side-by-side diff in
+// the centered container is a column of code half the page wide.
+func TestDiffPagesAreFullBleed(t *testing.T) {
+ root, mainSHA := gitFixture(t)
+ h := testServer(t, root, demoAuthorizer())
+
+ for _, tc := range []struct {
+ name, target, want string
+ }{
+ {"index", "/", `
`},
+ {"repo", "/~alice/demo", `
`},
+ {"compare", "/~alice/demo/compare/main...feature", `
`},
+ {"commit", "/~alice/demo/commit/" + mainSHA, `
`},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ rec := get(t, h, tc.target, "")
+ require.Equal(t, http.StatusOK, rec.Code)
+ assert.Contains(t, rec.Body.String(), tc.want)
+ })
}
}
func TestHealthz(t *testing.T) {
root, _ := gitFixture(t)
h := testServer(t, root, demoAuthorizer())
+
rec := get(t, h, "/healthz", "")
- if rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), "ok") {
- t.Fatalf("healthz = %d %q", rec.Code, rec.Body.String())
- }
+ require.Equal(t, http.StatusOK, rec.Code)
+ assert.Contains(t, rec.Body.String(), "ok")
}
func TestStaticBundleAndCSS(t *testing.T) {
root, _ := gitFixture(t)
- srvHandler := testServer(t, root, demoAuthorizer())
+ h := testServer(t, root, demoAuthorizer())
bundle := bundleName(t)
- rec := get(t, srvHandler, "/static/"+bundle, "")
- if rec.Code != http.StatusOK {
- t.Fatalf("%s status = %d", bundle, rec.Code)
- }
- if ct := rec.Header().Get("Content-Type"); !strings.Contains(ct, "javascript") {
- t.Fatalf("%s content-type = %q", bundle, ct)
- }
- if cc := rec.Header().Get("Cache-Control"); !strings.Contains(cc, "immutable") {
- t.Fatalf("hashed bundle cache-control = %q, want immutable", cc)
- }
+ rec := get(t, h, "/static/"+bundle, "")
+ require.Equalf(t, http.StatusOK, rec.Code, "%s", bundle)
+ assert.Contains(t, rec.Header().Get("Content-Type"), "javascript")
+ assert.Contains(t, rec.Header().Get("Cache-Control"), "immutable",
+ "a hashed bundle must be cacheable forever")
css := cssName(t)
- rec = get(t, srvHandler, "/static/"+css, "")
- if rec.Code != http.StatusOK {
- t.Fatalf("css status = %d", rec.Code)
- }
- if cc := rec.Header().Get("Cache-Control"); !strings.Contains(cc, "immutable") {
- t.Fatalf("hashed css cache-control = %q, want immutable", cc)
- }
+ rec = get(t, h, "/static/"+css, "")
+ require.Equalf(t, http.StatusOK, rec.Code, "%s", css)
+ assert.Contains(t, rec.Header().Get("Cache-Control"), "immutable",
+ "a hashed stylesheet must be cacheable forever")
}
// TestCompareJSONNoScriptBreakout verifies a file path containing ""
@@ -448,29 +409,23 @@ // cannot break out of the embedded .txt", Status: "A", Additions: 1}}
+
html, err := buildCompareJSON("compare", patch, files, jsonSpec{Base: "a", Head: "b"})
- if err != nil {
- t.Fatal(err)
- }
+ require.NoError(t, err)
+
s := string(html)
// The '<' of "" must be escaped, so no literal ".js).
func bundleName(t *testing.T) string {
t.Helper()
href, err := resolveBundleHref()
- if err != nil {
- t.Fatal(err)
- }
+ require.NoError(t, err)
return strings.TrimPrefix(href, "/static/")
}
@@ -489,17 +442,13 @@ func extractCompareData(t *testing.T, body string) compareData {
t.Helper()
const open = `id="compare-data" type="application/json">`
i := strings.Index(body, open)
- if i < 0 {
- t.Fatalf("no compare-data script in body:\n%s", body)
- }
+ require.GreaterOrEqualf(t, i, 0, "no compare-data script in body:\n%s", body)
+
rest := body[i+len(open):]
j := strings.Index(rest, "")
- if j < 0 {
- t.Fatal("compare-data script not closed")
- }
+ require.GreaterOrEqual(t, j, 0, "compare-data script not closed")
+
var cd compareData
- if err := json.Unmarshal([]byte(rest[:j]), &cd); err != nil {
- t.Fatalf("decode compare-data: %v\nraw: %s", err, rest[:j])
- }
+ require.NoErrorf(t, json.Unmarshal([]byte(rest[:j]), &cd), "raw: %s", rest[:j])
return cd
}