Skip to content

Commit

Permalink
Merge pull request eclipse-iceoryx#1879 from ApexAI/iox-1613-create-m…
Browse files Browse the repository at this point in the history
…echanism-to-replace-expect-death-tests

iox-1613 replace EXPECT_DEATH with custom mechanism
  • Loading branch information
elBoberido authored Feb 9, 2023
2 parents 10f1e87 + efc7d2e commit ace5f51
Show file tree
Hide file tree
Showing 26 changed files with 531 additions and 288 deletions.
17 changes: 17 additions & 0 deletions doc/design/error-handling.md
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,23 @@ auto errorFunc = [](Error& error) {
func(arg).and_then(successFunc).or_else(errorFunc);
```
### Testing fatal error
For fatal errors the error handler will terminate the execution of the binary. In order to test these paths the
`iox::testing::IOX_EXPECT_FATAL_FAILURE` function should be used instead of the `EXPECT_DEATH` gTest macro.
The `EXPECT_DEATH` gTest macro forks the process which slows down the test execution (especially with the ThreadSanitizer enabled)
and causes issues with running thread. The `IOX_EXPECT_FATAL_FAILURE` registers a temporary error handler and runs the provided
function in a separate thread. When the error handler is called `longjmp` is used to prevent the termination and instead ensures
to gracefully shutdown the thread.
```cpp
#include "iceoryx_hoofs/testing/fatal_failure.hpp"
TEST(MyTest, valueOnNulloptIsFatal) {
iox::optional<bool> sut;
IOX_EXPECT_FATAL_FAILURE<iox::HoofsError>([&] { sut.value(); }, iox::HoofsError::EXPECTS_ENSURES_FAILED);
}
```

## Open points

### Centralized error handling
Expand Down
14 changes: 4 additions & 10 deletions iceoryx_binding_c/source/c_runtime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
//
// SPDX-License-Identifier: Apache-2.0

#include "iceoryx_hoofs/cxx/requires.hpp"
#include "iceoryx_posh/runtime/posh_runtime.hpp"

using namespace iox;
Expand All @@ -26,16 +27,9 @@ extern "C" {

void iox_runtime_init(const char* const name)
{
if (name == nullptr)
{
LogError() << "Runtime name is a nullptr!";
std::terminate();
}
else if (strnlen(name, iox::MAX_RUNTIME_NAME_LENGTH + 1) > MAX_RUNTIME_NAME_LENGTH)
{
LogError() << "Runtime name has more than 100 characters!";
std::terminate();
}
iox::cxx::Expects(name != nullptr && "Runtime name is a nullptr!");
iox::cxx::Expects(strnlen(name, iox::MAX_RUNTIME_NAME_LENGTH + 1) <= MAX_RUNTIME_NAME_LENGTH
&& "Runtime name has more than 100 characters!");

PoshRuntime::initRuntime(RuntimeName_t(iox::TruncateToCapacity, name));
}
Expand Down
6 changes: 5 additions & 1 deletion iceoryx_binding_c/test/moduletests/test_publisher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
//
// SPDX-License-Identifier: Apache-2.0

#include "iceoryx_binding_c/error_handling/error_handling.hpp"
#include "iceoryx_binding_c/internal/cpp2c_enum_translation.hpp"
#include "iceoryx_binding_c/internal/cpp2c_publisher.hpp"
#include "iceoryx_hoofs/testing/fatal_failure.hpp"
#include "iceoryx_posh/internal/popo/building_blocks/chunk_queue_popper.hpp"
#include "iceoryx_posh/internal/popo/ports/publisher_port_roudi.hpp"
#include "iceoryx_posh/internal/popo/ports/publisher_port_user.hpp"
Expand All @@ -37,6 +39,7 @@ extern "C" {
namespace
{
using namespace ::testing;
using namespace iox::testing;
using namespace iox::capro;
using namespace iox::cxx;
using namespace iox::mepoo;
Expand Down Expand Up @@ -137,7 +140,8 @@ TEST(iox_pub_test_DeathTest, initPublisherWithNotInitializedPublisherOptionsTerm
iox_pub_options_t options;
iox_pub_storage_t storage;

EXPECT_DEATH({ iox_pub_init(&storage, "a", "b", "c", &options); }, ".*");
IOX_EXPECT_FATAL_FAILURE<iox::CBindingError>([&] { iox_pub_init(&storage, "a", "b", "c", &options); },
iox::CBindingError::BINDING_C__PUBLISHER_OPTIONS_NOT_INITIALIZED);
}

TEST_F(iox_pub_test, initPublisherWithDefaultOptionsWorks)
Expand Down
13 changes: 11 additions & 2 deletions iceoryx_binding_c/test/moduletests/test_runtime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,16 @@ extern "C" {
#include "iceoryx_binding_c/runtime.h"
}

#include "iceoryx_hoofs/error_handling/error_handling.hpp"
#include "iceoryx_hoofs/testing/fatal_failure.hpp"
#include "iceoryx_posh/iceoryx_posh_types.hpp"
#include "iceoryx_posh/testing/roudi_gtest.hpp"

namespace
{
using namespace iox;
using namespace iox::runtime;
using namespace iox::testing;

class BindingC_Runtime_test : public RouDi_GTest
{
Expand Down Expand Up @@ -69,13 +72,19 @@ TEST_F(BindingC_Runtime_test, RuntimeNameLengthIsOutOfLimit)
::testing::Test::RecordProperty("TEST_ID", "8fd6735d-f331-4c9c-9a91-3f06d3856d15");
std::string tooLongName(iox::MAX_RUNTIME_NAME_LENGTH + 1, 's');

EXPECT_DEATH({ iox_runtime_init(tooLongName.c_str()); }, ".*");
IOX_EXPECT_FATAL_FAILURE<iox::HoofsError>(
[&] {
iox_runtime_init(tooLongName.c_str());
;
},
iox::HoofsError::EXPECTS_ENSURES_FAILED);
}

TEST_F(BindingC_Runtime_test, RuntimeNameIsNullptr)
{
::testing::Test::RecordProperty("TEST_ID", "eb1b76c9-5420-42a9-88b3-db2e36e332de");
EXPECT_DEATH({ iox_runtime_init(nullptr); }, ".*");
IOX_EXPECT_FATAL_FAILURE<iox::HoofsError>([&] { iox_runtime_init(nullptr); },
iox::HoofsError::EXPECTS_ENSURES_FAILED);
}

TEST_F(BindingC_Runtime_test, GetInstanceNameIsNullptr)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,14 @@
//
// SPDX-License-Identifier: Apache-2.0

#include "iceoryx_hoofs/error_handling/error_handling.hpp"
#include "iceoryx_hoofs/testing/fatal_failure.hpp"
#include "iceoryx_posh/runtime/service_discovery.hpp"
#include "iceoryx_posh/testing/roudi_gtest.hpp"

using namespace iox;
using namespace iox::runtime;
using namespace iox::testing;

extern "C" {
#include "iceoryx_binding_c/publisher.h"
Expand Down Expand Up @@ -63,7 +66,8 @@ description_vector iox_service_discovery_test::searchResult;
TEST(iox_service_discovery_DeathTest, InitServiceDiscoveryWithNullptrForStorageTerminates)
{
::testing::Test::RecordProperty("TEST_ID", "be551a9e-7dcf-406a-a74c-7dcb1ee16c30");
EXPECT_DEATH({ iox_service_discovery_init(nullptr); }, ".*");
IOX_EXPECT_FATAL_FAILURE<iox::HoofsError>([&] { iox_service_discovery_init(nullptr); },
iox::HoofsError::EXPECTS_ENSURES_FAILED);
}

/// @note We test only if the arguments of iox_service_discovery_find_service are correctly passed to
Expand Down
9 changes: 6 additions & 3 deletions iceoryx_binding_c/test/moduletests/test_subscriber.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
//
// SPDX-License-Identifier: Apache-2.0

#include "iceoryx_binding_c/error_handling/error_handling.hpp"
#include "iceoryx_binding_c/internal/cpp2c_enum_translation.hpp"
#include "iceoryx_binding_c/internal/cpp2c_subscriber.hpp"
#include "iceoryx_hoofs/testing/fatal_failure.hpp"
#include "iceoryx_posh/internal/mepoo/memory_manager.hpp"
#include "iceoryx_posh/internal/popo/building_blocks/chunk_queue_popper.hpp"
#include "iceoryx_posh/internal/popo/building_blocks/chunk_queue_pusher.hpp"
Expand All @@ -28,6 +30,7 @@

using namespace iox;
using namespace iox::popo;
using namespace iox::testing;

extern "C" {
#include "iceoryx_binding_c/chunk.h"
Expand Down Expand Up @@ -130,14 +133,14 @@ TEST_F(iox_sub_test, initSubscriberWithNullptrForStorageReturnsNullptr)
EXPECT_EQ(iox_sub_init(nullptr, "all", "glory", "hypnotoad", &options), nullptr);
}

// this crashes if the fixture is used, therefore a test without a fixture
TEST(iox_sub_test_DeathTest, initSubscriberWithNotInitializedPublisherOptionsTerminates)
TEST_F(iox_sub_test, initSubscriberWithNotInitializedSubscriberOptionsTerminates)
{
::testing::Test::RecordProperty("TEST_ID", "6a33309e-fe21-45f6-815a-eebe0136c572");
iox_sub_options_t options;
iox_sub_storage_t storage;

EXPECT_DEATH({ iox_sub_init(&storage, "a", "b", "c", &options); }, ".*");
IOX_EXPECT_FATAL_FAILURE<iox::CBindingError>([&] { iox_sub_init(&storage, "a", "b", "c", &options); },
iox::CBindingError::BINDING_C__SUBSCRIBER_OPTIONS_NOT_INITIALIZED);
}

TEST_F(iox_sub_test, initSubscriberWithDefaultOptionsWorks)
Expand Down
84 changes: 33 additions & 51 deletions iceoryx_dust/test/moduletests/test_cxx_forward_list.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,15 @@

#include "iceoryx_dust/cxx/forward_list.hpp"
#include "iceoryx_hoofs/cxx/attributes.hpp"
#include "iceoryx_hoofs/error_handling/error_handling.hpp"
#include "iceoryx_hoofs/testing/fatal_failure.hpp"
#include "test.hpp"

namespace
{
using namespace ::testing;
using namespace iox::cxx;
using namespace iox::testing;

constexpr uint64_t TESTLISTCAPACITY{10U};
constexpr int64_t TEST_LIST_ELEMENT_DEFAULT_VALUE{-99L};
Expand Down Expand Up @@ -142,15 +145,6 @@ int64_t iteratorTraitReturnDoubleValue(IterType iter)
IterValueType m_value = *iter;
return (2 * m_value); // will only work for integer-convertible m_value types
}

// in context of EXPECT_DEATH tests, dummyFunc() shall help suppressing following warning :
// -Wunused-comparison
// reason: the warning is already addressed with the internal handling, which shall be tested here
bool dummyFunc(bool whatever)
{
std::cerr << "Never get here - ever " << whatever << std::endl;
return whatever;
}
} // namespace


Expand Down Expand Up @@ -292,9 +286,7 @@ TEST_F(forward_list_test, FullWhenFilledWithMoreThanCapacityElements)
}

EXPECT_THAT(sut.full(), Eq(true));
// @todo iox-#1613 remove EXPECT_DEATH
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg, hicpp-avoid-goto, cert-err33-c)
EXPECT_DEATH(sut.emplace_front(), "");
IOX_EXPECT_FATAL_FAILURE<iox::HoofsError>([&] { sut.emplace_front(); }, iox::HoofsError::EXPECTS_ENSURES_FAILED);
}
TEST_F(forward_list_test, NotFullWhenFilledWithCapacityAndEraseOneElements)
{
Expand Down Expand Up @@ -666,9 +658,8 @@ TEST_F(forward_list_test, EmplaceAfterWithWrongListIterator)
++cnt;
}

// @todo iox-#1613 remove EXPECT_DEATH
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg, hicpp-avoid-goto, cert-err33-c)
EXPECT_DEATH(sut11.emplace_after(iterOfSut12, cnt), "");
IOX_EXPECT_FATAL_FAILURE<iox::HoofsError>([&] { sut11.emplace_after(iterOfSut12, cnt); },
iox::HoofsError::EXPECTS_ENSURES_FAILED);
}

TEST_F(forward_list_test, PushFrontConstCustomSuccessfullWhenSpaceAvailableLValue)
Expand Down Expand Up @@ -1166,39 +1157,39 @@ TEST_F(forward_list_test, IteratorComparisonOfDifferentLists)

auto iterSut1 = sut11.begin();
auto iterSut2 = sut12.begin();
// @todo iox-#1613 remove EXPECT_DEATH
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg, hicpp-avoid-goto, cert-err33-c)
EXPECT_DEATH(dummyFunc(iterSut1 == iterSut2), "");

IOX_EXPECT_FATAL_FAILURE<iox::HoofsError>([&] { IOX_DISCARD_RESULT(iterSut1 == iterSut2); },
iox::HoofsError::EXPECTS_ENSURES_FAILED);

iterSut1 = sut11.before_begin();
iterSut2 = sut12.before_begin();
// @todo iox-#1613 remove EXPECT_DEATH
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg, hicpp-avoid-goto, cert-err33-c)
EXPECT_DEATH(dummyFunc(iterSut1 == iterSut2), "");

IOX_EXPECT_FATAL_FAILURE<iox::HoofsError>([&] { IOX_DISCARD_RESULT(iterSut1 == iterSut2); },
iox::HoofsError::EXPECTS_ENSURES_FAILED);

iterSut1 = sut11.end();
iterSut2 = sut12.end();
// @todo iox-#1613 remove EXPECT_DEATH
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg, hicpp-avoid-goto, cert-err33-c)
EXPECT_DEATH(dummyFunc(iterSut1 == iterSut2), "");

IOX_EXPECT_FATAL_FAILURE<iox::HoofsError>([&] { IOX_DISCARD_RESULT(iterSut1 == iterSut2); },
iox::HoofsError::EXPECTS_ENSURES_FAILED);

iterSut1 = sut11.begin();
iterSut2 = sut12.begin();
// @todo iox-#1613 remove EXPECT_DEATH
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg, hicpp-avoid-goto, cert-err33-c)
EXPECT_DEATH(dummyFunc(iterSut1 != iterSut2), "");

IOX_EXPECT_FATAL_FAILURE<iox::HoofsError>([&] { IOX_DISCARD_RESULT(iterSut1 != iterSut2); },
iox::HoofsError::EXPECTS_ENSURES_FAILED);

iterSut1 = sut11.before_begin();
iterSut2 = sut12.before_begin();
// @todo iox-#1613 remove EXPECT_DEATH
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg, hicpp-avoid-goto, cert-err33-c)
EXPECT_DEATH(dummyFunc(iterSut1 != iterSut2), "");

IOX_EXPECT_FATAL_FAILURE<iox::HoofsError>([&] { IOX_DISCARD_RESULT(iterSut1 != iterSut2); },
iox::HoofsError::EXPECTS_ENSURES_FAILED);

iterSut1 = sut11.end();
iterSut2 = sut12.end();
// @todo iox-#1613 remove EXPECT_DEATH
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg, hicpp-avoid-goto, cert-err33-c)
EXPECT_DEATH(dummyFunc(iterSut1 != iterSut2), "");

IOX_EXPECT_FATAL_FAILURE<iox::HoofsError>([&] { IOX_DISCARD_RESULT(iterSut1 != iterSut2); },
iox::HoofsError::EXPECTS_ENSURES_FAILED);
}


Expand Down Expand Up @@ -1988,9 +1979,7 @@ TEST_F(forward_list_test, invalidIteratorErase)
auto iter = sut.begin();
sut.pop_front();

// @todo iox-#1613 remove EXPECT_DEATH
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg, hicpp-avoid-goto, cert-err33-c)
EXPECT_DEATH(sut.erase_after(iter), "");
IOX_EXPECT_FATAL_FAILURE<iox::HoofsError>([&] { sut.erase_after(iter); }, iox::HoofsError::EXPECTS_ENSURES_FAILED);
}

TEST_F(forward_list_test, invalidIteratorIncrement)
Expand All @@ -2005,9 +1994,7 @@ TEST_F(forward_list_test, invalidIteratorIncrement)
auto iter = sut.cbegin();
sut.pop_front();

// @todo iox-#1613 remove EXPECT_DEATH
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg, hicpp-avoid-goto, cert-err33-c)
EXPECT_DEATH(++iter, "");
IOX_EXPECT_FATAL_FAILURE<iox::HoofsError>([&] { ++iter; }, iox::HoofsError::EXPECTS_ENSURES_FAILED);
}

TEST_F(forward_list_test, invalidIteratorComparison)
Expand All @@ -2022,9 +2009,8 @@ TEST_F(forward_list_test, invalidIteratorComparison)
auto iter = sut.cbegin();
sut.pop_front();

// @todo iox-#1613 remove EXPECT_DEATH
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg, hicpp-avoid-goto, cert-err33-c)
EXPECT_DEATH(dummyFunc(sut.cbegin() == iter), "");
IOX_EXPECT_FATAL_FAILURE<iox::HoofsError>([&] { IOX_DISCARD_RESULT(sut.cbegin() == iter); },
iox::HoofsError::EXPECTS_ENSURES_FAILED);
}

TEST_F(forward_list_test, invalidIteratorComparisonUnequal)
Expand All @@ -2039,9 +2025,8 @@ TEST_F(forward_list_test, invalidIteratorComparisonUnequal)
sut.pop_front();
auto iter2 = sut.cbegin();

// @todo iox-#1613 remove EXPECT_DEATH
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg, hicpp-avoid-goto, cert-err33-c)
EXPECT_DEATH(dummyFunc(iter2 != iter), "");
IOX_EXPECT_FATAL_FAILURE<iox::HoofsError>([&] { IOX_DISCARD_RESULT(iter2 != iter); },
iox::HoofsError::EXPECTS_ENSURES_FAILED);
}

TEST_F(forward_list_test, invalidIteratorDereferencing)
Expand All @@ -2056,9 +2041,7 @@ TEST_F(forward_list_test, invalidIteratorDereferencing)
auto iter = sut.cbegin();
sut.pop_front();

// @todo iox-#1613 remove EXPECT_DEATH
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg, hicpp-avoid-goto, cert-err33-c)
EXPECT_DEATH(sut.remove(*iter), "");
IOX_EXPECT_FATAL_FAILURE<iox::HoofsError>([&] { sut.remove(*iter); }, iox::HoofsError::EXPECTS_ENSURES_FAILED);
}

TEST_F(forward_list_test, invalidIteratorAddressOfOperator)
Expand All @@ -2073,9 +2056,8 @@ TEST_F(forward_list_test, invalidIteratorAddressOfOperator)
auto iter = sut.cbegin();
sut.pop_front();

// @todo iox-#1613 remove EXPECT_DEATH
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg, hicpp-avoid-goto, cert-err33-c)
EXPECT_DEATH(dummyFunc(iter->m_value == 12U), "");
IOX_EXPECT_FATAL_FAILURE<iox::HoofsError>([&] { IOX_DISCARD_RESULT(iter->m_value == 12U); },
iox::HoofsError::EXPECTS_ENSURES_FAILED);
}

TEST_F(forward_list_test, ListIsCopyableViaMemcpy)
Expand Down
1 change: 1 addition & 0 deletions iceoryx_dust/test/moduletests/test_file_reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ TEST_F(FileReader_test, errorTerminateMode)
std::set_terminate([]() { std::cout << "", std::abort(); });

// @todo iox-#1613 remove EXPECT_DEATH
// using IOX_EXPECT_FATAL_FAILURE currently causes issues with the leak sanitizer with this test
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg, hicpp-avoid-goto, cert-err33-c)
EXPECT_DEATH(
{
Expand Down
Loading

0 comments on commit ace5f51

Please sign in to comment.