-
Notifications
You must be signed in to change notification settings - Fork 514
Temporal: Consolidate non-ISO month boundary tests #4754
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
ptomato
left a comment
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.
Looks like a good improvement. Didn't have time to go through thoroughly, but here's what jumped out at me.
test/intl402/Temporal/PlainDate/prototype/add/month-boundary-non-iso.js
Outdated
Show resolved
Hide resolved
| const datesBuddhist = [ | ||
| Temporal.PlainDate.from({ year: 2555, monthCode: "M12", day: 1, calendar: "buddhist" }, options), | ||
| ]; | ||
|
|
||
| const datesChinese = [ | ||
| Temporal.PlainDate.from({ year: 2019, monthCode: "M01", day: 1, calendar: "chinese" }, options), | ||
| Temporal.PlainDate.from({ year: 2019, monthCode: "M06", day: 1, calendar: "chinese" }, options), | ||
| Temporal.PlainDate.from({ year: 2019, monthCode: "M11", day: 1, calendar: "chinese" }, options), | ||
| Temporal.PlainDate.from({ year: 2019, monthCode: "M12", day: 1, calendar: "chinese" }, options), | ||
| Temporal.PlainDate.from({ year: 2000, monthCode: "M12", day: 1, calendar: "chinese" }, options), | ||
| ]; | ||
|
|
||
| const datesCoptic = [ | ||
| Temporal.PlainDate.from({ year: 1716, monthCode: "M12", day: 1, calendar: "coptic" }, options), | ||
| ]; | ||
|
|
||
| const datesDangi = [ | ||
| Temporal.PlainDate.from({ year: 2019, monthCode: "M01", day: 1, calendar: "dangi" }, options), | ||
| Temporal.PlainDate.from({ year: 2019, monthCode: "M06", day: 1, calendar: "dangi" }, options), | ||
| Temporal.PlainDate.from({ year: 2019, monthCode: "M11", day: 1, calendar: "dangi" }, options), | ||
| Temporal.PlainDate.from({ year: 2019, monthCode: "M12", day: 1, calendar: "dangi" }, options), | ||
| Temporal.PlainDate.from({ year: 2000, monthCode: "M12", day: 1, calendar: "dangi" }, options), | ||
| ]; | ||
|
|
||
| const datesEthioaa = [ | ||
| Temporal.PlainDate.from({ year: 7492, monthCode: "M12", day: 1, calendar: "ethioaa" }, options), | ||
| ]; | ||
|
|
||
| const datesEthiopic = [ | ||
| Temporal.PlainDate.from({ year: 2000, monthCode: "M12", day: 1, calendar: "ethiopic" }, options), | ||
| ]; | ||
|
|
||
| const datesGregory = [ | ||
| Temporal.PlainDate.from({ year: 2000, monthCode: "M12", day: 1, calendar: "gregory" }, options), | ||
| ]; | ||
|
|
||
| const datesHebrew = [ | ||
| Temporal.PlainDate.from({ year: 5761, monthCode: "M12", day: 1, calendar: "hebrew" }, options), | ||
| ]; | ||
|
|
||
| const datesIndian = [ | ||
| Temporal.PlainDate.from({ year: 1919, monthCode: "M12", day: 1, calendar: "indian" }), | ||
| ]; | ||
|
|
||
| const datesIslamicCivil = [ | ||
| Temporal.PlainDate.from({ year: 1420, monthCode: "M12", day: 1, calendar: "islamic-civil" }), | ||
| ]; | ||
|
|
||
| const datesIslamicTbla = [ | ||
| Temporal.PlainDate.from({ year: 1420, monthCode: "M12", day: 1, calendar: "islamic-tbla" }), | ||
| ]; | ||
|
|
||
| const datesIslamicUmalqura = [ | ||
| Temporal.PlainDate.from({ year: 1420, monthCode: "M12", day: 1, calendar: "islamic-umalqura" }), | ||
| ]; | ||
|
|
||
| const datesJapanese = [ | ||
| Temporal.PlainDate.from({ year: 2000, monthCode: "M12", day: 1, calendar: "japanese" }), | ||
| ]; | ||
|
|
||
| const datesPersian = [ | ||
| Temporal.PlainDate.from({ year: 1378, monthCode: "M01", day: 1, calendar: "persian" }), | ||
| ]; | ||
|
|
||
| const datesRoc = [ | ||
| Temporal.PlainDate.from({ year: 90, monthCode: "M01", day: 1, calendar: "roc" }), | ||
| ]; |
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.
Somehow I'd gotten the impression that all the existing tests were all starting on the calendar equivalent of ISO date 2000-01-01, but looking at this, I guess I was mistaken. Is there anything that relates all the dates? (except for the extra chinese and dangi dates of course)
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.
No, it was arbitrary. I changed the dates to be equivalent to ISO 2000-01-01 (except for the extra dates).
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.
Sorry for the back-and-forth. I should have started out by asking the second part of my question, which is, if the dates are all related, can we avoid writing them all out? Like in intl402/Temporal/PlainDate/from/roundtrip-from-iso.js, we just loop through all the calendars and create
Temporal.PlainDate.from(`2000-01-01[u-ca=${calendar}]`)and then add a few additional cases down below (we could do the same for the extra chinese and dangi dates here.)
Since the test is just verifying an invariant for addition and subtraction, it shouldn't matter how exactly we are creating the instances, and we already verify that those dates can be created from a property bag in roundtrip-from-propertybag.js.
bd2ca1c to
afbffdd
Compare
No description provided.