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 $ +) + #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 +#include +#include +#include + +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 b ) { return b; } ) ); + REQUIRE_THROWS_MATCHES( + f5(), size_t, Catch::Matchers::Predicate( []( 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 ); +}