Skip to content

Conversation

@ioannad
Copy link
Contributor

@ioannad ioannad commented Dec 19, 2025

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.

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).

@ioannad ioannad requested a review from a team as a code owner December 19, 2025 18:17
@ioannad ioannad marked this pull request as draft December 26, 2025 13:25
@ioannad
Copy link
Contributor Author

ioannad commented Dec 26, 2025

Found some mistakes, converting to draft while I fix them.

@ioannad ioannad force-pushed the Temporal-unknown-calendar-tests branch from d7217b7 to 7bedd62 Compare December 29, 2025 01:16
@ioannad ioannad marked this pull request as ready for review December 29, 2025 01:17
@ioannad
Copy link
Contributor Author

ioannad commented Jan 2, 2026

All the mistakes are fixed, and I added new tests, this is ready for review.

Comment on lines 15 to 16
["calendar: '1997-12-04[u-ca=iso8601]'", "ISO string with calendar annotation"],
["calendar: 'notacal'", "Unknown calendar"],
Copy link
Contributor

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

Copy link
Contributor Author

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).
@ioannad ioannad force-pushed the Temporal-unknown-calendar-tests branch from 7bedd62 to c3ab41f Compare January 5, 2026 17:01
@ioannad
Copy link
Contributor Author

ioannad commented Jan 5, 2026

@Ms2ger I noticed there were already tests for the relativeTo option for Duration.prototype.round and Duration.prototype.total, and renamed them to match the naming scheme of the other tests, also added the new invalid calendar string "notacal".

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 git mv test/built-ins/Temporal/Duration/prototype/round/relativeto-propertybag-calendar-invalid-iso-string.js test/built-ins/Temporal/Duration/prototype/round/relativeto-argument-propertybag-calendar-invalid-iso-string.js and git mv test/built-ins/Temporal/Duration/prototype/total/relativeto-propertybag-calendar-invalid-iso-string.js test/built-ins/Temporal/Duration/prototype/total/relativeto-argument-propertybag-calendar-invalid-iso-string.js, so I don't know why they are displayed like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants