Pressio / pressio

core C++ library
Other
45 stars 7 forks source link

tests: fix gtest expect/assert if used incorrecrtly #437

Closed fnrizzi closed 1 year ago

fnrizzi commented 2 years ago

@mzuzek you raise a good point:

mzuzek commented 2 years ago

@fnrizzi

Here's the review of EXPECT_DOUBLE_EQ suspected usage in tests on develop at 7d2dbb9. Let me know what you think of categories and what's your opinion on which cases should be fixed (i.e. exact EXPECT_DOUBLE_EQ replaced with tolerance-based EXPECT_NEAR, like in #438).

literal

Comparisons against literal expressions - like EXPECT_DOUBLE_EQ(R(7), 21*1.+22*2.+23*3.+1.) - were excluded as not suspected (i.e. correctly assuming highest level of accuracy). This includes also indirect cases where a literal is first assigned to a variable and that variable is then used in comparison (for instance see _ops/ops_eigendiag.cc:244-252).

value

These cases compare value obtained from tested calculation against value obtained from reference calculation. Most probably should use tolerance-based comparisons instead of exact ones (even if integer values are actually used at this particular moment - like for the time steps).

These cases compare against std::sqrt(NUM_LITERAL) - assuming certain accuracy of the standard function (correctly or not):

fnrizzi commented 1 year ago

i agree with this , if you want to make a PR for the changes that would be great