lvl0/dishplanner

Fork 0

55 - Add browser test for registration #62

Merged

myrmidex merged 2 commits from 55-add-browser-test-for-registration into release/v0.9.0

2026-08-21 00:10:57 +02:00

myrmidex commented

2026-08-20 23:39:09 +02:00

Owner

Closes #55.

What this does

Adds tests/Browser/Auth/RegisterTest.php — four Pest browser tests covering /register, following the conventions of the existing tests/Browser/Auth/LoginTest.php. Test-only: no production code, no tests/Pest.php change, no phpunit.xml change.

Test	Covers
`registers successfully`	Fills the form, asserts redirect to `/dashboard` and `assertSee('Test User')` to prove the session is authenticated, not just redirected
`requires all registration fields`	`assertScript` that all four inputs are `.required`; `assertAttribute` that email is `type="email"` and both password inputs are `type="password"`; then asserts the form did not submit
`rejects a mismatched password confirmation`	Stays on `/register`, sees `confirmation does not match`
`rejects a duplicate email`	Pre-creates a planner via `createPlanner()`, then sees `has already been taken`

One design decision worth reviewing

The ticket lists "missing fields" and "invalid email" under validation errors render in the form. They cannot render server-side messages. All four fields in resources/views/auth/register.blade.php carry required, and email is type="email", so the browser blocks submission client-side — the request never reaches $request->validate().

Both cases are therefore asserted as client-side gating (assertScript / assertAttribute plus "the path did not change"), mirroring what LoginTest.php:31-39 already does. The server-side email rule remains covered at feature level in tests/Feature/RegistrationTest.php. This was raised and approved during planning.

Two supporting details, both verified rather than assumed:

Validation message substrings were checked against Laravel's real Translator and the vendored validation.php (this repo has no lang/ override), then asserted as distinctive substrings rather than full sentences.
Rules\Password::defaults() resolves to min(8) — no defaultCallback is registered anywhere. Tests use password123, deliberately off the boundary.

Verification

Gate	Result
style (Pint)	✅ 231 files
types (PHPStan L7)	✅ no errors
test:fast (Unit+Feature)	✅ 160 tests, 1385 assertions, 1 pre-existing skip
test:browser	⚠️ not run locally

The browser tests in this PR have never been executed. The dev container is Alpine/musl and Playwright's browser builds are glibc-linked, so tests/Browser fails there regardless of correctness (documented in .claude/PLATFORM.md). CI on Debian is the first and only place these run — please confirm the Browser tests CI step is green before merging. Correctness was otherwise derived by reading the rendered markup, the controller, and the reference test.

Unrelated flaky test found (not fixed here)

The final gate run hit a failure in tests/Unit/Schedule/ScheduleGeneratorTest.php:138:

test_it_takes_minimum_recurrences_into_account
Recurring dish was not scheduled often enough to verify spacing
Failed asserting that 1 is greater than 1.

It is intermittent and unrelated to this branch — 5 of 6 runs on this identical tree were green, and this commit adds only a browser test that phpunit.xml never executes.

Root cause: schedule generation picks dishes with unseeded ->random() (RegenerateScheduleForDateForUsersAction.php:33, RegenerateScheduleDayForUserAction.php:28,45, DraftScheduleForDateAction.php:22, GenerateScheduleForMonthAction.php:90). The test fixture has two dishes, so whether the recurring one is scheduled more than once is a coin flip.

Not fixed here deliberately — it touches an unrelated subsystem and would break commit atomicity. Note that c3a5532 60 - Make schedule test assertion counts deterministic already addressed this same flakiness class in this same file. Suggest a follow-up ticket.

Closes #55. ## What this does Adds `tests/Browser/Auth/RegisterTest.php` — four Pest browser tests covering `/register`, following the conventions of the existing `tests/Browser/Auth/LoginTest.php`. Test-only: no production code, no `tests/Pest.php` change, no `phpunit.xml` change. | Test | Covers | |---|---| | `registers successfully` | Fills the form, asserts redirect to `/dashboard` **and** `assertSee('Test User')` to prove the session is authenticated, not just redirected | | `requires all registration fields` | `assertScript` that all four inputs are `.required`; `assertAttribute` that email is `type="email"` and both password inputs are `type="password"`; then asserts the form did not submit | | `rejects a mismatched password confirmation` | Stays on `/register`, sees `confirmation does not match` | | `rejects a duplicate email` | Pre-creates a planner via `createPlanner()`, then sees `has already been taken` | ## One design decision worth reviewing The ticket lists "missing fields" and "invalid email" under *validation errors render in the form*. **They cannot render server-side messages.** All four fields in `resources/views/auth/register.blade.php` carry `required`, and email is `type="email"`, so the browser blocks submission client-side — the request never reaches `$request->validate()`. Both cases are therefore asserted as client-side gating (`assertScript` / `assertAttribute` plus "the path did not change"), mirroring what `LoginTest.php:31-39` already does. The server-side `email` rule remains covered at feature level in `tests/Feature/RegistrationTest.php`. This was raised and approved during planning. Two supporting details, both verified rather than assumed: - Validation message substrings were checked against Laravel's real `Translator` and the vendored `validation.php` (this repo has no `lang/` override), then asserted as distinctive substrings rather than full sentences. - `Rules\Password::defaults()` resolves to `min(8)` — no `defaultCallback` is registered anywhere. Tests use `password123`, deliberately off the boundary. ## Verification | Gate | Result | |---|---| | style (Pint) | ✅ 231 files | | types (PHPStan L7) | ✅ no errors | | test:fast (Unit+Feature) | ✅ 160 tests, 1385 assertions, 1 pre-existing skip | | test:browser | ⚠️ **not run locally** | **The browser tests in this PR have never been executed.** The dev container is Alpine/musl and Playwright's browser builds are glibc-linked, so `tests/Browser` fails there regardless of correctness (documented in `.claude/PLATFORM.md`). CI on Debian is the first and only place these run — **please confirm the `Browser tests` CI step is green before merging.** Correctness was otherwise derived by reading the rendered markup, the controller, and the reference test. ## Unrelated flaky test found (not fixed here) The final gate run hit a failure in `tests/Unit/Schedule/ScheduleGeneratorTest.php:138`: ``` test_it_takes_minimum_recurrences_into_account Recurring dish was not scheduled often enough to verify spacing Failed asserting that 1 is greater than 1. ``` It is **intermittent and unrelated to this branch** — 5 of 6 runs on this identical tree were green, and this commit adds only a browser test that `phpunit.xml` never executes. Root cause: schedule generation picks dishes with unseeded `->random()` (`RegenerateScheduleForDateForUsersAction.php:33`, `RegenerateScheduleDayForUserAction.php:28,45`, `DraftScheduleForDateAction.php:22`, `GenerateScheduleForMonthAction.php:90`). The test fixture has two dishes, so whether the recurring one is scheduled more than once is a coin flip. Not fixed here deliberately — it touches an unrelated subsystem and would break commit atomicity. Note that `c3a5532 60 - Make schedule test assertion counts deterministic` already addressed this same flakiness class in this same file. **Suggest a follow-up ticket.**

myrmidex added this to the v0.9.0 milestone 2026-08-20 23:39:09 +02:00

myrmidex added the

testing

label 2026-08-20 23:39:09 +02:00

myrmidex self-assigned this 2026-08-20 23:39:09 +02:00

myrmidex added 1 commit 2026-08-20 23:39:10 +02:00

55 - Add browser test for registration

CI / ci-image (pull_request) Successful in 55s

Details

CI / ci (pull_request) Successful in 2m10s

Details

b048aca93b

myrmidex commented

2026-08-20 23:43:54 +02:00

Author

Owner

Update — CI is green, browser tests have now actually run.

The PR body's caveat that these tests "have never been executed" is no longer true. CI Run #26 succeeded (2m12s) on this branch, which is the Debian CI image running vendor/bin/pest tests/Browser. All four new browser tests executed and passed there.

Two notes for the reviewer:

The dev image fix is not in this branch. 9054c25 55 - Move the dev image to Debian so Playwright can run landed on release/v0.9.0 at 23:32, after this branch was cut from b543f5f. Rebasing was considered and deliberately declined — this PR stays a test-only change. It merges cleanly regardless, since the two commits touch disjoint files.
The dev image still needs a local rebuild to run browser tests on a workstation. Verified against the running container: still Alpine 3.19.1, /opt/playwright missing, PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1 still set. .claude/PLATFORM.md has been annotated with this. It does not affect CI, which has its own Debian image.

The flaky ScheduleGeneratorTest noted in the description is unchanged and still worth a follow-up ticket.

**Update — CI is green, browser tests have now actually run.** The PR body's caveat that these tests "have never been executed" is no longer true. CI **Run #26 succeeded** (2m12s) on this branch, which is the Debian CI image running `vendor/bin/pest tests/Browser`. All four new browser tests executed and passed there. Two notes for the reviewer: - **The dev image fix is not in this branch.** `9054c25 55 - Move the dev image to Debian so Playwright can run` landed on `release/v0.9.0` at 23:32, after this branch was cut from `b543f5f`. Rebasing was considered and deliberately declined — this PR stays a test-only change. It merges cleanly regardless, since the two commits touch disjoint files. - **The dev image still needs a local rebuild** to run browser tests on a workstation. Verified against the running container: still Alpine 3.19.1, `/opt/playwright` missing, `PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1` still set. `.claude/PLATFORM.md` has been annotated with this. It does not affect CI, which has its own Debian image. The flaky `ScheduleGeneratorTest` noted in the description is unchanged and still worth a follow-up ticket.

myrmidex added 1 commit 2026-08-20 23:44:35 +02:00

Merge branch 'release/v0.9.0' into 55-add-browser-test-for-registration

CI / ci-image (pull_request) Successful in 1m1s

Details

CI / ci (pull_request) Successful in 9m53s

Details

7b14aa240c

myrmidex changed title from ~~WIP: 55 - Add browser test for registration~~ to 55 - Add browser test for registration

2026-08-20 23:44:40 +02:00

myrmidex merged commit a84296ae29 into release/v0.9.0

2026-08-21 00:10:57 +02:00

myrmidex referenced this pull request from a commit