js-temporal / temporal-polyfill

Polyfill for Temporal (under construction)
ISC License
529 stars 28 forks source link

CI Tests for Node 18 and 19 #203

Closed justingrant closed 1 year ago

justingrant commented 1 year ago

Makes CI tests pass in Node 18 and Node 19. It was a little more complicated than https://github.com/tc39/proposal-temporal/pull/2448 because TS code and demitasse tests required some changes.

justingrant commented 1 year ago

All Demitasse tests now pass. Node 19 made many changes in the output of Intl.DateTimeFormat. It took a bit of time to write node-version-agnostic code for affected tests.

But I'm stumped with Test262, which is failing in Node 19 but when I look at the test output I don't see any failures, only successes and expected failures. @ptomato , you're a Test262 expert. Any ideas about what may be going on?

justingrant commented 1 year ago

But I'm stumped with Test262, which is failing in Node 19 but when I look at the test output I don't see any failures, only successes and expected failures. @ptomato , you're a Test262 expert. Any ideas about what may be going on?

Mystery solved by copying https://github.com/tc39/proposal-temporal/blob/main/polyfill/runtest262.mjs into this repo. Now I can see the failures, which seem to be related to the Intl format changes that caused the Node 19 Demitasse failures.

justingrant commented 1 year ago

PR filed in Test262 for the failures in Node 19: https://github.com/tc39/test262/pull/3750

I'm not sure what's going on with the test262-es5 and test262-opt runs. I assume these are rollup config issues?

justingrant commented 1 year ago

This PR should be ready to go, but unfortunately because Test262 changes were needed, it's blocked on catching up with the last few months of normative changes from proposal-temporal, so that we can pass the last few months of Test262 changes. When we've caught up with the normative changes then I can rebase this PR and it should be good to merge.

Caveat: there may be a problem with our ES5 and "opt" runs of Test262 with the optimized test runner from proposal-temporal. I may create a separate PR for that runner change so we can try to solve it in parallel.

I'll change to a draft PR in the meantime.

justingrant commented 1 year ago

@12wrigja All tests now pass because I just added a new expected-failures file to include the 897 (!!!) Test262 tests that fail because of commits that we have yet to migrate over from proposal-temporal. Seems like we have two options:

  1. Merge this PR as-is, and as we migrate commits from proposal-temporal, then remove lines from the new expected-failures file.
  2. Wait until proposal-temporal commits are migrated, then merge this PR (minus the new expected-failures file).

Which would you prefer? Personally I like (1) because it's one less rebase to worry about in the future, but I'm flexible either way.

12wrigja commented 1 year ago

Let's proceed with 1 - I don't want this work to bit-rot, and my understanding is the 262 runner will flag tests that are supposed to fail but pass and we can remove the relevant lines then.