[bpatrik]

The test phase of the critical user journeys epic #1254 (sub-issues #1305–#1316), done test-first. The fixes of #1344 will come in this same PR. It is a draft until then.

Behaviour that doesn't work yet has tests that are marked as failing, so CI stays green:

• Playwright: test.fail()
• mocha: the test asserts that the check is still rejected, with a // TODO #13xx comment
• Vitest: it.fails()

#1344 lists every one of them. The fix phase removes each marker as its fix lands.

Tests, one or more commits per sub-issue

• CUJ tests: error handling for API errors and failed requests #1305 API errors:
  • HTTP status per ErrorCodes value: 401 auth, 403 rights, 404 missing directory or file (PATH_ERROR), 400 for errors of user input (incl. SETTINGS_ERROR, ALBUM_ERROR, USER_CREATION_ERROR, ALREADY_AUTHENTICATED), 500 for the rest
  • JSON 404 for unknown API paths
  • folder or search load that gets a 500 or loses the connection, and a Retry button
  • "Page not found"
  • a red test for a video range request past the end of the file (answers 500, not 416)
• CUJ tests: login and session #1306 Login and session:
  • wrong password
  • remember-me cookie lifetime
  • deep links that come back after login, also after the session ended (session.spec.ts)
  • session that ends while the app is open (redirect green, message red)
  • a red test for logout being undone by a request that was still in flight (the cause of the flaky logout e2e test)
  • login page sanity test
• CUJ tests: sharing #1307 Sharing:
  • wrong password
  • expired, deleted and missing sharing links; a sharing that expires while open (sharing-lifecycle.spec.ts)
  • deleting a sharing ends the guest's session
  • editing a sharing; who may create, list, update and delete sharings (sharing-rights.spec.ts, SharingPermissions.spec.ts)
  • a shared folder renamed or removed on disk
  • sharing login page sanity test
• CUJ tests: browsing and navigation #1308 Browsing:
  • missing, hidden and empty folders (fallback green, toast red)
  • every sorting and grouping option, accent aware
  • rendering more of the grid on scroll
  • the flatten query, flattening an empty folder
  • browser back/forward and the scroll position; the Markdown blog
  • photos added, deleted and renamed on disk show up (ChangedOnDisk.spec.ts)
• CUJ tests: search #1309 Search:
  • search deep link with ?p=
  • malformed search links
  • search query parser round trips, autocomplete (also e2e: type, pick a suggestion, get results)
  • accent-insensitive search
  • "Too many results", a failed place lookup
• CUJ tests: lightbox #1310 Lightbox:
  • paging, zoom and pan limits, face boxes and captions, fullscreen
  • video playback (play, pause, loop key), video error, the video source
  • video range requests (backend)
  • loop icon on the last photo
  • tapping a face on a phone
  • browser back closes the lightbox; opening a photo with the keyboard
  • info panel with accented and CJK metadata; the map (map.spec.ts, tiles blocked) and its markers
• CUJ tests: downloads and zip #1311 Downloads:
  • original files served byte for byte
  • zip contents with non-ASCII names
  • a file missing from disk
  • zip download from the navbar (e2e): a folder with accents, a search, and a guest in a sharing; no zip button for an empty folder
  • the zip link with more than one query param
• CUJ tests: albums and faces #1312 Albums and faces:
  • the album journey through the UI (save a search as an album, open it, delete it)
  • delete confirmation
  • stale person link
  • "no albums yet" and "no faces yet" states
  • an album whose search matches nothing (BE and e2e); the .saved_searches.pg2conf albums
• CUJ tests: random link and smart photo frame #1313 Random link and photo frame: the URL builders, then a frame that advances, survives a connection loss, and matches nothing; a frame whose sharing ends; a random link that matches nothing (random-link.spec.ts).
• CUJ tests: upload #1314 Upload:
  • name clash
  • minimum role
  • Upload.maxFileSizeMB
  • per-file errors in the uploader
• CUJ tests: admin #1315 Admin:
  • admin page sanity test, and a normal user opening /admin
  • duplicates page sanity test, its empty and loading states
  • settings persist after save and reload; invalid values are rejected; a job schedule persists
  • indexing progress
  • JobManager (run, stop, a failing item, schedules), the photo and video converting jobs
  • users, role changes, protecting the admin's own account; managing users through the admin page (users.spec.ts)
  • admin notifications, duplicates, extension installer
• CUJ tests: test infrastructure and cross-cutting checks #1316 Mobile:
  • a lightbox journey on a phone: pinch, double tap, swipe, close
  • every "should look right" sanity test also runs on a phone, with a check that nothing scrolls horizontally
• No sub-issue:
  • CUJ 4 Fresh install: FreshInstall.spec.ts: an empty DB, the default admin logs in, the folder is listed and searchable; an older DataStructureVersion rebuilds the DB and keeps the users and their passwords.
  • Gallery without login (NoAuthentication.spec.ts): Admin is the intended default visitor role; lowering it to User in the settings closes the admin API.
  • Server.urlBase behind a proxy that strips it (UrlBase.spec.ts).

Test infrastructure

• playwright.config.ts has 4 projects:
  • chromium
  • mobile: Pixel 7, runs lightbox.spec.ts and navigation.spec.ts
  • mobile-sanity: runs the tests tagged @sanity on the same phone
  • admin-config: runs after the other three, for tests that change the config or run jobs. CI runs it on its own (separate report) when one of the other projects failed, since Playwright skips it then.
• E2E_PORT runs the e2e server on another port (e.g. in parallel worktrees); start-e2e-server gets its port from PORT.
• New helpers in test/playwright/helpers.ts:
  • pinch(), doubleTap()
  • desktopViewport(), clickNavbarTab()
  • expectNoHorizontalScroll()
  • expectNavbar() also handles the phone menu
  • sharing and user helpers for the API (createDirectorySharingWithApi, createUserWithApi, ...)
• AGENTS.md: the e2e section describes the new projects, helpers and covered journeys; architecture notes on urlBase and the default user right without login.

No production code changed yet.

Not covered by e2e, on purpose

• Upload through the UI: it would write into demo/images, the public demo.
• Theme and language switch: manual.
• WebKit/Safari: the touch helpers use CDP (Chromium only).
• No write permission on disk: the container runs as root.

Test runs (test phase)

• Backend (SQLite): 886 passing. MySQL was not run locally (no MariaDB available); the fix of the MySQL failure in FreshInstall.spec.ts waits for the background index save.
• Frontend: 435 passed, 16 expected failures.
• Full e2e, all 4 projects: 142 passed, 15 skipped (docs screenshots, and the phone-only tests in the desktop projects). Every expected failure fails on its intended check.

Part of #1254 and #1344.

---

Generated by Claude Code

[bpatrik]

Review of the CUJ tests (epic #1254, fix list #1344)

Verdict: the test phase is solid and mostly does what the issues asked, but it is not mergeable as is. CI is red on MySQL because of a race in a new test, and a number of agreed behaviours were dropped without being tracked anywhere.

What was run (head 2bcd972, Node 22 locally, so not the CI Node version):

• Backend, SQLite only: 822 passing.
• Frontend: 411 passed, 10 expected failures.
• E2E, all 4 projects: 104 passed, 14 skipped, matching the PR description.
• Every changed file, all 12 sub-issues and their closing comments were read.

Do the "red" tests really fail?

Yes, but they are wrapped as expected failures (test.fail(), it.fails(), inverted mocha asserts), so CI shows them green.

The real error of all 28 expected-failure e2e results was pulled from the JSON report: every one fails on its intended assertion, none on a broken selector or setup. The one exception in spirit is the logout race test (defect 2).

App bugs the tests surfaced:

• A video range request past the end answers 500 instead of 416.
• Logout can be undone by an in-flight request.
• A folder load that gets a 500 or loses the connection shows no error, while a search does.
• Flatten also matches sibling folders with the same name prefix.
• Group by name is not accent aware.
• The search parser collapses double spaces.
• SQLite text search is accent sensitive.
• On a phone, tapping an invisible face box opens the person search.
• The Faces page scrolls horizontally on a phone.
• A normal user opening /admin lands on /login.
• An admin can lower their own role when another admin exists.
• publicInfo ignores sharing expiry.
• Every API error goes out as HTTP 200.

What is covered

Area	Covered by this PR
Errors (#1305)	Status per ErrorCodes, JSON 404 for unknown API paths, folder/search 500 and connection loss, Retry, "Page not found"
Login (#1306)	Wrong password, remember-me cookie (2 levels), deep links after login, deleted user, logout race, login sanity
Sharing (#1307)	Wrong password, expired (API + e2e), deleted while open, deleted from the list, guest session ends, sharing-login sanity
Browsing (#1308)	Missing / hidden / empty folder, all sort and group options, grid renders on scroll, flatten query
Search (#1309)	Autocomplete (FE + e2e), ?p= deep link, malformed links, parser round trips, accents
Lightbox (#1310)	Paging and its limits, loop wrap, zoom/pan bounds, faces and captions, fullscreen, video play/pause/loop/error, range requests
Downloads (#1311)	Originals byte for byte, zip contents with non-ASCII names, missing file, zip e2e for folder, search and guest
Albums, faces (#1312)	Album journey through the UI, delete confirmation, stale person link, empty states (FE), no-match album (BE)
Frame, random (#1313)	Both URL builders, frame advances, survives connection loss, retries a no-match search
Upload (#1314)	Name clash, minimum role, size limit, per-file errors (BE + FE)
Admin (#1315)	Admin and duplicates sanity, settings persist, invalid save, indexing progress, JobManager, user/role/session rules
Mobile (#1316)	Pinch, double tap, swipe, plus every sanity test on a Pixel 7 with a horizontal-scroll check
Fresh install (CUJ 4)	Empty DB to usable gallery, DB rebuild keeps users (but see defect 1)

In the issues, but not covered

Agreed or proposed in the sub-issues, no test, and mostly not in #1344 either. They survive only in closing comments of closed issues, so they will get lost.

• CUJ tests: login and session #1306: the return URL after a session ends; cookie expiry and a new session secret.
• CUJ tests: sharing #1307: a sharing that expires while the guest has it open (countdown reaching zero); a shared folder renamed or removed on disk.
• CUJ tests: browsing and navigation #1308: scroll position after back/forward (navigation.spec.ts checks only URL, breadcrumb and content); flatten with no media showing the empty state.
• CUJ tests: search #1309: "Too many results" in the UI; a failed location lookup, which has no test at any level.
• CUJ tests: downloads and zip #1311: zip button hidden for an empty folder; the zip size-limit question is still open; the "file not available anymore" toast is in Fix the behaviour the critical user journey tests found (#1254) #1344 but has no test.
• CUJ tests: albums and faces #1312: "no faces yet" empty state; the UI of an album that matches nothing.
• CUJ tests: random link and smart photo frame #1313: frame session ending (expired share); the indicator after repeated failures; a random link that matches nothing.
• CUJ tests: upload #1314: no write permission on disk (stated as untestable in the container).
• CUJ tests: admin #1315: "no duplicates" at component level; role change disabled on the admin's own row (only delete is checked); job scheduling, thumbnail and convert jobs; add, edit and delete users through the UI.
• Fix the behaviour the critical user journey tests found (#1254) #1344 items without a test: the downloadZipLink & bug, the zip name, the duplicates summary i18n.
• Epic table overstates: CUJ 15 (converted vs original video source), 17 (non-ASCII info panel) and 38–40 are marked ✅ while their own "gap" column still lists work. CUJ 10 (map) is 🟡 and the epic is closed.

Not covered and not listed anywhere

• Photos changed on disk show up after reindexing (add, remove, rename). Arguably the core journey of a directory-first gallery; backend unit tests only.
• OIDC login: backend and frontend specs exist, but it is not in the CUJ list and has no journey test.
• Running under Server.urlBase or a reverse-proxy subpath: e2e runs only at /.
• Gallery without authentication (authenticationRequired=false) end to end.
• Markdown blog, GPX tracks and the map in e2e. A map check without tiles is possible by blocking the tile requests.
• Editing a sharing, and non-admin sharing rights (who may share, seeing only one's own sharings) in the UI.
• .pg2conf per-folder config and .saved_searches.pg2conf albums: the demo folder has one, e2e ignores it.
• Back button closing the lightbox, and keyboard-only use.
• Safari / WebKit: all touch helpers use CDP, so they are Chromium only.
• Transcoded video playback, Live Photos, admin notifications (e.g. the default-password warning).

Defects in the tests

1. CI is red: FreshInstall.spec.ts fails on MySQL (test (24.x); 26.x was cancelled). The PR description says MySQL was not run. Cause: on a never-scanned folder GalleryManager returns the disk scan and saves to the DB in the background (indexDirectory without waitForSave). The test searches right after, so it races the save. The second test in the file has the same race (count > 0 straight after the GET). The test should wait for the save.

2. The logout race test does not test its own mechanism. In 5 of 5 runs it fails at login.spec.ts:120, before release() is called. The held request is never the cause; what is was not traced (the comment in the neighbouring test suggests thumbnail requests still in flight). The app bug looks real, but "reproduces it every time" depends on timing. It should wait for pending requests to reach 0 before starting the held one.

3. Working behaviour is hidden inside expected-failure tests. Each of these fails only on its last line (the toast), so the parts that work are unprotected until the fix lands:

   • deleted user: the redirect to /login;
   • hidden folder: the fallback to Home and the no-leak check;
   • missing folder: the fallback to Home.

   Split them, as already done for wrong password and /admin.

4. Leftover data: the "Invalid sharing" tests in share.spec.ts create sharings through the API and mostly never delete them, against the isolation rule in AGENTS.md. Harmless in CI, but they pile up on a reused local server.

5. Non-waiting conditionals: albums.spec.ts if (await confirm.isVisible()) is already in Fix the behaviour the critical user journey tests found (#1254) #1344. The same pattern is in the autocomplete test (search.spec.ts, the URL check before pressing Enter) and in createSharing (the menu check).

6. "Deleted from the share list" logs the guest out first and then checks only an API status. It proves the key is rejected, not that an open guest page stops working; the backend test covers the session part.

7. admin-config depends on the other three projects, so any failure there skips it and that signal is lost on a red run.

8. Low-value specs: users.service, duplicates.service and extension-installer.service only assert the URL passed to a mock, and "50 MB by default" pins a config value.

9. Minor:

   • admin.spec.ts requires exactly /gallery/, although master just moved Home to /gallery.
   • The zip name regex _\d+ would mangle names like IMG_5910.jpg if the search changes.
   • errors.spec.ts accepts a loose regex for the invalid-link message, while share.spec.ts requires the exact text.

Is the approach reasonable?

Mostly yes:

• Tests sit at the cheapest level that can catch the bug.
• One sanity test per screen and role, reused on a phone.
• Tests wait on conditions instead of fixed timeouts.
• Tests run in parallel with unique data; config and job changes are isolated in their own project.
• The e2e suite runs in about 3 minutes.

Concerns:

• Expected-failure markers pass for any failure. Today's reasons were verified, but nothing keeps them honest. The CI summary also counts them as "Passed", so there is no visible count of known-red tests.
• Red tests prescribe UI that does not exist yet: exact texts, role="status", a Retry button name, an onRetry toast field, ionRepeatOutline. Treat them as a draft spec; the fix phase will have to edit some.
• Tests and fixes in one PR: about 3,900 lines of tests, then fixes on top, is hard to review. Merging the tests first (after fixing defect 1) would be safer.

Decisions in #1344 worth a second look

• Status map: SETTINGS_ERROR, ALBUM_ERROR, USER_CREATION_ERROR and ALREADY_AUTHENTICATED fall to 500, but they are user errors (4xx). A missing file is PATH_ERROR (mapped to 400), while the download test wants 404. "Missing share or album → 404" has no error code.
• 401 for a wrong password will collide with the planned global "session has ended" handling of 401 unless the login call is excluded. There is no test for that interaction.
• Upload.maxFileSizeMB default of 50 is low for a gallery that accepts videos.
• Process: Define critical user journeys and cover them with tests #1254 and all sub-issues are closed with unchecked boxes. The dropped items above should move into Fix the behaviour the critical user journey tests found (#1254) #1344 or a new issue.

---

Generated by Claude Code

[bpatrik]

Update: review fixes and the missing CUJ tests

25 new commits on this branch, following the review. No production code changed.

Fixes from the review

• MySQL failure in CI: FreshInstall.spec.ts searched before the background save to the DB had finished. It now waits for that save.
• Logout race test: route.fetch() stored the cookie of the held answer in the browser right away, so the test failed before the held request was released. The request now goes out with the cookie from before the logout, and its answer reaches the browser after it. It now fails on the last check, in 5 of 5 runs.
• Working parts split from red tests: the deleted user's redirect to /login, and the fallback to Home for a missing and for a hidden folder (including the no-leak check), are green tests now. Only the missing toasts are marked as failing.
• Other test fixes:
  • no more non-waiting isVisible() checks (albums, autocomplete, createSharing)
  • the "Invalid sharing" tests delete their sharings
  • the share-list deletion test keeps the guest logged in, so it also checks that the session ends
  • the zip name check is safe for names like IMG_5910.jpg
  • /gallery is accepted as the root
  • the invalid-link message is the exact agreed text
  • albums are named with randomUUID()
  • the deep-link login test opens a full-size photo (big_ben.jpg is smaller than the lightbox, so it could never pass)
• CI: admin-config runs on its own, with its own report, when another e2e project failed (Playwright skips it otherwise).
• Status codes (CUJ tests: error handling for API errors and failed requests #1305): errors of user input (SETTINGS_ERROR, ALBUM_ERROR, USER_CREATION_ERROR, ALREADY_AUTHENTICATED) expect 400. PATH_ERROR (e.g. a missing file) expects 404, matching the download test.
• Decisions:
  • urlBase expects a reverse proxy that strips it, so the test of the other setup is dropped.
  • Admin is the intended default visitor role without login; a new test checks it can be lowered to User in the settings.
  • Both are noted in AGENTS.md.
• Infrastructure: E2E_PORT runs the e2e server on another port.

New tests (see the updated description)

• Backend:
  • photos changed on disk show up after reindexing
  • gallery without login; urlBase
  • a failed place lookup; a random link that matches nothing
  • sharing permissions; a shared folder renamed on disk
  • job schedules; the converting jobs; admin notifications
• Browsing:
  • blog; map (tiles blocked)
  • back/forward in the lightbox and in folders; keyboard
  • empty folder, flattening and zip states
  • .saved_searches.pg2conf albums; an album that matches nothing
  • "Too many results"; video source; non-ASCII info panel
• Sessions and sharing:
  • return URL after the session ends
  • a sharing that expires or is deleted while open, also as a frame
  • editing a sharing; sharing rights
  • random link that matches nothing
• Admin:
  • users through the admin page; own role locked
  • duplicates empty and loading states
  • job schedule; duplicates summary i18n

New expected failures (to add to #1344)

Test	Behaviour	Issue
session.spec.ts (3 tests)	After the session ended, logging in again lands on the URL the user was on (photo, folder, search)	#1306
sharing-lifecycle.spec.ts › Countdown should not go below zero	Today it shows -1 days, 23:59:59	#1307
sharing-lifecycle.spec.ts › Sharing that expired while open …	"This link is invalid or has expired" instead of an empty gallery	#1307
sharing-lifecycle.spec.ts › as a photo frame … deleted	Full-screen invalid-link message instead of the generic error page	#1313
random-link.spec.ts › should tell that no photos match	A message instead of raw JSON with HTTP 200	#1313
SearchErrors.spec.ts › geocoding service fails	LocationLookUp_ERROR, not GENERAL_ERROR	#1309
SearchErrors.spec.ts › random link with an HTTP error status	INPUT_ERROR as 400	#1305
SharingPermissions.spec.ts › Admin updates another user's sharing	Today "Sharing not found"	#1307
lightbox.spec.ts › Browser back should scroll the grid to the last shown photo	Like closing the lightbox	#1310
lightbox.spec.ts › A photo of the grid should open with the keyboard	Grid photos can't get the focus	#1254
navigation.spec.ts › Browser back should restore the scroll position	Today back opens the folder at the top	#1308
navigation.spec.ts › Flattening an empty folder shows an icon and a message	Empty state of #1308	#1308
albums.spec.ts › album that matches nothing, opened	The search empty state	#1312
navigator.gallery.component.spec.ts › should keep every query param	Zip link joins params without &	#1311
faces.component.spec.ts › no faces yet	Icon and a short explanation	#1312
map.gallery.component.spec.ts › photo on the prime meridian	Latitude/longitude 0 gets no marker	#1254
map.gallery.component.spec.ts › duration of a short track	Tracks under about 17 minutes show no duration	#1254
duplicates.component.spec.ts › loading state	Today it shows "No duplicates found" while loading	#1315
frame.component.spec.ts › notification badge	The admin's badge doesn't render (plain field, not a signal)	#1315
translation.spec.ts › duplicates summary	Not translatable	#1344

Test runs

• Backend (SQLite): 886 passing. No MariaDB was available locally, so the next CI run is the first MySQL check.
• Frontend: 435 passed, 16 expected failures.
• E2E, all 4 projects: 142 passed, 15 skipped. Every expected failure fails on its intended check.
• Lint: clean.

---

Generated by Claude Code