@@ 1,8 1,7 @@
-// Package web is the HTTP layer of compare.sr.ht. It ports the SourceHut chrome
-// (nav/service-switcher, login block, environment banner) to Go html/templates,
-// renders the repository landing, compare (base...head) and single-commit pages
-// server-side, and embeds a compact JSON payload plus the vendored esbuild
-// bundle so the browser renders the diff with @pierre/diffs and @pierre/trees.
+// Package web is the HTTP layer of compare.sr.ht. It renders the repository
+// landing, compare (base...head) and single-commit pages server-side, and
+// embeds a compact JSON payload plus the vendored esbuild bundle so the browser
+// renders the diff with @pierre/diffs and @pierre/trees.
//
// The package owns no state of its own: identity comes from the authz cookie
// middleware, authorization from an authz.Authorizer (git.sr.ht GraphQL), and
@@ 10,6 9,22 @@
// a repository authorizes first (a not-found or forbidden repo is a 404, never
// a 403, so private-repo existence never leaks) and only then reads the disk.
//
+// # The chrome is not ours
+//
+// The nav/service-switcher, the brand, the login block and the environment
+// banner come from sourcecraft.dev/bigbes/sr-ht-ecore/chrome, which every custom
+// service on the instance shares. This package builds one chrome.Service at
+// startup, asks it for a chrome.Page per request, and embeds that Page in
+// viewData so the fields promote into the templates. Nothing here rebuilds the
+// switcher or re-derives a login URL: the copy that used to live in web/chrome.go
+// is exactly what ecore exists to have deleted.
+//
+// Two things about the chrome remain this service's own, because they are about
+// what compare renders and not about the instance: the vendored bundle's href
+// (BundleHref below), and the full-bleed ContainerClass the two diff views set —
+// a side-by-side diff in a centered "container" is a column of code half the
+// window wide.
+//
// # What the cmd layer must wire
//
// Register only installs routes; it assumes the following middleware is already
@@ 35,11 50,18 @@ import (
"regexp"
"github.com/vaughan0/go-ini"
- "sourcecraft.dev/bigbes/sr-ht-core/config"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/chrome"
"sourcecraft.dev/bigbes/sr-ht-compare/authz"
)
+// configSection is this service's literal section in the shared config.ini. It
+// is what the switcher's "which entry is me" test compares against, so it must
+// be spelled the same here, in the config file and in the middleware the cmd
+// layer installs — a service that spelled it two ways would appear in the
+// instance's navigation and fail to recognise itself in it.
+const configSection = "compare.sr.ht"
+
// hashedCSSRe matches the content-addressed stylesheet name so it can be served
// with an immutable cache lifetime (the hash changes whenever the bytes do).
var hashedCSSRe = regexp.MustCompile(`^main\.min\.[0-9a-f]{6,}\.css$`)
@@ 55,43 77,47 @@ var hashedBundleRe = regexp.MustCompile(`^bundle\.[0-9a-f]{6,}\.js$`)
type Server struct {
authorizer authz.Authorizer
reposRoot string
- conf ini.File
- siteName string
- environment string
- metaOrigin string
- compareOrigin string
- hubOrigin string
- cssHref string
- bundleHref string
+ // chromeSvc is the shared page frame of sr-ht-ecore: the brand, the service
+ // switcher, the login block and the environment banner, built once from
+ // config.ini and asked for a per-request chrome.Page in view (chrome.go's
+ // job until this service stopped carrying its own copy).
+ chromeSvc *chrome.Service
+
+ bundleHref string
- nav []navItem
staticFileServer http.Handler
}
// New assembles a Server from the shared SourceHut config. It reads
// [git.sr.ht] repos, [meta.sr.ht] origin and [compare.sr.ht] origin (all
-// required), the [sr.ht] site-name/environment display values, and resolves the
-// hashed stylesheet name by globbing the embedded static FS. A missing required
-// key is a clear error, not a panic, so the cmd layer can fail startup loudly.
+// required), hands the whole file to chrome.NewService — the switcher is a
+// question about every [*.sr.ht] section the instance defines, not about our own
+// keys — and resolves the hashed stylesheet and bundle names by globbing the
+// embedded static FS. A missing required key is a clear error, not a panic, so
+// the cmd layer can fail startup loudly.
func New(conf ini.File, authorizer authz.Authorizer) (*Server, error) {
reposRoot, ok := conf.Get("git.sr.ht", "repos")
if !ok || reposRoot == "" {
return nil, fmt.Errorf("web: [git.sr.ht] repos is required")
}
- metaOrigin := config.GetOrigin(conf, "meta.sr.ht", true)
- if metaOrigin == "" {
+
+ // The two origins are checked through the chrome that will render them
+ // rather than read a second time here, so the startup refusal and the links
+ // on the page cannot disagree about which origins this service has.
+ chromeSvc := chrome.NewService(conf, configSection)
+ if chromeSvc.MetaOrigin() == "" {
return nil, fmt.Errorf("web: [meta.sr.ht] origin is required")
}
- compareOrigin := config.GetOrigin(conf, "compare.sr.ht", true)
- if compareOrigin == "" {
- return nil, fmt.Errorf("web: [compare.sr.ht] origin is required")
+ if chromeSvc.SelfOrigin() == "" {
+ return nil, fmt.Errorf("web: [%s] origin is required", configSection)
}
cssHref, err := resolveCSSHref()
if err != nil {
return nil, err
}
+ chromeSvc.StyleHref = cssHref
bundleHref, err := resolveBundleHref()
if err != nil {
@@ 106,19 132,43 @@ func New(conf ini.File, authorizer authz.Authorizer) (*Server, error) {
return &Server{
authorizer: authorizer,
reposRoot: reposRoot,
- conf: conf,
- siteName: config.GetString(conf, "sr.ht", "site-name", "sourcehut"),
- environment: config.GetString(conf, "sr.ht", "environment", "production"),
- metaOrigin: metaOrigin,
- compareOrigin: compareOrigin,
- hubOrigin: config.GetOrigin(conf, "hub.sr.ht", true),
- cssHref: cssHref,
+ chromeSvc: chromeSvc,
bundleHref: bundleHref,
- nav: buildNav(conf),
staticFileServer: http.StripPrefix("/static/", http.FileServer(http.FS(staticSub))),
}, nil
}
+// viewData is the root value every template is executed against.
+//
+// chrome.Page is embedded rather than copied field by field, so the shared
+// partials — "srht-nav", "srht-env-banner", "srht-repo-list" — find the fields
+// they need on the dot they are handed, and a field ecore adds later arrives here
+// without an edit. The page's own payload lives under Data and is reached as
+// {{.Data.Something}}, which is what keeps a page from shadowing a chrome field.
+type viewData struct {
+ chrome.Page
+
+ // BundleHref is the hashed front-end bundle's URL. It is chrome — every page
+ // may load it — but it is this service's alone: no other service on the
+ // instance ships a diff renderer, so it stays here and not in ecore's Page.
+ BundleHref string
+
+ // Data is the page's own payload.
+ Data any
+}
+
+// view builds the frame for one request: the shared chrome plus a title.
+//
+// The username is whatever the authz cookie middleware resolved, which is "" for
+// a viewer whose cookie is missing, expired or unreadable — so the nav offers
+// login to exactly the viewers the handlers treat as anonymous.
+func (s *Server) view(r *http.Request, title string) viewData {
+ return viewData{
+ Page: s.chromeSvc.Page(r, title, authz.ForContext(r.Context())),
+ BundleHref: s.bundleHref,
+ }
+}
+
// resolveCSSHref globs the embedded static FS for the content-addressed
// stylesheet and returns its site-absolute URL.
func resolveCSSHref() (string, error) {
@@ 16,6 16,8 @@ import (
"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 @@ func gitFixture(t *testing.T) (root, mainSHA string) {
"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 @@ func gitFixture(t *testing.T) (root, mainSHA string) {
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 @@ func testServer(t *testing.T, root string, az authz.Authorizer) http.Handler {
"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 @@ func TestComparePage(t *testing.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 @@ func TestTwoDotVsThreeDot(t *testing.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 @@ func TestComparePatchRoute(t *testing.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 @@ func TestCommitPage(t *testing.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, `<small class="pull-right">private</small>`) {
- 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, `<small class="pull-right">private</small>`,
+ "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, `<ul class="navbar-nav">`):strings.Index(body, "</ul>")]
- 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, `<span class="text-danger">compare</span>`,
+ "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", "/", `<div class="container">`},
+ {"repo", "/~alice/demo", `<div class="container">`},
+ {"compare", "/~alice/demo/compare/main...feature", `<div class="container-fluid">`},
+ {"commit", "/~alice/demo/commit/" + mainSHA, `<div class="container-fluid">`},
+ } {
+ 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 "</script>"
@@ 448,29 409,23 @@ func TestStaticBundleAndCSS(t *testing.T) {
func TestCompareJSONNoScriptBreakout(t *testing.T) {
patch := &gitx.Patch{Text: "diff --git a/x b/x\n"}
files := []gitx.FileChange{{Path: "evil</script><script>alert(1)</script>.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 "</script>" must be escaped, so no literal "</script" tag can
- // appear to close the embedding element.
- if strings.Contains(s, "</script") {
- t.Fatalf("raw </script present in JSON (breakout possible): %s", s)
- }
- // ...and it must appear in its escaped </script form instead, proving
- // the marshaler HTML-escaped the '<'.
- if !strings.Contains(s, "\\u003c/script") {
- t.Fatalf("expected escaped \\u003c/script, got: %s", s)
- }
+ // appear to close the embedding element...
+ assert.NotContainsf(t, s, "</script", "raw </script in the JSON (breakout possible): %s", s)
+ // ...and it must appear in its escaped form instead, proving the marshaler
+ // HTML-escaped the '<'.
+ assert.Containsf(t, s, "\\u003c/script", "expected an escaped \\u003c/script, got: %s", s)
}
func cssName(t *testing.T) string {
t.Helper()
href, err := resolveCSSHref()
- if err != nil {
- t.Fatal(err)
- }
+ require.NoError(t, err)
return strings.TrimPrefix(href, "/static/")
}
@@ 478,9 433,7 @@ func cssName(t *testing.T) string {
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, "</script>")
- 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
}