Generalize EXPECT_NEAR/ASSERT_NEAR to work with arbitrary floating-point like types

[helloworld922]

trafficstars

The EXPECT_NEAR and ASSERT_NEAR macros expect double parameters, which potentially means loss of precision or for types with greater accuracy than double, or compile errors for types not implicitly convertible to double (gcc/intel quad precision long double, Boost.MultiPrecision, etc.).

I've attached a patch which demonstrates a working example fix which generalizes this to work with arbitrary types as long as the required basic operations are available (can find an abs function, subtraction operator, comparison operator).

fp_fix.txt

[chiphogg]

So, was this closed because the fix was merged? Closed because it won't be fixed? @gennadiycivil -- can you please elaborate?

[gennadiycivil]

I have to say that I dont know. I have been doing a lot of cleanup, this was back in August and it is possible that I simply clicked the wrong button. Sorry about that Thanks G

On Thu, Jan 24, 2019 at 1:38 PM Chip Hogg [email protected] wrote:

So, was this closed because the fix was merged? Closed because it won't be fixed? @gennadiycivil https://github.com/gennadiycivil -- can you please elaborate?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/googletest/issues/890#issuecomment-457308244, or mute the thread https://github.com/notifications/unsubscribe-auth/AMJSMnXKSsCQBhNLqeWKda6ZQ2P-DSn5ks5vGf2OgaJpZM4KJGnT .

[chiphogg]

It seems there is an open pull request. Perhaps this issue should be reopened?

[gennadiycivil]

Re-Opening. Please read https://github.com/google/googletest/pull/2077#issuecomment-460729350.

[chiphogg]

Even though it looks like I shouldn't expect this to close any time soon, I'm still happy the issue is in the correct state. :slightly_smiling_face: Thanks!

[chiphogg]

Perhaps a good way to move this forward would be:

• take one of Google's internal tests which this breaks
• create a similar test in the public repo which reproduces the failure

What do you think?

[EvanHonnold]

I am encountering similar issues with EXPECT_NEAR. Looks like there was some recent discussion in https://github.com/google/googletest/pull/2077#issuecomment-783465831 about a possible fix -- might this be a way forward?

[chiphogg]

Happy New Year 2025 to all of the various EXPECT_NEAR enthusiasts here! I'm moving the conversation here, because an open issue seems like a better home than a closed PR.

In revisiting this familiar page, I realized I was taking the wrong approach pinging @gennadiycivil and @mattcalabrese-google on #2077: I don't even know if they still work on this project. Instead, I went to the issues page, and took a peek to see who is actively responding to new issues. By far, the most common name was @derekmauro. So, Derek --- are you able to clear up the status here?

The "quickie version" of the background context goes something like this:

1. Sep 2016: @helloworld922 noticed a problem with EXPECT_NEAR in #890 (this issue).
2. Jan 2019: @helloworld922 provides a fix in #2077.
3. Feb 2019: The change was favorably received, but broke a lot of Google-internal tests: so, not a trivial change.
4. Aug 2019: #2077 was subsequently abandoned. No further input from Google past this point.
5. Feb 2021: Users suggested at least two technically feasible ways to deal with a blocker of this magnitude: make a new macro with the broken behavior, or make a new macro with the fixed behavior.
6. Further pings in May 2021 and Feb 2024: no response.

With that in mind, here's how I'd suggest moving the conversation forward.

1. Can we get some explicit acknowledgement from Google (as first asked in this comment) that yes, this is a design flaw? I'm not trying to get Google to commit to fixing it; that's a separate decision. But I just want to make sure we're having a conversation from shared premises.
2. This comment concisely lays out two technically feasible options for dealing with the blocking tests using a tractable amount of engineering effort. Are either of these options palatable for the team that maintains Googletest?

[chiphogg]

Here's a brand new idea! Given the modern guidance to prefer EXPECT_THAT to other macros, googletest should introduce a new matcher, IsNear(target, tolerance), that has the correct behavior from the beginning. Since this would be totally new, it wouldn't break any existing tests: it's a much lower effort approach than the previous suggestion to fix EXPECT_NEAR. And since it's a matcher, not a macro, it wouldn't cause as much confusion as the other previous suggestion to create a new macro.

@derekmauro (or anyone else on the team), do you think this approach/strategy would be acceptable to the googletest maintainers?

[zhanyong-wan]

See https://github.com/google/googletest/commit/0bdccf4aa2f5c67af967193caf31d42d5c49bde2 . Now one can write:

   // 0.5's distance from 0.6 should be <= 0.2.
  EXPECT_THAT(0.5, DistanceFrom(0.6, Le(0.2)));

  Vector2D v1(3.0, 4.0), v2(3.2, 6.0);
  // v1's distance from v2, as computed by EuclideanDistance(v1, v2),
  // should be >= 1.0.
  EXPECT_THAT(v1, DistanceFrom(v2, EuclideanDistance, Ge(1.0)));

[chiphogg]

This is extremely encouraging --- thank you!

That said, I think we're not quite there yet. See this godbolt link.

It turns out that std::abs doesn't make sense for quantity types, because under the hood, it's comparing to a 0 without units, which is a raw numeric type, and comparing a quantity to a raw numeric type is not well defined. (For the specific case of 0, it would be well defined, but the compilation can only look at the type, not the runtime values, of course. We have a solution for this "zero" problem in our library, but that doesn't help the googletest implementation.)

I think this might work, though. Instead of this:

return std::abs(lhs - rhs)

you might try this:

using std::abs;
return abs(lhs - rhs);

Then au::abs would be found by ADL, and Au (and any other library which provides an abs) would work.

Of course, we can make it work in the meantime by using the 3-argument version and passing a lambda, but this is a little more awkward than ideal, and thankfully I expect we could make the 2-argument version Just Work without much difficulty.

[chiphogg]

By the way: I love the other design change compared to EXPECT_NEAR, where we're using a matcher instead of a simple numeric tolerance! 🤩

[zhanyong-wan]

Thanks, @chiphogg , for the feedback and suggestion.

While implementing DistanceFrom(), I was debating with myself whether to support abs() as an extension point via ADL, as this behavior might be unexpected by some users. I decided to be cautious at first and postpone the support for the extension point.

Let me discuss this with the current owners of gtest.

[chiphogg]

Thanks for keeping us in the loop on this issue!

One thing that I would find instructive is an example of how this would cause a problem. I guess this would look like a case where an ADL-found abs(...) exists, but it is not the correct function to apply to convert a custom type into a distance. Or maybe an ADL-found abs has an incompatible signature and hides std::abs, but for a type where std::abs would apply.

Of course, if you're just trying to take a cautious approach generally, then I recognize that you may not have any specific examples in mind --- but if you could come up with some, it'd be helpful.

[zhanyong-wan]

@chiphogg , it was done: https://github.com/google/googletest/commit/4ee4b17bf5ae1bf6cdb95693c174b8830898c00b

[chiphogg]

Amazing! I'm excited to check back tomorrow and play with it once the godbolt link updates.

[chiphogg]

Confirmed: it simply works!

As far as I'm concerned, this issue is now resolved, with a good outcome. Thank you!