From 7815a6dfbdb68fa400c35631a14706362d4b994e Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 6 Aug 2026 03:08:02 +1000 Subject: [PATCH 1/3] fix(concierge): make three date tests independent of the runner timezone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `test (concierge)` fails on every pull request, and has since these tests were added. It is invisible on `develop` because `build.yml` does not run unit tests — only `pull-request.yml` does — so the breakage only shows up on other people's branches. The cause is the same in each case: the fixture states an instant in one timezone while the code under test renders or compares it in another. - parking-map: `setAvailabilityHour` picks the hour with `Date#setHours`, which is machine-local, but the test wrote both the input and the expected result as `+10:00`. It therefore only passed on a runner in eastern Australia. Both are now built from local components. - parking-bookings-list: `isParkingAllDayBooking` converts the booking into the display timezone (`Australia/Perth`) before checking that it starts and ends on the same day. The fixture was built from local hours, so on a UTC runner the end landed on the following Perth day and the booking stopped reading as all-day. The instants are now stated in Perth, which is what the assertion is actually about. - site-attendance-report: the export filename is formatted with date-fns `format`, which is machine-local, but the range was given as UTC midnight. Any runner west of Greenwich named the previous day. This one does not affect CI, which runs UTC, but it fails for anyone in the Americas. Verified green under UTC, Australia/Sydney, America/New_York, Pacific/Honolulu and Asia/Kolkata (a half-hour offset). Co-Authored-By: Claude Opus 5 --- .../parking/parking-bookings-list.component.spec.ts | 12 +++++++++--- .../src/tests/parking/parking-map.component.spec.ts | 8 ++++++-- .../site-attendance-report.service.spec.ts | 7 +++++-- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/apps/concierge/src/tests/parking/parking-bookings-list.component.spec.ts b/apps/concierge/src/tests/parking/parking-bookings-list.component.spec.ts index c09a520e06..266285f791 100644 --- a/apps/concierge/src/tests/parking/parking-bookings-list.component.spec.ts +++ b/apps/concierge/src/tests/parking/parking-bookings-list.component.spec.ts @@ -177,15 +177,21 @@ describe('ParkingBookingsListComponent', () => { it('should show all day when the booking matches the bookable period', () => { bookable_hours = { start: 8, end: 17 }; - selected_date = new Date(2026, 6, 21, 8).valueOf(); + // `isParkingAllDayBooking` converts the booking into `timezone` + // (Australia/Perth here) before checking that it starts and ends on the + // same day. Building these from machine-local hours only holds while + // the runner sits near +08:00 — on a UTC runner the end lands on the + // following Perth day and the booking stops reading as all-day. So + // state the instants in Perth, which is what the assertion is about. + selected_date = new Date('2026-07-21T08:00:00+08:00').valueOf(); bookings = [ { id: 'booking-1', asset_id: 'bay-1', status: 'approved', all_day: true, - date: new Date(2026, 6, 21, 8).valueOf(), - date_end: new Date(2026, 6, 21, 17).valueOf(), + date: new Date('2026-07-21T08:00:00+08:00').valueOf(), + date_end: new Date('2026-07-21T17:00:00+08:00').valueOf(), duration: 9 * 60, } as unknown as Booking, ]; diff --git a/apps/concierge/src/tests/parking/parking-map.component.spec.ts b/apps/concierge/src/tests/parking/parking-map.component.spec.ts index eb4bd65e04..1fad875f30 100644 --- a/apps/concierge/src/tests/parking/parking-map.component.spec.ts +++ b/apps/concierge/src/tests/parking/parking-map.component.spec.ts @@ -121,7 +121,11 @@ describe('ParkingMapComponent', () => { it('should select a one-hour parking availability window', () => { const state = spectator.inject(ParkingStateService); - const date = new Date('2026-07-13T09:30:00+10:00').valueOf(); + // `setAvailabilityHour` picks the hour with `Date#setHours`, which is + // the machine's timezone, so both the input and the expectation are + // built the same way. Writing either as a fixed UTC offset would pin + // the test to a runner in that zone. + const date = new Date(2026, 6, 13, 9, 30).valueOf(); Object.defineProperty(spectator.component, 'options', { value: () => ({ date, all_day: true, zones: [] }), configurable: true, @@ -130,7 +134,7 @@ describe('ParkingMapComponent', () => { spectator.component.setAvailabilityHour(14); expect(state.setOptions).toHaveBeenCalledWith({ - date: new Date('2026-07-13T14:00:00+10:00').valueOf(), + date: new Date(2026, 6, 13, 14, 0, 0, 0).valueOf(), all_day: false, duration: 60, }); diff --git a/apps/concierge/src/tests/reports/attendance/site-attendance-report.service.spec.ts b/apps/concierge/src/tests/reports/attendance/site-attendance-report.service.spec.ts index 29d6338dee..fb18072305 100644 --- a/apps/concierge/src/tests/reports/attendance/site-attendance-report.service.spec.ts +++ b/apps/concierge/src/tests/reports/attendance/site-attendance-report.service.spec.ts @@ -697,9 +697,12 @@ describe('SiteAttendanceReportService', () => { }); it('should export report data', () => { + // The filename is built with date-fns `format`, which renders in the + // machine's timezone, so the range is set the same way. A UTC instant + // here names the previous day once the runner is west of Greenwich. spectator.service.setOptions({ - start: new Date('2026-04-06T00:00:00Z').valueOf(), - end: new Date('2026-04-06T23:59:59Z').valueOf(), + start: new Date(2026, 3, 6).valueOf(), + end: new Date(2026, 3, 6, 23, 59, 59).valueOf(), }); (spectator.service as any)._report.set({ business_days: 1, From 080efeef68fafd1d1108381fc54530aba96efa43 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 6 Aug 2026 03:19:49 +1000 Subject: [PATCH 2/3] fix(concierge): tie the staff listing scroll timer to its effect The effect schedules a 30ms timer whose callback reads `document`. On CI that timer sometimes fires after the test environment has been torn down, raising `ReferenceError: document is not defined` and failing the whole concierge run even when every test passes. `AsyncHandler.ngOnDestroy` does clear pending timers, but the effect can flush during teardown and schedule a fresh one afterwards, which nothing then clears. Clearing it from the effect's own cleanup makes the timer's lifetime the effect's lifetime regardless of that ordering. Co-Authored-By: Claude Opus 5 --- apps/concierge/src/app/staff/staff-listing.component.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/apps/concierge/src/app/staff/staff-listing.component.ts b/apps/concierge/src/app/staff/staff-listing.component.ts index 92d9b708ed..b06ce94b36 100644 --- a/apps/concierge/src/app/staff/staff-listing.component.ts +++ b/apps/concierge/src/app/staff/staff-listing.component.ts @@ -141,9 +141,14 @@ export class StaffListingComponent extends AsyncHandler { constructor() { super(); - effect(() => { + effect((onCleanup) => { this.user_list(); this.timeout('scroll', () => this.onScroll({}), 30); + // `onScroll` reads the document, so the timer must not outlive the + // effect. Relying on `ngOnDestroy` alone is not enough: the effect + // can flush during teardown and schedule a fresh timer after the + // base class has already cleared them. + onCleanup(() => this.clearTimeout('scroll')); }); } From a501883bdf7eb91d5a9ac463a1aaa44fa2a01e8b Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 6 Aug 2026 03:31:01 +1000 Subject: [PATCH 3/3] test(concierge): destroy the staff listing fixture after each test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The component schedules a 30ms timer that reads the document. This file finishes well inside that window, and the Angular vitest builder does not destroy fixtures automatically, so the timer outlived the test environment and raised `document is not defined` during teardown. Vitest counts that as an unhandled error and fails the run — with all 1266 tests passing, which is what made it confusing to place. Co-Authored-By: Claude Opus 5 --- .../src/tests/staff/staff-listing.component.spec.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/apps/concierge/src/tests/staff/staff-listing.component.spec.ts b/apps/concierge/src/tests/staff/staff-listing.component.spec.ts index 2a97cc9047..09d05be32d 100644 --- a/apps/concierge/src/tests/staff/staff-listing.component.spec.ts +++ b/apps/concierge/src/tests/staff/staff-listing.component.spec.ts @@ -27,6 +27,12 @@ describe('StaffListingComponent', () => { beforeEach(() => (spectator = createComponent())); + // The component schedules a 30ms timer that reads the document. This file + // finishes well inside that window, so without an explicit destroy the + // timer outlives the test environment and throws `document is not defined` + // during teardown, failing the whole run with every test passing. + afterEach(() => spectator?.fixture?.destroy()); + it('should create component', () => { expect(spectator.component).toBeTruthy(); });