Skip to content

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

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
Closed
Lxxyx wants to merge 1 commit into from
Closed

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

Lxxyx wants to merge 1 commit into from

Conversation

@Lxxyx
Copy link
Member

@LxxyxLxxyx 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()

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

use toLocaleString()

newDate().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-actionsgithub-actionsbot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels May 20, 2021
@LxxyxLxxyx 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/ 💚

@jasnelljasnell 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

github-actionsbot commented May 21, 2021

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

@jasnell
Copy link
Member

jasnell commented May 24, 2021

Landed in 80ed0a6

@jasnelljasnell 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 <[email protected]> Reviewed-By: Richard Lau <[email protected]>
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 <[email protected]> Reviewed-By: Richard Lau <[email protected]>
@danielleadamsdanielleadams mentioned this pull request May 31, 2021
Merged
@richardlau
Copy link
Member

richardlau commented Jul 16, 2021

Depends on #38642.

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

Reviewers

@jasnelljasnelljasnell approved these changes

@richardlaurichardlaurichardlau approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.fast-trackPRs that do not need to wait for 48 hours to land.testIssues 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