From efb72a6006bb8b61571af8825438022454126332 Mon Sep 17 00:00:00 2001 From: Aleksandr Motsjonov Date: Mon, 22 Sep 2025 18:48:03 +1000 Subject: [PATCH 1/9] Add tests for default path argument Signed-off-by: Aleksandr Motsjonov --- src/rawtoaces_util/image_converter.cpp | 2 - tests/test_image_converter.cpp | 56 ++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/rawtoaces_util/image_converter.cpp b/src/rawtoaces_util/image_converter.cpp index 7fa5c188..d88cae6f 100644 --- a/src/rawtoaces_util/image_converter.cpp +++ b/src/rawtoaces_util/image_converter.cpp @@ -47,8 +47,6 @@ struct CameraIdentifier * * @param path The filesystem path to check * @param batch Reference to a vector of strings to add valid file paths to - * @return true if the file was processed (either added to batch or filtered out), - * false if the file should be ignored */ void check_and_add_file( const std::filesystem::path &path, std::vector &batch ) diff --git a/tests/test_image_converter.cpp b/tests/test_image_converter.cpp index 6b9aea1c..8b9b124c 100644 --- a/tests/test_image_converter.cpp +++ b/tests/test_image_converter.cpp @@ -491,6 +491,61 @@ void test_database_paths_both_env() unset_env_var( "AMPAS_DATA_PATH" ); } +/// Tests database_paths with override_path parameter (--data-dir functionality) +/// Verifies that override_path takes precedence over environment variables +void test_database_paths_override_path() +{ + std::cout << std::endl << "test_database_paths_override_path()" << std::endl; + // Set environment variables to ensure they are overridden +#ifdef WIN32 + set_env_var( + "RAWTOACES_DATA_PATH", "C:\\env\\path1;C:\\env\\path2" ); + set_env_var( + "AMPAS_DATA_PATH", "C:\\deprecated\\path1;C:\\deprecated\\path2" ); +#else + set_env_var( "RAWTOACES_DATA_PATH", "/env/path1:/env/path2" ); + set_env_var( "AMPAS_DATA_PATH", "/deprecated/path1:/deprecated/path2" ); +#endif + + // Test with override path - should take precedence over environment variables +#ifdef WIN32 + std::string override_path = "C:\\override\\path1;C:\\override\\path2;C:\\override\\path3"; + std::vector paths = database_paths( override_path ); + + // Should have 3 paths from override + OIIO_CHECK_EQUAL( paths.size(), 3 ); + OIIO_CHECK_EQUAL( paths[0], "C:\\override\\path1" ); + OIIO_CHECK_EQUAL( paths[1], "C:\\override\\path2" ); + OIIO_CHECK_EQUAL( paths[2], "C:\\override\\path3" ); +#else + std::string override_path = "/override/path1:/override/path2:/override/path3"; + std::vector paths = database_paths( override_path ); + + // Should have 3 paths from override + OIIO_CHECK_EQUAL( paths.size(), 3 ); + OIIO_CHECK_EQUAL( paths[0], "/override/path1" ); + OIIO_CHECK_EQUAL( paths[1], "/override/path2" ); + OIIO_CHECK_EQUAL( paths[2], "/override/path3" ); +#endif + + // Test with empty override path - should fall back to environment variables + paths = database_paths( "" ); + + // Should have 2 paths from RAWTOACES_DATA_PATH environment variable + OIIO_CHECK_EQUAL( paths.size(), 2 ); +#ifdef WIN32 + OIIO_CHECK_EQUAL( paths[0], "C:\\env\\path1" ); + OIIO_CHECK_EQUAL( paths[1], "C:\\env\\path2" ); +#else + OIIO_CHECK_EQUAL( paths[0], "/env/path1" ); + OIIO_CHECK_EQUAL( paths[1], "/env/path2" ); +#endif + + // Clean up + unset_env_var( "RAWTOACES_DATA_PATH" ); + unset_env_var( "AMPAS_DATA_PATH" ); +} + /// Tests fix_metadata with both Make and Model attributes void test_fix_metadata_both_attributes() { @@ -774,6 +829,7 @@ int main( int, char ** ) test_database_paths_rawtoaces_env(); test_database_paths_ampas_env(); test_database_paths_both_env(); + test_database_paths_override_path(); // Tests for fix_metadata test_fix_metadata_both_attributes(); From 412d41f1cf7e7762a05ce55b7b50dbaf5b68d186 Mon Sep 17 00:00:00 2001 From: Aleksandr Motsjonov Date: Mon, 22 Sep 2025 19:26:02 +1000 Subject: [PATCH 2/9] Add utility for less ifdef win32 code Signed-off-by: Aleksandr Motsjonov --- tests/test_image_converter.cpp | 159 ++++++++++++++++----------------- 1 file changed, 76 insertions(+), 83 deletions(-) diff --git a/tests/test_image_converter.cpp b/tests/test_image_converter.cpp index 8b9b124c..09dc3b5b 100644 --- a/tests/test_image_converter.cpp +++ b/tests/test_image_converter.cpp @@ -21,6 +21,21 @@ using namespace rta::util; + +std::string convert_linux_path_to_windows_path( const std::string &path ){ + std::vector segments; + OIIO::Strutil::split(path, segments, ":"); + + for (auto& segment : segments) { + // Convert forward slashes to backslashes + std::replace(segment.begin(), segment.end(), '/', '\\'); + // Add drive letter + segment = "c:" + segment; + } + + return OIIO::Strutil::join(segments, ";"); +} + // Cross-platform environment variable helpers /* Standard C Library vs POSIX @@ -28,22 +43,34 @@ getenv() - Part of standard C library (C89/C99) - available everywhere setenv()/unsetenv() - Part of POSIX standard - only on Unix-like systems */ #ifdef WIN32 -void set_env_var( const char *name, const char *value ) +void set_env_var( std::string name, std::string value ) +{ + _putenv_s( name.c_str(), value.c_str() ); +} + +std::string to_os_path(std::string linux_path ) { - _putenv_s( name, value ); + return convert_linux_path_to_windows_path(linux_path); } -void unset_env_var( const char *name ) + +void unset_env_var( std::string name ) { - _putenv_s( name, "" ); + _putenv_s( name.c_str(), "" ); } #else -void set_env_var( const char *name, const char *value ) +std::string to_os_path(std::string linux_path ) +{ + return linux_path; +} + +void set_env_var( std::string name, std::string value ) { - setenv( name, value, 1 ); + setenv( name.c_str(), value.c_str(), 1 ); } -void unset_env_var( const char *name ) + +void unset_env_var( std::string name ) { - unsetenv( name ); + unsetenv( name.c_str() ); } #endif @@ -392,14 +419,14 @@ void test_database_paths_default() OIIO_CHECK_EQUAL( paths.empty(), false ); // On Unix systems, should have both new and legacy paths -#if !defined( WIN32 ) && !defined( WIN64 ) - OIIO_CHECK_EQUAL( paths.size(), 2 ); - OIIO_CHECK_EQUAL( paths[0], "/usr/local/share/rawtoaces/data" ); - OIIO_CHECK_EQUAL( paths[1], "/usr/local/include/rawtoaces/data" ); -#else +#ifdef WIN32 // On Windows, should have just the current directory OIIO_CHECK_EQUAL( paths.size(), 1 ); OIIO_CHECK_EQUAL( paths[0], "." ); +#else + OIIO_CHECK_EQUAL( paths.size(), 2 ); + OIIO_CHECK_EQUAL( paths[0], "/usr/local/share/rawtoaces/data" ); + OIIO_CHECK_EQUAL( paths[1], "/usr/local/include/rawtoaces/data" ); #endif } @@ -408,24 +435,14 @@ void test_database_paths_rawtoaces_env() { std::cout << std::endl << "test_database_paths_rawtoaces_env()" << std::endl; - // Set RAWTOACES_DATA_PATH -#ifdef WIN32 - set_env_var( "RAWTOACES_DATA_PATH", "C:\\custom\\path1;C:\\custom\\path2" ); -#else - set_env_var( "RAWTOACES_DATA_PATH", "/custom/path1:/custom/path2" ); -#endif + set_env_var( "RAWTOACES_DATA_PATH", to_os_path( "/custom/path1:/custom/path2" ) ); unset_env_var( "AMPAS_DATA_PATH" ); std::vector paths = database_paths(); OIIO_CHECK_EQUAL( paths.size(), 2 ); -#ifdef WIN32 - OIIO_CHECK_EQUAL( paths[0], "C:\\custom\\path1" ); - OIIO_CHECK_EQUAL( paths[1], "C:\\custom\\path2" ); -#else - OIIO_CHECK_EQUAL( paths[0], "/custom/path1" ); - OIIO_CHECK_EQUAL( paths[1], "/custom/path2" ); -#endif + OIIO_CHECK_EQUAL( paths[0], to_os_path( "/custom/path1" ) ); + OIIO_CHECK_EQUAL( paths[1], to_os_path( "/custom/path2" ) ); // Clean up unset_env_var( "RAWTOACES_DATA_PATH" ); @@ -437,23 +454,14 @@ void test_database_paths_ampas_env() std::cout << std::endl << "test_database_paths_ampas_env()" << std::endl; // Set AMPAS_DATA_PATH (deprecated) unset_env_var( "RAWTOACES_DATA_PATH" ); -#ifdef WIN32 set_env_var( - "AMPAS_DATA_PATH", "C:\\deprecated\\path1;C:\\deprecated\\path2" ); -#else - set_env_var( "AMPAS_DATA_PATH", "/deprecated/path1:/deprecated/path2" ); -#endif + "AMPAS_DATA_PATH", to_os_path( "/deprecated/path1:/deprecated/path2" ) ); std::vector paths = database_paths(); OIIO_CHECK_EQUAL( paths.size(), 2 ); -#ifdef WIN32 - OIIO_CHECK_EQUAL( paths[0], "C:\\deprecated\\path1" ); - OIIO_CHECK_EQUAL( paths[1], "C:\\deprecated\\path2" ); -#else - OIIO_CHECK_EQUAL( paths[0], "/deprecated/path1" ); - OIIO_CHECK_EQUAL( paths[1], "/deprecated/path2" ); -#endif + OIIO_CHECK_EQUAL( paths[0], to_os_path( "/deprecated/path1" ) ); + OIIO_CHECK_EQUAL( paths[1], to_os_path( "/deprecated/path2" ) ); // Clean up unset_env_var( "AMPAS_DATA_PATH" ); @@ -464,27 +472,17 @@ void test_database_paths_both_env() { std::cout << std::endl << "test_database_paths_both_env()" << std::endl; // Set both environment variables -#ifdef WIN32 set_env_var( - "RAWTOACES_DATA_PATH", "C:\\preferred\\path1;C:\\preferred\\path2" ); + "RAWTOACES_DATA_PATH", to_os_path( "/preferred/path1:/preferred/path2" ) ); set_env_var( - "AMPAS_DATA_PATH", "C:\\deprecated\\path1;C:\\deprecated\\path2" ); -#else - set_env_var( "RAWTOACES_DATA_PATH", "/preferred/path1:/preferred/path2" ); - set_env_var( "AMPAS_DATA_PATH", "/deprecated/path1:/deprecated/path2" ); -#endif + "AMPAS_DATA_PATH", to_os_path( "/deprecated/path1:/deprecated/path2" ) ); std::vector paths = database_paths(); // RAWTOACES_DATA_PATH should take precedence OIIO_CHECK_EQUAL( paths.size(), 2 ); -#ifdef WIN32 - OIIO_CHECK_EQUAL( paths[0], "C:\\preferred\\path1" ); - OIIO_CHECK_EQUAL( paths[1], "C:\\preferred\\path2" ); -#else - OIIO_CHECK_EQUAL( paths[0], "/preferred/path1" ); - OIIO_CHECK_EQUAL( paths[1], "/preferred/path2" ); -#endif + OIIO_CHECK_EQUAL( paths[0], to_os_path( "/preferred/path1" ) ); + OIIO_CHECK_EQUAL( paths[1], to_os_path( "/preferred/path2" ) ); // Clean up unset_env_var( "RAWTOACES_DATA_PATH" ); @@ -497,55 +495,47 @@ void test_database_paths_override_path() { std::cout << std::endl << "test_database_paths_override_path()" << std::endl; // Set environment variables to ensure they are overridden -#ifdef WIN32 - set_env_var( - "RAWTOACES_DATA_PATH", "C:\\env\\path1;C:\\env\\path2" ); - set_env_var( - "AMPAS_DATA_PATH", "C:\\deprecated\\path1;C:\\deprecated\\path2" ); -#else - set_env_var( "RAWTOACES_DATA_PATH", "/env/path1:/env/path2" ); - set_env_var( "AMPAS_DATA_PATH", "/deprecated/path1:/deprecated/path2" ); -#endif + + set_env_var( "RAWTOACES_DATA_PATH", to_os_path( "/env/path1:/env/path2" ) ); + set_env_var( "AMPAS_DATA_PATH", to_os_path( "/deprecated/path1:/deprecated/path2" ) ); // Test with override path - should take precedence over environment variables -#ifdef WIN32 - std::string override_path = "C:\\override\\path1;C:\\override\\path2;C:\\override\\path3"; + std::string override_path = to_os_path( "/override/path1:/override/path2:/override/path3" ); std::vector paths = database_paths( override_path ); // Should have 3 paths from override OIIO_CHECK_EQUAL( paths.size(), 3 ); - OIIO_CHECK_EQUAL( paths[0], "C:\\override\\path1" ); - OIIO_CHECK_EQUAL( paths[1], "C:\\override\\path2" ); - OIIO_CHECK_EQUAL( paths[2], "C:\\override\\path3" ); -#else - std::string override_path = "/override/path1:/override/path2:/override/path3"; - std::vector paths = database_paths( override_path ); - - // Should have 3 paths from override - OIIO_CHECK_EQUAL( paths.size(), 3 ); - OIIO_CHECK_EQUAL( paths[0], "/override/path1" ); - OIIO_CHECK_EQUAL( paths[1], "/override/path2" ); - OIIO_CHECK_EQUAL( paths[2], "/override/path3" ); -#endif + OIIO_CHECK_EQUAL( paths[0], to_os_path( "/override/path1" ) ); + OIIO_CHECK_EQUAL( paths[1], to_os_path( "/override/path2" ) ); + OIIO_CHECK_EQUAL( paths[2], to_os_path( "/override/path3" ) ); // Test with empty override path - should fall back to environment variables paths = database_paths( "" ); // Should have 2 paths from RAWTOACES_DATA_PATH environment variable OIIO_CHECK_EQUAL( paths.size(), 2 ); -#ifdef WIN32 - OIIO_CHECK_EQUAL( paths[0], "C:\\env\\path1" ); - OIIO_CHECK_EQUAL( paths[1], "C:\\env\\path2" ); -#else - OIIO_CHECK_EQUAL( paths[0], "/env/path1" ); - OIIO_CHECK_EQUAL( paths[1], "/env/path2" ); -#endif + OIIO_CHECK_EQUAL( paths[0], to_os_path( "/env/path1" ) ); + OIIO_CHECK_EQUAL( paths[1], to_os_path( "/env/path2" ) ); // Clean up unset_env_var( "RAWTOACES_DATA_PATH" ); unset_env_var( "AMPAS_DATA_PATH" ); } +/// Tests convert_linux_path_to_windows_path utility function +void test_convert_linux_path_to_windows_path() +{ + std::cout << std::endl << "test_convert_linux_path_to_windows_path()" << std::endl; + + // Test single path with forward slashes + std::string result = convert_linux_path_to_windows_path( "/usr/local/share" ); + OIIO_CHECK_EQUAL( result, "c:\\usr\\local\\share" ); + + // Test multiple paths separated by ':' + result = convert_linux_path_to_windows_path( "/path1:/path2:/path3" ); + OIIO_CHECK_EQUAL( result, "c:\\path1;c:\\path2;c:\\path3" ); +} + /// Tests fix_metadata with both Make and Model attributes void test_fix_metadata_both_attributes() { @@ -831,6 +821,9 @@ int main( int, char ** ) test_database_paths_both_env(); test_database_paths_override_path(); + // Tests for utility functions + test_convert_linux_path_to_windows_path(); + // Tests for fix_metadata test_fix_metadata_both_attributes(); test_fix_metadata_destination_exists(); From bbb062a163b466e1486d1e9782b40b125bc9313c Mon Sep 17 00:00:00 2001 From: Aleksandr Motsjonov Date: Mon, 22 Sep 2025 21:25:03 +1000 Subject: [PATCH 3/9] Fix exiting tests and add two more Signed-off-by: Aleksandr Motsjonov --- src/rawtoaces_util/image_converter.cpp | 14 +- tests/test_image_converter.cpp | 279 +++++++++++-------------- 2 files changed, 124 insertions(+), 169 deletions(-) diff --git a/src/rawtoaces_util/image_converter.cpp b/src/rawtoaces_util/image_converter.cpp index d88cae6f..589b0a3c 100644 --- a/src/rawtoaces_util/image_converter.cpp +++ b/src/rawtoaces_util/image_converter.cpp @@ -1439,11 +1439,11 @@ bool ImageConverter::configure( return false; } - bool spectral_white_balance = + bool is_spectral_white_balance = settings.WB_method == Settings::WBMethod::Illuminant; - bool spectral_matrix = matrix_method == Settings::MatrixMethod::Spectral; + bool is_spectral_matrix = matrix_method == Settings::MatrixMethod::Spectral; - if ( spectral_white_balance || spectral_matrix ) + if ( is_spectral_white_balance || is_spectral_matrix ) { if ( !prepare_transform_spectral( image_spec, @@ -1457,7 +1457,7 @@ bool ImageConverter::configure( return false; } - if ( spectral_white_balance ) + if ( is_spectral_white_balance ) { float custom_WB[4]; @@ -1580,9 +1580,9 @@ bool ImageConverter::configure( if ( settings.crop_box[2] > 0 && settings.crop_box[3] > 0 ) { - std::cerr << " Crop box: [" << settings.crop_box[0] << ", " - << settings.crop_box[1] << ", " << settings.crop_box[2] - << ", " << settings.crop_box[3] << "]" << std::endl; + std::cerr << " Crop box: [" + << OIIO::Strutil::join( settings.crop_box, ", " ) << "]" + << std::endl; } std::cerr << " Demosaic: " << settings.demosaic_algorithm << std::endl; diff --git a/tests/test_image_converter.cpp b/tests/test_image_converter.cpp index 09dc3b5b..0cb6f3e5 100644 --- a/tests/test_image_converter.cpp +++ b/tests/test_image_converter.cpp @@ -21,19 +21,55 @@ using namespace rta::util; - -std::string convert_linux_path_to_windows_path( const std::string &path ){ +std::string convert_linux_path_to_windows_path( const std::string &path ) +{ std::vector segments; - OIIO::Strutil::split(path, segments, ":"); - - for (auto& segment : segments) { + OIIO::Strutil::split( path, segments, ":" ); + + for ( auto &segment: segments ) + { // Convert forward slashes to backslashes - std::replace(segment.begin(), segment.end(), '/', '\\'); + std::replace( segment.begin(), segment.end(), '/', '\\' ); // Add drive letter segment = "c:" + segment; } - - return OIIO::Strutil::join(segments, ";"); + + return OIIO::Strutil::join( segments, ";" ); +} + +/// Executes a rawtoaces command with the given arguments and captures its output. +/// +/// @param args Vector of command-line arguments to pass to rawtoaces (excluding program name). +/// For example, {"--list-cameras"} or {"--list-illuminants", "--verbose"}. +/// +/// @return std::string containing the captured stdout output from the rawtoaces execution +std::string run_rawtoaces_command( const std::vector &args ) +{ + // Build the command line - from build/tests/ to build/src/rawtoaces/ + std::string command = "../src/rawtoaces/rawtoaces"; + for ( const auto &arg: args ) + { + command += " " + arg; + } + + // Execute the command and capture output + FILE *pipe = popen( command.c_str(), "r" ); + OIIO_CHECK_ASSERT( + pipe != nullptr && "Failed to execute rawtoaces command" ); + + // Read output + std::string output; + char buffer[4096]; + while ( fgets( buffer, sizeof( buffer ), pipe ) != nullptr ) + { + output += buffer; + } + + // Get exit status and validate + int exit_status = pclose( pipe ); + OIIO_CHECK_EQUAL( WEXITSTATUS( exit_status ), 0 ); + + return output; } // Cross-platform environment variable helpers @@ -48,9 +84,9 @@ void set_env_var( std::string name, std::string value ) _putenv_s( name.c_str(), value.c_str() ); } -std::string to_os_path(std::string linux_path ) +std::string to_os_path( std::string linux_path ) { - return convert_linux_path_to_windows_path(linux_path); + return convert_linux_path_to_windows_path( linux_path ); } void unset_env_var( std::string name ) @@ -58,7 +94,7 @@ void unset_env_var( std::string name ) _putenv_s( name.c_str(), "" ); } #else -std::string to_os_path(std::string linux_path ) +std::string to_os_path( std::string linux_path ) { return linux_path; } @@ -435,7 +471,8 @@ void test_database_paths_rawtoaces_env() { std::cout << std::endl << "test_database_paths_rawtoaces_env()" << std::endl; - set_env_var( "RAWTOACES_DATA_PATH", to_os_path( "/custom/path1:/custom/path2" ) ); + set_env_var( + "RAWTOACES_DATA_PATH", to_os_path( "/custom/path1:/custom/path2" ) ); unset_env_var( "AMPAS_DATA_PATH" ); std::vector paths = database_paths(); @@ -455,7 +492,8 @@ void test_database_paths_ampas_env() // Set AMPAS_DATA_PATH (deprecated) unset_env_var( "RAWTOACES_DATA_PATH" ); set_env_var( - "AMPAS_DATA_PATH", to_os_path( "/deprecated/path1:/deprecated/path2" ) ); + "AMPAS_DATA_PATH", + to_os_path( "/deprecated/path1:/deprecated/path2" ) ); std::vector paths = database_paths(); @@ -473,9 +511,11 @@ void test_database_paths_both_env() std::cout << std::endl << "test_database_paths_both_env()" << std::endl; // Set both environment variables set_env_var( - "RAWTOACES_DATA_PATH", to_os_path( "/preferred/path1:/preferred/path2" ) ); + "RAWTOACES_DATA_PATH", + to_os_path( "/preferred/path1:/preferred/path2" ) ); set_env_var( - "AMPAS_DATA_PATH", to_os_path( "/deprecated/path1:/deprecated/path2" ) ); + "AMPAS_DATA_PATH", + to_os_path( "/deprecated/path1:/deprecated/path2" ) ); std::vector paths = database_paths(); @@ -493,16 +533,20 @@ void test_database_paths_both_env() /// Verifies that override_path takes precedence over environment variables void test_database_paths_override_path() { - std::cout << std::endl << "test_database_paths_override_path()" << std::endl; + std::cout << std::endl + << "test_database_paths_override_path()" << std::endl; // Set environment variables to ensure they are overridden set_env_var( "RAWTOACES_DATA_PATH", to_os_path( "/env/path1:/env/path2" ) ); - set_env_var( "AMPAS_DATA_PATH", to_os_path( "/deprecated/path1:/deprecated/path2" ) ); + set_env_var( + "AMPAS_DATA_PATH", + to_os_path( "/deprecated/path1:/deprecated/path2" ) ); // Test with override path - should take precedence over environment variables - std::string override_path = to_os_path( "/override/path1:/override/path2:/override/path3" ); + std::string override_path = + to_os_path( "/override/path1:/override/path2:/override/path3" ); std::vector paths = database_paths( override_path ); - + // Should have 3 paths from override OIIO_CHECK_EQUAL( paths.size(), 3 ); OIIO_CHECK_EQUAL( paths[0], to_os_path( "/override/path1" ) ); @@ -511,7 +555,7 @@ void test_database_paths_override_path() // Test with empty override path - should fall back to environment variables paths = database_paths( "" ); - + // Should have 2 paths from RAWTOACES_DATA_PATH environment variable OIIO_CHECK_EQUAL( paths.size(), 2 ); OIIO_CHECK_EQUAL( paths[0], to_os_path( "/env/path1" ) ); @@ -525,12 +569,14 @@ void test_database_paths_override_path() /// Tests convert_linux_path_to_windows_path utility function void test_convert_linux_path_to_windows_path() { - std::cout << std::endl << "test_convert_linux_path_to_windows_path()" << std::endl; - + std::cout << std::endl + << "test_convert_linux_path_to_windows_path()" << std::endl; + // Test single path with forward slashes - std::string result = convert_linux_path_to_windows_path( "/usr/local/share" ); + std::string result = + convert_linux_path_to_windows_path( "/usr/local/share" ); OIIO_CHECK_EQUAL( result, "c:\\usr\\local\\share" ); - + // Test multiple paths separated by ':' result = convert_linux_path_to_windows_path( "/path1:/path2:/path3" ); OIIO_CHECK_EQUAL( result, "c:\\path1;c:\\path2;c:\\path3" ); @@ -616,79 +662,43 @@ void test_fix_metadata_unsupported_type() OIIO_CHECK_EQUAL( spec.find_attribute( "Make" ), nullptr ); } -/// Helper function to set up test environment and capture output for parse_parameters tests -struct ParseParametersTestResult -{ - bool success; - std::string output; -}; - -/// Executes a parse_parameters test with the given command-line arguments and captures its output. -/// -/// This helper function encapsulates all the common setup required for testing ImageConverter::parse_parameters, -/// including environment configuration, argument parsing, stdout capture, and cleanup. -/// -/// @param args Vector of command-line arguments to pass to parse_parameters (excluding program name). -/// For example, {"--list-cameras"} or {"--list-illuminants", "--verbose"}. -/// @param database_path Path to the test database directory (optional, uses default if not provided) -/// -/// @return ParseParametersTestResult containing: -/// - success: true if parse_parameters executed successfully, false if argument parsing failed -/// - output: captured stdout output from the parse_parameters execution -ParseParametersTestResult run_parse_parameters_test( - const std::vector &args, - const std::string &database_path = "" ) +std::string run_rawtoaces_with_data_dir( + std::vector &args, + const std::string &datab_path, + bool use_dir_path_arg = false ) { - // Set up test data path to use the provided database path or default - set_env_var( "RAWTOACES_DATA_PATH", database_path.c_str() ); - - // Create ImageConverter instance - rta::util::ImageConverter converter; - - // Create argument parser and initialize it - OIIO::ArgParse arg_parser; - converter.init_parser( arg_parser ); - - // Convert args to char* array for parsing - std::vector argv; - argv.push_back( "rawtoaces" ); // Program name - for ( const auto &arg: args ) + if ( use_dir_path_arg ) { - argv.push_back( arg.c_str() ); + args.push_back( "--data-dir" ); + args.push_back( datab_path ); + unset_env_var( "RAWTOACES_DATA_PATH" ); } - - // Parse the arguments - int parse_result = - arg_parser.parse_args( static_cast( argv.size() ), argv.data() ); - if ( parse_result != 0 ) + else { - unset_env_var( "RAWTOACES_DATA_PATH" ); - return { false, "" }; + set_env_var( "RAWTOACES_DATA_PATH", datab_path ); } - // Capture stdout to verify the output - std::ostringstream captured_output; - std::streambuf *original_cout = std::cout.rdbuf(); - std::cout.rdbuf( captured_output.rdbuf() ); + std::string output = run_rawtoaces_command( args ); + OIIO::string_view output_view = output; + OIIO::Strutil::trim_whitespace( output_view ); - // Call parse_parameters - bool result = converter.parse_parameters( arg_parser ); - - // Restore original cout - std::cout.rdbuf( original_cout ); - - // Clean up environment variable - unset_env_var( "RAWTOACES_DATA_PATH" ); + if ( !use_dir_path_arg ) + { + // Clean up environment variable + unset_env_var( "RAWTOACES_DATA_PATH" ); + } - return { result, captured_output.str() }; + return output_view; } /// This test verifies that when --list-cameras is provided, the method /// calls supported_cameras() and outputs the camera list, then exits -void test_parse_parameters_list_cameras() +void test_parse_parameters_list_cameras( bool use_dir_path_arg = false ) { std::cout << std::endl - << "test_parse_parameters_list_cameras()" << std::endl; + << "test_parse_parameters_list_cameras(" + << ( use_dir_path_arg ? "with data dir" : "without data dir" ) + << ")" << std::endl; // Create test directory with dynamic database TestDirectory test_dir; @@ -699,41 +709,24 @@ void test_parse_parameters_list_cameras() test_dir.create_test_data_file( "camera", { { "manufacturer", "Mamiya" }, { "model", "Mamiya 7" } } ); - // Run the test with --list-cameras argument using the dynamic database - auto result = run_parse_parameters_test( - { "--list-cameras" }, test_dir.get_database_path() ); - - // The method should return true (though it calls exit in real usage) - OIIO_CHECK_EQUAL( result.success, true ); + std::vector args = { "--list-cameras" }; + auto output = run_rawtoaces_with_data_dir( + args, test_dir.get_database_path(), use_dir_path_arg ); - // Verify the output contains expected camera list information - OIIO_CHECK_EQUAL( - result.output.find( - "Spectral sensitivity data is available for the following cameras:" ) != - std::string::npos, - true ); + std::vector lines; + OIIO::Strutil::split( output, lines, "\n" ); - // Verify that actual camera names from test data are present - // The format is "manufacturer / model" as defined in supported_cameras() - OIIO_CHECK_EQUAL( - result.output.find( "Canon / EOS R6" ) != std::string::npos, true ); + OIIO_CHECK_EQUAL( lines.size(), 3 ); OIIO_CHECK_EQUAL( - result.output.find( "Mamiya / Mamiya 7" ) != std::string::npos, true ); - - // Count occurrences of " / " to verify we have 2 camera entries - size_t camera_count = 0; - size_t pos = 0; - while ( ( pos = result.output.find( " / ", pos ) ) != std::string::npos ) - { - camera_count++; - pos += 3; // Move past " / " - } - OIIO_CHECK_EQUAL( camera_count, 2 ); + lines[0], + "Spectral sensitivity data is available for the following cameras:" ); + OIIO_CHECK_EQUAL( lines[1], "Mamiya / Mamiya 7" ); + OIIO_CHECK_EQUAL( lines[2], "Canon / EOS R6" ); } /// This test verifies that when --list-illuminants is provided, the method /// calls supported_illuminants() and outputs the illuminant list, then exits -void test_parse_parameters_list_illuminants() +void test_parse_parameters_list_illuminants( bool use_dir_path_arg = false ) { std::cout << std::endl << "test_parse_parameters_list_illuminants()" << std::endl; @@ -745,60 +738,20 @@ void test_parse_parameters_list_illuminants() test_dir.create_test_data_file( "illuminant", { { "illuminant", "my-illuminant" } } ); - // Run the test with --list-illuminants argument using the dynamic database - auto result = run_parse_parameters_test( - { "--list-illuminants" }, test_dir.get_database_path() ); - - // The method should return true (though it calls exit in real usage) - OIIO_CHECK_EQUAL( result.success, true ); + std::vector args = { "--list-illuminants" }; - // Verify the output contains expected illuminant list information - OIIO_CHECK_EQUAL( - result.output.find( "The following illuminants are supported:" ) != - std::string::npos, - true ); - - // Verify that actual illuminant names from test data are present - // The hardcoded illuminant types should be present - OIIO_CHECK_EQUAL( - result.output.find( "Day-light (e.g., D60, D6025)" ) != - std::string::npos, - true ); - OIIO_CHECK_EQUAL( - result.output.find( "Blackbody (e.g., 3200K)" ) != std::string::npos, - true ); + // Run the test with --list-illuminants argument using the dynamic database + auto output = run_rawtoaces_with_data_dir( + args, test_dir.get_database_path(), use_dir_path_arg ); - // Verify that the specific illuminant from our test data is present - OIIO_CHECK_EQUAL( - result.output.find( "my-illuminant" ) != std::string::npos, true ); - - // Verify we have exactly 3 illuminants total (2 hardcoded + 1 from test data) - // Count newlines in the illuminant list section to verify count - size_t illuminant_count = 0; - size_t start_pos = - result.output.find( "The following illuminants are supported:" ); - if ( start_pos != std::string::npos ) - { - size_t end_pos = result.output.find( - "\n\n", start_pos ); // Look for double newline (end of list) - if ( end_pos == std::string::npos ) - { - end_pos = result.output.length(); // If no double newline, go to end - } + std::vector lines; + OIIO::Strutil::split( output, lines, "\n" ); - std::string illuminant_section = - result.output.substr( start_pos, end_pos - start_pos ); - size_t pos = 0; - while ( ( pos = illuminant_section.find( "\n", pos ) ) != - std::string::npos ) - { - illuminant_count++; - pos += 1; - } - // Subtract 1 for the header line - illuminant_count = ( illuminant_count > 0 ) ? illuminant_count - 1 : 0; - } - OIIO_CHECK_EQUAL( illuminant_count, 3 ); + OIIO_CHECK_EQUAL( lines.size(), 4 ); + OIIO_CHECK_EQUAL( lines[0], "The following illuminants are supported:" ); + OIIO_CHECK_EQUAL( lines[1], "Day-light (e.g., D60, D6025)" ); + OIIO_CHECK_EQUAL( lines[2], "Blackbody (e.g., 3200K)" ); + OIIO_CHECK_EQUAL( lines[3], "my-illuminant" ); } int main( int, char ** ) @@ -833,7 +786,9 @@ int main( int, char ** ) // Tests for parse_parameters test_parse_parameters_list_cameras(); + test_parse_parameters_list_cameras( true ); test_parse_parameters_list_illuminants(); + test_parse_parameters_list_illuminants( true ); } catch ( const std::exception &e ) { From a9eb90e81d0d1cf9807503ad9f394b93dc19ec8b Mon Sep 17 00:00:00 2001 From: Aleksandr Motsjonov Date: Mon, 22 Sep 2025 21:56:11 +1000 Subject: [PATCH 4/9] Fix windows part Signed-off-by: Aleksandr Motsjonov --- tests/test_image_converter.cpp | 50 +++++++++++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/tests/test_image_converter.cpp b/tests/test_image_converter.cpp index 0cb6f3e5..93a54279 100644 --- a/tests/test_image_converter.cpp +++ b/tests/test_image_converter.cpp @@ -13,9 +13,17 @@ #include #include #include -#include // for mkfifo #include +#ifdef WIN32 +#include +#include +#include // for _getcwd +#else +#include // for mkfifo +#include // for getcwd +#endif + #include "../src/rawtoaces_util/rawtoaces_util_priv.h" #include @@ -45,14 +53,47 @@ std::string convert_linux_path_to_windows_path( const std::string &path ) /// @return std::string containing the captured stdout output from the rawtoaces execution std::string run_rawtoaces_command( const std::vector &args ) { - // Build the command line - from build/tests/ to build/src/rawtoaces/ - std::string command = "../src/rawtoaces/rawtoaces"; + // Build the command line - use absolute path to avoid working directory issues + std::string command; +#ifdef WIN32 + // Get current working directory and build absolute path + char current_dir[1024]; + if ( _getcwd( current_dir, sizeof( current_dir ) ) != nullptr ) + { + command = std::string( current_dir ) + "\\src\\rawtoaces\\Release\\rawtoaces.exe"; + } + else + { + command = "..\\..\\src\\rawtoaces\\Release\\rawtoaces.exe"; + } +#else + command = "../src/rawtoaces/rawtoaces"; +#endif + for ( const auto &arg: args ) { command += " " + arg; } - // Execute the command and capture output +#ifdef WIN32 + // Windows implementation using _popen + FILE *pipe = _popen( command.c_str(), "r" ); + OIIO_CHECK_ASSERT( + pipe != nullptr && "Failed to execute rawtoaces command" ); + + // Read output + std::string output; + char buffer[4096]; + while ( fgets( buffer, sizeof( buffer ), pipe ) != nullptr ) + { + output += buffer; + } + + // Get exit status and validate + int exit_status = _pclose( pipe ); + OIIO_CHECK_EQUAL( exit_status, 0 ); +#else + // Unix implementation using popen FILE *pipe = popen( command.c_str(), "r" ); OIIO_CHECK_ASSERT( pipe != nullptr && "Failed to execute rawtoaces command" ); @@ -68,6 +109,7 @@ std::string run_rawtoaces_command( const std::vector &args ) // Get exit status and validate int exit_status = pclose( pipe ); OIIO_CHECK_EQUAL( WEXITSTATUS( exit_status ), 0 ); +#endif return output; } From ca8759faca8dd794a5c3e74be832101056520c11 Mon Sep 17 00:00:00 2001 From: Aleksandr Motsjonov Date: Mon, 22 Sep 2025 22:00:50 +1000 Subject: [PATCH 5/9] Fix test so order doesn't matter Signed-off-by: Aleksandr Motsjonov --- tests/test_image_converter.cpp | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/tests/test_image_converter.cpp b/tests/test_image_converter.cpp index 93a54279..0529dcd2 100644 --- a/tests/test_image_converter.cpp +++ b/tests/test_image_converter.cpp @@ -16,12 +16,12 @@ #include #ifdef WIN32 -#include -#include -#include // for _getcwd +# include +# include +# include // for _getcwd #else -#include // for mkfifo -#include // for getcwd +# include // for mkfifo +# include // for getcwd #endif #include "../src/rawtoaces_util/rawtoaces_util_priv.h" @@ -60,7 +60,8 @@ std::string run_rawtoaces_command( const std::vector &args ) char current_dir[1024]; if ( _getcwd( current_dir, sizeof( current_dir ) ) != nullptr ) { - command = std::string( current_dir ) + "\\src\\rawtoaces\\Release\\rawtoaces.exe"; + command = std::string( current_dir ) + + "\\src\\rawtoaces\\Release\\rawtoaces.exe"; } else { @@ -69,7 +70,7 @@ std::string run_rawtoaces_command( const std::vector &args ) #else command = "../src/rawtoaces/rawtoaces"; #endif - + for ( const auto &arg: args ) { command += " " + arg; @@ -762,8 +763,18 @@ void test_parse_parameters_list_cameras( bool use_dir_path_arg = false ) OIIO_CHECK_EQUAL( lines[0], "Spectral sensitivity data is available for the following cameras:" ); - OIIO_CHECK_EQUAL( lines[1], "Mamiya / Mamiya 7" ); - OIIO_CHECK_EQUAL( lines[2], "Canon / EOS R6" ); + + // Check that both cameras are present (order doesn't matter) + bool found_canon = false, found_mamiya = false; + for ( size_t i = 1; i < lines.size(); ++i ) + { + if ( lines[i] == "Canon / EOS R6" ) + found_canon = true; + if ( lines[i] == "Mamiya / Mamiya 7" ) + found_mamiya = true; + } + OIIO_CHECK_EQUAL( found_canon, true ); + OIIO_CHECK_EQUAL( found_mamiya, true ); } /// This test verifies that when --list-illuminants is provided, the method From 54a7b8d4d2f71dec51e6b98847372d5aadbeb456 Mon Sep 17 00:00:00 2001 From: Aleksandr Motsjonov Date: Mon, 22 Sep 2025 22:46:35 +1000 Subject: [PATCH 6/9] Find path for test in Win32 Signed-off-by: Aleksandr Motsjonov --- tests/test_image_converter.cpp | 38 +++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/tests/test_image_converter.cpp b/tests/test_image_converter.cpp index 2591bc57..e5daf9bb 100644 --- a/tests/test_image_converter.cpp +++ b/tests/test_image_converter.cpp @@ -58,19 +58,41 @@ std::string convert_linux_path_to_windows_path( const std::string &path ) /// @return std::string containing the captured stdout output from the rawtoaces execution std::string run_rawtoaces_command( const std::vector &args ) { - // Build the command line - use absolute path to avoid working directory issues + // Build the command line - try multiple possible paths for different environments std::string command; #ifdef WIN32 - // Get current working directory and build absolute path - char current_dir[1024]; - if ( _getcwd( current_dir, sizeof( current_dir ) ) != nullptr ) + // Try multiple possible paths in order of likelihood + std::vector possible_paths = { + "src\\rawtoaces\\Release\\rawtoaces.exe", // From project root + "build\\src\\rawtoaces\\Release\\rawtoaces.exe", // From project root with build dir + "..\\..\\src\\rawtoaces\\Release\\rawtoaces.exe", // From build/tests/Release + "..\\src\\rawtoaces\\Release\\rawtoaces.exe", // From build/tests + "..\\..\\..\\src\\rawtoaces\\Release\\rawtoaces.exe" // From deeper nested dirs + }; + + // Check if any of the possible paths exist + bool found = false; + for ( const auto &path : possible_paths ) { - command = std::string( current_dir ) + - "\\src\\rawtoaces\\Release\\rawtoaces.exe"; + std::ifstream file( path ); + if ( file.good() ) + { + command = path; + found = true; + std::cout << "DEBUG: Found rawtoaces executable at: " << path << std::endl; + break; + } + else + { + std::cout << "DEBUG: Path not found: " << path << std::endl; + } } - else + + // If no path found, use the first one as fallback + if ( !found ) { - command = "..\\..\\src\\rawtoaces\\Release\\rawtoaces.exe"; + command = possible_paths[0]; + std::cout << "DEBUG: No path found, using fallback: " << command << std::endl; } #else command = "../src/rawtoaces/rawtoaces"; From 8ef014cfebca7a9302fc2299a2bf94c3bede55d3 Mon Sep 17 00:00:00 2001 From: Aleksandr Motsjonov Date: Tue, 23 Sep 2025 00:02:25 +1000 Subject: [PATCH 7/9] correct path Signed-off-by: Aleksandr Motsjonov --- tests/test_image_converter.cpp | 35 ++-------------------------------- 1 file changed, 2 insertions(+), 33 deletions(-) diff --git a/tests/test_image_converter.cpp b/tests/test_image_converter.cpp index e5daf9bb..b8d964a4 100644 --- a/tests/test_image_converter.cpp +++ b/tests/test_image_converter.cpp @@ -61,39 +61,8 @@ std::string run_rawtoaces_command( const std::vector &args ) // Build the command line - try multiple possible paths for different environments std::string command; #ifdef WIN32 - // Try multiple possible paths in order of likelihood - std::vector possible_paths = { - "src\\rawtoaces\\Release\\rawtoaces.exe", // From project root - "build\\src\\rawtoaces\\Release\\rawtoaces.exe", // From project root with build dir - "..\\..\\src\\rawtoaces\\Release\\rawtoaces.exe", // From build/tests/Release - "..\\src\\rawtoaces\\Release\\rawtoaces.exe", // From build/tests - "..\\..\\..\\src\\rawtoaces\\Release\\rawtoaces.exe" // From deeper nested dirs - }; - - // Check if any of the possible paths exist - bool found = false; - for ( const auto &path : possible_paths ) - { - std::ifstream file( path ); - if ( file.good() ) - { - command = path; - found = true; - std::cout << "DEBUG: Found rawtoaces executable at: " << path << std::endl; - break; - } - else - { - std::cout << "DEBUG: Path not found: " << path << std::endl; - } - } - - // If no path found, use the first one as fallback - if ( !found ) - { - command = possible_paths[0]; - std::cout << "DEBUG: No path found, using fallback: " << command << std::endl; - } + // Use the standard path from build directory + command = "src\\rawtoaces\\Release\\rawtoaces.exe"; #else command = "../src/rawtoaces/rawtoaces"; #endif From 5856f555cb85771997b60e66cee6185e16190005 Mon Sep 17 00:00:00 2001 From: Aleksandr Motsjonov Date: Tue, 23 Sep 2025 08:12:15 +1000 Subject: [PATCH 8/9] try another path Signed-off-by: Aleksandr Motsjonov --- tests/test_image_converter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_image_converter.cpp b/tests/test_image_converter.cpp index b8d964a4..a63744f8 100644 --- a/tests/test_image_converter.cpp +++ b/tests/test_image_converter.cpp @@ -62,7 +62,7 @@ std::string run_rawtoaces_command( const std::vector &args ) std::string command; #ifdef WIN32 // Use the standard path from build directory - command = "src\\rawtoaces\\Release\\rawtoaces.exe"; + command = "..\\src\\rawtoaces\\Release\\rawtoaces.exe"; #else command = "../src/rawtoaces/rawtoaces"; #endif From f4992a710a40a68672d5b82aa2dad4eea37507f9 Mon Sep 17 00:00:00 2001 From: Aleksandr Motsjonov Date: Tue, 23 Sep 2025 23:03:02 +1000 Subject: [PATCH 9/9] Last touch ups / PR review address Signed-off-by: Aleksandr Motsjonov --- tests/test_image_converter.cpp | 55 ++++++++++++++++------------------ 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/tests/test_image_converter.cpp b/tests/test_image_converter.cpp index a63744f8..8481136e 100644 --- a/tests/test_image_converter.cpp +++ b/tests/test_image_converter.cpp @@ -29,9 +29,6 @@ # include // for getcwd #endif -#include "../src/rawtoaces_util/rawtoaces_util_priv.h" -#include - using namespace rta::util; std::string convert_linux_path_to_windows_path( const std::string &path ) @@ -50,6 +47,25 @@ std::string convert_linux_path_to_windows_path( const std::string &path ) return OIIO::Strutil::join( segments, ";" ); } +FILE *platform_popen( const char *command, const char *mode ) +{ +#ifdef WIN32 + return _popen( command, mode ); +#else + return popen( command, mode ); +#endif +} + +int platform_pclose( FILE *pipe ) +{ +#ifdef WIN32 + return _pclose( pipe ); +#else + int status = pclose( pipe ); + return WEXITSTATUS( status ); +#endif +} + /// Executes a rawtoaces command with the given arguments and captures its output. /// /// @param args Vector of command-line arguments to pass to rawtoaces (excluding program name). @@ -72,9 +88,8 @@ std::string run_rawtoaces_command( const std::vector &args ) command += " " + arg; } -#ifdef WIN32 - // Windows implementation using _popen - FILE *pipe = _popen( command.c_str(), "r" ); + // Execute command using platform-specific functions + FILE *pipe = platform_popen( command.c_str(), "r" ); OIIO_CHECK_ASSERT( pipe != nullptr && "Failed to execute rawtoaces command" ); @@ -87,26 +102,8 @@ std::string run_rawtoaces_command( const std::vector &args ) } // Get exit status and validate - int exit_status = _pclose( pipe ); + int exit_status = platform_pclose( pipe ); OIIO_CHECK_EQUAL( exit_status, 0 ); -#else - // Unix implementation using popen - FILE *pipe = popen( command.c_str(), "r" ); - OIIO_CHECK_ASSERT( - pipe != nullptr && "Failed to execute rawtoaces command" ); - - // Read output - std::string output; - char buffer[4096]; - while ( fgets( buffer, sizeof( buffer ), pipe ) != nullptr ) - { - output += buffer; - } - - // Get exit status and validate - int exit_status = pclose( pipe ); - OIIO_CHECK_EQUAL( WEXITSTATUS( exit_status ), 0 ); -#endif return output; } @@ -118,7 +115,7 @@ getenv() - Part of standard C library (C89/C99) - available everywhere setenv()/unsetenv() - Part of POSIX standard - only on Unix-like systems */ #ifdef WIN32 -void set_env_var( std::string name, std::string value ) +void set_env_var( const std::string &name, const std::string &value ) { _putenv_s( name.c_str(), value.c_str() ); } @@ -128,7 +125,7 @@ std::string to_os_path( std::string linux_path ) return convert_linux_path_to_windows_path( linux_path ); } -void unset_env_var( std::string name ) +void unset_env_var( const std::string &name ) { _putenv_s( name.c_str(), "" ); } @@ -138,12 +135,12 @@ std::string to_os_path( std::string linux_path ) return linux_path; } -void set_env_var( std::string name, std::string value ) +void set_env_var( const std::string &name, const std::string &value ) { setenv( name.c_str(), value.c_str(), 1 ); } -void unset_env_var( std::string name ) +void unset_env_var( const std::string &name ) { unsetenv( name.c_str() ); }