-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add error matchers #2293
Add error matchers #2293
Conversation
ace0171
to
067ef7d
Compare
Lt(0.0009)); | ||
EXPECT_THAT( | ||
elements.nodal_precession(), | ||
AllOf(AbsoluteErrorFrom(nominal_nodal_precession, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's nice that the new matchers make it easy to check both absolute and relative errors in a single expectations, but I have a hard time figuring out why we want that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure which kind of error we want here, and this means both matchers are tested.
class AbsoluteErrorFromMatcher : public MatcherInterface<Value const&> { | ||
public: | ||
using Error = | ||
decltype(AbsoluteError(std::declval<Value>(), std::declval<Value>())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this ever different from Difference<Value>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some vectors are not scalars.
using ::testing::MatcherInterface; | ||
using ::testing::MatchResultListener; | ||
|
||
template<typename Value> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put these classes in an unnamed namespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this is a header; that's what the internal
namespace is for.
using internal_error_matchers::RelativeErrorFrom; | ||
using internal_numerics_matchers::AbsoluteErrorFrom; | ||
using internal_numerics_matchers::DifferenceFrom; | ||
using internal_numerics_matchers::RelativeErrorFrom; | ||
|
||
} // namespace testing_utilities | ||
} // namespace principia | ||
|
||
#include "testing_utilities/numerics_body.hpp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't do this here (part 2).
Use them in a few tests, unbreaking that which was broken by #2290 in the process.