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.

Sign up for GitHub

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

Jump to bottom

test: set locale for datetime-change-notify test #38741

Closed
wants to merge 1 commit into from
Closed

test: set locale for datetime-change-notify test #38741

wants to merge 1 commit into from

Conversation

Lxxyx
Copy link
Member

@Lxxyx Lxxyx commented May 20, 2021 •
edited
Loading

The time zone output by toString() will change with user's language, use toLocaleString() to set the time zone.

This test will fail on a computer with the language set to Chinese.

Example:

use toString()

new Date().toString() 
// output: Thu May 20 2021 05:18:38 GMT+0000 (协调世界时)

use toLocaleString()

new Date().toLocaleString('en-US', { timeZoneName: 'long' }); 
// output: date 5/20/2021, 5:24:25 AM Coordinated Universal Time

@Lxxyx
test: set locale for datetime-change-notify test
c70bcb7 
The time zone output by toString() will change with user's language, use toLocaleString() to set the time zone.
@github-actions github-actions bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels May 20, 2021
@Lxxyx Lxxyx added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed needs-ci PRs that need a full CI run. labels May 21, 2021
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented May 21, 2021 •
edited by jasnell
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/38240/ 💚

@jasnell jasnell added the fast-track PRs that do not need to wait for 48 hours to land. label May 21, 2021
@github-actions
Copy link
Contributor

Fast-track has been requested by @jasnell. Please 👍 to approve.

@jasnell
Copy link
Member

jasnell commented May 24, 2021

Landed in 80ed0a6

@jasnell jasnell closed this May 24, 2021
jasnell pushed a commit that referenced this pull request May 24, 2021
@Lxxyx @jasnell
test: set locale for datetime-change-notify test
80ed0a6 
The time zone output by toString() will change with user's
language, use toLocaleString() to set the time zone.

PR-URL: #38741
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
danielleadams pushed a commit that referenced this pull request May 31, 2021
@Lxxyx @danielleadams
test: set locale for datetime-change-notify test
c7a5857 
The time zone output by toString() will change with user's
language, use toLocaleString() to set the time zone.

PR-URL: #38741
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
@danielleadams danielleadams mentioned this pull request May 31, 2021
Merged
@richardlau
Copy link
Member

Depends on #38642.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@richardlau richardlau richardlau approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Lxxyx @nodejs-github-bot @jasnell @richardlau