Refactor the Playwright suite onto page objects, shared fixtures, and a typed test build #137

New Issue

2026-08-23T08:16:21-05:00

bermudalamb commented

2026-08-23 08:16:21 -05:00

The e2e suite is 25 spec files and 2,687 lines, and it has grown by copy-paste. Reading a test means reading its locators, and changing a shared behaviour means finding every file that reimplemented it.

Registering a customer is implemented nine times — register in five files and registerCustomer in four more — and each copy carries its own re-explanation of the same bcrypt wait and the same "the header, not the URL, proves the session exists" reasoning. uniqueEmail is reinvented per file, each with a different prefix and a different encoding of Date.now() and Math.random(). Seeding an item through the admin API is written out at least twice, once with a hardcoded http://localhost:5173 base URL.

Thirteen locators reach directly into antd's internals: .ant-select-item-option[title="..."], .ant-tabs-tab-active .ant-tabs-tab-btn, .ant-tabs-tabpane-active .ant-tree, .ant-col, .item-card. These are the ones that break on an antd upgrade, and they break spread across seven files with no single place to fix them.

The suite uses zero data-testid and is entirely role- and label-driven. That is the good part and this refactor should preserve it as the default rather than introducing test ids wholesale.

Nothing type-checks or lints any of this. tsconfig.json includes only src, and eslint.config.mjs scopes itself to src/** with a note saying tests are deliberately out of scope because there is no program for them. That is tolerable for specs that are each self-contained. It stops being tolerable the moment page objects become shared code that 25 files depend on, because a typo in one surfaces as a confusing runtime failure across many specs rather than as a compile error.

What this covers

Page objects for each surface — storefront, auth modal, account, cart and checkout, password reset, and the admin shell with its inventory, taxonomy, customers, emails and settings tabs — each holding named locators and the actions that operate on them, so a spec reads as intent rather than as selectors.

Shared fixtures carrying the setup the specs currently duplicate: a registered customer, a seeded published item, and an admin API context that gets its base URL from the Playwright config instead of a hardcoded string.

password-reset.spec.ts opens its own pg.Client inline to read a reset token. That access is deliberate and should stay in the test process — the existing comment explains why an endpoint returning a reset token for an arbitrary address is account takeover if it ever ships, and that reasoning holds. But it should live behind a shared db helper rather than being copy-pasteable, and it should stop defaulting to port 55432, which is Hyper-V-reserved on at least one dev machine and makes the spec fail with a bare ECONNREFUSED.

A tsconfig covering tests/, with eslint widened to match, so the page objects are checked.

Conventions to settle in the design

Page objects expose named locators and actions; assertions stay in the specs, so a test states its own claim. The single exception is an action waiting for its own completion — register waiting for the account button — because that is the action's contract rather than a test's assertion.

Sequencing

One issue, staged commits: build the page objects and fixtures first, then convert specs in themed batches — admin, storefront, auth — one commit per batch, with the suite green between every batch. A single 25-file commit would be unreviewable and impossible to bisect.

Open decision

How the page objects reach the specs is not yet settled. The recommendation is classes delivered through Playwright fixtures, so a spec declares what it needs and gets it constructed and page-bound, composing with the existing collectCoverage fixture. The alternatives considered are hand-instantiated classes, and plain locator modules with no classes at all. To be confirmed before the design is written.

Out of scope

The shared-database interference between the e2e suite and the backend integration suite is #116 and stays there. This issue must not quietly become a fix for it.

The e2e suite is 25 spec files and 2,687 lines, and it has grown by copy-paste. Reading a test means reading its locators, and changing a shared behaviour means finding every file that reimplemented it. Registering a customer is implemented **nine** times — `register` in five files and `registerCustomer` in four more — and each copy carries its own re-explanation of the same bcrypt wait and the same "the header, not the URL, proves the session exists" reasoning. `uniqueEmail` is reinvented per file, each with a different prefix and a different encoding of `Date.now()` and `Math.random()`. Seeding an item through the admin API is written out at least twice, once with a hardcoded `http://localhost:5173` base URL. Thirteen locators reach directly into antd's internals: `.ant-select-item-option[title="..."]`, `.ant-tabs-tab-active .ant-tabs-tab-btn`, `.ant-tabs-tabpane-active .ant-tree`, `.ant-col`, `.item-card`. These are the ones that break on an antd upgrade, and they break spread across seven files with no single place to fix them. The suite uses zero `data-testid` and is entirely role- and label-driven. That is the good part and this refactor should preserve it as the default rather than introducing test ids wholesale. Nothing type-checks or lints any of this. `tsconfig.json` includes only `src`, and `eslint.config.mjs` scopes itself to `src/**` with a note saying tests are deliberately out of scope because there is no program for them. That is tolerable for specs that are each self-contained. It stops being tolerable the moment page objects become shared code that 25 files depend on, because a typo in one surfaces as a confusing runtime failure across many specs rather than as a compile error. ### What this covers Page objects for each surface — storefront, auth modal, account, cart and checkout, password reset, and the admin shell with its inventory, taxonomy, customers, emails and settings tabs — each holding named locators and the actions that operate on them, so a spec reads as intent rather than as selectors. Shared fixtures carrying the setup the specs currently duplicate: a registered customer, a seeded published item, and an admin API context that gets its base URL from the Playwright config instead of a hardcoded string. `password-reset.spec.ts` opens its own `pg.Client` inline to read a reset token. That access is deliberate and should stay in the test process — the existing comment explains why an endpoint returning a reset token for an arbitrary address is account takeover if it ever ships, and that reasoning holds. But it should live behind a shared `db` helper rather than being copy-pasteable, and it should stop defaulting to port `55432`, which is Hyper-V-reserved on at least one dev machine and makes the spec fail with a bare `ECONNREFUSED`. A tsconfig covering `tests/`, with eslint widened to match, so the page objects are checked. ### Conventions to settle in the design Page objects expose named locators and actions; assertions stay in the specs, so a test states its own claim. The single exception is an action waiting for its own completion — `register` waiting for the account button — because that is the action's contract rather than a test's assertion. ### Sequencing One issue, staged commits: build the page objects and fixtures first, then convert specs in themed batches — admin, storefront, auth — one commit per batch, with the suite green between every batch. A single 25-file commit would be unreviewable and impossible to bisect. ### Open decision How the page objects reach the specs is not yet settled. The recommendation is classes delivered through Playwright fixtures, so a spec declares what it needs and gets it constructed and page-bound, composing with the existing `collectCoverage` fixture. The alternatives considered are hand-instantiated classes, and plain locator modules with no classes at all. To be confirmed before the design is written. ### Out of scope The shared-database interference between the e2e suite and the backend integration suite is #116 and stays there. This issue must not quietly become a fix for it.

bermudalamb commented

2026-08-23 08:16:34 -05:00

Clarifying questions and answers from the brainstorming pass, recorded here so the decisions are not only in a chat log.

Q: How much should the refactor take on beyond readability — page objects only, page objects plus shared fixtures, or those plus the database coupling?

A: All three. Page objects and named elements are the stated goal, but the suite's worst duplication is setup rather than selectors — nine implementations of registering a customer — and only a fixture layer removes that. The same layer is where the inline pg.Client in password-reset.spec.ts belongs, so taking the database coupling in the same pass costs little and stops one spec knowing Postgres connection details.

Q: How should the work be sequenced, given it touches all 25 spec files?

A: One issue, staged commits. Page objects and fixtures land first, then specs convert in themed batches — admin, storefront, auth — one commit each, suite green between every batch. Bisectable, and each commit is reviewable on its own.

Q: Nothing type-checks or lints the e2e tests today. Should this work bring them in?

A: Yes — a tsconfig covering tests/, and eslint widened to match. Page objects are shared code that 25 specs depend on, and an unchecked typo in one becomes a confusing runtime failure spread across many specs. Expect an initial batch of pre-existing warnings to triage; that triage is part of this issue.

Q: Where should assertions live once page objects exist?

A: In the specs. Page objects expose named locators and actions; every expect() stays in the test, so a test reads as its own statement of what it verifies. The one exception is an action that waits for its own completion — register waiting for the account button to appear — because that wait is the action's contract, and pushing it to callers would recreate the duplication being removed.

Clarifying questions and answers from the brainstorming pass, recorded here so the decisions are not only in a chat log. **Q: How much should the refactor take on beyond readability — page objects only, page objects plus shared fixtures, or those plus the database coupling?** A: All three. Page objects and named elements are the stated goal, but the suite's worst duplication is setup rather than selectors — nine implementations of registering a customer — and only a fixture layer removes that. The same layer is where the inline `pg.Client` in `password-reset.spec.ts` belongs, so taking the database coupling in the same pass costs little and stops one spec knowing Postgres connection details. **Q: How should the work be sequenced, given it touches all 25 spec files?** A: One issue, staged commits. Page objects and fixtures land first, then specs convert in themed batches — admin, storefront, auth — one commit each, suite green between every batch. Bisectable, and each commit is reviewable on its own. **Q: Nothing type-checks or lints the e2e tests today. Should this work bring them in?** A: Yes — a tsconfig covering `tests/`, and eslint widened to match. Page objects are shared code that 25 specs depend on, and an unchecked typo in one becomes a confusing runtime failure spread across many specs. Expect an initial batch of pre-existing warnings to triage; that triage is part of this issue. **Q: Where should assertions live once page objects exist?** A: In the specs. Page objects expose named locators and actions; every `expect()` stays in the test, so a test reads as its own statement of what it verifies. The one exception is an action that waits for its own completion — `register` waiting for the account button to appear — because that wait is the action's contract, and pushing it to callers would recreate the duplication being removed.

bermudalamb added this to the Code Quality and Hardening 2 project 2026-08-23 08:17:12 -05:00

bermudalamb self-assigned this 2026-08-23 08:17:21 -05:00

bermudalamb referenced this issue from a commit

2026-08-23 17:17:38 -05:00

test(e2e): add page objects, fixtures and a typed test build (#137)

bermudalamb referenced this issue

2026-08-23 17:18:46 -05:00

test(e2e): add page objects, fixtures and a typed test build (#137) #149

bermudalamb added reference feature/137-playwright-page-objects

2026-08-23 17:19:25 -05:00

bermudalamb moved this to In Progress in Code Quality and Hardening 2 on 2026-08-23 17:19:34 -05:00

bermudalamb referenced this issue from a commit