Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specify UTC time zone for the test suite #9945

Merged
merged 1 commit into from Dec 6, 2021

Conversation

@joshkel
Copy link
Contributor

@joshkel joshkel commented Dec 3, 2021

The controller.bar/not-grouped/on-time test was failing on my computer because the date ranges happen to cross the end of Daylight Saving Time in the U.S., so chart's X axis was generated with one more hour of time than the test fixture expected.

Using moment-timezone to specify a fixed time zone with no DST seemed like the most robust fix. (Alternatively, I could pick a date range that doesn't change DST; that ought to work.)

package.json Outdated Show resolved Hide resolved
@kurkle kurkle added this to the Version 3.7.0 milestone Dec 5, 2021
@etimberg
Copy link
Member

@etimberg etimberg commented Dec 5, 2021

Needs a rebase for the package-lock changes in the v3.6.2 PR

The controller.bar/not-grouped/on-time test was failing on my computer because the date ranges happen to cross the end of Daylight Saving Time in the U.S., so chart was generated with one more hour of time than the test fixture expected.

Using moment-timezone to specify a fixed time zone with no DST seemed like the most robust fix. (Alternatively, I could pick a date range that doesn't change DST; that ought to work.)
@joshkel joshkel dismissed stale reviews from kurkle and etimberg via 13bb8b8 Dec 5, 2021
@joshkel joshkel force-pushed the test-time-zones branch from cc54e3b to 13bb8b8 Dec 5, 2021
@joshkel
Copy link
Contributor Author

@joshkel joshkel commented Dec 5, 2021

Rebased. Thanks.

kurkle
kurkle approved these changes Dec 5, 2021
@etimberg etimberg merged commit 957ca83 into chartjs:master Dec 6, 2021
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants