-
Notifications
You must be signed in to change notification settings - Fork 475
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
Intl: Add tests for table iteration order. #1866
Conversation
The order was defined in <tc39/ecma402#279>.
LGTM for landing together with tc39/ecma402#279 |
|
||
if ("caseFirst" in options) { | ||
expected.push("caseFirst"); | ||
} |
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.
Based on this:
Intl.Collator instances also have the following internal slots if the key corresponding to the name of the internal slot in Table 2 is included in the [[RelevantExtensionKeys]] internal slot of Intl.Collator:
- [[Numeric]] is a Boolean value, specifying whether numeric sorting is used.
- [[CaseFirst]] is one of the String values "upper", "lower", or "false".
There should be one test file that tests that these properties exist when the conditions are met, and one test file for when the condition is not met.
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.
That would be four tests, you mean? How would you distinguish them?
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.
Note, the conditions here are choices an implementation can make, not different runtime conditions achievable in a single implementation.
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.
the conditions here are choices an implementation can make,
After further reviewing the section "In Collator:" under Annex A Implementation Dependent Behaviour
, I recognize that this test is valid, however I think a specification that allows implementations to vary in this way is a poor design. All properties listed in each "Resolved Options" table should be present on the object returned by resolvedOptions()
with a value of undefined
if no useful value can be determined.
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 agree that the optionality is a sort of spec smell. I proposed that we make support mandatory in tc39/ecma402#113 but I don't think I did a great job explaining what the change would really mean. Maybe I should try to push for it again.
The order was defined in tc39/ecma402#279.