-
Notifications
You must be signed in to change notification settings - Fork 518
Add tests for invalid calendar ids, include unknown calendar id. #4797
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
base: main
Are you sure you want to change the base?
Conversation
|
Found some mistakes, converting to draft while I fix them. |
d7217b7 to
7bedd62
Compare
|
All the mistakes are fixed, and I added new tests, this is ready for review. |
| ["calendar: '1997-12-04[u-ca=iso8601]'", "ISO string with calendar annotation"], | ||
| ["calendar: 'notacal'", "Unknown calendar"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While these are indeed invalid ISO strings, I don't think they're what you meant to test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the second string and changed the tests to have the calendar: part directly in the property bag.
Also homogenise format of existing tests, adding the test for unknown calendar id u-ca=notacal. This addresses issue tc39#3896 Two tests (argument-string-* and argument-propertybag-* are added or addended for the following. Duration.(round, total)'s option relativeTo, Duration.compare's option relativeTo, PlainDate.prototype.(equals, since, until), PlainDate.(from, compare), PlainDateTime.prototype.(equals, since, until), PlainDateTime.(from, compare), PlainMonthDay.prototype.equals, PlainMonthDay.from, PlainYearMonth.prototype.(equals, since, until), PlainDate.(from, compare), ZonedDateTime.prototype.(equals, since, until), ZonedDateTime.(from, compare) The tests calendar-invalid-iso-string.js with string arguments are addended for each constructor and each withCalendar (these do not take argument bags).
7bedd62 to
c3ab41f
Compare
|
@Ms2ger I noticed there were already tests for the Looking at the PR "Files changed" something seems off with the marking of the renamed files, however, the test names do correspond to their contents. FWIW I moved them using |
Also homogenise format of existing tests, adding the test for unknown calendar id u-ca=notacal.
This addresses issue #3896
Two tests (argument-string-* and argument-propertybag-* are added or addended
for the following.
The tests calendar-invalid-iso-string.js with string arguments are addended
for each constructor and each withCalendar (these do not take argument bags).