From: Hugo van der Merwe <17109322+hugovdm@users.noreply.github.com>
Date: Fri, 27 Mar 2020 14:58:03 +0000 (+0100)
Subject: Fix getConversionRateInfo bug.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5f0fba7c171cbafc5286b2e583cfac50b245604c;p=icu

Fix getConversionRateInfo bug.
---

diff --git a/icu4c/source/i18n/getunitsdata.cpp b/icu4c/source/i18n/getunitsdata.cpp
index ffb0415807e..7724f4cce44 100644
--- a/icu4c/source/i18n/getunitsdata.cpp
+++ b/icu4c/source/i18n/getunitsdata.cpp
@@ -322,7 +322,7 @@ MaybeStackVector<ConversionRateInfo> getConversionRatesInfo(const MeasureUnit so
         MeasureUnit baseUnit;
         processSingleUnit(targetUnits[i], convertUnitsBundle.getAlias(), convertSink, &baseUnit, status);
         if (baseCompoundUnit != NULL) {
-            if (source.getComplexity(status) == UMEASURE_UNIT_SEQUENCE) {
+            if (target.getComplexity(status) == UMEASURE_UNIT_SEQUENCE) {
                 // TODO(hugovdm): add consistency checks.
                 *baseCompoundUnit = baseUnit;
             } else {
diff --git a/icu4c/source/test/intltest/unitstest.cpp b/icu4c/source/test/intltest/unitstest.cpp
index b5a5e83f0a5..d0dedfbd9df 100644
--- a/icu4c/source/test/intltest/unitstest.cpp
+++ b/icu4c/source/test/intltest/unitstest.cpp
@@ -265,7 +265,7 @@ void runDataDrivenConversionTest(void *context, char *fields[][2], int32_t field
     if (status.errIfFailureAndReset("forIdentifier(\"%.*s\")", y.length(), y.data())) { return; }
 
     // WIP(hugovdm): Debug branch is for useful output while UnitConverter is still segfaulting.
-    UBool FIXME_skip_UnitConverter = FALSE;
+    UBool FIXME_skip_UnitConverter = TRUE;
     if (FIXME_skip_UnitConverter) {
         fprintf(stderr,
                 "FIXME: skipping constructing UnitConverter(«%s», «%s», status) because it is "
@@ -581,29 +581,56 @@ void UnitsTest::testPreferences() {
 }
 
 void UnitsTest::testGetConversionRateInfo() {
+    const int MAX_NUM_RATES = 5;
     struct {
+        // The source unit passed to getConversionRateInfo.
         const char *sourceUnit;
+        // The target unit passed to getConversionRateInfo.
         const char *targetUnit;
-        const char *threeExpectedOutputs[3];
-        const char *baseUnit;
+        // Expected: units whose conversion rates are expected in the results.
+        const char *expectedOutputs[MAX_NUM_RATES];
+        // Expected "base unit", to serve as pivot between source and target.
+        const char *expectedBaseUnit;
     } testCases[]{
         {"centimeter-per-square-milligram",
          "inch-per-square-ounce",
-         {"pound", "stone", "ton"},
+         {"meter", "gram", "inch", "ounce", NULL},
          "meter-per-square-kilogram"},
-        {"liter", "gallon", {"liter", "gallon", NULL}, "cubic-meter"},
-        {"stone-and-pound", "ton", {"pound", "stone", "ton"}, "kilogram"},
-        {"mile-per-hour", "dekameter-per-hour", {"mile", "hour", "meter"}, "meter-per-second"},
-        {"kilovolt-ampere",
+
+        {"liter", "gallon", {"liter", "gallon", NULL, NULL, NULL}, "cubic-meter"},
+
+        // Sequence
+        {"stone-and-pound", "ton", {"pound", "stone", "ton", NULL, NULL}, "kilogram"},
+
+        {"mile-per-hour",
+         "dekameter-per-hour",
+         {"mile", "hour", "meter", NULL, NULL},
+         "meter-per-second"},
+
+        // Power: watt = kilogram-square-meter-per-cubic-second
+        {"kilovolt-dekaampere",
          "horsepower",
-         {"volt", "ampere", "horsepower"},
-         "kilogram-square-meter-per-cubic-second"}, // watt
+         {"volt", "ampere", "horsepower", NULL, NULL},
+         "kilogram-square-meter-per-cubic-second"},
+
+        // Energy: joule
+        {"therm-us",
+         "kilogram-square-meter-per-square-second",
+         {"therm-us", "kilogram", "meter", "second", NULL},
+         "kilogram-square-meter-per-square-second"},
+
+        // Joule-per-meter
+        {"therm-us-per-meter",
+         "joule-per-meter",
+         {"therm-us", "joule", "meter", NULL, NULL},
+         "kilogram-meter-per-square-second"},
+
         // TODO: include capacitance test case with base unit:
         // pow4-second-square-ampere-per-kilogram-square-meter;
     };
     for (const auto &t : testCases) {
         logln("---testing: source=\"%s\", target=\"%s\", expectedBaseUnit=\"%s\"", t.sourceUnit,
-              t.targetUnit, t.baseUnit);
+              t.targetUnit, t.expectedBaseUnit);
         IcuTestErrorCode status(*this, "testGetConversionRateInfo");
 
         MeasureUnit baseCompoundUnit;
@@ -616,16 +643,32 @@ void UnitsTest::testGetConversionRateInfo() {
             continue;
         }
 
-        assertEquals("baseCompoundUnit returned by getConversionRatesInfo", t.baseUnit,
+        assertEquals("baseCompoundUnit returned by getConversionRatesInfo", t.expectedBaseUnit,
                      baseCompoundUnit.getIdentifier());
+        int countExpected;
+        for (countExpected = 0; countExpected < MAX_NUM_RATES; countExpected++) {
+            auto expected = t.expectedOutputs[countExpected];
+            if (expected == NULL) break;
+            // Check if this conversion rate was expected
+            bool found = false;
+            for (int i = 0; i < conversionInfo.length(); i++) {
+                auto cri = conversionInfo[i];
+                if (strcmp(expected, cri->sourceUnit.data()) == 0) {
+                    found = true;
+                    break;
+                }
+            }
+            assertTrue(UnicodeString("<") + expected + "> expected", found);
+        }
+        assertEquals("number of conversion rates", countExpected, conversionInfo.length());
+
+        // Convenience output for debugging
         for (int i = 0; i < conversionInfo.length(); i++) {
-            ConversionRateInfo *cri;
-            cri = conversionInfo[i];
-            logln("* conversionInfo %d: source=\"%s\", baseUnit=\"%s\", factor=\"%s\", offset=\"%s\"", i,
-                  cri->sourceUnit.data(), cri->baseUnit.data(), cri->factor.data(), cri->offset.data());
-            assertTrue("ConversionRateInfo has source, baseUnit, and factor",
-                       cri->sourceUnit.length() > 0 && cri->baseUnit.length() > 0 &&
-                           cri->factor.length() > 0);
+            ConversionRateInfo *cri = conversionInfo[i];
+            logln("* conversionInfo %d: source=\"%s\", baseUnit=\"%s\", factor=\"%s\", "
+                  "offset=\"%s\"",
+                  i, cri->sourceUnit.data(), cri->baseUnit.data(), cri->factor.data(),
+                  cri->offset.data());
         }
     }
 }