Use subprocess rather than custom code

log/src/CMakeLists.txt

@@ -24,9 +24,11 @@ gz_build_tests(
 )
 
 foreach(test_target ${logging_tests})
-
-  set_tests_properties(${logging_tests} PROPERTIES
+  set_tests_properties(${test_target} PROPERTIES
     ENVIRONMENT GZ_TRANSPORT_LOG_SQL_PATH=${PROJECT_SOURCE_DIR}/log/sql)
+  target_compile_definitions(${test_target} PRIVATE
+    "CORRUPT_DB_TEST_PATH=\"${CMAKE_SOURCE_DIR}/log/test/data/state.tlog\""
+  )
 
 endforeach()
 
@@ -46,9 +48,6 @@ install(DIRECTORY ../sql DESTINATION ${SCHEMA_INSTALL_BASE})
 set(SCHEMA_INSTALL_PATH ${CMAKE_INSTALL_PREFIX}/${SCHEMA_INSTALL_BASE}/sql)
 configure_file(build_config.hh.in build_config.hh @ONLY)
 
-
-message(STATUS "CMAKE_CURRENT_SOURCE_DIR:${CMAKE_CURRENT_SOURCE_DIR}")
-
 target_include_directories(${log_lib_target}
   PUBLIC
     # Add this component's include directory to the build interface include

log/src/Log_TEST.cc

@@ -22,13 +22,16 @@
 
 #include "gz/transport/log/Log.hh"
 #include "test_config.hh"
-#include "log/test_config.hh"
 #include "gtest/gtest.h"
 
 using namespace gz;
 using namespace gz::transport;
 using namespace std::chrono_literals;
 
+namespace {
+constexpr const char * kCorruptDbTestPath = CORRUPT_DB_TEST_PATH;
+}
+
 //////////////////////////////////////////////////
 TEST(Log, OpenMemoryDatabase)
 {
@@ -251,10 +254,7 @@ TEST(Log, NullDescriptorUnopenedLog)
 TEST(Log, OpenCorruptDatabase)
 {
   log::Log logFile;
-  std::string path =
-    testing::portablePathUnion(GZ_TRANSPORT_LOG_TEST_PATH, "data");
-  path = testing::portablePathUnion(path, "state.tlog");
-  logFile.Open(path);
+  logFile.Open(kCorruptDbTestPath);
   EXPECT_GT(logFile.EndTime(), 0ns) << "logFile.EndTime() == "
     << logFile.EndTime().count() << "ns";;
 }

log/test/CMakeLists.txt

@@ -1,5 +1 @@
-configure_file (test_config.hh.in 
-  ${PROJECT_BINARY_DIR}/include/log/test_config.hh
-)
-
 add_subdirectory(integration)

log/test/integration/CMakeLists.txt

@@ -1,5 +1,14 @@
 # Integration tests
 
+
+add_library(ChirpParams STATIC ./ChirpParams.cc)
+target_link_libraries(ChirpParams PUBLIC ${PROJECT_LIBRARY_TARGET_NAME}-log ${EXTRA_TEST_LIB_DEPS})
+target_compile_definitions(ChirpParams
+  PRIVATE TOPIC_CHIRP_EXE="$<TARGET_FILE:topicChirp_aux>")
+
+gz_add_executable(topicChirp_aux topicChirp_aux.cc)
+target_link_libraries(topicChirp_aux ChirpParams)
+
 gz_build_tests(
   TYPE "INTEGRATION"
   TEST_LIST logging_tests
@@ -8,6 +17,7 @@ gz_build_tests(
     playback.cc
     query.cc
   LIB_DEPS
+    ChirpParams
     ${PROJECT_LIBRARY_TARGET_NAME}-log
     ${EXTRA_TEST_LIB_DEPS}
   INCLUDE_DIRS
@@ -21,48 +31,12 @@ if (UNIX AND NOT APPLE)
 endif()
 
 foreach(test_target ${logging_tests})
-
   set_tests_properties(${test_target} PROPERTIES
     ENVIRONMENT GZ_TRANSPORT_LOG_SQL_PATH=${PROJECT_SOURCE_DIR}/log/sql)
   target_compile_definitions(${test_target}
     PRIVATE GZ_TRANSPORT_LOG_SQL_PATH="${PROJECT_SOURCE_DIR}/log/sql")
-  target_compile_definitions(${test_target}
-    PRIVATE GZ_TRANSPORT_LOG_BUILD_PATH="$<TARGET_FILE_DIR:${test_target}>")
-
 endforeach()
 
-set (aux
-  topicChirp_aux.cc
-)
-
-foreach(source_file ${aux})
-  string(REGEX REPLACE ".cc" "" AUX_EXECUTABLE ${source_file})
-  set(BINARY_NAME ${TEST_TYPE}_${AUX_EXECUTABLE})
-
-  gz_add_executable(${BINARY_NAME} ${AUX_EXECUTABLE}.cc)
-
-  # Include the interface directories that we always need.
-  gz_target_interface_include_directories(${BINARY_NAME}
-    ${PROJECT_LIBRARY_TARGET_NAME})
-
-  # Link the libraries that we always need.
-  target_link_libraries(${BINARY_NAME}
-    PRIVATE
-      ${PROJECT_LIBRARY_TARGET_NAME}
-      ${log_lib_target}
-      gtest
-      ${EXTRA_TEST_LIB_DEPS}
-  )
-
-  if(UNIX)
-    # pthread is only available on Unix machines
-    target_link_libraries(${BINARY_NAME}
-      PRIVATE pthread)
-  endif()
-
-  target_compile_definitions(${BINARY_NAME}
-    PRIVATE GZ_TRANSPORT_LOG_BUILD_PATH="$<TARGET_FILE_DIR:${BINARY_NAME}>")
-endforeach()
 
 # gz log CLI test
 if (HAVE_GZ_TOOLS)

log/test/integration/ChirpParams.cc

@@ -0,0 +1,48 @@
+/*
+ * Copyright (C) 2023 Open Source Robotics Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+*/
+
+#include "ChirpParams.hh"
+
+#include <string>
+#include <vector>
+
+static constexpr const char* kTopicChirpExe = TOPIC_CHIRP_EXE;
+
+namespace gz::transport::log::test
+{
+  gz::utils::Subprocess BeginChirps(
+      const std::vector<std::string> &_topics,
+      const int _chirps,
+      const std::string &_partitionName)
+  {
+    // Argument list:
+    // [0]: Executable name
+    // [1]: Partition name
+    // [2]: Number of chirps
+    // [3]-[N]: Each topic name
+    // [N+1]: Null terminator, required by execv
+    const std::size_t numArgs = 3 + _topics.size() + 1;
+
+    std::vector<std::string> strArgs;
+    strArgs.reserve(numArgs-1);
+    strArgs.push_back(kTopicChirpExe);
+    strArgs.push_back(_partitionName);
+    strArgs.push_back(std::to_string(_chirps));
+    strArgs.insert(strArgs.end(), _topics.begin(), _topics.end());
+    return gz::utils::Subprocess(strArgs);
+  }
+}  // namespace gz::transport::log::test

log/test/integration/ChirpParams.hh

@@ -18,162 +18,45 @@
 #ifndef GZ_TRANSPORT_LOG_TEST_INTEGRATION_CHIRPPARAMS_HH_
 #define GZ_TRANSPORT_LOG_TEST_INTEGRATION_CHIRPPARAMS_HH_
 
-#ifdef _MSC_VER
-#pragma warning(push)
-#pragma warning(disable: 4251)
-#endif
-#include <gz/msgs/int32.pb.h>
-#ifdef _MSC_VER
-#pragma warning(pop)
-#endif
-#include <test_config.hh>
-
 #include <string>
 #include <vector>
 
+#include <gz/msgs/int32.pb.h>
 #include <gz/transport/Node.hh>
+#include <gz/utils/Subprocess.hh>
 
-
-namespace gz
+namespace gz::transport::log::test
 {
-  namespace transport
-  {
-    namespace log
-    {
-      namespace test
-      {
-        /// \brief Parameter used to determine how long the topicChirp_aux
-        /// program will wait between emitting message chirps from its topic.
-        /// Value is in milliseconds.
-        const int DelayBetweenChirps_ms = 1;
-
-        /// \brief Parameter used to determine how long the topicChirp_aux
-        /// program will wait (after it advertises) before it begins publishing
-        /// its message chirps. Value is in milliseconds.
-        const int DelayBeforePublishing_ms = 1000;
-
-        /// \brief This is the message type that will be used by the chirping
-        /// topics.
-        using ChirpMsgType = gz::msgs::Int32;
-
-
-        //////////////////////////////////////////////////
-        /// \brief Similar to testing::forkAndRun(), except this function
-        /// specifically calls the INTEGRATION_topicChirp_aux process and passes
-        /// it arguments to determine how it should chirp out messages over its
-        /// topics.
-        /// \param _topics A list of topic names to chirp on
-        /// \param _chirps The number of messages to chirp out. Each message
-        /// will count up starting from the value 1 and ending with the value
-        /// _chirps.
-        /// \return A handle to the process. This can be used with
-        /// testing::waitAndCleanupFork().
-        testing::forkHandlerType BeginChirps(
-            const std::vector<std::string> &_topics,
-            const int _chirps,
-            const std::string &_partitionName)
-        {
-          // Set the chirping process name
-          const std::string process =
-              GZ_TRANSPORT_LOG_BUILD_PATH"/INTEGRATION_topicChirp_aux";
-
-          // Argument list:
-          // [0]: Executable name
-          // [1]: Partition name
-          // [2]: Number of chirps
-          // [3]-[N]: Each topic name
-          // [N+1]: Null terminator, required by execv
-          const std::size_t numArgs = 3 + _topics.size() + 1;
-
-          std::vector<std::string> strArgs;
-          strArgs.reserve(numArgs-1);
-          strArgs.push_back(process);
-          strArgs.push_back(_partitionName);
-          strArgs.push_back(std::to_string(_chirps));
-          strArgs.insert(strArgs.end(), _topics.begin(), _topics.end());
-
-        #ifdef _MSC_VER
-          std::string fullArgs;
-          for (std::size_t i = 0; i < strArgs.size(); ++i)
-          {
-            if (i == 0)
-            {
-              // Windows prefers quotes around the process name
-              fullArgs += "\"";
-            }
-            else
-            {
-              fullArgs += " ";
-            }
-
-            fullArgs += strArgs[i];
-
-            if (i == 0)
-            {
-              fullArgs += "\"";
-            }
-          }
-
-          char * args = new char[fullArgs.size()+1];
-          std::snprintf(args, fullArgs.size()+1, "%s", fullArgs.c_str());
-
-          STARTUPINFO info = {sizeof(info)};
-          PROCESS_INFORMATION processInfo;
-
-          if (!CreateProcess(nullptr, args, nullptr, nullptr,
-                             TRUE, 0, nullptr, nullptr, &info, &processInfo))
-          {
-            std::cerr << "Error running the chirp process ["
-                      << args << "]\n";
-          }
-
-          delete[] args;
-
-          return processInfo;
-        #else
-          // Create a raw char* array to pass to execv
-          char * * args = new char*[numArgs];
-
-          // Allocate a char array for each argument and copy the data to it
-          for (std::size_t i = 0; i < strArgs.size(); ++i)
-          {
-            const std::string &arg = strArgs[i];
-            args[i] = new char[arg.size()+1];
-            std::snprintf(args[i], arg.size()+1, "%s", arg.c_str());
-          }
-
-          // The last item in the char array must be a nullptr, according to the
-          // documentation of execv
-          args[numArgs-1] = nullptr;
-
-          testing::forkHandlerType pid = fork();
-
-          if (pid == 0)
-          {
-            if (execv(process.c_str(), args) == -1)
-            {
-              int err = errno;
-              std::cerr << "Error running the chirp process [" << err << "]: "
-                        << strerror(err) << "\n";
-            }
-          }
-
-          // Clean up the array of arguments
-          for (std::size_t i = 0; i < numArgs; ++i)
-          {
-            char *arg = args[i];
-            delete[] arg;
-            arg = nullptr;
-          }
-          delete[] args;
-          args = nullptr;
-
-          return pid;
-        #endif
-        }
-      }
-    }
-  }
-}
+  /// \brief Parameter used to determine how long the topicChirp_aux
+  /// program will wait between emitting message chirps from its topic.
+  /// Value is in milliseconds.
+  const int DelayBetweenChirps_ms = 1;
+
+  /// \brief Parameter used to determine how long the topicChirp_aux
+  /// program will wait (after it advertises) before it begins publishing
+  /// its message chirps. Value is in milliseconds.
+  const int DelayBeforePublishing_ms = 1000;
+
+  /// \brief This is the message type that will be used by the chirping
+  /// topics.
+  using ChirpMsgType = gz::msgs::Int32;
+
+  //////////////////////////////////////////////////
+  /// \brief Similar to testing::forkAndRun(), except this function
+  /// specifically calls the INTEGRATION_topicChirp_aux process and passes
+  /// it arguments to determine how it should chirp out messages over its
+  /// topics.
+  /// \param[in] _topics A list of topic names to chirp on
+  /// \param[in] _chirps The number of messages to chirp out. Each message
+  /// will count up starting from the value 1 and ending with the value
+  /// _chirps.
+  /// \param[in] _paritionName Gz transport partition to use for the test
+  /// \return A handle to the process. This can be used with
+  /// testing::waitAndCleanupFork().
+  gz::utils::Subprocess BeginChirps(
+      const std::vector<std::string> &_topics,
+      const int _chirps,
+      const std::string &_partitionName);
+}  // namespace gz::transport::log::test
 
 #endif