fullcalendar/temporal

Should probably run Test262 against this polyfill

justingrant opened this issue · 23 comments

justingrant commented

There are almost 4000 Temporal tests in Test262. Some of them were never in the non-Test262 tests of proposal-temporal (the ones you're now running with Jest), so it'd probably be good for you to get additional test coverage from Test262.

Take a look at the npm run test262 script in https://github.com/js-temporal/temporal-polyfill/blob/main/package.json for some copypasta you can use to get Test262 running in this polyfill too. If you run into any trouble, feel free to tag me (@justingrant) or @ptomato or @12wrigja and we can help.

Note that Temporal's Test262 tests are slooooooooooow because the Test262 harness is inefficient. It takes about 5-10 mins to run all 3700+ tests. But if this polyfill passes both Jest and Test262 then that's a really good sign! 😄

arshaw commented

Thanks for letting me know. I will rig up the Tests262 tests and have them run alongside the demitasse ones.

LinusU commented

@arshaw did you make any progress on this? We have native BigInt support on our platform so I would like to take advantage of that, but don't feel comfortable switching over before this

If you want to I might be able to take a stab at this

justingrant commented

There's now over 5000 tests in Test262. It'd be good to get this repo hooked up to Test262.

FYI, there have been some improvements in Test262 performance over in https://github.com/tc39/proposal-temporal/blob/main/polyfill/runtest262.mjs which sped up test running a lot. Test runs can now finish in 1-2 mins in CI, and faster locally.

arshaw commented

Apologies, this was automatically closed when I merged the PR. reopening

arshaw commented

Hey @justingrant , do you have any suggestion how I might get this working with breakpoints (essentially the node --inspect option, paired with VSCode)? Right now, the test source code is grabbed as a string, so it'd be impossible for a debugger to work methinks.

BTW, this codebase now uses pnpm

12wrigja commented

You might want to use the pre-packaged runner here: https://www.npmjs.com/package/@js-temporal/temporal-test262-runner

In that repo, I added sourcemap URLs (in js-temporal/temporal-test262-runner#10) to the test sources, so it should be possible to set breakpoints. I haven't tried doing this through vscode, but I have successfully done so via the Chrome DevTools.

  • 👍1
arshaw commented

Just a quick update.

@12wrigja, I have the test runner working well now. It's brilliant, thank you. I added some minor features and might submit a PR eventually.

@justingrant and @LinusU, I've made significant progress on this. 87% of test are now passing:

  5403 passed
  815 failed
  0 passed unexpectedly
  64 expected failures

Most failures were related to input validation.

My WIP branch is test262-fixes. The code has become rather ugly and non-DRY, which I plan to optimize after all tests are passing.

  • 🎉2
  • ❤️3
justingrant commented

@arshaw That's great progress, and is really good to hear! Heads up: there's a large PR coming (hopefully the last big one before Stage 4) here: tc39/proposal-temporal#2482. This PR replaces the timeZone property of ZonedDateTime and the calendar property of ZDT and Plain* types with timeZoneId/calendarId properties that are strings, and getTimeZone()/getCalendar() methods that return objects.

When this PR's accompanying Test262 tests are merged, likely in the next few weeks, (@ptomato should know more details) then you should expect another few hundred broken tests until your polyfill is revised to match.

Also a bunch of smaller normative changes from last week's TC39 meeting, plus a handful of PRs that aren't merged yet, should be landing soon. See https://github.com/tc39/proposal-temporal/pulls. Apologies for the churn! Thankfully we're coming down to the end of normative changes... likely very few (perhaps zero!) before Stage 4. It all depends on the feedback we get from JS engine implementers over the next few months.

FYI, it's also remotely possible that there may be one last big change (to remove nanoseconds from the API) but at this point this seems unlikely. Instead we'll probably end up imposing limits on the properties of Temporal.Duration, e.g. some values cannot be larger than MAX_SAFE_INTEGER. Stay tuned... we should have this resolved before the next TC39 meeting in the middle of May.

  • 👍3
arshaw commented

I've made a lot of progress here. I've done a big refactor to fix some foundational issues. There's still a lot more work to do, but I've successfully ran the whole test suite. All tests are either passing, are failing with known bugs I've triaged, or are failing with minor intentionally deviations from the spec (which I plan on documenting). Test run:

6676 tests finished in 11.5 s
  5749 passed
  0 failed
  0 passed unexpectedly
  927 expected failures

The CI runs in the refactor-ci branch, example run here. Dev branch is refactor-scratch.

I'll update everyone as I make progress with the remaining failures.

@justingrant, I'm aware I'll probably need to pull in the latest test262 tests and adapt to new normative changes. I'm also aware of new normative changes in the PR pipeline.

  • 👍7
justingrant commented

Great progress here! Thanks for the update.