From f2d8c9a94318246d5581df1f746dd69eb8af5f0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9drik=20Fuoco?= Date: Wed, 15 Mar 2023 13:22:50 -0400 Subject: [PATCH 1/7] Fixinactive colorspaces in cg-config. Fix OpenColorIO errors that was introduced a while ago in the unit test because of requirement for config. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Cédrik Fuoco --- .../cg-config-v1.0.0_aces-v1.3_ocio-v2.1.ocio | 2 +- tests/cpu/Config_tests.cpp | 59 +++------- tests/cpu/FileRules_tests.cpp | 111 ++++++++++++++---- tests/cpu/UnitTestLogUtils.cpp | 50 ++++++++ tests/cpu/UnitTestLogUtils.h | 5 + .../BuiltinTransformRegistry_tests.cpp | 23 ++++ 6 files changed, 181 insertions(+), 69 deletions(-) diff --git a/src/OpenColorIO/builtinconfigs/configs/cg-config-v1.0.0_aces-v1.3_ocio-v2.1.ocio b/src/OpenColorIO/builtinconfigs/configs/cg-config-v1.0.0_aces-v1.3_ocio-v2.1.ocio index bae8f68094..c9e9bd203b 100644 --- a/src/OpenColorIO/builtinconfigs/configs/cg-config-v1.0.0_aces-v1.3_ocio-v2.1.ocio +++ b/src/OpenColorIO/builtinconfigs/configs/cg-config-v1.0.0_aces-v1.3_ocio-v2.1.ocio @@ -54,7 +54,7 @@ displays: active_displays: [sRGB - Display, Rec.1886 Rec.709 - Display, Rec.2100-PQ - Display, ST2084-P3-D65 - Display, P3-D65 - Display] active_views: [ACES 1.0 - SDR Video, ACES 1.1 - HDR Video (1000 nits & Rec.2020 lim), ACES 1.1 - HDR Video (1000 nits & P3 lim), ACES 1.0 - SDR Cinema, Un-tone-mapped, Raw] -inactive_colorspaces: [CIE-XYZ-D65, sRGB - Display, Rec.1886 Rec.709 - Display, Rec.1886 Rec.2020 - Display, sRGB - Display, Rec.1886 Rec.709 - Display, Rec.1886 Rec.2020 - Display, Rec.1886 Rec.2020 - Display, Rec.2100-HLG - Display, Rec.2100-PQ - Display, Rec.2100-PQ - Display, Rec.2100-PQ - Display, ST2084-P3-D65 - Display, ST2084-P3-D65 - Display, ST2084-P3-D65 - Display, P3-D60 - Display, P3-D65 - Display, P3-D65 - Display, P3-D65 - Display, P3-DCI - Display, P3-DCI - Display, ST2084-P3-D65 - Display] +inactive_colorspaces: [CIE-XYZ-D65, sRGB - Display, Rec.1886 Rec.709 - Display, sRGB - Display, Rec.1886 Rec.709 - Display, Rec.2100-PQ - Display, Rec.2100-PQ - Display, Rec.2100-PQ - Display, ST2084-P3-D65 - Display, ST2084-P3-D65 - Display, ST2084-P3-D65 - Display, P3-D65 - Display, P3-D65 - Display, P3-D65 - Display, ST2084-P3-D65 - Display] looks: - ! diff --git a/tests/cpu/Config_tests.cpp b/tests/cpu/Config_tests.cpp index 76b668fc8e..adf0e085c8 100644 --- a/tests/cpu/Config_tests.cpp +++ b/tests/cpu/Config_tests.cpp @@ -361,47 +361,8 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); - - StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - OCIO_CHECK_ASSERT( - StringUtils::Contain( - svec, - "[OpenColorIO Error]: The scene_linear role is required for a config version 2.2 "\ - "or higher." - ) - ); - - OCIO_CHECK_ASSERT( - StringUtils::Contain( - svec, - "[OpenColorIO Error]: The compositing_log role is required for a config version "\ - "2.2 or higher." - ) - ); - - OCIO_CHECK_ASSERT( - StringUtils::Contain( - svec, - "[OpenColorIO Error]: The color_timing role is required for a config version 2.2 "\ - "or higher." - ) - ); - - OCIO_CHECK_ASSERT( - StringUtils::Contain( - svec, - "[OpenColorIO Error]: The aces_interchange role is required when there are "\ - "scene-referred color spaces and the config version is 2.2 or higher." - ) - ); - - OCIO_CHECK_ASSERT( - StringUtils::Contain( - svec, - "[OpenColorIO Error]: The cie_xyz_d65_interchange role is required when there are"\ - " display-referred color spaces and the config version is 2.2 or higher." - ) - ); + // Expecting error since the major version was changed to version 2 without any modifications. + OCIO::checkAllRequiredRolesErrors(logGuard); } // Set colorspace for all required roles. @@ -6369,10 +6330,20 @@ OCIO_ADD_TEST(Config, display_view) auto cs = OCIO::ColorSpace::Create(OCIO::REFERENCE_SPACE_SCENE); cs->setName("scs"); config->addColorSpace(cs); + + // Set colorspace for required roles. + config->setRole(OCIO::ROLE_SCENE_LINEAR, cs->getName()); + config->setRole(OCIO::ROLE_INTERCHANGE_SCENE, cs->getName()); + cs = OCIO::ColorSpace::Create(OCIO::REFERENCE_SPACE_DISPLAY); cs->setName("dcs"); config->addColorSpace(cs); + // Set colorspace for required roles. + config->setRole(OCIO::ROLE_COMPOSITING_LOG, cs->getName()); + config->setRole(OCIO::ROLE_COLOR_TIMING, cs->getName()); + config->setRole(OCIO::ROLE_INTERCHANGE_DISPLAY, cs->getName()); + // Add a scene-referred and a display-referred view transform. auto vt = OCIO::ViewTransform::Create(OCIO::REFERENCE_SPACE_DISPLAY); vt->setName("display"); @@ -6415,7 +6386,11 @@ strictparsing: true luma: [0.2126, 0.7152, 0.0722] roles: - {} + aces_interchange: scs + cie_xyz_d65_interchange: dcs + color_timing: dcs + compositing_log: dcs + scene_linear: scs file_rules: - ! {name: Default, colorspace: default} diff --git a/tests/cpu/FileRules_tests.cpp b/tests/cpu/FileRules_tests.cpp index 1163499418..38d93cfc23 100644 --- a/tests/cpu/FileRules_tests.cpp +++ b/tests/cpu/FileRules_tests.cpp @@ -1372,7 +1372,13 @@ strictparsing: true // Upgrading is making sure to build a valid v2 config. config->upgradeToLatestVersion(); - OCIO_CHECK_NO_THROW(config->validate()); + + { + OCIO::LogGuard logGuard; + OCIO_CHECK_NO_THROW(config->validate()); + // Expecting error since the major version was changed to version 2 without any modifications. + OCIO::checkRequiredRolesErrors(logGuard); + } // Check the new version. @@ -1439,7 +1445,13 @@ strictparsing: true // Upgrading is making sure to build a valid v2 config. config->upgradeToLatestVersion(); - OCIO_CHECK_NO_THROW(config->validate()); + + { + OCIO::LogGuard logGuard; + OCIO_CHECK_NO_THROW(config->validate()); + // Expecting error since the major version was changed to version 2 without any modifications. + OCIO::checkRequiredRolesErrors(logGuard); + } // Check the new version. @@ -1514,7 +1526,13 @@ strictparsing: true cfg->setInactiveColorSpaces("cs1"); cfg->upgradeToLatestVersion(); - OCIO_CHECK_NO_THROW(cfg->validate()); + + { + OCIO::LogGuard logGuard; + OCIO_CHECK_NO_THROW(cfg->validate()); + // Expecting error since the major version was changed to version 2 without any modifications. + OCIO::checkRequiredRolesErrors(logGuard); + } // Check the new version. @@ -1554,7 +1572,12 @@ strictparsing: true l.output()); } - OCIO_CHECK_NO_THROW(cfg->validate()); + { + OCIO::LogGuard logGuard; + OCIO_CHECK_NO_THROW(cfg->validate()); + // Expecting error since the major version was changed to version 2 without any modifications. + OCIO::checkRequiredRolesErrors(logGuard); + } // Check the new version. @@ -1602,14 +1625,26 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) // Default rule is using 'Default' role that does not exist. config->setMajorVersion(2); - OCIO_CHECK_THROW_WHAT(config->validate(), OCIO::Exception, "rule named 'Default' is " - "referencing 'default' that is neither a color space nor a named " - "transform"); + + { + OCIO::LogGuard logGuard; + OCIO_CHECK_THROW_WHAT(config->validate(), OCIO::Exception, "rule named 'Default' is " + "referencing 'default' that is neither a color space nor a named " + "transform"); + // Expecting error since the major version was changed to version 2 without any modifications. + OCIO::checkRequiredRolesErrors(logGuard); + } // Upgrading is making sure to build a valid v2 config. config->setMajorVersion(1); config->upgradeToLatestVersion(); - OCIO_CHECK_NO_THROW(config->validate()); + + { + OCIO::LogGuard logGuard; + OCIO_CHECK_NO_THROW(config->validate()); + // Expecting error since the major version was changed to version 2 without any modifications. + OCIO::checkRequiredRolesErrors(logGuard); + } // Check the new version. @@ -1640,14 +1675,26 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) // Default rule is using 'Default' role but the associated color space does not exist. config->setMajorVersion(2); - OCIO_CHECK_THROW_WHAT(config->validate(), OCIO::Exception, "rule named 'Default' is " - "referencing 'default' that is neither a color space nor a named " - "transform"); + + { + OCIO::LogGuard logGuard; + OCIO_CHECK_THROW_WHAT(config->validate(), OCIO::Exception, "rule named 'Default' is " + "referencing 'default' that is neither a color space nor a named " + "transform"); + // Expecting error since the major version was changed to version 2 without any modifications. + OCIO::checkRequiredRolesErrors(logGuard); + } // Upgrading is making sure to build a valid v2 config. config->setMajorVersion(1); config->upgradeToLatestVersion(); - OCIO_CHECK_NO_THROW(config->validate()); + + { + OCIO::LogGuard logGuard; + OCIO_CHECK_NO_THROW(config->validate()); + // Expecting error since the major version was changed to version 2 without any modifications. + OCIO::checkRequiredRolesErrors(logGuard); + } // Check the new version. @@ -1678,27 +1725,39 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) // Default rule is using 'Default' role but the associated color space does not exist. config->setInactiveColorSpaces("cs1"); config->setMajorVersion(2); - OCIO_CHECK_THROW_WHAT(config->validate(), OCIO::Exception, "rule named 'Default' is " - "referencing 'default' that is neither a color space nor a named " - "transform"); + + { + OCIO::LogGuard logGuard; + OCIO_CHECK_THROW_WHAT(config->validate(), OCIO::Exception, "rule named 'Default' is " + "referencing 'default' that is neither a color space nor a named " + "transform"); + // Expecting error since the major version was changed to version 2 without any modifications. + OCIO::checkRequiredRolesErrors(logGuard); + } config->setMajorVersion(1); - + { - OCIO::LogGuard l; - + OCIO::LogGuard logGuard; config->upgradeToLatestVersion(); - - OCIO_CHECK_EQUAL( - std::string("[OpenColorIO Warning]: The default rule creation falls back to the"\ - " first color space because no suitable color space exists.\n"), - l.output()); + StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); + OCIO_CHECK_ASSERT( + StringUtils::Contain( + svec, + "[OpenColorIO Warning]: The default rule creation falls back to the"\ + " first color space because no suitable color space exists." + ) + ); + } + + { + OCIO::LogGuard logGuard; + OCIO_CHECK_NO_THROW(config->validate()); + // Expecting error since the major version was changed to version 2 without any modifications. + OCIO::checkRequiredRolesErrors(logGuard); } - - OCIO_CHECK_NO_THROW(config->validate()); // Check the new version. - OCIO_CHECK_EQUAL(config->getMajorVersion(), 2); // Check the 'default' rule. As there is not 'data' or active color space, the default diff --git a/tests/cpu/UnitTestLogUtils.cpp b/tests/cpu/UnitTestLogUtils.cpp index 28a2d3ee55..e01866c896 100644 --- a/tests/cpu/UnitTestLogUtils.cpp +++ b/tests/cpu/UnitTestLogUtils.cpp @@ -64,5 +64,55 @@ MuteLogging::~MuteLogging() ResetToDefaultLoggingFunction(); } +void checkAllRequiredRolesErrors(LogGuard & logGuard) +{ + checkRequiredRolesErrors(logGuard); + + StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); + OCIO_CHECK_ASSERT( + StringUtils::Contain( + svec, + "[OpenColorIO Error]: The aces_interchange role is required when there are "\ + "scene-referred color spaces and the config version is 2.2 or higher." + ) + ); +} + +void checkRequiredRolesErrors(LogGuard & logGuard) +{ + StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); + OCIO_CHECK_ASSERT( + StringUtils::Contain( + svec, + "[OpenColorIO Error]: The scene_linear role is required for a config version 2.2 "\ + "or higher." + ) + ); + + OCIO_CHECK_ASSERT( + StringUtils::Contain( + svec, + "[OpenColorIO Error]: The compositing_log role is required for a config version "\ + "2.2 or higher." + ) + ); + + OCIO_CHECK_ASSERT( + StringUtils::Contain( + svec, + "[OpenColorIO Error]: The color_timing role is required for a config version 2.2 "\ + "or higher." + ) + ); + + OCIO_CHECK_ASSERT( + StringUtils::Contain( + svec, + "[OpenColorIO Error]: The aces_interchange role is required when there are "\ + "scene-referred color spaces and the config version is 2.2 or higher." + ) + ); +} + } // namespace OCIO_NAMESPACE diff --git a/tests/cpu/UnitTestLogUtils.h b/tests/cpu/UnitTestLogUtils.h index aa961ba4a6..e6e81ed257 100644 --- a/tests/cpu/UnitTestLogUtils.h +++ b/tests/cpu/UnitTestLogUtils.h @@ -5,6 +5,8 @@ #define INCLUDED_OCIO_UNITTEST_LOGUTILS_H #include +#include "testutils/UnitTest.h" +#include "utils/StringUtils.h" namespace OCIO_NAMESPACE { @@ -37,6 +39,9 @@ class MuteLogging ~MuteLogging(); }; +void checkAllRequiredRolesErrors(LogGuard & logGuard); +void checkRequiredRolesErrors(LogGuard & logGuard); + } // namespace OCIO_NAMESPACE #endif // INCLUDED_OCIO_UNITTEST_LOGUTILS_H diff --git a/tests/cpu/transforms/builtins/BuiltinTransformRegistry_tests.cpp b/tests/cpu/transforms/builtins/BuiltinTransformRegistry_tests.cpp index 8eb529f06c..4abd04a627 100644 --- a/tests/cpu/transforms/builtins/BuiltinTransformRegistry_tests.cpp +++ b/tests/cpu/transforms/builtins/BuiltinTransformRegistry_tests.cpp @@ -105,7 +105,12 @@ strictparsing: true luma: [0.2126, 0.7152, 0.0722] roles: + aces_interchange: test + cie_xyz_d65_interchange: dcs + color_timing: test + compositing_log: test default: ref + scene_linear: test file_rules: - ! {name: Default, colorspace: default} @@ -117,6 +122,24 @@ luma: [0.2126, 0.7152, 0.0722] active_displays: [] active_views: [] +view_transforms: + - ! + name: display + from_display_reference: ! {} + + - ! + name: view_transform + from_scene_reference: ! {} + +display_colorspaces: + - ! + name: dcs + family: "" + equalitygroup: "" + bitdepth: unknown + isdata: false + allocation: uniform + colorspaces: - ! name: ref From f677e5ade4f029b370928d2ee3f9af7da59d4452 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9drik=20Fuoco?= Date: Fri, 17 Mar 2023 09:18:38 -0400 Subject: [PATCH 2/7] Reverting changes in cg-config Other warnings are not muted anymore when we check for role warnings. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Cédrik Fuoco --- .../cg-config-v1.0.0_aces-v1.3_ocio-v2.1.ocio | 2 +- tests/cpu/Config_tests.cpp | 31 ++---- tests/cpu/FileRules_tests.cpp | 35 ++++--- tests/cpu/UnitTestLogUtils.cpp | 97 +++++++++++-------- tests/cpu/UnitTestLogUtils.h | 4 +- .../BuiltinTransformRegistry_tests.cpp | 19 ---- 6 files changed, 87 insertions(+), 101 deletions(-) diff --git a/src/OpenColorIO/builtinconfigs/configs/cg-config-v1.0.0_aces-v1.3_ocio-v2.1.ocio b/src/OpenColorIO/builtinconfigs/configs/cg-config-v1.0.0_aces-v1.3_ocio-v2.1.ocio index c9e9bd203b..bae8f68094 100644 --- a/src/OpenColorIO/builtinconfigs/configs/cg-config-v1.0.0_aces-v1.3_ocio-v2.1.ocio +++ b/src/OpenColorIO/builtinconfigs/configs/cg-config-v1.0.0_aces-v1.3_ocio-v2.1.ocio @@ -54,7 +54,7 @@ displays: active_displays: [sRGB - Display, Rec.1886 Rec.709 - Display, Rec.2100-PQ - Display, ST2084-P3-D65 - Display, P3-D65 - Display] active_views: [ACES 1.0 - SDR Video, ACES 1.1 - HDR Video (1000 nits & Rec.2020 lim), ACES 1.1 - HDR Video (1000 nits & P3 lim), ACES 1.0 - SDR Cinema, Un-tone-mapped, Raw] -inactive_colorspaces: [CIE-XYZ-D65, sRGB - Display, Rec.1886 Rec.709 - Display, sRGB - Display, Rec.1886 Rec.709 - Display, Rec.2100-PQ - Display, Rec.2100-PQ - Display, Rec.2100-PQ - Display, ST2084-P3-D65 - Display, ST2084-P3-D65 - Display, ST2084-P3-D65 - Display, P3-D65 - Display, P3-D65 - Display, P3-D65 - Display, ST2084-P3-D65 - Display] +inactive_colorspaces: [CIE-XYZ-D65, sRGB - Display, Rec.1886 Rec.709 - Display, Rec.1886 Rec.2020 - Display, sRGB - Display, Rec.1886 Rec.709 - Display, Rec.1886 Rec.2020 - Display, Rec.1886 Rec.2020 - Display, Rec.2100-HLG - Display, Rec.2100-PQ - Display, Rec.2100-PQ - Display, Rec.2100-PQ - Display, ST2084-P3-D65 - Display, ST2084-P3-D65 - Display, ST2084-P3-D65 - Display, P3-D60 - Display, P3-D65 - Display, P3-D65 - Display, P3-D65 - Display, P3-DCI - Display, P3-DCI - Display, ST2084-P3-D65 - Display] looks: - ! diff --git a/tests/cpu/Config_tests.cpp b/tests/cpu/Config_tests.cpp index adf0e085c8..def0bb9080 100644 --- a/tests/cpu/Config_tests.cpp +++ b/tests/cpu/Config_tests.cpp @@ -357,12 +357,13 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) } { - // Test that all errors appears when all required roles are missing. + // Test that all errors appear when all required roles are missing. OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); - // Expecting error since the major version was changed to version 2 without any modifications. - OCIO::checkAllRequiredRolesErrors(logGuard); + // Check that the log contains the expected error messages for the missing roles and mute + // them so that (only) those messages don't appear in the test output. + OCIO::checkAllRequiredRolesErrors(logGuard, true); } // Set colorspace for all required roles. @@ -379,9 +380,7 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) OCIO_CHECK_NO_THROW(config->validate()); StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - OCIO_CHECK_ASSERT( - !StringUtils::Contain(svec, "[OpenColorIO Error]") - ); + OCIO_CHECK_ASSERT(StringUtils::Contain(svec, "")); } { @@ -6326,24 +6325,16 @@ OCIO_ADD_TEST(Config, display_view) config->addColorSpace(cs); } + config->setVersion(2, 1); + // Add a scene-referred and a display-referred color space. auto cs = OCIO::ColorSpace::Create(OCIO::REFERENCE_SPACE_SCENE); cs->setName("scs"); config->addColorSpace(cs); - - // Set colorspace for required roles. - config->setRole(OCIO::ROLE_SCENE_LINEAR, cs->getName()); - config->setRole(OCIO::ROLE_INTERCHANGE_SCENE, cs->getName()); - cs = OCIO::ColorSpace::Create(OCIO::REFERENCE_SPACE_DISPLAY); cs->setName("dcs"); config->addColorSpace(cs); - // Set colorspace for required roles. - config->setRole(OCIO::ROLE_COMPOSITING_LOG, cs->getName()); - config->setRole(OCIO::ROLE_COLOR_TIMING, cs->getName()); - config->setRole(OCIO::ROLE_INTERCHANGE_DISPLAY, cs->getName()); - // Add a scene-referred and a display-referred view transform. auto vt = OCIO::ViewTransform::Create(OCIO::REFERENCE_SPACE_DISPLAY); vt->setName("display"); @@ -6377,7 +6368,7 @@ OCIO_ADD_TEST(Config, display_view) std::stringstream os; os << *config.get(); - constexpr char expected[]{ R"(ocio_profile_version: 2.2 + constexpr char expected[]{ R"(ocio_profile_version: 2.1 environment: {} @@ -6386,11 +6377,7 @@ strictparsing: true luma: [0.2126, 0.7152, 0.0722] roles: - aces_interchange: scs - cie_xyz_d65_interchange: dcs - color_timing: dcs - compositing_log: dcs - scene_linear: scs + {} file_rules: - ! {name: Default, colorspace: default} diff --git a/tests/cpu/FileRules_tests.cpp b/tests/cpu/FileRules_tests.cpp index 38d93cfc23..0d089f6fad 100644 --- a/tests/cpu/FileRules_tests.cpp +++ b/tests/cpu/FileRules_tests.cpp @@ -1377,7 +1377,7 @@ strictparsing: true OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); // Expecting error since the major version was changed to version 2 without any modifications. - OCIO::checkRequiredRolesErrors(logGuard); + OCIO::checkRequiredRolesErrors(logGuard, true); } // Check the new version. @@ -1449,8 +1449,9 @@ strictparsing: true { OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); - // Expecting error since the major version was changed to version 2 without any modifications. - OCIO::checkRequiredRolesErrors(logGuard); + // Ignore (only) the errors logged regarding the missing roles that are required in + // configs with version >= 2.2. + OCIO::checkRequiredRolesErrors(logGuard, true); } // Check the new version. @@ -1530,8 +1531,9 @@ strictparsing: true { OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(cfg->validate()); - // Expecting error since the major version was changed to version 2 without any modifications. - OCIO::checkRequiredRolesErrors(logGuard); + // Ignore (only) the errors logged regarding the missing roles that are required in + // configs with version >= 2.2. + OCIO::checkRequiredRolesErrors(logGuard, true); } // Check the new version. @@ -1575,8 +1577,9 @@ strictparsing: true { OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(cfg->validate()); - // Expecting error since the major version was changed to version 2 without any modifications. - OCIO::checkRequiredRolesErrors(logGuard); + // Ignore (only) the errors logged regarding the missing roles that are required in + // configs with version >= 2.2. + OCIO::checkRequiredRolesErrors(logGuard, true); } // Check the new version. @@ -1621,7 +1624,7 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) OCIO::ColorSpaceRcPtr raw = OCIO::ColorSpace::Create(); raw->setName("rAw"); config->addColorSpace(raw); - OCIO_CHECK_NO_THROW(config->validate()); // because file rules are not validated. + OCIO_CHECK_NO_THROW(config->validate()); // (does not fail since the major version is 1) // Default rule is using 'Default' role that does not exist. config->setMajorVersion(2); @@ -1632,7 +1635,7 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) "referencing 'default' that is neither a color space nor a named " "transform"); // Expecting error since the major version was changed to version 2 without any modifications. - OCIO::checkRequiredRolesErrors(logGuard); + OCIO::checkRequiredRolesErrors(logGuard, true); } // Upgrading is making sure to build a valid v2 config. @@ -1643,7 +1646,7 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); // Expecting error since the major version was changed to version 2 without any modifications. - OCIO::checkRequiredRolesErrors(logGuard); + OCIO::checkRequiredRolesErrors(logGuard, true); } // Check the new version. @@ -1671,7 +1674,7 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) OCIO::ColorSpaceRcPtr raw = OCIO::ColorSpace::Create(); raw->setName("rAw"); config->addColorSpace(raw); - OCIO_CHECK_NO_THROW(config->validate()); // because file rules are not validated. + OCIO_CHECK_NO_THROW(config->validate()); // (does not fail since the major version is 1) // Default rule is using 'Default' role but the associated color space does not exist. config->setMajorVersion(2); @@ -1682,7 +1685,7 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) "referencing 'default' that is neither a color space nor a named " "transform"); // Expecting error since the major version was changed to version 2 without any modifications. - OCIO::checkRequiredRolesErrors(logGuard); + OCIO::checkRequiredRolesErrors(logGuard, true); } // Upgrading is making sure to build a valid v2 config. @@ -1693,7 +1696,7 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); // Expecting error since the major version was changed to version 2 without any modifications. - OCIO::checkRequiredRolesErrors(logGuard); + OCIO::checkRequiredRolesErrors(logGuard, true); } // Check the new version. @@ -1720,7 +1723,7 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) OCIO::ColorSpaceRcPtr cs1 = OCIO::ColorSpace::Create(); cs1->setName("cs1"); config->addColorSpace(cs1); - OCIO_CHECK_NO_THROW(config->validate()); // because file rules are not validated. + OCIO_CHECK_NO_THROW(config->validate()); // (does not fail since the major version is 1) // Default rule is using 'Default' role but the associated color space does not exist. config->setInactiveColorSpaces("cs1"); @@ -1732,7 +1735,7 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) "referencing 'default' that is neither a color space nor a named " "transform"); // Expecting error since the major version was changed to version 2 without any modifications. - OCIO::checkRequiredRolesErrors(logGuard); + OCIO::checkRequiredRolesErrors(logGuard, true); } config->setMajorVersion(1); @@ -1754,7 +1757,7 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); // Expecting error since the major version was changed to version 2 without any modifications. - OCIO::checkRequiredRolesErrors(logGuard); + OCIO::checkRequiredRolesErrors(logGuard, true); } // Check the new version. diff --git a/tests/cpu/UnitTestLogUtils.cpp b/tests/cpu/UnitTestLogUtils.cpp index e01866c896..1798b5b892 100644 --- a/tests/cpu/UnitTestLogUtils.cpp +++ b/tests/cpu/UnitTestLogUtils.cpp @@ -4,6 +4,8 @@ #include +#include +#include "Platform.h" #include "UnitTestLogUtils.h" namespace OCIO_NAMESPACE @@ -64,54 +66,67 @@ MuteLogging::~MuteLogging() ResetToDefaultLoggingFunction(); } -void checkAllRequiredRolesErrors(LogGuard & logGuard) + +void checkAndRemove(std::vector & svec, const std::string & str) { - checkRequiredRolesErrors(logGuard); + size_t index = -1; + size_t i = -1; + for (size_t i = 0; i < svec.size(); i++) + { + if (Platform::Strcasecmp(svec[i].c_str(), str.c_str()) == 0) + { + index = i; + break; + } + } + + OCIO_CHECK_GT(-1, index); + svec.erase(svec.begin() + index); +} + +void checkRequiredRolesErrors(StringUtils::StringVec & svec, bool printLog) +{ + const std::string interchange_scene = "[OpenColorIO Error]: The scene_linear role is "\ + "required for a config version 2.2 or higher."; + const std::string compositing_log = "[OpenColorIO Error]: The compositing_log role is "\ + "required for a config version 2.2 or higher."; + + const std::string color_timing = "[OpenColorIO Error]: The color_timing role is required "\ + "for a config version 2.2 or higher."; + + const std::string aces_interchange = "[OpenColorIO Error]: The aces_interchange role is "\ + "required when there are scene-referred color spaces and "\ + "the config version is 2.2 or higher."; + checkAndRemove(svec, interchange_scene); + checkAndRemove(svec, compositing_log); + checkAndRemove(svec, color_timing); + checkAndRemove(svec, aces_interchange); + if (printLog) + { + StringUtils::StringVec::iterator iter = svec.begin(); + for(iter; iter < svec.end(); iter++) + { + std::cout << *iter << std::endl; + } + } +} + +void checkRequiredRolesErrors(LogGuard & logGuard, bool printLog) +{ StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - OCIO_CHECK_ASSERT( - StringUtils::Contain( - svec, - "[OpenColorIO Error]: The aces_interchange role is required when there are "\ - "scene-referred color spaces and the config version is 2.2 or higher." - ) - ); + checkRequiredRolesErrors(svec, printLog); } -void checkRequiredRolesErrors(LogGuard & logGuard) +void checkAllRequiredRolesErrors(LogGuard & logGuard, bool printLog) { StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - OCIO_CHECK_ASSERT( - StringUtils::Contain( - svec, - "[OpenColorIO Error]: The scene_linear role is required for a config version 2.2 "\ - "or higher." - ) - ); - - OCIO_CHECK_ASSERT( - StringUtils::Contain( - svec, - "[OpenColorIO Error]: The compositing_log role is required for a config version "\ - "2.2 or higher." - ) - ); - - OCIO_CHECK_ASSERT( - StringUtils::Contain( - svec, - "[OpenColorIO Error]: The color_timing role is required for a config version 2.2 "\ - "or higher." - ) - ); - - OCIO_CHECK_ASSERT( - StringUtils::Contain( - svec, - "[OpenColorIO Error]: The aces_interchange role is required when there are "\ - "scene-referred color spaces and the config version is 2.2 or higher." - ) - ); + const std::string interchange_display = "[OpenColorIO Error]: The cie_xyz_d65_interchange "\ + "role is required when there are display-referred "\ + "color spaces and the config version is 2.2 or higher."; + + checkAndRemove(svec, interchange_display); + checkRequiredRolesErrors(svec, true); } } // namespace OCIO_NAMESPACE diff --git a/tests/cpu/UnitTestLogUtils.h b/tests/cpu/UnitTestLogUtils.h index e6e81ed257..58f8adcbf1 100644 --- a/tests/cpu/UnitTestLogUtils.h +++ b/tests/cpu/UnitTestLogUtils.h @@ -39,8 +39,8 @@ class MuteLogging ~MuteLogging(); }; -void checkAllRequiredRolesErrors(LogGuard & logGuard); -void checkRequiredRolesErrors(LogGuard & logGuard); +void checkAllRequiredRolesErrors(LogGuard & logGuard, bool printLog); +void checkRequiredRolesErrors(LogGuard & logGuard, bool printLog); } // namespace OCIO_NAMESPACE diff --git a/tests/cpu/transforms/builtins/BuiltinTransformRegistry_tests.cpp b/tests/cpu/transforms/builtins/BuiltinTransformRegistry_tests.cpp index 4abd04a627..ceaed13ee0 100644 --- a/tests/cpu/transforms/builtins/BuiltinTransformRegistry_tests.cpp +++ b/tests/cpu/transforms/builtins/BuiltinTransformRegistry_tests.cpp @@ -106,7 +106,6 @@ luma: [0.2126, 0.7152, 0.0722] roles: aces_interchange: test - cie_xyz_d65_interchange: dcs color_timing: test compositing_log: test default: ref @@ -122,24 +121,6 @@ luma: [0.2126, 0.7152, 0.0722] active_displays: [] active_views: [] -view_transforms: - - ! - name: display - from_display_reference: ! {} - - - ! - name: view_transform - from_scene_reference: ! {} - -display_colorspaces: - - ! - name: dcs - family: "" - equalitygroup: "" - bitdepth: unknown - isdata: false - allocation: uniform - colorspaces: - ! name: ref From b52c2dc9f6bcf51e5222730d441035028c652bdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9drik=20Fuoco?= Date: Tue, 21 Mar 2023 10:21:46 -0400 Subject: [PATCH 3/7] Refactoring the new function in unit test log utils. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Cédrik Fuoco --- src/OpenColorIO/Config.cpp | 2 +- src/apps/ociocheck/main.cpp | 3 ++ tests/cpu/Config_tests.cpp | 14 ++++--- tests/cpu/FileRules_tests.cpp | 73 +++++++++++++++++++++++++++++----- tests/cpu/UnitTestLogUtils.cpp | 69 ++++++++++++++++++-------------- tests/cpu/UnitTestLogUtils.h | 10 +++-- 6 files changed, 122 insertions(+), 49 deletions(-) diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index 665d522390..c8f2a4bb83 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -1996,7 +1996,7 @@ void Config::validate() const { std::ostringstream os; os << "Inactive '" << name << "' is neither a color space nor a named transform."; - LogWarning(os.str()); + LogInfo(os.str()); } } } diff --git a/src/apps/ociocheck/main.cpp b/src/apps/ociocheck/main.cpp index c4298e1eaf..b3c8c33854 100644 --- a/src/apps/ociocheck/main.cpp +++ b/src/apps/ociocheck/main.cpp @@ -56,6 +56,9 @@ int main(int argc, const char **argv) return 1; } + // Set the logging level to INFO. + OCIO::SetLoggingLevel(OCIO::LOGGING_LEVEL_INFO); + try { OCIO::ConstConfigRcPtr srcConfig; diff --git a/tests/cpu/Config_tests.cpp b/tests/cpu/Config_tests.cpp index def0bb9080..92e00ce4a3 100644 --- a/tests/cpu/Config_tests.cpp +++ b/tests/cpu/Config_tests.cpp @@ -363,7 +363,13 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) OCIO_CHECK_NO_THROW(config->validate()); // Check that the log contains the expected error messages for the missing roles and mute // them so that (only) those messages don't appear in the test output. - OCIO::checkAllRequiredRolesErrors(logGuard, true); + StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeDisplayRoleWarning(svec)); + OCIO::printVectorOfLog(svec); } // Set colorspace for all required roles. @@ -378,9 +384,7 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); - - StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - OCIO_CHECK_ASSERT(StringUtils::Contain(svec, "")); + OCIO_CHECK_ASSERT(StringUtils::StartsWith(logGuard.output(), "")); } { @@ -5866,7 +5870,7 @@ OCIO_ADD_TEST(Config, inactive_color_space_read_write) OCIO::LogGuard log; OCIO_CHECK_NO_THROW(config->validate()); OCIO_CHECK_EQUAL(log.output(), - "[OpenColorIO Warning]: Inactive 'unknown' is neither a color " + "[OpenColorIO Info]: Inactive 'unknown' is neither a color " "space nor a named transform.\n"); } diff --git a/tests/cpu/FileRules_tests.cpp b/tests/cpu/FileRules_tests.cpp index 0d089f6fad..1384e73b8b 100644 --- a/tests/cpu/FileRules_tests.cpp +++ b/tests/cpu/FileRules_tests.cpp @@ -1376,8 +1376,14 @@ strictparsing: true { OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); - // Expecting error since the major version was changed to version 2 without any modifications. - OCIO::checkRequiredRolesErrors(logGuard, true); + // Check that the log contains the expected error messages for the missing roles and mute + // them so that (only) those messages don't appear in the test output. + StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); + OCIO::printVectorOfLog(svec); } // Check the new version. @@ -1451,7 +1457,12 @@ strictparsing: true OCIO_CHECK_NO_THROW(config->validate()); // Ignore (only) the errors logged regarding the missing roles that are required in // configs with version >= 2.2. - OCIO::checkRequiredRolesErrors(logGuard, true); + StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); + OCIO::printVectorOfLog(svec); } // Check the new version. @@ -1533,7 +1544,12 @@ strictparsing: true OCIO_CHECK_NO_THROW(cfg->validate()); // Ignore (only) the errors logged regarding the missing roles that are required in // configs with version >= 2.2. - OCIO::checkRequiredRolesErrors(logGuard, true); + StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); + OCIO::printVectorOfLog(svec); } // Check the new version. @@ -1579,7 +1595,12 @@ strictparsing: true OCIO_CHECK_NO_THROW(cfg->validate()); // Ignore (only) the errors logged regarding the missing roles that are required in // configs with version >= 2.2. - OCIO::checkRequiredRolesErrors(logGuard, true); + StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); + OCIO::printVectorOfLog(svec); } // Check the new version. @@ -1635,7 +1656,12 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) "referencing 'default' that is neither a color space nor a named " "transform"); // Expecting error since the major version was changed to version 2 without any modifications. - OCIO::checkRequiredRolesErrors(logGuard, true); + StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); + OCIO::printVectorOfLog(svec); } // Upgrading is making sure to build a valid v2 config. @@ -1646,7 +1672,12 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); // Expecting error since the major version was changed to version 2 without any modifications. - OCIO::checkRequiredRolesErrors(logGuard, true); + StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); + OCIO::printVectorOfLog(svec); } // Check the new version. @@ -1685,7 +1716,12 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) "referencing 'default' that is neither a color space nor a named " "transform"); // Expecting error since the major version was changed to version 2 without any modifications. - OCIO::checkRequiredRolesErrors(logGuard, true); + StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); + OCIO::printVectorOfLog(svec); } // Upgrading is making sure to build a valid v2 config. @@ -1696,7 +1732,12 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); // Expecting error since the major version was changed to version 2 without any modifications. - OCIO::checkRequiredRolesErrors(logGuard, true); + StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); + OCIO::printVectorOfLog(svec); } // Check the new version. @@ -1735,7 +1776,12 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) "referencing 'default' that is neither a color space nor a named " "transform"); // Expecting error since the major version was changed to version 2 without any modifications. - OCIO::checkRequiredRolesErrors(logGuard, true); + StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); + OCIO::printVectorOfLog(svec); } config->setMajorVersion(1); @@ -1757,7 +1803,12 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); // Expecting error since the major version was changed to version 2 without any modifications. - OCIO::checkRequiredRolesErrors(logGuard, true); + StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); + OCIO::printVectorOfLog(svec); } // Check the new version. diff --git a/tests/cpu/UnitTestLogUtils.cpp b/tests/cpu/UnitTestLogUtils.cpp index 1798b5b892..aacec474f3 100644 --- a/tests/cpu/UnitTestLogUtils.cpp +++ b/tests/cpu/UnitTestLogUtils.cpp @@ -6,7 +6,9 @@ #include #include "Platform.h" +#include "testutils/UnitTest.h" #include "UnitTestLogUtils.h" +#include "utils/StringUtils.h" namespace OCIO_NAMESPACE { @@ -66,8 +68,9 @@ MuteLogging::~MuteLogging() ResetToDefaultLoggingFunction(); } - -void checkAndRemove(std::vector & svec, const std::string & str) +// Check and remove str from vector of string if the str is found. +// Return true if found, otherwise, false. +bool checkAndRemove(std::vector & svec, const std::string & str) { size_t index = -1; size_t i = -1; @@ -80,53 +83,61 @@ void checkAndRemove(std::vector & svec, const std::string & str) } } - OCIO_CHECK_GT(-1, index); - svec.erase(svec.begin() + index); + // Expecting a 0 since the str is expected to be found. + if (index != -1) + { + // found the string in the vector. + svec.erase(svec.begin() + index); + return true; + } + + return false; } -void checkRequiredRolesErrors(StringUtils::StringVec & svec, bool printLog) +bool checkAndMuteInterchangeSceneRoleWarning(StringUtils::StringVec & svec) { const std::string interchange_scene = "[OpenColorIO Error]: The scene_linear role is "\ "required for a config version 2.2 or higher."; + return checkAndRemove(svec, interchange_scene); +} + +bool checkAndMuteCompositingLogRoleWarning(StringUtils::StringVec & svec) +{ const std::string compositing_log = "[OpenColorIO Error]: The compositing_log role is "\ "required for a config version 2.2 or higher."; + return checkAndRemove(svec, compositing_log); +} +bool checkAndMuteColorTimingRoleWarning(StringUtils::StringVec & svec) +{ const std::string color_timing = "[OpenColorIO Error]: The color_timing role is required "\ "for a config version 2.2 or higher."; + return checkAndRemove(svec, color_timing); +} +bool checkAndMuteAcesInterchangeRoleWarning(StringUtils::StringVec & svec) +{ const std::string aces_interchange = "[OpenColorIO Error]: The aces_interchange role is "\ "required when there are scene-referred color spaces and "\ "the config version is 2.2 or higher."; - - checkAndRemove(svec, interchange_scene); - checkAndRemove(svec, compositing_log); - checkAndRemove(svec, color_timing); - checkAndRemove(svec, aces_interchange); - if (printLog) - { - StringUtils::StringVec::iterator iter = svec.begin(); - for(iter; iter < svec.end(); iter++) - { - std::cout << *iter << std::endl; - } - } -} - -void checkRequiredRolesErrors(LogGuard & logGuard, bool printLog) -{ - StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - checkRequiredRolesErrors(svec, printLog); + return checkAndRemove(svec, aces_interchange); } -void checkAllRequiredRolesErrors(LogGuard & logGuard, bool printLog) +bool checkAndMuteInterchangeDisplayRoleWarning(StringUtils::StringVec & svec) { - StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); const std::string interchange_display = "[OpenColorIO Error]: The cie_xyz_d65_interchange "\ "role is required when there are display-referred "\ "color spaces and the config version is 2.2 or higher."; - - checkAndRemove(svec, interchange_display); - checkRequiredRolesErrors(svec, true); + return checkAndRemove(svec, interchange_display); +} + +void printVectorOfLog(StringUtils::StringVec & svec) +{ + StringUtils::StringVec::iterator iter = svec.begin(); + for(iter; iter < svec.end(); iter++) + { + std::cout << *iter << std::endl; + } } } // namespace OCIO_NAMESPACE diff --git a/tests/cpu/UnitTestLogUtils.h b/tests/cpu/UnitTestLogUtils.h index 58f8adcbf1..603a311ace 100644 --- a/tests/cpu/UnitTestLogUtils.h +++ b/tests/cpu/UnitTestLogUtils.h @@ -5,7 +5,6 @@ #define INCLUDED_OCIO_UNITTEST_LOGUTILS_H #include -#include "testutils/UnitTest.h" #include "utils/StringUtils.h" namespace OCIO_NAMESPACE @@ -39,8 +38,13 @@ class MuteLogging ~MuteLogging(); }; -void checkAllRequiredRolesErrors(LogGuard & logGuard, bool printLog); -void checkRequiredRolesErrors(LogGuard & logGuard, bool printLog); +bool checkAndMuteInterchangeSceneRoleWarning(StringUtils::StringVec & svec); +bool checkAndMuteCompositingLogRoleWarning(StringUtils::StringVec & svec); +bool checkAndMuteColorTimingRoleWarning(StringUtils::StringVec & svec); +bool checkAndMuteAcesInterchangeRoleWarning(StringUtils::StringVec & svec); +bool checkAndMuteInterchangeDisplayRoleWarning(StringUtils::StringVec & svec); + +void printVectorOfLog(StringUtils::StringVec & svec); } // namespace OCIO_NAMESPACE From bbdf8386ed504793ccb1fc220cfc0cf1c60c5ec6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9drik=20Fuoco?= Date: Wed, 22 Mar 2023 17:05:58 -0400 Subject: [PATCH 4/7] Integrating the code into LogGuard Changing some misuses of StartsWith MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Cédrik Fuoco --- tests/cpu/Config_tests.cpp | 37 ++++------ tests/cpu/FileRules_tests.cpp | 128 ++++++++++++++++----------------- tests/cpu/UnitTestLogUtils.cpp | 58 +++++++++------ tests/cpu/UnitTestLogUtils.h | 14 ++-- 4 files changed, 119 insertions(+), 118 deletions(-) diff --git a/tests/cpu/Config_tests.cpp b/tests/cpu/Config_tests.cpp index 92e00ce4a3..1ccb9f3bbb 100644 --- a/tests/cpu/Config_tests.cpp +++ b/tests/cpu/Config_tests.cpp @@ -363,13 +363,12 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) OCIO_CHECK_NO_THROW(config->validate()); // Check that the log contains the expected error messages for the missing roles and mute // them so that (only) those messages don't appear in the test output. - StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeDisplayRoleWarning(svec)); - OCIO::printVectorOfLog(svec); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteSceneLinearRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeDisplayRoleError(logGuard)); + logGuard.print(); } // Set colorspace for all required roles. @@ -384,7 +383,7 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); - OCIO_CHECK_ASSERT(StringUtils::StartsWith(logGuard.output(), "")); + OCIO_CHECK_ASSERT(logGuard.empty()); } { @@ -458,12 +457,9 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); - OCIO_CHECK_ASSERT( - StringUtils::StartsWith( - logGuard.output(), - "[OpenColorIO Error]: The aces_interchange role is required when there are "\ - "scene-referred color spaces and the config version is 2.2 or higher." - ) + OCIO_CHECK_ASSERT(logGuard.findAndRemove( + "[OpenColorIO Error]: The aces_interchange role is required when there are "\ + "scene-referred color spaces and the config version is 2.2 or higher.") ); // Set aces_interchange for next test. @@ -501,9 +497,8 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); OCIO_CHECK_ASSERT( - StringUtils::StartsWith( - logGuard.output(), - "[OpenColorIO Error]: The aces_interchange role must be a scene-referred color space.") + logGuard.findAndRemove("[OpenColorIO Error]: The aces_interchange role must be a "\ + "scene-referred color space.") ); } @@ -517,9 +512,8 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); OCIO_CHECK_ASSERT( - StringUtils::StartsWith( - logGuard.output(), - "[OpenColorIO Error]: The cie_xyz_d65_interchange role must be a display-referred color space.") + logGuard.findAndRemove("[OpenColorIO Error]: The cie_xyz_d65_interchange role must "\ + "be a display-referred color space.") ); } @@ -538,8 +532,7 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); - OCIO_CHECK_ASSERT( - StringUtils::StartsWith(logGuard.output(), "")); + OCIO_CHECK_ASSERT(logGuard.empty()); } } diff --git a/tests/cpu/FileRules_tests.cpp b/tests/cpu/FileRules_tests.cpp index 1384e73b8b..9e81533abf 100644 --- a/tests/cpu/FileRules_tests.cpp +++ b/tests/cpu/FileRules_tests.cpp @@ -1378,12 +1378,11 @@ strictparsing: true OCIO_CHECK_NO_THROW(config->validate()); // Check that the log contains the expected error messages for the missing roles and mute // them so that (only) those messages don't appear in the test output. - StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); - OCIO::printVectorOfLog(svec); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteSceneLinearRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + logGuard.print(); } // Check the new version. @@ -1457,12 +1456,11 @@ strictparsing: true OCIO_CHECK_NO_THROW(config->validate()); // Ignore (only) the errors logged regarding the missing roles that are required in // configs with version >= 2.2. - StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); - OCIO::printVectorOfLog(svec); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteSceneLinearRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + logGuard.print(); } // Check the new version. @@ -1544,12 +1542,11 @@ strictparsing: true OCIO_CHECK_NO_THROW(cfg->validate()); // Ignore (only) the errors logged regarding the missing roles that are required in // configs with version >= 2.2. - StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); - OCIO::printVectorOfLog(svec); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteSceneLinearRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + logGuard.print(); } // Check the new version. @@ -1595,12 +1592,11 @@ strictparsing: true OCIO_CHECK_NO_THROW(cfg->validate()); // Ignore (only) the errors logged regarding the missing roles that are required in // configs with version >= 2.2. - StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); - OCIO::printVectorOfLog(svec); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteSceneLinearRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + logGuard.print(); } // Check the new version. @@ -1655,13 +1651,13 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) OCIO_CHECK_THROW_WHAT(config->validate(), OCIO::Exception, "rule named 'Default' is " "referencing 'default' that is neither a color space nor a named " "transform"); - // Expecting error since the major version was changed to version 2 without any modifications. - StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); - OCIO::printVectorOfLog(svec); + // Ignore (only) the errors logged regarding the missing roles that are required in + // configs with version >= 2.2. + OCIO_CHECK_ASSERT(OCIO::checkAndMuteSceneLinearRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + logGuard.print(); } // Upgrading is making sure to build a valid v2 config. @@ -1671,13 +1667,13 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) { OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); - // Expecting error since the major version was changed to version 2 without any modifications. - StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); - OCIO::printVectorOfLog(svec); + // Ignore (only) the errors logged regarding the missing roles that are required in + // configs with version >= 2.2. + OCIO_CHECK_ASSERT(OCIO::checkAndMuteSceneLinearRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + logGuard.print(); } // Check the new version. @@ -1715,13 +1711,13 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) OCIO_CHECK_THROW_WHAT(config->validate(), OCIO::Exception, "rule named 'Default' is " "referencing 'default' that is neither a color space nor a named " "transform"); - // Expecting error since the major version was changed to version 2 without any modifications. - StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); - OCIO::printVectorOfLog(svec); + // Ignore (only) the errors logged regarding the missing roles that are required in + // configs with version >= 2.2. + OCIO_CHECK_ASSERT(OCIO::checkAndMuteSceneLinearRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + logGuard.print(); } // Upgrading is making sure to build a valid v2 config. @@ -1731,13 +1727,13 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) { OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); - // Expecting error since the major version was changed to version 2 without any modifications. - StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); - OCIO::printVectorOfLog(svec); + // Ignore (only) the errors logged regarding the missing roles that are required in + // configs with version >= 2.2. + OCIO_CHECK_ASSERT(OCIO::checkAndMuteSceneLinearRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + logGuard.print(); } // Check the new version. @@ -1775,13 +1771,13 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) OCIO_CHECK_THROW_WHAT(config->validate(), OCIO::Exception, "rule named 'Default' is " "referencing 'default' that is neither a color space nor a named " "transform"); - // Expecting error since the major version was changed to version 2 without any modifications. - StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); - OCIO::printVectorOfLog(svec); + // Ignore (only) the errors logged regarding the missing roles that are required in + // configs with version >= 2.2. + OCIO_CHECK_ASSERT(OCIO::checkAndMuteSceneLinearRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + logGuard.print(); } config->setMajorVersion(1); @@ -1802,13 +1798,13 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) { OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); - // Expecting error since the major version was changed to version 2 without any modifications. - StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeSceneRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleWarning(svec)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleWarning(svec)); - OCIO::printVectorOfLog(svec); + // Ignore (only) the errors logged regarding the missing roles that are required in + // configs with version >= 2.2. + OCIO_CHECK_ASSERT(OCIO::checkAndMuteSceneLinearRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + logGuard.print(); } // Check the new version. diff --git a/tests/cpu/UnitTestLogUtils.cpp b/tests/cpu/UnitTestLogUtils.cpp index aacec474f3..145f6d6f84 100644 --- a/tests/cpu/UnitTestLogUtils.cpp +++ b/tests/cpu/UnitTestLogUtils.cpp @@ -5,10 +5,11 @@ #include #include +#include + #include "Platform.h" #include "testutils/UnitTest.h" #include "UnitTestLogUtils.h" -#include "utils/StringUtils.h" namespace OCIO_NAMESPACE { @@ -58,6 +59,29 @@ bool LogGuard::empty() const return g_output.empty(); } +// Find and remove specified line from g_output. +// Return true if found, otherwise, false. +bool LogGuard::findAndRemove(const std::string & line) const +{ + // Escape the line to prevent error in regex if the line contains regex character. + std::string escaped_line = std::regex_replace(line, std::regex("[\\[\\](){}*+?.^$|\\\\]"), "\\$&"); + std::regex pattern(escaped_line + R"([\r\n]+)"); + std::smatch match; + if (std::regex_search(g_output, match, pattern)) { + // If the line is found, remove it. + auto pos = std::next(g_output.begin(), match.position()); + auto end = std::next(pos, match.length()); + g_output.erase(pos, end); + return true; + } + return false; +} + +void LogGuard::print() +{ + std::cout << g_output; +} + MuteLogging::MuteLogging() { SetLoggingFunction(&MuteLoggingFunction); @@ -73,7 +97,6 @@ MuteLogging::~MuteLogging() bool checkAndRemove(std::vector & svec, const std::string & str) { size_t index = -1; - size_t i = -1; for (size_t i = 0; i < svec.size(); i++) { if (Platform::Strcasecmp(svec[i].c_str(), str.c_str()) == 0) @@ -83,10 +106,8 @@ bool checkAndRemove(std::vector & svec, const std::string & str) } } - // Expecting a 0 since the str is expected to be found. if (index != -1) { - // found the string in the vector. svec.erase(svec.begin() + index); return true; } @@ -94,50 +115,41 @@ bool checkAndRemove(std::vector & svec, const std::string & str) return false; } -bool checkAndMuteInterchangeSceneRoleWarning(StringUtils::StringVec & svec) +bool checkAndMuteSceneLinearRoleError(LogGuard & logGuard) { const std::string interchange_scene = "[OpenColorIO Error]: The scene_linear role is "\ "required for a config version 2.2 or higher."; - return checkAndRemove(svec, interchange_scene); + return logGuard.findAndRemove(interchange_scene); } -bool checkAndMuteCompositingLogRoleWarning(StringUtils::StringVec & svec) +bool checkAndMuteCompositingLogRoleError(LogGuard & logGuard) { const std::string compositing_log = "[OpenColorIO Error]: The compositing_log role is "\ "required for a config version 2.2 or higher."; - return checkAndRemove(svec, compositing_log); + return logGuard.findAndRemove(compositing_log); } -bool checkAndMuteColorTimingRoleWarning(StringUtils::StringVec & svec) +bool checkAndMuteColorTimingRoleError(LogGuard & logGuard) { const std::string color_timing = "[OpenColorIO Error]: The color_timing role is required "\ "for a config version 2.2 or higher."; - return checkAndRemove(svec, color_timing); + return logGuard.findAndRemove(color_timing); } -bool checkAndMuteAcesInterchangeRoleWarning(StringUtils::StringVec & svec) +bool checkAndMuteAcesInterchangeRoleError(LogGuard & logGuard) { const std::string aces_interchange = "[OpenColorIO Error]: The aces_interchange role is "\ "required when there are scene-referred color spaces and "\ "the config version is 2.2 or higher."; - return checkAndRemove(svec, aces_interchange); + return logGuard.findAndRemove(aces_interchange); } -bool checkAndMuteInterchangeDisplayRoleWarning(StringUtils::StringVec & svec) +bool checkAndMuteInterchangeDisplayRoleError(LogGuard & logGuard) { const std::string interchange_display = "[OpenColorIO Error]: The cie_xyz_d65_interchange "\ "role is required when there are display-referred "\ "color spaces and the config version is 2.2 or higher."; - return checkAndRemove(svec, interchange_display); -} - -void printVectorOfLog(StringUtils::StringVec & svec) -{ - StringUtils::StringVec::iterator iter = svec.begin(); - for(iter; iter < svec.end(); iter++) - { - std::cout << *iter << std::endl; - } + return logGuard.findAndRemove(interchange_display); } } // namespace OCIO_NAMESPACE diff --git a/tests/cpu/UnitTestLogUtils.h b/tests/cpu/UnitTestLogUtils.h index 603a311ace..11b9a59535 100644 --- a/tests/cpu/UnitTestLogUtils.h +++ b/tests/cpu/UnitTestLogUtils.h @@ -26,6 +26,8 @@ class LogGuard void clear(); bool empty() const; + bool findAndRemove(const std::string & str) const; + void print(); private: LoggingLevel m_logLevel; }; @@ -38,13 +40,11 @@ class MuteLogging ~MuteLogging(); }; -bool checkAndMuteInterchangeSceneRoleWarning(StringUtils::StringVec & svec); -bool checkAndMuteCompositingLogRoleWarning(StringUtils::StringVec & svec); -bool checkAndMuteColorTimingRoleWarning(StringUtils::StringVec & svec); -bool checkAndMuteAcesInterchangeRoleWarning(StringUtils::StringVec & svec); -bool checkAndMuteInterchangeDisplayRoleWarning(StringUtils::StringVec & svec); - -void printVectorOfLog(StringUtils::StringVec & svec); +bool checkAndMuteSceneLinearRoleError(LogGuard & logGuard); +bool checkAndMuteCompositingLogRoleError(LogGuard & logGuard); +bool checkAndMuteColorTimingRoleError(LogGuard & logGuard); +bool checkAndMuteAcesInterchangeRoleError(LogGuard & logGuard); +bool checkAndMuteInterchangeDisplayRoleError(LogGuard & logGuard); } // namespace OCIO_NAMESPACE From c4f6563273cb1dec266cf4404b3cf2fe889f5065 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9drik=20Fuoco?= Date: Thu, 23 Mar 2023 15:25:55 -0400 Subject: [PATCH 5/7] Adding a MuteLogging class for python side Mute log from built-in config test Comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Cédrik Fuoco --- tests/cpu/Config_tests.cpp | 61 ++++++++++++++++------------------ tests/cpu/FileRules_tests.cpp | 10 ++++++ tests/cpu/UnitTestLogUtils.cpp | 25 +------------- tests/cpu/UnitTestLogUtils.h | 2 +- tests/python/ConfigTest.py | 35 +++++++++++++++---- tests/python/UnitTestUtils.py | 13 ++++++++ 6 files changed, 81 insertions(+), 65 deletions(-) diff --git a/tests/cpu/Config_tests.cpp b/tests/cpu/Config_tests.cpp index 1ccb9f3bbb..0c34a5c015 100644 --- a/tests/cpu/Config_tests.cpp +++ b/tests/cpu/Config_tests.cpp @@ -367,7 +367,8 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); - OCIO_CHECK_ASSERT(OCIO::checkAndMuteInterchangeDisplayRoleError(logGuard)); + OCIO_CHECK_ASSERT(OCIO::checkAndMuteDisplayInterchangeRoleError(logGuard)); + // If there are any unexpected log messages, print them to the shell. logGuard.print(); } @@ -417,12 +418,7 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) OCIO_CHECK_NO_THROW(config->validate()); StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - OCIO_CHECK_ASSERT( - StringUtils::Contain( - svec, - "[OpenColorIO Error]: The compositing_log role is required for a config version 2.2 "\ - "or higher.") - ); + checkAndMuteCompositingLogRoleError(logGuard); // Set compositing_log for next test. config->setRole(OCIO::ROLE_COMPOSITING_LOG, dcs->getName()); @@ -438,12 +434,7 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) OCIO_CHECK_NO_THROW(config->validate()); StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - OCIO_CHECK_ASSERT( - StringUtils::Contain( - svec, - "[OpenColorIO Error]: The color_timing role is required for a config version 2.2 or "\ - "higher.") - ); + checkAndMuteColorTimingRoleError(logGuard); // Set color_timing for next test. config->setRole(OCIO::ROLE_COLOR_TIMING, dcs->getName()); @@ -457,10 +448,7 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); - OCIO_CHECK_ASSERT(logGuard.findAndRemove( - "[OpenColorIO Error]: The aces_interchange role is required when there are "\ - "scene-referred color spaces and the config version is 2.2 or higher.") - ); + OCIO::checkAndMuteAcesInterchangeRoleError(logGuard); // Set aces_interchange for next test. config->setRole(OCIO::ROLE_INTERCHANGE_SCENE, scs->getName()); @@ -474,14 +462,7 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); - - StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); - OCIO_CHECK_ASSERT( - StringUtils::Contain( - svec, - "[OpenColorIO Error]: The cie_xyz_d65_interchange role is required when there are "\ - "display-referred color spaces and the config version is 2.2 or higher.") - ); + OCIO::checkAndMuteDisplayInterchangeRoleError(logGuard); // Set cie_xyz_d65_interchange for next test. config->setRole(OCIO::ROLE_INTERCHANGE_DISPLAY, dcs->getName()); @@ -496,10 +477,7 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); - OCIO_CHECK_ASSERT( - logGuard.findAndRemove("[OpenColorIO Error]: The aces_interchange role must be a "\ - "scene-referred color space.") - ); + checkAndMuteAcesInterchangeRoleError(logGuard); } { @@ -511,10 +489,7 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); - OCIO_CHECK_ASSERT( - logGuard.findAndRemove("[OpenColorIO Error]: The cie_xyz_d65_interchange role must "\ - "be a display-referred color space.") - ); + checkAndMuteDisplayInterchangeRoleError(logGuard); } { @@ -8997,6 +8972,10 @@ OCIO_ADD_TEST(Config, create_builtin_config) ); OCIO_REQUIRE_ASSERT(config); + // Mute the logs about inactive colorspace as it is expected. + // No other warnings are expected. + OCIO::MuteLogging mute; + OCIO_CHECK_NO_THROW(config->validate()); OCIO_CHECK_EQUAL( std::string(config->getName()), @@ -9014,6 +8993,10 @@ OCIO_ADD_TEST(Config, create_builtin_config) OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromEnv()); OCIO_REQUIRE_ASSERT(config); + // Mute the logs about inactive colorspace as it is expected. + // No other warnings are expected. + OCIO::MuteLogging mute; + OCIO_CHECK_NO_THROW(config->validate()); OCIO_CHECK_EQUAL( std::string(config->getName()), @@ -9031,6 +9014,10 @@ OCIO_ADD_TEST(Config, create_builtin_config) ); OCIO_REQUIRE_ASSERT(config); + // Mute the logs about inactive colorspace as it is expected. + // No other warnings are expected. + OCIO::MuteLogging mute; + OCIO_CHECK_NO_THROW(config->validate()); OCIO_CHECK_EQUAL( std::string(config->getName()), @@ -9110,6 +9097,10 @@ OCIO_ADD_TEST(Config, create_builtin_config) OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromEnv()); OCIO_REQUIRE_ASSERT(config); + // Mute the logs about inactive colorspace as it is expected. + // No other warnings are expected. + OCIO::MuteLogging mute; + OCIO_CHECK_NO_THROW(config->validate()); OCIO_CHECK_EQUAL( std::string(config->getName()), @@ -9124,6 +9115,10 @@ OCIO_ADD_TEST(Config, create_builtin_config) OCIO::ConstConfigRcPtr config; OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromFile("ocio://default")); OCIO_REQUIRE_ASSERT(config); + + // Mute the logs about inactive colorspace as it is expected. + // No other warnings are expected. + OCIO::MuteLogging mute; OCIO_CHECK_NO_THROW(config->validate()); OCIO_CHECK_EQUAL( diff --git a/tests/cpu/FileRules_tests.cpp b/tests/cpu/FileRules_tests.cpp index 9e81533abf..0f6e7529be 100644 --- a/tests/cpu/FileRules_tests.cpp +++ b/tests/cpu/FileRules_tests.cpp @@ -1382,6 +1382,7 @@ strictparsing: true OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + // If there are any unexpected log messages, print them to the shell. logGuard.print(); } @@ -1460,6 +1461,7 @@ strictparsing: true OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + // If there are any unexpected log messages, print them to the shell. logGuard.print(); } @@ -1546,6 +1548,7 @@ strictparsing: true OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + // If there are any unexpected log messages, print them to the shell. logGuard.print(); } @@ -1596,6 +1599,7 @@ strictparsing: true OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + // If there are any unexpected log messages, print them to the shell. logGuard.print(); } @@ -1657,6 +1661,7 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + // If there are any unexpected log messages, print them to the shell. logGuard.print(); } @@ -1673,6 +1678,7 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + // If there are any unexpected log messages, print them to the shell. logGuard.print(); } @@ -1717,6 +1723,7 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + // If there are any unexpected log messages, print them to the shell. logGuard.print(); } @@ -1733,6 +1740,7 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + // If there are any unexpected log messages, print them to the shell. logGuard.print(); } @@ -1777,6 +1785,7 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + // If there are any unexpected log messages, print them to the shell. logGuard.print(); } @@ -1804,6 +1813,7 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory) OCIO_CHECK_ASSERT(OCIO::checkAndMuteCompositingLogRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteColorTimingRoleError(logGuard)); OCIO_CHECK_ASSERT(OCIO::checkAndMuteAcesInterchangeRoleError(logGuard)); + // If there are any unexpected log messages, print them to the shell. logGuard.print(); } diff --git a/tests/cpu/UnitTestLogUtils.cpp b/tests/cpu/UnitTestLogUtils.cpp index 145f6d6f84..5868697a16 100644 --- a/tests/cpu/UnitTestLogUtils.cpp +++ b/tests/cpu/UnitTestLogUtils.cpp @@ -92,29 +92,6 @@ MuteLogging::~MuteLogging() ResetToDefaultLoggingFunction(); } -// Check and remove str from vector of string if the str is found. -// Return true if found, otherwise, false. -bool checkAndRemove(std::vector & svec, const std::string & str) -{ - size_t index = -1; - for (size_t i = 0; i < svec.size(); i++) - { - if (Platform::Strcasecmp(svec[i].c_str(), str.c_str()) == 0) - { - index = i; - break; - } - } - - if (index != -1) - { - svec.erase(svec.begin() + index); - return true; - } - - return false; -} - bool checkAndMuteSceneLinearRoleError(LogGuard & logGuard) { const std::string interchange_scene = "[OpenColorIO Error]: The scene_linear role is "\ @@ -144,7 +121,7 @@ bool checkAndMuteAcesInterchangeRoleError(LogGuard & logGuard) return logGuard.findAndRemove(aces_interchange); } -bool checkAndMuteInterchangeDisplayRoleError(LogGuard & logGuard) +bool checkAndMuteDisplayInterchangeRoleError(LogGuard & logGuard) { const std::string interchange_display = "[OpenColorIO Error]: The cie_xyz_d65_interchange "\ "role is required when there are display-referred "\ diff --git a/tests/cpu/UnitTestLogUtils.h b/tests/cpu/UnitTestLogUtils.h index 11b9a59535..3eff4f9fe5 100644 --- a/tests/cpu/UnitTestLogUtils.h +++ b/tests/cpu/UnitTestLogUtils.h @@ -44,7 +44,7 @@ bool checkAndMuteSceneLinearRoleError(LogGuard & logGuard); bool checkAndMuteCompositingLogRoleError(LogGuard & logGuard); bool checkAndMuteColorTimingRoleError(LogGuard & logGuard); bool checkAndMuteAcesInterchangeRoleError(LogGuard & logGuard); -bool checkAndMuteInterchangeDisplayRoleError(LogGuard & logGuard); +bool checkAndMuteDisplayInterchangeRoleError(LogGuard & logGuard); } // namespace OCIO_NAMESPACE diff --git a/tests/python/ConfigTest.py b/tests/python/ConfigTest.py index 1ea0e2d928..3260d67990 100644 --- a/tests/python/ConfigTest.py +++ b/tests/python/ConfigTest.py @@ -11,7 +11,8 @@ SIMPLE_CONFIG_VIRTUAL_DISPLAY_ACTIVE_DISPLAY, SIMPLE_CONFIG_VIRTUAL_DISPLAY_V1, SIMPLE_CONFIG_VIRTUAL_DISPLAY_EXCEPTION, - TEST_DATAFILES_DIR) + TEST_DATAFILES_DIR, + MuteLogging) # Legacy tests kept for reference. # @@ -911,7 +912,11 @@ def test_create_builtin_config(self): numberOfExpectedColorspaces = 14 # Testing CreateFromBuiltinConfig with a known built-in config name. builtinCfgA = OCIO.Config.CreateFromBuiltinConfig("cg-config-v1.0.0_aces-v1.3_ocio-v2.1") - builtinCfgA.validate() + + log = MuteLogging() + with log: + builtinCfgA.validate() + self.assertEqual(builtinCfgA.getName(), "cg-config-v1.0.0_aces-v1.3_ocio-v2.1") self.assertEqual(len(builtinCfgA.getColorSpaceNames()), numberOfExpectedColorspaces) @@ -919,7 +924,11 @@ def test_create_builtin_config(self): try: OCIO.SetEnvVariable('OCIO', 'ocio://cg-config-v1.0.0_aces-v1.3_ocio-v2.1') builtinCfgB = OCIO.Config.CreateFromEnv() - builtinCfgB.validate() + + log = MuteLogging() + with log: + builtinCfgB.validate() + self.assertEqual(builtinCfgB.getName(), "cg-config-v1.0.0_aces-v1.3_ocio-v2.1") self.assertEqual(len(builtinCfgB.getColorSpaceNames()), numberOfExpectedColorspaces) finally: @@ -927,7 +936,11 @@ def test_create_builtin_config(self): # Testing CreateFromFile with an known built-in config name using URI Syntax. builtinCfgC = OCIO.Config.CreateFromFile("ocio://cg-config-v1.0.0_aces-v1.3_ocio-v2.1") - builtinCfgC.validate() + + log = MuteLogging() + with log: + builtinCfgC.validate() + self.assertEqual(builtinCfgC.getName(), "cg-config-v1.0.0_aces-v1.3_ocio-v2.1") self.assertEqual(len(builtinCfgC.getColorSpaceNames()), numberOfExpectedColorspaces) @@ -935,7 +948,7 @@ def test_create_builtin_config(self): # Testing STUDIO config. # ******************************** - numberOfExpectedColorspaces = 39; + numberOfExpectedColorspaces = 39 # Testing CreateFromBuiltinConfig with a known built-in config name. builtinCfgA = OCIO.Config.CreateFromBuiltinConfig("studio-config-v1.0.0_aces-v1.3_ocio-v2.1") builtinCfgA.validate() @@ -966,7 +979,11 @@ def test_create_builtin_config(self): try: OCIO.SetEnvVariable('OCIO', 'ocio://default') builtinCfgD = OCIO.Config.CreateFromEnv() - builtinCfgD.validate() + + log = MuteLogging() + with log: + builtinCfgD.validate() + self.assertEqual(builtinCfgD.getName(), "cg-config-v1.0.0_aces-v1.3_ocio-v2.1") self.assertEqual(len(builtinCfgD.getColorSpaceNames()), 14) finally: @@ -974,7 +991,11 @@ def test_create_builtin_config(self): # Testing CreateFromFile with the default config using URI Syntax. builtinCfgE = OCIO.Config.CreateFromFile("ocio://default") - builtinCfgE.validate() + + log = MuteLogging() + with log: + builtinCfgE.validate() + self.assertEqual(builtinCfgE.getName(), "cg-config-v1.0.0_aces-v1.3_ocio-v2.1") self.assertEqual(len(builtinCfgE.getColorSpaceNames()), 14) diff --git a/tests/python/UnitTestUtils.py b/tests/python/UnitTestUtils.py index 23d11d5a33..1b83c1cb92 100644 --- a/tests/python/UnitTestUtils.py +++ b/tests/python/UnitTestUtils.py @@ -2,6 +2,8 @@ import sys from random import randint +import PyOpenColorIO as OCIO + def assertEqualRGBM(testCase, first, second): testCase.assertEqual(first.red, second.red) testCase.assertEqual(first.green, second.green) @@ -65,6 +67,17 @@ def assertEqualRGBCurve(testCase, first, second): assertEqualBSpline(testCase, first.blue, second.blue) assertEqualBSpline(testCase, first.master, second.master) +class MuteLogging: + def __init__(self): + self.previous_function = None + + def __enter__(self): + self.previous_function = OCIO.SetLoggingFunction(lambda message: None) + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + OCIO.ResetToDefaultLoggingFunction() + # ----------------------------------------------------------------------------- # Python 2/3 compatibility # ----------------------------------------------------------------------------- From 360422dac642c0722c3e6c2338ea802d95f2b6e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9drik=20Fuoco?= Date: Fri, 31 Mar 2023 11:51:11 -0400 Subject: [PATCH 6/7] Adding new utilities function to mute inactive colorspace info log while not muting all logs (C++ side) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Cédrik Fuoco --- tests/cpu/Config_tests.cpp | 54 ++++++++++++++++++++-------------- tests/cpu/UnitTestLogUtils.cpp | 26 +++++++++++++++- tests/cpu/UnitTestLogUtils.h | 3 ++ tests/python/ConfigTest.py | 2 ++ 4 files changed, 62 insertions(+), 23 deletions(-) diff --git a/tests/cpu/Config_tests.cpp b/tests/cpu/Config_tests.cpp index 0c34a5c015..4d76fda434 100644 --- a/tests/cpu/Config_tests.cpp +++ b/tests/cpu/Config_tests.cpp @@ -477,7 +477,11 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); - checkAndMuteAcesInterchangeRoleError(logGuard); + OCIO_CHECK_ASSERT( + StringUtils::StartsWith( + logGuard.output(), + "[OpenColorIO Error]: The aces_interchange role must be a scene-referred color space.") + ); } { @@ -489,7 +493,11 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); - checkAndMuteDisplayInterchangeRoleError(logGuard); + OCIO_CHECK_ASSERT( + StringUtils::StartsWith( + logGuard.output(), + "[OpenColorIO Error]: The cie_xyz_d65_interchange role must be a display-referred color space.") + ); } { @@ -8972,11 +8980,13 @@ OCIO_ADD_TEST(Config, create_builtin_config) ); OCIO_REQUIRE_ASSERT(config); - // Mute the logs about inactive colorspace as it is expected. - // No other warnings are expected. - OCIO::MuteLogging mute; - + OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); + // Mute output related to a bug in the initial CG config where the inactive_colorspaces + // list has color spaces that don't exist. + OCIO::muteInactiveColorspaceInfo(logGuard); + logGuard.print(); + OCIO_CHECK_EQUAL( std::string(config->getName()), cgConfigName @@ -8993,11 +9003,11 @@ OCIO_ADD_TEST(Config, create_builtin_config) OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromEnv()); OCIO_REQUIRE_ASSERT(config); - // Mute the logs about inactive colorspace as it is expected. - // No other warnings are expected. - OCIO::MuteLogging mute; - + OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); + OCIO::muteInactiveColorspaceInfo(logGuard); + logGuard.print(); + OCIO_CHECK_EQUAL( std::string(config->getName()), cgConfigName @@ -9014,11 +9024,11 @@ OCIO_ADD_TEST(Config, create_builtin_config) ); OCIO_REQUIRE_ASSERT(config); - // Mute the logs about inactive colorspace as it is expected. - // No other warnings are expected. - OCIO::MuteLogging mute; - + OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); + OCIO::muteInactiveColorspaceInfo(logGuard); + logGuard.print(); + OCIO_CHECK_EQUAL( std::string(config->getName()), cgConfigName @@ -9097,11 +9107,11 @@ OCIO_ADD_TEST(Config, create_builtin_config) OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromEnv()); OCIO_REQUIRE_ASSERT(config); - // Mute the logs about inactive colorspace as it is expected. - // No other warnings are expected. - OCIO::MuteLogging mute; - + OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); + OCIO::muteInactiveColorspaceInfo(logGuard); + logGuard.print(); + OCIO_CHECK_EQUAL( std::string(config->getName()), std::string("cg-config-v1.0.0_aces-v1.3_ocio-v2.1") @@ -9116,11 +9126,11 @@ OCIO_ADD_TEST(Config, create_builtin_config) OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromFile("ocio://default")); OCIO_REQUIRE_ASSERT(config); - // Mute the logs about inactive colorspace as it is expected. - // No other warnings are expected. - OCIO::MuteLogging mute; - + OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config->validate()); + OCIO::muteInactiveColorspaceInfo(logGuard); + logGuard.print(); + OCIO_CHECK_EQUAL( std::string(config->getName()), std::string("cg-config-v1.0.0_aces-v1.3_ocio-v2.1") diff --git a/tests/cpu/UnitTestLogUtils.cpp b/tests/cpu/UnitTestLogUtils.cpp index 5868697a16..60a5471a15 100644 --- a/tests/cpu/UnitTestLogUtils.cpp +++ b/tests/cpu/UnitTestLogUtils.cpp @@ -67,7 +67,8 @@ bool LogGuard::findAndRemove(const std::string & line) const std::string escaped_line = std::regex_replace(line, std::regex("[\\[\\](){}*+?.^$|\\\\]"), "\\$&"); std::regex pattern(escaped_line + R"([\r\n]+)"); std::smatch match; - if (std::regex_search(g_output, match, pattern)) { + if (std::regex_search(g_output, match, pattern)) + { // If the line is found, remove it. auto pos = std::next(g_output.begin(), match.position()); auto end = std::next(pos, match.length()); @@ -77,6 +78,23 @@ bool LogGuard::findAndRemove(const std::string & line) const return false; } +bool LogGuard::findAllAndRemove(const std::string & line) const +{ + // Escape the line to prevent error in regex if the line contains regex character. + bool found = false; + std::string escaped_line = std::regex_replace(line, std::regex("[\\[\\](){}*+?.^$|\\\\]"), "\\$&"); + std::regex pattern(R"(^\[OpenColorIO Info\]: Inactive.*)" + escaped_line + R"([\r\n]+)"); + std::smatch match; + while (std::regex_search(g_output, match, pattern)) + { + found = true; + auto pos = std::next(g_output.begin(), match.position()); + auto end = std::next(pos, match.length()); + g_output.erase(pos, end); + } + return found; +} + void LogGuard::print() { std::cout << g_output; @@ -129,5 +147,11 @@ bool checkAndMuteDisplayInterchangeRoleError(LogGuard & logGuard) return logGuard.findAndRemove(interchange_display); } +void muteInactiveColorspaceInfo(LogGuard & logGuard) +{ + const std::string str = "- Display' is neither a color space nor a named transform."; + logGuard.findAllAndRemove(str); +} + } // namespace OCIO_NAMESPACE diff --git a/tests/cpu/UnitTestLogUtils.h b/tests/cpu/UnitTestLogUtils.h index 3eff4f9fe5..470088dd72 100644 --- a/tests/cpu/UnitTestLogUtils.h +++ b/tests/cpu/UnitTestLogUtils.h @@ -27,6 +27,7 @@ class LogGuard bool empty() const; bool findAndRemove(const std::string & str) const; + bool findAllAndRemove(const std::string & str) const; void print(); private: LoggingLevel m_logLevel; @@ -46,6 +47,8 @@ bool checkAndMuteColorTimingRoleError(LogGuard & logGuard); bool checkAndMuteAcesInterchangeRoleError(LogGuard & logGuard); bool checkAndMuteDisplayInterchangeRoleError(LogGuard & logGuard); +void muteInactiveColorspaceInfo(LogGuard & logGuard); + } // namespace OCIO_NAMESPACE #endif // INCLUDED_OCIO_UNITTEST_LOGUTILS_H diff --git a/tests/python/ConfigTest.py b/tests/python/ConfigTest.py index 3260d67990..a687a5f0e2 100644 --- a/tests/python/ConfigTest.py +++ b/tests/python/ConfigTest.py @@ -913,6 +913,8 @@ def test_create_builtin_config(self): # Testing CreateFromBuiltinConfig with a known built-in config name. builtinCfgA = OCIO.Config.CreateFromBuiltinConfig("cg-config-v1.0.0_aces-v1.3_ocio-v2.1") + # Mute output related to a bug in the initial CG config where the inactive_colorspaces + # list has color spaces that don't exist. log = MuteLogging() with log: builtinCfgA.validate() From 1dfa15e3dc6a7f56c9b5a0fc037dea4d341e17dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9drik=20Fuoco?= Date: Mon, 3 Apr 2023 10:27:38 -0400 Subject: [PATCH 7/7] findAllAndRemove method from LogGuard now takes a regex pattern instead of just the line to find MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Cédrik Fuoco --- tests/cpu/Config_tests.cpp | 4 ++-- tests/cpu/UnitTestLogUtils.cpp | 9 ++++----- tests/cpu/UnitTestLogUtils.h | 3 +-- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/tests/cpu/Config_tests.cpp b/tests/cpu/Config_tests.cpp index 4d76fda434..47ed7528f0 100644 --- a/tests/cpu/Config_tests.cpp +++ b/tests/cpu/Config_tests.cpp @@ -469,7 +469,7 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) } { - // Test that aces_interchange role has the wrong colorspace type. + // Test detection of the aces_interchange role having the wrong colorspace type. // Set a display-referred colorspace to both interchange roles. config->setRole(OCIO::ROLE_INTERCHANGE_SCENE, dcs->getName()); @@ -485,7 +485,7 @@ OCIO_ADD_TEST(Config, required_roles_for_version_2_2) } { - // Test that cie_xyz_d65_interchange role has the wrong colorspace type. + // Test detection of the cie_xyz_d65_interchange role having the wrong colorspace type. // Set a scene-referred colorspace to both interchange roles. config->setRole(OCIO::ROLE_INTERCHANGE_SCENE, scs->getName()); diff --git a/tests/cpu/UnitTestLogUtils.cpp b/tests/cpu/UnitTestLogUtils.cpp index 60a5471a15..6fd3896561 100644 --- a/tests/cpu/UnitTestLogUtils.cpp +++ b/tests/cpu/UnitTestLogUtils.cpp @@ -78,12 +78,10 @@ bool LogGuard::findAndRemove(const std::string & line) const return false; } -bool LogGuard::findAllAndRemove(const std::string & line) const +bool LogGuard::findAllAndRemove(const std::string & sPattern) const { - // Escape the line to prevent error in regex if the line contains regex character. bool found = false; - std::string escaped_line = std::regex_replace(line, std::regex("[\\[\\](){}*+?.^$|\\\\]"), "\\$&"); - std::regex pattern(R"(^\[OpenColorIO Info\]: Inactive.*)" + escaped_line + R"([\r\n]+)"); + std::regex pattern(sPattern); std::smatch match; while (std::regex_search(g_output, match, pattern)) { @@ -150,7 +148,8 @@ bool checkAndMuteDisplayInterchangeRoleError(LogGuard & logGuard) void muteInactiveColorspaceInfo(LogGuard & logGuard) { const std::string str = "- Display' is neither a color space nor a named transform."; - logGuard.findAllAndRemove(str); + const std::string pattern = R"(^\[OpenColorIO Info\]: Inactive.*)" + str + R"([\r\n]+)"; + logGuard.findAllAndRemove(pattern); } } // namespace OCIO_NAMESPACE diff --git a/tests/cpu/UnitTestLogUtils.h b/tests/cpu/UnitTestLogUtils.h index 470088dd72..ba05ecf1f6 100644 --- a/tests/cpu/UnitTestLogUtils.h +++ b/tests/cpu/UnitTestLogUtils.h @@ -5,7 +5,6 @@ #define INCLUDED_OCIO_UNITTEST_LOGUTILS_H #include -#include "utils/StringUtils.h" namespace OCIO_NAMESPACE { @@ -27,7 +26,7 @@ class LogGuard bool empty() const; bool findAndRemove(const std::string & str) const; - bool findAllAndRemove(const std::string & str) const; + bool findAllAndRemove(const std::string & sPattern) const; void print(); private: LoggingLevel m_logLevel;