Skip to content

Support running Temporal tests in Node 19 (cont'd for staging tests)#3751

Merged
gibson042 merged 3 commits into
tc39:mainfrom
justingrant:node-19-staging-tests
Dec 18, 2022
Merged

Support running Temporal tests in Node 19 (cont'd for staging tests)#3751
gibson042 merged 3 commits into
tc39:mainfrom
justingrant:node-19-staging-tests

Conversation

@justingrant

Copy link
Copy Markdown
Contributor

Following up from #3750, this PR supports running Temporal "staging" tests (https://github.com/tc39/test262/tree/main/test/staging) under Node 19.

For context, Intl.DateTimeFormat implementations in Node 19 have ICU format changes:

  • replace some spaces with non-breaking-space unicode characters
  • change German date separators (these may be bugs)
  • months in German are shown with a leading zero, but only in Year/Month formats (also may be a bug)

This PR makes affected tests more resilient to Intl.DateTimeFormat output changes between Node versions. They'll continue to work even if the behavior (e.g. in German) turns out to be a bug and is fixed in the platform.

Comment thread test/staging/Intl402/Temporal/old/date-time-format.js Outdated
Comment thread test/staging/Intl402/Temporal/old/date-time-format.js Outdated
@justingrant justingrant force-pushed the node-19-staging-tests branch 7 times, most recently from b84bd27 to f29267b Compare December 16, 2022 12:02
@justingrant justingrant marked this pull request as draft December 16, 2022 12:10
@justingrant justingrant force-pushed the node-19-staging-tests branch 2 times, most recently from fca9c2c to ceb4232 Compare December 16, 2022 13:07
@justingrant justingrant marked this pull request as ready for review December 16, 2022 13:11
@justingrant

Copy link
Copy Markdown
Contributor Author

@gibson042 - I made the changes you recommended, rebased to main, and cleaned up commits by running prettier first and then layering my changes on top so the diff will be easier to review.

Comment thread test/staging/Intl402/Temporal/old/date-time-format.js Outdated
Comment thread test/staging/Intl402/Temporal/old/date-time-format.js Outdated
Comment thread test/staging/Intl402/Temporal/old/instant-toLocaleString.js
Comment thread test/staging/Intl402/Temporal/old/time-toLocaleString.js
@justingrant justingrant force-pushed the node-19-staging-tests branch 4 times, most recently from 31b1bd2 to eeee385 Compare December 16, 2022 21:31
@justingrant justingrant marked this pull request as draft December 16, 2022 21:39
@justingrant justingrant force-pushed the node-19-staging-tests branch 3 times, most recently from 741c062 to 00e1205 Compare December 16, 2022 21:58
@justingrant justingrant marked this pull request as ready for review December 16, 2022 22:41
@justingrant

justingrant commented Dec 16, 2022

Copy link
Copy Markdown
Contributor Author

@gibson042 All code review issues should be resolved, but there's been enough change that it may be worth one more review before merging.

When reviewing, you'll want to look at each commit individually for easier review.

I apologize for all the commit notification spam from this PR. I wasn't able to figure out how to get my local install of Node 19 to use ICU 72 like GH Actions does, so I had to use CI to validate all changes. Grrrrr.

Make locales consistent throughout these tests, and splitting into a
separate commit to make it easier to review substantial changes later.
Adapt to the following changes in CLDR 42 in Node 19:
- Some spaces in formats replaced by other whitespace unicode characters
- German format changes, which may be bugs (see
  https://unicode-org.atlassian.net/browse/CLDR-16243)

This commit makes affected tests more resilient to Intl.DTF output
changes between Node versions.

A good idea for a future PR would be to change these tests to rely less
on CLDR formats while still testing that Temporal and Intl are behaving
as expected.
@gibson042 gibson042 force-pushed the node-19-staging-tests branch from 00e1205 to cb9fff8 Compare December 18, 2022 04:49
@gibson042 gibson042 merged commit 9f6da57 into tc39:main Dec 18, 2022
justingrant added a commit to justingrant/test262 that referenced this pull request Jan 10, 2023
Fixes 4 more Temporal-related tests that break in ICU 72 / CLDR 42.

Follows up on tc39#3676 and
tc39#3751.
ptomato pushed a commit that referenced this pull request Jan 10, 2023
Fixes 4 more Temporal-related tests that break in ICU 72 / CLDR 42.

Follows up on #3676 and
#3751.
justingrant added a commit to justingrant/test262 that referenced this pull request Jan 10, 2023
Following up on tc39#3751 and tc39#3762, this commit makes a few tests
work on both CLDR 42 and CLDR 41. Previously these tests were
tied to a specific CLDR 42 format.
Ms2ger pushed a commit that referenced this pull request Jan 11, 2023
Following up on #3751 and #3762, this commit makes a few tests
work on both CLDR 42 and CLDR 41. Previously these tests were
tied to a specific CLDR 42 format.
ptomato pushed a commit to ptomato/test262 that referenced this pull request Jan 19, 2023
Following up on tc39#3751 and tc39#3762, this commit makes a few tests
work on both CLDR 42 and CLDR 41. Previously these tests were
tied to a specific CLDR 42 format.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants