Run CI tests in Node 19 / ICU 72 / CLDR 42

[justingrant]

Support CI tests in Node 19, and specifically supports running with ICU 72 which contains CLDR 42 which broke a lot of tests.

This PR is a counterpart to js-temporal/temporal-polyfill#203, but this PR was somewhat easier because there's no TS-specific issues to contend with like there are in the other repo.

• Update GH actions to latest versions
• Use Node19 on all other actions and tests
• Run Test262 against Node 19 ,18, 16, 14 Removed older Node version testing at @ptomato's request; instead, he'll work with Test262 maintainers to avoid downlevel-incompatible tests from getting into Test262.
• Extend expected-failures.txt handling so that some tests can be expected to fail only on some Node versions, but pass on later versions.
• Upgrades @js-temporal/temporal-test262-runner to include some Temporal-related 402 tests that we weren't running before. (And which failed in Node 19 / CLDR 42)
• Updates Test262 so those newly-included tests will pass
• Updates the codecov script to run those previously-missed tests

[codecov]

Codecov Report

Merging #2448 (6f468c4) into main (4664de5) will increase coverage by 0.08%.
The diff coverage is n/a.

❗ Current head 6f468c4 differs from pull request most recent head 7b78da5. Consider uploading reports for the commit 7b78da5 to get more accurate results

@@            Coverage Diff             @@
##             main    #2448      +/-   ##
==========================================
+ Coverage   94.72%   94.81%   +0.08%     
==========================================
  Files          20       20              
  Lines       10852    10841      -11     
  Branches     1970     1971       +1     
==========================================
- Hits        10280    10279       -1     
+ Misses        506      502       -4     
+ Partials       66       60       -6     

Impacted Files	Coverage Δ
polyfill/lib/ecmascript.mjs	98.29% <0.00%> (-0.01%)	⬇️
polyfill/lib/intl.mjs	99.80% <0.00%> (+1.97%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[justingrant]

Not sure why "test-polyfill" and "test-test262" are showing as "Waiting for status to be reported" when I renamed those jobs in test.yml. I think it's a GH glitch and can be ignored.

In any case, this PR is all green and ready for review.

[ptomato]

I'd prefer not to reintroduce multi-versioned Node tests into this repo. I removed it because it takes effort to keep it maintained, and I don't think there was any benefit to be gotten from it at this point. If there are discrepancies it's overwhelmingly likely to be due to ICU data.

[justingrant]

I'd prefer not to reintroduce multi-versioned Node tests into this repo. I removed it because it takes effort to keep it maintained, and I don't think there was any benefit to be gotten from it at this point. If there are discrepancies it's overwhelmingly likely to be due to ICU data.

@ptomato The benefit I had in mind was ensuring that polyfills (which will rely on the same Test262 tests that this repo does) aren't accidentally broken by Temporal contributors who check in new or changed Test262 tests.

The Temporal proposal doesn't require testing against older Node versions, but production polyfills do need to test against older versions. And it'd be a bummer if polyfills are blocked from updating to the latest Test262 because someone inadvertently checked in a new test that breaks in Node 18/16/14. Especially since (as you note above) those breaks are pretty much guaranteed to be unrelated to Temporal code.

If we run Test262 against older Node versions in this repo's CI, then it should prevent Node-version-specific tests from ever making it into Test262 in the first place, which seems like a better outcome than requiring polyfills to send PRs into Test262 in order to update their own Test262 submodules.

What do you think?

[ptomato]

I'd prefer not to reintroduce multi-versioned Node tests into this repo. I removed it because it takes effort to keep it maintained, and I don't think there was any benefit to be gotten from it at this point. If there are discrepancies it's overwhelmingly likely to be due to ICU data.

@ptomato The benefit I had in mind was ensuring that polyfills (which will rely on the same Test262 tests that this repo does) aren't accidentally broken by Temporal contributors who check in new or changed Test262 tests.

The Temporal proposal doesn't require testing against older Node versions, but production polyfills do need to test against older versions. And it'd be a bummer if polyfills are blocked from updating to the latest Test262 because someone inadvertently checked in a new test that breaks in Node 18/16/14. Especially since (as you note above) those breaks are pretty much guaranteed to be unrelated to Temporal code.

If we run Test262 against older Node versions in this repo's CI, then it should prevent Node-version-specific tests from ever making it into Test262 in the first place, which seems like a better outcome than requiring polyfills to send PRs into Test262 in order to update their own Test262 submodules.

I'd prefer to make that the responsibility of those production polyfills. They don't have to be blocked from updating Test262, because they can put failing tests onto an expected-failures list. As browsers finish their Temporal implementations they might contribute more tests directly to test262, so it's not guaranteed that all new tests would be vetted by this repo's CI anyway.

But mostly I'm concerned about the amount of time that I spent in the past diagnosing failures on older Node versions. Going forward, I won't have time available for that kind of task.

[justingrant]

@ptomato - I removed older node version testing, and rebased this PR. Here's what's remaining:

• Updates GH actions for Node 19
• Upgrades @js-temporal/temporal-test262-runner to include some Temporal-related 402 tests that we weren't running before. (And which failed in Node 19 / CLDR 42)
• Updates Test262 so those newly-added tests will pass
• Updates the codecov script to run those previously-missed tests

Want to review?

[ptomato]

One open question about avoiding a rename if possible, otherwise I think this is good to merge. Thanks!

[ptomato]

It occurred to me that we should probably run all the tests in test262/test/intl402/DateTimeFormat since the polyfill patches DateTimeFormat. (There's also a test262/test/intl402/Intl/DateTimeFormat folder that should be merged with the former. I'll open a test262 issue for that.)

I think that's out of scope for this pull request. Especially since I tried it and there are some failures. Just noting it for the future.

[justingrant]

Yeah, I had the same thought right after I sent the Test262 PR to fix the four tests that broke in test262/test/intl402/**/*[tT]emporal*.js, but it was late and I didn't want to chew off fixing all the others. I'll send a Test262 PR to fix those tests too, assuming it's easy.

[justingrant]

Also, I noticed that test262/test/built-ins/Date/*/toTemporalInstant/*.js doesn't seem to exist. Did it move? Or are we anticipating a time later when we will populate this folder?

[ptomato]

The test262 issue I opened is tc39/test262#3763

There just aren't any tests for Date.prototype.toTemporalInstant yet. You're correct that the script anticipates them.

[justingrant]

It occurred to me that we should probably run all the tests in test262/test/intl402/DateTimeFormat since the polyfill patches DateTimeFormat. {snip}
I think that's out of scope for this pull request. Especially since I tried it and there are some failures. Just noting it for the future.

Yep. There were 37 failures. I filed #2471 to track the fixes for them. Definitely out of scope for this PR to fix all those!

On CLDR 41 and earlier, there were 5 more failures caused by CLDR 42 compatibility. I filed tc39/test262#3765 to fix those. I tested the fixes both locally and in CI against the Temporal polyfill to make sure that the CLDR 42 issues weren't masking legit polyfill issues, but all 5 passed. After that's merged, there shouldn't be any more CLDR42-related compatibility issues remaining in Test262.

But I don't think we need to block merging this PR on those 5 Test262 fixes being merged because those tests aren't actually testing Temporal objects... and because Intl.DTF tests don't pass anyways.