Repository navigation
test(e2e): move the workplace specs to the new default views - #569
Conversation
PR #561 changed the components that /landing, /book/desk, /book/meeting, /book/visitor and /your-bookings open, and added /team-schedule. The local specs drive the replaced views, so these routes had no UI coverage that passes. Add 19 tests in six spec files, each with its own page object: - desk-flow-new: book a desk; Confirm with no desk is refused - meeting-flow-new: book a room through the three steps; each step refuses to go forward without its requirement - visitor-flow-new: invite a visitor; no address is refused; a recent visitor fills the fields - landing-new: next-booking card, edit and delete from the card, favourite desk, desk week, add colleague - schedule-views: day, week and list views; day navigation; cancel from the day view; type filter - team-schedule: colleague row with their desk booking; favourites view Section 1c of E2E_USER_STORIES.md lists the rows, the gaps and the findings. It also records that 102 of the 140 tests that were there before fail against the new views.
|
Deployment failed for project frontend-templates with the following error: Learn More: https://vercel.com/placeos?upgradeToPro=build-rate-limit |
PR #561 changed the views that /landing, /book/desk, /book/meeting, /book/visitor and /your-bookings open. The local specs drove the replaced views, and 102 of 140 tests failed. The specs keep their stories and their backend assertions. The page objects now drive the new views, and the page objects of the replaced views are deleted. No app code changes. - Your Bookings: open the list view; change the day through the day view; match the type filters on their icons - Desk: one page, no picker modal, no confirm overlay - Room: three steps on one route, no confirm screen - Visitor: one page; single or multiple is a toggle of the page - Home: the next-booking card shows one booking, so each spec clears the bookings its user has today; quick book is auto-assign desk - Parking: new submit control; the favourite key is favourite_parking Four stories are test.fixme because the new views block them: VIS-08, the international half of VIS-20, ROOM-28 and NEW-DESK-03. Six rows that were blocked on the replaced flows now pass. A full local run of 161 tests has 155 passes, 5 fixme and 1 failure. The failure is VIS-28, the detector for REG-09. It passes alone and its file is not changed. Section 1c of E2E_USER_STORIES.md has the detail and the findings.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
MrYuion
changed the title
MrYuion
left a comment
MrYuion
left a comment
There was a problem hiding this comment.
I reviewed the full diff against develop at 61bab77. I read each changed spec against its develop version and each page object against the Angular template it drives. bun run e2e:typecheck passes and oxlint reports warnings only. I did not run the Playwright suite. The findings below come from the source and from the log of the CI run on this head.
The move itself holds up. No test was removed. The only new test.fixme are the four the PR declares. The locators match the new templates. The PR removes 22 waitForTimeout calls and adds 2.
Real issues, most severe first
clearTodayracesvisitor-delegateacross workers. It deletes a booking of today that another worker's test made for this worker's user. (e2e/support/home/home.api.ts)- The HOME-B1 guard passes before the card loads. The clear state is the default state of the card. (
home-upcoming.spec.ts) - CI on this head has 5 failures, not 1. Four are in the
mockproject. One of them,apps/workplace/e2e/landing.spec.ts:104, still drivesapp-landing, the view that #561 replaced. (comment onE2E_USER_STORIES.md) - Coverage lost. No spec now has a booking that starts later today on the home card. (
home-upcoming.spec.ts) - NEW-ROOM-02, the room guard, has no positive signal. Its two assertions are true before the click. (
meeting-flow-new.spec.ts) - Two of the declared helpers hide their defect with no trace in the report.
addAssetRequest(NEW-F5) andopenDetails(NEW-F3). Both are bounded and both still fail on a full loss. See the two comments.
The three helpers the PR asked about
openDetailsis bounded at 30 s and cannot pass when the modal never opens. The second click is silent.addAssetRequestis bounded at 3 tries and a lost request still fails ROOM-24. The retry covers any cause, and onlyconsole.warnrecords it.- The 422 signature is one line in
visitor.api.ts. Six other places already had it. The retries that use it are bounded.
Optional
Comments marked Optional cover the parking-edit loop, the default length in desk-times, ensureShown, expectRefused, three duplicated helpers, NEW-DESK-01 as a copy of WP-E2E-05, the slot table, and two doc lines.
Not verified
- I did not run any spec, so I did not repeat the red-checks the document claims.
- CON-CAT-03 (concierge) was read against the page object and the templates, and they agree. It still has no run behind it.
- VIS-28 is as described. It uses the API only, its file and the helpers it calls are unchanged, and it failed 3 of 3 in CI with phantom writes.
Review written by Claude Opus 5.5 in the Claude Code harness.
| * This is safe in a parallel run: a worker runs one test at a time, and no | ||
| * spec makes a booking of today for the user of a different worker. | ||
| */ | ||
| export async function clearToday(api: APIRequestContext): Promise<void> { |
There was a problem hiding this comment.
Real issue. clearToday can delete a booking that another worker's test needs.
The comment above says no spec makes a booking of today for the user of a different worker. visitor-delegate.spec.ts does. Both of its tests invite a visitor with the host staffEmail((mine + 1) % WORKERS) and no date. The invite is today, and the next worker's user owns it (the spec asserts stored.user_email is the host).
Failing state. Worker A is inside a visitor-delegate test. Worker A+1 starts one of the seven tests that call clearToday. clearToday lists visitor bookings as the host and deletes A's invite. A then fails on its card or listing checks.
The reverse also fails. A's invite is a booking of today for A+1's user. After NEW-HOME-03 deletes its own booking, the card of A+1 can show A's invite, and the clear state never shows.
Fix. Give the two visitor-delegate invites a future date. inviteVisitorViaUI has the option. Or make clearToday skip rows where booked_by_email is not the caller. Then correct this comment.
Also, .catch(() => []) on the listing turns a failed sweep into no sweep. Let it throw.
There was a problem hiding this comment.
Fixed in 4173342. Both visitor-delegate invites are now three days on, and both schedule views go to that day. I checked the other specs that book for the user of another worker: all of them book a later day already. The clearToday comment now states the rule, and the listing throws when it fails.
| // loaded and has no booking to show. | ||
| await expect( | ||
| home.upcomingCard(booking_id), | ||
| home.clearStateDeskLink, |
There was a problem hiding this comment.
Real issue. This guard passes before the card has loaded.
The comment says the clear state only shows when the card has loaded. That is not so. The clear state is the @else of @if (nextEvent(); as event) in landing-upcoming-booking.component.ts, and _upcoming_events starts as [] (landing-state.service.ts:66). After the reload the card shows the clear state until the bookings arrive. Both assertions can pass before the request returns.
So if the app lists cancelled bookings again (remove !i.deleted at landing-state.service.ts:168), this test can stay green. That is the HOME-B1 fault it is here to catch.
Fix. Use the control from home-scoping.spec.ts. Create a second live booking that starts a minute later, cancel the first, reload, and expect upcomingTitle to have the title of the second.
Related. LandingNewPage.upcomingTitle says the h2 is not present in the clear state. The clear state has an h2 too (UPCOMING_CLEAR_TITLE).
There was a problem hiding this comment.
Fixed in 4173342. The test holds a second live booking that starts a minute later, on the scoping desk of the worker, and expects its title on the card after the cancel. The upcomingTitle comment is corrected.
One correction to the mutation. Removing !i.deleted alone does not bring cancelled bookings back, because resolveStatus gives a deleted booking the status cancelled and the status filter drops it too. I ran it: the test passes with that line removed, and fails with both filters removed. The test comment records this.
| "Blocked by the new views" below. | ||
| - **Six rows that were blocked on the replaced flows now pass**: ROOM-11, ROOM-13, ROOM-14, | ||
| ROOM-21, ROOM-22 and VIS-15. | ||
| - **After the move, a full local run of 161 tests has 155 passes, 5 `fixme` and 1 failure.** |
There was a problem hiding this comment.
Real issue. The CI run on this head has 5 failures, not 1.
Run 38386909981 on 61bab77 reports 209 passed, 5 skipped and 5 failed. One is VIS-28, as this line says. The other four are in the mock project, and the PR does not mention them.
apps/workplace/e2e/landing.spec.ts:104waits forapp-landing. PR feat: promote new custom workplace UIs to be the default #561 replaced that view withlanding-new, and the other tests in the same file already uselanding-new. The test then looks forbutton[name="landing-book-room"]and the room booking modal, which this document says are gone from the page. This is the breakage this PR fixes for the local specs. Move the test or delete it here.discovery.spec.ts:74and:84time out ondeals-n-offers.error-states.spec.ts:11findsapp-unauthorisedbut it stays hidden. This PR does not change these files. I found no earlier CI run that includes the mock project, so I cannot say when they started to fail. Please record them in the PR.
There was a problem hiding this comment.
Fixed in 4173342. The test at landing.spec.ts:104 is deleted. app-landing has no route, and the room rows and the booking modal are gone from the page. HOME-10 holds the free status of a room on the new page.
The other three are recorded in §1c and in the PR description, and are not fixed here. They fail in the same way on a local run. /#/deals-n-offers?mock=true shows the home page, and I did not find the cause. app-unauthorised has one child that is absolute inset-0, so the host has no box. That component was last changed in July.
| start: slot.start, | ||
| end: slot.end, | ||
| title, | ||
| ...inProgressSlot(), |
There was a problem hiding this comment.
Coverage lost. No spec has a booking that has not started yet.
On develop this test was "a booking later today is listed on the panel" and used laterToday(). Every home and landing spec now uses inProgressSlot(). The card has its own branch for a booking that starts later (minutesUntilStart() >= 0, the "starts in" text), and no spec renders it. landing-new.spec.ts and schedule-views.spec.ts do not cover it, and the PR text does not list it.
Fix. Add one test with a start later today and assert the title. It needs the stillToday skip that the old test had.
There was a problem hiding this comment.
Fixed in 4173342. There is a new test for a booking that starts in 30 minutes, with the stillToday skip. It asserts the title and the timelapse marker that only the "starts in" branch shows.
| timeout: 20_000, | ||
| }); | ||
| await flow.continue.click(); | ||
| await expect(flow.roomSelect, 'no room: the flow stays on the room list').toBeVisible(); |
There was a problem hiding this comment.
Real issue, weak assertion. The room guard has no positive signal.
Both assertions are true at the moment of the click. roomSelect is visible and options has count 0 before continue() runs. If continue() lost its has_space() check, the test fails only when the route change wins a race with these two lines.
The app shows an error notice when it refuses. continue() in meeting-flow-space-select.component.ts calls notifyError. Assert it, as the title guard does. Wait for the first snack bar to go, click, then expect mat-snack-bar-container.error. The notice type is a class on the container (notifications.ts, panelClass: [type]).
There was a problem hiding this comment.
Fixed in 4173342. Both refusals assert mat-snack-bar-container.error, and the first notice must go before the second click. Red-checked: with the has_space() check removed from continue() the test fails on the missing notice.
| await flow.title.fill(title); | ||
| // All Day replaces the default slot, which is the next boundary of | ||
| // the clock and makes the booking depend on how fast the run is. | ||
| await flow.setAllDay(true); |
There was a problem hiding this comment.
Optional.
setAllDay(true) works here only because events.allow_all_day is true in the workplace defaults (environments/settings.ts:71). The page object says the checkbox needs the setting. Spread ALLOW_ALL_DAY, as room-allday does, so the spec does not depend on a default. The notes in room.settings.ts and asset.seed.ts that call has_assets off by default are out of date for the same reason.
This is also the one room spec that books today, and for the whole day. room.env.ts keeps room bookings three days out. If this test dies before its finally, the room stays held today. home-meeting-with selects the same room at the default time and does not sweep it first. On worker 0 that room is E2E Room 0, which CON-CAT-03 also selects.
There was a problem hiding this comment.
Fixed in 4173342. The spec spreads ALLOW_ALL_DAY, and the two stale notes are corrected. NEW-ROOM-01 now books FOURTH_DAY, a day no other room spec uses, so a leftover cannot hold the room today.
| url.includes('/api/staff/v1/bookings') && method === 'POST'; | ||
|
|
||
| test.describe('desk booking — the new flow', () => { | ||
| test('NEW-DESK-01: a non-admin books a desk from the list and the backend stores it', async ({ |
There was a problem hiding this comment.
Optional. NEW-DESK-01 repeats WP-E2E-05.
desk-booking.spec.ts books the same desk through the same flow and makes the same API checks. This test adds two assertions on the success view. Move those two into WP-E2E-05 and drop this test. That saves one full UI booking in each run.
There was a problem hiding this comment.
Fixed in 4173342. The two success view assertions are in WP-E2E-05 and NEW-DESK-01 is gone. The ids of NEW-DESK-02 and NEW-DESK-03 are kept.
| */ | ||
| concurrency: { base: 40, count: 8 }, | ||
| /** visitor-flow-new.spec.ts — clear of the concurrency block above. 52 is `details`. */ | ||
| new_flow: { stores: 50, recent: 51 }, |
There was a problem hiding this comment.
Optional.
Slot 50 is not clear of the concurrency block. VIS-28b uses base + BURST + index, which is 48, 49 and 50 (visitor-concurrency.spec.ts:268). It does no harm at run time, because a worker runs one test at a time and both tests sweep first. Use 53 and 54, or record 48 to 50 in the concurrency comment.
There was a problem hiding this comment.
Fixed in 4173342. The new flow uses 53 and 54, and the concurrency comment records 48 to 50.
| export function isReg09(status: number, body: string): boolean { | ||
| if (status === 400 && /Expected BeginObject but was EOF/.test(body)) return true; | ||
| if (status === 500 && /could not serialize access/.test(body)) return true; | ||
| if (status === 422 && /"failures"\s*:\s*\[\s*\]/.test(body)) return true; |
There was a problem hiding this comment.
Optional.
This is the seventh copy of the same REG-09 check. The others are in desk.api.ts, home.api.ts, parking.api.ts, bookings.api.ts, concierge.api.ts and room.flows.ts. One export in e2e/support/api.ts makes the later removal one deletion.
A note on the comment above. e2e/README.md says a desk held by a leftover booking also answers 422 with an empty failures list. So the empty list does not prove REG-09 for every booking type. The retries are bounded, so a real refusal still fails, only later.
There was a problem hiding this comment.
Fixed in 4173342. isReg09 is one export in e2e/support/api.ts and the seven copies are gone. Its comment says that the empty failures list is not proof on its own, with the leftover booking case.
| | ROOM-20 | P1 | A room booked with the default settings is stored **unapproved** (`tentative`), and still holds the room. | **done** — `local/room-approval.spec.ts`. Also the control for ROOM-21. | | ||
| | ROOM-21 | P2 | With `app.bookings.no_approval`, the booking is stored **approved**. | **blocked** — `local/room-approval.spec.ts`, `test.fixme`. ROOM-B1 below, **re-measured 2026-09-16**: the booking POST is still a `500`. | | ||
| | ROOM-22 | P1 | **Catering** ordered on the form reaches the backend as its own `catering-order` booking, linked to the meeting. | **blocked** — `local/room-catering.spec.ts`, `test.fixme`. ROOM-B5 below. The menu seeding works and the form offers catering; it is the order that cannot be saved in this mode. | | ||
| | ROOM-21 | P2 | With `app.bookings.no_approval`, the booking is stored **approved**. | **done** — `local/room-approval.spec.ts`. With `app.bookings.no_approval` the booking is stored, and it is stored pending. ROOM-B1 (the `500`) does not occur, because the booking now has zones. | |
There was a problem hiding this comment.
Optional.
The story says the booking is stored approved. The status says done and stored pending, and pending is what room-approval.spec.ts asserts. Reword the story to match.
There was a problem hiding this comment.
Fixed in 4173342. The story now says the booking is stored pending, not approved.
Real issues from the review of #569: - visitor-delegate invited today for the user of the next worker, and clearToday of that worker could delete the invite. The invites are now three days on. clearToday throws when a listing fails. - The HOME-B1 guard passed on the clear state, which the card also shows before its bookings load. It now holds a second live booking as its control. - A home spec again has a booking that has not started. - NEW-ROOM-02 asserts the error notice of each refusal. - addAssetRequest and openDetails put each retry in the report as a test annotation (NEW-F5, NEW-F3). - The mock test that drove app-landing is deleted. The three other mock failures of CI are recorded in E2E_USER_STORIES.md. Optional findings: one isReg09 in e2e/support/api.ts for seven copies, ScheduleViewsPage extends SchedulePage, one openDetails with two hooks, NEW-DESK-01 merged into WP-E2E-05, NEW-ROOM-01 on a day of its own, the parking-edit loop compares with the wanted time, desk-times picks 90 minutes, ensureShown fails on a missing filter, and the refusal helpers match the error notice only.
- ROOM-25b picked its day in Sydney time and listed the local day of the machine. On the UTC runner the two are different dates from 13:00 to 24:00 UTC. It now lists the Sydney day of the booking. - mock US-071 and US-072: the default features do not include deals-n-offers, and the page goes home when its feature is off. The spec turns the feature on. - mock US-090: the host of app-unauthorised has no box. The spec asserts on the element inside. - mock US-041 and US-033/036 counted an element before the page rendered it, then passed with no check, and failed when the element was there. They now wait for the element and assert what the page does. - VIS-28 and VIS-28b are test.fixme again. Phantom writes still occur on staff-api placeos-2.2609.6: 3 of 10 runs alone, and booking 2519 answered 201 and reads back as 404. The REG-09 row has the measurement. - NEW-ROOM-01 books day 8. ROOM-25b uses day 6 or 7.
home-meeting-with failed 2 of 3 tries in CI run 38490657250 with an empty attendee list. The shortcut of the new home page writes the colleague to the meeting form on a 300 ms timer, and a form page that loads shortly after the timer drops it. #571 has the fix.
1 participant
Problem
PR #561 changed the components that
/landing,/book/desk,/book/meeting,/book/visitorand/your-bookingsopen, and added/team-schedule. The local e2e specs drove the replaced views. In a full local run, 102 of the 140 existing tests failed.Change
No app code changes.
favourite_parking.Result
A full local run of 161 tests on four workers: 154 pass, 7
fixme, none fails. Themockproject has 57 passes and no failure, three times in a row.fixmeare new here: VIS-28 and VIS-28b, the detector for REG-09. See the next section.REG-09 is not fixed on the image that the suite runs
The detector failed with a phantom write in each full run, so I measured it. On staff-api
placeos-2.2609.6, the version that has the pg-orm change,POST /bookingsstill answers201for a row that is never committed.201, andGET /bookings/2519as the admin answered404afterwards. The other member of its group was stored.VIS-28 and VIS-28b are
test.fixmeagain, as the header of the spec prescribes while the image is unfixed. The retries in the helpers stay. PPT-2642 needs to be opened again.Tests that failed for a reason of their own
a48a422fixes these. PR #561 broke the first. The others were wrong before it.mocklanding.spec.ts, "controlled free status…"app-landing, which has no route now.mockUS-071, US-072deals-n-offers, and the page goes home when its feature is off.mockUS-090app-unauthorisedhas no box, so Playwright reports it as hidden.mockUS-041, US-033/036Stories that the new views block
Each is
test.fixmewith the measurement in its comment.visitors.reason_required.featureslist throws indesk-flow-select.component.ts, and then no desk shows as selected.Section 1c of
E2E_USER_STORIES.mdhas the rows and thirteen more findings.Notes for review
NEW-F3).NEW-F5).parking-editandhome-meeting-withset a value again when the form drops it while it loads (REG-10, finding NEW-F12).422signature is retried as REG-09. The check is now oneisReg09ine2e/support/api.ts.home-meeting-withcan be flaky until fix(workplace): keep the colleague or room when a new home shortcut opens the meeting form #571 is merged. It failed 2 of 3 tries in CI run 38490657250 with an empty attendee list. The cause is the app: the "meeting with" shortcut of the new home page writes the colleague on a 300 ms timer, and a form page that loads shortly after the timer drops it. This is finding NEW-F13, and fix(workplace): keep the colleague or room when a new home shortcut opens the meeting form #571 has the fix.bun run e2e:typecheckpasses andoxlintreports warnings only.Made by Claude Opus 5.5 in the Claude Code harness.
🤖 Generated with Claude Code