Start working on improving test coverage (#1424)

* fix potential crash * Add initial tests for half utility operations in core * add dump of attr to support testing of debug code (coverage) * Add simple script around running and generating coverage report locally using clang tools --------- Signed-off-by: Kimball Thurston <[email protected]>
AcademySoftwareFoundation · May 20, 2023 · a00bac5 · a00bac5
1 parent a91df52
commit a00bac5
Show file tree

Hide file tree

Showing 7 changed files with 204 additions and 18 deletions.
diff --git a/share/clang_coverage.sh b/share/clang_coverage.sh
@@ -0,0 +1,98 @@
+#! /bin/bash
+
+# This script can be used to generate a local test coverage report
+# using the clang / llvm toolset for doing so
+# It should hopefully be obvious how to adjust, but should work
+# on most linux distros
+
+buildtype=Debug
+builddir=build.coverage
+
+haveninja=`which ninja`
+
+#imathoverride="-DOPENEXR_FORCE_INTERNAL_IMATH=ON -DIMATH_REPO=/home/user/Development/Imath"
+
+# also turn on most of the warnings because we should look at that
+# as well...
+cwarns="-fstack-protector-all -Weverything -Wno-reserved-identifier -Wno-covered-switch-default -Wno-cast-align -Wno-overlength-strings -fprofile-arcs -fprofile-instr-generate -fcoverage-mapping"
+cxxwarns="-fstack-protector-all -Wno-sign-conversion -Wno-float-equal -Wno-padded -Wno-zero-as-null-pointer-constant -Wno-old-style-cast -Wno-c++98-compat -Wno-c++98-compat-pedantic -Wno-missing-braces -fprofile-instr-generate -fcoverage-mapping"
+
+genargs=""
+if [[ "$haveninja" != "" ]]; then
+    genargs="-G Ninja"
+fi
+
+if [[ ! -e "${builddir}" ]]; then
+    export CC=/usr/bin/clang
+    export CXX=/usr/bin/clang++
+
+    cmake -B ${blddir} -S . ${genargs} ${imathoverride} -DCMAKE_C_FLAGS="${cwarns}" -DCMAKE_BUILD_TYPE=${buildtype} || exit 1
+fi
+
+if [[ "$haveninja" != "" ]]; then
+    ninja -C ${builddir} || exit 1
+else
+    nproc=`cat /proc/cpuinfo|grep processor|wc -l`
+    make -j${nproc} -C ${builddir} || exit 1
+fi
+
+# archive previous runs????
+rm -rf coverage
+
+/usr/bin/env LLVM_PROFILE_FILE=coverage/core.profraw build.coverage/bin/OpenEXRCoreTest || exit 1
+/usr/bin/env LLVM_PROFILE_FILE=coverage/exr.profraw build.coverage/bin/OpenEXRTest || exit 1
+/usr/bin/env LLVM_PROFILE_FILE=coverage/util.profraw build.coverage/bin/OpenEXRUtilTest || exit 1
+
+llvm-profdata merge -sparse -o coverage/exr.profdata coverage/core.profraw coverage/exr.profraw coverage/util.profraw
+
+llvm-cov show \
+         build.coverage/bin/OpenEXRCoreTest \
+         -object build.coverage/src/lib/OpenEXRCore/libOpenEXRCore-3_2_d.so \
+         -instr-profile=coverage/exr.profdata \
+         -show-regions \
+         -show-expansions \
+         --output-dir=coverage/core_only \
+         --format="html" \
+         -ignore-filename-regex='src/test/.*' \
+         -ignore-filename-regex='build.coverage/.*' \
+         -ignore-filename-regex 'src/lib/OpenEXR/Imf*'
+
+llvm-cov show \
+         build.coverage/bin/OpenEXRTest \
+         -object build.coverage/src/lib/OpenEXR/libOpenEXR-3_2_d.so \
+         -instr-profile=coverage/exr.profdata \
+         -show-regions \
+         -show-expansions \
+         --output-dir=coverage/exr_only \
+         --format="html" \
+         -ignore-filename-regex='src/test/.*' \
+         -ignore-filename-regex='build.coverage/.*' \
+         -ignore-filename-regex 'src/lib/OpenEXRCore/*'
+
+llvm-cov show \
+         build.coverage/bin/OpenEXRUtilTest \
+         -object build.coverage/src/lib/OpenEXRUtil/libOpenEXRUtil-3_2_d.so \
+         -instr-profile=coverage/exr.profdata \
+         -show-regions \
+         -show-expansions \
+         --output-dir=coverage/util_only \
+         --format="html" \
+         -ignore-filename-regex='src/test/.*' \
+         -ignore-filename-regex='build.coverage/.*' \
+         -ignore-filename-regex 'src/lib/OpenEXR/Imf*' \
+         -ignore-filename-regex 'src/lib/OpenEXRCore/*'
+
+llvm-cov show \
+         build.coverage/bin/OpenEXRCoreTest \
+         -object build.coverage/bin/OpenEXRTest \
+         -object build.coverage/bin/OpenEXRUtilTest \
+         -object build.coverage/src/lib/OpenEXRCore/libOpenEXRCore-3_2_d.so \
+         -object build.coverage/src/lib/OpenEXR/libOpenEXR-3_2_d.so \
+         -object build.coverage/src/lib/OpenEXRUtil/libOpenEXRUtil-3_2_d.so \
+         -instr-profile=coverage/exr.profdata \
+         -show-regions \
+         -show-expansions \
+         --output-dir=coverage/combined \
+         --format="html" \
+         -ignore-filename-regex='src/test/.*' \
+         -ignore-filename-regex='build.coverage/.*'
diff --git a/src/lib/OpenEXRCore/debug.c b/src/lib/OpenEXRCore/debug.c
@@ -17,6 +17,9 @@
 static void
 print_attr (const exr_attribute_t* a, int verbose)
 {
+    if (!a)
+        return;
+
     printf ("%s: ", a->name);
     if (verbose) printf ("%s ", a->type_name);
     switch (a->type)

diff --git a/src/test/OpenEXRCoreTest/CMakeLists.txt b/src/test/OpenEXRCoreTest/CMakeLists.txt
@@ -62,6 +62,7 @@ define_openexrcore_tests(
  testBaseLimits
  testBaseDebug
  testCPUIdent
+ testHalf
  testXDR
  testBufferCompression
 

diff --git a/src/test/OpenEXRCoreTest/base_units.cpp b/src/test/OpenEXRCoreTest/base_units.cpp
@@ -3,6 +3,13 @@
 ** Copyright Contributors to the OpenEXR Project.
 */
 
+#if (defined(_WIN32) || defined(_WIN64))
+#    ifdef NOMINMAX
+#        undef NOMINMAX
+#    endif
+#    define NOMINMAX
+#endif
+
 #include <openexr.h>
 
 #include "base_units.h"
@@ -14,36 +21,37 @@
 
 #include <ImfSystemSpecific.h>
 #include "../../lib/OpenEXRCore/internal_cpuid.h"
+#include "../../lib/OpenEXRCore/internal_coding.h"
 
 void
 testBase (const std::string& tempdir)
 {
-    int         maj, min, patch;
+    int         major, minor, patch;
     const char* extra;
     const char* compextra = COMP_EXTRA;
 
-    exr_get_library_version (&maj, &min, &patch, &extra);
-    if (maj != COMP_MAJ || min != COMP_MIN || patch != COMP_PATCH ||
+    exr_get_library_version (&major, &minor, &patch, &extra);
+    if (major != COMP_MAJ || minor != COMP_MIN || patch != COMP_PATCH ||
         !strcmp (extra, compextra))
     {
-        std::cerr << "ERROR testing library, wrong library version: " << maj
-                  << "." << min << "." << patch;
+        std::cerr << "ERROR testing library, wrong library version: " << major
+                  << "." << minor << "." << patch;
         if (extra[0] != '\0') std::cerr << "-" << extra;
         std::cerr << " vs compiled in " << COMP_MAJ << "." << COMP_MIN << "."
                   << COMP_PATCH;
         if (compextra[0] != '\0') std::cerr << "-" << compextra;
         std::cerr << std::endl;
         EXRCORE_TEST (false);
     }
-    std::cout << "Testing OpenEXR library version: " << maj << "." << min << "."
+    std::cout << "Testing OpenEXR library version: " << major << "." << minor << "."
               << patch;
     if (extra[0] != '\0') std::cout << "-" << extra;
     std::cout << std::endl;
 
-    exr_get_library_version (NULL, &min, &patch, &extra);
-    exr_get_library_version (&maj, NULL, &patch, &extra);
-    exr_get_library_version (&maj, &min, NULL, &extra);
-    exr_get_library_version (&maj, &min, &patch, NULL);
+    exr_get_library_version (NULL, &minor, &patch, &extra);
+    exr_get_library_version (&major, NULL, &patch, &extra);
+    exr_get_library_version (&major, &minor, NULL, &extra);
+    exr_get_library_version (&major, &minor, &patch, NULL);
 }
 
 void
@@ -298,6 +306,36 @@ testBaseLimits (const std::string& tempdir)
     }
     exr_set_default_maximum_image_size (0, 0);
     exr_set_default_maximum_tile_size (0, 0);
+
+    exr_set_default_zip_compression_level (4);
+    exr_get_default_zip_compression_level (&mxw);
+    EXRCORE_TEST (mxw == 4);
+
+    exr_set_default_zip_compression_level (-1);
+    exr_get_default_zip_compression_level (&mxw);
+    EXRCORE_TEST (mxw == -1);
+    exr_set_default_zip_compression_level (-2);
+    exr_get_default_zip_compression_level (&mxw);
+    EXRCORE_TEST (mxw == -1);
+
+    exr_set_default_zip_compression_level (15);
+    exr_get_default_zip_compression_level (&mxw);
+    EXRCORE_TEST (mxw == 9);
+    exr_set_default_zip_compression_level (-1);
+
+    float dcq;
+    exr_set_default_dwa_compression_quality (23.f);
+    exr_get_default_dwa_compression_quality (&dcq);
+    EXRCORE_TEST (dcq == 23.f);
+
+    exr_set_default_dwa_compression_quality (-1.f);
+    exr_get_default_dwa_compression_quality (&dcq);
+    EXRCORE_TEST (dcq == 0.f);
+
+    exr_set_default_dwa_compression_quality (200.f);
+    exr_get_default_dwa_compression_quality (&dcq);
+    EXRCORE_TEST (dcq == 100.f);
+    exr_set_default_dwa_compression_quality (45.f);
 }
 
 void
@@ -337,4 +375,34 @@ void testCPUIdent (const std::string& tempdir)
             << "CPU Id test sse2 mismatch: " << hsse2 << " vs " << (int)id.sse2 << std::endl;
         EXRCORE_TEST (false);
     }
+
+#if defined(__x86_64__) || defined(_M_X64)
+    if (has_native_half () != (hf16c && havx))
+    {
+        std::cerr << "CPU Id test has native half mismatch" << std::endl;
+        EXRCORE_TEST (false);
+    }
+#else
+    has_native_half ();
+#endif
+}
+
+void testHalf (const std::string& tempdir)
+{
+    EXRCORE_TEST (half_to_float (0) == 0.f);
+    EXRCORE_TEST (float_to_half (0.f) == 0);
+    EXRCORE_TEST (float_to_half_int (0.f) == 0);
+    EXRCORE_TEST (half_to_float_int (0) == 0);
+    EXRCORE_TEST (half_to_uint (0) == 0);
+    EXRCORE_TEST (half_to_uint (0x8000) == 0);
+    EXRCORE_TEST (float_to_uint (0) == 0);
+    EXRCORE_TEST (float_to_uint (-1.f) == 0);
+    EXRCORE_TEST (float_to_uint_int (0) == 0);
+
+    EXRCORE_TEST (uint_to_half (0) == 0);
+    EXRCORE_TEST (uint_to_half (128344) == 0x7c00);
+
+    EXRCORE_TEST (uint_to_float (0) == 0.f);
+    EXRCORE_TEST (uint_to_float_int (0) == 0);
 }
+
diff --git a/src/test/OpenEXRCoreTest/base_units.h b/src/test/OpenEXRCoreTest/base_units.h
@@ -11,5 +11,6 @@ void testBaseErrors (const std::string& tempdir);
 void testBaseLimits (const std::string& tempdir);
 void testBaseDebug (const std::string& tempdir);
 void testCPUIdent (const std::string& tempdir);
+void testHalf (const std::string& tempdir);
 
 #endif // OPENEXR_CORE_TEST_BASE_H
diff --git a/src/test/OpenEXRCoreTest/general_attr.cpp b/src/test/OpenEXRCoreTest/general_attr.cpp
@@ -282,6 +282,8 @@ testAttrStrings (const std::string& tempdir)
     //testStringHelper (NULL);
     exr_context_t f = createDummyFile ("<string>");
     testStringHelper (f);
+    exr_print_context_info (f, 0);
+    exr_print_context_info (f, 1);
     exr_finish (&f);
 }
 
@@ -445,6 +447,8 @@ testAttrStringVectors (const std::string& tempdir)
     //testStringVectorHelper (NULL);
     exr_context_t f = createDummyFile ("<stringvector>");
     testStringVectorHelper (f);
+    exr_print_context_info (f, 0);
+    exr_print_context_info (f, 1);
     exr_finish (&f);
 }
 
@@ -528,6 +532,8 @@ testAttrFloatVectors (const std::string& tempdir)
     //testFloatVectorHelper (NULL);
     exr_context_t f = createDummyFile ("<floatvector>");
     testFloatVectorHelper (f);
+    exr_print_context_info (f, 0);
+    exr_print_context_info (f, 1);
     exr_finish (&f);
 }
 
@@ -784,6 +790,8 @@ testAttrChlists (const std::string& tempdir)
     //testChlistHelper (NULL);
     exr_context_t f = createDummyFile ("<chlist>");
     testChlistHelper (f);
+    exr_print_context_info (f, 0);
+    exr_print_context_info (f, 1);
     exr_finish (&f);
 }
 
@@ -843,6 +851,8 @@ testAttrPreview (const std::string& tempdir)
     //testPreviewHelper (NULL);
     exr_context_t f = createDummyFile ("<preview>");
     testPreviewHelper (f);
+    exr_print_context_info (f, 0);
+    exr_print_context_info (f, 1);
     exr_finish (&f);
 }
 
@@ -937,6 +947,8 @@ testAttrOpaque (const std::string& tempdir)
     //testOpaqueHelper (NULL);
     exr_context_t f = createDummyFile ("<opaque>");
     testOpaqueHelper (f);
+    exr_print_context_info (f, 0);
+    exr_print_context_info (f, 1);
     exr_finish (&f);
 }
 
@@ -1746,27 +1758,29 @@ testXDR (const std::string& tempdir)
     uint16_t v16buf[] = {0xAA00, 0xBB11, 0xCC22, 0xDD33, 0xEE44};
     uint32_t v32buf[] = {0xAA00BB11, 0xCC22DD33};
     uint64_t v64buf[] = {0xAA00BB11CC22DD33, 0xEE44FF5500661177};
-
-    EXRCORE_TEST (one_from_native64 (one_to_native64 (v64)) == v64);
-    EXRCORE_TEST (one_from_native32 (one_to_native32 (v32)) == v32);
-    EXRCORE_TEST (one_from_native16 (one_to_native16 (v16)) == v16);
+    float v32f = 42.f;
+    EXRCORE_TEST (one_to_native64 (one_from_native64 (v64)) == v64);
+    EXRCORE_TEST (one_to_native32 (one_from_native32 (v32)) == v32);
+    EXRCORE_TEST (one_to_native16 (one_from_native16 (v16)) == v16);
 #if EXR_HOST_IS_NOT_LITTLE_ENDIAN
     EXRCORE_TEST (one_to_native64 (v64) == ov64);
     EXRCORE_TEST (one_to_native32 (v32) == ov32);
     EXRCORE_TEST (one_to_native16 (v16) == ov16);
 #endif
-    priv_to_native (v8buf, 5, sizeof (uint8_t));
     priv_from_native (v8buf, 5, sizeof (uint8_t));
+    priv_to_native (v8buf, 5, sizeof (uint8_t));
     EXRCORE_TEST (v8buf[2] == 0xCC);
-    priv_to_native (v16buf, 5, sizeof (uint16_t));
     priv_from_native (v16buf, 5, sizeof (uint16_t));
+    priv_to_native (v16buf, 5, sizeof (uint16_t));
     EXRCORE_TEST (v16buf[2] == 0xCC22);
-    priv_to_native (v32buf, 2, sizeof (uint32_t));
     priv_from_native (v32buf, 2, sizeof (uint32_t));
+    priv_to_native (v32buf, 2, sizeof (uint32_t));
     EXRCORE_TEST (v32buf[1] == 0xCC22DD33);
-    priv_to_native (v64buf, 2, sizeof (uint64_t));
     priv_from_native (v64buf, 2, sizeof (uint64_t));
+    priv_to_native (v64buf, 2, sizeof (uint64_t));
     EXRCORE_TEST (v64buf[0] == 0xAA00BB11CC22DD33);
+
+    EXRCORE_TEST (one_to_native_float (one_from_native_float (v32f)) == v32f);
 }
 
 #if defined(__GNUC__) && __GNUC__ > 7

diff --git a/src/test/OpenEXRCoreTest/main.cpp b/src/test/OpenEXRCoreTest/main.cpp
@@ -150,6 +150,7 @@ main (int argc, char* argv[])
     TEST (testBaseLimits, "core");
     TEST (testBaseDebug, "core");
     TEST (testCPUIdent, "core");
+    TEST (testHalf, "core");
     TEST (testXDR, "core");
     TEST (testBufferCompression, "core");