Cover the host-page guard and the profile, and run the e2e suite in parallel

.github/workflows/e2e-tests.yml

@@ -61,7 +61,7 @@ jobs:
   tests:
     name: Tests
     needs: vet
-    # generous against the docker build plus one 8m go test: a job cancelled on timeout skips
+    # generous against the docker build plus the full browser suite: a job cancelled on timeout skips
     # its own failure steps, so the run would end with neither logs nor traces
     timeout-minutes: 45
     runs-on: ubuntu-latest
@@ -106,10 +106,9 @@ jobs:
         # log names the run it came from
         env:
           E2E_RUN_ID: ${{ github.run_id }}-${{ github.run_attempt }}
-        # 20m, matching the Makefile. the suite runs about four minutes on a laptop and a runner
-        # is slower, so a tighter budget turns a loaded runner into a timeout panic instead of a
-        # readable failure. the job's own timeout above is what bounds a wedged run
-        run: cd e2e && go test -tags=e2e -count 1 -timeout 20m -v ./...
+        # 20m and four parallel tests, matching the Makefile. the timeout leaves a loaded runner
+        # room to produce a readable test failure; the job's own timeout bounds a wedged run
+        run: cd e2e && go test -tags=e2e -count 1 -parallel 4 -timeout 20m -v ./...
 
       - name: Server logs on failure
         if: failure()

Makefile

@@ -51,9 +51,9 @@ e2e-down:
 # the suite brings the stack up itself when it finds none, so e2e-up is only worth running
 # to keep the containers between invocations
 e2e:
-	cd e2e && go test -tags=e2e -count 1 -timeout 20m ./...
+	cd e2e && go test -tags=e2e -count 1 -parallel 4 -timeout 20m ./...
 
 e2e-ui:
-	cd e2e && E2E_HEADLESS=false E2E_KEEP=1 go test -tags=e2e -count 1 -v -timeout 20m ./...
+	cd e2e && E2E_HEADLESS=false E2E_KEEP=1 go test -tags=e2e -count 1 -parallel 4 -v -timeout 20m ./...
 
 .PHONY: bin docker dockerx release race_test backend frontend rundev e2e e2e-up e2e-down e2e-ui

compose-e2e-test.yml

@@ -1,5 +1,10 @@
 # compose for the e2e suite; the tests themselves run on the host, see e2e/
 #
+# TRUSTED_PROXY stays unset on every instance. The loopback-only harness gives each browser context
+# a distinct X-Real-IP, and the backend honours a forwarding header from any client only while no
+# trusted proxy is configured. Setting one collapses every context onto the docker gateway address
+# and puts the whole parallel run in one rate-limit and vote-dedup bucket.
+#
 # every port is bound to the loopback interface on purpose. this stack runs with a known
 # secret, dev oauth2 and an admin shared id, and `go test` can start it unattended, so it
 # must not be reachable from the network. the suite only ever talks to 127.0.0.1.

e2e/README.md

@@ -24,6 +24,8 @@ make e2e
 make e2e-down
 ```
 
+The default run admits four top-level tests at once. Each browser context has isolated cookies, storage, a unique thread URL and a stable test-only client IP, so the backend's per-IP controls remain local to one reader. The last-comments case reads a site-wide feed, so it stays serial.
+
 Run from `e2e/`; the compose path is relative to it. A single test:
 
 ```
@@ -78,17 +80,20 @@ Both TLS services read a self-signed certificate from `e2e/tls/`, which `e2e/tls
 
 The main instance enables the notify module (`NOTIFY_USERS=email`). Without it `email_notifications` is false in the config, the widget never renders the subscribe control, and the whole subscribe, confirm and unsubscribe flow is unreachable from a browser.
 
-The remark42 instances beyond the first offer anonymous sign-in only, for the reason `remark42-shortedit` does: the dev oauth2 provider binds a port fixed at 8084 and cannot be published twice. `remark42-adminedit` gets its admin from `ADMIN_SHARED_ID`, since the anonymous provider derives the user id from the name and the id for a chosen name can be written into the compose file ahead of time.
+The remark42 instances beyond the first offer anonymous sign-in only, for the reason `remark42-shortedit` does: the dev oauth2 provider binds a port fixed at 8084 and cannot be published twice. `remark42-adminedit` enables email authentication and sets `ADMIN_SHARED_ID` to the SHA-1-derived ID of `adminedit@example.com`, which is the address its browser case uses.
 
-Three settings exist for the tests and not for realism, and each is there for a reason:
+Four settings exist for the tests and not for realism, and each is there for a reason:
 
 - `REMARK_URL` uses a **hostname**, not `127.0.0.1`. The dev oauth2 server binds whatever host it reads out of `REMARK_URL` (`localBindAddr` in go-pkgz/auth), and a loopback bind inside a container cannot be published. The browser maps the names back with `--host-resolver-rules`.
 - `UPDATE_LIMIT=100`, because the default of 0.5 updates a second rejects any test that posts twice in a row.
-- The suite paces its own calls to `/auth/`, which is limited to two requests a second by a bare literal at `backend/app/rest/api/rest.go:242` and not by a setting. See `pauseForAuthLimit`.
+- Each browser context has its own forwarded client IP. Fresh contexts use the limiter's initial allowance; later auth actions wait one token-refill interval through `pauseForAuthLimit`. The limit is a bare literal at `backend/app/rest/api/rest.go:242`, not a setting.
+- `TRUSTED_PROXY` stays unset. The stack is loopback-only, and the harness verifies that its per-context `X-Real-IP` reaches the backend and separates auth limiter buckets.
+
+`VOTES_IP` follows the same reader model: one browser context is one voter IP. A vote-deduplication case that needs repeated actions from one reader must perform them in the same context.
 
 ## What this suite cannot reach
 
-The stack now carries TLS on two services, so behaviour the browser gates on the page protocol is reachable: `Secure` cookies, `SameSite=None`, `Partitioned`, and anything keyed on `window.location.protocol`. `https_test.go` is where those cases live. What is still out of reach is a browser engine other than Chromium for them, since the resolver rules the hostnames need are a Chromium flag.
+The stack carries TLS on two services, so behaviour the browser gates on the page protocol is reachable: `Secure` cookies, `SameSite=None`, `Partitioned`, and anything keyed on `window.location.protocol`. `https_test.go` is where those cases live. What is still out of reach is a browser engine other than Chromium for them, since the resolver rules the hostnames need are a Chromium flag.
 
 The trap that remains is which cookie policy a run is under. Playwright's own default `--disable-features` argument carries `ThirdPartyStoragePartitioning`, and it beats both `--test-third-party-cookie-phaseout` and `--block-third-party-cookies` passed through `Args`. A run configured that way keeps an ordinary third-party cookie exactly as it would with no flags at all, so it proves nothing while looking like it proved something. The lever is `IgnoreDefaultArgs` on the launch options: drop that default entry and re-supply `--disable-features` without that one feature, which is what `TestHTTPS_SessionSurvivesThirdPartyCookieBlocking` does. Measured on a cross-site https embed:
 

e2e/assets_test.go

@@ -22,9 +22,10 @@ const remarkURLPlaceholder = "{% REMARK_URL %}"
 // The demo pages set remark_config.host from location.origin themselves, so the compiled-in
 // fallback is never read and the marker could survive into a release without a test noticing.
 func TestAssets_InstanceURLIsFilledIn(t *testing.T) {
+	t.Parallel()
+
 	page := newPage(t)
 
-	pauseForAuthLimit()
 	_, err := page.Goto(baseURL + "/web/")
 	require.NoError(t, err)
 
@@ -52,6 +53,8 @@ func TestAssets_InstanceURLIsFilledIn(t *testing.T) {
 // The demo pages cannot show this. Their loader builds the bundle's own script url from
 // remark_config.host, so a page without one never gets as far as loading the widget.
 func TestAssets_WidgetRunsOnTheCompiledInURL(t *testing.T) {
+	t.Parallel()
+
 	page := newPage(t)
 
 	var configURL string
@@ -61,7 +64,6 @@ func TestAssets_WidgetRunsOnTheCompiledInURL(t *testing.T) {
 		}
 	})
 
-	pauseForAuthLimit()
 	_, err := page.Goto(fmt.Sprintf("%s/web/iframe.html?site_id=remark&url=%s",
 		baseURL, neturl.QueryEscape(threadURL(t))))
 	require.NoError(t, err)
@@ -93,9 +95,10 @@ func TestAssets_WidgetRunsOnTheCompiledInURL(t *testing.T) {
 // intact, so asserting the element or the attribute proves nothing; only a decoded image has a
 // non-zero natural size. That catches a 404, a malformed URL and a CSP refusal alike.
 func TestAssets_WidgetImagesActuallyLoad(t *testing.T) {
+	t.Parallel()
+
 	page := newPage(t)
 
-	pauseForAuthLimit()
 	_, err := page.Goto(threadURL(t))
 	require.NoError(t, err)
 

e2e/auth_test.go

@@ -24,6 +24,9 @@ func signInDev(t *testing.T, page playwright.Page, frame playwright.FrameLocator
 	})
 	require.NoError(t, err)
 
+	// Opening the provider and completing its callback are separate /auth/ requests. The initial
+	// status probe may still occupy the other token in this reader's bucket.
+	pauseForAuthLimit()
 	require.NoError(t, popup.Locator("text=Authorize").Click())
 
 	// the popup closes itself through the ?selfClose stub. while an oauth sign-in is pending
@@ -64,6 +67,8 @@ func signInAnon(t *testing.T, page playwright.Page, frame playwright.FrameLocato
 }
 
 func TestAuth_DevProviderSignsIn(t *testing.T) {
+	t.Parallel()
+
 	page := newPage(t)
 	frame := openThread(t, page)
 
@@ -72,6 +77,8 @@ func TestAuth_DevProviderSignsIn(t *testing.T) {
 }
 
 func TestAuth_AnonymousSignsIn(t *testing.T) {
+	t.Parallel()
+
 	page := newPage(t)
 	frame := openThread(t, page)
 
@@ -87,6 +94,8 @@ func TestAuth_AnonymousSignsIn(t *testing.T) {
 // mail catcher, and submit it. The token is what the widget sends, so a broken template or a
 // broken token round-trip fails here and not silently in production.
 func TestAuth_EmailSignsIn(t *testing.T) {
+	t.Parallel()
+
 	page := newPage(t)
 	frame := openThread(t, page)
 
@@ -127,13 +136,12 @@ func signInEmail(t *testing.T, page playwright.Page, frame playwright.FrameLocat
 // Sign Out is an icon button, so its title is the only text it carries.
 //
 // The first wait is deliberately short. Everything under /auth/ is capped at two requests a
-// second for the whole suite, a bare literal at backend/app/rest/api/rest.go:242, and a case
-// that signs in on two pages spends that budget twice over. When the read that repaints the
-// panel is the request the limiter refuses, the widget shows signed out over a session that
-// exists, and waiting longer cannot help because nothing will ask again. So on the short wait
-// expiring, hand focus back to the frame: the widget re-probes on visibilitychange and window
-// focus while a sign-in is pending, and by then the cookie is long since set. A sign-in that
-// genuinely failed still fails here, since the second read finds no state either
+// second per reader, a bare literal at backend/app/rest/api/rest.go:242. The initial status and
+// login can spend the bucket before the read that repaints the panel, leaving the widget signed
+// out over a session that exists. Waiting longer cannot help because nothing will ask again, so
+// on the short wait expiring, hand focus back to the frame: the widget re-probes on
+// visibilitychange and window focus while a sign-in is pending. A sign-in that genuinely failed
+// still fails here, since the second read finds no state either
 func assertSignedIn(t *testing.T, page playwright.Page, frame playwright.FrameLocator) {
 	t.Helper()
 
@@ -164,6 +172,8 @@ func verificationToken(t *testing.T, body string) string {
 // assertion after the reload is the point, since a cleared store with a live cookie looks
 // identical until the page comes back
 func TestAuth_SignOutEndsTheSession(t *testing.T) {
+	t.Parallel()
+
 	page := newPage(t)
 	frame := openThread(t, page)
 
@@ -191,6 +201,8 @@ func TestAuth_SignOutEndsTheSession(t *testing.T) {
 // delivered in order, so a widget that has visibly acted on the later message has already had the
 // title message. Without it this would assert on a dropdown that simply has not closed yet.
 func TestAuth_HostPageMessageDoesNotCloseTheDropdown(t *testing.T) {
+	t.Parallel()
+
 	page := newPage(t)
 	stubSignedOut(t, page)
 	embedConfig(t, page, map[string]any{"theme": "light"})
@@ -230,18 +242,22 @@ func TestAuth_HostPageMessageDoesNotCloseTheDropdown(t *testing.T) {
 // response says nothing about it. A reader on a flaky connection otherwise appears to be logged
 // out and cannot get back without signing in again
 func TestAuth_SessionSurvivesATransientStatusFailure(t *testing.T) {
+	t.Parallel()
+
 	page := newPage(t)
 	frame := openThread(t, page)
 	signInDev(t, page, frame)
 
 	// one failure, then out of the way. Unroute and not a counter, so the restored state is the
 	// real endpoint and not a stub standing in for it
 	require.NoError(t, page.Route("**/auth/status**", func(route playwright.Route) {
-		_ = route.Fulfill(playwright.RouteFulfillOptions{
+		if err := route.Fulfill(playwright.RouteFulfillOptions{
 			Status:      playwright.Int(http.StatusInternalServerError),
 			ContentType: playwright.String("application/json"),
 			Body:        playwright.String(`{"error":"failed"}`),
-		})
+		}); err != nil {
+			t.Errorf("fulfill transient auth failure: %v", err)
+		}
 	}))
 
 	pauseForAuthLimit()

e2e/comment_test.go

@@ -19,6 +19,8 @@ import (
 )
 
 func TestComment_PostRendersMarkdownAndSurvivesReload(t *testing.T) {
+	t.Parallel()
+
 	page := newPage(t)
 	frame := openThread(t, page)
 	signInDev(t, page, frame)
@@ -43,6 +45,8 @@ func TestComment_PostRendersMarkdownAndSurvivesReload(t *testing.T) {
 }
 
 func TestComment_ReplyNestsUnderItsParent(t *testing.T) {
+	t.Parallel()
+
 	page := newPage(t)
 	frame := openThread(t, page)
 	signInDev(t, page, frame)
@@ -61,6 +65,8 @@ func TestComment_ReplyNestsUnderItsParent(t *testing.T) {
 }
 
 func TestComment_EditWithinTheDeadline(t *testing.T) {
+	t.Parallel()
+
 	page := newPage(t)
 	frame := openThread(t, page)
 	signInDev(t, page, frame)
@@ -99,6 +105,8 @@ func TestComment_EditWithinTheDeadline(t *testing.T) {
 const editWindow = 15 * time.Second
 
 func TestComment_EditExpiresAfterTheDeadline(t *testing.T) {
+	t.Parallel()
+
 	page := newPage(t)
 	url := threadURLOn(t, shortEditURL)
 	frame := openURL(t, page, url)
@@ -135,6 +143,8 @@ func TestComment_EditExpiresAfterTheDeadline(t *testing.T) {
 }
 
 func TestComment_DeleteRemovesTheText(t *testing.T) {
+	t.Parallel()
+
 	page := newPage(t)
 	frame := openThread(t, page)
 	// deliberately not the dev user: ADMIN_SHARED_ID makes that one an admin, and the widget
@@ -171,6 +181,8 @@ func TestComment_DeleteRemovesTheText(t *testing.T) {
 // posted with: #2040 shipped a version that handed back the rendered text, and everything the
 // author had written in entities or markup was lost on the next save
 func TestComment_EditKeepsTheOriginalSource(t *testing.T) {
+	t.Parallel()
+
 	page := newPage(t)
 	frame := openThread(t, page)
 	signInDev(t, page, frame)
@@ -200,6 +212,8 @@ func TestComment_EditKeepsTheOriginalSource(t *testing.T) {
 // TestComment_DraftSurvivesReloadAndClearsAfterPost covers the local draft. A reader who reloads
 // mid-sentence keeps what they typed, and a reader who posts does not get it handed back
 func TestComment_DraftSurvivesReloadAndClearsAfterPost(t *testing.T) {
+	t.Parallel()
+
 	page := newPage(t)
 	frame := openThread(t, page)
 	signInDev(t, page, frame)
@@ -226,16 +240,20 @@ func TestComment_DraftSurvivesReloadAndClearsAfterPost(t *testing.T) {
 // comment. The text is the only copy they have, so it has to stay in the form, and the failure has
 // to say something instead of swallowing itself
 func TestComment_PostFailureKeepsTheText(t *testing.T) {
+	t.Parallel()
+
 	page := newPage(t)
 	frame := openThread(t, page)
 	signInDev(t, page, frame)
 
 	require.NoError(t, page.Route("**/api/v1/comment?**", func(route playwright.Route) {
-		require.NoError(t, route.Fulfill(playwright.RouteFulfillOptions{
+		if err := route.Fulfill(playwright.RouteFulfillOptions{
 			Status:      playwright.Int(http.StatusBadRequest),
 			ContentType: playwright.String("application/json"),
 			Body:        playwright.String(`{"code":19,"details":"comment contains restricted words","error":"rejected"}`),
-		}))
+		}); err != nil {
+			t.Errorf("fulfill refused comment response: %v", err)
+		}
 	}))
 
 	text := "rejected " + runID
@@ -257,6 +275,8 @@ func TestComment_PostFailureKeepsTheText(t *testing.T) {
 // sees. Both are server-side, so the assertions come after a reload on a second reader's page
 // and not from the moderator's own optimistic render
 func TestComment_AdminPinsAndVerifies(t *testing.T) {
+	t.Parallel()
+
 	text := "moderated " + runID
 
 	// verification is a property of the user and outlives the run in the stack's database, so a
@@ -304,6 +324,8 @@ func TestComment_AdminPinsAndVerifies(t *testing.T) {
 // The second half is the part a reader notices most, since a failed upload that leaves the
 // placeholder behind corrupts what they were writing
 func TestComment_ImageUploadRendersAndRecovers(t *testing.T) {
+	t.Parallel()
+
 	page := newPage(t)
 	frame := openThread(t, page)
 	signInDev(t, page, frame)
@@ -325,11 +347,13 @@ func TestComment_ImageUploadRendersAndRecovers(t *testing.T) {
 			// comes and goes inside one frame, and "the text is unchanged" would hold just as
 			// well for an upload that never started
 			time.Sleep(300 * time.Millisecond)
-			require.NoError(t, route.Fulfill(playwright.RouteFulfillOptions{
+			if err := route.Fulfill(playwright.RouteFulfillOptions{
 				Status:      playwright.Int(http.StatusInternalServerError),
 				ContentType: playwright.String("application/json"),
 				Body:        playwright.String(`{"code":0,"details":"upload failed","error":"nope"}`),
-			}))
+			}); err != nil {
+				t.Errorf("fulfill failed upload response: %v", err)
+			}
 		}))
 		defer func() { require.NoError(t, page.Unroute("**/api/v1/picture**")) }()
 
@@ -379,6 +403,8 @@ func TestComment_ImageUploadRendersAndRecovers(t *testing.T) {
 // answers with its own code, and the widget has to turn that into something the reader can read
 // instead of swallowing it, which is the half no unit test can speak for
 func TestComment_BlockedAuthorCannotPost(t *testing.T) {
+	t.Parallel()
+
 	text := "before the block " + runID
 
 	// the block is permanent and the stack's database outlives the run, so a fixed name would
@@ -412,6 +438,8 @@ func TestComment_BlockedAuthorCannotPost(t *testing.T) {
 // reader arriving afterwards has to find no way to post, and the state has to come from the
 // server and not from the admin's own page
 func TestComment_ReadOnlyThreadTakesTheFormAway(t *testing.T) {
+	t.Parallel()
+
 	page := newPage(t)
 	url := threadURL(t)
 	frame := openURL(t, page, url)
@@ -425,7 +453,6 @@ func TestComment_ReadOnlyThreadTakesTheFormAway(t *testing.T) {
 	// not openURL: it waits for a comment form, and a read-only thread is exactly the case with
 	// no form to wait for
 	reader := newPage(t)
-	pauseForAuthLimit()
 	_, err := reader.Goto(url, playwright.PageGotoOptions{WaitUntil: playwright.WaitUntilStateDomcontentloaded})
 	require.NoError(t, err)