-
-
Notifications
You must be signed in to change notification settings - Fork 708
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
ICU-22746 Import tests from ICU4J and make more tests data-driven #2998
Conversation
LGTM, thank you. I think it would be good to share the files somehow, but that is not to be solved here. Mihai |
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.
Thank you!
M
@markusicu Reviewed, but I can't squash. I get:
|
Optional, @catamorphism : you can squash the 6 commits on your side (making sure the final commit is named |
4467440
to
d87b650
Compare
Hooray! The files in the branch are the same across the force-push. 😃 ~ Your Friendly Jira-GitHub PR Checker Bot |
Squashed, thanks! |
Includes code fixes for `numberingSystem`, `percent`, and `precision` options in `:number` Also includes a code fix for number selection: Refactor code to conform more closely to the steps in the spec, and call the number formatter before the selector so that a FormattedNumber with the right options is selected on Some modifications were needed to add missing params and to mark some tests as ignored (see ICU-22754). Also added decimal arguments in JSON test reader Finally, some redundant tests are removed: all tests in messageformat2test_features and messageformat2test_icu, and messageformat2test_builtin are now covered by JSON tests
d87b650
to
efe650a
Compare
Hooray! The files in the branch are the same across the force-push. 😃 ~ Your Friendly Jira-GitHub PR Checker Bot |
I've rebased and force-pushed, which Mihai suggested in order to fix the MSYS2 GCC build failure. |
This change brings in all the tests from ICU4J (some are marked as "ignored" with a JIRA ticket number, which is a capability I added to the JSON test harness added in #2980 .)
It also includes some code changes: to fix number selection and to fix some of the options for number formatting, which were exposed as not-working by the ICU4J tests. I chose to include these code changes in this PR so that all of the ICU4J tests could be added in one PR.
I put all of the ICU4J tests in a separate subdirectory: testdata/message2/icu4j/ . These files will need to be kept consistent with ICU4J manually (from previous discussions, I understand that there's no way to share test data between ICU4J and ICU4C).
Checklist