Avoid logWarning when environment file filename is empty (#2365)

* Refs 13302: Avoid logWarning when the filename is empty Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> * Refs 13302: Add regression tests Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> * Refs 13302: Apply suggestions Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> * Refs 13302: Uncrustify Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com>
eProsima · Dec 21, 2021 · 91c7380 · 91c7380
1 parent 2705118
commit 91c7380
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 4 deletions.
diff --git a/src/cpp/rtps/RTPSDomain.cpp b/src/cpp/rtps/RTPSDomain.cpp
@@ -122,7 +122,7 @@ RTPSParticipant* RTPSDomain::createParticipant(
             // Create filewatch
             RTPSDomainImpl::file_watch_handle_ = SystemInfo::watch_file(filename, RTPSDomainImpl::file_watch_callback);
         }
-        else
+        else if (!filename.empty())
         {
             logWarning(RTPS_PARTICIPANT, filename + " does not exist. File watching not initialized.");
         }

diff --git a/test/blackbox/common/BlackboxTestsPubSubBasic.cpp b/test/blackbox/common/BlackboxTestsPubSubBasic.cpp
@@ -14,16 +14,22 @@
 
 #include "BlackboxTests.hpp"
 
+#include "mock/BlackboxMockConsumer.h"
 #include "PubSubParticipant.hpp"
 #include "PubSubReader.hpp"
 #include "PubSubWriter.hpp"
-#include "ReqRepAsReliableHelloWorldRequester.hpp"
 #include "ReqRepAsReliableHelloWorldReplier.hpp"
+#include "ReqRepAsReliableHelloWorldRequester.hpp"
+
+#include <fastdds/dds/log/Log.hpp>
 #include <fastrtps/xmlparser/XMLProfileManager.h>
 
 #include <gtest/gtest.h>
 
+#include <cstdlib>
+#include <string>
 #include <tuple>
+#include <vector>
 
 using namespace eprosima::fastrtps;
 using namespace eprosima::fastrtps::rtps;
@@ -81,6 +87,52 @@ class PubSubBasic : public testing::TestWithParam<std::tuple<communication_type,
 
 };
 
+/**
+ * @brief Test function for EnvFileWarning* tests
+ *
+ * Sets environment variable @c FASTDDS_ENVIRONMENT_FILE to @c env_file_name, and then initializes a DomainParticipant,
+ * using a custom log consumer to check how many logWarnings matching
+ * ".*does not exist. File watching not initialized." are issued, comparing that number with @c expected_logs.
+ * Finally, it resets the log module
+ *
+ * @param env_file_name The value to assign to @c FASTDDS_ENVIRONMENT_FILE
+ * @param expected_logs The number of expected logs (error or warning) matching
+ *                      ".*does not exist. File watching not initialized."
+ */
+void env_file_warning(
+        const char* env_file_name,
+        size_t expected_logs)
+{
+    /* Set environment variable */
+#ifdef _WIN32
+    _putenv_s("FASTDDS_ENVIRONMENT_FILE", env_file_name);
+#else
+    setenv("FASTDDS_ENVIRONMENT_FILE", env_file_name, 1);
+#endif // _WIN32
+
+    using namespace eprosima::fastdds::dds;
+    /* Set up log */
+    BlackboxMockConsumer* helper_consumer = new BlackboxMockConsumer();
+    Log::ClearConsumers();  // Remove default consumers
+    Log::RegisterConsumer(std::unique_ptr<LogConsumer>(helper_consumer)); // Registering a consumer transfer ownership
+    // Filter specific message
+    Log::SetVerbosity(Log::Kind::Warning);
+    Log::SetCategoryFilter(std::regex("RTPS_PARTICIPANT"));
+    Log::SetErrorStringFilter(std::regex(".*does not exist. File watching not initialized."));
+
+    /* Create and enable DomainParticipant */
+    PubSubReader<HelloWorldType> reader(TEST_TOPIC_NAME);
+    reader.init();
+    EXPECT_TRUE(reader.isInitialized());
+
+    /* Check logs */
+    Log::Flush();
+    EXPECT_EQ(helper_consumer->ConsumedEntries().size(), expected_logs);
+
+    /* Clean-up */
+    Log::Reset();  // This calls to ClearConsumers, which deletes the registered consumer
+}
+
 TEST_P(PubSubBasic, PubSubAsNonReliableHelloworld)
 {
     // Best effort incompatible with best effort
@@ -756,6 +808,22 @@ TEST_P(PubSubBasic, ReliableTransientLocalTwoWritersConsecutives)
     }
 }
 
+/*
+ * Check that setting FASTDDS_ENVIRONMENT_FILE to an unexisting file issues 1 logWarning
+ */
+TEST(PubSubBasic, EnvFileWarningWrongFile)
+{
+    env_file_warning("unexisting_file", 1);
+}
+
+/*
+ * Check that setting FASTDDS_ENVIRONMENT_FILE to an empty string issues 0 logWarning
+ */
+TEST(PubSubBasic, EnvFileWarningEmpty)
+{
+    env_file_warning("", 0);
+}
+
 #ifdef INSTANTIATE_TEST_SUITE_P
 #define GTEST_INSTANTIATE_TEST_MACRO(x, y, z, w) INSTANTIATE_TEST_SUITE_P(x, y, z, w)
 #else

diff --git a/test/blackbox/common/mock/BlackboxMockConsumer.h b/test/blackbox/common/mock/BlackboxMockConsumer.h
@@ -16,8 +16,10 @@
 #define MOCK_BLACKBOX_LOG_CONSUMER_H
 
 #include <fastdds/dds/log/Log.hpp>
-#include <thread>
+
+#include <condition_variable>
 #include <mutex>
+#include <thread>
 #include <vector>
 
 namespace eprosima {
@@ -33,7 +35,7 @@ class BlackboxMockConsumer : public LogConsumer
     {
         std::unique_lock<std::mutex> guard(mMutex);
         mEntriesConsumed.push_back(entry);
-        cv_.notify_one();
+        cv_.notify_all();
     }
 
     const std::vector<Log::Entry> ConsumedEntries() const
@@ -42,6 +44,13 @@ class BlackboxMockConsumer : public LogConsumer
         return mEntriesConsumed;
     }
 
+    void clear_entries()
+    {
+        std::unique_lock<std::mutex> guard(mMutex);
+        mEntriesConsumed.clear();
+        cv_.notify_all();
+    }
+
     std::condition_variable& cv()
     {
         return cv_;