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

Add 402 staging tests for Islamic and Persian calendars #3973

Merged

Conversation

justingrant
Copy link
Contributor

Tests the changes in tc39/proposal-temporal#2743.

@justingrant justingrant requested review from a team as code owners December 22, 2023 02:59
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.

These should probably cover dayOfYear and monthsInYear as well.

Do you have a reference for where the test data comes from? I imagine that'd be helpful to implementors who are not passing these tests. I've tried looking at various Islamic calendar websites, but it's hard to tell which variant they are using.

@justingrant justingrant force-pushed the temporal-2743-islamic-persian-calendars branch 2 times, most recently from 15111f6 to 7a424de Compare January 12, 2024 12:17
@justingrant
Copy link
Contributor Author

These should probably cover dayOfYear and monthsInYear as well.

OK, I pushed an update that adds both. Note that monthsInYear is trivial because all non-lunisolar calendars have a constant number of months in every year. Both of these calendars have 12 months per year.

Do you have a reference for where the test data comes from? I imagine that'd be helpful to implementors who are not passing these tests. I've tried looking at various Islamic calendar websites, but it's hard to tell which variant they are using.

I just pulled the data from Intl.DateTimeFormat in Chrome, which in turn gets its data from ICU. If ICU is wrong, then figuring that out is beyond my ability to figure it out, because calendar implementations (especially those like the islamic calendar that involve lunar measurements and/or complex calculations) are beyond my knowledge.

@justingrant justingrant force-pushed the temporal-2743-islamic-persian-calendars branch 2 times, most recently from 16d71aa to 6b59e75 Compare January 12, 2024 12:23
@ptomato ptomato force-pushed the temporal-2743-islamic-persian-calendars branch from 6b59e75 to 6ee935a Compare January 13, 2024 00:38
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.

OK, thanks. I've made a note in the files that the data comes from ICU.

@ptomato ptomato merged commit a1ba783 into tc39:main Jan 13, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants