Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assert Info reset need to also reset result disposition to normal to handle uncaught exception correctly #2723

Merged
merged 4 commits into from
Aug 7, 2023
Merged

Assert Info reset need to also reset result disposition to normal to handle uncaught exception correctly #2723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
26fa6b7
AssertionEnd does not reset the assertion info yet. That is done afte…
Jul 21, 2023
d92fa76
Approving test output changes due to added unit tests
Jul 21, 2023
3b50218
Unit tests to throw std::runtime_error instead of std::exception
Jul 22, 2023
52f9ea7
Add a unit test to test incomplete assertion handler
Jul 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/catch2/internal/catch_run_context.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <catch2/internal/catch_output_redirect.hpp>
#include <catch2/internal/catch_assertion_handler.hpp>
#include <catch2/internal/catch_test_failure_exception.hpp>
#include <catch2/internal/catch_result_type.hpp>

#include <cassert>
#include <algorithm>
Expand Down Expand Up @@ -293,13 +294,14 @@ namespace Catch {
m_messageScopes.clear();
}

// Reset working state
resetAssertionInfo();
// Reset working state. assertion info will be reset after
// populateReaction is run if it is needed
m_lastResult = CATCH_MOVE( result );
}
void RunContext::resetAssertionInfo() {
m_lastAssertionInfo.macroName = StringRef();
m_lastAssertionInfo.capturedExpression = "{Unknown expression after the reported line}"_sr;
m_lastAssertionInfo.resultDisposition = ResultDisposition::Normal;
}

void RunContext::notifyAssertionStarted( AssertionInfo const& info ) {
Expand Down Expand Up @@ -447,6 +449,7 @@ namespace Catch {
AssertionResult result(m_lastAssertionInfo, CATCH_MOVE(tempResult));

assertionEnded(CATCH_MOVE(result) );
resetAssertionInfo();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually neeeded? RunContext::assertionEnded (called on the line before this one) already calls resetAssertionInfo.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait sorry, I didn't notice that you changed assertionEnded to not reset assertion info anymore.


handleUnfinishedSections();

Expand Down Expand Up @@ -583,6 +586,7 @@ namespace Catch {
reportExpr(info, ResultWas::ExpressionFailed, &expr, negated );
populateReaction( reaction );
}
resetAssertionInfo();
}
void RunContext::reportExpr(
AssertionInfo const &info,
Expand Down Expand Up @@ -621,6 +625,7 @@ namespace Catch {
// considered "OK"
reaction.shouldSkip = true;
}
resetAssertionInfo();
}
void RunContext::handleUnexpectedExceptionNotThrown(
AssertionInfo const& info,
Expand All @@ -641,6 +646,7 @@ namespace Catch {
AssertionResult assertionResult{ info, CATCH_MOVE(data) };
assertionEnded( CATCH_MOVE(assertionResult) );
populateReaction( reaction );
resetAssertionInfo();
}

void RunContext::populateReaction( AssertionReaction& reaction ) {
Expand All @@ -658,6 +664,7 @@ namespace Catch {
data.message = "Exception translation was disabled by CATCH_CONFIG_FAST_COMPILE"s;
AssertionResult assertionResult{ info, CATCH_MOVE( data ) };
assertionEnded( CATCH_MOVE(assertionResult) );
resetAssertionInfo();
}
void RunContext::handleNonExpr(
AssertionInfo const &info,
Expand All @@ -672,6 +679,7 @@ namespace Catch {
const auto isOk = assertionResult.isOk();
assertionEnded( CATCH_MOVE(assertionResult) );
if ( !isOk ) { populateReaction( reaction ); }
resetAssertionInfo();
}


Expand Down
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ endif(MSVC) #Temporary workaround
set(TEST_SOURCES
${SELF_TEST_DIR}/TestRegistrations.cpp
${SELF_TEST_DIR}/IntrospectiveTests/Algorithms.tests.cpp
${SELF_TEST_DIR}/IntrospectiveTests/AssertionHandler.tests.cpp
${SELF_TEST_DIR}/IntrospectiveTests/Clara.tests.cpp
${SELF_TEST_DIR}/IntrospectiveTests/CmdLine.tests.cpp
${SELF_TEST_DIR}/IntrospectiveTests/CmdLineHelpers.tests.cpp
Expand Down
3 changes: 3 additions & 0 deletions tests/SelfTest/Baselines/automake.sw.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ Nor would this
:test-result: FAIL INFO gets logged on failure
:test-result: FAIL INFO gets logged on failure, even if captured before successful assertions
:test-result: FAIL INFO is reset for each loop
:test-result: XFAIL Incomplete AssertionHandler
:test-result: XFAIL Inequality checks that should fail
:test-result: PASS Inequality checks that should succeed
:test-result: PASS Lambdas in assertions
Expand Down Expand Up @@ -265,6 +266,8 @@ Message from section two
:test-result: PASS Testing checked-if
:test-result: XFAIL Testing checked-if 2
:test-result: XFAIL Testing checked-if 3
:test-result: XFAIL Testing checked-if 4
:test-result: XFAIL Testing checked-if 5
:test-result: FAIL The NO_FAIL macro reports a failure but does not fail the test
:test-result: PASS The default listing implementation write to provided stream
:test-result: FAIL This test 'should' fail but doesn't
Expand Down
3 changes: 3 additions & 0 deletions tests/SelfTest/Baselines/automake.sw.multi.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@
:test-result: FAIL INFO gets logged on failure
:test-result: FAIL INFO gets logged on failure, even if captured before successful assertions
:test-result: FAIL INFO is reset for each loop
:test-result: XFAIL Incomplete AssertionHandler
:test-result: XFAIL Inequality checks that should fail
:test-result: PASS Inequality checks that should succeed
:test-result: PASS Lambdas in assertions
Expand Down Expand Up @@ -258,6 +259,8 @@
:test-result: PASS Testing checked-if
:test-result: XFAIL Testing checked-if 2
:test-result: XFAIL Testing checked-if 3
:test-result: XFAIL Testing checked-if 4
:test-result: XFAIL Testing checked-if 5
:test-result: FAIL The NO_FAIL macro reports a failure but does not fail the test
:test-result: PASS The default listing implementation write to provided stream
:test-result: FAIL This test 'should' fail but doesn't
Expand Down
9 changes: 7 additions & 2 deletions tests/SelfTest/Baselines/compact.sw.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,7 @@ Message.tests.cpp:<line number>: passed: i < 10 for: 7 < 10 with 2 messages: 'cu
Message.tests.cpp:<line number>: passed: i < 10 for: 8 < 10 with 2 messages: 'current counter 8' and 'i := 8'
Message.tests.cpp:<line number>: passed: i < 10 for: 9 < 10 with 2 messages: 'current counter 9' and 'i := 9'
Message.tests.cpp:<line number>: failed: i < 10 for: 10 < 10 with 2 messages: 'current counter 10' and 'i := 10'
AssertionHandler.tests.cpp:<line number>: failed: unexpected exception with message: 'Exception translation was disabled by CATCH_CONFIG_FAST_COMPILE'; expression was: Dummy
Condition.tests.cpp:<line number>: failed: data.int_seven != 7 for: 7 != 7
Condition.tests.cpp:<line number>: failed: data.float_nine_point_one != Approx( 9.1f ) for: 9.1f != Approx( 9.1000003815 )
Condition.tests.cpp:<line number>: failed: data.double_pi != Approx( 3.1415926535 ) for: 3.1415926535 != Approx( 3.1415926535 )
Expand Down Expand Up @@ -1750,6 +1751,10 @@ Misc.tests.cpp:<line number>: passed: true
Misc.tests.cpp:<line number>: failed: explicitly
Misc.tests.cpp:<line number>: failed - but was ok: false
Misc.tests.cpp:<line number>: failed: explicitly
Misc.tests.cpp:<line number>: passed: true
Misc.tests.cpp:<line number>: failed: unexpected exception with message: 'Uncaught exception should fail!'; expression was: {Unknown expression after the reported line}
Misc.tests.cpp:<line number>: failed - but was ok: false
Misc.tests.cpp:<line number>: failed: unexpected exception with message: 'Uncaught exception should fail!'; expression was: {Unknown expression after the reported line}
Message.tests.cpp:<line number>: failed - but was ok: 1 == 2
Reporters.tests.cpp:<line number>: passed: listingString, ContainsSubstring("[fakeTag]"s) for: "All available tags:
1 [fakeTag]
Expand Down Expand Up @@ -2538,7 +2543,7 @@ InternalBenchmark.tests.cpp:<line number>: passed: med == 18. for: 18.0 == 18.0
InternalBenchmark.tests.cpp:<line number>: passed: q3 == 23. for: 23.0 == 23.0
Misc.tests.cpp:<line number>: passed:
Misc.tests.cpp:<line number>: passed:
test cases: 409 | 308 passed | 84 failed | 6 skipped | 11 failed as expected
assertions: 2225 | 2048 passed | 145 failed | 32 failed as expected
test cases: 412 | 308 passed | 84 failed | 6 skipped | 14 failed as expected
assertions: 2229 | 2049 passed | 145 failed | 35 failed as expected


9 changes: 7 additions & 2 deletions tests/SelfTest/Baselines/compact.sw.multi.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -959,6 +959,7 @@ Message.tests.cpp:<line number>: passed: i < 10 for: 7 < 10 with 2 messages: 'cu
Message.tests.cpp:<line number>: passed: i < 10 for: 8 < 10 with 2 messages: 'current counter 8' and 'i := 8'
Message.tests.cpp:<line number>: passed: i < 10 for: 9 < 10 with 2 messages: 'current counter 9' and 'i := 9'
Message.tests.cpp:<line number>: failed: i < 10 for: 10 < 10 with 2 messages: 'current counter 10' and 'i := 10'
AssertionHandler.tests.cpp:<line number>: failed: unexpected exception with message: 'Exception translation was disabled by CATCH_CONFIG_FAST_COMPILE'; expression was: Dummy
Condition.tests.cpp:<line number>: failed: data.int_seven != 7 for: 7 != 7
Condition.tests.cpp:<line number>: failed: data.float_nine_point_one != Approx( 9.1f ) for: 9.1f != Approx( 9.1000003815 )
Condition.tests.cpp:<line number>: failed: data.double_pi != Approx( 3.1415926535 ) for: 3.1415926535 != Approx( 3.1415926535 )
Expand Down Expand Up @@ -1743,6 +1744,10 @@ Misc.tests.cpp:<line number>: passed: true
Misc.tests.cpp:<line number>: failed: explicitly
Misc.tests.cpp:<line number>: failed - but was ok: false
Misc.tests.cpp:<line number>: failed: explicitly
Misc.tests.cpp:<line number>: passed: true
Misc.tests.cpp:<line number>: failed: unexpected exception with message: 'Uncaught exception should fail!'; expression was: {Unknown expression after the reported line}
Misc.tests.cpp:<line number>: failed - but was ok: false
Misc.tests.cpp:<line number>: failed: unexpected exception with message: 'Uncaught exception should fail!'; expression was: {Unknown expression after the reported line}
Message.tests.cpp:<line number>: failed - but was ok: 1 == 2
Reporters.tests.cpp:<line number>: passed: listingString, ContainsSubstring("[fakeTag]"s) for: "All available tags:
1 [fakeTag]
Expand Down Expand Up @@ -2527,7 +2532,7 @@ InternalBenchmark.tests.cpp:<line number>: passed: med == 18. for: 18.0 == 18.0
InternalBenchmark.tests.cpp:<line number>: passed: q3 == 23. for: 23.0 == 23.0
Misc.tests.cpp:<line number>: passed:
Misc.tests.cpp:<line number>: passed:
test cases: 409 | 308 passed | 84 failed | 6 skipped | 11 failed as expected
assertions: 2225 | 2048 passed | 145 failed | 32 failed as expected
test cases: 412 | 308 passed | 84 failed | 6 skipped | 14 failed as expected
assertions: 2229 | 2049 passed | 145 failed | 35 failed as expected


37 changes: 35 additions & 2 deletions tests/SelfTest/Baselines/console.std.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,17 @@ with messages:
current counter 10
i := 10

-------------------------------------------------------------------------------
Incomplete AssertionHandler
-------------------------------------------------------------------------------
AssertionHandler.tests.cpp:<line number>
...............................................................................

AssertionHandler.tests.cpp:<line number>: FAILED:
REQUIRE( Dummy )
due to unexpected exception with message:
Exception translation was disabled by CATCH_CONFIG_FAST_COMPILE

-------------------------------------------------------------------------------
Inequality checks that should fail
-------------------------------------------------------------------------------
Expand Down Expand Up @@ -997,6 +1008,28 @@ Misc.tests.cpp:<line number>

Misc.tests.cpp:<line number>: FAILED:

-------------------------------------------------------------------------------
Testing checked-if 4
-------------------------------------------------------------------------------
Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: FAILED:
{Unknown expression after the reported line}
due to unexpected exception with message:
Uncaught exception should fail!

-------------------------------------------------------------------------------
Testing checked-if 5
-------------------------------------------------------------------------------
Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: FAILED:
{Unknown expression after the reported line}
due to unexpected exception with message:
Uncaught exception should fail!

-------------------------------------------------------------------------------
Thrown string literals are translated
-------------------------------------------------------------------------------
Expand Down Expand Up @@ -1543,6 +1576,6 @@ due to unexpected exception with message:
Why would you throw a std::string?

===============================================================================
test cases: 409 | 322 passed | 69 failed | 7 skipped | 11 failed as expected
assertions: 2208 | 2048 passed | 128 failed | 32 failed as expected
test cases: 412 | 322 passed | 69 failed | 7 skipped | 14 failed as expected
assertions: 2212 | 2049 passed | 128 failed | 35 failed as expected

43 changes: 41 additions & 2 deletions tests/SelfTest/Baselines/console.sw.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7143,6 +7143,17 @@ with messages:
current counter 10
i := 10

-------------------------------------------------------------------------------
Incomplete AssertionHandler
-------------------------------------------------------------------------------
AssertionHandler.tests.cpp:<line number>
...............................................................................

AssertionHandler.tests.cpp:<line number>: FAILED:
REQUIRE( Dummy )
due to unexpected exception with message:
Exception translation was disabled by CATCH_CONFIG_FAST_COMPILE

-------------------------------------------------------------------------------
Inequality checks that should fail
-------------------------------------------------------------------------------
Expand Down Expand Up @@ -12522,6 +12533,34 @@ Misc.tests.cpp:<line number>: FAILED - but was ok:

Misc.tests.cpp:<line number>: FAILED:

-------------------------------------------------------------------------------
Testing checked-if 4
-------------------------------------------------------------------------------
Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: PASSED:
CHECKED_ELSE( true )

Misc.tests.cpp:<line number>: FAILED:
{Unknown expression after the reported line}
due to unexpected exception with message:
Uncaught exception should fail!

-------------------------------------------------------------------------------
Testing checked-if 5
-------------------------------------------------------------------------------
Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: FAILED - but was ok:
CHECKED_ELSE( false )

Misc.tests.cpp:<line number>: FAILED:
{Unknown expression after the reported line}
due to unexpected exception with message:
Uncaught exception should fail!

-------------------------------------------------------------------------------
The NO_FAIL macro reports a failure but does not fail the test
-------------------------------------------------------------------------------
Expand Down Expand Up @@ -18232,6 +18271,6 @@ Misc.tests.cpp:<line number>
Misc.tests.cpp:<line number>: PASSED:

===============================================================================
test cases: 409 | 308 passed | 84 failed | 6 skipped | 11 failed as expected
assertions: 2225 | 2048 passed | 145 failed | 32 failed as expected
test cases: 412 | 308 passed | 84 failed | 6 skipped | 14 failed as expected
assertions: 2229 | 2049 passed | 145 failed | 35 failed as expected

43 changes: 41 additions & 2 deletions tests/SelfTest/Baselines/console.sw.multi.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7141,6 +7141,17 @@ with messages:
current counter 10
i := 10

-------------------------------------------------------------------------------
Incomplete AssertionHandler
-------------------------------------------------------------------------------
AssertionHandler.tests.cpp:<line number>
...............................................................................

AssertionHandler.tests.cpp:<line number>: FAILED:
REQUIRE( Dummy )
due to unexpected exception with message:
Exception translation was disabled by CATCH_CONFIG_FAST_COMPILE

-------------------------------------------------------------------------------
Inequality checks that should fail
-------------------------------------------------------------------------------
Expand Down Expand Up @@ -12515,6 +12526,34 @@ Misc.tests.cpp:<line number>: FAILED - but was ok:

Misc.tests.cpp:<line number>: FAILED:

-------------------------------------------------------------------------------
Testing checked-if 4
-------------------------------------------------------------------------------
Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: PASSED:
CHECKED_ELSE( true )

Misc.tests.cpp:<line number>: FAILED:
{Unknown expression after the reported line}
due to unexpected exception with message:
Uncaught exception should fail!

-------------------------------------------------------------------------------
Testing checked-if 5
-------------------------------------------------------------------------------
Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: FAILED - but was ok:
CHECKED_ELSE( false )

Misc.tests.cpp:<line number>: FAILED:
{Unknown expression after the reported line}
due to unexpected exception with message:
Uncaught exception should fail!

-------------------------------------------------------------------------------
The NO_FAIL macro reports a failure but does not fail the test
-------------------------------------------------------------------------------
Expand Down Expand Up @@ -18221,6 +18260,6 @@ Misc.tests.cpp:<line number>
Misc.tests.cpp:<line number>: PASSED:

===============================================================================
test cases: 409 | 308 passed | 84 failed | 6 skipped | 11 failed as expected
assertions: 2225 | 2048 passed | 145 failed | 32 failed as expected
test cases: 412 | 308 passed | 84 failed | 6 skipped | 14 failed as expected
assertions: 2229 | 2049 passed | 145 failed | 35 failed as expected

Loading