Add unit tests to core using Google Test (#49)

Add unit tests for MediaProcessor using Google Test - Implement an initial set of unit tests for various components under `MediaProcessor` - Improve test coverage for file certain utilities - Ensure test configurations are properly loaded in tests - Refactor test names to follow gtest naming conventions - Resolve minor code issues identified during testing --------- Co-authored-by: Nikhil <[email protected]> Co-authored-by: Nikhil <[email protected]>
omeryusufyagci · Oct 20, 2024 · c67b3a4 · c67b3a4
1 parent e043132
commit c67b3a4
Show file tree

Hide file tree

Showing 22 changed files with 767 additions and 93 deletions.
diff --git a/.gitignore b/.gitignore
@@ -157,6 +157,9 @@ dmypy.json
 # Cython debug symbols
 cython_debug/
 
+# VSCode
+.vscode/
+
 # PyCharm
 #  JetBrains specific template is maintained in a separate JetBrains.gitignore that can
 #  be found at https://github.com/github/gitignore/blob/main/Global/JetBrains.gitignore

diff --git a/MediaProcessor/CMakeLists.txt b/MediaProcessor/CMakeLists.txt
@@ -12,31 +12,26 @@ endif()
 
 set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O2")
 
-include_directories(include)
-include_directories(${CMAKE_SOURCE_DIR}/third_party/nlohmann)
+# Tests are excluded by default, use `cmake -DBUILD_TESTING=ON` to build with tests.
+option(BUILD_TESTING "Test Build" OFF)
 
-add_executable(MediaProcessor
-    src/main.cpp
-    src/ConfigManager.cpp
-    src/AudioProcessor.cpp
-    src/VideoProcessor.cpp
-    src/Utils.cpp
-    src/CommandBuilder.cpp
-    src/HardwareUtils.cpp
-    src/FFmpegSettingsManager.cpp
-)
+# Set include directories
+include(CTest)
+include_directories(${CMAKE_SOURCE_DIR}/include)
+include_directories(${CMAKE_SOURCE_DIR}/third_party/nlohmann)
 
 # Threads is required
 set(THREADS_PREFER_PTHREAD_FLAG ON)
 find_package(Threads REQUIRED)
-target_link_libraries(MediaProcessor PRIVATE Threads::Threads)
-
-# Link the DeepFilter library
-target_link_libraries(MediaProcessor PRIVATE ${CMAKE_SOURCE_DIR}/lib/libdf.so)
 
 # SNDFILE: for read/write of sampled audio files
 # https://github.com/libsndfile/libsndfile
 find_package(PkgConfig REQUIRED)
 pkg_check_modules(SNDFILE REQUIRED sndfile)
 include_directories(${SNDFILE_INCLUDE_DIRS})
-target_link_libraries(MediaProcessor PRIVATE ${SNDFILE_LIBRARIES})
+
+# Include the cmake files under cmake/
+include(cmake/src.cmake)
+if(BUILD_TESTING)
+    include(cmake/test.cmake)
+endif()
diff --git a/MediaProcessor/cmake/src.cmake b/MediaProcessor/cmake/src.cmake
@@ -0,0 +1,20 @@
+# CMake configuration for building the main MediaProcessor executable
+
+add_executable(MediaProcessor
+    ${CMAKE_SOURCE_DIR}/src/main.cpp
+    ${CMAKE_SOURCE_DIR}/src/ConfigManager.cpp
+    ${CMAKE_SOURCE_DIR}/src/AudioProcessor.cpp
+    ${CMAKE_SOURCE_DIR}/src/VideoProcessor.cpp
+    ${CMAKE_SOURCE_DIR}/src/Utils.cpp
+    ${CMAKE_SOURCE_DIR}/src/CommandBuilder.cpp
+    ${CMAKE_SOURCE_DIR}/src/HardwareUtils.cpp
+    ${CMAKE_SOURCE_DIR}/src/FFmpegSettingsManager.cpp
+)
+
+target_link_libraries(MediaProcessor PRIVATE Threads::Threads)
+target_link_libraries(MediaProcessor PRIVATE ${CMAKE_SOURCE_DIR}/lib/libdf.so)
+target_link_libraries(MediaProcessor PRIVATE ${SNDFILE_LIBRARIES})
+
+set_target_properties(MediaProcessor PROPERTIES
+    RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}"
+)
diff --git a/MediaProcessor/cmake/test.cmake b/MediaProcessor/cmake/test.cmake
@@ -0,0 +1,71 @@
+include(FetchContent)
+cmake_policy(SET CMP0135 NEW)  # Use the latest policy for FetchContent consistency
+
+find_package(GTest QUIET)
+if(NOT GTEST_FOUND)
+    # Fetch GTest if not found
+    FetchContent_Declare(
+        googletest
+        URL https://github.com/google/googletest/archive/5376968f6948923e2411081fd9372e71a59d8e77.zip
+        DOWNLOAD_EXTRACT_TIMESTAMP TRUE
+    )
+    set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)
+    FetchContent_MakeAvailable(googletest)
+endif()
+
+# Setup test media directory
+set(TEST_MEDIA_DIR "${CMAKE_SOURCE_DIR}/tests/TestMedia" CACHE PATH "Path to test media files")
+
+# Common libraries for all test targets
+set(COMMON_LIBRARIES gtest_main ${CMAKE_SOURCE_DIR}/lib/libdf.so ${SNDFILE_LIBRARIES})
+
+# Macro for adding a test executable
+macro(add_test_executable name)
+    add_executable(${name} ${ARGN})
+    target_compile_definitions(${name} PRIVATE TEST_MEDIA_DIR="${TEST_MEDIA_DIR}")
+    target_link_libraries(${name} PRIVATE ${COMMON_LIBRARIES})
+    add_test(NAME ${name} COMMAND ${name})
+endmacro()
+
+# Add test executables using the macro
+add_test_executable(ConfigManagerTester 
+    ${CMAKE_SOURCE_DIR}/tests/ConfigManagerTester.cpp 
+    ${CMAKE_SOURCE_DIR}/src/ConfigManager.cpp 
+    ${CMAKE_SOURCE_DIR}/src/CommandBuilder.cpp 
+    ${CMAKE_SOURCE_DIR}/src/HardwareUtils.cpp 
+    ${CMAKE_SOURCE_DIR}/src/Utils.cpp 
+    ${CMAKE_SOURCE_DIR}/tests/TestUtils.cpp
+)
+
+add_test_executable(UtilsTester 
+    ${CMAKE_SOURCE_DIR}/tests/UtilsTester.cpp 
+    ${CMAKE_SOURCE_DIR}/src/Utils.cpp
+    ${CMAKE_SOURCE_DIR}/src/CommandBuilder.cpp 
+)
+
+add_test_executable(AudioProcessorTester
+    ${CMAKE_SOURCE_DIR}/tests/AudioProcessorTester.cpp 
+    ${CMAKE_SOURCE_DIR}/src/AudioProcessor.cpp 
+    ${CMAKE_SOURCE_DIR}/src/CommandBuilder.cpp 
+    ${CMAKE_SOURCE_DIR}/src/Utils.cpp 
+    ${CMAKE_SOURCE_DIR}/src/HardwareUtils.cpp 
+    ${CMAKE_SOURCE_DIR}/src/ConfigManager.cpp 
+    ${CMAKE_SOURCE_DIR}/tests/TestUtils.cpp
+)
+
+add_test_executable(VideoProcessorTester
+    ${CMAKE_SOURCE_DIR}/tests/VideoProcessorTester.cpp 
+    ${CMAKE_SOURCE_DIR}/src/VideoProcessor.cpp 
+    ${CMAKE_SOURCE_DIR}/src/CommandBuilder.cpp 
+    ${CMAKE_SOURCE_DIR}/src/Utils.cpp 
+    ${CMAKE_SOURCE_DIR}/src/ConfigManager.cpp 
+    ${CMAKE_SOURCE_DIR}/src/HardwareUtils.cpp 
+    ${CMAKE_SOURCE_DIR}/tests/TestUtils.cpp
+)
+
+add_test_executable(CommandBuilderTester
+    ${CMAKE_SOURCE_DIR}/tests/CommandBuilderTester.cpp 
+    ${CMAKE_SOURCE_DIR}/src/CommandBuilder.cpp 
+    ${CMAKE_SOURCE_DIR}/src/Utils.cpp
+)
+
diff --git a/MediaProcessor/src/AudioProcessor.cpp b/MediaProcessor/src/AudioProcessor.cpp
@@ -22,9 +22,9 @@ AudioProcessor::AudioProcessor(const fs::path& inputVideoPath, const fs::path& o
     : m_inputVideoPath(inputVideoPath),
       m_outputAudioPath(outputAudioPath),
       m_overlapDuration(DEFAULT_OVERLAP_DURATION) {
-    m_outputDir = m_outputAudioPath.parent_path();
-    m_chunksDir = m_outputDir / "chunks";
-    m_processedChunksDir = m_outputDir / "processed_chunks";
+    m_outputPath = m_outputAudioPath.parent_path();
+    m_chunksPath = m_outputPath / "chunks";
+    m_processedChunksDir = m_outputPath / "processed_chunks";
 
     m_numChunks = ConfigManager::getInstance().getOptimalThreadCount();
     std::cout << "INFO: using " << m_numChunks << " threads." << std::endl;
@@ -38,7 +38,7 @@ bool AudioProcessor::isolateVocals() {
     ConfigManager& configManager = ConfigManager::getInstance();
 
     // Ensure output directory exists and remove output file if it exists
-    Utils::ensureDirectoryExists(m_outputDir);
+    Utils::ensureDirectoryExists(m_outputPath);
     Utils::removeFileIfExists(m_outputAudioPath);
 
     std::cout << "Input video path: " << m_inputVideoPath << std::endl;
@@ -48,7 +48,7 @@ bool AudioProcessor::isolateVocals() {
         return false;
     }
 
-    m_totalDuration = getAudioDuration(m_outputAudioPath);
+    m_totalDuration = Utils::getMediaDuration(m_outputAudioPath);
     if (m_totalDuration <= 0) {
         std::cerr << "Error: Invalid audio duration." << std::endl;
         return false;
@@ -67,7 +67,7 @@ bool AudioProcessor::isolateVocals() {
     }
 
     // Intermediary files
-    fs::remove_all(m_chunksDir);
+    fs::remove_all(m_chunksPath);
     fs::remove_all(m_processedChunksDir);
 
     return true;
@@ -100,7 +100,7 @@ bool AudioProcessor::splitAudioIntoChunks() {
     ConfigManager& configManager = ConfigManager::getInstance();
     fs::path ffmpegPath = configManager.getFFmpegPath();
 
-    Utils::ensureDirectoryExists(m_chunksDir);
+    Utils::ensureDirectoryExists(m_chunksPath);
 
     std::vector<double> chunkStartTimes;
     std::vector<double> chunkDurations;
@@ -117,7 +117,7 @@ bool AudioProcessor::splitAudioIntoChunks() {
 
 bool AudioProcessor::generateChunkFile(int index, const double startTime, const double duration,
                                        const fs::path& ffmpegPath) {
-    fs::path chunkPath = m_chunksDir / ("chunk_" + std::to_string(index) + ".wav");
+    fs::path chunkPath = m_chunksPath / ("chunk_" + std::to_string(index) + ".wav");
 
     // Set higher precision for chunk boundaries
     std::ostringstream ssStartTime, ssDuration;
@@ -139,7 +139,7 @@ bool AudioProcessor::generateChunkFile(int index, const double startTime, const
         return false;
     }
 
-    m_chunkPaths.push_back(chunkPath);
+    m_chunkPathCol.push_back(chunkPath);
     return true;
 }
 
@@ -221,7 +221,7 @@ bool AudioProcessor::filterChunks() {
     std::vector<std::future<void>> results;
     for (int i = 0; i < m_numChunks; ++i) {
         results.emplace_back(pool.enqueue([&, i]() {
-            fs::path chunkPath = m_chunkPaths[i];
+            fs::path chunkPath = m_chunkPathCol[i];
 
             invokeDeepFilter(chunkPath);
             // invokeDeepFilterFFI(chunkPath);  // RT API still under validation
@@ -235,9 +235,9 @@ bool AudioProcessor::filterChunks() {
 
     // Prepare paths for processed chunks
     for (int i = 0; i < m_numChunks; ++i) {
-        fs::path chunkPath = m_chunkPaths[i];
+        fs::path chunkPath = m_chunkPathCol[i];
         fs::path processedChunkPath = m_processedChunksDir / chunkPath.filename();
-        m_processedChunkPaths.push_back(processedChunkPath);
+        m_processedChunkCol.push_back(processedChunkPath);
     }
 
     return true;
@@ -265,12 +265,12 @@ std::string AudioProcessor::buildFilterComplex() const {
     std::string filterComplex = "";
     int filterIndex = 0;
 
-    if (m_processedChunkPaths.size() < 2) {
+    if (m_processedChunkCol.size() < 2) {
         return filterComplex;  // Return empty string if not enough chunks
     }
 
     // TODO: extract this into an `applyCrossFade()` method.
-    for (int i = 0; i < static_cast<int>(m_processedChunkPaths.size()) - 1; ++i) {
+    for (int i = 0; i < static_cast<int>(m_processedChunkCol.size()) - 1; ++i) {
         if (i == 0) {
             // Generate a `crossfade` for the first chunk pair (0 and 1)
             filterComplex += "[" + std::to_string(i) + ":a][" + std::to_string(i + 1) +
@@ -299,11 +299,11 @@ bool AudioProcessor::mergeChunks() {
     CommandBuilder cmd;
     cmd.addArgument(ffmpegPath.string());
     cmd.addFlag("-y");
-    for (const auto& chunkPath : m_processedChunkPaths) {
+    for (const auto& chunkPath : m_processedChunkCol) {
         cmd.addFlag("-i", chunkPath.string());
     }
 
-    if (static_cast<int>(m_processedChunkPaths.size()) >= 2) {
+    if (static_cast<int>(m_processedChunkCol.size()) >= 2) {
         cmd.addFlag("-filter_complex", buildFilterComplex());
         cmd.addFlag("-map", "[outa]");
     }
@@ -321,34 +321,4 @@ bool AudioProcessor::mergeChunks() {
     return true;
 }
 
-double AudioProcessor::getAudioDuration(const fs::path& audioPath) {
-    // Prepare ffprobe
-    CommandBuilder cmd;
-    cmd.addArgument("ffprobe");
-    cmd.addFlag("-v", "error");
-    cmd.addFlag("-show_entries", "format=duration");
-    cmd.addFlag("-of", "default=noprint_wrappers=1:nokey=1");
-    cmd.addArgument(audioPath.string());
-
-    FILE* pipe = popen(cmd.build().c_str(), "r");
-    if (!pipe) {
-        std::cerr << "Error: Failed to run ffprobe to get audio duration." << std::endl;
-        return -1;
-    }
-
-    char buffer[128];
-    std::string result;
-    while (fgets(buffer, sizeof buffer, pipe) != nullptr) {
-        result += buffer;
-    }
-    pclose(pipe);
-
-    try {
-        return std::stod(result);
-    } catch (std::exception& e) {
-        std::cerr << "Error: Could not parse audio duration." << std::endl;
-        return -1;
-    }
-}
-
 }  // namespace MediaProcessor
diff --git a/MediaProcessor/src/AudioProcessor.h b/MediaProcessor/src/AudioProcessor.h
@@ -34,11 +34,11 @@ class AudioProcessor {
    private:
     fs::path m_inputVideoPath;
     fs::path m_outputAudioPath;
-    fs::path m_outputDir;
-    fs::path m_chunksDir;
+    fs::path m_outputPath;
+    fs::path m_chunksPath;
     fs::path m_processedChunksDir;
-    std::vector<fs::path> m_chunkPaths;
-    std::vector<fs::path> m_processedChunkPaths;
+    std::vector<fs::path> m_chunkPathCol;
+    std::vector<fs::path> m_processedChunkCol;
 
     int m_numChunks;
 
@@ -56,8 +56,6 @@ class AudioProcessor {
 
     std::string buildFilterComplex() const;
 
-    double getAudioDuration(const fs::path &audioPath);
-
     void populateChunkDurations(std::vector<double> &startTimes,
                                 std::vector<double> &durations) const;
 };

diff --git a/MediaProcessor/src/ConfigManager.cpp b/MediaProcessor/src/ConfigManager.cpp
@@ -16,32 +16,35 @@ ConfigManager& ConfigManager::getInstance() {
 bool ConfigManager::loadConfig(const fs::path& configFilePath) {
     std::ifstream config_file(configFilePath);
     if (!config_file.is_open()) {
-        std::cerr << "Error: Could not open " << configFilePath << std::endl;
+        throw std::runtime_error("Error: Could not open " + configFilePath.string());
         return false;
     }
-
-    config_file >> m_config;
+    try {
+        config_file >> m_config;
+    } catch (const std::exception& e) {
+        throw std::runtime_error("Could not read config file: " + std::string(e.what()));
+    }
     return true;
 }
 
 fs::path ConfigManager::getDeepFilterPath() const {
-    return m_config["deep_filter_path"].get<std::string>();
+    return getConfigValue<std::string>("deep_filter_path");
 }
 
 fs::path ConfigManager::getDeepFilterTarballPath() const {
-    return m_config["deep_filter_tarball_path"].get<std::string>();
+    return getConfigValue<std::string>("deep_filter_tarball_path");
 }
 
 fs::path ConfigManager::getDeepFilterEncoderPath() const {
-    return m_config["deep_filter_encoder_path"].get<std::string>();
+    return getConfigValue<std::string>("deep_filter_encoder_path");
 }
 
 fs::path ConfigManager::getDeepFilterDecoderPath() const {
-    return m_config["deep_filter_decoder_path"].get<std::string>();
+    return getConfigValue<std::string>("deep_filter_decoder_path");
 }
 
 fs::path ConfigManager::getFFmpegPath() const {
-    return m_config["ffmpeg_path"].get<std::string>();
+    return getConfigValue<std::string>("ffmpeg_path");
 }
 
 unsigned int ConfigManager::getOptimalThreadCount() {
@@ -52,9 +55,10 @@ unsigned int ConfigManager::getOptimalThreadCount() {
 }
 
 unsigned int ConfigManager::getNumThreadsValue() {
-    return (m_config["use_thread_cap"].get<bool>())
-               ? m_config["max_threads_if_capped"].get<unsigned int>()
-               : 0;
+    if (!getConfigValue<bool>("use_thread_cap")) {
+        return 0;
+    }
+    return getConfigValue<unsigned int>("max_threads_if_capped");
 }
 
 unsigned int ConfigManager::determineNumThreads(unsigned int configNumThreads,