From b5687db554a295e697f5d459cf6d3f343d2ca179 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Mon, 21 Sep 2020 18:22:01 -0400 Subject: [PATCH] Googletest export Improve DoubleNearPredFormat output on bad epsilons DoubleNearPredFormat will happily accept epsilon values (abs_error) that are so small that they are meaningless. This turns EXPECT_NEAR into a complicated and non-obvious version of EXPECT_EQ. This change modifies DoubleNearPredFormat) so that when there is a failure it calculates the smallest meaningful epsilon value, given the input values, and then prints a message which explains what happened. If a true equality test is wanted either pass a literal 0.0 as abs_error or use EXPECT_EQ. If a check for being almost equal is wanted consider using EXPECT_DOUBLE_EQ which, contrary to its name, verifies that the two numbers are *almost* equal (within four ULPs). With this change the flaky test mentioned in crbug.com/786046 gives this output: The difference between 4.2934311416234112e+18 and 4.2934311416234107e+18 is 512, where 4.2934311416234112e+18 evaluates to 4.2934311416234112e+18, 4.2934311416234107e+18 evaluates to 4.2934311416234107e+18. The abs_error parameter 1.0 evaluates to 1 which is smaller than the minimum distance between doubles for numbers of this magnitude which is 512, thus making this EXPECT_NEAR check equivalent to EXPECT_EQUAL. Consider using EXPECT_DOUBLE_EQ instead. Tested: I confirmed that this change detects the bad epsilon value that caused crbug.com/786046 in Chromium and added a test for the desired output. PiperOrigin-RevId: 332946880 --- googletest/src/gtest.cc | 24 ++++++++++++++++++++++++ googletest/test/gtest_unittest.cc | 7 +++++++ 2 files changed, 31 insertions(+) diff --git a/googletest/src/gtest.cc b/googletest/src/gtest.cc index 16cef5e683..293cc0390e 100644 --- a/googletest/src/gtest.cc +++ b/googletest/src/gtest.cc @@ -1516,6 +1516,30 @@ AssertionResult DoubleNearPredFormat(const char* expr1, const double diff = fabs(val1 - val2); if (diff <= abs_error) return AssertionSuccess(); + // Find the value which is closest to zero. + const double min_abs = std::min(fabs(val1), fabs(val2)); + // Find the distance to the next double from that value. + const double epsilon = + nextafter(min_abs, std::numeric_limits::infinity()) - min_abs; + // Detect the case where abs_error is so small that EXPECT_NEAR is + // effectively the same as EXPECT_EQUAL, and give an informative error + // message so that the situation can be more easily understood without + // requiring exotic floating-point knowledge. + // Don't do an epsilon check if abs_error is zero because that implies + // that an equality check was actually intended. + if (!isnan(val1) && !isnan(val2) && abs_error > 0 && abs_error < epsilon) { + return AssertionFailure() + << "The difference between " << expr1 << " and " << expr2 << " is " + << diff << ", where\n" + << expr1 << " evaluates to " << val1 << ",\n" + << expr2 << " evaluates to " << val2 << ".\nThe abs_error parameter " + << abs_error_expr << " evaluates to " << abs_error + << " which is smaller than the minimum distance between doubles for " + "numbers of this magnitude which is " + << epsilon + << ", thus making this EXPECT_NEAR check equivalent to " + "EXPECT_EQUAL. Consider using EXPECT_DOUBLE_EQ instead."; + } return AssertionFailure() << "The difference between " << expr1 << " and " << expr2 << " is " << diff << ", which exceeds " << abs_error_expr << ", where\n" diff --git a/googletest/test/gtest_unittest.cc b/googletest/test/gtest_unittest.cc index 4df9124712..7aa884a0fc 100644 --- a/googletest/test/gtest_unittest.cc +++ b/googletest/test/gtest_unittest.cc @@ -3084,6 +3084,13 @@ TEST_F(DoubleTest, EXPECT_NEAR) { EXPECT_NONFATAL_FAILURE(EXPECT_NEAR(1.0, 1.5, 0.25), // NOLINT "The difference between 1.0 and 1.5 is 0.5, " "which exceeds 0.25"); + // At this magnitude adjacent doubles are 512.0 apart, so this triggers a + // slightly different failure reporting path. + EXPECT_NONFATAL_FAILURE( + EXPECT_NEAR(4.2934311416234112e+18, 4.2934311416234107e+18, 1.0), + "The abs_error parameter 1.0 evaluates to 1 which is smaller than the " + "minimum distance between doubles for numbers of this magnitude which is " + "512"); } // Tests ASSERT_NEAR.