From e556bc25fbac21aa8ea46379f0a027eaf8ea459d Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Tue, 19 Jan 2021 09:27:13 -0500 Subject: [PATCH] test: fix threading for FaultInjector death tests addresses test timeout and warning message: [WARNING] /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest-death-test.cc:1121:: Death tests use fork(), which is unsafe particularly in a threaded context. For this test, Google Test detected 3 threads. See https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#death-tests-and-threads for more explanation and suggested solutions, especially if this is the last message you see before your test times out. Signed-off-by: Casey Bodley --- src/test/common/CMakeLists.txt | 5 +- src/test/common/test_fault_injector.cc | 74 +++++++++++++++++--------- 2 files changed, 51 insertions(+), 28 deletions(-) diff --git a/src/test/common/CMakeLists.txt b/src/test/common/CMakeLists.txt index 06bd13a2afe9c..63afd33bb93d1 100644 --- a/src/test/common/CMakeLists.txt +++ b/src/test/common/CMakeLists.txt @@ -351,9 +351,8 @@ add_ceph_unittest(unittest_cdc) add_executable(unittest_ceph_timer test_ceph_timer.cc) add_ceph_unittest(unittest_ceph_timer) -add_executable(unittest_fault_injector test_fault_injector.cc - $) -target_link_libraries(unittest_fault_injector global) +add_executable(unittest_fault_injector test_fault_injector.cc) +target_link_libraries(unittest_fault_injector common) add_ceph_unittest(unittest_fault_injector) add_executable(unittest_blocked_completion test_blocked_completion.cc) diff --git a/src/test/common/test_fault_injector.cc b/src/test/common/test_fault_injector.cc index 92dc2ec7abd83..dfa147478ea58 100644 --- a/src/test/common/test_fault_injector.cc +++ b/src/test/common/test_fault_injector.cc @@ -13,13 +13,10 @@ */ #include "common/fault_injector.h" +#include "common/common_init.h" +#include "common/ceph_argparse.h" #include -static const DoutPrefixProvider* dpp() { - static NoDoutPrefix d{g_ceph_context, ceph_subsys_context}; - return &d; -} - TEST(FaultInjectorDeathTest, InjectAbort) { constexpr FaultInjector f{false, InjectAbort{}}; @@ -35,8 +32,29 @@ TEST(FaultInjectorDeathTest, AssignAbort) EXPECT_DEATH([[maybe_unused]] int r = f.check(false), "FaultInjector"); } +// death tests have to run in single-threaded mode, so we can't initialize a +// CephContext until after those have run (gtest automatically runs them first) +class Fixture : public testing::Test { + boost::intrusive_ptr cct; + std::optional prefix; + protected: + void SetUp() override { + CephInitParameters params(CEPH_ENTITY_TYPE_CLIENT); + cct = common_preinit(params, CODE_ENVIRONMENT_UTILITY, + CINIT_FLAG_NO_DEFAULT_CONFIG_FILE); + prefix.emplace(cct.get(), ceph_subsys_context); + } + void TearDown() override { + prefix.reset(); + cct.reset(); + } + const DoutPrefixProvider* dpp() { return &*prefix; } +}; + // test int as a Key type -TEST(FaultInjectorInt, Default) +using FaultInjectorInt = Fixture; + +TEST_F(FaultInjectorInt, Default) { constexpr FaultInjector f; EXPECT_EQ(f.check(0), 0); @@ -45,7 +63,7 @@ TEST(FaultInjectorInt, Default) EXPECT_EQ(f.check(3), 0); } -TEST(FaultInjectorInt, InjectError) +TEST_F(FaultInjectorInt, InjectError) { constexpr FaultInjector f{2, InjectError{-EINVAL}}; EXPECT_EQ(f.check(0), 0); @@ -54,7 +72,7 @@ TEST(FaultInjectorInt, InjectError) EXPECT_EQ(f.check(3), 0); } -TEST(FaultInjectorInt, InjectErrorMessage) +TEST_F(FaultInjectorInt, InjectErrorMessage) { FaultInjector f{2, InjectError{-EINVAL, dpp()}}; EXPECT_EQ(f.check(0), 0); @@ -63,7 +81,7 @@ TEST(FaultInjectorInt, InjectErrorMessage) EXPECT_EQ(f.check(3), 0); } -TEST(FaultInjectorInt, AssignError) +TEST_F(FaultInjectorInt, AssignError) { FaultInjector f; ASSERT_EQ(f.check(0), 0); @@ -71,7 +89,7 @@ TEST(FaultInjectorInt, AssignError) EXPECT_EQ(f.check(0), -EINVAL); } -TEST(FaultInjectorInt, AssignErrorMessage) +TEST_F(FaultInjectorInt, AssignErrorMessage) { FaultInjector f; ASSERT_EQ(f.check(0), 0); @@ -80,7 +98,9 @@ TEST(FaultInjectorInt, AssignErrorMessage) } // test std::string_view as a Key type -TEST(FaultInjectorString, Default) +using FaultInjectorString = Fixture; + +TEST_F(FaultInjectorString, Default) { constexpr FaultInjector f; EXPECT_EQ(f.check("Red"), 0); @@ -88,7 +108,7 @@ TEST(FaultInjectorString, Default) EXPECT_EQ(f.check("Blue"), 0); } -TEST(FaultInjectorString, InjectError) +TEST_F(FaultInjectorString, InjectError) { FaultInjector f{"Red", InjectError{-EIO}}; EXPECT_EQ(f.check("Red"), -EIO); @@ -96,7 +116,7 @@ TEST(FaultInjectorString, InjectError) EXPECT_EQ(f.check("Blue"), 0); } -TEST(FaultInjectorString, InjectErrorMessage) +TEST_F(FaultInjectorString, InjectErrorMessage) { FaultInjector f{"Red", InjectError{-EIO, dpp()}}; EXPECT_EQ(f.check("Red"), -EIO); @@ -104,7 +124,7 @@ TEST(FaultInjectorString, InjectErrorMessage) EXPECT_EQ(f.check("Blue"), 0); } -TEST(FaultInjectorString, AssignError) +TEST_F(FaultInjectorString, AssignError) { FaultInjector f; ASSERT_EQ(f.check("Red"), 0); @@ -112,7 +132,7 @@ TEST(FaultInjectorString, AssignError) EXPECT_EQ(f.check("Red"), -EINVAL); } -TEST(FaultInjectorString, AssignErrorMessage) +TEST_F(FaultInjectorString, AssignErrorMessage) { FaultInjector f; ASSERT_EQ(f.check("Red"), 0); @@ -121,6 +141,8 @@ TEST(FaultInjectorString, AssignErrorMessage) } // test enum class as a Key type +using FaultInjectorEnum = Fixture; + enum class Color { Red, Green, Blue }; static std::ostream& operator<<(std::ostream& out, const Color& c) { @@ -132,7 +154,7 @@ static std::ostream& operator<<(std::ostream& out, const Color& c) { return out; } -TEST(FaultInjectorEnum, Default) +TEST_F(FaultInjectorEnum, Default) { constexpr FaultInjector f; EXPECT_EQ(f.check(Color::Red), 0); @@ -140,7 +162,7 @@ TEST(FaultInjectorEnum, Default) EXPECT_EQ(f.check(Color::Blue), 0); } -TEST(FaultInjectorEnum, InjectError) +TEST_F(FaultInjectorEnum, InjectError) { FaultInjector f{Color::Red, InjectError{-EIO}}; EXPECT_EQ(f.check(Color::Red), -EIO); @@ -148,7 +170,7 @@ TEST(FaultInjectorEnum, InjectError) EXPECT_EQ(f.check(Color::Blue), 0); } -TEST(FaultInjectorEnum, InjectErrorMessage) +TEST_F(FaultInjectorEnum, InjectErrorMessage) { FaultInjector f{Color::Red, InjectError{-EIO, dpp()}}; EXPECT_EQ(f.check(Color::Red), -EIO); @@ -156,7 +178,7 @@ TEST(FaultInjectorEnum, InjectErrorMessage) EXPECT_EQ(f.check(Color::Blue), 0); } -TEST(FaultInjectorEnum, AssignError) +TEST_F(FaultInjectorEnum, AssignError) { FaultInjector f; ASSERT_EQ(f.check(Color::Red), 0); @@ -164,7 +186,7 @@ TEST(FaultInjectorEnum, AssignError) EXPECT_EQ(f.check(Color::Red), -EINVAL); } -TEST(FaultInjectorEnum, AssignErrorMessage) +TEST_F(FaultInjectorEnum, AssignErrorMessage) { FaultInjector f; ASSERT_EQ(f.check(Color::Red), 0); @@ -173,6 +195,8 @@ TEST(FaultInjectorEnum, AssignErrorMessage) } // test custom move-only Key type +using FaultInjectorMoveOnly = Fixture; + struct MoveOnlyKey { MoveOnlyKey() = default; MoveOnlyKey(const MoveOnlyKey&) = delete; @@ -189,25 +213,25 @@ static std::ostream& operator<<(std::ostream& out, const MoveOnlyKey&) { return out; } -TEST(FaultInjectorMoveOnly, Default) +TEST_F(FaultInjectorMoveOnly, Default) { constexpr FaultInjector f; EXPECT_EQ(f.check(MoveOnlyKey{}), 0); } -TEST(FaultInjectorMoveOnly, InjectError) +TEST_F(FaultInjectorMoveOnly, InjectError) { FaultInjector f{MoveOnlyKey{}, InjectError{-EIO}}; EXPECT_EQ(f.check(MoveOnlyKey{}), -EIO); } -TEST(FaultInjectorMoveOnly, InjectErrorMessage) +TEST_F(FaultInjectorMoveOnly, InjectErrorMessage) { FaultInjector f{MoveOnlyKey{}, InjectError{-EIO, dpp()}}; EXPECT_EQ(f.check(MoveOnlyKey{}), -EIO); } -TEST(FaultInjectorMoveOnly, AssignError) +TEST_F(FaultInjectorMoveOnly, AssignError) { FaultInjector f; ASSERT_EQ(f.check({}), 0); @@ -215,7 +239,7 @@ TEST(FaultInjectorMoveOnly, AssignError) EXPECT_EQ(f.check({}), -EINVAL); } -TEST(FaultInjectorMoveOnly, AssignErrorMessage) +TEST_F(FaultInjectorMoveOnly, AssignErrorMessage) { FaultInjector f; ASSERT_EQ(f.check({}), 0); -- 2.39.5