Merge remote-tracking branch 'upstream/main' into add-more-util-tests

Author: soswow

.gitignore

@@ -1,4 +1,5 @@
 .DS_Store
 build/
 build-coverage/
-tests/materials/*.exr
+tests/materials/*.exr
+__pycache__/

build_scripts/install_deps_linux.bash

@@ -10,3 +10,5 @@ time sudo apt-get -q -f install -y \
     libopencv-dev \
     openimageio-tools libopenimageio-dev \
     nanobind-dev
+
+pip3 install pytest

build_scripts/install_deps_mac.bash

@@ -3,3 +3,5 @@
 set -ex
 
 brew install ceres-solver nlohmann-json openimageio nanobind robin-map
+
+python3 -m pip install --break-system-packages pytest

build_scripts/install_deps_windows.bash

@@ -8,3 +8,9 @@ vcpkg x-update-baseline \
 vcpkg install \
     --x-install-root="C:/vcpkg/installed" \
     --x-manifest-root="./build_scripts"
+
+# Install pip and pytest to the vcpkg Python
+# Since vcpkg Python doesn't include pip, install it first using ensurepip
+VCPKG_PYTHON="C:/vcpkg/installed/x64-windows/tools/python3/python.exe"
+"$VCPKG_PYTHON" -m ensurepip --upgrade
+"$VCPKG_PYTHON" -m pip install pytest

build_scripts/install_deps_yum.bash

@@ -3,5 +3,5 @@
 set -ex
 
 sudo yum install --setopt=tsflags=nodocs -y eigen3-devel ceres-solver-devel json-devel
-            
-sudo python -m pip install nanobind
+
+sudo python -m pip install nanobind pytest

src/rawtoaces_util/image_converter.cpp

@@ -1744,53 +1744,67 @@ bool ImageConverter::apply_crop(
 bool ImageConverter::make_output_path(
     std::string &path, const std::string &suffix )
 {
-    std::filesystem::path temp_path( path );
+    // Validate input path
+    if ( path.empty() )
+    {
+        std::cerr << "ERROR: Empty input path provided." << std::endl;
+        return false;
+    }
+    try
+    {
+        std::filesystem::path temp_path( path );
 
-    temp_path.replace_extension();
-    temp_path += suffix + ".exr";
+        temp_path.replace_extension();
+        temp_path += suffix + ".exr";
 
-    if ( !settings.output_dir.empty() )
-    {
-        auto new_directory = std::filesystem::path( settings.output_dir );
+        if ( !settings.output_dir.empty() )
+        {
+            auto new_directory = std::filesystem::path( settings.output_dir );
 
-        auto filename      = temp_path.filename();
-        auto old_directory = temp_path.remove_filename();
+            auto filename      = temp_path.filename();
+            auto old_directory = temp_path.remove_filename();
 
-        new_directory = old_directory / new_directory;
+            new_directory = old_directory / new_directory;
 
-        if ( !std::filesystem::exists( new_directory ) )
-        {
-            if ( settings.create_dirs )
+            if ( !std::filesystem::exists( new_directory ) )
             {
-                if ( !std::filesystem::create_directory( new_directory ) )
+                if ( settings.create_dirs )
+                {
+                    if ( !std::filesystem::create_directory( new_directory ) )
+                    {
+                        std::cerr << "ERROR: Failed to create directory "
+                                  << new_directory << "." << std::endl;
+                        return false;
+                    }
+                }
+                else
                 {
-                    std::cerr << "ERROR: Failed to create directory "
-                              << new_directory << "." << std::endl;
+                    std::cerr << "ERROR: The output directory " << new_directory
+                              << " does not exist." << std::endl;
                     return false;
                 }
             }
-            else
-            {
-                std::cerr << "ERROR: The output directory " << new_directory
-                          << " does not exist." << std::endl;
-                return false;
-            }
+            temp_path = std::filesystem::absolute( new_directory / filename );
         }
 
-        temp_path = std::filesystem::absolute( new_directory / filename );
-    }
+        if ( !settings.overwrite && std::filesystem::exists( temp_path ) )
+        {
+            std::cerr
+                << "ERROR: file " << temp_path << " already exists. Use "
+                << "--overwrite to allow overwriting existing files. Skipping "
+                << "this file." << std::endl;
+            return false;
+        }
 
-    if ( !settings.overwrite && std::filesystem::exists( temp_path ) )
+        path = temp_path.string();
+        return true;
+    }
+    catch ( const std::exception &e )
     {
-        std::cerr
-            << "ERROR: file " << temp_path << " already exists. Use "
-            << "--overwrite to allow overwriting existing files. Skipping "
-            << "this file." << std::endl;
+        std::cerr << "ERROR: Invalid path format '" << path << "': " << e.what()
+                  << std::endl;
         return false;
     }
-
-    path = temp_path.string();
-    return true;
 }
 
 bool ImageConverter::save_image(
@@ -1824,6 +1838,40 @@ bool ImageConverter::save_image(
 
 bool ImageConverter::process_image( const std::string &input_filename )
 {
+    // Early validation: check if input file exists and is valid
+    if ( input_filename.empty() )
+    {
+        if ( settings.verbosity > 0 )
+        {
+            std::cerr << "ERROR: Empty input filename provided." << std::endl;
+        }
+        return false;
+    }
+
+    // Validate input file exists
+    // Wrap in try-catch to handle filesystem exceptions on Windows
+    try
+    {
+        if ( !std::filesystem::exists( input_filename ) )
+        {
+            if ( settings.verbosity > 0 )
+            {
+                std::cerr << "ERROR: Input file does not exist: "
+                          << input_filename << std::endl;
+            }
+            return false;
+        }
+    }
+    catch ( const std::filesystem::filesystem_error &e )
+    {
+        if ( settings.verbosity > 0 )
+        {
+            std::cerr << "ERROR: Filesystem error while checking input file '"
+                      << input_filename << "': " << e.what() << std::endl;
+        }
+        return false;
+    }
+
     std::string output_filename = input_filename;
     if ( !make_output_path( output_filename ) )
     {

tests/CMakeLists.txt

@@ -200,6 +200,30 @@ target_link_libraries(
 setup_test_coverage(Test_ImageConverter)
 add_test ( NAME Test_ImageConverter COMMAND Test_ImageConverter )
 
+################################################################################
+# Python tests
+if(RTA_BUILD_PYTHON_BINDINGS)
+    if(Python_EXECUTABLE)
+        add_test(
+            NAME Test_Python_ImageConverter
+            COMMAND ${Python_EXECUTABLE} -m pytest ${CMAKE_CURRENT_SOURCE_DIR}/python/test_image_converter.py -v
+            WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
+        )
+
+        # Set environment for Python test to find the compiled module
+        # Handle both single-config (Linux/macOS) and multi-config (Windows) generators
+        if(WIN32)
+            set_tests_properties(Test_Python_ImageConverter PROPERTIES
+                ENVIRONMENT "PYTHONPATH=${CMAKE_BINARY_DIR}/src/bindings/Release;${CMAKE_BINARY_DIR}/src/bindings;$ENV{PYTHONPATH}"
+            )
+        else()
+            set_tests_properties(Test_Python_ImageConverter PROPERTIES
+                ENVIRONMENT "PYTHONPATH=${CMAKE_BINARY_DIR}/src/bindings:$ENV{PYTHONPATH}"
+            )
+        endif()
+    endif()
+endif()
+
 ################################################################################
 # Coverage report generation
 if( ENABLE_COVERAGE AND COVERAGE_SUPPORTED )

tests/python/test_image_converter.py

@@ -0,0 +1,84 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: Apache-2.0
+# Copyright Contributors to the rawtoaces Project.
+
+"""
+Unit tests for rawtoaces Python bindings - ImageConverter class
+"""
+
+import pytest
+
+# The PYTHONPATH should be set by CMake to find the compiled module
+# No need to manually add paths here as CMake handles this via environment variables
+
+try:
+    import rawtoaces
+except ImportError as e:
+    pytest.skip(f"rawtoaces module not found. Build the Python bindings first: {e}", allow_module_level=True)
+
+
+class TestImageConverter:
+    """Test cases for the ImageConverter class"""
+
+    def test_converter_creation(self):
+        """Test that ImageConverter can be instantiated"""
+        converter = rawtoaces.ImageConverter()
+        assert converter is not None
+        assert isinstance(converter, rawtoaces.ImageConverter)
+
+    def test_converter_has_settings(self):
+        """Test that ImageConverter has a settings attribute"""
+        converter = rawtoaces.ImageConverter()
+        assert hasattr(converter, "settings")
+        assert converter.settings is not None
+
+    def test_converter_has_process_image_method(self):
+        """Test that ImageConverter has process_image method"""
+        converter = rawtoaces.ImageConverter()
+        assert hasattr(converter, "process_image")
+        assert callable(converter.process_image)
+
+    def test_process_image_with_invalid_path(self):
+        """Test process_image with non-existent file returns False"""
+        import os
+        converter = rawtoaces.ImageConverter()
+
+        # Use a simple invalid filename (no path separators to avoid OS-specific issues)
+        invalid_path = "nonexistent_file.txt"
+
+        # Test with invalid path
+        try:
+            result = converter.process_image(invalid_path)
+            assert result is False
+        except Exception:
+            # If an exception is thrown, that's also acceptable behavior for invalid input
+            pass
+
+        # Test with empty path
+        try:
+            result = converter.process_image("")
+            assert result is False
+        except Exception:
+            # If an exception is thrown, that's also acceptable behavior for invalid input
+            pass
+
+
+class TestSettings:
+    """Test cases for the ImageConverter.Settings class"""
+
+    def test_settings_creation(self):
+        """Test that Settings can be instantiated"""
+        converter = rawtoaces.ImageConverter()
+        settings = converter.settings
+        assert settings is not None
+        assert isinstance(settings, rawtoaces.ImageConverter.Settings)
+
+    def test_settings_direct_creation(self):
+        """Test that Settings can be created directly"""
+        settings = rawtoaces.ImageConverter.Settings()
+        assert settings is not None
+        assert isinstance(settings, rawtoaces.ImageConverter.Settings)
+
+
+if __name__ == "__main__":
+    pytest.main([__file__])