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

Merged
clawbot merged 1 commits from issue-507-load-sensitive-tests into next 2026-10-06 08:51:37 +02:00
3 changed files with 37 additions and 13 deletions
+8 -1
View File
@@ -147,9 +147,16 @@ RUN script/assets
# shown in full, so there is nothing to rerun. The step fails after the rerun # 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. # 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. # bash with pipefail, so that the first run's status is go test's, not tee's.
SHELL ["/bin/bash", "-o", "pipefail", "-c"] 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 '|' -)"; \ tests="$(awk '/^--- FAIL: / { print $3 }' /tmp/go-test.log | paste -s -d '|' -)"; \
packages="$(awk '/^FAIL\t/ { print $2 }' /tmp/go-test.log)"; \ packages="$(awk '/^FAIL\t/ { print $2 }' /tmp/go-test.log)"; \
if [ -n "$tests" ]; then \ if [ -n "$tests" ]; then \
+4 -3
View File
@@ -48,8 +48,9 @@ func fSweepSetup(
// //
// Every caller drives the dispatch paths synchronously and has already // Every caller drives the dispatch paths synchronously and has already
// waited for them to return, so anything they queued is in the channel // 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 { func fDrain(e *delivery.Engine) []delivery.Task {
var out []delivery.Task var out []delivery.Task
@@ -59,7 +60,7 @@ func fDrain(e *delivery.Engine) []delivery.Task {
out = append(out, task) out = append(out, task)
case task := <-e.ExportRetryCh(): case task := <-e.ExportRetryCh():
out = append(out, task) out = append(out, task)
case <-time.After(25 * time.Millisecond): default:
return out return out
} }
} }
+25 -9
View File
@@ -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 // checkAddEntrypoint loads a webhook page and checks that the add
// entrypoint form stays hidden until the Add button beside its heading // entrypoint form stays hidden until the Add button beside its heading
// is clicked. The click looks for a button element there, so it also // 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+ assert.Truef(t, shown(ctx, `//span[text()="`+name+
`"]/following-sibling::div/span[text()="`+badge+`"]`), `"]/following-sibling::div/span[text()="`+badge+`"]`),
"%s: the added target is not listed as %s", targetType, 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"]`), `/following-sibling::span[text()="daily"]`),
"a database target added with daily is not listed as 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( require.NoError(t, chromedp.Run(
ctx, ctx,
chromedp.WaitReady("#expiry", chromedp.ByQuery),
chromedp.Value("#expiry", &editedExpiry, chromedp.ByQuery), chromedp.Value("#expiry", &editedExpiry, chromedp.ByQuery),
chromedp.Value("#rotation", &editedRotation, 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), chromedp.Click(forwardQuery, chromedp.ByQuery),
)) ))
click(ctx, t, saveButton) clickAndLoad(ctx, t, saveButton)
assert.True(t, shown(ctx, reason), assert.True(t, shown(ctx, reason),
"a refused target does not show the 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), assert.Truef(t, shown(ctx, reason),
"%s: a refused save does not show the reason", edit.url) "%s: a refused save does not show the reason", edit.url)
@@ -760,7 +776,7 @@ func checkEntrypointEdit(
require.NoError(t, chromedp.Run( require.NoError(t, chromedp.Run(
ctx, chromedp.SendKeys(input, "Billing sender", chromedp.ByQuery), 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"]`), assert.True(t, shown(ctx, `//span[text()="Billing sender"]`),
"saving the edit form does not change the description") "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") "clicking the newest event does not collapse it")
require.NoError(t, chromedp.Run(ctx, loadPage(url))) 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"]`), assert.True(t, shown(ctx, `//h2[text()="Body"]`),
"Open does not lead to the event's own page") "Open does not lead to the event's own page")
@@ -1199,7 +1215,7 @@ func checkNewWebhookTargets(
`","rotation":"none"}` `","rotation":"none"}`
} }
click(ctx, t, createButton) clickAndLoad(ctx, t, createButton)
require.Truef(t, shown(ctx, `//h1[text()="`+name+`"]`), require.Truef(t, shown(ctx, `//h1[text()="`+name+`"]`),
"%s: the new webhook's page does not open", 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(pruningChoice, "2160h", chromedp.BySearch),
chromedp.SetValue("#archive_rotation", "monthly", chromedp.ByQuery), chromedp.SetValue("#archive_rotation", "monthly", chromedp.ByQuery),
)) ))
click(ctx, t, createButton) clickAndLoad(ctx, t, createButton)
assert.True(t, shown(ctx, `//div[@class="alert-error"]`), assert.True(t, shown(ctx, `//div[@class="alert-error"]`),
"a refused webhook does not show the reason") "a refused webhook does not show the reason")