Skip to content
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.

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

Tests for proposal-canonical-tz Stage 3 (will be merged into Temporal) #3837

Merged
merged 5 commits into from
Aug 7, 2023

Conversation

justingrant
Copy link
Contributor

@justingrant justingrant commented May 29, 2023

This PR includes tests for the Time Zone Canonicalization proposal (proposal-canonical-tz) that reached Stage 3 at the July 2023 TC39 meeting.

At the same meeting, it was decided that this proposal should be merged into Temporal, so these tests are now part of tc39/proposal-temporal#2633.

Test roadmap

  • DONE Add lightweight "Demitasse" tests in https://github.com/tc39/proposal-canonical-tz/blob/main/polyfill/test/canonicaltz.mjs that cover this proposal's full surface area.
  • DONE Fix 15 existing Test262 tests that were broken by this proposal, because they assumed that time zone identifiers are always canonicalized.
  • DONE: Migrate Temporal.TimeZone.p.equals (the only new API in this proposal) Demitasse tests to Test262.
  • DONE: Migrate Temporal.TimeZone Demitasse tests to Test262.
  • DONE: Migrate Temporal.ZonedDateTime Demitasse tests to Test262.
  • DONE: Migrate Intl.DateTimeFormat Demitasse tests to Test262.

Copy link
Contributor

@ptomato ptomato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This matches my understanding of the proposal.

Looks like there's a merge conflict in features.txt, so it needs a rebase.

Copy link
Contributor Author

@justingrant justingrant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review! I'll make the changes you recommend. Will try to get these changed before I present in 15 hours. :-)

@ptomato
Copy link
Contributor

ptomato commented Jul 11, 2023

Thanks for the review! I'll make the changes you recommend. Will try to get these changed before I present in 15 hours. :-)

I don't think it's necessary to rush! Having test262 tests merged is a requirement for stage 4, but not 3. Anyway the comments are just minor 😄

@justingrant
Copy link
Contributor Author

All review feedback is resolved in the latest fixup commits.

I'll autosquash them if this proposal achieves Stage 3 later tonight (my time), but figured that it'd be helpful to keep them separate for now so they're easier to review.

@justingrant
Copy link
Contributor Author

@ptomato - let me know if you want to review the fixup commits that I made in response to your feedback. Otherwise I'll just rebase and autosquash them. Thanks!

@justingrant justingrant changed the title DRAFT: Tests for proposal-canonical-tz (currently Stage 2) DRAFT: Tests for proposal-canonical-tz Stage 3 (will be merged into Temporal) Jul 19, 2023
@justingrant justingrant force-pushed the proposal-canonical-tz-tests branch 4 times, most recently from c50ba7b to 38eaa3a Compare July 21, 2023 01:21
@justingrant justingrant marked this pull request as ready for review July 21, 2023 01:26
@justingrant justingrant changed the title DRAFT: Tests for proposal-canonical-tz Stage 3 (will be merged into Temporal) Tests for proposal-canonical-tz Stage 3 (will be merged into Temporal) Jul 21, 2023
@justingrant
Copy link
Contributor Author

Marked as ready for review because this proposal reached Stage 3. Also made a few updates:

@justingrant justingrant force-pushed the proposal-canonical-tz-tests branch 4 times, most recently from ae03d21 to a2b7621 Compare July 21, 2023 20:13
@justingrant justingrant force-pushed the proposal-canonical-tz-tests branch 3 times, most recently from 0b1547d to 6a9a4b1 Compare July 22, 2023 05:45
@ptomato ptomato force-pushed the proposal-canonical-tz-tests branch from 6a9a4b1 to 490ac24 Compare August 7, 2023 21:55
@ptomato ptomato merged commit 29dde1c into tc39:main Aug 7, 2023
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting stage 2.7 Supports a "Stage 2" feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants