diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index d8af5b5b..d977a08e 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -88,8 +88,12 @@ jobs: key: playwright-${{ hashFiles('e2e/go.sum') }} restore-keys: playwright- + # E2E_STAMP is what the suite compares the running stack against, so a stack started here + # has to carry the same value `make e2e-up` and the suite itself would give it - name: Build & start the stack - run: COMPOSE_DOCKER_CLI_BUILD=1 DOCKER_BUILDKIT=1 docker compose -f compose-e2e-test.yml up -d --build --quiet-pull --wait + run: | + COMPOSE_DOCKER_CLI_BUILD=1 DOCKER_BUILDKIT=1 E2E_STAMP=$(./e2e/stamp.sh) \ + docker compose -f compose-e2e-test.yml up -d --build --quiet-pull --wait # no retry: a failure here is evidence about a suite too young to have a flake rate, # and a rerun is how an intermittent regression becomes invisible. revisit when there @@ -99,7 +103,10 @@ jobs: # log names the run it came from env: E2E_RUN_ID: ${{ github.run_id }}-${{ github.run_attempt }} - run: cd e2e && go test -tags=e2e -count 1 -timeout 8m -v ./... + # 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 ./... - name: Server logs on failure if: failure() diff --git a/Makefile b/Makefile index 366b9073..f1b4f677 100644 --- a/Makefile +++ b/Makefile @@ -39,8 +39,10 @@ rundev: docker compose -f compose-private.yml build docker compose -f compose-private.yml up +# stamped the same way the suite stamps a stack it starts itself, so one brought up here is +# accepted instead of rejected as belonging to another checkout e2e-up: - docker compose -f compose-e2e-test.yml up -d --build --quiet-pull --wait + E2E_STAMP=$$(./e2e/stamp.sh) docker compose -f compose-e2e-test.yml up -d --build --quiet-pull --wait e2e-down: docker compose -f compose-e2e-test.yml down -v diff --git a/compose-e2e-test.yml b/compose-e2e-test.yml index 85ab0067..bf6493fa 100644 --- a/compose-e2e-test.yml +++ b/compose-e2e-test.yml @@ -21,7 +21,7 @@ services: # dev oauth2 provider, registered from the REMARK_URL hostname - "127.0.0.1:8084:8084" - # the hostname has to be a name rather than 127.0.0.1: the dev oauth2 server binds the + # the hostname has to be a name, not 127.0.0.1: the dev oauth2 server binds the # host it reads out of REMARK_URL, and a loopback bind inside the container cannot be # published. the suite maps the name back to 127.0.0.1 in the browser, see e2e/ environment: @@ -35,13 +35,27 @@ services: - ADMIN_SHARED_ID=dev_user # set admin flag for default user on local oauth2 - AUTH_ANON=true - AUTH_EMAIL_ENABLE=true - # the auth sender reads AUTH_EMAIL_FROM; NOTIFY_EMAIL_FROM feeds the notify module, - # which this stack does not enable + # the auth sender reads AUTH_EMAIL_FROM, the notify module reads NOTIFY_EMAIL_FROM. both + # go to mailpit, which the suite reads back over HTTP - AUTH_EMAIL_FROM=remark42@example.com + # without this email_notifications is false and the subscribe control never renders, so + # the whole subscribe, confirm and unsubscribe flow is unreachable from a browser + - NOTIFY_USERS=email + - NOTIFY_EMAIL_FROM=notify@example.com - SMTP_HOST=mailpit - SMTP_PORT=1025 # default is 0.5/sec, which the suite exceeds whenever a test posts twice in a row - UPDATE_LIMIT=100 + # digest of the sources this stack was brought up from, from e2e/stamp.sh, which the suite + # reads back and compares against the checkout it is running in. every checkout builds the + # image tag above, so a stack from another worktree answers on these ports and passes every + # readiness probe while serving code nobody is looking at. remark42 ignores it. + # + # set as the environment and not as a build argument on purpose: an argument reaching + # the backend stage is part of the go build's cache key, so every frontend edit would + # rebuild the backend too. the cost is that `docker compose up` without --build would stamp + # a stale image, which neither `make e2e-up` nor the suite does + - E2E_SOURCE_STAMP=${E2E_STAMP:-unstamped} volumes: - remark42-e2e-var:/srv/var depends_on: @@ -88,6 +102,149 @@ services: timeout: 3s retries: 30 + # ADMIN_EDIT gives admins an unlimited edit window, which #1986 reported the frontend ignoring. + # a separate instance because the setting is global and removes the countdown the ordinary + # deadline cases assert on + remark42-adminedit: + image: ghcr.io/umputun/remark42:dev + container_name: "remark42-e2e-adminedit" + pull_policy: never + depends_on: + remark42: + condition: service_started + mailpit: + condition: service_healthy + + ports: + - "127.0.0.1:8082:8080" + + environment: + - REMARK_URL=http://remark42-adminedit:8082 + - SECRET=12345 + - ADMIN_EDIT=true + # email auth, because its user id is the only one that can be written down ahead of time. + # the dev provider's port is fixed at 8084 and cannot be published twice, and the anonymous + # id is hashed from the name AND the client address (server.go:1220), which differs between + # a laptop and a runner, so an id pinned here would make an admin of nobody on CI. + # the email id is sha1 of the address alone (go-pkgz/auth provider/verify.go:92), so this is + # email_$(printf adminedit@example.com | shasum) and the test signs in as that address + - AUTH_EMAIL_ENABLE=true + - AUTH_EMAIL_FROM=remark42@example.com + - SMTP_HOST=mailpit + - SMTP_PORT=1025 + - ADMIN_SHARED_ID=email_0212c0d9ebf846e898888d617f82d4147747bdc3 + # short, so "after the deadline" is a few seconds instead of the default five minutes + - EDIT_TIME=15s + - UPDATE_LIMIT=100 + volumes: + - remark42-e2e-adminedit-var:/srv/var + healthcheck: + test: ["CMD", "curl", "--fail", "http://localhost:8080/ping"] + interval: 2s + timeout: 3s + retries: 30 + + # AUTH_SEND_JWT_HEADER moves the token out of the cookie and into a header the frontend has to + # keep for itself, which #1877 reported losing on every reload + remark42-jwtheader: + image: ghcr.io/umputun/remark42:dev + container_name: "remark42-e2e-jwtheader" + pull_policy: never + depends_on: + remark42: + condition: service_started + + ports: + - "127.0.0.1:8083:8080" + + environment: + - REMARK_URL=http://remark42-jwtheader:8083 + - SECRET=12345 + - AUTH_ANON=true + - AUTH_SEND_JWT_HEADER=true + - UPDATE_LIMIT=100 + volumes: + - remark42-e2e-jwtheader-var:/srv/var + healthcheck: + test: ["CMD", "curl", "--fail", "http://localhost:8080/ping"] + interval: 2s + timeout: 3s + retries: 30 + + # no auth provider at all, which #1456 reported rendering as a widget with nothing to say. it + # cannot be a setting on another instance: the absence is the configuration + remark42-noauth: + image: ghcr.io/umputun/remark42:dev + container_name: "remark42-e2e-noauth" + pull_policy: never + depends_on: + remark42: + condition: service_started + + ports: + - "127.0.0.1:8085:8080" + + environment: + - REMARK_URL=http://remark42-noauth:8085 + - SECRET=12345 + # its own address and nothing else, so this instance also serves as the negative case for + # embedding: the browser refuses to frame it anywhere but here, and the widget document + # never runs to report itself inited + - ALLOWED_HOSTS=http://remark42-noauth:8085 + - UPDATE_LIMIT=100 + volumes: + - remark42-e2e-noauth-var:/srv/var + healthcheck: + test: ["CMD", "curl", "--fail", "http://localhost:8080/ping"] + interval: 2s + timeout: 3s + retries: 30 + + # anonymous voting, which the default configuration refuses: rest_private.go turns down a vote + # from an anonymous user unless ANON_VOTE is set, and it only works alongside VOTES_IP, which + # is what scopes it + remark42-anonvote: + image: ghcr.io/umputun/remark42:dev + container_name: "remark42-e2e-anonvote" + pull_policy: never + depends_on: + remark42: + condition: service_started + + ports: + - "127.0.0.1:8086:8080" + + environment: + - REMARK_URL=http://remark42-anonvote:8086 + - SECRET=12345 + - AUTH_ANON=true + - ANON_VOTE=true + - VOTES_IP=true + - UPDATE_LIMIT=100 + volumes: + - remark42-e2e-anonvote-var:/srv/var + healthcheck: + test: ["CMD", "curl", "--fail", "http://localhost:8080/ping"] + interval: 2s + timeout: 3s + retries: 30 + + # a page on an origin the widget is not served from, which is the separate-domain setup the + # manuals describe and the one readers actually hit problems with. every other host page in the + # suite is served by remark42 itself, so nothing else exercises a cross-origin embed + host-site: + image: nginx:1.29-alpine + container_name: "remark42-e2e-hostsite" + ports: + - "127.0.0.1:8090:80" + volumes: + - ./e2e/hostsite:/usr/share/nginx/html:ro + healthcheck: + test: ["CMD", "wget", "-q", "-O", "/dev/null", "http://localhost/post.html"] + interval: 2s + timeout: 3s + retries: 30 + # catches the email-auth verification message; the suite reads it back over the HTTP API mailpit: image: axllent/mailpit:v1.30.7 @@ -102,8 +259,12 @@ services: timeout: 3s retries: 30 -# named rather than bind mounts, so `docker compose down -v` really does discard the +# named volumes, not bind mounts, so `docker compose down -v` really does discard the # databases and a run can start from an empty site volumes: remark42-e2e-var: remark42-e2e-shortedit-var: + remark42-e2e-adminedit-var: + remark42-e2e-jwtheader-var: + remark42-e2e-noauth-var: + remark42-e2e-anonvote-var: diff --git a/e2e/README.md b/e2e/README.md index c373ddaa..d1bddc03 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -49,21 +49,57 @@ CI does not retry a failing test. The suite is young enough that a failure is ev Beyond that: `docker compose -f compose-e2e-test.yml logs` for the server side, and mailpit's web UI on for anything email. +A failing test also logs whatever the browser wrote to its console, which is where a failed request or a widget-side error shows up. Those are context only. What does fail a test on its own is an uncaught exception in the page and a rate-limit response, both of which otherwise corrupt a run silently: a widget that throws while rendering still satisfies most assertions here, and a refused `/auth/status` renders as a signed-out reader. + +## The stack the suite runs against + +The suite refuses a running stack that was not brought up from the sources under test. Every checkout builds the image tag the compose file names, so a stack from another worktree, or from this one before an edit, answers on these ports and passes every readiness probe while serving code nobody is looking at. + +`e2e/stamp.sh` digests the content of `backend`, `frontend`, `Dockerfile` and `docker-init.sh`; compose puts it in the container's environment and the suite reads it back. It digests content and not `HEAD`, so a commit touching only the suite does not invalidate a stack. `make e2e-up` stamps the same way, so a stack started by hand is accepted. On a mismatch the failure says to run `make e2e-down`. + Before pushing, `cd e2e && go vet -tags=e2e ./...` and `golangci-lint run --build-tags=e2e --config ../backend/.golangci.yml`. CI runs both, and neither is covered by a plain `go vet ./...` because of the build tag. ## The stack -`compose-e2e-test.yml` at the repository root runs three services, each bound to the loopback interface since it holds a known secret and an admin shared id: +`compose-e2e-test.yml` at the repository root runs six services, each bound to the loopback interface since it holds a known secret and an admin shared id: - **remark42** on `:8080`, with the dev oauth2 provider on `:8084`, anonymous and email sign-in - **remark42-shortedit** on `:8081`, with `EDIT_TIME=15s` and anonymous sign-in only, since the dev oauth2 provider's port is fixed at 8084 and cannot be published twice. It exists so the expired-edit path is observable without holding a test open for the default five minutes -- **mailpit** on `:8025`, which catches the email-auth verification message for the suite to read back +- **remark42-adminedit** on `:8082`, with `ADMIN_EDIT=true` and the same short window, for the unlimited window an admin is supposed to get +- **remark42-jwtheader** on `:8083`, with `AUTH_SEND_JWT_HEADER=true`, where the token arrives in a header instead of a cookie and the frontend has to keep it itself +- **remark42-noauth** on `:8085`, with no auth provider at all, which the widget has to say something about, and with `ALLOWED_HOSTS` set to its own address so it doubles as the instance that refuses to be framed elsewhere +- **remark42-anonvote** on `:8086`, with `ANON_VOTE` and the `VOTES_IP` it depends on, since the default configuration turns an anonymous vote down +- **host-site** on `:8090`, an nginx serving `e2e/hostsite/`, which is a page on an origin the widget is not served from. Every other host page here is served by remark42 itself, so without it the separate-domain setup the manuals describe is never exercised. `post.html` embeds the main instance; `restricted.html` embeds the one whose `ALLOWED_HOSTS` names only itself, which is the refusal case +- **mailpit** on `:8025`, which catches the email-auth verification message and the subscription token for the suite to read back -Three settings exist for the tests rather than for realism, and each is there for a reason: +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 four 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. + +Three 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` rather than by a setting. See `pauseForAuthLimit`. +- 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`. + +## What this suite cannot reach + +Every service here speaks http, and nothing in it holds a certificate. Any behaviour the browser gates on the page protocol is therefore invisible: a cookie the widget writes with `Secure`, anything keyed on `window.location.protocol`, and the whole third-party cookie form of `SameSite=None; Secure; Partitioned`, which is the only one browsers still accept from an embedded frame. + +That is not hypothetical. `setAuthCookie` prefixed its cookies with `__Host-` on any https page, so a real deployment stored `__Host-JWT` while the backend looked for `JWT`; it survived because the prefix comes from the page protocol and every test and the dev server run on http. Fixed in #2197, under a second suite pinned to an https page, because this one cannot show it. + +There is a second trap waiting for whoever gives the stack TLS and then tries to prove the third-party case. 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. Measured on a cross-site https embed: + +| | ordinary third-party cookie | `Partitioned` cookie | +|---|---|---| +| Playwright defaults | kept | kept | +| partitioning left enabled | dropped | stored, with its partition key | + +So a blocking run has to assert a control before anything it reports can be believed: set an ordinary `SameSite=None` cookie from inside the widget frame and require the browser to drop it. If it survives, the run is not blocking anything. + +None of that reaches the widget's own storage fallback, which the auth panel offers as `comments.html` when `IS_THIRD_PARTY && !IS_STORAGE_AVAILABLE`. `IS_STORAGE_AVAILABLE` stays true even with partitioning properly enforced, because Chromium partitions `localStorage` instead of denying it, so the probe behind that constant never throws and the condition cannot fire. That case needs WebKit, not a Chromium flag. + +The practical consequence is for the cross-origin case in `crossorigin_test.go`, which asserts rendering and deliberately not signing in. Give the stack TLS and signing in there becomes testable, and the assertion that matters is **the reload**: the widget holds its token in memory for the life of a page, so a case that signs in and posts without reloading passes while persistence is entirely broken. ## Isolation @@ -73,9 +109,9 @@ Thread URLs deliberately keep the underscores a test name carries, since collaps ## Browsers -The `iframe_test.go` group runs in Chromium, Firefox and WebKit. It is about rendering rather than logic: the widget holds the frame hidden until its document reports itself inited, and the opaque canvas that guards against is a WebKit behaviour, so Chromium alone would not exercise it. +The `iframe_test.go` group runs in Chromium, Firefox and WebKit. It is about rendering and not logic: the widget holds the frame hidden until its document reports itself inited, and the opaque canvas that guards against is a WebKit behaviour, so Chromium alone would not exercise it. -Those tests address the server as `127.0.0.1` rather than by name, since `--host-resolver-rules` is a Chromium flag and they need no dev oauth2, which is the only reason the hostname exists. +Those tests address the server as `127.0.0.1` and not by name, since `--host-resolver-rules` is a Chromium flag and they need no dev oauth2, which is the only reason the hostname exists. Everything else runs in Chromium alone, for the same reason inverted: those tests sign in, sign-in needs the dev oauth2 provider, and reaching it by name from the host is Chromium-only. Running them in the other engines would mean putting the suite back inside the compose network. @@ -84,5 +120,6 @@ Everything else runs in Chromium alone, for the same reason inverted: those test The production bundle strips `data-testid`, so tests use what ships: the stable class hooks the widget keeps outside CSS modules (`.auth-button`, `.auth-submit`, `.comment-actions`, `.sort-picker`, `.preloader`), `title` attributes on icon-only controls, and visible text. Three shapes are worth knowing: - `.auth` only exists while signed out, so waiting on it hangs after sign-in. `widget()` waits on the comment form, which is present either way. +- The production build hashes every css-module class name to a short opaque id, so a component's own class is not something a test can hold. `role` is: the footer is `[role="contentinfo"]` and the edit countdown is `[role="timer"]`. - Comments render through an IntersectionObserver, so one below the fold is an empty `article` with no text in it. That makes any absence assertion written as a text filter pass whether the comment is gone or merely off screen; count articles instead, which is what `articleCount` is for. - Collapsing a thread hides the comment text, so a locator filtered by that text stops matching the element under test. `TestThread_CollapsePersistsAcrossReload` anchors on the comment's id instead. diff --git a/e2e/auth_test.go b/e2e/auth_test.go index c739273d..24a2326a 100644 --- a/e2e/auth_test.go +++ b/e2e/auth_test.go @@ -4,6 +4,7 @@ package e2e import ( "fmt" + "net/http" "regexp" "testing" @@ -30,11 +31,12 @@ func signInDev(t *testing.T, page playwright.Page, frame playwright.FrameLocator // frame to make it re-read auth state pauseForAuthLimit() require.NoError(t, page.Locator("#remark42 iframe").Press("Tab")) - assertSignedIn(t, frame) + assertSignedIn(t, page, frame) } -// signInAnon signs in through the anonymous provider, an in-frame form with no popup -func signInAnon(t *testing.T, frame playwright.FrameLocator, username string) { +// signInAnon signs in through the anonymous provider, an in-frame form with no popup. it takes +// the page because assertSignedIn may have to nudge it, see there +func signInAnon(t *testing.T, page playwright.Page, frame playwright.FrameLocator, username string) { t.Helper() pauseForAuthLimit() @@ -48,9 +50,17 @@ func signInAnon(t *testing.T, frame playwright.FrameLocator, username string) { require.NoError(t, tab.Click()) } require.NoError(t, frame.Locator(".auth-input-username").Fill(username)) - require.NoError(t, frame.Locator(".auth-submit").Click()) - assertSignedIn(t, frame) + // wait for the request the submit is supposed to make, so a form the browser refuses to + // submit fails here naming that, and not fifteen seconds later on a panel that was never + // going to change. the input is validated against a pattern, see anonName + _, err := page.ExpectResponse("**/auth/anonymous/login**", func() error { + return frame.Locator(".auth-submit").Click() + }, playwright.PageExpectResponseOptions{Timeout: playwright.Float(float64(waitTimeout.Milliseconds()))}) + require.NoError(t, err, "the anonymous form was never submitted, most likely because %q is not "+ + "a username it accepts", username) + + assertSignedIn(t, page, frame) } func TestAuth_DevProviderSignsIn(t *testing.T) { @@ -58,15 +68,15 @@ func TestAuth_DevProviderSignsIn(t *testing.T) { frame := openThread(t, page) signInDev(t, page, frame) - assertSignedIn(t, frame) + assertSignedIn(t, page, frame) } func TestAuth_AnonymousSignsIn(t *testing.T) { page := newPage(t) frame := openThread(t, page) - signInAnon(t, frame, "anontester") - assertSignedIn(t, frame) + signInAnon(t, page, frame, "anontester") + assertSignedIn(t, page, frame) name, err := frame.Locator(`[title="Open My Profile"]`).InnerText() require.NoError(t, err) @@ -75,21 +85,32 @@ func TestAuth_AnonymousSignsIn(t *testing.T) { // TestAuth_EmailSignsIn drives the full email flow: request a code, read it back out of the // mail catcher, and submit it. The token is what the widget sends, so a broken template or a -// broken token round-trip fails here rather than silently in production. +// broken token round-trip fails here and not silently in production. func TestAuth_EmailSignsIn(t *testing.T) { page := newPage(t) frame := openThread(t, page) // the mailbox is per-run: mailpit keeps everything, and with the stack left up between // runs a fixed address would let this test read a previous run's token - address := fmt.Sprintf("email-tester-%s@example.com", runID) + signInEmail(t, page, frame, "emailtester", fmt.Sprintf("email-tester-%s@example.com", runID)) +} + +// signInEmail completes the email flow: ask for a code, read it out of the mail catcher, submit +// it. The address decides the user id, sha1 of it, so a case needing a known id picks the address +func signInEmail(t *testing.T, page playwright.Page, frame playwright.FrameLocator, username, address string) { + t.Helper() pauseForAuthLimit() require.NoError(t, frame.Locator(".auth-button").Click()) waitVisible(t, frame.Locator(".auth-dropdown")) - require.NoError(t, frame.Locator(`label[for="form-provider-email"]`).Click()) - require.NoError(t, frame.Locator(".auth-input-username").Fill("emailtester")) + // only rendered when more than one form provider is enabled; with email alone the form is + // shown directly + tab := frame.Locator(`label[for="form-provider-email"]`) + if n, err := tab.Count(); err == nil && n > 0 { + require.NoError(t, tab.Click()) + } + require.NoError(t, frame.Locator(".auth-input-username").Fill(username)) require.NoError(t, frame.Locator(".auth-input-email").Fill(address)) require.NoError(t, frame.Locator(".auth-submit").Click()) @@ -99,16 +120,35 @@ func TestAuth_EmailSignsIn(t *testing.T) { require.NoError(t, frame.Locator(".auth-token-textarea").Fill(token)) require.NoError(t, frame.Locator(".auth-submit").Click()) - assertSignedIn(t, frame) + assertSignedIn(t, page, frame) } // assertSignedIn checks the panel has swapped Sign In for the signed-in user's own controls. -// Sign Out is an icon button, so its title is the only text it carries -func assertSignedIn(t *testing.T, frame playwright.FrameLocator) { +// 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 +func assertSignedIn(t *testing.T, page playwright.Page, frame playwright.FrameLocator) { t.Helper() - waitVisible(t, frame.Locator(`[title="Sign Out"]`)) + + signOut := frame.Locator(`[title="Sign Out"]`) + if err := signOut.WaitFor(playwright.LocatorWaitForOptions{ + State: playwright.WaitForSelectorStateVisible, + Timeout: playwright.Float(float64(authRepaintWait.Milliseconds())), + }); err != nil { + pauseForAuthLimit() + require.NoError(t, page.Locator("#remark42 iframe").Press("Tab")) + } + + waitVisible(t, signOut) waitVisible(t, frame.Locator(`[title="Open My Profile"]`)) - waitHidden(t, frame.Locator(".auth-button")) + waitHidden(t, frame.Locator(".auth-button"), "the panel still offers sign-in after a sign-in") } // verificationToken pulls the JWT out of the confirmation mail @@ -118,3 +158,101 @@ func verificationToken(t *testing.T, body string) string { require.NotEmpty(t, m, "no token in message body:\n%s", body) return m } + +// TestAuth_SignOutEndsTheSession covers the one half of authentication nothing tested at any +// level: that signing out actually ends the session instead of only repainting the panel. The +// 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) { + page := newPage(t) + frame := openThread(t, page) + + signInDev(t, page, frame) + + pauseForAuthLimit() + require.NoError(t, frame.Locator(`[title="Sign Out"]`).Click()) + waitVisible(t, frame.Locator(".auth-button")) + waitHidden(t, frame.Locator(`[title="Sign Out"]`)) + + frame = reload(t, page) + waitVisible(t, frame.Locator(".auth-button")) + waitHidden(t, frame.Locator(`[title="Sign Out"]`), + "the panel signed out but the session survived the reload, so the cookie was never cleared") +} + +// TestAuth_HostPageMessageDoesNotCloseTheDropdown covers #2139. Every message reaching the widget +// closed the sign-in dropdown, because the handler returned early only for a clickOutside payload +// and fell through to closing in every other case. embed.ts watches the host page's title element +// and posts on every mutation, so a page that updates its own title discarded whatever the reader +// had typed into the login form. Browser extensions posting into the page did the same, which is +// what #1761 reports. +// +// The theme change at the end is the synchronization, not a second assertion: postMessage is +// 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) { + page := newPage(t) + stubSignedOut(t, page) + embedConfig(t, page, map[string]any{"theme": "light"}) + frame := widget(t, page) + + require.NoError(t, frame.Locator(".auth-button").Click()) + waitVisible(t, frame.Locator(".auth-dropdown")) + + require.NoError(t, frame.Locator(`label[for="form-provider-email"]`).Click()) + const typed = "half-written name" + require.NoError(t, frame.Locator(".auth-input-username").Fill(typed)) + + // an ordinary thing for a host page to do, and all it took + _, err := page.Evaluate(`() => { document.title = 'the host page renamed itself'; }`) + require.NoError(t, err) + + _, err = page.Evaluate(`() => window.REMARK42.changeTheme('dark')`) + require.NoError(t, err) + eventually(t, waitTimeout, "the widget never acted on the message sent after the title change", func() bool { + return widgetColorScheme(t, page) == "dark" + }) + + waitVisible(t, frame.Locator(".auth-dropdown")) + value, err := frame.Locator(".auth-input-username").InputValue() + require.NoError(t, err) + assert.Equal(t, typed, value, "the host page's own message emptied the login form") + + // and the message that is supposed to close it still does, or the fix above would be a + // dropdown that never closes + require.NoError(t, page.Mouse().Click(5, 5)) + waitHidden(t, frame.Locator(".auth-dropdown"), "a genuine click outside the widget no longer closes the dropdown") +} + +// TestAuth_SessionSurvivesATransientStatusFailure covers the probe #1763 introduced. The widget +// asks /auth/status on every load, and a single failed answer must not be taken for a signed-out +// reader in any lasting way: the session lives in a cookie the server issued, and one bad +// 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) { + 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{ + Status: playwright.Int(http.StatusInternalServerError), + ContentType: playwright.String("application/json"), + Body: playwright.String(`{"error":"failed"}`), + }) + })) + + pauseForAuthLimit() + _, err := page.Reload() + require.NoError(t, err) + widget(t, page) + + require.NoError(t, page.Unroute("**/auth/status**")) + + // the session has to be there again once the endpoint is, which is the whole claim: a + // transient failure cost the reader nothing + frame = reload(t, page) + assertSignedIn(t, page, frame) +} diff --git a/e2e/comment_test.go b/e2e/comment_test.go index 725464cf..1cfe1edc 100644 --- a/e2e/comment_test.go +++ b/e2e/comment_test.go @@ -3,8 +3,12 @@ package e2e import ( + "encoding/base64" "fmt" + "net/http" neturl "net/url" + "os" + "path/filepath" "strings" "testing" "time" @@ -72,7 +76,7 @@ func TestComment_EditWithinTheDeadline(t *testing.T) { submitForm(t, replyForm(t, frame), edited) waitVisible(t, comment(frame, edited)) - // an edit replaces the comment rather than adding one, and counting is the only sound + // an edit replaces the comment instead of adding one, and counting is the only sound // way to say the old text is gone: a text filter cannot tell absent from off screen assert.Equal(t, 1, articleCount(t, frame), "editing should not add a comment") txt, err := frame.Locator("article").First().InnerText() @@ -98,7 +102,7 @@ func TestComment_EditExpiresAfterTheDeadline(t *testing.T) { page := newPage(t) url := threadURLOn(t, shortEditURL) frame := openURL(t, page, url) - signInAnon(t, frame, "expirytester") + signInAnon(t, page, frame, "expirytester") text := "expires " + runID postComment(t, frame, text) @@ -136,7 +140,7 @@ func TestComment_DeleteRemovesTheText(t *testing.T) { // deliberately not the dev user: ADMIN_SHARED_ID makes that one an admin, and the widget // sends admins to the admin endpoint, so signing in there would leave the path every // ordinary reader takes untested - signInAnon(t, frame, "deletetester") + signInAnon(t, page, frame, "deletetester") text := "doomed " + runID survivor := "survivor " + runID @@ -154,10 +158,282 @@ func TestComment_DeleteRemovesTheText(t *testing.T) { // scrolled out of view would also satisfy waitVisible(t, comment(frame, "This comment was deleted")) - // and it stays gone rather than reappearing from cache on the next load. the survivor is + // and it stays gone instead of reappearing from cache on the next load. the survivor is // what makes this assertion mean anything: without it a thread that had not rendered yet // would satisfy "the deleted text is absent" just as well frame = reload(t, page) waitVisible(t, comment(frame, survivor)) assert.Equal(t, 1, articleCount(t, frame), "the deleted comment should be gone from the thread") } + +// TestComment_EditKeepsTheOriginalSource covers what the widget puts back in the textarea when a +// comment is edited. The thread shows rendered html, so the form has to hold the source it was +// 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) { + page := newPage(t) + frame := openThread(t, page) + signInDev(t, page, frame) + + // entities, markup and a character outside latin1, each of which a render-and-read-back + // round trip mangles differently + source := "5 < 10 & **bold** tag ю " + runID + postCommentMatching(t, frame, source, runID) + + require.NoError(t, actions(frame, runID).Locator(`button:has-text("Edit")`).Click()) + form := replyForm(t, frame) + + got, err := form.Locator("textarea").InputValue() + require.NoError(t, err) + assert.Equal(t, source, got, "the edit form has to hold the source that was posted, not the rendered comment") + + submitForm(t, form, source+" edited") + waitVisible(t, comment(frame, "edited")) + + frame = reload(t, page) + require.NoError(t, actions(frame, runID).Locator(`button:has-text("Edit")`).Click()) + got, err = replyForm(t, frame).Locator("textarea").InputValue() + require.NoError(t, err) + assert.Equal(t, source+" edited", got, "the stored source has to survive the round trip through the backend") +} + +// 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) { + page := newPage(t) + frame := openThread(t, page) + signInDev(t, page, frame) + + draft := "half written " + runID + require.NoError(t, frame.Locator(commentFormSel).First().Locator("textarea").Fill(draft)) + + frame = reload(t, page) + textarea := frame.Locator(commentFormSel).First().Locator("textarea") + eventually(t, waitTimeout, "the draft was not restored after the reload", func() bool { + v, err := textarea.InputValue() + return err == nil && v == draft + }) + + postCommentMatching(t, frame, draft, draft) + + frame = reload(t, page) + got, err := frame.Locator(commentFormSel).First().Locator("textarea").InputValue() + require.NoError(t, err) + assert.Empty(t, got, "a posted draft has to be cleared, or the reader is handed their own comment back") +} + +// TestComment_PostFailureKeepsTheText covers the path a reader hits when the server refuses the +// 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) { + 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{ + Status: playwright.Int(http.StatusBadRequest), + ContentType: playwright.String("application/json"), + Body: playwright.String(`{"code":19,"details":"comment contains restricted words","error":"rejected"}`), + })) + })) + + text := "rejected " + runID + form := frame.Locator(commentFormSel).First() + submitForm(t, form, text) + + waitVisible(t, form.Locator(`p[role="alert"]`)) + + got, err := form.Locator("textarea").InputValue() + require.NoError(t, err) + assert.Equal(t, text, got, "a refused comment has to stay in the form, it is the only copy the reader has") + + require.NoError(t, page.Unroute("**/api/v1/comment?**")) + require.NoError(t, form.Locator(`button[type="submit"]`).Click()) + waitVisible(t, comment(frame, text)) +} + +// TestComment_AdminPinsAndVerifies covers two moderator actions that change what every reader +// 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) { + text := "moderated " + runID + + // verification is a property of the user and outlives the run in the stack's database, so a + // fixed name is only verifiable once: the next run would toggle an already verified author + // off and wait for a badge that is being taken away + author := newPage(t) + authorFrame := openThread(t, author) + signInAnon(t, author, authorFrame, anonName("moderated")) + postComment(t, authorFrame, text) + + admin := newPage(t) + adminFrame := openURL(t, admin, threadURL(t)) + signInDev(t, admin, adminFrame) + + admin.OnDialog(func(d playwright.Dialog) { _ = d.Accept() }) + require.NoError(t, actions(adminFrame, text).Locator(`button:has-text("Pin")`).Click()) + // pinning re-renders the thread, and a click that lands during that render is lost, so wait + // for the pinned region to exist before touching the same comment again + waitVisible(t, adminFrame.Locator(`[role="region"][aria-label="Pinned comments"]`)) + + // the verification toggle sits in the comment header beside the author, not in the action bar + require.NoError(t, comment(adminFrame, text).Locator(`[title="Toggle verification"]`).First().Click()) + waitVisible(t, comment(adminFrame, text).Locator(`[title="Verified user"]`).First()) + + reader := newPage(t) + readerFrame := openURL(t, reader, threadURL(t)) + + pinned := readerFrame.Locator(`[role="region"][aria-label="Pinned comments"]`) + waitVisible(t, pinned) + waitVisible(t, pinned.Locator("article", playwright.LocatorLocatorOptions{HasText: text})) + waitVisible(t, comment(readerFrame, text).Locator(`[title="Verified user"]`).First()) + + // unpinning has to reach every reader too, so the region goes away instead of merely + // emptying on the moderator's own page + require.NoError(t, actions(adminFrame, text).Locator(`button:has-text("Unpin")`).Click()) + + readerFrame = reload(t, reader) + waitHidden(t, readerFrame.Locator(`[role="region"][aria-label="Pinned comments"]`), + "the comment was unpinned but readers still see the pinned region") +} + +// TestComment_ImageUploadRendersAndRecovers covers the upload path end to end, which nothing +// exercised in a browser: the file input, the temporary markdown the form writes while the request +// is in flight, the final picture URL, and the image actually loading in the posted comment. +// 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) { + page := newPage(t) + frame := openThread(t, page) + signInDev(t, page, frame) + + // a 1x1 png, written out inline so the case does not depend on a fixture file + png, err := base64.StdEncoding.DecodeString( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==") + require.NoError(t, err) + + path := filepath.Join(t.TempDir(), "pixel.png") + require.NoError(t, os.WriteFile(path, png, 0o600)) + + form := frame.Locator(commentFormSel).First() + textarea := form.Locator("textarea") + + t.Run("a failed upload leaves the text as it was", func(t *testing.T) { + require.NoError(t, page.Route("**/api/v1/picture**", func(route playwright.Route) { + // held briefly so the in-flight state is observable: without it the placeholder + // 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{ + Status: playwright.Int(http.StatusInternalServerError), + ContentType: playwright.String("application/json"), + Body: playwright.String(`{"code":0,"details":"upload failed","error":"nope"}`), + })) + })) + defer func() { require.NoError(t, page.Unroute("**/api/v1/picture**")) }() + + written := "before the upload " + runID + require.NoError(t, textarea.Fill(written)) + require.NoError(t, form.Locator(`input[type="file"]`).SetInputFiles(path)) + + eventually(t, waitTimeout, "the form never showed the upload in progress", func() bool { + v, verr := textarea.InputValue() + return verr == nil && v != written + }) + + waitVisible(t, form.Locator(`p[role="alert"]`)) + eventually(t, waitTimeout, "the upload placeholder was left in the text after the failure", func() bool { + v, verr := textarea.InputValue() + return verr == nil && v == written + }) + }) + + t.Run("an uploaded image is posted and renders", func(t *testing.T) { + require.NoError(t, textarea.Fill("with an image "+runID+" ")) + require.NoError(t, form.Locator(`input[type="file"]`).SetInputFiles(path)) + + eventually(t, waitTimeout, "the upload never produced a picture url", func() bool { + v, verr := textarea.InputValue() + return verr == nil && strings.Contains(v, "/api/v1/picture/") + }) + + require.NoError(t, form.Locator(`button[type="submit"]`).Click()) + posted := comment(frame, "with an image "+runID) + waitVisible(t, posted) + + img := posted.Locator(`img[src*="/api/v1/picture/"]`).First() + waitVisible(t, img) + + // visible is not loaded: a broken src renders as an empty box, and naturalWidth is the + // only thing that says the bytes came back + eventually(t, waitTimeout, "the posted image never loaded", func() bool { + w, jerr := img.Evaluate("el => el.naturalWidth", nil) + n, ok := w.(int) + return jerr == nil && ok && n > 0 + }) + }) +} + +// TestComment_BlockedAuthorCannotPost covers the refusal a blocked author meets. The backend +// 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) { + text := "before the block " + runID + + // the block is permanent and the stack's database outlives the run, so a fixed name would + // only be postable once: every later run would find the author already blocked + author := newPage(t) + authorFrame := openThread(t, author) + signInAnon(t, author, authorFrame, anonName("blocked")) + postComment(t, authorFrame, text) + + admin := newPage(t) + adminFrame := openURL(t, admin, threadURL(t)) + signInDev(t, admin, adminFrame) + + admin.OnDialog(func(d playwright.Dialog) { _ = d.Accept() }) + _, err := actions(adminFrame, text).Locator("select").SelectOption(playwright.SelectOptionValues{ + Values: &[]string{"permanently"}, + }) + require.NoError(t, err) + + // the author's own page still believes it can post, which is the point: the refusal has to + // come back from the server and be shown + form := authorFrame.Locator(commentFormSel).First() + submitForm(t, form, "after the block "+runID) + + // not scoped to the form: the widget re-renders the whole panel once the server reports the + // author as blocked, so where the message lands is not the point, only that it is said + waitVisible(t, authorFrame.Locator("text=blocked").First()) +} + +// TestComment_ReadOnlyThreadTakesTheFormAway covers the admin switch that closes a thread. A +// 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) { + page := newPage(t) + url := threadURL(t) + frame := openURL(t, page, url) + signInDev(t, page, frame) + + // the admin panel swaps its own button instead of showing the read-only notice, which is + // what an ordinary reader gets + require.NoError(t, frame.Locator(`button:has-text("Disable comments")`).Click()) + waitVisible(t, frame.Locator(`button:has-text("Enable comments")`)) + + // 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) + + readerFrame := reader.FrameLocator("#remark42 iframe") + waitVisible(t, readerFrame.Locator(`text=Read-only`)) + waitHidden(t, readerFrame.Locator(commentFormSel).First(), + "the thread is read-only but a reader is still shown a comment form") + + require.NoError(t, frame.Locator(`button:has-text("Enable comments")`).Click()) + waitVisible(t, frame.Locator(commentFormSel).First()) +} diff --git a/e2e/config_test.go b/e2e/config_test.go new file mode 100644 index 00000000..7dbb4eeb --- /dev/null +++ b/e2e/config_test.go @@ -0,0 +1,220 @@ +//go:build e2e + +package e2e + +import ( + "encoding/json" + "fmt" + "net/http" + neturl "net/url" + "testing" + + "github.com/mxschmitt/playwright-go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// remark_config is the widget's public surface: whatever an integrator sets there has to keep +// working, and discussion #1714 asks for it to be written down. None of it was covered, so a +// setting could stop reaching the widget without anything here going red. + +// TestConfig_ColorsReachTheWidget covers __colors__, the only setting that does not travel in the +// iframe's query string: the parent puts it in window.name and an inline script in the widget +// document, in templates/iframe.ejs, reads it back before the bundle runs. Nothing else opens +// window.name, so the whole path could be removed unnoticed +func TestConfig_ColorsReachTheWidget(t *testing.T) { + page := newPage(t) + stubSignedOut(t, page) + embedConfig(t, page, map[string]any{"__colors__": map[string]any{"--color15": "rgb(1, 2, 3)"}}) + widget(t, page) + + value, err := page.FrameLocator("#remark42 iframe").Locator(":root").Evaluate( + `(el) => getComputedStyle(el).getPropertyValue('--color15').trim()`, nil, + playwright.LocatorEvaluateOptions{Timeout: playwright.Float(float64(waitTimeout.Milliseconds()))}) + require.NoError(t, err) + assert.Equal(t, "rgb(1, 2, 3)", value, "the colors an integrator sets never reached the widget document") +} + +// TestConfig_URLOverrideDecidesTheThread covers remark_config.url, which is how a canonical +// address keeps one conversation across pages that differ: a print view, a path with tracking +// parameters, a page that moved. Two host pages at different addresses name the same thread here, +// and the second has to see what the first posted. +// +// The override carries a query string on purpose, since createInstance strips the fragment and +// nothing else, so the query is part of the thread's identity and dropping it would split the +// conversation in two. One parameter and not two: a url containing "&" cannot be commented on +// at all, see #2204 +func TestConfig_URLOverrideDecidesTheThread(t *testing.T) { + shared := fmt.Sprintf("%s/web/?e2e=config-url-%s", baseURL, runID) + text := "shared thread " + runID + + page := newPage(t) + embedConfigOn(t, page, "/web/privacy.html", map[string]any{"url": shared}) + frame := widget(t, page) + signInAnon(t, page, frame, anonName("urloverride")) + postComment(t, frame, text) + + // a different host page, same override + other := newPage(t) + stubSignedOut(t, other) + embedConfigOn(t, other, "/web/markdown-help.html", map[string]any{"url": shared}) + otherFrame := widget(t, other) + // markdown-help.html is long enough to leave the widget below the fold, and comments render + // through an IntersectionObserver, so one that has not been reached is an empty article no + // text filter matches + require.NoError(t, other.Locator("#remark42 iframe").ScrollIntoViewIfNeeded()) + waitVisible(t, comment(otherFrame, text)) + + // and the query string reached the widget, which is what makes this a test of the override + // and not of two pages agreeing on a default + src, err := other.Locator("#remark42 iframe").GetAttribute("src") + require.NoError(t, err) + assert.Contains(t, src, "e2e%3Dconfig-url", "the query string was dropped from the thread url") +} + +// TestConfig_SubscriptionControlsCanBeHidden covers show_rss_subscription and +// show_email_subscription, which an integrator turns off to keep the form to itself, read in +// common/settings.ts. The both-shown case is the control: without it these would hold on a +// widget that offers neither +func TestConfig_SubscriptionControlsCanBeHidden(t *testing.T) { + for _, tc := range []struct { + name string + config map[string]any + rss, byMail bool + }{ + {"both shown", map[string]any{}, true, true}, + {"rss hidden", map[string]any{"show_rss_subscription": false}, false, true}, + {"email hidden", map[string]any{"show_email_subscription": false}, true, false}, + } { + t.Run(tc.name, func(t *testing.T) { + page := newPage(t) + embedConfig(t, page, tc.config) + frame := widget(t, page) + // the email control is offered to a registered reader only, which anonymous is not + signInDev(t, page, frame) + + rss := frame.Locator(`[title="Subscribe by RSS"]`) + byMail := frame.Locator(`[title="Subscribe by Email"]`) + + if tc.rss { + waitVisible(t, rss) + } else { + waitHidden(t, rss, "show_rss_subscription=false left the RSS control in place") + } + if tc.byMail { + waitVisible(t, byMail) + } else { + waitHidden(t, byMail, "show_email_subscription=false left the email control in place") + } + }) + } +} + +// TestConfig_UnknownLocaleFallsBackToEnglish covers loadLocale's only observable guarantee. It +// returns the English defaults both for a name it does not recognize and for a chunk it fails to +// fetch, and the two are indistinguishable from outside, so a widget rendering English for +// locale=xx is all a caller can be promised. Without this, a build that stopped resolving +// catalogs altogether would still look correct to anyone reading English +func TestConfig_UnknownLocaleFallsBackToEnglish(t *testing.T) { + page := newPage(t) + stubSignedOut(t, page) + embedConfig(t, page, map[string]any{"locale": "xx"}) + + frame := widget(t, page) + placeholder, err := frame.Locator(commentFormSel).First().Locator("textarea").GetAttribute("placeholder") + require.NoError(t, err) + assert.Equal(t, "Your comment here", placeholder, + "an unrecognized locale did not fall back to the english defaults") +} + +// TestConfig_TimesRenderInTheReadersTimezone covers the one part of a comment's rendering that +// cannot move to the server. The locale is configured and the server knows it, but the timezone +// belongs to the reader's machine, so a design that renders timestamps anywhere else has to keep +// this working. Kiritimati is UTC+14, far enough that a wrong timezone usually lands on the wrong +// day as well as the wrong hour +func TestConfig_TimesRenderInTheReadersTimezone(t *testing.T) { + const zone = "Pacific/Kiritimati" + + page := newPageInContext(t, browser, playwright.BrowserNewContextOptions{ + TimezoneId: playwright.String(zone), + }) + frame := openThread(t, page) + signInAnon(t, page, frame, anonName("timezone")) + + text := "timezone " + runID + posted := postCommentMatching(t, frame, text, text) + + shown, err := posted.Locator(`a[href*="#remark42__comment-"]`).First().InnerText() + require.NoError(t, err) + + // the server's own timestamp for that comment, so the comparison is against what was stored + // and not against the clock this process reads + status, body := pageFetch(t, page, "GET", + fmt.Sprintf("%s/api/v1/find?site=remark&url=%s&format=tree", baseURL, neturl.QueryEscape(threadURL(t))), nil) + require.Equal(t, http.StatusOK, status, "could not read the thread back: %s", body) + + var thread struct { + Comments []struct { + Comment struct { + Time string `json:"time"` + } `json:"comment"` + } `json:"comments"` + } + require.NoError(t, json.Unmarshal([]byte(body), &thread)) + require.NotEmpty(t, thread.Comments, "the thread came back empty") + + // formatted in the page, by the same Intl the widget uses, so this asserts the timezone + // reached the rendering and not that two libraries agree on a format + want, err := page.Evaluate(`([iso, zone]) => { + const d = new Date(iso); + const day = new Intl.DateTimeFormat('en', {timeZone: zone}).format(d); + const time = new Intl.DateTimeFormat('en', {timeZone: zone, hour: 'numeric', minute: 'numeric'}).format(d); + return {day, time}; + }`, []any{thread.Comments[0].Comment.Time, zone}) + require.NoError(t, err) + + parts, ok := want.(map[string]any) + require.True(t, ok, "unexpected shape from the page: %#v", want) + day, _ := parts["day"].(string) + require.NotEmpty(t, day) + + assert.Contains(t, shown, day, + "the comment is dated %q, which is not the reader's own day in %s", shown, zone) +} + +// TestConfig_PageTitleReachesTheStoredComment covers the title path, which runs the other way +// from everything else here: the host page's title is posted into the widget, the widget sends it +// with the comment, and the backend stores it against the thread. It is what a feed and the admin +// listing show, and nothing else here would notice it going +func TestConfig_PageTitleReachesTheStoredComment(t *testing.T) { + title := "A title only this test uses " + runID + + page := newPage(t) + embedConfig(t, page, map[string]any{}) + frame := widget(t, page) + + // set after the widget is up, so this covers the observer embed.ts installs on the title + // element and not merely the value read once at boot + _, err := page.Evaluate(`(t) => { document.title = t; }`, title) + require.NoError(t, err) + + signInAnon(t, page, frame, anonName("pagetitle")) + text := "titled " + runID + postComment(t, frame, text) + + status, body := pageFetch(t, page, "GET", + fmt.Sprintf("%s/api/v1/find?site=remark&url=%s&format=tree", baseURL, neturl.QueryEscape(threadURL(t))), nil) + require.Equal(t, http.StatusOK, status, "could not read the thread back: %s", body) + + var thread struct { + Comments []struct { + Comment struct { + Title string `json:"title"` + } `json:"comment"` + } `json:"comments"` + } + require.NoError(t, json.Unmarshal([]byte(body), &thread)) + require.NotEmpty(t, thread.Comments) + assert.Equal(t, title, thread.Comments[0].Comment.Title, + "the host page's title never reached the stored comment") +} diff --git a/e2e/crossorigin_test.go b/e2e/crossorigin_test.go new file mode 100644 index 00000000..ec161aef --- /dev/null +++ b/e2e/crossorigin_test.go @@ -0,0 +1,99 @@ +//go:build e2e + +package e2e + +import ( + "fmt" + "testing" + "time" + + "github.com/mxschmitt/playwright-go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestCrossOrigin_WidgetRendersOnAnotherOrigin covers the separate-domain setup the manuals +// describe, which is the configuration readers actually hit problems with and which nothing else +// here reaches: every other host page in this suite is served by remark42 itself, so the widget +// and its embedder always share an origin and the cross-site path is never taken. +// +// The host page is served by its own nginx on a different name and port, and the widget it embeds +// addresses remark42 by remark_config.host. What has to hold is that the frame is revealed at all, +// which means its document loaded and reported itself inited through postMessage across origins, +// and that the thread it renders is the one the page's own address names. +// +// Signing in is deliberately not asserted. An embedded cookie needs SameSite=None, which browsers +// only accept as Secure, and this stack speaks http, so the form cannot be delivered here at all; +// see "What this suite cannot reach" in the README. Give the stack TLS and the case to add is +// signing in and then reloading, since the widget holds its token in memory for the life of a +// page and a sign-in that never reloads passes while persistence is broken +func TestCrossOrigin_WidgetRendersOnAnotherOrigin(t *testing.T) { + thread := fmt.Sprintf("%s/post.html?e2e=%s-%s", hostSiteURL, "crossorigin", runID) + text := "cross origin " + runID + + // seeded from a page on remark42's own origin, since posting needs a session and this case is + // about rendering, not about carrying a cookie across origins + seeder := newPage(t) + seederFrame := openThread(t, seeder) + signInAnon(t, seeder, seederFrame, anonName("crossorigin")) + status, body := pageFetch(t, seeder, "POST", baseURL+"/api/v1/comment?site=remark", map[string]any{ + "text": text, + "locator": map[string]string{"site": "remark", "url": thread}, + }) + require.Equal(t, 201, status, "could not seed the cross-origin thread: %s", body) + + page := newPage(t) + stubSignedOut(t, page) + pauseForAuthLimit() + _, err := page.Goto(thread) + require.NoError(t, err) + + frame := widget(t, page) + + // revealed, so the document loaded and its inited message crossed the origin boundary. a + // frame that never reported would still be here, hidden, until the fallback timer + waitVisible(t, page.Locator("#remark42 iframe")) + waitVisible(t, comment(frame, text)) +} + +// TestCrossOrigin_DisallowedHostNeverReportsInited is the other half of ALLOWED_HOSTS. An +// operator sets it so their comments cannot be framed by anyone else, and what the reader on such +// a page gets is decided entirely by the widget's own fallback: the browser refuses to load the +// document, nothing ever posts inited, and the frame is revealed five seconds later by the timer. +// +// Worth pinning in both directions. Without the fallback an integrator who mistyped the host +// would be left with a permanently invisible widget and nothing in the page to say why, and +// without the refusal ALLOWED_HOSTS would be doing nothing at all +func TestCrossOrigin_DisallowedHostNeverReportsInited(t *testing.T) { + page := newPage(t) + stubSignedOut(t, page) + + pauseForAuthLimit() + _, err := page.Goto(fmt.Sprintf("%s/restricted.html?e2e=%s-%s", hostSiteURL, "crossorigin-blocked", runID)) + require.NoError(t, err) + + require.NoError(t, page.Locator("#remark42 iframe").WaitFor(playwright.LocatorWaitForOptions{ + State: playwright.WaitForSelectorStateAttached, + Timeout: playwright.Float(float64(waitTimeout.Milliseconds())), + })) + + // hidden for as long as the fallback runs, since the document the browser refused cannot + // report anything + require.Equal(t, "hidden", iframeVisibility(t, page)) + + eventually(t, revealTimeout*2, "the frame was never revealed, so a mistyped host would stay invisible", func() bool { + _, ok := revealDelay(t, page) + return ok + }) + + delay, ok := revealDelay(t, page) + require.True(t, ok) + assert.Greater(t, delay, revealTimeout-500*time.Millisecond, + "the frame was revealed too early to have been the fallback, so something reported inited "+ + "from a document the browser should not have loaded") + + // and the widget really is not there, which is what makes the reveal above the fallback's + forms, err := page.FrameLocator("#remark42 iframe").Locator(commentFormSel).Count() + require.NoError(t, err) + assert.Zero(t, forms, "the widget rendered on a host the instance does not allow") +} diff --git a/e2e/deployment_test.go b/e2e/deployment_test.go new file mode 100644 index 00000000..851fd72b --- /dev/null +++ b/e2e/deployment_test.go @@ -0,0 +1,116 @@ +//go:build e2e + +package e2e + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Three configurations that cannot be settings on an instance shared with anything else: each +// changes the widget for every reader of that instance, and each broke in a way nothing else in +// this suite can reach. Their services are in compose-e2e-test.yml. + +// adminEditAddress is the address whose email-derived id compose names in ADMIN_SHARED_ID, so +// signing in with it on that instance produces an admin. +// +// Email and not anonymous: remark42 hashes an anonymous id from the name and the client address +// together (server.go:1220, to tell apart two people picking the same name), so an id written +// down here would belong to nobody on any machine but the one it was read from. The email id is +// sha1 of the address alone +const adminEditAddress = "adminedit@example.com" + +// TestComment_AdminEditHasNoDeadline covers #1986: the backend honored ADMIN_EDIT while the +// frontend went on counting an admin's edit window down and taking the button away at the end, +// so the setting did nothing where it is visible. The instance's ordinary window is short, so the +// wait is the same one TestComment_EditExpiresAfterTheDeadline pays, and both halves are asserted +// against it: no countdown at any point, and an edit that still lands afterwards +func TestComment_AdminEditHasNoDeadline(t *testing.T) { + page := newPage(t) + url := threadURLOn(t, adminEditURL) + frame := openURL(t, page, url) + signInEmail(t, page, frame, "admineditor", adminEditAddress) + + text := "admin edit " + runID + postedAt := time.Now() + postComment(t, frame, text) + + // the countdown is what #2001 left running. an admin has no deadline, so it should never + // have been rendered at all + // asserted on a loaded thread, not on the comment the widget has just added to the + // page. The optimistic render puts a countdown on an admin's own new comment and drops it on + // the next load, so asserting here would fail on that instead of on the deadline logic this + // case is about + frame = reload(t, page) + count, err := actions(frame, text).Locator(`[role="timer"]`).Count() + require.NoError(t, err) + assert.Zero(t, count, "an admin's comment is counting down an edit window that does not apply to it") + + // past the window every other user on this instance is held to. editWindow is EDIT_TIME on + // both short-window instances in compose-e2e-test.yml + waitPastEditWindow(postedAt) + + require.NoError(t, actions(frame, text).Locator(`button:has-text("Edit")`).Click()) + edited := "admin edited after the deadline " + runID + submitForm(t, replyForm(t, frame), edited) + waitVisible(t, comment(frame, edited)) + + // stored and not only rendered: the backend has its own view of the deadline, and the + // widget showing the new text says nothing about which of the two answered + frame = reload(t, page) + waitVisible(t, comment(frame, edited)) +} + +// TestAuth_HeaderJWTSurvivesReload covers #1877. With AUTH_SEND_JWT_HEADER the token arrives in a +// response header instead of a cookie, so the frontend holds it itself and a reload starts with +// nothing in hand. Signing out matters as much as signing in: a token kept somewhere the sign-out +// does not clear leaves a session that outlives the button +func TestAuth_HeaderJWTSurvivesReload(t *testing.T) { + page := newPage(t) + frame := openURL(t, page, threadURLOn(t, jwtHeaderURL)) + signInAnon(t, page, frame, anonName("headerjwt")) + + frame = reload(t, page) + assertSignedIn(t, page, frame) + + pauseForAuthLimit() + require.NoError(t, frame.Locator(`[title="Sign Out"]`).Click()) + waitVisible(t, frame.Locator(".auth-button")) + + frame = reload(t, page) + waitVisible(t, frame.Locator(".auth-button")) + waitHidden(t, frame.Locator(`[title="Sign Out"]`), + "the header-borne session came back after signing out") +} + +// TestAuth_NoProvidersSaysSo covers #1456, where an instance with no auth provider rendered a +// sign-in panel offering nothing and no explanation, so the operator's own misconfiguration read +// as the widget being broken +func TestAuth_NoProvidersSaysSo(t *testing.T) { + page := newPage(t) + frame := openURL(t, page, threadURLOn(t, noAuthURL)) + + require.NoError(t, frame.Locator(".auth-button").Click()) + waitVisible(t, frame.Locator("text=No providers available")) + + // and no empty form is offered alongside it + inputs, err := frame.Locator(".auth-input-username").Count() + require.NoError(t, err) + assert.Zero(t, inputs, "a sign-in form is offered on an instance with nothing to sign in with") +} + +// waitPastEditWindow waits out the instance's ordinary edit window, measured from the moment the +// comment was posted. There is nothing to poll for here: the admin's comment shows no countdown, +// which is the assertion above, so the deadline passing is not observable in the page. Sleeping +// until a deadline computed from a known setting is not the same as sleeping for a guess +func waitPastEditWindow(postedAt time.Time) { + // a margin over the window itself, since the backend compares against its own clock and the + // post round trip sits between the two + deadline := postedAt.Add(editWindow + time.Second) + if wait := time.Until(deadline); wait > 0 { + time.Sleep(wait) + } +} diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go index cf0b6286..db685081 100644 --- a/e2e/e2e_test.go +++ b/e2e/e2e_test.go @@ -6,11 +6,19 @@ // up itself when nothing is listening. Files: // // - e2e_test.go: TestMain, shared helpers, constants +// - harness_test.go: the stale-stack guard and the browser failures no assertion covers // - auth_test.go: dev, anonymous and email sign-in // - comment_test.go: post, reply, edit, delete // - vote_test.go: voting and its failure path // - thread_test.go: sorting and collapse persistence // - iframe_test.go: the iframe's color scheme and reveal handshake +// - geometry_test.go: the height the widget reports and the parent applies +// - embed_test.go: the surface the host page holds, placeholder to destroy +// - config_test.go: the remark_config surface an integrator sets +// - crossorigin_test.go: a host page on an origin the widget is not served from +// - deployment_test.go: the instances whose configuration is the thing under test +// - subscribe_test.go: the email subscription round trip +// - webfiles_test.go: the published /web surface // - widgets_test.go: last-comments, counter and the profile iframe package e2e @@ -28,22 +36,37 @@ import ( "sync/atomic" "testing" "time" + "unicode" "github.com/mxschmitt/playwright-go" "github.com/stretchr/testify/require" ) const ( - // what the browser asks for. these have to be names rather than 127.0.0.1: the dev oauth2 + // what the browser asks for. these have to be names, not 127.0.0.1: the dev oauth2 // server binds whatever host it reads out of REMARK_URL, and a loopback bind inside the // container cannot be published, so the browser resolves the names back with // --host-resolver-rules and reaches the published ports baseURL = "http://remark42:8080" shortEditURL = "http://remark42-shortedit:8081" + // the deployment modes that cannot be a setting on an instance shared with anything else, + // each covering a regression the default configuration cannot reach. see compose-e2e-test.yml + adminEditURL = "http://remark42-adminedit:8082" + jwtHeaderURL = "http://remark42-jwtheader:8083" + noAuthURL = "http://remark42-noauth:8085" + anonVoteURL = "http://remark42-anonvote:8086" + + // a page on an origin the widget is not served from, see compose-e2e-test.yml + hostSiteURL = "http://host-site:8090" // what this process asks for, since it is not the browser and has no resolver rules probeURL = "http://127.0.0.1:8080" shortEditProbeURL = "http://127.0.0.1:8081" + adminEditProbeURL = "http://127.0.0.1:8082" + jwtHeaderProbeURL = "http://127.0.0.1:8083" + noAuthProbeURL = "http://127.0.0.1:8085" + anonVoteProbeURL = "http://127.0.0.1:8086" + hostSiteProbeURL = "http://127.0.0.1:8090" mailpitURL = "http://127.0.0.1:8025" composeFile = "../compose-e2e-test.yml" @@ -57,6 +80,11 @@ const ( // generous because CI runners are slower and less predictable than a laptop waitTimeout = 15 * time.Second + // how long assertSignedIn waits before nudging the widget into re-reading auth state. short + // enough that a refused status read costs a fraction of a test instead of a whole timeout, + // long enough that the ordinary path never takes the nudge + authRepaintWait = 3 * time.Second + // what a single locator call inside a poll body gets. playwright's own default is 30s, // twice the budget of the loops here, so without this a missing element would block one // attempt for longer than the whole loop and then report the wrong thing @@ -73,7 +101,7 @@ var ( extraBrowsersMu sync.Mutex // distinguishes this run's threads from those a previous run left in the database. - // E2E_RUN_ID pins it, so the urls a CI run works on name that run rather than the moment + // E2E_RUN_ID pins it, so the urls a CI run works on name that run and not the moment // the process started runID = firstNonEmpty(os.Getenv("E2E_RUN_ID"), fmt.Sprintf("%d", time.Now().UnixNano())) @@ -88,12 +116,16 @@ var ( ) // everything under /auth/ is rate limited to 2 requests a second, and that figure is a bare -// literal at backend/app/rest/api/rest.go:242 rather than a setting, so the suite has to pace +// literal at backend/app/rest/api/rest.go:242 and not a setting, so the suite has to pace // itself: the widget calls /auth/status on every load, and again on visibilitychange or // window focus while an oauth popup sign-in is pending. without this the limiter starts // answering 429 and the widget renders as signed out func pauseForAuthLimit() { - const spacing = 700 * time.Millisecond + // 1200ms, up from the 700 this started at: the widget fires an unpaced /auth/status on + // every load and again on visibilitychange, so the paced side has to stay well under the + // 2/s cap to leave room for them. at 700 the suite manufactured its own 429s, and a lost + // probe renders as signed out, which fails whichever test happens to be signing in + const spacing = 1200 * time.Millisecond authGate.Lock() defer authGate.Unlock() @@ -129,7 +161,10 @@ func TestMain(m *testing.M) { Headless: playwright.Bool(headless), SlowMo: playwright.Float(slowMo), Args: []string{ - "--host-resolver-rules=MAP remark42 127.0.0.1, MAP remark42-shortedit 127.0.0.1", + "--host-resolver-rules=MAP remark42 127.0.0.1, MAP remark42-shortedit 127.0.0.1, " + + "MAP remark42-adminedit 127.0.0.1, MAP remark42-jwtheader 127.0.0.1, " + + "MAP remark42-noauth 127.0.0.1, MAP remark42-anonvote 127.0.0.1, " + + "MAP host-site 127.0.0.1", }, }) if err != nil { @@ -150,11 +185,22 @@ func TestMain(m *testing.M) { // ensureStack waits for a running stack and starts one with compose when there is none func ensureStack() error { + // the digest of the sources the image is built from, exported so compose stamps the build + // with it. read back off the running stack below, since answering on the right ports says + // nothing about which checkout the code came from + stamp, err := sourceStamp() + if err != nil { + return err + } + if err := os.Setenv(stampEnv, stamp); err != nil { + return fmt.Errorf("exporting %s: %w", stampEnv, err) + } + // every service, not just the first: compose-dev-backend.yml and `make rundev` also // publish 8080, and adopting one of those would run the suite against a developer's own // database and fail later as unexplained locator timeouts if stackReady(2 * time.Second) { - return nil + return assertStackMatches(stamp, true) } log.Printf("[INFO] no complete stack on 127.0.0.1, bringing one up from %s", composeFile) @@ -175,7 +221,9 @@ func ensureStack() error { if !stackReady(waitTimeout) { return fmt.Errorf("compose reported the stack healthy but it does not answer") } - return nil + // checked after our own build too, so a stamp that never reaches the image is a failure + // here, and not a guard that silently passes everything for the rest of its life + return assertStackMatches(stamp, false) } func firstNonEmpty(values ...string) string { @@ -192,6 +240,11 @@ func stackReady(timeout time.Duration) bool { for _, url := range []string{ probeURL + "/ping", shortEditProbeURL + "/ping", + adminEditProbeURL + "/ping", + jwtHeaderProbeURL + "/ping", + noAuthProbeURL + "/ping", + anonVoteProbeURL + "/ping", + hostSiteProbeURL + "/post.html", mailpitURL + "/api/v1/messages", } { if err := serverReady(url, timeout); err != nil { @@ -246,11 +299,18 @@ func newPage(t *testing.T) playwright.Page { func newPageOn(t *testing.T, b playwright.Browser) playwright.Page { t.Helper() - ctx, err := b.NewContext() + return newPageInContext(t, b, playwright.BrowserNewContextOptions{}) +} + +// newPageInContext is newPageOn with the context configured, for the cases where what the browser +// says about itself is the thing under test +func newPageInContext(t *testing.T, b playwright.Browser, opts playwright.BrowserNewContextOptions) playwright.Page { + t.Helper() + ctx, err := b.NewContext(opts) require.NoError(t, err) // the reveal timers start when the iframe element is created, so the tests that bound - // them have to measure from there rather than from anything this process can time + // them have to measure from there and not from anything this process can time require.NoError(t, ctx.AddInitScript(playwright.Script{Content: playwright.String(iframeMarkScript)})) tracing := ctx.Tracing().Start(playwright.TracingStartOptions{ @@ -268,7 +328,7 @@ func newPageOn(t *testing.T, b playwright.Browser) playwright.Page { _ = ctx.Close() return } - // say so rather than swallowing it: this runs only on a test that already failed, + // say so instead of swallowing it: this runs only on a test that already failed, // and a silently missing trace is what the reader goes looking for // the pid keeps two processes sharing a run id, and so a trace directory, from // overwriting each other: the counter restarts with every process @@ -288,12 +348,17 @@ func newPageOn(t *testing.T, b playwright.Browser) playwright.Page { page, err := ctx.NewPage() require.NoError(t, err) + // before the first navigation, so nothing the page reports on its way up is missed + watchPage(t, page) debug := os.Getenv("E2E_DEBUG") != "" page.OnResponse(func(r playwright.Response) { switch { case r.Status() == http.StatusTooManyRequests: // the rate limiter answers the widget, not the test, so without this the failure - // arrives as an unexplained locator timeout + // arrives as an unexplained locator timeout. recorded as well as logged: a lost + // /auth/status renders as signed out, and every sign-in assertion then fails + // somewhere that does not name the cause + recordPageIssue(page, "rate limited", r.URL()) log.Printf("[WARN] rate limited: %s", r.URL()) case debug && r.Status() >= 400: body, _ := r.Text() @@ -382,6 +447,31 @@ func threadURLOn(t *testing.T, base string) string { return fmt.Sprintf("%s/web/?e2e=%s-%s", base, name, runID) } +// anonName builds a username no earlier run has used. Blocking a user and verifying one are +// properties of the user, and the stack's database outlives the run, so a name that comes back +// arrives already blocked or already verified: the case then toggles the state off and waits for +// something that is being taken away. +// +// The run id alone is not enough, since E2E_RUN_ID pins it and a second run against a surviving +// stack repeats it, which is exactly what `make e2e-up` invites. The pid distinguishes those +// while leaving CI, where every job is one process, with the run id it wants in the name. +// +// Everything outside [\p{L}\d\s_] is dropped, because the form validates its input against that +// pattern (auth.tsx:332) and the browser refuses to submit anything else: no request is made, and +// the test waits out its timeout on a panel that was never going to change. E2E_RUN_ID is +// "-" on CI, whose hyphen is exactly such a character +func anonName(prefix string) string { + var b strings.Builder + b.WriteString(prefix) + for _, r := range runID { + if unicode.IsLetter(r) || unicode.IsDigit(r) || r == '_' { + b.WriteRune(r) + } + } + fmt.Fprintf(&b, "_%d", os.Getpid()) + return b.String() +} + // openThread loads the demo page on this test's own thread and waits for the widget func openThread(t *testing.T, page playwright.Page) playwright.FrameLocator { t.Helper() @@ -398,6 +488,63 @@ func openURL(t *testing.T, page playwright.Page, url string) playwright.FrameLoc return widget(t, page) } +// embedConfig loads a page carrying a remark_config of the test's choosing and runs the embed +// script against it. +// +// The demo page cannot be used for this: it writes its own remark_config, so anything a test +// needs to vary has to go into a page that carries no widget of its own. privacy.html is served +// by the instance and has none, which also keeps the widget on its own origin, where its CSP +// allows the chunks it loads. host, site_id and url are filled in when the caller leaves them out +// placeholder, when given, is markup left inside the root before the widget runs, which the +// documentation promises is cleared once the iframe reports itself inited +func embedConfig(t *testing.T, page playwright.Page, config map[string]any, placeholder ...string) { + t.Helper() + embedConfigOn(t, page, "/web/privacy.html", config, placeholder...) +} + +// embedConfigOn is embedConfig on a chosen host page, for the cases where the page's own address +// is part of what is under test +func embedConfigOn(t *testing.T, page playwright.Page, hostPage string, config map[string]any, placeholder ...string) { + t.Helper() + + for key, value := range map[string]any{"host": baseURL, "site_id": "remark", "url": threadURL(t)} { + if _, ok := config[key]; !ok { + config[key] = value + } + } + + pauseForAuthLimit() + _, err := page.Goto(baseURL + hostPage) + require.NoError(t, err) + + _, err = page.Evaluate(`([config, placeholder]) => { + window.remark_config = config; + const node = document.createElement('div'); + node.id = 'remark42'; + node.innerHTML = placeholder; + document.body.appendChild(node); + }`, []any{config, strings.Join(placeholder, "")}) + require.NoError(t, err) + + _, err = page.AddScriptTag(playwright.PageAddScriptTagOptions{URL: playwright.String(baseURL + "/web/embed.mjs")}) + require.NoError(t, err) +} + +// stubSignedOut answers the widget's auth probe from the browser, for a page that never signs +// in. /auth/ is capped at two requests a second for the whole suite and the widget probes on +// every load, so a case that only needs a signed-out widget should not spend that budget: the +// tests that do sign in are the ones that cannot fake it +func stubSignedOut(t *testing.T, page playwright.Page) { + t.Helper() + require.NoError(t, page.Route("**/auth/status**", func(route playwright.Route) { + require.NoError(t, route.Fulfill(playwright.RouteFulfillOptions{ + Status: playwright.Int(http.StatusOK), + ContentType: playwright.String("application/json"), + Body: playwright.String(`{"status":"not logged in"}`), + })) + })) +} + // reload re-navigates and returns the widget once the thread itself has loaded. // // waiting for the comment form is not enough here: root.tsx renders it as soon as the user @@ -437,7 +584,7 @@ func postCommentMatching(t *testing.T, frame playwright.FrameLocator, source, ma } // submitForm fills a comment form and submits it. the submit label is Send, Reply or Save -// depending on the form's mode, so match on the type rather than the text +// depending on the form's mode, so match on the type and not the text func submitForm(t *testing.T, form playwright.Locator, text string) { t.Helper() require.NoError(t, form.Locator("textarea").Fill(text)) @@ -487,7 +634,7 @@ func actions(frame playwright.FrameLocator, text string) playwright.Locator { } // widget returns the widget's iframe once its own content has rendered. it waits on the -// comment form rather than the auth panel, because .auth is only present while signed out +// comment form and not the auth panel, because .auth is only present while signed out func widget(t *testing.T, page playwright.Page) playwright.FrameLocator { t.Helper() frame := page.FrameLocator("#remark42 iframe") @@ -503,12 +650,15 @@ func waitVisible(t *testing.T, loc playwright.Locator) { })) } -func waitHidden(t *testing.T, loc playwright.Locator) { +// waitHidden waits for the element to go away. msgAndArgs says what its staying would mean, +// since the failure otherwise names a selector and not the behavior that broke +func waitHidden(t *testing.T, loc playwright.Locator, msgAndArgs ...any) { t.Helper() - require.NoError(t, loc.WaitFor(playwright.LocatorWaitForOptions{ + err := loc.WaitFor(playwright.LocatorWaitForOptions{ State: playwright.WaitForSelectorStateHidden, Timeout: playwright.Float(float64(waitTimeout.Milliseconds())), - })) + }) + require.NoError(t, err, msgAndArgs...) } // pollText reads an element's text with a timeout short enough to be used inside eventually diff --git a/e2e/embed_test.go b/e2e/embed_test.go new file mode 100644 index 00000000..e40e8b93 --- /dev/null +++ b/e2e/embed_test.go @@ -0,0 +1,121 @@ +//go:build e2e + +package e2e + +import ( + "testing" + + "github.com/mxschmitt/playwright-go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestEmbed_PlaceholderGivesWayToOneOwnedIframe covers #2192 together with the placeholder +// support #2002 added. createInstance took root.firstElementChild as its iframe, so anything the +// integrator left in the root was adopted instead: a