Skip to content

e2e: cover logged-in state; fix rating-button contrast and login-page JS error - #13366

Draft
lokesh wants to merge 1 commit into
internetarchive:masterfrom
lokesh:e2e-fixes
Draft

e2e: cover logged-in state; fix rating-button contrast and login-page JS error#13366
lokesh wants to merge 1 commit into
internetarchive:masterfrom
lokesh:e2e-fixes

Conversation

@lokesh

@lokesh lokesh commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Closes #

The Playwright e2e suite only ever ran anonymously — no test logged in. This adds logged-in coverage alongside the existing anonymous checks, and fixes two bugs that coverage exposed:

  • Contrast fix on book pages: the "Clear my rating" button (.star-messaging) used --mid-grey, a non-text token, and failed WCAG AA at 2.48:1. Now --color-text-muted. Only visible once a patron has rated the book, which is why the anonymous a11y scan never saw it.
  • JS error on /account/login for logged-in users (dev only): the LOCAL_DEV credential-autofill hint rendered even when the login form didn't, so index.js threw on a null #username.

Technical

  • tests/e2e/helpers.ts gains login(page), which POSTs /account/login.json through page.request — that shares the context's cookie jar, so the next page.goto() is authenticated without driving the form. Specs use it as test.beforeEach(({ page }) => login(page)) inside a test.describe('when logged in') block; this is the pattern for any future logged-in e2e test. Dev defaults to the seeded openlibrary patron; elsewhere OL_E2E_USERNAME/PASSWORD, OL_E2E_S3_ACCESS/SECRET, or OL_E2E_EMAIL (form path), otherwise logged-in tests skip. Documented in tests/e2e/README.md.
  • New logged-in checks: header shows account menu vs Log In/Sign Up; reading-log dropper enabled vs disabled; already-logged-in notice on /account/login with no console errors; axe scans of home + work page while logged in; new my-books.spec.ts (anon → redirect to login, logged in → /people/<user>/books and shelves).
  • The old "invalid credentials" login test was passing vacuously: the mock IA auth in docker/mockservices accepts any non-empty password, so wrongpassword123 was actually logging in and its only assertion was url.not.toContain('500'). The mock now rejects the sentinel password bad_password (with a unit test), and the e2e test asserts an error message renders and the page stays on /account/login. Also added a real form-login happy path (form is type=email, so it needs an email rather than a username).

Not addressed here: /people/<user>/books has several axe violations (cover images without alt, invalid list structure, unlabeled <select>), so no full-page a11y assertion was added for it — that's its own fix PR. The suite is still local-only, not in CI.

Testing

OL_MOUNT_DIR="$(pwd)" docker compose up -d --build mockservices   # picks up the sentinel
docker compose exec web make css                                    # picks up the token change
npm run test:e2e

Locally: 36 passed, 7 skipped (opt-in OL_VISUAL snapshots). Both app fixes were observed failing before and passing after: the pageerror on /account/login and the color-contrast violation on the work page each appear in a pre-fix logged-in run and are gone post-fix. pytest docker/mockservices/tests/test_e2e.py -k Xauthn passes in the home container.

Screenshot

n/a — the only visual change is the "Clear my rating" button going from neutral-400 to neutral-500 grey.

Stakeholders

The Playwright suite only ever ran anonymously. Add a login() helper and
'when logged in' blocks (home, work, login, a11y, new my-books spec).

Running pages logged in surfaced:
- login.html rendered the LOCAL_DEV autofill hint for logged-in users,
  whose form isn't rendered, so index.js threw on a null #username.
- .star-messaging ("Clear my rating") used the non-text --mid-grey token
  and failed WCAG contrast (2.48:1).

The 'invalid credentials' login test was passing vacuously: the mock IA
auth accepts any non-empty password, so it was actually logging in.
Mock now rejects the sentinel 'bad_password' so the error path is real.

Claude-Session: https://claude.ai/code/session_01Vmhke1AR5VjcNKP7KnciCJ
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant