Add notify_exception_handled() for explicit clearing of held state; f…

…ixes #179 too
snitch-org · Sep 29, 2024 · 4b42dd2 · 4b42dd2
1 parent 2f0f16c
commit 4b42dd2
Show file tree

Hide file tree

Showing 22 changed files with 393 additions and 45 deletions.
diff --git a/README.md b/README.md
@@ -16,7 +16,7 @@ The goal of _snitch_ is to be a simple, cheap, non-invasive, and user-friendly t
     - [Using _snitch_ as a regular library](#using-snitch-as-a-regular-library)
     - [Using _snitch_ as a header-only library](#using-snitch-as-a-header-only-library)
 - [Example build configuration with meson](#example-build-configuration-with-meson)
-- [Example build configuration with vcpkg](https://github.com/snitch-org/snitch/tree/main/doc/vcpkg-example/README.md)
+- [Example build configuration with vcpkg](#example-build-configuration-with-vcpkg)
 - [Benchmark](#benchmark)
 - [Documentation](#documentation)
     - [Detailed comparison with _Catch2_](#detailed-comparison-with-catch2)
@@ -29,6 +29,7 @@ The goal of _snitch_ is to be a simple, cheap, non-invasive, and user-friendly t
         - [Run-time and compile-time](#run-time-and-compile-time)
         - [Exception checks](#exception-checks)
         - [Miscellaneous](#miscellaneous)
+    - [Advanced API](#advanced-api)
     - [Tags](#tags)
     - [Matchers](#matchers)
     - [Sections](#sections)
@@ -214,6 +215,11 @@ then you can configure with:
 And this disables the build step that generates the single-header file "`snitch_all.hpp`".
 
 
+## Example build configuration with vcpkg
+
+See [the dedicated page in the docs folder](doc/vcpkg-example/README.md).
+
+
 ## Benchmark
 
 The following benchmarks were done using real-world tests from another library ([_observable_unique_ptr_](https://github.com/cschreib/observable_unique_ptr)), which generates about 4000 test cases and 25000 checks. This library uses "typed" tests almost exclusively, where each test case is instantiated several times, each time with a different tested type (here, 25 types). Building and running the tests was done without parallelism to simplify the comparison. The benchmarks were run on a desktop with the following specs:
@@ -476,6 +482,13 @@ This reports the current test case as "skipped". Any previously reported status
 This is similar to `SKIP`, except that the test case continues. Further failure will not be reported. This is only recommended as an alternative to `SKIP()` when exceptions cannot be used.
 
 
+### Advanced API
+
+`snitch::notify_exception_handled();`
+
+If handling exceptions explicitly with a `try/catch` block in a test case, this should be called at the end of the `catch` block. This clears up internal state that would have been used to report that exception, had it not been handled. Calling this is not strictly necessary in most cases, but omitting it can lead to confusing contextual data (incorrect section/capture/info) if another exception is thrown afterwards and not handled.
+
+
 ### Tags
 
 Tags are assigned to each test case using the [Test case macros](#test-case-macros), as a single string. Within this string, individual tags must be surrounded by square brackets, with no white-space between tags (although white space within a tag is allowed). For example:

diff --git a/include/snitch/snitch_macros_exceptions.hpp b/include/snitch/snitch_macros_exceptions.hpp
@@ -17,6 +17,7 @@
                 SNITCH_NO_EXCEPTION_THROWN = true;                                                 \
             } catch (const __VA_ARGS__&) {                                                         \
                 snitch::registry::report_assertion(true, #__VA_ARGS__ " was thrown as expected");  \
+                snitch::notify_exception_handled();                                                \
             } catch (...) {                                                                        \
                 try {                                                                              \
                     throw;                                                                         \
@@ -25,12 +26,12 @@
                         false,                                                                     \
                         #__VA_ARGS__ " expected but other std::exception thrown; message: ",       \
                         e.what());                                                                 \
-                    MAYBE_ABORT;                                                                   \
                 } catch (...) {                                                                    \
                     snitch::registry::report_assertion(                                            \
                         false, #__VA_ARGS__ " expected but other unknown exception thrown");       \
-                    MAYBE_ABORT;                                                                   \
                 }                                                                                  \
+                snitch::notify_exception_handled();                                                \
+                MAYBE_ABORT;                                                                       \
             }                                                                                      \
             if (SNITCH_NO_EXCEPTION_THROWN) {                                                      \
                 snitch::registry::report_assertion(                                                \
@@ -58,12 +59,14 @@
                         false, "could not match caught " #EXCEPTION " with expected content: ",    \
                         SNITCH_TEMP_MATCHER.describe_match(                                        \
                             e, snitch::matchers::match_status::failed));                           \
+                    snitch::notify_exception_handled();                                            \
                     MAYBE_ABORT;                                                                   \
                 } else {                                                                           \
                     snitch::registry::report_assertion(                                            \
                         true, "caught " #EXCEPTION " matched expected content: ",                  \
                         SNITCH_TEMP_MATCHER.describe_match(                                        \
                             e, snitch::matchers::match_status::matched));                          \
+                    snitch::notify_exception_handled();                                            \
                 }                                                                                  \
             } catch (...) {                                                                        \
                 try {                                                                              \
@@ -72,12 +75,12 @@
                     snitch::registry::report_assertion(                                            \
                         false, #EXCEPTION " expected but other std::exception thrown; message: ",  \
                         e.what());                                                                 \
-                    MAYBE_ABORT;                                                                   \
                 } catch (...) {                                                                    \
                     snitch::registry::report_assertion(                                            \
                         false, #EXCEPTION " expected but other unknown exception thrown");         \
-                    MAYBE_ABORT;                                                                   \
                 }                                                                                  \
+                snitch::notify_exception_handled();                                                \
+                MAYBE_ABORT;                                                                       \
             }                                                                                      \
             if (SNITCH_NO_EXCEPTION_THROWN) {                                                      \
                 snitch::registry::report_assertion(                                                \
@@ -111,6 +114,7 @@
                         false, "expected " #__VA_ARGS__                                            \
                                " not to throw but it threw an unknown exception");                 \
                 }                                                                                  \
+                snitch::notify_exception_handled();                                                \
                 MAYBE_ABORT;                                                                       \
             }                                                                                      \
         } while (0)

diff --git a/include/snitch/snitch_test_data.hpp b/include/snitch/snitch_test_data.hpp
@@ -374,4 +374,18 @@ struct scoped_test_check {
 };
 } // namespace snitch::impl
 
+namespace snitch {
+#if SNITCH_WITH_EXCEPTIONS
+/*! \brief Notify the testing framework that an exception was manually handled.
+ * \details If handling exceptions explicitly with a `try/catch` block in a test case,
+ * this should be called at the end of the `catch` block. This clears up internal state
+ * that would have been used to report that exception, had it not been handled. Calling
+ * this is not strictly necessary in most cases, but omitting it can lead to confusing
+ * contextual data (incorrect section/capture/info) if another exception is thrown afterwards
+ * and not handled.
+ */
+SNITCH_EXPORT void notify_exception_handled() noexcept;
+#endif
+} // namespace snitch
+
 #endif
diff --git a/src/snitch_capture.cpp b/src/snitch_capture.cpp
@@ -92,7 +92,9 @@ small_string<max_capture_length>& add_capture(test_state& state) {
     }
 
 #if SNITCH_WITH_EXCEPTIONS
-    state.held_info.reset();
+    if (std::uncaught_exceptions() == 0) {
+        notify_exception_handled();
+    }
 #endif
 
     state.info.captures.grow(1);

diff --git a/src/snitch_registry.cpp b/src/snitch_registry.cpp
@@ -456,7 +456,8 @@ void report_assertion_impl(
     register_assertion(success, state);
 
 #if SNITCH_WITH_EXCEPTIONS
-    const bool use_held_info = state.unhandled_exception && state.held_info.has_value();
+    const bool use_held_info = (state.unhandled_exception || std::uncaught_exceptions() > 0) &&
+                               state.held_info.has_value();
 
     const auto captures_buffer = impl::make_capture_buffer(
         use_held_info ? state.held_info.value().captures : state.info.captures);
@@ -655,15 +656,7 @@ impl::test_state registry::run(impl::test_case& test) noexcept {
     }
 
     if (state.unhandled_exception) {
-        auto& current_section = state.held_info.has_value()
-                                    ? state.held_info.value().sections.current_section
-                                    : state.info.sections.current_section;
-
-        // Close all sections that were left open by the exception.
-        while (!current_section.empty()) {
-            registry::report_section_ended(current_section.back());
-            current_section.pop_back();
-        }
+        notify_exception_handled();
     }
 
     state.unhandled_exception = false;

diff --git a/src/snitch_section.cpp b/src/snitch_section.cpp
@@ -71,7 +71,9 @@ section_entry_checker::~section_entry_checker() {
 
 section_entry_checker::operator bool() {
 #if SNITCH_WITH_EXCEPTIONS
-    state.held_info.reset();
+    if (std::uncaught_exceptions() == 0) {
+        notify_exception_handled();
+    }
 #endif
 
     auto& sections = state.info.sections;

diff --git a/src/snitch_test_data.cpp b/src/snitch_test_data.cpp
@@ -1,5 +1,7 @@
 #include "snitch/snitch_test_data.hpp"
 
+#include "snitch/snitch_registry.hpp"
+
 #if SNITCH_WITH_EXCEPTIONS
 #    include <exception>
 #endif
@@ -36,8 +38,11 @@ void pop_location(test_state& test) noexcept {
 
 scoped_test_check::scoped_test_check(const source_location& location) noexcept :
     test(get_current_test()) {
+
 #if SNITCH_WITH_EXCEPTIONS
-    test.held_info.reset();
+    if (std::uncaught_exceptions() == 0) {
+        notify_exception_handled();
+    }
 #endif
 
     push_location(test, {location.file, location.line, location_type::in_check});
@@ -59,3 +64,24 @@ scoped_test_check::~scoped_test_check() noexcept {
     pop_location(test);
 }
 } // namespace snitch::impl
+
+namespace snitch {
+#if SNITCH_WITH_EXCEPTIONS
+void notify_exception_handled() noexcept {
+    auto& state = impl::get_current_test();
+    if (!state.held_info.has_value()) {
+        return;
+    }
+
+    // Close all sections that were left open by the exception.
+    auto&       current_held_section = state.held_info.value().sections.current_section;
+    const auto& current_section      = state.info.sections.current_section;
+    while (current_held_section.size() > current_section.size()) {
+        registry::report_section_ended(current_held_section.back());
+        current_held_section.pop_back();
+    }
+
+    state.held_info.reset();
+}
+#endif
+} // namespace snitch
diff --git a/tests/approval_tests/data/expected/reporter_catch2_xml_default b/tests/approval_tests/data/expected/reporter_catch2_xml_default
@@ -250,6 +250,15 @@
     </Section>
     <OverallResult success="false" skips="0" durationInSeconds="*"/>
   </TestCase>
+  <TestCase name="test unexpected throw in check &amp; section mayfail" tags="[!mayfail]" filename="*testing_reporters.cpp" line="*">
+    <Section name="section 1" filename="*testing_reporters.cpp" line="*">
+      <Failure filename="*testing_reporters.cpp" line="*">
+        unexpected std::exception caught; message: unexpected error
+      </Failure>
+      <OverallResults successes="0" failures="0" expectedFailures="1" skipped="false" durationInSeconds="*"/>
+    </Section>
+    <OverallResult success="true" skips="0" durationInSeconds="*"/>
+  </TestCase>
   <TestCase name="test SKIP" tags="" filename="*testing_reporters.cpp" line="*">
     <Skip filename="*testing_reporters.cpp" line="*">
       not interesting
@@ -301,6 +310,15 @@
     </Section>
     <OverallResult success="false" skips="0" durationInSeconds="*"/>
   </TestCase>
+  <TestCase name="test SECTION mayfail" tags="[!mayfail]" filename="*testing_reporters.cpp" line="*">
+    <Section name="section" filename="*testing_reporters.cpp" line="*">
+      <Failure filename="*testing_reporters.cpp" line="*">
+        failure
+      </Failure>
+      <OverallResults successes="0" failures="0" expectedFailures="1" skipped="false" durationInSeconds="*"/>
+    </Section>
+    <OverallResult success="true" skips="0" durationInSeconds="*"/>
+  </TestCase>
   <TestCase name="test multiple SECTION" tags="" filename="*testing_reporters.cpp" line="*">
     <Section name="section 1" filename="*testing_reporters.cpp" line="*">
       <Failure filename="*testing_reporters.cpp" line="*">
@@ -451,6 +469,6 @@
     </Failure>
     <OverallResult success="false" skips="0" durationInSeconds="*"/>
   </TestCase>
-  <OverallResults successes="43" failures="48" expectedFailures="2" skips="2"/>
-  <OverallResultsCases successes="16" failures="28" expectedFailures="2" skips="2"/>
+  <OverallResults successes="44" failures="48" expectedFailures="4" skips="2"/>
+  <OverallResultsCases successes="16" failures="28" expectedFailures="4" skips="2"/>
 </Catch2TestRun>
diff --git a/tests/approval_tests/data/expected/reporter_catch2_xml_full b/tests/approval_tests/data/expected/reporter_catch2_xml_full
@@ -376,6 +376,15 @@
     </Section>
     <OverallResult success="false" skips="0" durationInSeconds="*"/>
   </TestCase>
+  <TestCase name="test unexpected throw in check &amp; section mayfail" tags="[!mayfail]" filename="*testing_reporters.cpp" line="*">
+    <Section name="section 1" filename="*testing_reporters.cpp" line="*">
+      <Failure filename="*testing_reporters.cpp" line="*">
+        unexpected std::exception caught; message: unexpected error
+      </Failure>
+      <OverallResults successes="0" failures="0" expectedFailures="1" skipped="false" durationInSeconds="*"/>
+    </Section>
+    <OverallResult success="true" skips="0" durationInSeconds="*"/>
+  </TestCase>
   <TestCase name="test SKIP" tags="" filename="*testing_reporters.cpp" line="*">
     <Skip filename="*testing_reporters.cpp" line="*">
       not interesting
@@ -436,6 +445,18 @@
     </Success>
     <OverallResult success="false" skips="0" durationInSeconds="*"/>
   </TestCase>
+  <TestCase name="test SECTION mayfail" tags="[!mayfail]" filename="*testing_reporters.cpp" line="*">
+    <Section name="section" filename="*testing_reporters.cpp" line="*">
+      <Failure filename="*testing_reporters.cpp" line="*">
+        failure
+      </Failure>
+      <OverallResults successes="0" failures="0" expectedFailures="1" skipped="false" durationInSeconds="*"/>
+    </Section>
+    <Success filename="*testing_reporters.cpp" line="*">
+      no exception caught
+    </Success>
+    <OverallResult success="true" skips="0" durationInSeconds="*"/>
+  </TestCase>
   <TestCase name="test multiple SECTION" tags="" filename="*testing_reporters.cpp" line="*">
     <Section name="section 1" filename="*testing_reporters.cpp" line="*">
       <Failure filename="*testing_reporters.cpp" line="*">
@@ -595,6 +616,6 @@
     </Failure>
     <OverallResult success="false" skips="0" durationInSeconds="*"/>
   </TestCase>
-  <OverallResults successes="43" failures="48" expectedFailures="2" skips="2"/>
-  <OverallResultsCases successes="16" failures="28" expectedFailures="2" skips="2"/>
+  <OverallResults successes="44" failures="48" expectedFailures="4" skips="2"/>
+  <OverallResultsCases successes="16" failures="28" expectedFailures="4" skips="2"/>
 </Catch2TestRun>
diff --git a/tests/approval_tests/data/expected/reporter_catch2_xml_list_tests b/tests/approval_tests/data/expected/reporter_catch2_xml_list_tests
@@ -324,6 +324,15 @@
       <Line>*</Line>
     </SourceInfo>
   </TestCase>
+  <TestCase>
+    <Name>test unexpected throw in check &amp; section mayfail</Name>
+    <ClassName/>
+    <Tags>[!mayfail]</Tags>
+    <SourceInfo>
+      <File>*testing_reporters.cpp</File>
+      <Line>*</Line>
+    </SourceInfo>
+  </TestCase>
   <TestCase>
     <Name>test SKIP</Name>
     <ClassName/>
@@ -360,6 +369,15 @@
       <Line>*</Line>
     </SourceInfo>
   </TestCase>
+  <TestCase>
+    <Name>test SECTION mayfail</Name>
+    <ClassName/>
+    <Tags>[!mayfail]</Tags>
+    <SourceInfo>
+      <File>*testing_reporters.cpp</File>
+      <Line>*</Line>
+    </SourceInfo>
+  </TestCase>
   <TestCase>
     <Name>test multiple SECTION</Name>
     <ClassName/>

diff --git a/tests/approval_tests/data/expected/reporter_console_default b/tests/approval_tests/data/expected/reporter_console_default
@@ -80,6 +80,10 @@ failed: running test case "test unexpected throw in check & section fail"
           in section "section 1"
           somewhere inside check at *testing_reporters.cpp:*
           unexpected std::exception caught; message: unexpected error
+allowed failure: running test case "test unexpected throw in check & section mayfail"
+          in section "section 1"
+          somewhere inside check at *testing_reporters.cpp:*
+          unexpected std::exception caught; message: unexpected error
 skipped: running test case "test SKIP"
           at *testing_reporters.cpp:*
           not interesting
@@ -107,6 +111,10 @@ failed: running test case "test SECTION"
           in section "section"
           at *testing_reporters.cpp:*
           failure
+allowed failure: running test case "test SECTION mayfail"
+          in section "section"
+          at *testing_reporters.cpp:*
+          failure
 failed: running test case "test multiple SECTION"
           in section "section 1"
           at *testing_reporters.cpp:*
@@ -195,4 +203,4 @@ skipped: running test case "test SKIP in SECTION"
           at *testing_reporters.cpp:*
           stopping here
 ==========================================
-error: some tests failed (26 out of 44 test cases, 91 assertions, 2 test cases skipped, * seconds)
+error: some tests failed (26 out of 46 test cases, 94 assertions, 2 test cases skipped, * seconds)