Approx cleanup: More tests, INFINITY handling, etc

include/internal/catch_approx.cpp

@@ -8,18 +8,22 @@
 
 #include "catch_approx.h"
 
+#include <cmath>
 #include <limits>
 
+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<float>::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<Catch::Detail::Approx>::convert(Catch::Detail::Approx const& value) {

include/internal/catch_approx.h

@@ -11,16 +11,15 @@
 #include "catch_enforce.h"
 #include "catch_tostring.h"
 
-#include <cmath>
-
 #include <type_traits>
 
 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 <typename T, typename = typename std::enable_if<std::is_constructible<double, T>::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<double>(lhs);
-
+            return rhs.equalityComparisonImpl(lhs_v);
-            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;
         }
 
         template <typename T, typename = typename std::enable_if<std::is_constructible<double, T>::value>::type>
@@ -90,18 +82,21 @@ namespace Detail {
 
         template <typename T, typename = typename std::enable_if<std::is_constructible<double, T>::value>::type>
         Approx& epsilon( T const& newEpsilon ) {
-            double asDouble = static_cast<double>(newEpsilon);
+            double epsilonAsDouble = static_cast<double>(newEpsilon);
-            CATCH_ENFORCE(asDouble >= 0 && asDouble <= 1.0,
+            CATCH_ENFORCE(epsilonAsDouble >= 0 && epsilonAsDouble <= 1.0,
-                          "Invalid Approx::epsilon: " << m_epsilon <<
+                          "Invalid Approx::epsilon: " << epsilonAsDouble
-                          ", Approx::epsilon has to be between 0 and 1");
+                          << ", Approx::epsilon has to be between 0 and 1");
-            m_epsilon = asDouble;
+            m_epsilon = epsilonAsDouble;
             return *this;
         }
 
         template <typename T, typename = typename std::enable_if<std::is_constructible<double, T>::value>::type>
         Approx& margin( T const& newMargin ) {
-            m_margin = static_cast<double>(newMargin);
+            double marginAsDouble = static_cast<double>(newMargin);
-            CATCH_ENFORCE(m_margin >= 0, "Invalid Approx::margin: " << m_margin << ", Approx::Margin has to be non-negative.");
+            CATCH_ENFORCE(marginAsDouble >= 0,
+                          "Invalid Approx::margin: " << marginAsDouble
+                          << ", Approx::Margin has to be non-negative.");
+            m_margin = marginAsDouble;
             return *this;
         }
 

projects/SelfTest/ApproxTests.cpp

@@ -8,6 +8,8 @@
 
 #include "catch.hpp"
 
+#include <cmath>
+
 ///////////////////////////////////////////////////////////////////////////////
 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);
 
 }
-
-////////////////////////////////////////////////////////////////////////////////

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
+test cases: 185 | 134 passed | 47 failed |  4 failed as expected
-assertions: 896 | 779 passed | 96 failed | 21 failed as expected
+assertions: 904 | 787 passed | 96 failed | 21 failed as expected
 

projects/SelfTest/Baselines/console.sw.approved.txt

@@ -576,6 +576,22 @@ ApproxTests.cpp:<line number>:
 PASSED:
   REQUIRE_THROWS_AS( Approx(0).margin(-2), std::domain_error )
 
+ApproxTests.cpp:<line number>:
+PASSED:
+  REQUIRE_NOTHROW( Approx(0).epsilon(0) )
+
+ApproxTests.cpp:<line number>:
+PASSED:
+  REQUIRE_NOTHROW( Approx(0).epsilon(1) )
+
+ApproxTests.cpp:<line number>:
+PASSED:
+  REQUIRE_THROWS_AS( Approx(0).epsilon(-0.001), std::domain_error )
+
+ApproxTests.cpp:<line number>:
+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:<line number>:
 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:<line number>
+...............................................................................
+
+ApproxTests.cpp:<line number>:
+PASSED:
+  REQUIRE( INFINITY == Approx(INFINITY) )
+with expansion:
+  inff == Approx( inf )
+
 -------------------------------------------------------------------------------
 Bitfields can be captured (#1027)
 -------------------------------------------------------------------------------
@@ -1293,6 +1321,24 @@ ExceptionTests.cpp:<line number>: FAILED:
 due to unexpected exception with message:
   custom std exception
 
+-------------------------------------------------------------------------------
+Default scale is invisible to comparison
+-------------------------------------------------------------------------------
+ApproxTests.cpp:<line number>
+...............................................................................
+
+ApproxTests.cpp:<line number>:
+PASSED:
+  REQUIRE( 101.000001 != Approx(100).epsilon(0.01) )
+with expansion:
+  101.000001 != Approx( 100.0 )
+
+ApproxTests.cpp:<line number>:
+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:<line number>: FAILED:
 with expansion:
   "this string contains 'abc' as a substring" ends with: "this"
 
+-------------------------------------------------------------------------------
+Epsilon only applies to Approx's value
+-------------------------------------------------------------------------------
+ApproxTests.cpp:<line number>
+...............................................................................
+
+ApproxTests.cpp:<line number>:
+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:<line number>:
 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:<line number>:
 PASSED:
 
 ===============================================================================
-test cases: 182 | 129 passed | 49 failed |  4 failed as expected
+test cases: 185 | 132 passed | 49 failed |  4 failed as expected
-assertions: 895 | 775 passed | 99 failed | 21 failed as expected
+assertions: 903 | 783 passed | 99 failed | 21 failed as expected
 

projects/SelfTest/Baselines/junit.sw.approved.txt

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <testsuitesloose text artifact
 >
-  <testsuite name="<exe-name>" errors="15" failures="85" tests="896" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
+  <testsuite name="<exe-name>" errors="15" failures="85" tests="904" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
     <testcase classname="<exe-name>.global" name="# A test name that starts with a #" time="{duration}"/>
     <testcase classname="<exe-name>.global" name="#1005: Comparing pointer to int and long (NULL can be either on various systems)" time="{duration}"/>
     <testcase classname="<exe-name>.global" name="#748 - captures with unexpected exceptions/outside assertions" time="{duration}">
@@ -111,6 +111,7 @@ ExceptionTests.cpp:<line number>
     <testcase classname="<exe-name>.global" name="Assertions then sections/A section" time="{duration}"/>
     <testcase classname="<exe-name>.global" name="Assertions then sections/A section/Another section" time="{duration}"/>
     <testcase classname="<exe-name>.global" name="Assertions then sections/A section/Another other section" time="{duration}"/>
+    <testcase classname="<exe-name>.global" name="Assorted miscellaneous tests" time="{duration}"/>
     <testcase classname="<exe-name>.global" name="Bitfields can be captured (#1027)" time="{duration}"/>
     <testcase classname="<exe-name>.global" name="Capture and info messages/Capture should stringify like assertions" time="{duration}"/>
     <testcase classname="<exe-name>.global" name="Capture and info messages/Info should NOT stringify the way assertions do" time="{duration}"/>
@@ -146,11 +147,13 @@ custom std exception
 ExceptionTests.cpp:<line number>
       </error>
     </testcase>
+    <testcase classname="<exe-name>.global" name="Default scale is invisible to comparison" time="{duration}"/>
     <testcase classname="<exe-name>.global" name="EndsWith string matcher" time="{duration}">
       <failure message="&quot;this string contains 'abc' as a substring&quot; ends with: &quot;this&quot;" type="CHECK_THAT">
 MatchersTests.cpp:<line number>
       </failure>
     </testcase>
+    <testcase classname="<exe-name>.global" name="Epsilon only applies to Approx's value" time="{duration}"/>
     <testcase classname="<exe-name>.global" name="Equality checks that should fail" time="{duration}">
       <failure message="7 == 6" type="CHECK">
 ConditionTests.cpp:<line number>

projects/SelfTest/Baselines/xml.sw.approved.txt

@@ -601,6 +601,38 @@
           Approx(0).margin(-2), std::domain_error
         </Expanded>
       </Expression>
+      <Expression success="true" type="REQUIRE_NOTHROW" filename="projects/<exe-name>/ApproxTests.cpp" >
+        <Original>
+          Approx(0).epsilon(0)
+        </Original>
+        <Expanded>
+          Approx(0).epsilon(0)
+        </Expanded>
+      </Expression>
+      <Expression success="true" type="REQUIRE_NOTHROW" filename="projects/<exe-name>/ApproxTests.cpp" >
+        <Original>
+          Approx(0).epsilon(1)
+        </Original>
+        <Expanded>
+          Approx(0).epsilon(1)
+        </Expanded>
+      </Expression>
+      <Expression success="true" type="REQUIRE_THROWS_AS" filename="projects/<exe-name>/ApproxTests.cpp" >
+        <Original>
+          Approx(0).epsilon(-0.001), std::domain_error
+        </Original>
+        <Expanded>
+          Approx(0).epsilon(-0.001), std::domain_error
+        </Expanded>
+      </Expression>
+      <Expression success="true" type="REQUIRE_THROWS_AS" filename="projects/<exe-name>/ApproxTests.cpp" >
+        <Original>
+          Approx(0).epsilon(1.0001), std::domain_error
+        </Original>
+        <Expanded>
+          Approx(0).epsilon(1.0001), std::domain_error
+        </Expanded>
+      </Expression>
       <OverallResult success="true"/>
     </TestCase>
     <TestCase name="Approx with exactly-representable margin" tags="[Approx]" filename="projects/<exe-name>/ApproxTests.cpp" >
@@ -741,7 +773,7 @@
       </Expression>
       <Expression success="true" type="REQUIRE" filename="projects/<exe-name>/ApproxTests.cpp" >
         <Original>
-          0 == Approx( dSmall ).epsilon( 0.001 )
+          0 == Approx( dSmall ).margin( 0.001 )
         </Original>
         <Expanded>
           0 == Approx( 0.00001 )
@@ -828,6 +860,17 @@
       </Section>
       <OverallResult success="true"/>
     </TestCase>
+    <TestCase name="Assorted miscellaneous tests" tags="[Approx]" filename="projects/<exe-name>/ApproxTests.cpp" >
+      <Expression success="true" type="REQUIRE" filename="projects/<exe-name>/ApproxTests.cpp" >
+        <Original>
+          INFINITY == Approx(INFINITY)
+        </Original>
+        <Expanded>
+          inff == Approx( inf )
+        </Expanded>
+      </Expression>
+      <OverallResult success="true"/>
+    </TestCase>
     <TestCase name="Bitfields can be captured (#1027)" filename="projects/<exe-name>/TrickyTests.cpp" >
       <Expression success="true" type="REQUIRE" filename="projects/<exe-name>/TrickyTests.cpp" >
         <Original>
@@ -1433,6 +1476,25 @@
       </Exception>
       <OverallResult success="false"/>
     </TestCase>
+    <TestCase name="Default scale is invisible to comparison" tags="[Approx]" filename="projects/<exe-name>/ApproxTests.cpp" >
+      <Expression success="true" type="REQUIRE" filename="projects/<exe-name>/ApproxTests.cpp" >
+        <Original>
+          101.000001 != Approx(100).epsilon(0.01)
+        </Original>
+        <Expanded>
+          101.000001 != Approx( 100.0 )
+        </Expanded>
+      </Expression>
+      <Expression success="true" type="REQUIRE" filename="projects/<exe-name>/ApproxTests.cpp" >
+        <Original>
+          std::pow(10, -5) != Approx(std::pow(10, -7))
+        </Original>
+        <Expanded>
+          0.00001 != Approx( 0.0000001 )
+        </Expanded>
+      </Expression>
+      <OverallResult success="true"/>
+    </TestCase>
     <TestCase name="EndsWith string matcher" tags="[.][failing][matchers]" filename="projects/<exe-name>/MatchersTests.cpp" >
       <Expression success="false" type="CHECK_THAT" filename="projects/<exe-name>/MatchersTests.cpp" >
         <Original>
@@ -1444,6 +1506,17 @@
       </Expression>
       <OverallResult success="false"/>
     </TestCase>
+    <TestCase name="Epsilon only applies to Approx's value" tags="[Approx]" filename="projects/<exe-name>/ApproxTests.cpp" >
+      <Expression success="true" type="REQUIRE" filename="projects/<exe-name>/ApproxTests.cpp" >
+        <Original>
+          101.01 != Approx(100).epsilon(0.01)
+        </Original>
+        <Expanded>
+          101.01 != Approx( 100.0 )
+        </Expanded>
+      </Expression>
+      <OverallResult success="true"/>
+    </TestCase>
     <TestCase name="Equality checks that should fail" tags="[!mayfail][.][failing]" filename="projects/<exe-name>/ConditionTests.cpp" >
       <Expression success="false" type="CHECK" filename="projects/<exe-name>/ConditionTests.cpp" >
         <Original>
@@ -4875,10 +4948,10 @@ A string sent directly to stderr
       </Expression>
       <Expression success="true" type="REQUIRE" filename="projects/<exe-name>/ApproxTests.cpp" >
         <Original>
-          0 == Approx(0)
+          INFINITY == Approx(INFINITY)
         </Original>
         <Expanded>
-          0 == Approx( 0.0 )
+          inff == Approx( inf )
         </Expanded>
       </Expression>
       <OverallResult success="true"/>
@@ -8373,7 +8446,7 @@ loose text artifact
       </Section>
       <OverallResult success="true"/>
     </TestCase>
-    <OverallResults successes="775" failures="100" expectedFailures="21"/>
+    <OverallResults successes="783" failures="100" expectedFailures="21"/>
   </Group>
-  <OverallResults successes="775" failures="99" expectedFailures="21"/>
+  <OverallResults successes="783" failures="99" expectedFailures="21"/>
 </Catch>

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