From 22ac9d2184b3868cccfd635eb631d0eee1529121 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Wed, 1 Nov 2017 07:30:11 +0100 Subject: [PATCH] Approx cleanup: More tests, INFINITY handling, etc --- include/internal/catch_approx.cpp | 24 ++++-- include/internal/catch_approx.h | 33 ++++---- projects/SelfTest/ApproxTests.cpp | 29 +++++-- .../Baselines/console.std.approved.txt | 4 +- .../Baselines/console.sw.approved.txt | 68 +++++++++++++-- .../SelfTest/Baselines/junit.sw.approved.txt | 5 +- .../SelfTest/Baselines/xml.sw.approved.txt | 83 +++++++++++++++++-- scripts/approvalTests.py | 6 ++ 8 files changed, 207 insertions(+), 45 deletions(-) diff --git a/include/internal/catch_approx.cpp b/include/internal/catch_approx.cpp index 826a1049..82c6ff12 100644 --- a/include/internal/catch_approx.cpp +++ b/include/internal/catch_approx.cpp @@ -8,18 +8,22 @@ #include "catch_approx.h" +#include #include +namespace { + +// Performs equivalent check of std::fabs(lhs - rhs) <= margin +// But without the subtraction to allow for INFINITY in comparison +bool marginComparison(double lhs, double rhs, double margin) { + return (lhs + margin >= rhs) && (rhs + margin >= lhs); +} + +} + namespace Catch { namespace Detail { - double dmax(double lhs, double rhs) { - if (lhs < rhs) { - return rhs; - } - return lhs; - } - Approx::Approx ( double value ) : m_epsilon( std::numeric_limits::epsilon()*100 ), m_margin( 0.0 ), @@ -37,6 +41,12 @@ namespace Detail { return oss.str(); } + bool Approx::equalityComparisonImpl(const double other) const { + // First try with fixed margin, then compute margin based on epsilon, scale and Approx's value + // Thanks to Richard Harris for his help refining the scaled margin value + return marginComparison(m_value, other, m_margin) || marginComparison(m_value, other, m_epsilon * (m_scale + std::fabs(m_value))); + } + } // end namespace Detail std::string StringMaker::convert(Catch::Detail::Approx const& value) { diff --git a/include/internal/catch_approx.h b/include/internal/catch_approx.h index 8ca7f315..64f92db7 100644 --- a/include/internal/catch_approx.h +++ b/include/internal/catch_approx.h @@ -11,16 +11,15 @@ #include "catch_enforce.h" #include "catch_tostring.h" -#include - #include namespace Catch { namespace Detail { - double dmax(double lhs, double rhs); - class Approx { + private: + bool equalityComparisonImpl(double other) const; + public: explicit Approx ( double value ); @@ -42,15 +41,8 @@ namespace Detail { template ::value>::type> friend bool operator == ( const T& lhs, Approx const& rhs ) { - // Thanks to Richard Harris for his help refining this formula auto lhs_v = static_cast(lhs); - - bool relativeOK = std::fabs( lhs_v - rhs.m_value ) < rhs.m_epsilon * (rhs.m_scale + std::fabs(rhs.m_value) ); - - if (relativeOK) { - return true; - } - return std::fabs(lhs_v - rhs.m_value) <= rhs.m_margin; + return rhs.equalityComparisonImpl(lhs_v); } template ::value>::type> @@ -90,18 +82,21 @@ namespace Detail { template ::value>::type> Approx& epsilon( T const& newEpsilon ) { - double asDouble = static_cast(newEpsilon); - CATCH_ENFORCE(asDouble >= 0 && asDouble <= 1.0, - "Invalid Approx::epsilon: " << m_epsilon << - ", Approx::epsilon has to be between 0 and 1"); - m_epsilon = asDouble; + double epsilonAsDouble = static_cast(newEpsilon); + CATCH_ENFORCE(epsilonAsDouble >= 0 && epsilonAsDouble <= 1.0, + "Invalid Approx::epsilon: " << epsilonAsDouble + << ", Approx::epsilon has to be between 0 and 1"); + m_epsilon = epsilonAsDouble; return *this; } template ::value>::type> Approx& margin( T const& newMargin ) { - m_margin = static_cast(newMargin); - CATCH_ENFORCE(m_margin >= 0, "Invalid Approx::margin: " << m_margin << ", Approx::Margin has to be non-negative."); + double marginAsDouble = static_cast(newMargin); + CATCH_ENFORCE(marginAsDouble >= 0, + "Invalid Approx::margin: " << marginAsDouble + << ", Approx::Margin has to be non-negative."); + m_margin = marginAsDouble; return *this; } diff --git a/projects/SelfTest/ApproxTests.cpp b/projects/SelfTest/ApproxTests.cpp index 208be8e4..09008e5a 100644 --- a/projects/SelfTest/ApproxTests.cpp +++ b/projects/SelfTest/ApproxTests.cpp @@ -8,6 +8,8 @@ #include "catch.hpp" +#include + /////////////////////////////////////////////////////////////////////////////// TEST_CASE ( @@ -25,7 +27,7 @@ TEST_CASE REQUIRE( Approx( d ) != 1.22 ); REQUIRE( Approx( d ) != 1.24 ); - REQUIRE( 0 == Approx(0) ); + REQUIRE(INFINITY == Approx(INFINITY)); } /////////////////////////////////////////////////////////////////////////////// @@ -106,7 +108,7 @@ TEST_CASE REQUIRE( 1.0f == Approx( 1 ) ); REQUIRE( 0 == Approx( dZero) ); - REQUIRE( 0 == Approx( dSmall ).epsilon( 0.001 ) ); + REQUIRE( 0 == Approx( dSmall ).margin( 0.001 ) ); REQUIRE( 1.234f == Approx( dMedium ) ); REQUIRE( dMedium == Approx( 1.234f ) ); } @@ -120,7 +122,7 @@ TEST_CASE { double d = 1.23; - Approx approx = Approx::custom().epsilon( 0.005 ); + Approx approx = Approx::custom().epsilon( 0.01 ); REQUIRE( d == approx( 1.23 ) ); REQUIRE( d == approx( 1.22 ) ); @@ -169,9 +171,26 @@ TEST_CASE("Approx setters validate their arguments", "[Approx]") { REQUIRE_NOTHROW(Approx(0).margin(1234656)); REQUIRE_THROWS_AS(Approx(0).margin(-2), std::domain_error); + + REQUIRE_NOTHROW(Approx(0).epsilon(0)); + REQUIRE_NOTHROW(Approx(0).epsilon(1)); + + REQUIRE_THROWS_AS(Approx(0).epsilon(-0.001), std::domain_error); + REQUIRE_THROWS_AS(Approx(0).epsilon(1.0001), std::domain_error); } -//////////////////////////////////////////////////////////////////////////////// +TEST_CASE("Default scale is invisible to comparison", "[Approx]") { + REQUIRE(101.000001 != Approx(100).epsilon(0.01)); + REQUIRE(std::pow(10, -5) != Approx(std::pow(10, -7))); +} + +TEST_CASE("Epsilon only applies to Approx's value", "[Approx]") { + REQUIRE(101.01 != Approx(100).epsilon(0.01)); +} + +TEST_CASE("Assorted miscellaneous tests", "[Approx]") { + REQUIRE(INFINITY == Approx(INFINITY)); +} class StrongDoubleTypedef { @@ -207,5 +226,3 @@ TEST_CASE( "Comparison with explicitly convertible types", "[Approx]" ) REQUIRE(Approx(11.0) >= td); } - -//////////////////////////////////////////////////////////////////////////////// diff --git a/projects/SelfTest/Baselines/console.std.approved.txt b/projects/SelfTest/Baselines/console.std.approved.txt index d15e5ab8..f0791caf 100644 --- a/projects/SelfTest/Baselines/console.std.approved.txt +++ b/projects/SelfTest/Baselines/console.std.approved.txt @@ -1003,6 +1003,6 @@ with expansion: "{?}" == "1" =============================================================================== -test cases: 182 | 131 passed | 47 failed | 4 failed as expected -assertions: 896 | 779 passed | 96 failed | 21 failed as expected +test cases: 185 | 134 passed | 47 failed | 4 failed as expected +assertions: 904 | 787 passed | 96 failed | 21 failed as expected diff --git a/projects/SelfTest/Baselines/console.sw.approved.txt b/projects/SelfTest/Baselines/console.sw.approved.txt index f47b7329..2c205275 100644 --- a/projects/SelfTest/Baselines/console.sw.approved.txt +++ b/projects/SelfTest/Baselines/console.sw.approved.txt @@ -576,6 +576,22 @@ ApproxTests.cpp:: PASSED: REQUIRE_THROWS_AS( Approx(0).margin(-2), std::domain_error ) +ApproxTests.cpp:: +PASSED: + REQUIRE_NOTHROW( Approx(0).epsilon(0) ) + +ApproxTests.cpp:: +PASSED: + REQUIRE_NOTHROW( Approx(0).epsilon(1) ) + +ApproxTests.cpp:: +PASSED: + REQUIRE_THROWS_AS( Approx(0).epsilon(-0.001), std::domain_error ) + +ApproxTests.cpp:: +PASSED: + REQUIRE_THROWS_AS( Approx(0).epsilon(1.0001), std::domain_error ) + ------------------------------------------------------------------------------- Approx with exactly-representable margin ------------------------------------------------------------------------------- @@ -704,7 +720,7 @@ with expansion: ApproxTests.cpp:: PASSED: - REQUIRE( 0 == Approx( dSmall ).epsilon( 0.001 ) ) + REQUIRE( 0 == Approx( dSmall ).margin( 0.001 ) ) with expansion: 0 == Approx( 0.00001 ) @@ -798,6 +814,18 @@ PASSED: with expansion: true +------------------------------------------------------------------------------- +Assorted miscellaneous tests +------------------------------------------------------------------------------- +ApproxTests.cpp: +............................................................................... + +ApproxTests.cpp:: +PASSED: + REQUIRE( INFINITY == Approx(INFINITY) ) +with expansion: + inff == Approx( inf ) + ------------------------------------------------------------------------------- Bitfields can be captured (#1027) ------------------------------------------------------------------------------- @@ -1293,6 +1321,24 @@ ExceptionTests.cpp:: FAILED: due to unexpected exception with message: custom std exception +------------------------------------------------------------------------------- +Default scale is invisible to comparison +------------------------------------------------------------------------------- +ApproxTests.cpp: +............................................................................... + +ApproxTests.cpp:: +PASSED: + REQUIRE( 101.000001 != Approx(100).epsilon(0.01) ) +with expansion: + 101.000001 != Approx( 100.0 ) + +ApproxTests.cpp:: +PASSED: + REQUIRE( std::pow(10, -5) != Approx(std::pow(10, -7)) ) +with expansion: + 0.00001 != Approx( 0.0000001 ) + ------------------------------------------------------------------------------- EndsWith string matcher ------------------------------------------------------------------------------- @@ -1304,6 +1350,18 @@ MatchersTests.cpp:: FAILED: with expansion: "this string contains 'abc' as a substring" ends with: "this" +------------------------------------------------------------------------------- +Epsilon only applies to Approx's value +------------------------------------------------------------------------------- +ApproxTests.cpp: +............................................................................... + +ApproxTests.cpp:: +PASSED: + REQUIRE( 101.01 != Approx(100).epsilon(0.01) ) +with expansion: + 101.01 != Approx( 100.0 ) + ------------------------------------------------------------------------------- Equality checks that should fail ------------------------------------------------------------------------------- @@ -4263,9 +4321,9 @@ with expansion: ApproxTests.cpp:: PASSED: - REQUIRE( 0 == Approx(0) ) + REQUIRE( INFINITY == Approx(INFINITY) ) with expansion: - 0 == Approx( 0.0 ) + inff == Approx( inf ) Message from section one ------------------------------------------------------------------------------- @@ -7574,6 +7632,6 @@ MiscTests.cpp:: PASSED: =============================================================================== -test cases: 182 | 129 passed | 49 failed | 4 failed as expected -assertions: 895 | 775 passed | 99 failed | 21 failed as expected +test cases: 185 | 132 passed | 49 failed | 4 failed as expected +assertions: 903 | 783 passed | 99 failed | 21 failed as expected diff --git a/projects/SelfTest/Baselines/junit.sw.approved.txt b/projects/SelfTest/Baselines/junit.sw.approved.txt index 8c186f0c..0aba9725 100644 --- a/projects/SelfTest/Baselines/junit.sw.approved.txt +++ b/projects/SelfTest/Baselines/junit.sw.approved.txt @@ -1,7 +1,7 @@ - + @@ -111,6 +111,7 @@ ExceptionTests.cpp: + @@ -146,11 +147,13 @@ custom std exception ExceptionTests.cpp: + MatchersTests.cpp: + ConditionTests.cpp: diff --git a/projects/SelfTest/Baselines/xml.sw.approved.txt b/projects/SelfTest/Baselines/xml.sw.approved.txt index 07eb6a52..d3ef33c9 100644 --- a/projects/SelfTest/Baselines/xml.sw.approved.txt +++ b/projects/SelfTest/Baselines/xml.sw.approved.txt @@ -601,6 +601,38 @@ Approx(0).margin(-2), std::domain_error + + + Approx(0).epsilon(0) + + + Approx(0).epsilon(0) + + + + + Approx(0).epsilon(1) + + + Approx(0).epsilon(1) + + + + + Approx(0).epsilon(-0.001), std::domain_error + + + Approx(0).epsilon(-0.001), std::domain_error + + + + + Approx(0).epsilon(1.0001), std::domain_error + + + Approx(0).epsilon(1.0001), std::domain_error + + @@ -741,7 +773,7 @@ - 0 == Approx( dSmall ).epsilon( 0.001 ) + 0 == Approx( dSmall ).margin( 0.001 ) 0 == Approx( 0.00001 ) @@ -828,6 +860,17 @@ + + + + INFINITY == Approx(INFINITY) + + + inff == Approx( inf ) + + + + @@ -1433,6 +1476,25 @@ + + + + 101.000001 != Approx(100).epsilon(0.01) + + + 101.000001 != Approx( 100.0 ) + + + + + std::pow(10, -5) != Approx(std::pow(10, -7)) + + + 0.00001 != Approx( 0.0000001 ) + + + + @@ -1444,6 +1506,17 @@ + + + + 101.01 != Approx(100).epsilon(0.01) + + + 101.01 != Approx( 100.0 ) + + + + @@ -4875,10 +4948,10 @@ A string sent directly to stderr - 0 == Approx(0) + INFINITY == Approx(INFINITY) - 0 == Approx( 0.0 ) + inff == Approx( inf ) @@ -8373,7 +8446,7 @@ loose text artifact - + - + diff --git a/scripts/approvalTests.py b/scripts/approvalTests.py index a963531b..7f9447b9 100755 --- a/scripts/approvalTests.py +++ b/scripts/approvalTests.py @@ -45,6 +45,11 @@ errnoParser = re.compile(r''' \(\*_errno\(\)\) ''', re.VERBOSE) sinceEpochParser = re.compile(r'\d+ .+ since epoch') +infParser = re.compile(r''' + \(\(float\)\(1e\+300\ \*\ 1e\+300\)\) # MSVC INFINITY macro + | + \(__builtin_inff\(\)\) # Clang INFINITY macro +''', re.VERBOSE) if len(sys.argv) == 2: cmdPath = sys.argv[1] @@ -102,6 +107,7 @@ def filterLine(line): line = specialCaseParser.sub('file:\g<1>', line) line = errnoParser.sub('errno', line) line = sinceEpochParser.sub('{since-epoch-report}', line) + line = infParser.sub('INFINITY', line) return line