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