test(e2e): convert the auth specs onto page objects (#137) #150

2026-08-23T17:34:17-05:00

bermudalamb commented

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

First conversion batch: auth, password-reset, resend-verification. verify-email is left alone — it already used semantic locators and duplicated nothing, and rewriting it to prove a point would be churn.

Four of the nine copies of "register a customer" go here. auth.spec.ts and password-reset.spec.ts each carried their own register, and resend-verification.spec.ts its own registerCustomer, all three re-explaining the same bcrypt wait and the same "the header, not the URL, proves the session exists" reasoning in slightly different words. Two also carried their own logout, and two their own uniqueEmail with different prefixes.

Most of those tests were not about registering. They needed an account to exist so they could test logging out, resetting a password, or resending a verification email, and they paid for a bcrypt round-trip through the form to get one. Those now take the customer fixture, which registers through the API. The three tests that genuinely are about the registration form still drive it, because the thing under test has to be the thing exercised.

The batch drops from 47s to 26s as a side effect, which is the cost of that round-trip made visible.

password-reset.spec.ts loses its inline pg.Client. The reasoning for reading the database directly is unchanged and still right — an endpoint returning a reset token for an arbitrary address is account takeover if it is ever reachable — but it now lives in support/db.ts where it cannot be copied into the next spec wanting a shortcut. It also stops defaulting to port 55432, which is the integration suite's disposable Postgres rather than the database the app under test is connected to, and is Hyper-V-reserved on at least one machine here. Both tests in that file previously failed with a bare ECONNREFUSED unless TEST_PGPORT was set by hand; they now pass with no environment at all.

New PasswordResetPages object covers both halves of recovery — requesting a link, and using one — because they are one flow and a test usually crosses between them.

One lint decision worth recording. Requesting a Playwright fixture IS using it: destructuring customer is what makes the account exist, whether or not the body then reads the address. The linter cannot see that side effect and reports every such fixture as an unused variable. The first attempt at appeasing it was a void customer; line per test, which is noise standing in for a comment — and sonarjs flags that too, so it traded one warning for another. no-unused-vars is now configured with args: 'none' for tests only, with the reason written next to it. Variables are still checked; only parameters are exempt.

Verified: 26/26 in the converted batch, and 127 passed in the full suite with four failures — three in the known #116 flaky family, and resend-verification's rate-limit test, which passes 9/9 across three repeats in isolation and is timing-sensitive under parallel load rather than changed by this commit.

Refs #137

First conversion batch: auth, password-reset, resend-verification. verify-email is left alone — it already used semantic locators and duplicated nothing, and rewriting it to prove a point would be churn. Four of the nine copies of "register a customer" go here. auth.spec.ts and password-reset.spec.ts each carried their own `register`, and resend-verification.spec.ts its own `registerCustomer`, all three re-explaining the same bcrypt wait and the same "the header, not the URL, proves the session exists" reasoning in slightly different words. Two also carried their own `logout`, and two their own `uniqueEmail` with different prefixes. Most of those tests were not about registering. They needed an account to exist so they could test logging out, resetting a password, or resending a verification email, and they paid for a bcrypt round-trip through the form to get one. Those now take the `customer` fixture, which registers through the API. The three tests that genuinely are about the registration form still drive it, because the thing under test has to be the thing exercised. The batch drops from 47s to 26s as a side effect, which is the cost of that round-trip made visible. password-reset.spec.ts loses its inline pg.Client. The reasoning for reading the database directly is unchanged and still right — an endpoint returning a reset token for an arbitrary address is account takeover if it is ever reachable — but it now lives in support/db.ts where it cannot be copied into the next spec wanting a shortcut. It also stops defaulting to port 55432, which is the integration suite's disposable Postgres rather than the database the app under test is connected to, and is Hyper-V-reserved on at least one machine here. Both tests in that file previously failed with a bare ECONNREFUSED unless TEST_PGPORT was set by hand; they now pass with no environment at all. New PasswordResetPages object covers both halves of recovery — requesting a link, and using one — because they are one flow and a test usually crosses between them. One lint decision worth recording. Requesting a Playwright fixture IS using it: destructuring `customer` is what makes the account exist, whether or not the body then reads the address. The linter cannot see that side effect and reports every such fixture as an unused variable. The first attempt at appeasing it was a `void customer;` line per test, which is noise standing in for a comment — and sonarjs flags that too, so it traded one warning for another. `no-unused-vars` is now configured with `args: 'none'` for tests only, with the reason written next to it. Variables are still checked; only parameters are exempt. Verified: 26/26 in the converted batch, and 127 passed in the full suite with four failures — three in the known #116 flaky family, and resend-verification's rate-limit test, which passes 9/9 across three repeats in isolation and is timing-sensitive under parallel load rather than changed by this commit. Refs #137

bermudalamb added 1 commit 2026-08-23 17:34:17 -05:00

test(e2e): convert the auth specs onto page objects (#137 )

SonarQube Analysis / sonarqube (pull_request) Successful in 18m4s

Details

Linting / lint (pull_request) Successful in 2m9s

Details

abcc684447

First conversion batch: auth, password-reset, resend-verification. verify-email is left alone — it already used semantic locators and duplicated nothing, and rewriting it to prove a point would be churn.

Four of the nine copies of "register a customer" go here. auth.spec.ts and password-reset.spec.ts each carried their own `register`, and resend-verification.spec.ts its own `registerCustomer`, all three re-explaining the same bcrypt wait and the same "the header, not the URL, proves the session exists" reasoning in slightly different words. Two also carried their own `logout`, and two their own `uniqueEmail` with different prefixes.

Most of those tests were not about registering. They needed an account to exist so they could test logging out, resetting a password, or resending a verification email, and they paid for a bcrypt round-trip through the form to get one. Those now take the `customer` fixture, which registers through the API. The three tests that genuinely are about the registration form still drive it, because the thing under test has to be the thing exercised.

The batch drops from 47s to 26s as a side effect, which is the cost of that round-trip made visible.

password-reset.spec.ts loses its inline pg.Client. The reasoning for reading the database directly is unchanged and still right — an endpoint returning a reset token for an arbitrary address is account takeover if it is ever reachable — but it now lives in support/db.ts where it cannot be copied into the next spec wanting a shortcut. It also stops defaulting to port 55432, which is the integration suite's disposable Postgres rather than the database the app under test is connected to, and is Hyper-V-reserved on at least one machine here. Both tests in that file previously failed with a bare ECONNREFUSED unless TEST_PGPORT was set by hand; they now pass with no environment at all.

New PasswordResetPages object covers both halves of recovery — requesting a link, and using one — because they are one flow and a test usually crosses between them.

One lint decision worth recording. Requesting a Playwright fixture IS using it: destructuring `customer` is what makes the account exist, whether or not the body then reads the address. The linter cannot see that side effect and reports every such fixture as an unused variable. The first attempt at appeasing it was a `void customer;` line per test, which is noise standing in for a comment — and sonarjs flags that too, so it traded one warning for another. `no-unused-vars` is now configured with `args: 'none'` for tests only, with the reason written next to it. Variables are still checked; only parameters are exempt.

Verified: 26/26 in the converted batch, and 127 passed in the full suite with four failures — three in the known #116 flaky family, and resend-verification's rate-limit test, which passes 9/9 across three repeats in isolation and is timing-sensitive under parallel load rather than changed by this commit.

Refs #137

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

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

bermudalamb merged commit ed679986de into main

2026-08-23 17:36:31 -05:00

bermudalamb deleted branch feature/137-convert-auth-specs

2026-08-23 17:36:32 -05:00

bermudalamb moved this to Review in Code Quality and Hardening 2 on 2026-08-24 13:14:19 -05:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: bermudalamb/redefined-designs#150