Merge branch 'doubleEqualsWithDelta' into units-staging

icu4c/source/test/intltest/intltest.cpp

@@ -2040,7 +2040,6 @@ UBool IntlTest::assertEquals(const char* message,
     return TRUE;
 }
 
-
 UBool IntlTest::assertEquals(const char* message,
                              UBool expected,
                              UBool actual) {
@@ -2174,22 +2173,25 @@ UBool IntlTest::assertNotEquals(const char* message,
     return TRUE;
 }
 
-// http://junit.sourceforge.net/javadoc/org/junit/Assert.html#assertEquals(java.lang.String,%20double,%20double,%20double)
-UBool IntlTest::assertEqualsNear(const char *message, double expected, double actual, double precision) {
-    double diff = std::abs(expected - actual);
-    double diffPercent =
-        expected != 0 ? diff / expected
-                      : diff; // If the expected is equals zero, we assume that
-                              // the `diffPercent` is equal to the difference
-                              // between the actual and the expected
-
-    if (diffPercent > precision) {
-        errln((UnicodeString) "FAIL: " + message + "; got " + actual + "; expected " + expected);
+UBool IntlTest::assertEqualsNear(const char* message,
+                                 double expected,
+                                 double actual,
+                                 double delta) {
+    if (std::isnan(delta) || std::isinf(delta)) {
+        errln((UnicodeString)("FAIL: ") + message + "; nonsensical delta " + delta +
+              " - delta may not be NaN or Inf");
+        return FALSE;
+    }
+    bool bothNaN = std::isnan(expected) && std::isnan(actual);
+    double difference = abs(expected - actual);
+    if (expected != actual && (difference > delta || std::isnan(difference)) && !bothNaN) {
+        errln((UnicodeString)("FAIL: ") + message + "; got " + actual + "; expected " + expected +
+              "; acceptable delta " + delta);
         return FALSE;
     }
 #ifdef VERBOSE_ASSERTIONS
     else {
-        logln((UnicodeString) "Ok: " + message + "; got " + expected);
+        logln((UnicodeString)("Ok: ") + message + "; got " + actual);
     }
 #endif
     return TRUE;
@@ -2269,6 +2271,12 @@ UBool IntlTest::assertNotEquals(const UnicodeString &message,
                                 int32_t actual) {
     return assertNotEquals(extractToAssertBuf(message), expectedNot, actual);
 }
+UBool IntlTest::assertEqualsNear(const UnicodeString& message,
+                                 double expected,
+                                 double actual,
+                                 double delta) {
+    return assertEqualsNear(extractToAssertBuf(message), expected, actual, delta);
+}
 
 #if !UCONFIG_NO_FORMATTING
 UBool IntlTest::assertEquals(const UnicodeString& message,

icu4c/source/test/intltest/intltest.h

@@ -299,11 +299,25 @@ public:
     UBool assertEquals(const char* message, int32_t expected, int32_t actual);
     UBool assertEquals(const char* message, int64_t expected, int64_t actual);
     UBool assertEquals(const char* message, double expected, double actual);
+    /**
+     * Asserts that two doubles are equal to within a positive delta. Returns
+     * false if they are not.
+     *
+     * NaNs are considered equal: assertEquals(msg, NaN, NaN, *) passes.
+     * Infs are considered equal: assertEquals(msg, inf, inf, *) passes.
+     *
+     * @param message - the identifying message for the AssertionError.
+     * @param expected - expected value.
+     * @param actual - the value to check against expected.
+     * @param delta - the maximum delta for the absolute difference between
+     * expected and actual for which both numbers are still considered equal.
+     */
+    UBool assertEqualsNear(const char* message, double expected, double actual, double delta);
     UBool assertEquals(const char* message, UErrorCode expected, UErrorCode actual);
     UBool assertEquals(const char* message, const UnicodeSet& expected, const UnicodeSet& actual);
     UBool assertEquals(const char* message,
         const std::vector<std::string>& expected, const std::vector<std::string>& actual);
-    UBool assertEqualsNear(const char* message, double expected, double actual, double precision);     
+
 #if !UCONFIG_NO_FORMATTING
     UBool assertEquals(const char* message, const Formattable& expected,
                        const Formattable& actual, UBool possibleDataError=FALSE);
@@ -321,6 +335,20 @@ public:
     UBool assertEquals(const UnicodeString& message, int32_t expected, int32_t actual);
     UBool assertEquals(const UnicodeString& message, int64_t expected, int64_t actual);
     UBool assertEquals(const UnicodeString& message, double expected, double actual);
+    /**
+     * Asserts that two doubles are equal to within a positive delta. Returns
+     * false if they are not.
+     *
+     * NaNs are considered equal: assertEquals(msg, NaN, NaN, *) passes.
+     * Infs are considered equal: assertEquals(msg, inf, inf, *) passes.
+     *
+     * @param message - the identifying message for the AssertionError.
+     * @param expected - expected value.
+     * @param actual - the value to check against expected.
+     * @param delta - the maximum delta between expected and actual for which
+     * both numbers are still considered equal.
+     */
+    UBool assertEqualsNear(const UnicodeString& message, double expected, double actual, double delta);
     UBool assertEquals(const UnicodeString& message, UErrorCode expected, UErrorCode actual);
     UBool assertEquals(const UnicodeString& message, const UnicodeSet& expected, const UnicodeSet& actual);
     UBool assertEquals(const UnicodeString& message,

icu4c/source/test/intltest/unitstest.cpp

@@ -13,6 +13,7 @@
 #include "filestrm.h"
 #include "intltest.h"
 #include "number_decimalquantity.h"
+#include "putilimp.h"
 #include "unicode/ctest.h"
 #include "unicode/measunit.h"
 #include "unicode/unistr.h"
@@ -65,8 +66,8 @@ void UnitsTest::runIndexedTest(int32_t index, UBool exec, const char *&name, cha
 
 void UnitsTest::testConversionCapability() {
     struct TestCase {
-        const StringPiece source;
-        const StringPiece target;
+        const char *const source;
+        const char *const target;
         const UnitsConvertibilityState expectedState;
     } testCases[]{
         {"meter", "foot", CONVERTIBLE},                                         //
@@ -88,7 +89,9 @@ void UnitsTest::testConversionCapability() {
         ConversionRates conversionRates(status);
         auto convertibility = checkConvertibility(source, target, conversionRates, status);
 
-        assertEquals("Conversion Capability", testCase.expectedState, convertibility);
+        assertEquals(UnicodeString("Conversion Capability: ") + testCase.source + " to " +
+                         testCase.target,
+                     testCase.expectedState, convertibility);
     }
 }
 
@@ -96,8 +99,8 @@ void UnitsTest::testSiPrefixes() {
     IcuTestErrorCode status(*this, "Units testSiPrefixes");
     // Test Cases
     struct TestCase {
-        StringPiece source;
-        StringPiece target;
+        const char *source;
+        const char *target;
         const double inputValue;
         const double expectedValue;
     } testCases[]{
@@ -121,8 +124,9 @@ void UnitsTest::testSiPrefixes() {
         ConversionRates conversionRates(status);
         UnitConverter converter(source, target, conversionRates, status);
 
-        assertEqualsNear("test conversion", testCase.expectedValue,
-                         converter.convert(testCase.inputValue), 0.001);
+        assertEqualsNear(UnicodeString("testSiPrefixes: ") + testCase.source + " to " + testCase.target,
+                         testCase.expectedValue, converter.convert(testCase.inputValue),
+                         0.0001 * testCase.expectedValue);
     }
 }
 
@@ -131,8 +135,8 @@ void UnitsTest::testMass() {
 
     // Test Cases
     struct TestCase {
-        StringPiece source;
-        StringPiece target;
+        const char *source;
+        const char *target;
         const double inputValue;
         const double expectedValue;
     } testCases[]{
@@ -155,8 +159,9 @@ void UnitsTest::testMass() {
         ConversionRates conversionRates(status);
         UnitConverter converter(source, target, conversionRates, status);
 
-        assertEqualsNear("test conversion", testCase.expectedValue,
-                         converter.convert(testCase.inputValue), 0.001);
+        assertEqualsNear(UnicodeString("testMass: ") + testCase.source + " to " + testCase.target,
+                         testCase.expectedValue, converter.convert(testCase.inputValue),
+                         0.0001 * testCase.expectedValue);
     }
 }
 
@@ -164,8 +169,8 @@ void UnitsTest::testTemperature() {
     IcuTestErrorCode status(*this, "Units testTemperature");
     // Test Cases
     struct TestCase {
-        StringPiece source;
-        StringPiece target;
+        const char *source;
+        const char *target;
         const double inputValue;
         const double expectedValue;
     } testCases[]{
@@ -188,8 +193,9 @@ void UnitsTest::testTemperature() {
         ConversionRates conversionRates(status);
         UnitConverter converter(source, target, conversionRates, status);
 
-        assertEqualsNear("test conversion", testCase.expectedValue,
-                         converter.convert(testCase.inputValue), 0.001);
+        assertEqualsNear(UnicodeString("testTemperature: ") + testCase.source + " to " + testCase.target,
+                         testCase.expectedValue, converter.convert(testCase.inputValue),
+                         0.0001 * uprv_fabs(testCase.expectedValue));
     }
 }
 
@@ -198,8 +204,8 @@ void UnitsTest::testArea() {
 
     // Test Cases
     struct TestCase {
-        StringPiece source;
-        StringPiece target;
+        const char *source;
+        const char *target;
         const double inputValue;
         const double expectedValue;
     } testCases[]{
@@ -225,8 +231,9 @@ void UnitsTest::testArea() {
         ConversionRates conversionRates(status);
         UnitConverter converter(source, target, conversionRates, status);
 
-        assertEqualsNear("test conversion", testCase.expectedValue,
-                         converter.convert(testCase.inputValue), 0.001);
+        assertEqualsNear(UnicodeString("testArea: ") + testCase.source + " to " + testCase.target,
+                         testCase.expectedValue, converter.convert(testCase.inputValue),
+                         0.0001 * testCase.expectedValue);
     }
 }
 
@@ -337,7 +344,7 @@ void unitsTestDataLineFn(void *context, char *fields[][2], int32_t fieldCount, U
     double got = converter.convert(1000);
     msg.clear();
     msg.append("Converting 1000 ", status).append(x, status).append(" to ", status).append(y, status);
-    unitsTest->assertEqualsNear(msg.data(), expected, got, 0.0001);
+    unitsTest->assertEqualsNear(msg.data(), expected, got, 0.0001 * expected);
 }
 
 /**
@@ -453,38 +460,14 @@ class ExpectedOutput {
     }
 };
 
-// TODO(Hugo): Add a comment and Use AssertEqualsNear.
+// Checks a vector of Measure instances against ExpectedOutput.
 void checkOutput(UnitsTest *unitsTest, const char *msg, ExpectedOutput expected,
                  const MaybeStackVector<Measure> &actual, double precision) {
     IcuTestErrorCode status(*unitsTest, "checkOutput");
-    bool success = true;
-    if (expected._compoundCount != actual.length()) {
-        success = false;
-    }
-    for (int i = 0; i < actual.length(); i++) {
-        if (i >= expected._compoundCount) {
-            break;
-        }
 
-        // assertEqualsNear("test conversion", expected._amounts[i],
-        //                 actual[i]->getNumber().getDouble(status), 0.0001);
-
-        double diff = std::abs(expected._amounts[i] - actual[i]->getNumber().getDouble(status));
-        double diffPercent = expected._amounts[i] != 0 ? diff / expected._amounts[i] : diff;
-        if (diffPercent > precision) {
-            success = false;
-            break;
-        }
-
-        if (expected._measureUnits[i] != actual[i]->getUnit()) {
-            success = false;
-            break;
-        }
-    }
-
-    CharString testMessage("test case: ", status);
+    CharString testMessage("Test case \"", status);
     testMessage.append(msg, status);
-    testMessage.append(", expected output: ", status);
+    testMessage.append("\": expected output: ", status);
     testMessage.append(expected.toDebugString().c_str(), status);
     testMessage.append(", obtained output:", status);
     for (int i = 0; i < actual.length(); i++) {
@@ -493,8 +476,19 @@ void checkOutput(UnitsTest *unitsTest, const char *msg, ExpectedOutput expected,
         testMessage.append(" ", status);
         testMessage.appendInvariantChars(actual[i]->getUnit().getIdentifier(), status);
     }
-
-    unitsTest->assertTrue(testMessage.data(), success);
+    if (!unitsTest->assertEquals(testMessage.data(), expected._compoundCount, actual.length())) {
+        return;
+    };
+    for (int i = 0; i < actual.length(); i++) {
+        double permittedDiff = precision * expected._amounts[i];
+        if (permittedDiff == 0) {
+            // If 0 is expected, still permit a small delta.
+            // TODO: revisit this experimentally chosen value:
+            permittedDiff = 0.00000001;
+        }
+        unitsTest->assertEqualsNear(testMessage.data(), expected._amounts[i],
+                                    actual[i]->getNumber().getDouble(status), permittedDiff);
+    }
 }
 
 /**
@@ -574,7 +568,8 @@ void unitPreferencesTestDataLineFn(void *context, char *fields[][2], int32_t fie
     if (status.errIfFailureAndReset("router.route(inputAmount, ...)")) {
         return;
     }
-    checkOutput(unitsTest, msg.data(), expected, result, 0.0001);
+    // TODO: revisit this experimentally chosen precision:
+    checkOutput(unitsTest, msg.data(), expected, result, 0.0000000001);
 }
 
 /**