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

Temporal: tests for IETF annotations standard #3683

Merged
merged 11 commits into from
Oct 11, 2022
Merged

Conversation

ptomato
Copy link
Contributor

@ptomato ptomato commented Oct 8, 2022

This is another giant pull request, I recommend reviewing it commit by commit again.

Most of the tests in each commit are the same, so I'd recommend reviewing a representative sample in each commit; maybe pay special attention to Calendar.dateUntil, ZonedDateTime.from, ZonedDateTime.equals, and the compare() functions, since those are the ones where I sometimes had to do something out of the ordinary compared to the rest of the tests.

These tests are for the normative changes in tc39/proposal-temporal#2397 which reached consensus at the September 2022 TC39 meeting.

ptomato added 11 commits October 7, 2022 11:39
As of tc39/proposal-temporal#2397 which reached
consensus in the August 2022 TC39 meeting, a date-time + Z with no bracket
annotation is no longer accepted as a relativeTo parameter; either the Z
should be removed or a bracket annotation should be added.

This requires adjusting a few existing tests, but doesn't require any new
ones.
See tc39/proposal-temporal#2397
Adds tests for ISO strings with named and numeric offset time zone
annotations, with and without the critical flag, with various combinations
of Z and offset in front of the annotation.
See tc39/proposal-temporal#2397
Adds tests for ISO strings with calendar annotations, with and without the
critical flag, and also a check that the second calendar annotation is
disregarded, as per the IETF draft.
See tc39/proposal-temporal#2397
Adds tests for ISO strings with unrecognized annotations, (i.e., neither
time zone nor calendar), in various combinations with recognized
annotations.
See tc39/proposal-temporal#2397
Adds tests for ISO strings with unrecognized annotations with the critical
flag. These strings should all be rejected.
See tc39/proposal-temporal#2397
Adds tests for ISO strings with more than one time zone annotation. These
are not syntactically correct according to the grammar and should be
rejected.
Since we are going to be adding a new test for calendarName: "critical",
take the existing tests for various values of the calendarName option, and
regularize them. Previously, depending on which type's toString() method
was under test, the tests had various degrees of thoroughness, and some
were only present in staging.
See tc39/proposal-temporal#2397
This normative change adds a new accepted value for the calendarName
option in various toString() methods.
Based on the improvements just made to the calendarName option, improve
the tests for the timeZoneName option of ZonedDateTime.prototype.toString
as well.
See tc39/proposal-temporal#2397
This normative change adds a new accepted value for the timeZoneName
option in ZonedDateTime.prototype.toString().
The programmer always gets the last word over how the string is
interpreted, since otherwise it's not possible to make any guarantees
about the offset option. (This is the "out-of-band" mechanism mentioned in
the IETF draft.) Add a test for this.
@ptomato ptomato requested review from a team as code owners October 8, 2022 01:46
@ptomato ptomato requested review from Aditi-1400 and Ms2ger October 8, 2022 01:47
@ptomato ptomato added needs review has consensus This has committee consensus labels Oct 10, 2022
Copy link
Contributor

@Aditi-1400 Aditi-1400 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 these! :)

@Ms2ger Ms2ger merged commit 1550f7f into tc39:main Oct 11, 2022
@ptomato ptomato deleted the temporal-2397 branch October 11, 2022 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has consensus This has committee consensus needs review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants