Improve some tests (#6167)

* Refs #23732. Use cmake wrapper for `TwoPublishersCommunicationReliable`

Signed-off-by: Miguel Company <[email protected]>

* Refs #23732. Improve RefCountedPointerTests.

Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>

Author: MiguelCompany

test/dds/communication/CMakeLists.txt

@@ -298,7 +298,7 @@ if(Python3_Interpreter_FOUND)
     endif()
 
     add_test(NAME TwoPublishersCommunicationReliable
-        COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_BINARY_DIR}/two_publishers_communication.py)
+        COMMAND ${CMAKE_COMMAND} -DACTUAL_TEST=${PYTHON_EXECUTABLE} -DACTUAL_ARGS=${CMAKE_CURRENT_BINARY_DIR}/two_publishers_communication.py -P ${CMAKE_CURRENT_BINARY_DIR}/test_wrapper.cmake)
 
     # Set test with label NoMemoryCheck
     set_property(TEST TwoPublishersCommunicationReliable PROPERTY LABELS "NoMemoryCheck")

test/unittest/utils/RefCountedPointerTests.cpp

@@ -15,6 +15,10 @@
 #include <gtest/gtest.h>
 
 #include <atomic>
+#include <chrono>
+#include <condition_variable>
+#include <memory>
+#include <mutex>
 #include <thread>
 #include <vector>
 
@@ -63,18 +67,30 @@ enum class RoutineStatus
 
 struct EntityOwner
 {
+    struct SyncPoint
+    {
+        virtual void notify_and_wait() = 0;
+    };
+
     EntityOwner(
             const EntityMock& entity)
         : entity_ptr(entity.get_refcounter_pointer())
         , routine_status(RoutineStatus::NON_INITIALIZED)
     {
     }
 
-    void spawn_routine()
+    void spawn_routine(
+            SyncPoint* sync = nullptr)
     {
-        th = std::thread([&]()
+        th = std::thread([this, sync]()
                         {
                             RefCountedPointer<EntityMock>::Instance entity_instance(entity_ptr);
+
+                            if (sync != nullptr)
+                            {
+                                sync->notify_and_wait();
+                            }
+
                             if (entity_instance)
                             {
                                 entity_instance->dummy_process_data(nullptr);
@@ -105,7 +121,7 @@ class RefCountedPointerTests : public ::testing::Test
 
     void SetUp() override
     {
-        owners_.reserve(5);
+        owners_.reserve(n_owners);
         for (std::size_t i = 0; i < n_owners; ++i)
         {
             owners_.emplace_back(entity_);
@@ -153,21 +169,61 @@ TEST_F(RefCountedPointerTests, refcountedpointer_inactive)
 
 TEST_F(RefCountedPointerTests, refcounterpointer_deactivate_waits_for_no_references)
 {
+    struct WaitForAllOwners : public EntityOwner::SyncPoint
+    {
+        WaitForAllOwners()
+            : num_notifications_(0)
+        {
+        }
+
+        void notify_and_wait() override
+        {
+            std::unique_lock<std::mutex> lock(mutex_);
+            ++num_notifications_;
+            if (num_notifications_ == n_owners)
+            {
+                cv_.notify_all();
+            }
+
+            cv_.wait(lock, [this]() -> bool
+                    {
+                        return num_notifications_ >= n_owners;
+                    });
+        }
+
+        void wait_for_all_notifications()
+        {
+            std::unique_lock<std::mutex> lock(mutex_);
+            cv_.wait(lock, [this]() -> bool
+                    {
+                        return num_notifications_ == n_owners;
+                    });
+        }
+
+    private:
+
+        std::mutex mutex_;
+        std::condition_variable cv_;
+        std::size_t num_notifications_;
+    };
+
+    WaitForAllOwners sync_point;
+
     // Spawn some routines
     for (std::size_t i = 0; i < n_owners; ++i)
     {
-        owners_[i].spawn_routine();
+        owners_[i].spawn_routine(&sync_point);
     }
 
-    // Ensure owners' routines have started
-    std::this_thread::sleep_for(std::chrono::milliseconds(20));
+    // Wait for all routines to be started
+    sync_point.wait_for_all_notifications();
 
     auto t0 = std::chrono::steady_clock::now();
     entity_.destroy();
     auto elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - t0).count();
 
     std::cout << "Elapsed time: " << elapsed << " ms" << std::endl;
-    ASSERT_GT(elapsed, 50); // destroy should have taken at least 50 ms. Being strict it should be 80, but we allow some margin
+    ASSERT_GT(elapsed, 50); // destroy should have taken at least 50 ms. Being strict it should be 100, but we allow some margin
     ASSERT_EQ(entity_.n_times_data_processed, 5);
 }