Merge pull request #800 from ApexAI/iox-#563-move-tests-in-binding-c-…

…and-dds-into-anonymous-namespace

Iox #563 move tests in binding c and dds into anonymous namespace

doc/website/advanced/best-practice-for-testing.md

@@ -247,6 +247,35 @@ Some classes are hard to test or to reach a full coverage. This might be due to
 Mocks can help to have full control over the `sut` and reliably cause error conditions to test the negative code path.
 There is an [extensive gmock documentation](https://github.com/google/googletest/tree/release-1.8.1/googlemock/docs) in the gtest github repository.
 
+## Pitfalls
+Some tests require creating dummy classes and it might be that the same name is chosen multiple times, e.g. “class DummyData {...};”.
+Usually, at some time the compiler would complain about double definitions.
+But since the definitions are not in header but source files and therefore confined in a translation unit, this is not the case and the test binary gets created.
+There might still be issues though, since the binary contains multiple symbols with the same name.
+One of these issues may arise with the usage of sanitizers, e.g. the address or leak sanitizer.
+If there are multiple “DummyData” classes with different sizes and they are created on the heap, which is totally fine for tests, the address sanitizer might detect an error since something with a different size than expected will be freed.
+To prevent such issues, the tests should be placed within an anonymous namespace which makes all the symbols unique.
+
+```cpp
+namespace
+{
+struct DummyData {
+    uint32_t foo{0};
+};
+
+class MyTest : public Test
+{
+    //...
+};
+
+TEST_F(MyTest, TestName)
+{
+    EXPECT_EQ(ANSWER, 42)
+}
+
+}
+```
+
 # Conclusion
 
 - apply the AAA pattern to structure the test and check only one state transition per test case (all side effects of that transition must be checked, though)
@@ -259,3 +288,4 @@ There is an [extensive gmock documentation](https://github.com/google/googletest
 - use `ASSERT_*` before doing a potential dangerous action which might crash the test application, like accessing a `nullptr` or a `cxx::optional` with a `nullopt`
 - use mocks to reduce the complexity of the test arrangement
 - apply the **DRY** principle by using typed and parameterized tests
+- wrap the tests in an anonymous namespace

iceoryx_binding_c/test/moduletests/test_c2cpp_enum_translation.cpp

@@ -19,6 +19,8 @@
 
 #include "test.hpp"
 
+namespace
+{
 using namespace ::testing;
 
 template <typename CPP, typename C>
@@ -99,3 +101,5 @@ TEST(c2cpp_enum_translation_test, SubscriberEvent)
 #endif
 #pragma GCC diagnostic pop
 }
+
+} // namespace

iceoryx_binding_c/test/moduletests/test_cpp2c_service_description_translation.cpp

@@ -16,13 +16,13 @@
 
 #include "iceoryx_binding_c/internal/cpp2c_service_description_translation.hpp"
 
-using namespace iox;
-using namespace iox::capro;
-
-
 #include "test.hpp"
 
+namespace
+{
 using namespace ::testing;
+using namespace iox;
+using namespace iox::capro;
 
 TEST(iox_service_description_translation_test, TranslatesIntegersCorrectly)
 {
@@ -52,3 +52,5 @@ TEST(iox_service_description_translation_test, TranslatesStringCorrectly)
     EXPECT_THAT(std::string(cServiceDescription.instanceString), Eq("FunkyInstance"));
     EXPECT_THAT(std::string(cServiceDescription.eventString), Eq("BumbleBeeSighted"));
 }
+
+} // namespace

iceoryx_binding_c/test/moduletests/test_listener.cpp

@@ -38,13 +38,12 @@ extern "C" {
 #include <atomic>
 #include <thread>
 
+namespace
+{
 using namespace ::testing;
 using namespace iox::posix;
 using namespace iox::mepoo;
 
-
-namespace
-{
 iox_user_trigger_t g_userTriggerCallbackArgument = nullptr;
 iox_sub_t g_subscriberCallbackArgument = nullptr;
 void* g_contextData = nullptr;
@@ -171,9 +170,6 @@ class iox_listener_test : public Test
 };
 constexpr std::chrono::milliseconds iox_listener_test::TIMEOUT;
 
-
-} // namespace
-
 TEST_F(iox_listener_test, InitListenerWithNullptrForStorageReturnsNullptr)
 {
     EXPECT_EQ(iox_listener_init(nullptr), nullptr);
@@ -333,3 +329,5 @@ TIMING_TEST_F(iox_listener_test, SubscriberCallbackWithContextDataIsCalledSample
     EXPECT_THAT(g_subscriberCallbackArgument, Eq(&m_subscriber[0U]));
     EXPECT_THAT(g_contextData, Eq(static_cast<void*>(&someContextData)));
 });
+
+} // namespace

iceoryx_binding_c/test/moduletests/test_log.cpp

@@ -22,6 +22,8 @@ extern "C" {
 
 #include "test.hpp"
 
+namespace
+{
 using namespace ::testing;
 using namespace iox::log;
 
@@ -50,3 +52,5 @@ TEST(iox_log_test, LogLevelIsSetCorrectly)
     iox_set_loglevel(Iceoryx_LogLevel_Verbose);
     EXPECT_EQ(logManager.DefaultLogLevel(), LogLevel::kVerbose);
 }
+
+} // namespace

iceoryx_binding_c/test/moduletests/test_node.cpp

@@ -18,14 +18,15 @@
 #include "iceoryx_posh/runtime/posh_runtime.hpp"
 #include "iceoryx_posh/testing/roudi_gtest.hpp"
 
-using namespace iox;
-using namespace iox::runtime;
-
 extern "C" {
 #include "iceoryx_binding_c/node.h"
 #include "iceoryx_binding_c/runtime.h"
 }
 
+namespace
+{
+using namespace iox;
+using namespace iox::runtime;
 class iox_node_test : public RouDi_GTest
 {
   public:
@@ -105,3 +106,5 @@ TEST_F(iox_node_test, getNodeRuntimeNameBufferIsLessThanNodeProcessNameLength)
     ASSERT_THAT(nameLength, Eq(m_runtimeName.size()));
     EXPECT_THAT(truncatedProcessName, StrEq(expectedProcessName));
 }
+
+} // namespace

iceoryx_binding_c/test/moduletests/test_notification_info.cpp

@@ -28,11 +28,6 @@
 
 using namespace iox;
 using namespace iox::popo;
-using namespace iox::capro;
-using namespace iox::mepoo;
-using namespace iox::cxx;
-using namespace iox::posix;
-
 
 extern "C" {
 #include "iceoryx_binding_c/notification_info.h"
@@ -42,7 +37,13 @@ extern "C" {
 
 #include "test.hpp"
 
+namespace
+{
 using namespace ::testing;
+using namespace iox::capro;
+using namespace iox::cxx;
+using namespace iox::mepoo;
+using namespace iox::posix;
 
 class iox_notification_info_test : public Test
 {
@@ -236,3 +237,5 @@ TEST_F(iox_notification_info_test, callbackCanBeCalledMultipleTimes)
 
     EXPECT_EQ(m_lastNotificationCallbackArgument, &m_userTrigger);
 }
+
+} // namespace

iceoryx_binding_c/test/moduletests/test_publisher.cpp

@@ -25,10 +25,6 @@
 
 using namespace iox;
 using namespace iox::popo;
-using namespace iox::capro;
-using namespace iox::mepoo;
-using namespace iox::cxx;
-using namespace iox::posix;
 
 extern "C" {
 #include "iceoryx_binding_c/chunk.h"
@@ -38,7 +34,13 @@ extern "C" {
 
 #include "test.hpp"
 
+namespace
+{
 using namespace ::testing;
+using namespace iox::capro;
+using namespace iox::cxx;
+using namespace iox::mepoo;
+using namespace iox::posix;
 
 class iox_pub_test : public Test
 {
@@ -374,3 +376,5 @@ TEST(iox_pub_options_test, publisherOptionInitializationWithNullptrDoesNotCrash)
         ::testing::ExitedWithCode(0),
         ".*");
 }
+
+} // namespace

iceoryx_binding_c/test/moduletests/test_runtime.cpp

@@ -21,6 +21,8 @@ extern "C" {
 #include "iceoryx_posh/iceoryx_posh_types.hpp"
 #include "iceoryx_posh/testing/roudi_gtest.hpp"
 
+namespace
+{
 using namespace iox;
 using namespace iox::runtime;
 
@@ -100,3 +102,5 @@ TEST_F(BindingC_Runtime_test, GetInstanceNameLengthIsLessThanRuntimeNameLength)
     ASSERT_THAT(nameLength, Eq(strnlen(ACTUAL_RUNTIME_NAME, iox::MAX_RUNTIME_NAME_LENGTH + 1)));
     EXPECT_THAT(truncatedRuntimeName, StrEq(EXPECTED_RUNTIME_NAME));
 }
+
+} // namespace

iceoryx_binding_c/test/moduletests/test_service_description.cpp

@@ -16,10 +16,6 @@
 
 #include "iceoryx_posh/capro/service_description.hpp"
 
-using namespace iox;
-using namespace iox::capro;
-
-
 extern "C" {
 #include "iceoryx_binding_c/service_description.h"
 }
@@ -28,7 +24,11 @@ extern "C" {
 
 #include <type_traits>
 
+namespace
+{
 using namespace ::testing;
+using namespace iox;
+using namespace iox::capro;
 
 TEST(iox_service_description_test, StringSizesAreCorrect)
 {
@@ -39,3 +39,5 @@ TEST(iox_service_description_test, StringSizesAreCorrect)
     EXPECT_THAT(sizeof(decltype(std::declval<iox_service_description_t>().eventString)),
                 Eq(iox::capro::IdString_t().capacity()));
 }
+
+} // namespace

iceoryx_binding_c/test/moduletests/test_subscriber.cpp

@@ -28,10 +28,6 @@
 
 using namespace iox;
 using namespace iox::popo;
-using namespace iox::capro;
-using namespace iox::mepoo;
-using namespace iox::cxx;
-using namespace iox::posix;
 
 extern "C" {
 #include "iceoryx_binding_c/chunk.h"
@@ -43,7 +39,13 @@ extern "C" {
 
 #include "test.hpp"
 
+namespace
+{
 using namespace ::testing;
+using namespace iox::capro;
+using namespace iox::cxx;
+using namespace iox::mepoo;
+using namespace iox::posix;
 
 class iox_sub_test : public Test
 {
@@ -446,3 +448,5 @@ TEST(iox_sub_options_test, subscriberOptionInitializationWithNullptrDoesNotCrash
         ::testing::ExitedWithCode(0),
         ".*");
 }
+
+} // namespace

iceoryx_binding_c/test/moduletests/test_types.cpp

@@ -24,13 +24,14 @@
 using namespace iox;
 using namespace iox::popo;
 
-
 extern "C" {
 #include "iceoryx_binding_c/types.h"
 }
 
 #include "test.hpp"
 
+namespace
+{
 using namespace ::testing;
 
 TEST(iox_types_test, WaitSetStorageSizeFits)
@@ -62,3 +63,5 @@ TEST(iox_types_test, cpp2c_PublisherStorageSizeFits)
     EXPECT_THAT(sizeof(cpp2c_Publisher), Le(sizeof(iox_pub_storage_t)));
     EXPECT_THAT(alignof(cpp2c_Publisher), Le(alignof(iox_pub_storage_t)));
 }
+
+} // namespace

iceoryx_binding_c/test/moduletests/test_user_trigger.cpp

@@ -28,6 +28,8 @@ extern "C" {
 
 #include "test.hpp"
 
+namespace
+{
 using namespace ::testing;
 
 class iox_user_trigger_test : public Test
@@ -126,3 +128,5 @@ TEST_F(iox_user_trigger_test, disable_trigger_eventingItFromWaitsetCleansup)
 
     EXPECT_EQ(m_waitSet.size(), 0U);
 }
+
+} // namespace

iceoryx_binding_c/test/moduletests/test_wait_set.cpp

@@ -38,6 +38,8 @@ extern "C" {
 #include <atomic>
 #include <thread>
 
+namespace
+{
 using namespace ::testing;
 
 namespace
@@ -630,3 +632,5 @@ TEST_F(iox_ws_test, UserTriggerCallbackWithContextDataIsCalled)
     EXPECT_THAT(m_callbackOrigin, Eq(m_userTrigger[0]));
     EXPECT_THAT(m_contextData, Eq(&someContextData));
 }
+
+} // namespace

iceoryx_dds/test/moduletests/test_cyclone_data_reader.cpp

@@ -22,13 +22,12 @@
 #include <Mempool_DCPS.hpp>
 #include <dds/dds.hpp>
 
+namespace
+{
 using namespace ::testing;
 using ::testing::_;
+using namespace iox::dds;
 
-namespace iox
-{
-namespace dds
-{
 // ======================================== Helpers ======================================== //
 using TestDataReader = CycloneDataReader;
 
@@ -97,5 +96,4 @@ TEST_F(CycloneDataReaderTest, ReturnsErrorWhenAttemptingToReadIntoANullBuffer)
     EXPECT_EQ(iox::dds::DataReaderError::INVALID_BUFFER_PARAMETER_FOR_USER_PAYLOAD, takeNextResult2.get_error());
 }
 
-} // namespace dds
-} // namespace iox
+} // namespace

iceoryx_dds/test/moduletests/test_dds_to_iox.cpp

@@ -23,6 +23,8 @@
 #include "mocks/google_mocks.hpp"
 #include "test.hpp"
 
+namespace
+{
 using namespace ::testing;
 using ::testing::_;
 
@@ -121,3 +123,5 @@ TEST_F(DDS2IceoryxGatewayTest, PublishesMemoryChunksContainingSamplesToNetwork)
     gw.forward(testChannel);
 }
 #endif
+
+} // namespace

iceoryx_dds/test/moduletests/test_iox_to_dds.cpp

@@ -32,6 +32,8 @@
 
 #include <limits>
 
+namespace
+{
 using namespace ::testing;
 using ::testing::_;
 using ::testing::InSequence;
@@ -317,3 +319,5 @@ TEST_F(Iceoryx2DDSGatewayTest, DestroysCorrespondingSubscriberWhenAPublisherStop
     gw.discover(offerMsg);
 }
 #endif
+
+} // namespace