From 7994241954f5324f4191ab9f4aa3c4b042125c54 Mon Sep 17 00:00:00 2001 From: Corentin Schreiber Date: Wed, 28 Aug 2024 09:02:22 +0100 Subject: [PATCH] Port the fix for #177 to SECTION as well --- include/snitch/snitch_capture.hpp | 17 +---- include/snitch/snitch_test_data.hpp | 13 ++-- src/snitch_capture.cpp | 16 ++--- src/snitch_registry.cpp | 56 ++++++++-------- src/snitch_section.cpp | 46 +++++++------ src/snitch_test_data.cpp | 16 +++-- tests/runtime_tests/capture.cpp | 6 ++ tests/runtime_tests/section.cpp | 100 ++++++++++++++++++++++++++++ 8 files changed, 192 insertions(+), 78 deletions(-) diff --git a/include/snitch/snitch_capture.hpp b/include/snitch/snitch_capture.hpp index 9d95b674..86a33cfa 100644 --- a/include/snitch/snitch_capture.hpp +++ b/include/snitch/snitch_capture.hpp @@ -11,10 +11,7 @@ namespace snitch::impl { struct scoped_capture { - capture_state& captures; -#if SNITCH_WITH_EXCEPTIONS - std::optional& held_captures; -#endif + test_state& state; std::size_t count = 0; SNITCH_EXPORT ~scoped_capture(); @@ -38,11 +35,7 @@ void add_capture(test_state& state, std::string_view& names, const T& arg) { template scoped_capture add_captures(test_state& state, std::string_view names, const Args&... args) { (add_capture(state, names, args), ...); -#if SNITCH_WITH_EXCEPTIONS - return {state.captures, state.held_captures, sizeof...(args)}; -#else - return {state.captures, sizeof...(args)}; -#endif + return {state, sizeof...(args)}; } // Requires: number of captures < max_captures. @@ -50,11 +43,7 @@ template scoped_capture add_info(test_state& state, const Args&... args) { auto& capture = add_capture(state); append_or_truncate(capture, args...); -#if SNITCH_WITH_EXCEPTIONS - return {state.captures, state.held_captures, 1}; -#else - return {state.captures, 1}; -#endif + return {state, 1}; } } // namespace snitch::impl diff --git a/include/snitch/snitch_test_data.hpp b/include/snitch/snitch_test_data.hpp index 31de353e..7d1d81a7 100644 --- a/include/snitch/snitch_test_data.hpp +++ b/include/snitch/snitch_test_data.hpp @@ -279,15 +279,20 @@ using capture_state = small_vector, max_capture // NB: +2 is because we need one for the test case location, and one for the check location using location_state = small_vector; -struct test_state { - registry& reg; - test_case& test; +struct info_state { section_state sections = {}; capture_state captures = {}; location_state locations = {}; +}; + +struct test_state { + registry& reg; + test_case& test; + + info_state info = {}; #if SNITCH_WITH_EXCEPTIONS - std::optional held_captures = {}; + std::optional held_info = {}; #endif std::size_t asserts = 0; diff --git a/src/snitch_capture.cpp b/src/snitch_capture.cpp index 9b27185c..0c406cc8 100644 --- a/src/snitch_capture.cpp +++ b/src/snitch_capture.cpp @@ -22,15 +22,15 @@ void trim(std::string_view& str, std::string_view patterns) noexcept { scoped_capture::~scoped_capture() { #if SNITCH_WITH_EXCEPTIONS - if (std::uncaught_exceptions() > 0 && !held_captures.has_value()) { + if (std::uncaught_exceptions() > 0 && !state.held_info.has_value()) { // We are unwinding the stack because an exception has been thrown; // keep a copy of the full capture state since we will want to preserve the information // when reporting the exception. - held_captures = captures; + state.held_info = state.info; } #endif - captures.resize(captures.size() - count); + state.info.captures.resize(state.info.captures.size() - count); } std::string_view extract_next_name(std::string_view& names) noexcept { @@ -82,7 +82,7 @@ std::string_view extract_next_name(std::string_view& names) noexcept { } small_string& add_capture(test_state& state) { - if (state.captures.available() == 0) { + if (state.info.captures.available() == 0) { state.reg.print( make_colored("error:", state.reg.with_color, color::fail), " max number of captures reached; " @@ -92,11 +92,11 @@ small_string& add_capture(test_state& state) { } #if SNITCH_WITH_EXCEPTIONS - state.held_captures.reset(); + state.held_info.reset(); #endif - state.captures.grow(1); - state.captures.back().clear(); - return state.captures.back(); + state.info.captures.grow(1); + state.info.captures.back().clear(); + return state.info.captures.back(); } } // namespace snitch::impl diff --git a/src/snitch_registry.cpp b/src/snitch_registry.cpp index df65fa8e..698c25a6 100644 --- a/src/snitch_registry.cpp +++ b/src/snitch_registry.cpp @@ -413,24 +413,27 @@ void report_assertion_impl( register_assertion(success, state); - const auto captures_buffer = #if SNITCH_WITH_EXCEPTIONS - impl::make_capture_buffer( - state.unhandled_exception && state.held_captures.has_value() - ? state.held_captures.value() - : state.captures); -#else - impl::make_capture_buffer(state.captures); -#endif + const bool use_held_info = state.unhandled_exception && state.held_info.has_value(); + + const auto captures_buffer = impl::make_capture_buffer( + use_held_info ? state.held_info.value().captures : state.info.captures); + + const auto& current_section = use_held_info ? state.held_info.value().sections.current_section + : state.info.sections.current_section; + + const auto& last_location = + use_held_info ? state.held_info.value().locations.back() : state.info.locations.back(); - const auto& last_location = state.locations.back(); -#if SNITCH_WITH_EXCEPTIONS const auto location = state.in_check ? assertion_location{last_location.file, last_location.line, location_type::exact} : last_location; #else - const auto location = + const auto captures_buffer = impl::make_capture_buffer(state.info.captures); + const auto& current_section = state.info.sections.current_section; + const auto& last_location = state.info.locations.back(); + const auto location = assertion_location{last_location.file, last_location.line, location_type::exact}; #endif @@ -438,14 +441,13 @@ void report_assertion_impl( if (r.verbose >= registry::verbosity::full) { r.report_callback( r, event::assertion_succeeded{ - state.test.id, state.sections.current_section, captures_buffer.span(), - location, data}); + state.test.id, current_section, captures_buffer.span(), location, data}); } } else { r.report_callback( r, event::assertion_failed{ - state.test.id, state.sections.current_section, captures_buffer.span(), location, - data, state.should_fail, state.may_fail}); + state.test.id, current_section, captures_buffer.span(), location, data, + state.should_fail, state.may_fail}); } } } // namespace @@ -482,13 +484,13 @@ void registry::report_skipped(std::string_view message) noexcept { impl::test_state& state = impl::get_current_test(); impl::set_state(state.test, impl::test_case_state::skipped); - const auto captures_buffer = impl::make_capture_buffer(state.captures); - const auto& location = state.locations.back(); + const auto captures_buffer = impl::make_capture_buffer(state.info.captures); + const auto& location = state.info.locations.back(); state.reg.report_callback( state.reg, event::test_case_skipped{ state.test.id, - state.sections.current_section, + state.info.sections.current_section, captures_buffer.span(), {location.file, location.line, location_type::exact}, message}); @@ -515,7 +517,7 @@ impl::test_state registry::run(impl::test_case& test) noexcept { impl::test_state state{ .reg = *this, .test = test, .may_fail = may_fail, .should_fail = should_fail}; - state.locations.push_back( + state.info.locations.push_back( {test.location.file, test.location.line, location_type::test_case_scope}); // Store previously running test, to restore it later. @@ -534,24 +536,24 @@ impl::test_state registry::run(impl::test_case& test) noexcept { do { // Reset section state. - state.sections.leaf_executed = false; - for (std::size_t i = 0; i < state.sections.levels.size(); ++i) { - state.sections.levels[i].current_section_id = 0; + state.info.sections.leaf_executed = false; + for (std::size_t i = 0; i < state.info.sections.levels.size(); ++i) { + state.info.sections.levels[i].current_section_id = 0; } // Run the test case. test.func(); - if (state.sections.levels.size() == 1) { + if (state.info.sections.levels.size() == 1) { // This test case contained sections; check if there are any more left to evaluate. - auto& child = state.sections.levels[0]; + auto& child = state.info.sections.levels[0]; if (child.previous_section_id == child.max_section_id) { // No more; clear the section state. - state.sections.levels.clear(); - state.sections.current_section.clear(); + state.info.sections.levels.clear(); + state.info.sections.current_section.clear(); } } - } while (!state.sections.levels.empty() && + } while (!state.info.sections.levels.empty() && state.test.state != impl::test_case_state::skipped); #if SNITCH_WITH_EXCEPTIONS diff --git a/src/snitch_section.cpp b/src/snitch_section.cpp index 0d0a7c26..bc56461d 100644 --- a/src/snitch_section.cpp +++ b/src/snitch_section.cpp @@ -9,31 +9,33 @@ namespace snitch::impl { section_entry_checker::~section_entry_checker() { + auto& sections = state.info.sections; + if (entered) { #if SNITCH_WITH_EXCEPTIONS - if (std::uncaught_exceptions() > 0) { + if (std::uncaught_exceptions() > 0 && !state.held_info.has_value()) { // We are unwinding the stack because an exception has been thrown; - // avoid touching the section state since we will want to report where - // the exception was thrown. - return; + // keep a copy of the full section state since we will want to preserve the information + // when reporting the exception. + state.held_info = state.info; } #endif pop_location(state); - if (state.sections.depth == state.sections.levels.size()) { + if (sections.depth == sections.levels.size()) { // We just entered this section, and there was no child section in it. // This is a leaf; flag that a leaf has been executed so that no other leaf // is executed in this run. // Note: don't pop this level from the section state yet, it may have siblings // that we don't know about yet. Popping will be done when we exit from the parent, // since then we will know if there is any sibling. - state.sections.leaf_executed = true; + sections.leaf_executed = true; } else { // Check if there is any child section left to execute, at any depth below this one. bool no_child_section_left = true; - for (std::size_t c = state.sections.depth; c < state.sections.levels.size(); ++c) { - auto& child = state.sections.levels[c]; + for (std::size_t c = sections.depth; c < sections.levels.size(); ++c) { + auto& child = sections.levels[c]; if (child.previous_section_id != child.max_section_id) { no_child_section_left = false; break; @@ -42,19 +44,25 @@ section_entry_checker::~section_entry_checker() { if (no_child_section_left) { // No more children, we can pop this level and never go back. - state.sections.levels.pop_back(); + sections.levels.pop_back(); } } - state.sections.current_section.pop_back(); + sections.current_section.pop_back(); } - --state.sections.depth; + --sections.depth; } section_entry_checker::operator bool() { - if (state.sections.depth >= state.sections.levels.size()) { - if (state.sections.depth >= max_nested_sections) { +#if SNITCH_WITH_EXCEPTIONS + state.held_info.reset(); +#endif + + auto& sections = state.info.sections; + + if (sections.depth >= sections.levels.size()) { + if (sections.depth >= max_nested_sections) { using namespace snitch::impl; state.reg.print( make_colored("error:", state.reg.with_color, color::fail), @@ -64,19 +72,19 @@ section_entry_checker::operator bool() { assertion_failed("max number of nested sections reached"); } - state.sections.levels.push_back({}); + sections.levels.push_back({}); } - ++state.sections.depth; + ++sections.depth; - auto& level = state.sections.levels[state.sections.depth - 1]; + auto& level = sections.levels[sections.depth - 1]; ++level.current_section_id; if (level.current_section_id > level.max_section_id) { level.max_section_id = level.current_section_id; } - if (state.sections.leaf_executed) { + if (sections.leaf_executed) { // We have already executed another leaf section; can't execute more // on this run, so don't bother going inside this one now. return false; @@ -87,10 +95,10 @@ section_entry_checker::operator bool() { // - This section was already entered in the previous run, and child sections exist in it. if (level.current_section_id == level.previous_section_id + 1 || (level.current_section_id == level.previous_section_id && - state.sections.depth < state.sections.levels.size())) { + sections.depth < sections.levels.size())) { level.previous_section_id = level.current_section_id; - state.sections.current_section.push_back(data); + sections.current_section.push_back(data); push_location( state, {data.location.file, data.location.line, location_type::section_scope}); entered = true; diff --git a/src/snitch_test_data.cpp b/src/snitch_test_data.cpp index 87d7b3b5..c1d90f5c 100644 --- a/src/snitch_test_data.cpp +++ b/src/snitch_test_data.cpp @@ -27,15 +27,19 @@ void set_current_test(test_state* current) noexcept { } void push_location(test_state& test, const assertion_location& location) noexcept { - test.locations.push_back(location); + test.info.locations.push_back(location); } void pop_location(test_state& test) noexcept { - test.locations.pop_back(); + test.info.locations.pop_back(); } scoped_test_check::scoped_test_check(const source_location& location) noexcept : test(get_current_test()) { +#if SNITCH_WITH_EXCEPTIONS + test.held_info.reset(); +#endif + push_location(test, {location.file, location.line, location_type::in_check}); test.in_check = true; } @@ -44,11 +48,11 @@ scoped_test_check::~scoped_test_check() noexcept { test.in_check = false; #if SNITCH_WITH_EXCEPTIONS - if (std::uncaught_exceptions() > 0) { + if (std::uncaught_exceptions() > 0 && !test.held_info.has_value()) { // We are unwinding the stack because an exception has been thrown; - // avoid touching the location state since we will want to report where - // the exception was thrown. - return; + // keep a copy of the full location state since we will want to preserve the information + // when reporting the exception. + test.held_info = test.info; } #endif diff --git a/tests/runtime_tests/capture.cpp b/tests/runtime_tests/capture.cpp index 6ad3ff64..15ab2bf2 100644 --- a/tests/runtime_tests/capture.cpp +++ b/tests/runtime_tests/capture.cpp @@ -207,6 +207,7 @@ TEST_CASE("capture", "[test macros]") { }; framework.run_test(); + REQUIRE(framework.get_num_failures() == 1u); CHECK_CAPTURES("j := 2"); } @@ -224,6 +225,7 @@ TEST_CASE("capture", "[test macros]") { }; framework.run_test(); + REQUIRE(framework.get_num_failures() == 1u); CHECK_NO_CAPTURE; } @@ -249,6 +251,7 @@ TEST_CASE("capture", "[test macros]") { }; framework.run_test(); + REQUIRE(framework.get_num_failures() == 1u); CHECK_CAPTURES("k := 3"); } @@ -267,6 +270,7 @@ TEST_CASE("capture", "[test macros]") { }; framework.run_test(); + REQUIRE(framework.get_num_failures() == 1u); CHECK_CAPTURES("j := 2"); } @@ -283,7 +287,9 @@ TEST_CASE("capture", "[test macros]") { }; framework.run_test(); + REQUIRE(framework.get_num_failures() == 1u); // FIXME: expected nothing + // https://github.com/snitch-org/snitch/issues/179 CHECK_CAPTURES("i := 1"); } #endif diff --git a/tests/runtime_tests/section.cpp b/tests/runtime_tests/section.cpp index 78bebc4e..fa545143 100644 --- a/tests/runtime_tests/section.cpp +++ b/tests/runtime_tests/section.cpp @@ -336,6 +336,106 @@ TEST_CASE("section", "[test macros]") { CHECK_SECTIONS("section 2"); CHECK_CASE(snitch::test_case_state::failed, 1u, 1u); } + + SECTION("with handled exception") { + framework.test_case.func = []() { + try { + SNITCH_SECTION("section 1") { + throw std::runtime_error("bad"); + } + } catch (...) { + } + + SNITCH_SECTION("section 2") { + SNITCH_FAIL_CHECK("trigger"); + } + }; + + framework.run_test(); + REQUIRE(framework.get_num_failures() == 1u); + CHECK_SECTIONS("section 2"); + } + + SECTION("with handled exception no section") { + framework.test_case.func = []() { + try { + SNITCH_SECTION("section 1") { + throw std::runtime_error("bad"); + } + } catch (...) { + } + + SNITCH_FAIL_CHECK("trigger"); + }; + + framework.run_test(); + REQUIRE(framework.get_num_failures() == 1u); + CHECK_NO_SECTION; + } + + SECTION("with handled exceptions") { + framework.test_case.func = []() { + try { + SNITCH_SECTION("section 1") { + throw std::runtime_error("bad"); + } + } catch (...) { + } + + try { + SNITCH_SECTION("section 2") { + throw std::runtime_error("bad"); + } + } catch (...) { + } + + SNITCH_SECTION("section 3") { + SNITCH_FAIL_CHECK("trigger"); + } + }; + + framework.run_test(); + REQUIRE(framework.get_num_failures() == 1u); + CHECK_SECTIONS("section 3"); + } + + SECTION("with handled exception then unhandled") { + framework.test_case.func = []() { + try { + SNITCH_SECTION("section 1") { + throw std::runtime_error("bad"); + } + } catch (...) { + } + + SNITCH_SECTION("section 2") { + throw std::runtime_error("bad"); + } + }; + + framework.run_test(); + REQUIRE(framework.get_num_failures() == 1u); + CHECK_SECTIONS("section 2"); + } + + SECTION("with handled exception then unhandled no section") { + framework.test_case.func = []() { + try { + SNITCH_SECTION("section 1") { + throw std::runtime_error("bad"); + } + } catch (...) { + } + + throw std::runtime_error("bad"); + }; + + framework.run_test(); + REQUIRE(framework.get_num_failures() == 1u); + // FIXME: expected nothing + // https://github.com/snitch-org/snitch/issues/179 + CHECK_SECTIONS("section 1"); + } #endif }