More unit tests for util (#192)

* Unit test collect_image_files function

Signed-off-by: Aleksandr Motsjonov <[email protected]>

* Unit tests for database_paths function

Signed-off-by: Aleksandr Motsjonov <[email protected]>

* Unit test for fix_metadata

Signed-off-by: Aleksandr Motsjonov <[email protected]>

* fix win32 env tests

Signed-off-by: Aleksandr Motsjonov <[email protected]>

* fix win32

Signed-off-by: Aleksandr Motsjonov <[email protected]>

* debugging to find where win32 fails

Signed-off-by: Aleksandr Motsjonov <[email protected]>

* trying fix win32

Signed-off-by: Aleksandr Motsjonov <[email protected]>

* Fix win32

Signed-off-by: Aleksandr Motsjonov <[email protected]>

* format

Signed-off-by: Aleksandr Motsjonov <[email protected]>

* Fix win32 fix

Signed-off-by: Aleksandr Motsjonov <[email protected]>

* formatting

Signed-off-by: Aleksandr Motsjonov <[email protected]>

* formatting and such

Signed-off-by: Aleksandr Motsjonov <[email protected]>

* Move outside loop from main.cpp into collect_image_files

Signed-off-by: Aleksandr Motsjonov <[email protected]>

* cleanup

Signed-off-by: Aleksandr Motsjonov <[email protected]>

* fix up float type validation

Signed-off-by: Aleksandr Motsjonov <[email protected]>

* Bring back empty batches

Signed-off-by: Aleksandr Motsjonov <[email protected]>

---------

Signed-off-by: Aleksandr Motsjonov <[email protected]>

Author: soswow

include/rawtoaces/image_converter.h

@@ -11,16 +11,16 @@ namespace rta
 namespace util
 {
 
-/// Collect all files from a given `path` into batchs. If the `path` is a
-/// directory, create an entry in `batches` and fill it with the file names
-/// from that directory. If the `path` is a file, add its name to the first
-/// entry in `batches`.
-/// @param path path to a file or directory to process.
-/// @param batches the collection of batches to fill in.
-/// @result `false` if the file or directory requested in `path` does not
-/// exist.
-bool collect_image_files(
-    const std::string &path, std::vector<std::vector<std::string>> &batches );
+/// Collect all files from given `paths` into batches.
+/// For each path that is a directory, entries are created in the returned batches
+/// and fill it with the file names. Invalid paths are skipped with an error message.
+/// First batch is reserved for all paths that are files. If no such paths are provided,
+/// first batch will be empty.
+///
+/// @param paths vector of paths to files or directories to process.
+/// @return vector of batches, where each batch contains files from one input path.
+std::vector<std::vector<std::string>>
+collect_image_files( const std::vector<std::string> &paths );
 
 class ImageConverter
 {

src/rawtoaces/main.cpp

@@ -36,19 +36,9 @@ int main( int argc, const char *argv[] )
         return 1;
     }
 
-    // Create a separate batch for each input directory.
-    // Reserve the first batch for the individual input files.
-    std::vector<std::vector<std::string>> batches( 1 );
-
     // Gather all the raw images from arg list
-    for ( const auto &path: files )
-    {
-        if ( !rta::util::collect_image_files( path, batches ) )
-        {
-            std::cerr << "File or directory not found: " << path << std::endl;
-            return 1;
-        }
-    }
+    std::vector<std::vector<std::string>> batches =
+        rta::util::collect_image_files( files );
 
     // Process raw files ...
     bool empty  = true;

src/rawtoaces_util/image_converter.cpp

@@ -50,60 +50,65 @@ struct CameraIdentifier
  * @return true if the file was processed (either added to batch or filtered out), 
  *         false if the file should be ignored
  */
-bool check_and_add_file(
+void check_and_add_file(
     const std::filesystem::path &path, std::vector<std::string> &batch )
 {
     bool is_regular_file = std::filesystem::is_regular_file( path ) ||
                            std::filesystem::is_symlink( path );
     if ( !is_regular_file )
     {
         std::cerr << "Not a regular file: " << path << std::endl;
-        return false;
+        return;
     }
 
     static const std::set<std::string> ignore_filenames = { ".DS_Store" };
     std::string                        filename = path.filename().string();
     if ( ignore_filenames.count( filename ) > 0 )
-        return false;
+        return;
 
     static const std::set<std::string> ignore_extensions = { ".exr",
                                                              ".jpg",
                                                              ".jpeg" };
     std::string extension = OIIO::Strutil::lower( path.extension().string() );
     if ( ignore_extensions.count( extension ) > 0 )
-        return false;
+        return;
 
     batch.push_back( path.string() );
-
-    return true;
+    return;
 }
 
-bool collect_image_files(
-    const std::string &path, std::vector<std::vector<std::string>> &batches )
+std::vector<std::vector<std::string>>
+collect_image_files( const std::vector<std::string> &paths )
 {
-    if ( !std::filesystem::exists( path ) )
+    std::vector<std::vector<std::string>> batches( 1 );
+
+    for ( const auto &path: paths )
     {
-        return false;
-    }
+        if ( !std::filesystem::exists( path ) )
+        {
+            std::cerr << "File or directory not found: " << path << std::endl;
+            continue;
+        }
 
-    auto canonical_filename = std::filesystem::canonical( path );
+        auto canonical_filename = std::filesystem::canonical( path );
 
-    if ( std::filesystem::is_directory( path ) )
-    {
-        std::vector<std::string> &curr_batch = batches.emplace_back();
-        auto it = std::filesystem::directory_iterator( path );
+        if ( std::filesystem::is_directory( path ) )
+        {
+            std::vector<std::string> &curr_batch = batches.emplace_back();
+            auto it = std::filesystem::directory_iterator( path );
 
-        for ( auto filename: it )
+            for ( auto filename: it )
+            {
+                check_and_add_file( filename, curr_batch );
+            }
+        }
+        else
         {
-            check_and_add_file( filename, curr_batch );
+            check_and_add_file( path, batches[0] );
         }
     }
-    else
-    {
-        check_and_add_file( path, batches[0] );
-    }
 
-    return true;
+    return batches;
 }
 
 /// Gets the list of database paths for rawtoaces data files.
@@ -1165,6 +1170,8 @@ std::vector<std::string> ImageConverter::supported_cameras()
 /// Normalise the metadata in the cases where the OIIO attribute name
 /// doesn't match the standard OpenEXR and/or ACES Container attribute name.
 /// We only check the attribute names which are set by the raw input plugin.
+///
+/// @param spec ImageSpec to modify
 void fix_metadata( OIIO::ImageSpec &spec )
 {
     const std::map<std::string, std::string> standard_mapping = {
@@ -1186,8 +1193,6 @@ void fix_metadata( OIIO::ImageSpec &spec )
             {
                 if ( type.basetype == OIIO::TypeDesc::STRING )
                     spec[dst_name] = src_attribute->get_string();
-                else if ( type.basetype == OIIO::TypeDesc::FLOAT )
-                    spec[dst_name] = src_attribute->get_float();
             }
             spec.erase_attribute( src_name );
         }

src/rawtoaces_util/rawtoaces_util_priv.h

@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright Contributors to the rawtoaces Project.
+
+#pragma once
+
+#include <filesystem>
+#include <string>
+#include <vector>
+#include <OpenImageIO/imageio.h>
+
+// Contains the declarations of the private functions,
+// exposed here for unit-testing.
+
+namespace rta
+{
+namespace util
+{
+
+std::vector<std::string> database_paths();
+
+void fix_metadata( OIIO::ImageSpec &spec );
+
+} // namespace util
+} // namespace rta

tests/CMakeLists.txt

@@ -183,7 +183,22 @@ target_link_libraries(
 setup_test_coverage(Test_UsageTimer)
 add_test ( NAME Test_UsageTimer COMMAND Test_UsageTimer )
 
+################################################################################
 
+add_executable (
+	Test_ImageConverter
+	test_image_converter.cpp
+)
+
+target_link_libraries(
+    Test_ImageConverter
+    PUBLIC
+        ${RAWTOACES_UTIL_LIB}
+        OpenImageIO::OpenImageIO
+)
+
+setup_test_coverage(Test_ImageConverter)
+add_test ( NAME Test_ImageConverter COMMAND Test_ImageConverter )
 
 ################################################################################
 # Coverage report generation
@@ -193,4 +208,4 @@ if( ENABLE_COVERAGE AND COVERAGE_SUPPORTED )
     add_subdirectory(config_tests/util)
 
     generate_coverage_report()
-endif()
+endif()