fix(concierge): make three date tests independent of the runner timezone - #485
Merged
Conversation
`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 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
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 <noreply@anthropic.com>
|
Deployment failed with the following error: Learn More: https://vercel.com/placeos?upgradeToPro=build-rate-limit |
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
test (concierge)is red on every pull request and has been since these tests were added. It doesn't show ondevelopbecausebuild.ymldoesn't run unit tests — onlypull-request.ymldoes — so the failure only ever appears on someone else's branch. I hit it on an unrelated e2e PR and initially assumed I'd broken something.It's not flaky in the usual sense. It's deterministic: it depends on where the machine is.
The three cases
Each is the same mistake — the fixture states an instant in one timezone while the code under test renders or compares it in another.
parking-mapDate#setHours(machine-local)+10:00parking-bookings-listtoZonedTime(…, 'Australia/Perth')site-attendance-reportformat(machine-local)For
parking-bookings-listthe failure is worth spelling out: on a UTC runner the booking's end lands on the following Perth day,isSameDaygoes false, and the booking stops reading as all-day. The assertion is about the Perth day, so the fixture now states Perth instants.Verification
Full concierge suite (247 files / 1301 tests) green under UTC, Australia/Sydney, America/New_York, Pacific/Honolulu, and Asia/Kolkata — the last one to cover a half-hour offset.
One thing worth a second opinion (not changed here)
While tracing this I noticed
parking-bookings-listis internally inconsistent about timezone, and I've deliberately left it alone rather than change behaviour:isParkingAllDayBookinguses date-fnstoZonedTime, which understands IANA names, so it evaluates inAustralia/Perth.datepipe passing that IANA name as the timezone argument. Angular'sDatePipeexpects a UTC offset (+0800), not an IANA name, so it falls back to machine-local.So whether a booking is labelled "All Day" is decided in the building's timezone, while the start/end times shown next to it are drawn in the viewer's. That's a real product question rather than a test bug, so I haven't touched it — flagging it for whoever owns parking. Happy to raise a ticket if it's worth one.
🤖 Generated with Claude Code