Make the load-sensitive tests wait for what they check (closes #507)

The browser test waits for each page a click opens to load, and for
Alpine.js to start on it, before reading that page; before, a read
could find an element of the page being left.

The delivery tests' drain takes what is queued without a timer. The
dispatch paths queue before they return, and the 25 ms timer could be
due by the time select looked, which then chose at random between it
and a queued task.

The test phase keeps the tests' temporary directories, and with them
their SQLite databases, on a tmpfs: waiting for the disk at each commit
was about 40% of internal/handlers' run time on a busy host.

Model: opus-5-5

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 \

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
+// by now, and nothing is waited for. A timer here would race the queued
-// kept small because one of these tests runs the drain forty times.
+// 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/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")