Speed up tests

cmd/licenses/main_test.go

@@ -1,62 +1,81 @@
 package main
 
 import (
+	"slices"
 	"sort"
 	"testing"
 )
 
-// TestScanModulesIsHostIndependent is a regression test.
+// probePkg is a single package whose test dependencies differ between Linux and
+// macOS (via testcontainers/gopsutil). Listing one package keeps this test cheap
+// — scanning ./... across the whole matrix costs ~9 `go list` runs per call, and
+// doing that twice dominated the CI test step.
+const probePkg = "./internal/store"
+
+func probeArgs() []string {
+	return []string{"list", "-deps", "-test", "-json", probePkg}
+}
+
+func paths(mods map[string]moduleInfo) []string {
+	out := make([]string, 0, len(mods))
+	for p := range mods {
+		out = append(out, p)
+	}
+	sort.Strings(out)
+	return out
+}
+
+// TestListModulesPinsPlatform is a regression test.
 //
 // `go list -deps` answers for exactly one GOOS/GOARCH, and the module set
 // genuinely differs between them: Linux pulls in moby/sys/userns and
-// tklauser/numcpus, macOS pulls in ebitengine/purego. scanModules used to
+// tklauser/numcpus, macOS pulls in ebitengine/purego. listModules used to
 // inherit the host's platform, so THIRD-PARTY-NOTICES.md generated on a macOS
 // laptop listed 87 modules while the Linux CI runner computed 88 — the drift
 // check failed on every CI run and regenerating locally could not fix it.
 //
-// scanModules now pins GOOS/GOARCH per invocation and unions a fixed matrix, so
-// the host must not matter. Setting GOOS/GOARCH in the environment is what the
-// old code was (wrongly) sensitive to, which is precisely what this asserts is
-// no longer true.
+// Two things have to hold, and checking only one of them is how this would rot:
+// the requested platform must actually take effect, and the host's own
+// GOOS/GOARCH must not leak through.
 //
 // This shells out to `go list`, so it needs a module cache — the same
 // requirement `go test ./...` already has.
-func TestScanModulesIsHostIndependent(t *testing.T) {
+func TestListModulesPinsPlatform(t *testing.T) {
 	root, err := repoRoot()
 	if err != nil {
 		t.Fatalf("repoRoot: %v", err)
 	}
 
-	scanAs := func(goos, goarch string) []string {
+	list := func(p platform) []string {
 		t.Helper()
-		t.Setenv("GOOS", goos)
-		t.Setenv("GOARCH", goarch)
-
-		mods, _, err := scanModules(root)
+		mods, err := listModules(root, p, probeArgs())
 		if err != nil {
-			t.Fatalf("scanModules as %s/%s: %v", goos, goarch, err)
+			t.Fatalf("listModules for %s/%s: %v", p.GOOS, p.GOARCH, err)
 		}
-		paths := make([]string, 0, len(mods))
-		for _, m := range mods {
-			paths = append(paths, m.Path)
+		if len(mods) == 0 {
+			t.Fatalf("listModules for %s/%s found nothing — is the module cache populated?", p.GOOS, p.GOARCH)
 		}
-		sort.Strings(paths)
-		return paths
+		return paths(mods)
 	}
 
-	asLinux := scanAs("linux", "amd64")
-	asDarwin := scanAs("darwin", "arm64")
+	linux := list(platform{GOOS: "linux", GOARCH: "amd64"})
+	darwin := list(platform{GOOS: "darwin", GOARCH: "arm64"})
 
-	if len(asLinux) == 0 {
-		t.Fatal("scanned no modules at all — is the module cache populated?")
+	// Positive control. Two things make these match: listModules ignoring the
+	// platform it was handed (the regression this guards — both lists then come
+	// from the host), or probePkg's dependencies no longer differing by
+	// platform, which would leave the test proving nothing.
+	if slices.Equal(linux, darwin) {
+		t.Fatalf("%s resolved identically for linux/amd64 and darwin/arm64 (%d modules each). "+
+			"Either listModules is not applying the platform it was given, or probePkg no longer "+
+			"has platform-specific dependencies and this test needs re-pointing.", probePkg, len(linux))
 	}
-	if len(asLinux) != len(asDarwin) {
-		t.Fatalf("module count depends on the host platform: linux/amd64 saw %d, darwin/arm64 saw %d", len(asLinux), len(asDarwin))
-	}
-	for i := range asLinux {
-		if asLinux[i] != asDarwin[i] {
-			t.Errorf("module list depends on the host platform: linux/amd64 has %q where darwin/arm64 has %q", asLinux[i], asDarwin[i])
-		}
+
+	// The actual regression: the host must not influence the answer.
+	t.Setenv("GOOS", "darwin")
+	t.Setenv("GOARCH", "arm64")
+	if got := list(platform{GOOS: "linux", GOARCH: "amd64"}); !slices.Equal(got, linux) {
+		t.Errorf("a linux/amd64 listing changed when the host env said darwin/arm64: %d modules vs %d", len(got), len(linux))
 	}
 }
 

internal/web/assets.go

@@ -11,6 +11,7 @@ import (
 	"path"
 	"strconv"
 	"strings"
+	"sync"
 
 	"github.com/andybalholm/brotli"
 )
@@ -43,10 +44,18 @@ type assetManifest struct {
 	byHashed  map[string]*staticAsset // "ui.<hash>.js" -> asset (fingerprinted requests)
 }
 
-// assets is the process-wide manifest, built from the embedded static FS at
-// package init. The FS is embedded and deterministic, so any failure here is a
+// assets is the process-wide manifest, built from the embedded static FS on
+// first use. The FS is embedded and deterministic, so any failure here is a
 // build/programming error — panic to fail fast at startup (and in tests).
-var assets = buildAssetManifest(staticFS, "static")
+//
+// Built lazily rather than at package init because construction brotli- and
+// gzip-compresses ~1.3MB of static files at their best levels: ~1.5s normally,
+// but ~17s under -race, which instruments every byte of that pure-CPU work. As
+// a package-level var, every test binary importing this package paid it whether
+// or not it ever served an asset — six of them did, dominating the CI test step.
+// Env.SharedRoutes resolves it at registration, so a real server still
+// compresses everything at startup and no request pays the cost.
+var assets = sync.OnceValue(func() *assetManifest { return buildAssetManifest(staticFS, "static") })
 
 func buildAssetManifest(fsys fs.FS, dir string) *assetManifest {
 	sub, err := fs.Sub(fsys, dir)

internal/web/assets_test.go

@@ -18,7 +18,7 @@ var fingerprintedRe = regexp.MustCompile(`^/static/ui\.[0-9a-f]{8}\.js$`)
 func TestAssetURLFingerprints(t *testing.T) {
 	t.Parallel()
 
-	got := assets.URL("/static/ui.js")
+	got := assets().URL("/static/ui.js")
 	if !fingerprintedRe.MatchString(got) {
 		t.Fatalf("asset URL not fingerprinted: got %q, want /static/ui.<8hex>.js", got)
 	}
@@ -27,12 +27,12 @@ func TestAssetURLFingerprints(t *testing.T) {
 	}
 
 	// Unknown assets pass through unchanged so a stray reference still resolves.
-	if got := assets.URL("/static/does-not-exist.js"); got != "/static/does-not-exist.js" {
+	if got := assets().URL("/static/does-not-exist.js"); got != "/static/does-not-exist.js" {
 		t.Fatalf("unknown asset should pass through, got %q", got)
 	}
 
 	// Extensions with a dotted stem keep the hash before the final extension.
-	if got := assets.URL("/static/tabler.min.css"); !regexp.MustCompile(`^/static/tabler\.min\.[0-9a-f]{8}\.css$`).MatchString(got) {
+	if got := assets().URL("/static/tabler.min.css"); !regexp.MustCompile(`^/static/tabler\.min\.[0-9a-f]{8}\.css$`).MatchString(got) {
 		t.Fatalf("dotted-stem asset mis-fingerprinted: %q", got)
 	}
 }
@@ -41,7 +41,7 @@ func TestAssetURLFingerprints(t *testing.T) {
 // StripPrefix + handler path.
 func staticRouter() http.Handler {
 	r := chi.NewRouter()
-	r.Handle("/static/*", http.StripPrefix("/static/", http.HandlerFunc(assets.serveHTTP)))
+	r.Handle("/static/*", http.StripPrefix("/static/", http.HandlerFunc(assets().serveHTTP)))
 	return r
 }
 
@@ -55,7 +55,7 @@ func TestStaticFingerprintedImmutable(t *testing.T) {
 	}
 
 	rec := httptest.NewRecorder()
-	srv.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, assets.URL("/static/ui.js"), nil))
+	srv.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, assets().URL("/static/ui.js"), nil))
 
 	if rec.Code != http.StatusOK {
 		t.Fatalf("fingerprinted asset: status %d, want 200", rec.Code)
@@ -77,7 +77,7 @@ func TestStaticServesBrotli(t *testing.T) {
 		t.Fatalf("read embedded asset: %v", err)
 	}
 
-	req := httptest.NewRequest(http.MethodGet, assets.URL("/static/tabler.min.css"), nil)
+	req := httptest.NewRequest(http.MethodGet, assets().URL("/static/tabler.min.css"), nil)
 	req.Header.Set("Accept-Encoding", "gzip, deflate, br")
 	rec := httptest.NewRecorder()
 	srv.ServeHTTP(rec, req)
@@ -112,7 +112,7 @@ func TestStaticServesGzipWhenBrotliUnwanted(t *testing.T) {
 		t.Fatalf("read embedded asset: %v", err)
 	}
 
-	req := httptest.NewRequest(http.MethodGet, assets.URL("/static/ui.js"), nil)
+	req := httptest.NewRequest(http.MethodGet, assets().URL("/static/ui.js"), nil)
 	req.Header.Set("Accept-Encoding", "gzip") // no br
 	rec := httptest.NewRecorder()
 	srv.ServeHTTP(rec, req)
@@ -144,7 +144,7 @@ func TestStaticIdentityStillVaries(t *testing.T) {
 
 	// No Accept-Encoding: serve raw, but still advertise that the resource varies
 	// so shared caches don't hand a compressed copy to a client that can't decode.
-	req := httptest.NewRequest(http.MethodGet, assets.URL("/static/ui.js"), nil)
+	req := httptest.NewRequest(http.MethodGet, assets().URL("/static/ui.js"), nil)
 	rec := httptest.NewRecorder()
 	srv.ServeHTTP(rec, req)
 
@@ -209,3 +209,14 @@ func TestStaticUnfingerprintedStillServed(t *testing.T) {
 		t.Fatalf("un-fingerprinted asset should not be immutable, got Cache-Control %q", cc)
 	}
 }
+
+// BenchmarkBuildAssetManifest measures what `assets` costs to construct, which
+// is why it is built lazily rather than at package init. Run it with -race to
+// see the number that mattered: the race detector instruments this pure-CPU
+// compression heavily, and as a package-level var it was charged to every test
+// binary that imported this package, not just the ones serving assets.
+func BenchmarkBuildAssetManifest(b *testing.B) {
+	for b.Loop() {
+		buildAssetManifest(staticFS, "static")
+	}
+}

internal/web/env.go

@@ -208,7 +208,7 @@ var templateFuncs = template.FuncMap{
 	"ts": TimeElement,
 	// asset maps a logical static path ("/static/ui.js") to its content-hashed,
 	// immutably-cacheable URL. Use it for every /static/ reference in templates.
-	"asset": assets.URL,
+	"asset": func(l string) string { return assets().URL(l) },
 }
 
 // tsFallbackLayouts maps a display kind to the Go layout used for the server-side
@@ -446,7 +446,13 @@ func limitBody(next http.Handler) http.Handler {
 // row per report.
 func (e *Env) SharedRoutes(r chi.Router) {
 	r.Get("/healthz", e.healthz)
-	r.Handle("/static/*", http.StripPrefix("/static/", http.HandlerFunc(assets.serveHTTP)))
+	// Resolve the manifest here, at registration, rather than inside the
+	// handler. Registration happens during server startup, so the assets are
+	// compressed before the first request exactly as they were when the
+	// manifest was a package-level var — while a test binary that never
+	// registers routes skips the work entirely. See the comment on `assets`.
+	m := assets()
+	r.Handle("/static/*", http.StripPrefix("/static/", http.HandlerFunc(m.serveHTTP)))
 	if e.csp != nil {
 		r.Post(CSPReportPath, e.csp.handleReport)
 	}