Fix assertionStarting events being sent after the expr is evaluated

Closes #2678

7 files changed, 110 insertions, 12 deletions

diff --git a/docs/reporter-events.md b/docs/reporter-events.md
index 32a0ae50..015f67be 100644
--- a/docs/reporter-events.md
+++ b/docs/reporter-events.md
@@ -96,12 +96,12 @@ void assertionStarting( AssertionInfo const& assertionInfo );
 void assertionEnded( AssertionStats const& assertionStats );
 ```
 
-`assertionStarting` is called after the expression is captured, but before
-the assertion expression is evaluated. This might seem like a minor
-distinction, but what it means is that if you have assertion like
-`REQUIRE( a + b == c + d )`, then what happens is that `a + b` and `c + d`
-are evaluated before `assertionStarting` is emitted, while the `==` is
-evaluated after the event.
+The `assertionStarting` event is emitted before the expression in the
+assertion is captured or evaluated and `assertionEnded` is emitted
+afterwards. This means that given assertion like `REQUIRE(a + b == c + d)`,
+Catch2 first emits `assertionStarting` event, then `a + b` and `c + d`
+are evaluated, then their results are captured, the comparison is evaluated,
+and then `assertionEnded` event is emitted.
 
 
 ## Benchmarking events
diff --git a/src/catch2/interfaces/catch_interfaces_capture.hpp b/src/catch2/interfaces/catch_interfaces_capture.hpp
index e605f998..a1876a4c 100644
--- a/src/catch2/interfaces/catch_interfaces_capture.hpp
+++ b/src/catch2/interfaces/catch_interfaces_capture.hpp
@@ -43,6 +43,7 @@ namespace Catch {
     public:
         virtual ~IResultCapture();
 
+        virtual void notifyAssertionStarted( AssertionInfo const& info ) = 0;
         virtual bool sectionStarted( StringRef sectionName,
                                      SourceLineInfo const& sectionLineInfo,
                                      Counts& assertions ) = 0;
diff --git a/src/catch2/internal/catch_assertion_handler.cpp b/src/catch2/internal/catch_assertion_handler.cpp
index b66a1456..e5232f70 100644
--- a/src/catch2/internal/catch_assertion_handler.cpp
+++ b/src/catch2/internal/catch_assertion_handler.cpp
@@ -23,7 +23,9 @@ namespace Catch {
             ResultDisposition::Flags resultDisposition )
     :   m_assertionInfo{ macroName, lineInfo, capturedExpression, resultDisposition },
         m_resultCapture( getResultCapture() )
-    {}
+    {
+        m_resultCapture.notifyAssertionStarted( m_assertionInfo );
+    }
 
     void AssertionHandler::handleExpr( ITransientExpression const& expr ) {
         m_resultCapture.handleExpr( m_assertionInfo, expr, m_reaction );
diff --git a/src/catch2/internal/catch_run_context.cpp b/src/catch2/internal/catch_run_context.cpp
index 6fd63450..c6171a5c 100644
--- a/src/catch2/internal/catch_run_context.cpp
+++ b/src/catch2/internal/catch_run_context.cpp
@@ -301,7 +301,13 @@ namespace Catch {
         m_lastAssertionInfo.capturedExpression = "{Unknown expression after the reported line}"_sr;
     }
 
-    bool RunContext::sectionStarted(StringRef sectionName, SourceLineInfo const& sectionLineInfo, Counts & assertions) {
+    void RunContext::notifyAssertionStarted( AssertionInfo const& info ) {
+        m_reporter->assertionStarting( info );
+    }
+
+    bool RunContext::sectionStarted( StringRef sectionName,
+                                     SourceLineInfo const& sectionLineInfo,
+                                     Counts& assertions ) {
         ITracker& sectionTracker =
             SectionTracker::acquire( m_trackerContext,
                                      TestCaseTracking::NameAndLocationRef(
@@ -561,8 +567,6 @@ namespace Catch {
         ITransientExpression const& expr,
         AssertionReaction& reaction
     ) {
-        m_reporter->assertionStarting( info );
-
         bool negated = isFalseTest( info.resultDisposition );
         bool result = expr.getResult() != negated;
 
@@ -600,8 +604,6 @@ namespace Catch {
             StringRef message,
             AssertionReaction& reaction
     ) {
-        m_reporter->assertionStarting( info );
-
         m_lastAssertionInfo = info;
 
         AssertionResultData data( resultType, LazyExpression( false ) );
diff --git a/src/catch2/internal/catch_run_context.hpp b/src/catch2/internal/catch_run_context.hpp
index fc65f651..76cdd84d 100644
--- a/src/catch2/internal/catch_run_context.hpp
+++ b/src/catch2/internal/catch_run_context.hpp
@@ -70,6 +70,7 @@ namespace Catch {
                     ResultWas::OfType resultType,
                     AssertionReaction &reaction ) override;
 
+        void notifyAssertionStarted( AssertionInfo const& info ) override;
         bool sectionStarted( StringRef sectionName,
                              SourceLineInfo const& sectionLineInfo,
                              Counts& assertions ) override;
diff --git a/tests/ExtraTests/CMakeLists.txt b/tests/ExtraTests/CMakeLists.txt
index 4172d7a0..2a810e25 100644
--- a/tests/ExtraTests/CMakeLists.txt
+++ b/tests/ExtraTests/CMakeLists.txt
@@ -468,6 +468,17 @@ set_tests_properties(
 )
 
 
+add_executable(AssertionStartingEventGoesBeforeAssertionIsEvaluated
+  X20-AssertionStartingEventGoesBeforeAssertionIsEvaluated.cpp
+)
+target_link_libraries(AssertionStartingEventGoesBeforeAssertionIsEvaluated
+  PRIVATE Catch2::Catch2WithMain
+)
+add_test(
+  NAME ReporterEvents::AssertionStartingHappensBeforeAssertionIsEvaluated
+  COMMAND $<TARGET_FILE:AssertionStartingEventGoesBeforeAssertionIsEvaluated>
+)
+
 #add_executable(DebugBreakMacros ${TESTS_DIR}/X12-CustomDebugBreakMacro.cpp)
 #target_link_libraries(DebugBreakMacros Catch2)
 #add_test(NAME DebugBreakMacros COMMAND DebugBreakMacros --break)
diff --git a/tests/ExtraTests/X20-AssertionStartingEventGoesBeforeAssertionIsEvaluated.cpp b/tests/ExtraTests/X20-AssertionStartingEventGoesBeforeAssertionIsEvaluated.cpp
new file mode 100644
index 00000000..ef5b46b9
--- /dev/null
+++ b/tests/ExtraTests/X20-AssertionStartingEventGoesBeforeAssertionIsEvaluated.cpp
@@ -0,0 +1,81 @@
+
+//              Copyright Catch2 Authors
+// Distributed under the Boost Software License, Version 1.0.
+//   (See accompanying file LICENSE.txt or copy at
+//        https://www.boost.org/LICENSE_1_0.txt)
+
+// SPDX-License-Identifier: BSL-1.0
+
+/**\file
+ * TODO: FIXES Registers custom reporter that reports testCase* events
+ *
+ * The resulting executable can then be used by an external Python script
+ * to verify that testCase{Starting,Ended} and testCasePartial{Starting,Ended}
+ * events are properly nested.
+ */
+
+#include <catch2/catch_test_macros.hpp>
+#include <catch2/reporters/catch_reporter_event_listener.hpp>
+#include <catch2/reporters/catch_reporter_registrars.hpp>
+#include <catch2/matchers/catch_matchers_predicate.hpp>
+
+namespace {
+
+    static size_t assertion_starting_events_seen = 0;
+
+    // TODO: custom matcher to check that "assertion_starting_events_seen" has
+    // the right number of checks
+
+    class AssertionStartingListener : public Catch::EventListenerBase {
+    public:
+        AssertionStartingListener( Catch::IConfig const* config ):
+            EventListenerBase( config ) {}
+
+        void assertionStarting( Catch::AssertionInfo const& ) override {
+            ++assertion_starting_events_seen;
+        }
+    };
+
+    static bool f1() {
+        return assertion_starting_events_seen == 1;
+    }
+
+    static void f2() {
+        if ( assertion_starting_events_seen != 2 ) { throw 1; }
+    }
+
+    static void f3() {
+        if ( assertion_starting_events_seen == 3 ) { throw 1; }
+    }
+
+    static bool f4() { return assertion_starting_events_seen == 4; }
+
+    static void f5() { throw assertion_starting_events_seen; }
+
+} // anonymous namespace
+
+CATCH_REGISTER_LISTENER( AssertionStartingListener )
+
+TEST_CASE() {
+    // **IMPORTANT**
+    // The order of assertions below matters.
+    REQUIRE( f1() );
+    REQUIRE_NOTHROW( f2() );
+    REQUIRE_THROWS( f3() );
+    REQUIRE_THAT( f4(),
+                  Catch::Matchers::Predicate<bool>( []( bool b ) { return b; } ) );
+    REQUIRE_THROWS_MATCHES(
+        f5(), size_t, Catch::Matchers::Predicate<size_t>( []( size_t i ) {
+            return i == 5;
+        } ) );
+
+    CAPTURE( assertion_starting_events_seen ); // **not** an assertion
+    INFO( "some info msg" );                   // **not** an assertion
+    WARN( "warning! warning!" );               // assertion-like message
+    SUCCEED();                                 // assertion-like message
+
+    // We skip FAIL/SKIP and so on, which fail the test.
+
+    // This require will also increment the count once
+    REQUIRE( assertion_starting_events_seen == 8 );
+}