Repository navigation
test(e2e): add Workplace App settings coverage (215 settings, 445 scenarios) - #595
Conversation
…rios) Add permanent Playwright specs under apps/workplace/e2e/settings for 36 Workplace App settings (parking booking, parking requests, lockers, explore legend, directory and app shell), one setting per spec, 71 scenarios. Each scenario applies the setting through the existing browser override (PLACEOS.setting_overrides) and asserts its effect in two or more states. A shared read-only fixture blocks every non-GET API request, verifies the staff-only identity, intercepts WebSockets and attaches a per-test safety audit. Backend lookups that the app would otherwise answer with a create are served by GET-only browser fixtures. The local project now also discovers settings/**; the mock project ignores it. Workers and CI configuration are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
a1e0dfc to
4074e93
Compare
MrYuion
left a comment
There was a problem hiding this comment.
Review of the hand-written specs, the read-only fixture and the Playwright config. There are no generated files or data fixtures. All 16k lines are spec code.
Summary
bun run e2e:typecheckpasses. I did not run the specs, because they need the local stack.- The safety guard is correct. Each mutation is blocked, and I found no request to a non-local system. The config change is correct.
- Some tests cannot fail, and some tests lock in app behaviour that is different from
docs/settings/workplace.md. See the inline comments. - About 73% of the non-blank spec lines (11.6k of 15.9k) are lines that occur in 10 or more specs. The harness is copied into each file, with 9 different variants.
noUnusedLocalsfinds about 190 unused declarations, for exampledayEnabled,timezoneBuildings,scheduleCardandpickDay. - The PR description is out of date. It says 36 settings and 71 scenarios, and that landing is not included. The branch now has 54 specs, and these include 6 landing specs and desk, room and schedule specs. Please update the description.
Review by Claude Opus 5.5 in Claude Code.
|
Deployment failed for project frontend-templates with the following error: Learn More: https://vercel.com/placeos?upgradeToPro=build-rate-limit |
MrYuion
left a comment
There was a problem hiding this comment.
Follow-up review of the 7 commits after 6709a90 (422a433..13d4492). These commits add 63 specs (desk, room, bookings, visitor invite and general) and remove some draft headers.
Checks
bun run e2e:typecheckpasses.oxlinthas 0 errors. The new files have 295no-unused-varsand 108no-conditional-expectwarnings. I did not run the specs, because no local stack was available.- Safety guard: OK. All 63 new specs use
readOnlyTestandassertLocalOnly. The page routes that callroute.fetch()orroute.continue()accept only GET requests to the local origins. Other requests fall back to the context guard. The extra contexts ingeneral.bannerinstall the same guard before they open a page. Each test gets a new browser context, so setting overrides do not leak between tests. - Vercel failure on 4cdbd60: this PR did not cause it. The error is
api-deployments-free-per-day, the daily deployment quota for the team account. The build did not start. The Ignored Build Step skipped all the other commits, and the head commit is green. No change is necessary.
Earlier findings
- Copied harness: not fixed, and now larger. The
getOnly/prepareharness is copied into 90 specs (54 before).noUnusedLocalsfinds 237 unused declarations in the new files alone. - Draft headers and names: partly fixed. The "CLAUDE OWNED DRAFT" and "DRAFT / NOT EXECUTED" lines are gone from the old specs. But 39 specs still have "Migrated unchanged from …" headers,
WPA next20/next25 DRAFTnames orDRAFT_SETTING_KEYS. These include the new bookings and visitor-invite specs. New test names useremaining draft:andprepared …:.general.features.spec.ts:1says "NOT RUN — awaiting manual validation". Please say which of the new specs ran against a local stack. general.default-routecannot fail: not fixed.general.css-variablescannot fail: not fixed.parking.allow_all_daylocks in the||getter: not fixed. The same problem is now in a desk spec (see the new findings).lockers.use_building_timezonehas nolockers: false, bookings: truecase: not fixed.lockers.disabled_date_selectasserts a side effect: not fixed.lockers.allow_time_changesdoes not check the time fields: not fixed.desk-booking.hide-checkindoes not setapp.desks.auto_checkin: not fixed.parking-booking.bookable-hoursdayCellfails on some dates: not fixed. The new specs usepickCalendarDay, which moves to the correct month, so they do not have this problem.
- The PR title and description still say 36 settings and 71 scenarios. The branch now has 116 settings specs. Please update them.
New findings (inline)
- Bug:
desk-booking.available-periodclicks the month button that goes back, so it fails in the last week of each month. - Bug:
room-booking.allowed-future-daysfails near the end of each month. - Improvement: the desk duration specs and
bookings.duration-boundshave identical cases, and some cases use the default value, so they cannot fail. - Improvement: room and desk
bookable_hoursaccept a start time at closing time. No duration is possible at that time. - Improvement:
desk-booking.allow-time-changeslocks in behaviour that is different from the docs, and itstruecase is the same as inallow-all-day. - Improvement:
bookings.allow-all-daylocks indesks || bookings, but the docs say thatdesks.*overrides. - Improvement:
general.basic-user-searchrepeats a landing scenario. - Improvement:
general.use-geolocationdoes not test "nearest". - Nits: an assertion that is always true, a selector, a docs mismatch, a missing banner case and a rare clock-dependent failure.
Reviewed by Claude Opus 5.5 in Claude Code.
events.no_standalone: 1 scenario, 1 PASS hide_landing_customise: 2 scenarios, 2 PASS hide_landing_widgets: 2 scenarios, 2 PASS explore.colors: 2 scenarios, 2 PASS Total: 4 unique settings, 7 scenarios, 7 locally verified PASS. desk-free remains an unresolved product defect.
…arios) Verified locally: 19/19 PASS across 10 unique Settings. virtual-concierge: 5 Settings, 9 scenarios PASS parking-request.form-details: 3 Settings, 6 scenarios PASS parking-request.location-preference: 2 Settings, 4 scenarios PASS
Fresh local Playwright execution: 30/30 PASS. Coverage by area: - Desk booking height: 1 setting, 2 scenarios - Explore map overlays: 4 settings, 8 scenarios - General default building: 1 setting, 2 scenarios - General help menu: 1 setting, 2 scenarios - Landing feature groups: 1 setting, 2 scenarios - General dark CSS variables: 1 setting, 2 scenarios - General support ticket: 2 settings, 3 scenarios - Lockers available period: 1 setting, 2 scenarios - Visitor bookable hours: 1 setting, 1 scenario - Visitor duration options: 4 settings, 4 scenarios - Visitor building timezone: 1 setting, 2 scenarios TOTAL: 18 UNIQUE SETTINGS | 30 SCENARIOS | 30 PASS Execution: local project, 1 worker, 0 retries. No product or shared framework changes.
MrYuion
left a comment
There was a problem hiding this comment.
Follow-up review of the 6 commits after 13d4492 (d36d8a7..0659d70). These commits add 20 specs. They do not change files from the earlier reviews.
Checks
bun run e2e:typecheckpasses.oxlinton the new files: 0 errors, 9no-conditional-expectwarnings (all in the copied audit fixtures). I did not run the specs, because they need the local stack (mockignoressettings/**).- Safety guard: OK. All 20 new specs use
readOnlyTestandassertLocalOnly. The oneroute.fetch()(visitor-invite.building-timezone) accepts only GET to the local origins.
Earlier findings
All earlier points are still open. The new commits do not touch those files.
- Copied harness: not fixed, and larger. A copied audit fixture is now in 132 of 136 specs.
- Draft headers: not fixed. The new headers add run logs that will go out of date ("PASS, as reported by the user", "2026-10-22 15:26 run", "Phase 2 migration", "Scenario owners: S45, S46").
room-booking.booking-unavailable.spec.ts:4refers toreports/wpa-settings-reference/…json, which is not in the repo.
3–10.general.default-route,general.css-variables,parking.allow_all_day,lockers.use_building_timezone,lockers.disabled_date_select,lockers.allow_time_changes,desk-booking.hide-checkin,parking-booking.bookable-hoursdayCell: not fixed. The copieddayCellis also in the newlockers.available-period. - Second-review findings (desk available-period and room allowed-future-days month-end bugs, duplicate duration cases, closing-hour start,
||getters, and the nits): not fixed.
- The PR title and description still say 36 settings and 71 scenarios, and "one setting per spec". The branch now has 136 settings specs, and several new specs cover 2–5 settings. Please update them.
New findings (inline)
- Bug (app): selecting None after Daily triggers the daily all-day recurrence error. No case covers it.
- Cannot fail:
lockers.available_period 10,default_buildingA. - Documented effect not tested:
hide_landing_customise(saved layout),hide_landing_widgets("cannot add"),feature_groups(member case),parking.hidden_buildings(active building fallback),virtual_concierge.side(position),css_variables_dark(variable name, light theme). - Depends on stack data: visitor duration fallbacks and building timezone.
parking.can_book_for_anyonefalse does not test the documented override.- Nits: help icon shape, wrong docs line numbers.
Claude Opus 5.5 in Claude Code (T3 Code) wrote this review.
The counts were from before the review fixes: 430 scenarios with 13 not yet run, headings that disagreed with --list in seven areas, and rows naming explore.show-legend and room-booking.allow-multiple-spaces-legacy, which the fixes merged into explore.legend and room-booking.multiple-spaces. Every heading and row now matches --list (407 settings and 36 settings-browser tests in 187 files), lockers.use_building_timezone is listed as tested now that it has a case, and the statuses come from a full run of both projects on 30 Oct. The run instructions cover both projects, their --list counts, running one spec, re-running a load timeout alone and E2E_WEBSERVER_TIMEOUT_MS. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VtrxAzu8XnqXeQG3QbRXt
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VtrxAzu8XnqXeQG3QbRXt
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VtrxAzu8XnqXeQG3QbRXt
The apps animate a full-page gradient on html and body until 3 s after the loading screen hides. Headless Chromium rasterises each frame in software: about 5 CPU-s per page load (6.3 s -> 1.0 s when paused). On the CPU-bound self-hosted runner that is most of the e2e run time. pauseBackground() pauses only that animation, which is how the page looks once the app has settled. The context fixtures and every manual browser.newContext() in the Workplace specs call it.
…ting Three settings specs acted on the page before Angular had rendered the state they depended on. A faster page made them fail more often: - dayCell read a cell of the previous month: the calendar component moves to a new grid before the page renders it. It now waits until the rendered day numbers match displayed_dates(). - The locker drawer looked open for a moment after the modal opened, so the spec skipped the toggle. At 600 px it starts closed, so the spec now always opens it. - The colleague search was filled before the modal's first change detection, whose ngOnInit then cleared it. The specs now wait for ng-pristine, which that pass sets.
With the animated page background paused, the browsers no longer use all of the runner's CPU, so two more workers can run. Locally six workers keep the suite isolated (same results as four) and cut the wall time from 12.2 to 7.7 minutes.
MOCK_EVENTS is built when the module loads, which the dev build does on every page load, mock mode or not. Its duplicate removal searched the whole list for each event. A set gives the same events in the same order and cuts about 250 ms of script time from each load.
…specs The app can still have requests in flight when a test ends. A body read or a route.fetch() then fails with "Target page, context or browser has been closed", which failed tests on their first attempt on the busy CI runner. teardown.ts tells those errors apart: the settings audits drop the ones recorded after their teardown starts, the read-only fixture skips identity bodies that were cut off, and the handlers that flaked are wrapped in ignoreClosed. Reviewed with sol-loop: 1 round with gpt-6.1-sol. Sol verdict: approved
Run 41193893174 ran both Workplace test steps in 19.1 minutes on six workers (local+mock 7.4, settings 11.7), against 23.3 minutes on four. The docs said four workers used all the CPU and six were no faster; that was true before the specs paused the animated page background.
Diagnostic-only change affecting 8 existing unique Settings and 16 existing scenarios across 6 Explore specs. Adds 0 unique Settings and 0 scenarios. Verified e2e:typecheck and all 16 affected browser-only scenarios with retries disabled. Preserves protocol validation, binding state and strict teardown.
|
Hi Cam, I've pushed the approved control WebSocket diagnostic improvement to PR #595. Commit: Investigation findings:
What changed:
Verification completed in an isolated checkout:
Remaining limitation: The change has been pushed to PR #595. CI verification for the new commit is still pending. Could you please review the change? Thanks! |
… 36 existing scenarios; 0 new Settings, 0 new scenarios)
Brings in #654 (PPT-2872), #655 (PPT-2871) and the desk asset changes from #656, which the settings specs are updated for next. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VtrxAzu8XnqXeQG3QbRXt
…asset fixes - desks and events bookable_hours: the last start now leaves room for the shortest duration (PPT-2872, #654). The closing-time case asserts no start is offered at closing time, and the specs pin min_duration so the expected last start doesn't depend on the stack's settings. - parking use_building_timezone: drop test.fail now that an explicit parking false overrides bookings (PPT-2871, #655). - desks.use_assets: the assets fixture also answers the building-wide `zones=<building>` query the desk list sends since #656. - QA status doc updated to match. Full settings and settings-browser run with develop merged in: 443 passed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VtrxAzu8XnqXeQG3QbRXt
|
Hi Sharmila, I've made the changes from my review on this branch, so there's nothing you need to do for them. Pushed (fast-forward on top of your HMR commit
I also updated the title to 215 settings and 443 scenarios, the numbers in your QA status doc. Local run with develop merged in, 4 workers, no retries: CI trial: all green.
The flaky tests all passed on retry. In The browser-only job first failed before running anything: GitHub's |
ubuntu-latest is now Ubuntu 26.04, and the pinned Playwright fails to
install Chromium there ("Playwright does not support chromium on
ubuntu26.04-x64"), so the job failed before running a test in trial
41559638200.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015VtrxAzu8XnqXeQG3QbRXt
…cs by suffix - browserOnlyTest drops closed-target errors from route handlers during teardown and bounds the pending-task drain to 10 s. - protectNetwork ignores closed-target errors on socket close. - Socket, console and redaction checks use the shared harness helpers. - BROWSER_APP reads APP_URL. - readOnlyTest loads the staff storage state file directly, so each test opens one context instead of two. - Browser-only settings specs end in .browser.spec.ts; the config matches the suffix instead of a hand-kept list. - e2e-advisory job timeout covers the settings step's own 60 minutes.
Format only. The parsed syntax trees are the same before and after.
Coverage: 1 unique Setting (app.show_presence_indicators), 2 existing scenarios, 0 new scenarios. Verified on 2db5cef with --retries=0: this spec 20/20 (--repeat-each=10), settings-browser 36/36. No product, harness or config changes.
…ookings (PPT-2871) Coverage: 0 new unique Settings, 2 new scenarios (desks false over bookings true; lockers false over bookings true). Totals: 187 specs, 215 Settings, 445 scenarios. Updates the stale bookings || lockers comment in the locker spec. Verified on 2db5cef against the local e2e stack: 35/35 over 5 repeats, 0 retries.
Update: 3 commits pushed today (3 Nov)All three change test code or the Playwright config only -- no shared harness changes. Each was verified locally with 1.
|
…scenarios) Coverage: 0 new unique Settings, 2 new scenarios (desk-booking 60 → 61, lockers 23 → 24). Totals: 187 specs, 215 Settings, 445 scenarios; settings project 407 → 409. Adds the 3 Nov evidence for 2e9a3f4 (35/35, 5 repeats, 0 retries). No code changes.
|
Status on 3 Nov (evening):
|
Workplace Settings E2E Automation — Coverage & QA Status
Summary
This PR introduces Playwright E2E automation for Workplace application Settings, validating configuration-driven behaviour across desk, room, parking, visitor, locker, Explore and other supported workflows.
The tests verify enabled/disabled configurations, UI visibility, booking restrictions, permissions, recurrence and other Settings-dependent behaviour.
The automation uses the existing E2E framework without intentional changes to product source code or shared framework implementation.
1. Final Coverage Summary
QA Status: All 445 scenarios are recorded as passing locally in the final QA audit (
docs/qa/workplace-settings-final-status.md): 443 in the full run on 3 Nov, and the 2 PPT-2871 override cases added in2e9a3f467passed 35/35 (5 repeats, 0 retries).These figures represent the committed PR scope and exclude held local tests and developer-approved exclusions. GitHub CI status is tracked separately.
2. Area-wise Coverage Breakdown
3. Test Coverage
Automated scenarios cover:
Tests verify both enabled and disabled configurations where applicable, including expected UI behaviour and control scenarios.
4. Latest Update
Commit:
2f6fd46b5Changes included:
Updated
bookings.allow-all-day.spec.tsto use a future booking date.Added final Excel coverage tracker:
docs/qa/workplace-settings-e2e-coverage-30-10-2026.xlsxAdded detailed Markdown QA status report:
docs/qa/workplace-settings-final-status.mdThis commit updates existing coverage and adds QA documentation. It does not introduce additional unique Settings or scenarios into the reported totals.
Review fixes
Commits:
1906bbd14b,2db5cef6a5browserOnlyTest,protectNetwork). ThebrowserOnlyTestteardown wait stops after 10 s.BROWSER_APPreadsAPP_URL.readOnlyTestloads the staff storage state file directly, so each test opens one browser context instead of two.*.browser.spec.ts. Thesettings-browserproject matches that suffix instead of a hand-kept list.e2e-advisoryjob timeout is now 225 minutes, so it covers thesettingsstep's own 60 minutes.After these fixes:
e2e:typecheckpasses,--listis unchanged (407 + 36 = 443) and--project=settings-browserpassed 36/36 locally. Thesettingsproject was not re-run (it needs the local stack).5. Known Product Issues and Follow-up Work
Some existing tests pass against current branch behaviour but require updates once corresponding product fixes from
developbecome available.The detailed QA report records these follow-up requirements.
6. Held Tests — Not Included in PR
The following work remains local and is not included in the committed PR coverage:
parking.hide_selected_spacedevelop; held locallyexplore.hide_device_fieldsThese tests are tracked separately and are not counted as additional passing PR scenarios.
7. Developer-Approved Exclusions
The following Settings are excluded from the in-scope coverage:
new_featuresbookings.allowed_daily_visitor_countspaces.show_imagesdesks.hide_reasonlockers.use_building_timezoneExclusion decisions and supporting rationale are recorded in the final QA audit.
8. QA Reference Documents
The following documents are committed to this PR for team review:
Excel Coverage Tracker
docs/qa/workplace-settings-e2e-coverage-30-10-2026.xlsxDetailed QA Status Report
docs/qa/workplace-settings-final-status.mdThese documents contain the Settings inventory, scenario coverage, verification results, exclusions, Jira references and outstanding work.
9. Review Notes
Final QA Status
187 Playwright specs | 215 unique Settings | 445 scenarios
445/445 scenarios reported as locally passed (443 in the full run on 3 Nov + the 2 new PPT-2871 cases, 35/35).
Latest changes and QA reference documents have been committed and pushed to
test/e2e-workplace-settings.Ready for team review, subject to normal CI and code-review checks.
Review fixes (
1906bbd14b,2db5cef6a5) made by Claude Opus 5.5 in Claude Code (T3 Code).Reviewed with sol-loop: 1 round with gpt-6.1-sol on xhigh. Sol verdict: approved
🤖 Generated with Claude Code
3 Nov updates
Commits:
345aeb728,2e9a3f467,64ed1007a,3480ebc74explore.show-presence-indicators.browser.spec.tsnow uses the shared browser-only harness (browserOnlyTest,controlSocket,browserSignIn,syntheticOrgGets): 629 → 316 lines, with both scenarios and their assertions unchanged. Verified: 20/20 (--repeat-each=10) andsettings-browser36/36, 0 retries.desks.use_building_timezone: falseandlockers.use_building_timezone: falsenow each have a case showing they overridebookings.use_building_timezone: true. The stalebookings || lockerscomment in the locker spec is corrected. Verified: 35/35 over 5 repeats, 0 retries, against the local e2e stack.Coverage after these commits: 187 specs, 215 unique Settings, 445 scenarios.
--list: 409 + 36 = 445.