Improve reliability/readability of bin tests scripts and failure output (#2044)

* Improve reliability and readability of bin tests scripts and failure output

The src/test/bin tests have been failing intermittently in the CI
checks, and the problem appears to be known instability in CMake's
file(DOWNLOAD) when invoked from processes in GitHub actions. The
downloaded files occasionally have 0 size.

This change brings the test images into the git repo rather than
downloading them from
https://github.com/AcademySoftwareFoundation/openexr-images at
configure time. I did that way originally to avoid repo bloat, so I
trimmed the list of files used by the tests, so the additional data is
not that signifiant, a few Mb.

This also restructures the test scripts to produce more informative
output, and restructures the CI steps to make it easier to navigate
the logs.

Signed-off-by: Cary Phillips <cary@ilm.com>

* put bin test images in dict; label tests with OpenEXR.bin

Signed-off-by: Cary Phillips <cary@ilm.com>

* typo

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Cary Phillips <cary@ilm.com>

Author: cary-ilm

.github/workflows/ci_steps.yml

@@ -167,27 +167,39 @@ jobs:
         working-directory: _build
         shell: bash
 
-      - name: Configure, Build, Test
+      - name: Configure & Build
         if: inputs.msystem == ''
         run: |
+          set -x
           cmake --version
           cmake ${{ env.CMAKE_ARGS }}
           cmake --build . --target install --config ${{ inputs.build-type }}
-          if [ "${{ inputs.BUILD_TESTING }}" == "ON" ]; then
-            ctest -T Test -C ${{ inputs.build-type }} --timeout 7200 --output-on-failure -VV
-          fi
         working-directory: _build
         shell: bash
 
-      - name: Configure, Build, Test (msys2) # requires msys2 shell
+      - name: Configure & Build (msys2) # requires msys2 shell
         if: inputs.msystem != ''
         run: |
+          set -x
           cmake --version
           cmake ${{ env.CMAKE_ARGS }}
           cmake --build . --target install --config ${{ inputs.build-type }}
-          if [ "${{ inputs.BUILD_TESTING }}" == "ON" ]; then
-            ctest -T Test -C ${{ inputs.build-type }} --timeout 7200 --output-on-failure -VV
-          fi
+        working-directory: _build
+        shell: msys2 {0}
+
+      - name: Test
+        if: inputs.BUILD_TESTING == 'ON' && inputs.msystem == ''
+        run: |
+          set -x
+          ctest -T Test -C ${{ inputs.build-type }} --timeout 7200 --output-on-failure -VV
+        working-directory: _build
+        shell: bash
+
+      - name: Test (msys2) # requires msys2 shell
+        if: inputs.BUILD_TESTING == 'ON' && inputs.msystem != ''
+        run: |
+          set -x
+          ctest -T Test -C ${{ inputs.build-type }} --timeout 7200 --output-on-failure -VV
         working-directory: _build
         shell: msys2 {0}
 

src/test/bin/CMakeLists.txt

@@ -11,52 +11,19 @@ if(BUILD_TESTING)
 
   message(STATUS "adding bin tests")
 
-  set(OPENEXR_IMAGES_REPO "https://raw.githubusercontent.com/AcademySoftwareFoundation/openexr-images" CACHE STRING
-    "Bin test Image repo")
-
-  set(OPENEXR_IMAGES_TAG "v1.0" CACHE STRING "Bin test image repo tag")
-
-  if ("${OPENEXR_IMAGES_TAG}" STREQUAL "")
-    set(images_url ${OPENEXR_IMAGES_REPO})
-  else()
-    set(images_url ${OPENEXR_IMAGES_REPO}/${OPENEXR_IMAGES_TAG})
-  endif()
-
-  set(images
-      TestImages/GrayRampsHorizontal.exr
-      LuminanceChroma/Garden.exr
-      MultiResolution/ColorCodedLevels.exr
-      MultiResolution/WavyLinesLatLong.exr
-      MultiResolution/WavyLinesCube.exr
-      LuminanceChroma/Flowers.exr
-      MultiView/Adjuster.exr
-      Chromaticities/Rec709_YC.exr
-      Chromaticities/Rec709.exr
-      Chromaticities/XYZ_YC.exr
-      Chromaticities/XYZ.exr
-      TestImages/GammaChart.exr
-      Beachball/singlepart.0001.exr
-      v2/LeftView/Balls.exr
-      v2/Stereo/Trunks.exr
-      Beachball/multipart.0001.exr
-  )
-
-  foreach(image ${images})
-    if (NOT EXISTS ${CMAKE_CURRENT_BINARY_DIR}/${image})
-      message(STATUS "Downloading ${images_url}/${image}")
-      file(DOWNLOAD ${images_url}/${image} ${CMAKE_CURRENT_BINARY_DIR}/${image})
-    endif()
-  endforeach()
+  set(TEST_IMAGE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/test_images")
 
   add_test(NAME OpenEXR.bin.exrinfo
            COMMAND ${Python3_EXECUTABLE}
            ${CMAKE_CURRENT_SOURCE_DIR}/test_exrinfo.py
-           $<TARGET_FILE:exrinfo> ${CMAKE_CURRENT_BINARY_DIR} ${OPENEXR_VERSION})
+           $<TARGET_FILE:exrinfo> ${TEST_IMAGE_DIR} ${OPENEXR_VERSION})
+  set_tests_properties(OpenEXR.bin.exrinfo PROPERTIES LABELS "OpenEXR.bin")
 
   add_test(NAME OpenEXR.bin.exrheader
            COMMAND ${Python3_EXECUTABLE}
            ${CMAKE_CURRENT_SOURCE_DIR}/test_exrheader.py
-           $<TARGET_FILE:exrheader> ${CMAKE_CURRENT_BINARY_DIR} ${OPENEXR_VERSION})
+           $<TARGET_FILE:exrheader> ${TEST_IMAGE_DIR} ${OPENEXR_VERSION})
+  set_tests_properties(OpenEXR.bin.exrheader PROPERTIES LABELS "OpenEXR.bin")
 
   set(tests
       exr2aces
@@ -75,16 +42,25 @@ if(BUILD_TESTING)
            COMMAND ${Python3_EXECUTABLE}
            ${CMAKE_CURRENT_SOURCE_DIR}/test_${test}.py
            $<TARGET_FILE:${test}> $<TARGET_FILE:exrinfo>
-           ${CMAKE_CURRENT_BINARY_DIR} ${OPENEXR_VERSION})
+           ${TEST_IMAGE_DIR} ${OPENEXR_VERSION})
+    set_tests_properties(OpenEXR.bin.${test} PROPERTIES LABELS "OpenEXR.bin")
   endforeach()
 
-  # run exrcheck on all images
+  # run exrcheck on test images
+  set(images
+      ${TEST_IMAGE_DIR}/GrayRampsHorizontal.exr
+      ${TEST_IMAGE_DIR}/WavyLinesLatLong.exr
+      ${TEST_IMAGE_DIR}/WavyLinesCube.exr
+      ${TEST_IMAGE_DIR}/Flowers.exr
+      ${TEST_IMAGE_DIR}/GammaChart.exr
+      ${TEST_IMAGE_DIR}/multipart.0001.exr
+  )
   add_test(NAME OpenEXR.bin.exrcheck
            COMMAND ${Python3_EXECUTABLE}
            ${CMAKE_CURRENT_SOURCE_DIR}/test_exrcheck.py
            $<TARGET_FILE:exrcheck>
-           ${CMAKE_CURRENT_BINARY_DIR}
            ${images})
+  set_tests_properties(OpenEXR.bin.exrcheck PROPERTIES LABELS "OpenEXR.bin")
 
 endif()
 

src/test/bin/do_run.py

@@ -0,0 +1,27 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) Contributors to the OpenEXR Project.
+
+from subprocess import PIPE, run
+import sys
+
+def do_run(cmd, expect_error = False, data=None):
+    cmd_string = " ".join(cmd)
+    print(f"running {cmd_string}")
+    if data:
+        result = run (cmd, input=data, stdout=PIPE, stderr=PIPE)
+    else:
+        result = run (cmd, stdout=PIPE, stderr=PIPE, universal_newlines=True)
+    if expect_error:
+        if result.returncode != 0:
+            return result
+        print(f"error: {cmd_string} did not fail as expected")
+        print(f"stdout:\n{result.stdout}")
+        print(f"stderr:\n{result.stderr}")
+        sys.exit(1)
+            
+    if result.returncode != 0:
+        print(f"error: {cmd_string} failed: returncode={result.returncode}")
+        print(f"stdout:\n{result.stdout}")
+        print(f"stderr:\n{result.stderr}")
+        sys.exit(1)
+    return result

src/test/bin/test_exr2aces.py

@@ -4,43 +4,40 @@
 # Copyright (c) Contributors to the OpenEXR Project.
 
 import sys, os, tempfile, atexit
-from subprocess import PIPE, run
+from do_run import do_run
 
-print(f"testing exr2aces: {sys.argv}")
+print(f"testing exr2aces: {' '.join(sys.argv)}")
 
 exr2aces = sys.argv[1]
 exrinfo = sys.argv[2]
 image_dir = sys.argv[3]
 version = sys.argv[4]
 
+test_images = {}
+test_images["GrayRampsHorizontal"] = f"{image_dir}/GrayRampsHorizontal.exr"
+
+if not os.path.isfile(exr2aces) or not os.access(exr2aces, os.X_OK):
+    print(f"error: no such file: {exr2aces}")
+    sys.exit(1)
+
 # no args = usage message, error
-result = run ([exr2aces], stdout=PIPE, stderr=PIPE, universal_newlines=True)
-print(" ".join(result.args))
-assert(result.returncode != 0), "\n"+result.stderr
-assert(result.stderr.startswith ("Usage: ")), "\n"+result.stderr
+result = do_run ([exr2aces], True)
+assert result.stderr.startswith ("Usage: ")
 
 # -h = usage message
-result = run ([exr2aces, "-h"], stdout=PIPE, stderr=PIPE, universal_newlines=True)
-print(" ".join(result.args))
-assert(result.returncode == 0), "\n"+result.stderr
-assert(result.stdout.startswith ("Usage: ")), "\n"+result.stdout
+result = do_run ([exr2aces, "-h"])
+assert result.stdout.startswith ("Usage: ")
 
-result = run ([exr2aces, "--help"], stdout=PIPE, stderr=PIPE, universal_newlines=True)
-print(" ".join(result.args))
-assert(result.returncode == 0), "\n"+result.stderr
-assert(result.stdout.startswith ("Usage: ")), "\n"+result.stdout
+result = do_run ([exr2aces, "--help"])
+assert result.stdout.startswith ("Usage: ")
 
 # --version
-result = run ([exr2aces, "--version"], stdout=PIPE, stderr=PIPE, universal_newlines=True)
-print(" ".join(result.args))
-assert(result.returncode == 0), "\n"+result.stderr
-assert(result.stdout.startswith ("exr2aces")), "\n"+result.stdout
-assert(version in result.stdout), "\n"+result.stdout
+result = do_run ([exr2aces, "--version"])
+assert result.stdout.startswith ("exr2aces")
+assert version in result.stdout
 
 # invalid arguments
-result = run ([exr2aces, "foo.exr", "bar.exr"], stdout=PIPE, stderr=PIPE, universal_newlines=True)
-print(" ".join(result.args))
-assert(result.returncode != 0), "\n"+result.stderr
+result = do_run ([exr2aces, "foo.exr", "bar.exr"], True)
 
 def find_line(keyword, lines):
     for line in lines:
@@ -55,17 +52,12 @@ def cleanup():
     print(f"deleting {outimage}")
 atexit.register(cleanup)
 
-image = f"{image_dir}/TestImages/GrayRampsHorizontal.exr"
-result = run ([exr2aces, "-v", image, outimage], stdout=PIPE, stderr=PIPE, universal_newlines=True)
-print(" ".join(result.args))
-assert(result.returncode == 0), "\n"+result.stderr
+result = do_run ([exr2aces, "-v", test_images["GrayRampsHorizontal"], outimage])
 
-result = run ([exrinfo, "-v", outimage], stdout=PIPE, stderr=PIPE, universal_newlines=True)
-print(" ".join(result.args))
-assert(result.returncode == 0), "\n"+result.stderr
+result = do_run ([exrinfo, "-v", outimage])
 
 # confirm the output has the proper chromaticities
-assert("chromaticities: chromaticities r[0.7347, 0.2653] g[0, 1] b[0.0001, -0.077] w[0.32168, 0.33767]" in result.stdout), "\n"+result.stdout
+assert "chromaticities: chromaticities r[0.7347, 0.2653] g[0, 1] b[0.0001, -0.077] w[0.32168, 0.33767]" in result.stdout
 
 print("success")
 

src/test/bin/test_exrcheck.py

@@ -4,37 +4,25 @@
 # Copyright (c) Contributors to the OpenEXR Project.
 
 import sys, os
-from subprocess import PIPE, run
+from do_run import do_run
 
 print(f"testing exrcheck: {' '.join(sys.argv)}")
 
 exrcheck = sys.argv[1]
-image_dir = sys.argv[2]
+image_files = sys.argv[2:]
 
-for exr_file in sys.argv[3:]:
+if not os.path.isfile(exrcheck) or not os.access(exrcheck, os.X_OK):
+    print(f"error: no such file: {exrcheck}")
+    sys.exit(1)
 
-    exr_path = f"{image_dir}/{exr_file}"
-
-    result = run ([exrcheck, exr_path], stdout=PIPE, stderr=PIPE, universal_newlines=True)
-    print(" ".join(result.args))
-    assert(result.returncode == 0), "\n"+result.stderr
-
-    result = run ([exrcheck, "-m", exr_path], stdout=PIPE, stderr=PIPE, universal_newlines=True)
-    print(" ".join(result.args))
-    assert(result.returncode == 0), "\n"+result.stderr
-
-    result = run ([exrcheck, "-t", exr_path], stdout=PIPE, stderr=PIPE, universal_newlines=True)
-    print(" ".join(result.args))
-    assert(result.returncode == 0), "\n"+result.stderr
-
-    result = run ([exrcheck, "-s", exr_path], stdout=PIPE, stderr=PIPE, universal_newlines=True)
-    print(" ".join(result.args))
-    assert(result.returncode == 0), "\n"+result.stderr
-
-    result = run ([exrcheck, "-c", exr_path], stdout=PIPE, stderr=PIPE, universal_newlines=True)
-    print(" ".join(result.args))
-    assert(result.returncode == 0), "\n"+result.stderr
+for exr_file in image_files:
 
+    do_run([exrcheck, exr_file])
+    do_run([exrcheck, "-m", exr_file])
+    do_run([exrcheck, "-t", exr_file])
+    do_run([exrcheck, "-s", exr_file])
+    do_run([exrcheck, "-c", exr_file])
 
 print("success.")
+sys.exit(0)