ICU-21284 More MeasureFormatTest and NumberFormatterApiTest test cases

See #1530

icu4c/source/i18n/unicode/measfmt.h

@@ -91,7 +91,8 @@ class DateFormat;
 /**
  * <p><strong>IMPORTANT:</strong> New users are strongly encouraged to see if
  * numberformatter.h fits their use case.  Although not deprecated, this header
- * is provided for backwards compatibility only.
+ * is provided for backwards compatibility only, and has much more limited
+ * capabilities.
  *
  * @see Format
  * @author Alan Liu

icu4c/source/i18n/units_complexconverter.cpp

@@ -10,6 +10,7 @@
 #include "cmemory.h"
 #include "number_decimalquantity.h"
 #include "number_roundingutils.h"
+#include "putilimp.h"
 #include "uarrsort.h"
 #include "uassert.h"
 #include "unicode/fmtable.h"
@@ -149,6 +150,12 @@ MaybeStackVector<Measure> ComplexUnitsConverter::convert(double quantity,
             // If quantity is at the limits of double's precision from an
             // integer value, we take that integer value.
             int64_t flooredQuantity = floor(quantity * (1 + DBL_EPSILON));
+            if (uprv_isNaN(quantity)) {
+                // With clang on Linux: floor does not support NaN, resulting in
+                // a giant negative number. For now, we produce "0 feet, NaN
+                // inches". TODO(icu-units#131): revisit desired output.
+                flooredQuantity = 0;
+            }
             intValues[i] = flooredQuantity;
 
             // Keep the residual of the quantity.

icu4c/source/test/intltest/measfmttest.cpp

@@ -19,6 +19,7 @@
 
 #include "charstr.h"
 #include "cstr.h"
+#include "cstring.h"
 #include "measunit_impl.h"
 #include "unicode/decimfmt.h"
 #include "unicode/measfmt.h"
@@ -85,6 +86,7 @@ private:
     void TestInvalidIdentifiers();
     void TestIdentifierDetails();
     void TestPrefixes();
+    void TestParseBuiltIns();
     void TestParseToBuiltIn();
     void TestKilogramIdentifier();
     void TestCompoundUnitOperations();
@@ -216,6 +218,7 @@ void MeasureFormatTest::runIndexedTest(
     TESTCASE_AUTO(TestInvalidIdentifiers);
     TESTCASE_AUTO(TestIdentifierDetails);
     TESTCASE_AUTO(TestPrefixes);
+    TESTCASE_AUTO(TestParseBuiltIns);
     TESTCASE_AUTO(TestParseToBuiltIn);
     TESTCASE_AUTO(TestKilogramIdentifier);
     TESTCASE_AUTO(TestCompoundUnitOperations);
@@ -3666,8 +3669,16 @@ void MeasureFormatTest::TestIdentifiers() {
         {"pow2-foot-and-pow2-mile", "square-foot-and-square-mile"},
         {"gram-square-gram-per-dekagram", "cubic-gram-per-dekagram"},
         {"kilogram-per-meter-per-second", "kilogram-per-meter-second"},
+        {"kilometer-per-second-per-megaparsec", "kilometer-per-megaparsec-second"},
 
         // TODO(ICU-21284): Add more test cases once the proper ranking is available.
+        // TODO(ICU-21284,icu-units#70): These cases are the wrong way around:
+        {"pound-force-foot", "foot-pound-force"},
+        {"foot-pound-force", "foot-pound-force"},
+        {"kilowatt-hour", "hour-kilowatt"},
+        {"hour-kilowatt", "hour-kilowatt"},
+        {"newton-meter", "meter-newton"},
+        {"meter-newton", "meter-newton"},
 
         // Testing prefixes are parsed and produced correctly (ensures no
         // collisions in the enum values)
@@ -3706,7 +3717,6 @@ void MeasureFormatTest::TestIdentifiers() {
         // TODO(icu-units#70): revisit when fixing normalization. For now we're
         // just checking some consistency between C&J.
         {"megafoot-mebifoot-kibifoot-kilofoot", "kibifoot-mebifoot-kilofoot-megafoot"},
-
     };
     for (const auto &cas : cases) {
         status.setScope(cas.id);
@@ -3836,6 +3846,42 @@ void MeasureFormatTest::TestPrefixes() {
     }
 }
 
+void MeasureFormatTest::TestParseBuiltIns() {
+    IcuTestErrorCode status(*this, "TestParseBuiltIns()");
+    int32_t totalCount = MeasureUnit::getAvailable(nullptr, 0, status);
+    status.expectErrorAndReset(U_BUFFER_OVERFLOW_ERROR);
+    std::unique_ptr<MeasureUnit[]> units(new MeasureUnit[totalCount]);
+    totalCount = MeasureUnit::getAvailable(units.get(), totalCount, status);
+    status.assertSuccess();
+    for (int32_t i = 0; i < totalCount; i++) {
+        MeasureUnit &unit = units[i];
+        if (uprv_strcmp(unit.getType(), "currency") == 0) {
+            continue;
+        }
+
+        // TODO(ICU-21284,icu-units#70): fix normalization. Until then, ignore:
+        if (uprv_strcmp(unit.getIdentifier(), "pound-force-foot") == 0) continue;
+        if (uprv_strcmp(unit.getIdentifier(), "kilowatt-hour") == 0) continue;
+        if (uprv_strcmp(unit.getIdentifier(), "newton-meter") == 0) continue;
+
+        // Prove that all built-in units are parseable, except "generic" temperature:
+        MeasureUnit parsed = MeasureUnit::forIdentifier(unit.getIdentifier(), status);
+        if (unit == MeasureUnit::getGenericTemperature()) {
+            status.expectErrorAndReset(U_ILLEGAL_ARGUMENT_ERROR);
+        } else {
+            status.assertSuccess();
+            CharString msg;
+            msg.append("parsed MeasureUnit '", status);
+            msg.append(parsed.getIdentifier(), status);
+            msg.append("' should equal built-in '", status);
+            msg.append(unit.getIdentifier(), status);
+            msg.append("'", status);
+            status.assertSuccess();
+            assertTrue(msg.data(), unit == parsed);
+        }
+    }
+}
+
 void MeasureFormatTest::TestParseToBuiltIn() {
     IcuTestErrorCode status(*this, "TestParseToBuiltIn()");
     const struct TestCase {

icu4c/source/test/intltest/numbertest_api.cpp

@@ -715,6 +715,16 @@ void NumberFormatterApiTest::unitMeasure() {
             5,
             u"5 a\u00F1os");
 
+    // TODO(ICU-20941): arbitrary unit formatting
+//     assertFormatSingle(
+//             u"Hubble Constant",
+//             u"unit/kilometer-per-megaparsec-second",
+//             u"unit/kilometer-per-megaparsec-second",
+//             NumberFormatter::with().unit(MeasureUnit::forIdentifier("kilometer-per-megaparsec-second", status)),
+//             Locale("en"),
+//             74, // Approximate 2019-03-18 measurement
+//             u"74 km/s.Mpc");
+
     assertFormatSingle(
             u"Mixed unit",
             u"unit/yard-and-foot-and-inch",
@@ -849,7 +859,7 @@ void NumberFormatterApiTest::unitMeasure() {
             NumberFormatter::with().unit(MeasureUnit::forIdentifier("celsius", status)),
             Locale("nl-NL"),
             -6.5,
-            u"-6,5\u00B0C");
+            u"-6,5°C");
 
     assertFormatSingle(
             u"Negative numbers: time",
@@ -868,6 +878,39 @@ void NumberFormatterApiTest::unitMeasure() {
             Locale("en"),
             100,
             u"100");
+
+    // TODO: desired behaviour for this "pathological" case?
+    // Since this is pointless, we don't test that its behaviour doesn't change.
+    // As of January 2021, the produced result has a missing sign: 23.5 Kelvin
+    // is "23 Kelvin and -272.65 degrees Celsius":
+//     assertFormatSingle(
+//             u"Meaningless: kelvin-and-celcius",
+//             u"unit/kelvin-and-celsius",
+//             u"unit/kelvin-and-celsius",
+//             NumberFormatter::with().unit(MeasureUnit::forIdentifier("kelvin-and-celsius", status)),
+//             Locale("en"),
+//             23.5,
+//             u"23 K, 272.65°C");
+
+    if (uprv_getNaN() != 0.0) {
+        assertFormatSingle(
+                u"Measured -Inf",
+                u"measure-unit/electric-ampere",
+                u"unit/ampere",
+                NumberFormatter::with().unit(MeasureUnit::getAmpere()),
+                Locale("en"),
+                -uprv_getInfinity(),
+                u"-∞ A");
+
+        assertFormatSingle(
+                u"Measured NaN",
+                u"measure-unit/temperature-celsius",
+                u"unit/celsius",
+                NumberFormatter::with().unit(MeasureUnit::forIdentifier("celsius", status)),
+                Locale("en"),
+                uprv_getNaN(),
+                u"NaN°C");
+    }
 }
 
 void NumberFormatterApiTest::unitCompoundMeasure() {
@@ -1433,6 +1476,26 @@ void NumberFormatterApiTest::unitUsage() {
             u"8,765E0 square metres",
             u"0E0 square centimetres");
 
+    assertFormatSingle(
+            u"Negative Infinity with Unit Preferences",
+            u"measure-unit/area-acre usage/default",
+            u"unit/acre usage/default",
+            NumberFormatter::with().unit(MeasureUnit::getAcre()).usage("default"),
+            Locale::getEnglish(),
+            -uprv_getInfinity(),
+            u"-∞ km²");
+
+//     // TODO(icu-units#131): do we care about NaN?
+//     // TODO: on some platforms with MSVC, "-NaN sec" is returned.
+//     assertFormatSingle(
+//             u"NaN with Unit Preferences",
+//             u"measure-unit/area-acre usage/default",
+//             u"unit/acre usage/default",
+//             NumberFormatter::with().unit(MeasureUnit::getAcre()).usage("default"),
+//             Locale::getEnglish(),
+//             uprv_getNaN(),
+//             u"NaN cm²");
+
     assertFormatSingle(
             u"Negative numbers: minute-and-second",
             u"measure-unit/duration-second usage/media",
@@ -1442,6 +1505,34 @@ void NumberFormatterApiTest::unitUsage() {
             -77.7,
             u"-1 min, 18 sec");
 
+    assertFormatSingle(
+            u"Negative numbers: media seconds",
+            u"measure-unit/duration-second usage/media",
+            u"unit/second usage/media",
+            NumberFormatter::with().unit(SECOND).usage("media"),
+            Locale("nl-NL"),
+            -2.7,
+            u"-2,7 sec");
+
+//     // TODO: on some platforms with MSVC, "-NaN sec" is returned.
+//     assertFormatSingle(
+//             u"NaN minute-and-second",
+//             u"measure-unit/duration-second usage/media",
+//             u"unit/second usage/media",
+//             NumberFormatter::with().unit(SECOND).usage("media"),
+//             Locale("nl-NL"),
+//             uprv_getNaN(),
+//             u"NaN sec");
+
+    assertFormatSingle(
+            u"NaN meter-and-centimeter",
+            u"measure-unit/length-meter usage/person-height",
+            u"unit/meter usage/person-height",
+            NumberFormatter::with().unit(METER).usage("person-height"),
+            Locale("de-DE"),
+            uprv_getNaN(),
+            u"0 m, NaN cm");
+
     assertFormatSingle(
             u"Rounding Mode propagates: rounding down",
             u"usage/road measure-unit/length-centimeter rounding-mode-floor",
@@ -1490,6 +1581,13 @@ void NumberFormatterApiTest::unitUsageErrorCodes() {
     // Adding the unit as part of the fluent chain leads to success.
     unloc_formatter.unit(MeasureUnit::getMeter()).locale("en-GB").formatInt(1, status);
     status.assertSuccess();
+
+    // Setting unit to the "base dimensionless unit" is like clearing unit.
+    unloc_formatter = NumberFormatter::with().unit(MeasureUnit()).usage("default");
+    // This does not give an error, because usage-vs-unit isn't resolved yet.
+    status.errIfFailureAndReset("Expected behaviour: no immediate error for invalid unit");
+    unloc_formatter.locale("en-GB").formatInt(1, status);
+    status.expectErrorAndReset(U_ILLEGAL_ARGUMENT_ERROR);
 }
 
 // Tests for the "skeletons" field in unitPreferenceData, as well as precision

icu4j/main/classes/core/src/com/ibm/icu/text/MeasureFormat.java

@@ -62,7 +62,8 @@ import com.ibm.icu.util.UResourceBundle;
  * <p>
  * <strong>IMPORTANT:</strong> New users are strongly encouraged to see if
  * {@link NumberFormatter} fits their use case.  Although not deprecated, this
- * class, MeasureFormat, is provided for backwards compatibility only.
+ * class, MeasureFormat, is provided for backwards compatibility only, and has
+ * much more limited capabilities.
  * <hr>
  *
  * <p>

icu4j/main/classes/core/src/com/ibm/icu/util/MeasureUnit.java

@@ -695,7 +695,8 @@ public class MeasureUnit implements Serializable {
      */
     @Override
     public String toString() {
-        return type + "-" + subType;
+        String result = measureUnitImpl == null ? type + "-" + subType : measureUnitImpl.getIdentifier();
+        return result == null ? "" : result;
     }
 
     /**

icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/MeasureUnitTest.java

@@ -3494,8 +3494,16 @@ public class MeasureUnitTest extends TestFmwk {
             new TestCase("pow2-foot-and-pow2-mile", "square-foot-and-square-mile"),
             new TestCase("gram-square-gram-per-dekagram", "cubic-gram-per-dekagram"),
             new TestCase("kilogram-per-meter-per-second", "kilogram-per-meter-second"),
+            new TestCase("kilometer-per-second-per-megaparsec", "kilometer-per-megaparsec-second"),
 
             // TODO(ICU-21284): Add more test cases once the proper ranking is available.
+            // TODO(ICU-21284,icu-units#70): These cases are the wrong way around:
+            new TestCase("pound-force-foot", "foot-pound-force"),
+            new TestCase("foot-pound-force", "foot-pound-force"),
+            new TestCase("kilowatt-hour", "hour-kilowatt"),
+            new TestCase("hour-kilowatt", "hour-kilowatt"),
+            new TestCase("newton-meter", "meter-newton"),
+            new TestCase("meter-newton", "meter-newton"),
 
             // Testing prefixes are parsed and produced correctly (ensures no
             // collisions in the enum values)
@@ -3664,6 +3672,35 @@ public class MeasureUnitTest extends TestFmwk {
         }
     }
 
+    @Test
+    public void TestParseBuiltIns() {
+        for (MeasureUnit unit : MeasureUnit.getAvailable()) {
+            System.out.println("unit ident: " + unit.getIdentifier() + ", type: " + unit.getType());
+            if (unit.getType() == "currency") {
+                continue;
+            }
+
+            // TODO(ICU-21284,icu-units#70): fix normalization. Until then, ignore:
+            if (unit.getIdentifier() == "pound-force-foot") continue;
+            if (unit.getIdentifier() == "kilowatt-hour") continue;
+            if (unit.getIdentifier() == "newton-meter") continue;
+
+            // Prove that all built-in units are parseable, except "generic" temperature:
+            if (unit == MeasureUnit.GENERIC_TEMPERATURE) {
+                try {
+                    MeasureUnit.forIdentifier(unit.getIdentifier());
+                    Assert.fail("GENERIC_TEMPERATURE should not be parseable");
+                } catch (IllegalArgumentException e) {
+                    continue;
+                }
+            } else {
+                MeasureUnit parsed = MeasureUnit.forIdentifier(unit.getIdentifier());
+                assertTrue("parsed MeasureUnit '" + parsed + "'' should equal built-in '" + unit + "'",
+                           unit.equals(parsed));
+            }
+        }
+    }
+
     @Test
     public void TestParseToBuiltIn() {
         class TestCase {

icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java

@@ -673,6 +673,17 @@ public class NumberFormatterApiTest extends TestFmwk {
                 5,
                 "5 a\u00F1os");
 
+        // TODO(ICU-20941): arbitrary unit formatting
+        // assertFormatSingle(
+        //         "Hubble Constant",
+        //         "unit/kilometer-per-megaparsec-second",
+        //         "unit/kilometer-per-megaparsec-second",
+        //         NumberFormatter.with()
+        //                 .unit(MeasureUnit.forIdentifier("kilometer-per-megaparsec-second")),
+        //         new ULocale("en"),
+        //         74, // Approximate 2019-03-18 measurement
+        //         "74 km/s.Mpc");
+
         assertFormatSingle(
                 "Mixed unit",
                 "unit/yard-and-foot-and-inch",
@@ -813,7 +824,7 @@ public class NumberFormatterApiTest extends TestFmwk {
                 NumberFormatter.with().unit(MeasureUnit.forIdentifier("celsius")),
                 new ULocale("nl-NL"),
                 -6.5,
-                "-6,5\u00B0C");
+                "-6,5°C");
 
         assertFormatSingle(
                 "Negative numbers: time",
@@ -832,6 +843,37 @@ public class NumberFormatterApiTest extends TestFmwk {
                 new ULocale("en"),
                 100,
                 "100");
+
+        // TODO: desired behaviour for this "pathological" case?
+        // Since this is pointless, we don't test that its behaviour doesn't change.
+        // As of January 2021, the produced result has a missing sign: 23.5 Kelvin
+        // is "23 Kelvin and -272.65 degrees Celsius":
+        // assertFormatSingle(
+        //         "Meaningless: kelvin-and-celcius",
+        //         "unit/kelvin-and-celsius",
+        //         "unit/kelvin-and-celsius",
+        //         NumberFormatter.with().unit(MeasureUnit.forIdentifier("kelvin-and-celsius")),
+        //         new ULocale("en"),
+        //         23.5,
+        //         "23 K, 272.65°C");
+
+        assertFormatSingle(
+                "Measured -Inf",
+                "measure-unit/electric-ampere",
+                "unit/ampere",
+                NumberFormatter.with().unit(MeasureUnit.AMPERE),
+                new ULocale("en"),
+                Double.NEGATIVE_INFINITY,
+                "-∞ A");
+
+        assertFormatSingle(
+                "Measured NaN",
+                "measure-unit/temperature-celsius",
+                "unit/celsius",
+                NumberFormatter.with().unit(MeasureUnit.forIdentifier("celsius")),
+                new ULocale("en"),
+                Double.NaN,
+                "NaN°C");
     }
 
     @Test
@@ -1402,6 +1444,30 @@ public class NumberFormatterApiTest extends TestFmwk {
                "8,765E0 square metres",
                "0E0 square centimetres");
 
+        // TODO(icu-units#132): Java BigDecimal does not support Inf and NaN, so
+        // we get a misleading "0" out of this:
+        assertFormatSingle(
+                "Negative Infinity with Unit Preferences",
+                "measure-unit/area-acre usage/default",
+                "unit/acre usage/default",
+                NumberFormatter.with().unit(MeasureUnit.ACRE).usage("default"),
+                ULocale.ENGLISH,
+                Double.NEGATIVE_INFINITY,
+                // "-∞ km²");
+                "0 cm²");
+
+        // TODO(icu-units#132): Java BigDecimal does not support Inf and NaN, so
+        // we get a misleading "0" out of this:
+        assertFormatSingle(
+                "NaN with Unit Preferences",
+                "measure-unit/area-acre usage/default",
+                "unit/acre usage/default",
+                NumberFormatter.with().unit(MeasureUnit.ACRE).usage("default"),
+                ULocale.ENGLISH,
+                Double.NaN,
+                // "NaN cm²");
+                "0 cm²");
+
         assertFormatSingle(
                 "Negative numbers: minute-and-second",
                 "measure-unit/duration-second usage/media",
@@ -1411,6 +1477,39 @@ public class NumberFormatterApiTest extends TestFmwk {
                 -77.7,
                 "-1 min, 18 sec");
 
+        assertFormatSingle(
+                "Negative numbers: media seconds",
+                "measure-unit/duration-second usage/media",
+                "unit/second usage/media",
+                NumberFormatter.with().unit(MeasureUnit.SECOND).usage("media"),
+                new ULocale("nl-NL"),
+                -2.7,
+                "-2,7 sec");
+
+        // TODO(icu-units#132): Java BigDecimal does not support Inf and NaN, so
+        // we get a misleading "0" out of this:
+        assertFormatSingle(
+                "NaN minute-and-second",
+                "measure-unit/duration-second usage/media",
+                "unit/second usage/media",
+                NumberFormatter.with().unit(MeasureUnit.SECOND).usage("media"),
+                new ULocale("nl-NL"),
+                Double.NaN,
+                // "NaN sec");
+                "0 sec");
+
+        // TODO(icu-units#132): Java BigDecimal does not support Inf and NaN, so
+        // we get a misleading "0" out of this:
+        assertFormatSingle(
+                "NaN meter-and-centimeter",
+                "measure-unit/length-meter usage/person-height",
+                "unit/meter usage/person-height",
+                NumberFormatter.with().unit(MeasureUnit.METER).usage("person-height"),
+                new ULocale("en-DE"),
+                Double.NaN,
+                // "0 m, NaN cm");
+                "0 m, 0 cm");
+
         assertFormatSingle(
                 "Rounding Mode propagates: rounding down",
                 "usage/road measure-unit/length-centimeter rounding-mode-floor",
@@ -1468,6 +1567,14 @@ public class NumberFormatterApiTest extends TestFmwk {
         // Adding the unit as part of the fluent chain leads to success.
         unloc_formatter.unit(MeasureUnit.METER).locale(new ULocale("en-GB")).format(1); /* No Exception should be thrown */
 
+        // Setting unit to the "base dimensionless unit" is like clearing unit.
+        unloc_formatter = NumberFormatter.with().unit(NoUnit.BASE).usage("default");
+        try {
+            unloc_formatter.locale(new ULocale("en-GB")).format(1);
+            fail("should throw IllegalArgumentException");
+        } catch (IllegalArgumentException e) {
+            // Pass
+        }
     }