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.

Sign up for GitHub

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-22152 Uncommented a bunch of commented-out test cases in ULocaleC… #2883

Merged
merged 1 commit into from
Mar 19, 2024
Merged

ICU-22152 Uncommented a bunch of commented-out test cases in ULocaleC… #2883

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 53 additions & 47 deletions icu4j/main/collate/src/test/java/com/ibm/icu/dev/test/util/ULocaleCollationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,25 +242,25 @@ void checkService(String requestedLocale, ServiceFacade svc,
public void TestNameList() {
String[][][] tests = {
/* name in French, name in self, minimized, modified */
// {{"fr-Cyrl-BE", "fr-Cyrl-CA"},
// {"Français (cyrillique, Belgique)", "Français (cyrillique, Belgique)", "fr_Cyrl_BE", "fr_Cyrl_BE"},
// {"Français (cyrillique, Canada)", "Français (cyrillique, Canada)", "fr_Cyrl_CA", "fr_Cyrl_CA"},
// },
{{"fr-Cyrl-BE", "fr-Cyrl-CA"},
{"Français (cyrillique, Belgique)", "French (Cyrillic, Belgium)", "fr_Cyrl_BE", "fr_Cyrl_BE"},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional:

This hardcodes the English result, and I see that you are changing the test code to temporarily hardcode the default language to be English. This should work.

I wonder if it's worth it. A simpler approach would be to change the expected values that fall back to the default locale to null or "" and have the test code below simply ignore them.

On the other hand, you have done this and you could simply merge it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoding the English results and forcing the default locale to be English seemed like the path of least resistance, so I took it. TBH, I didn't think about using an empty string (or a null) as an indicator to skip the test altogether. I kind of like that, but then you're not testing the "in self" case at all, whereas forcing the default locale to be English does test that logic. On the other hand, getting English in these cases feels really wrong...

I'm inclined to just merge this and call it a day until some other problem comes up or somebody suggests a better change to the underlying code. It sounds like you don't object to that, but I'll leave this PR open until at least the end of the day just in case you change your mind or somebody else weighs in.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm inclined to just merge this and call it a day

+1

{"Français (cyrillique, Canada)", "French (Cyrillic, Canada)", "fr_Cyrl_CA", "fr_Cyrl_CA"},
},
{{"en", "de", "fr", "zh"},
{"Allemand", "Deutsch", "de", "de"},
{"Anglais", "English", "en", "en"},
{"Chinois", "中文", "zh", "zh"},
{"Français", "Français", "fr", "fr"},
},
// some non-canonical names
// {{"iw", "iw-US", "no", "no-Cyrl", "in", "in-YU"},
// {"Hébreu (États-Unis)", "עברית (ארצות הברית)", "iw_US", "iw_US"},
// {"Hébreu (Israël)", "עברית (ישראל)", "iw", "iw_IL"},
// {"Indonésien (Indonésie)", "Indonesia (Indonesia)", "in", "in_ID"},
// {"Indonésien (Serbie)", "Indonesia (Serbia)", "in_YU", "in_YU"},
// {"Norvégien (cyrillique)", "Norsk (kyrillisk)", "no_Cyrl", "no_Cyrl"},
// {"Norvégien (latin)", "Norsk (latinsk)", "no", "no_Latn"},
// },
{{"iw", "iw-US", "no", "no-Cyrl", "in", "in-YU"},
{"Hébreu (États-Unis)", "עברית (ארצות הברית)", "he_US", "he_US"},
{"Hébreu (Israël)", "עברית (ישראל)", "he", "he_IL"},
{"Indonésien (Indonésie)", "Indonesia (Indonesia)", "id", "id_ID"},
{"Indonésien (Serbie)", "Indonesia (Serbia)", "id_RS", "id_RS"},
{"Norvégien (cyrillique)", "Norwegian (Cyrillic)", "no_Cyrl", "no_Cyrl"},
{"Norvégien (latin)", "Norsk (latinsk)", "no", "no_Latn"},
},
{{"zh-Hant-TW", "en", "en-gb", "fr", "zh-Hant", "de", "de-CH", "zh-TW"},
{"Allemand (Allemagne)", "Deutsch (Deutschland)", "de", "de_DE"},
{"Allemand (Suisse)", "Deutsch (Schweiz)", "de_CH", "de_CH"},
Expand All @@ -283,46 +283,52 @@ public void TestNameList() {
{"Serbe (cyrillique)", "Српски (ћирилица)", "sr", "sr_Cyrl"},
{"Serbe (latin)", "Srpski (latinica)", "sr_Latn", "sr_Latn"},
},
// {{"fr-Cyrl", "fr-Arab"},
// {"Français (arabe)", "Français (arabe)", "fr_Arab", "fr_Arab"},
// {"Français (cyrillique)", "Français (cyrillique)", "fr_Cyrl", "fr_Cyrl"},
// },
// {{"fr-Cyrl-BE", "fr-Arab-CA"},
// {"Français (arabe, Canada)", "Français (arabe, Canada)", "fr_Arab_CA", "fr_Arab_CA"},
// {"Français (cyrillique, Belgique)", "Français (cyrillique, Belgique)", "fr_Cyrl_BE", "fr_Cyrl_BE"},
// }
{{"fr-Cyrl", "fr-Arab"},
{"Français (arabe)", "French (Arabic)", "fr_Arab", "fr_Arab"},
{"Français (cyrillique)", "French (Cyrillic)", "fr_Cyrl", "fr_Cyrl"},
},
{{"fr-Cyrl-BE", "fr-Arab-CA"},
{"Français (arabe, Canada)", "French (Arabic, Canada)", "fr_Arab_CA", "fr_Arab_CA"},
{"Français (cyrillique, Belgique)", "French (Cyrillic, Belgium)", "fr_Cyrl_BE", "fr_Cyrl_BE"},
}
};
ULocale french = ULocale.FRENCH;
LocaleDisplayNames names = LocaleDisplayNames.getInstance(french,
DisplayContext.CAPITALIZATION_FOR_UI_LIST_OR_MENU);
for (Type type : DisplayContext.Type.values()) {
logln("Contexts: " + names.getContext(type).toString());
}
Collator collator = Collator.getInstance(french);

for (String[][] test : tests) {
Set<ULocale> list = new LinkedHashSet<ULocale>();
List<UiListItem> expected = new ArrayList<UiListItem>();
for (String item : test[0]) {
list.add(new ULocale(item));
}
for (int i = 1; i < test.length; ++i) {
String[] rawRow = test[i];
expected.add(new UiListItem(new ULocale(rawRow[2]), new ULocale(rawRow[3]), rawRow[0], rawRow[1]));
ULocale originalDefaultLocale = ULocale.getDefault();
ULocale.setDefault(ULocale.US);
try {
ULocale french = ULocale.FRENCH;
LocaleDisplayNames names = LocaleDisplayNames.getInstance(french,
DisplayContext.CAPITALIZATION_FOR_UI_LIST_OR_MENU);
for (Type type : DisplayContext.Type.values()) {
logln("Contexts: " + names.getContext(type).toString());
}
List<UiListItem> newList = names.getUiList(list, false, collator);
if (!expected.equals(newList)) {
if (expected.size() != newList.size()) {
errln(list.toString() + ": wrong size" + expected + ", " + newList);
} else {
errln(list.toString());
for (int i = 0; i < expected.size(); ++i) {
assertEquals(i+"", expected.get(i), newList.get(i));
Collator collator = Collator.getInstance(french);

for (String[][] test : tests) {
Set<ULocale> list = new LinkedHashSet<ULocale>();
List<UiListItem> expected = new ArrayList<UiListItem>();
for (String item : test[0]) {
list.add(new ULocale(item));
}
for (int i = 1; i < test.length; ++i) {
String[] rawRow = test[i];
expected.add(new UiListItem(new ULocale(rawRow[2]), new ULocale(rawRow[3]), rawRow[0], rawRow[1]));
}
List<UiListItem> newList = names.getUiList(list, false, collator);
if (!expected.equals(newList)) {
if (expected.size() != newList.size()) {
errln(list.toString() + ": wrong size" + expected + ", " + newList);
} else {
// errln(list.toString());
for (int i = 0; i < expected.size(); ++i) {
assertEquals(i + "", expected.get(i), newList.get(i));
}
}
} else {
assertEquals(list.toString(), expected, newList);
}
} else {
assertEquals(list.toString(), expected, newList);
}
} finally {
ULocale.setDefault(originalDefaultLocale);
}
}

Expand Down
Loading