From 4f18ef2ef857738e977c211873a7b157bcdb87ea Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Wed, 2 Sep 2020 21:02:45 -0500 Subject: [PATCH] ICU-21258 Refactor code and tests for compact data known issue --- icu4c/source/i18n/number_compact.cpp | 5 +++ icu4c/source/test/intltest/numbertest_api.cpp | 11 ++++++ .../intltest/numbertest_decimalquantity.cpp | 10 ----- .../com/ibm/icu/impl/number/CompactData.java | 5 +++ .../test/format/CompactDecimalFormatTest.java | 3 -- .../icu/dev/test/format/PluralRulesTest.java | 3 -- .../dev/test/number/DecimalQuantityTest.java | 9 ----- .../test/number/NumberFormatterApiTest.java | 37 +++++++++++-------- 8 files changed, 43 insertions(+), 40 deletions(-) diff --git a/icu4c/source/i18n/number_compact.cpp b/icu4c/source/i18n/number_compact.cpp index e1fef8feb52..d781b6fada2 100644 --- a/icu4c/source/i18n/number_compact.cpp +++ b/icu4c/source/i18n/number_compact.cpp @@ -167,6 +167,11 @@ void CompactData::CompactDataSink::put(const char *key, ResourceValue &value, UB if (U_FAILURE(status)) { return; } for (int i4 = 0; pluralVariantsTable.getKeyAndValue(i4, key, value); ++i4) { + if (uprv_strcmp(key, "0") == 0 || uprv_strcmp(key, "1") == 0) { + // TODO(ICU-21258): Handle this case. For now, skip. + continue; + } + // Skip this magnitude/plural if we already have it from a child locale. // Note: This also skips USE_FALLBACK entries. StandardPlural::Form plural = StandardPlural::fromString(key, status); diff --git a/icu4c/source/test/intltest/numbertest_api.cpp b/icu4c/source/test/intltest/numbertest_api.cpp index b0e10155acc..88cb1da77e8 100644 --- a/icu4c/source/test/intltest/numbertest_api.cpp +++ b/icu4c/source/test/intltest/numbertest_api.cpp @@ -485,6 +485,17 @@ void NumberFormatterApiTest::notationCompact() { 1e7, u"1000\u842C"); + if (!logKnownIssue("21258", "StandardPlural cannot handle keywords 1, 0")) { + assertFormatSingle( + u"Compact with plural form =1 (ICU-21258)", + u"compact-long", + u"K", + NumberFormatter::with().notation(Notation::compactLong()), + Locale("fr-FR"), + 1e3, + u"mille"); + } + assertFormatSingle( u"Compact Infinity", u"compact-short", diff --git a/icu4c/source/test/intltest/numbertest_decimalquantity.cpp b/icu4c/source/test/intltest/numbertest_decimalquantity.cpp index b2efef25bb6..9339396e41c 100644 --- a/icu4c/source/test/intltest/numbertest_decimalquantity.cpp +++ b/icu4c/source/test/intltest/numbertest_decimalquantity.cpp @@ -531,9 +531,6 @@ void DecimalQuantityTest::testCompactDecimalSuppressedExponent() { for (const auto& cas : cases) { // test the helper methods used to compute plural operand values - if (cas.input > 900.0 && logKnownIssue("21258", "StandardPlural cannot handle keywords 1, 0")) { - continue; - } LocalizedNumberFormatter formatter = NumberFormatter::forSkeleton(cas.skeleton, status) .locale(ulocale); @@ -576,10 +573,6 @@ void DecimalQuantityTest::testCompactDecimalSuppressedExponent() { double actualIOperand = dq.getPluralOperand(PLURAL_OPERAND_I); double actualEOperand = dq.getPluralOperand(PLURAL_OPERAND_E); - assertEquals( - u"formatted number " + cas.skeleton + u" toString: " + cas.input, - cas.expectedString, - actualString); assertDoubleEquals( u"compact decimal " + cas.skeleton + u" n operand: " + cas.input, expectedNOperand, @@ -620,9 +613,6 @@ void DecimalQuantityTest::testSuppressedExponentUnchangedByInitialScaling() { }; for (const auto& cas : cases) { - if (cas.input > 900 && logKnownIssue("21258", "StandardPlural cannot handle keywords 1, 0")) { - continue; - } FormattedNumber fnCompactScaled = compactScaled.formatInt(cas.input, status); DecimalQuantity dqCompactScaled; fnCompactScaled.getDecimalQuantity(dqCompactScaled, status); diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/CompactData.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/CompactData.java index 23259df57e5..31fa08fc852 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/CompactData.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/CompactData.java @@ -177,6 +177,11 @@ public class CompactData implements MultiplierProducer { UResource.Table pluralVariantsTable = value.getTable(); for (int i4 = 0; pluralVariantsTable.getKeyAndValue(i4, key, value); ++i4) { + if ("0".equals(key.toString()) || "1".equals(key.toString())) { + // TODO(ICU-21258): Handle this case. For now, skip. + continue; + } + // Skip this magnitude/plural if we already have it from a child locale. // Note: This also skips USE_FALLBACK entries. StandardPlural plural = StandardPlural.fromString(key.toString()); diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/CompactDecimalFormatTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/CompactDecimalFormatTest.java index 60f54b5413b..86583c77716 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/CompactDecimalFormatTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/CompactDecimalFormatTest.java @@ -623,9 +623,6 @@ public class CompactDecimalFormatTest extends TestFmwk { // Run a CDF over all locales to make sure there are no unexpected exceptions. ULocale[] locs = ULocale.getAvailableLocales(); for (ULocale loc : locs) { - if (loc.getLanguage().equals("fr") && logKnownIssue("21258", "StandardPlural cannot handle keywords 1, 0")) { - continue; - } CompactDecimalFormat cdfShort = CompactDecimalFormat.getInstance(loc, CompactStyle.SHORT); CompactDecimalFormat cdfLong = CompactDecimalFormat.getInstance(loc, CompactStyle.LONG); for (double d = 12345.0; d > 0.01; d /= 10) { diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/PluralRulesTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/PluralRulesTest.java index 7d1253db029..c2084a8c77b 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/PluralRulesTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/PluralRulesTest.java @@ -937,9 +937,6 @@ public class PluralRulesTest extends TestFmwk { PluralRules rules = PluralRules.createRules("one: i = 0,1 @integer 0, 1 @decimal 0.0~1.5; many: e = 0 and i % 1000000 = 0 and v = 0 or " + "e != 0 .. 5; other: @integer 2~17, 100, 1000, 10000, 100000, 1000000, @decimal 2.0~3.5, 10.0, 100.0, 1000.0, 10000.0, 100000.0, 1000000.0, …"); ULocale locale = new ULocale("fr-FR"); - if (logKnownIssue("21258", "StandardPlural cannot handle keywords 1, 0")) { - return; - } Object[][] casesData = { // unlocalized formatter skeleton, input, string output, plural rule keyword diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/DecimalQuantityTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/DecimalQuantityTest.java index 3dcdcc21197..f57f11f8a8f 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/DecimalQuantityTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/DecimalQuantityTest.java @@ -610,9 +610,6 @@ public class DecimalQuantityTest extends TestFmwk { @Test public void testCompactDecimalSuppressedExponent() { - if (logKnownIssue("21258", "StandardPlural cannot handle keywords 1, 0")) { - return; - } ULocale locale = new ULocale("fr-FR"); Object[][] casesData = { @@ -744,9 +741,6 @@ public class DecimalQuantityTest extends TestFmwk { @Test public void testCompactNotationFractionPluralOperands() { - if (logKnownIssue("21258", "StandardPlural cannot handle keywords 1, 0")) { - return; - } ULocale locale = new ULocale("fr-FR"); LocalizedNumberFormatter formatter = NumberFormatter.withLocale(locale) @@ -812,9 +806,6 @@ public class DecimalQuantityTest extends TestFmwk { @Test public void testSuppressedExponentUnchangedByInitialScaling() { - if (logKnownIssue("21258", "StandardPlural cannot handle keywords 1, 0")) { - return; - } ULocale locale = new ULocale("fr-FR"); LocalizedNumberFormatter withLocale = NumberFormatter.withLocale(locale); LocalizedNumberFormatter compactLong = diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java index 73df02c744b..e965db6bbec 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java @@ -2,12 +2,6 @@ // License & terms of use: http://www.unicode.org/copyright.html#License package com.ibm.icu.dev.test.number; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.math.BigDecimal; @@ -20,9 +14,11 @@ import java.util.Locale; import java.util.Map; import java.util.Set; +import org.junit.Assert; import org.junit.Ignore; import org.junit.Test; +import com.ibm.icu.dev.test.TestFmwk; import com.ibm.icu.dev.test.format.FormattedValueTest; import com.ibm.icu.dev.test.serializable.SerializableTestUtility; import com.ibm.icu.impl.number.Grouper; @@ -58,7 +54,7 @@ import com.ibm.icu.util.MeasureUnit; import com.ibm.icu.util.NoUnit; import com.ibm.icu.util.ULocale; -public class NumberFormatterApiTest { +public class NumberFormatterApiTest extends TestFmwk { private static final Currency USD = Currency.getInstance("USD"); private static final Currency GBP = Currency.getInstance("GBP"); @@ -443,6 +439,17 @@ public class NumberFormatterApiTest { 1e7, "1000\u842C"); + if (!logKnownIssue("21258", "StandardPlural cannot handle keywords 1, 0")) { + assertFormatSingle( + "Compact with plural form =1 (ICU-21258)", + "compact-long", + "K", + NumberFormatter.with().notation(Notation.compactLong()), + ULocale.FRANCE, + 1e3, + "mille"); + } + assertFormatSingle( "Compact Infinity", "compact-short", @@ -2642,10 +2649,10 @@ public class NumberFormatterApiTest { @Test public void locale() { // Coverage for the locale setters. - assertEquals(NumberFormatter.with().locale(ULocale.ENGLISH), NumberFormatter.with().locale(Locale.ENGLISH)); - assertEquals(NumberFormatter.with().locale(ULocale.ENGLISH), NumberFormatter.withLocale(ULocale.ENGLISH)); - assertEquals(NumberFormatter.with().locale(ULocale.ENGLISH), NumberFormatter.withLocale(Locale.ENGLISH)); - assertNotEquals(NumberFormatter.with().locale(ULocale.ENGLISH), NumberFormatter.with().locale(Locale.FRENCH)); + Assert.assertEquals(NumberFormatter.with().locale(ULocale.ENGLISH), NumberFormatter.with().locale(Locale.ENGLISH)); + Assert.assertEquals(NumberFormatter.with().locale(ULocale.ENGLISH), NumberFormatter.withLocale(ULocale.ENGLISH)); + Assert.assertEquals(NumberFormatter.with().locale(ULocale.ENGLISH), NumberFormatter.withLocale(Locale.ENGLISH)); + Assert.assertNotEquals(NumberFormatter.with().locale(ULocale.ENGLISH), NumberFormatter.with().locale(Locale.FRENCH)); } @Test @@ -2653,19 +2660,19 @@ public class NumberFormatterApiTest { LocalizedNumberFormatter formatter = NumberFormatter.withLocale(ULocale.ENGLISH); // Double - assertEquals("514.23", formatter.format(514.23).toString()); + Assert.assertEquals("514.23", formatter.format(514.23).toString()); // Int64 - assertEquals("51,423", formatter.format(51423L).toString()); + Assert.assertEquals("51,423", formatter.format(51423L).toString()); // BigDecimal - assertEquals("987,654,321,234,567,890", + Assert.assertEquals("987,654,321,234,567,890", formatter.format(new BigDecimal("98765432123456789E1")).toString()); // Also test proper DecimalQuantity bytes storage when all digits are in the fraction. // The number needs to have exactly 40 digits, which is the size of the default buffer. // (issue discovered by the address sanitizer in C++) - assertEquals("0.009876543210987654321098765432109876543211", + Assert.assertEquals("0.009876543210987654321098765432109876543211", formatter.precision(Precision.unlimited()) .format(new BigDecimal("0.009876543210987654321098765432109876543211")) .toString());