Read the export test's heap only after pools drop their caches (closes #511)

`TestArchiveExport_Streams` read the heap after one garbage collection, but the libraries the export calls (`regexp` under GORM's table names, `encoding/json`, GORM's row scanning) keep spare buffers in a `sync.Pool`, which keeps them through one collection. Each reading therefore counted however many happened to be cached, which varied by about as much as the test's limit and failed `go test` on `next`. The test now collects twice before each reading, so a reading is the memory the export holds. The limit, the row counts and the claim are unchanged, and an export that does not stream still fails it.

Unverified: the branch's CI run waits for the runner outage to clear.

Model: opus-5-5

.dockerignore

@@ -76,3 +76,9 @@
 # build extracts its own).
 /bin
 /static/js/alpine.min.js
+
+# SQLite databases, which hold the session key and webhook payloads, at
+# any depth.
+**/*.db
+**/*.sqlite
+**/*.sqlite3

.editorconfig

@@ -10,3 +10,6 @@ insert_final_newline = true
 
 [Makefile]
 indent_style = tab
+
+[*.go]
+indent_style = tab

.gitignore

@@ -45,3 +45,39 @@ node_modules/
 [iI][dD]_[eE][cC][dD][sS][aA]_[sS][kK]
 [iI][dD]_[eE][dD]25519
 [iI][dD]_[eE][dD]25519_[sS][kK]
+
+# This repository's own entries, after the shared content above.
+
+# Binaries
+*.exe
+*.dll
+*.so
+*.dylib
+bin/
+/webhooker
+
+# Test binary, built with `go test -c`
+*.test
+
+# Output of the go coverage tool
+*.out
+
+# Go vendor directory
+vendor/
+
+# Data directory (SQLite databases)
+data/
+*.db
+*.sqlite
+*.sqlite3
+
+# Log files
+*.log
+
+# Temporary files
+tmp/
+temp/
+
+# Alpine.js, extracted by `make assets` from its tarball in 3p/, which is
+# what is committed.
+/static/js/alpine.min.js

Dockerfile

@@ -147,9 +147,16 @@ RUN script/assets
 # shown in full, so there is nothing to rerun. The step fails after the rerun
 # whatever its result: the first run already showed the suite is broken.
 #
+# TMPDIR, where the tests keep their SQLite databases, is a tmpfs: SQLite
+# waits for the disk at every commit, and on a busy host that waiting was
+# about 40% of the slowest package's run time. GOTMPDIR keeps go's own
+# build files, the test binaries among them, on disk.
+#
 # bash with pipefail, so that the first run's status is go test's, not tee's.
 SHELL ["/bin/bash", "-o", "pipefail", "-c"]
-RUN go test -race -cover -p 4 -parallel 8 -timeout 90s ./... 2>&1 | tee /tmp/go-test.log && exit 0; \
+RUN --mount=type=tmpfs,target=/tmp/tests,size=512m \
+    export TMPDIR=/tmp/tests GOTMPDIR=/tmp; \
+    go test -race -cover -p 4 -parallel 8 -timeout 90s ./... 2>&1 | tee /tmp/go-test.log && exit 0; \
     tests="$(awk '/^--- FAIL: / { print $3 }' /tmp/go-test.log | paste -s -d '|' -)"; \
     packages="$(awk '/^FAIL\t/ { print $2 }' /tmp/go-test.log)"; \
     if [ -n "$tests" ]; then \

README.md

@@ -44,7 +44,7 @@ make check
 # Run the server from the clone. DATA_DIR defaults to
 # /var/lib/webhooker in every environment, so set it (in .env or the
 # shell) to a writable directory outside the clone: the databases hold
-# the session key, and git does not ignore them.
+# the session key.
 DATA_DIR=../webhooker-data make dev
 
 # Build Docker image

internal/delivery/inflight_test.go

@@ -48,8 +48,9 @@ func fSweepSetup(
 //
 // Every caller drives the dispatch paths synchronously and has already
 // waited for them to return, so anything they queued is in the channel
-// by now. The short grace covers nothing but scheduler jitter, and is
-// kept small because one of these tests runs the drain forty times.
+// by now, and nothing is waited for. A timer here would race the queued
+// tasks: on a busy host it can be due by the time select looks, and
+// select picks at random among the cases that are ready.
 func fDrain(e *delivery.Engine) []delivery.Task {
 	var out []delivery.Task
 
@@ -59,7 +60,7 @@ func fDrain(e *delivery.Engine) []delivery.Task {
 			out = append(out, task)
 		case task := <-e.ExportRetryCh():
 			out = append(out, task)
-		case <-time.After(25 * time.Millisecond):
+		default:
 			return out
 		}
 	}

internal/delivery/target_database_export_test.go

@@ -460,8 +460,13 @@ func TestArchiveExport_OneFileOpenAtATime(t *testing.T) {
 }
 
 // heapPeak is an io.Writer that discards what it is given and records
-// the largest heap it saw at a write. It collects garbage before each
-// reading, so the heap it reads is what is still held.
+// the largest heap it saw at a write. It collects garbage twice before
+// each reading, so the heap it reads is what is still held. Once is not
+// enough: the libraries the export calls (regexp, under GORM's table
+// names, and encoding/json among them) cache buffers in a sync.Pool,
+// which keeps them through one collection, so after one the reading
+// counts however many happen to be cached. That varies from run to run
+// by about as much as the limit in TestArchiveExport_Streams.
 type heapPeak struct {
 	max uint64
 }
@@ -469,6 +474,7 @@ type heapPeak struct {
 func (p *heapPeak) Write(b []byte) (int, error) {
 	var m runtime.MemStats
 
+	runtime.GC()
 	runtime.GC()
 	runtime.ReadMemStats(&m)
 	p.max = max(p.max, m.HeapAlloc)
@@ -498,6 +504,8 @@ func exportHeapGrowth(t *testing.T, rows, bodySize int) uint64 {
 
 	export := listExport(t, path)
 
+	// Twice, for the reason heapPeak gives.
+	runtime.GC()
 	runtime.GC()
 
 	var start runtime.MemStats

internal/server/alpine_browser_test.go

@@ -280,6 +280,23 @@ func click(ctx context.Context, t *testing.T, xpath string) {
 	))
 }
 
+// clickAndLoad clicks the link or button matching an XPath expression
+// and waits, as loadPage does, for the page the click opens to load and
+// for Alpine.js to start on it. Reading earlier, a check can find an
+// element of the page being left, gone by the time its value is read;
+// and the wait in shown is too short for a page load on a busy host.
+func clickAndLoad(ctx context.Context, t *testing.T, xpath string) {
+	t.Helper()
+
+	_, err := chromedp.RunResponse(
+		ctx, chromedp.Click(xpath, chromedp.BySearch),
+	)
+	require.NoError(t, err)
+	require.NoError(t, chromedp.Run(
+		ctx, chromedp.WaitNotPresent("[x-cloak]", chromedp.ByQuery),
+	))
+}
+
 // checkAddEntrypoint loads a webhook page and checks that the add
 // entrypoint form stays hidden until the Add button beside its heading
 // is clicked. The click looks for a button element there, so it also
@@ -422,7 +439,7 @@ func checkAddTarget(
 		)))
 	}
 
-	click(ctx, t, saveButton)
+	clickAndLoad(ctx, t, saveButton)
 	assert.Truef(t, shown(ctx, `//span[text()="`+name+
 		`"]/following-sibling::div/span[text()="`+badge+`"]`),
 		"%s: the added target is not listed as %s", targetType, badge)
@@ -485,10 +502,9 @@ func checkArchiveChoices(ctx context.Context, t *testing.T, url string) {
 		`/following-sibling::span[text()="daily"]`),
 		"a database target added with daily is not listed as daily")
 
-	click(ctx, t, row+`//a[text()="Edit"]`)
+	clickAndLoad(ctx, t, row+`//a[text()="Edit"]`)
 	require.NoError(t, chromedp.Run(
 		ctx,
-		chromedp.WaitReady("#expiry", chromedp.ByQuery),
 		chromedp.Value("#expiry", &editedExpiry, chromedp.ByQuery),
 		chromedp.Value("#rotation", &editedRotation, chromedp.ByQuery),
 	))
@@ -523,7 +539,7 @@ func checkRefusedTarget(ctx context.Context, t *testing.T, url string) {
 		chromedp.Click(forwardQuery, chromedp.ByQuery),
 	))
 
-	click(ctx, t, saveButton)
+	clickAndLoad(ctx, t, saveButton)
 
 	assert.True(t, shown(ctx, reason),
 		"a refused target does not show the reason")
@@ -636,7 +652,7 @@ func checkRefusedEdits(
 			))
 		}
 
-		click(ctx, t, `//button[text()="Save Changes"]`)
+		clickAndLoad(ctx, t, `//button[text()="Save Changes"]`)
 
 		assert.Truef(t, shown(ctx, reason),
 			"%s: a refused save does not show the reason", edit.url)
@@ -760,7 +776,7 @@ func checkEntrypointEdit(
 	require.NoError(t, chromedp.Run(
 		ctx, chromedp.SendKeys(input, "Billing sender", chromedp.ByQuery),
 	))
-	click(ctx, t, saveEdit)
+	clickAndLoad(ctx, t, saveEdit)
 
 	assert.True(t, shown(ctx, `//span[text()="Billing sender"]`),
 		"saving the edit form does not change the description")
@@ -798,7 +814,7 @@ func checkRecentEvents(ctx context.Context, t *testing.T, url string) {
 		"clicking the newest event does not collapse it")
 
 	require.NoError(t, chromedp.Run(ctx, loadPage(url)))
-	click(ctx, t, newest+`/ancestor::div[@x-data][1]//a[text()="Open"]`)
+	clickAndLoad(ctx, t, newest+`/ancestor::div[@x-data][1]//a[text()="Open"]`)
 
 	assert.True(t, shown(ctx, `//h2[text()="Body"]`),
 		"Open does not lead to the event's own page")
@@ -1199,7 +1215,7 @@ func checkNewWebhookTargets(
 					`","rotation":"none"}`
 			}
 
-			click(ctx, t, createButton)
+			clickAndLoad(ctx, t, createButton)
 			require.Truef(t, shown(ctx, `//h1[text()="`+name+`"]`),
 				"%s: the new webhook's page does not open", name)
 
@@ -1260,7 +1276,7 @@ func checkRefusedNewWebhook(ctx context.Context, t *testing.T, url string) {
 		chromedp.SetValue(pruningChoice, "2160h", chromedp.BySearch),
 		chromedp.SetValue("#archive_rotation", "monthly", chromedp.ByQuery),
 	))
-	click(ctx, t, createButton)
+	clickAndLoad(ctx, t, createButton)
 
 	assert.True(t, shown(ctx, `//div[@class="alert-error"]`),
 		"a refused webhook does not show the reason")

script/bootstrap

@@ -11,6 +11,7 @@ ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
 
 PKGMGR=""
 SUDO=""
+APT_UPDATED=""
 
 detect_pkgmgr() {
     [ -n "$PKGMGR" ] && return 0
@@ -39,7 +40,14 @@ pkg_install() {
     detect_pkgmgr
     case "$PKGMGR" in
         nix) nix-env -iA "nixpkgs.$1" ;;
-        apt) $SUDO env DEBIAN_FRONTEND=noninteractive apt-get install -y "$2" ;;
+        apt)
+            # Package lists may be empty (fresh images); refresh once per run.
+            if [ -z "$APT_UPDATED" ]; then
+                $SUDO env DEBIAN_FRONTEND=noninteractive apt-get update
+                APT_UPDATED=1
+            fi
+            $SUDO env DEBIAN_FRONTEND=noninteractive apt-get install -y "$2"
+            ;;
         brew) brew install "$3" ;;
         apk) apk add --no-cache "$4" ;;
     esac