Skip to content

Commit

Permalink
Googletest export
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Abseil Team authored and vslashg committed Sep 24, 2020
1 parent 7aca844 commit b5687db
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 0 deletions.
24 changes: 24 additions & 0 deletions googletest/src/gtest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<double>::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"
Expand Down
7 changes: 7 additions & 0 deletions googletest/test/gtest_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit b5687db

Please sign in to comment.