Analysis: more test related fixes to try to increase code coverage (#3643)

lgritz · web-flow · commit 1beeb27dae7c · 2022-11-01T08:48:49.000-07:00
* Test oiiotool control flow syntax mistakes
* Test oiiotool --set with array of strings
* oiiotool: better adjust_geometry unit testing

Helper utility additions:

* oiiotool --no-error-exit

  `--no-error-exit` causes an error to not exit immediately, but
  rather to try its best to continue execuing command line
  operations. Use with caution! This is mainly for unit tests and
  debugging.

  `--unittest` also turns this on automatically, so that tests that
  are probing failures don't cause an exit.

* unittest.h: If OIIO_UNIT_TEST_QUIET_SUCCESS is defined, don't print
  anything extra for unit test success
diff --git a/src/build-scripts/ci-test.bash b/src/build-scripts/ci-test.bash
@@ -8,7 +8,10 @@ set -ex
 : ${CTEST_TEST_TIMEOUT:=180}
 
 $OpenImageIO_ROOT/bin/oiiotool --version --help || /bin/true
-$OpenImageIO_ROOT/bin/oiiotool --unittest --list-formats --runstats || /bin/true
+
+# Catch-all of some unit tests and args that aren't used elsewhere
+$OpenImageIO_ROOT/bin/oiiotool --unittest --list-formats --threads 0 \
+                 --cache 1000 --autotile --autopremult --runstats || /bin/true
 
 echo "Parallel test ${CTEST_PARALLEL_LEVEL}"
 echo "Default timeout ${CTEST_TEST_TIMEOUT}"
diff --git a/src/doc/oiiotool.rst b/src/doc/oiiotool.rst
@@ -876,6 +876,11 @@ output each one to a different file, with names `sub0001.tif`,
     sequence will do and what it costs, but without producing any saved
     output files.
 
+.. option:: --no-error-exit
+
+    If an error is encountered, try to continue executing any remaining
+    commands, rather than exiting immediately. Use with caution!
+
 .. option:: --debug
 
     Debug mode --- print lots of information about what operations are being
diff --git a/src/include/OpenImageIO/unittest.h b/src/include/OpenImageIO/unittest.h
@@ -44,7 +44,9 @@ class UnitTestFailureCounter {
             std::cout << Sysutil::Term(std::cout).ansi("red", "ERRORS!\n");
             std::exit(m_failures != 0);
         } else {
+#ifndef OIIO_UNIT_TEST_QUIET_SUCCESS
             std::cout << Sysutil::Term(std::cout).ansi("green", "OK\n");
+#endif
         }
     }
     const UnitTestFailureCounter& operator++() noexcept  // prefix
diff --git a/src/oiiotool/oiiotool.cpp b/src/oiiotool/oiiotool.cpp
@@ -37,6 +37,11 @@
 #include <OpenImageIO/sysutil.h>
 #include <OpenImageIO/timer.h>
 
+#ifndef NDEBUG
+#    define OIIO_UNIT_TEST_QUIET_SUCCESS
+#    include <OpenImageIO/unittest.h>
+#endif
+
 using namespace OIIO;
 using namespace OiioTool;
 using namespace ImageBufAlgo;
@@ -276,10 +281,10 @@ unit_test_scan_box()
 {
     Strutil::print("unit test scan_box...\n");
     int xmin = -1, ymin = -1, xmax = -1, ymax = -1;
-    OIIO_ASSERT(scan_box("11,12,13,14", xmin, ymin, xmax, ymax) && xmin == 11
-                && ymin == 12 && xmax == 13 && ymax == 14);
-    OIIO_ASSERT(scan_box("1,2,3", xmin, ymin, xmax, ymax) == false);
-    OIIO_ASSERT(scan_box("1,2,3,4,5", xmin, ymin, xmax, ymax) == false);
+    OIIO_CHECK_ASSERT(scan_box("11,12,13,14", xmin, ymin, xmax, ymax)
+                      && xmin == 11 && ymin == 12 && xmax == 13 && ymax == 14);
+    OIIO_CHECK_ASSERT(scan_box("1,2,3", xmin, ymin, xmax, ymax) == false);
+    OIIO_CHECK_ASSERT(scan_box("1,2,3,4,5", xmin, ymin, xmax, ymax) == false);
 }
 #endif
 
@@ -493,8 +498,10 @@ Oiiotool::error(string_view command, string_view explanation) const
     // Repeat the command line, so if oiiotool is being called from a
     // script, it's easy to debug how the command was mangled.
     errstream << "Full command line was:\n> " << full_command_line << "\n";
-    ot.ap.abort();  // Cease further processing of the command line
-    ot.return_value = EXIT_FAILURE;
+    if (!ot.noerrexit) {
+        ot.ap.abort();  // Cease further processing of the command line
+        ot.return_value = EXIT_FAILURE;
+    }
 }
 
 
@@ -1583,50 +1590,79 @@ Oiiotool::adjust_geometry(string_view command, int& w, int& h, int& x, int& y,
 static void
 unit_test_adjust_geometry()
 {
+    // box
     int w, h, x, y;
     w = h = x = y = -42;
-    OIIO_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "10,20,130,145")
-                && x == 10 && y == 20 && w == 121 && h == 126);
+    OIIO_CHECK_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "10,20,130,145")
+                      && x == 10 && y == 20 && w == 121 && h == 126);
+
+    // geom
     w = h = x = y = -42;
-    OIIO_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "10x20+100+200")
-                && x == 100 && y == 200 && w == 10 && h == 20);
+    OIIO_CHECK_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "10x20+100+200")
+                      && x == 100 && y == 200 && w == 10 && h == 20);
     w = h = x = y = -42;
-    OIIO_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "10x20-100-200")
-                && x == -100 && y == -200 && w == 10 && h == 20);
+    OIIO_CHECK_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "10x20-100-200")
+                      && x == -100 && y == -200 && w == 10 && h == 20);
+    w = 100, h = 50, x = y = 0;
+    OIIO_CHECK_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "20x0+100+200")
+                      && x == 100 && y == 200 && w == 20 && h == 10);
+    w = 100, h = 50, x = y = 0;
+    OIIO_CHECK_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "0x20+100+200")
+                      && x == 100 && y == 200 && w == 40 && h == 20);
+    OIIO_CHECK_ASSERT(
+        !ot.adjust_geometry("foo", w, h, x, y, "10x20+100+200", true, false));
+
+    // res
     w = h = x = y = -42;
-    OIIO_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "10x20") && x == -42
-                && y == -42 && w == 10 && h == 20);
+    OIIO_CHECK_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "10x20") && x == -42
+                      && y == -42 && w == 10 && h == 20);
+    w = 100, h = 50, x = y = 0;
+    OIIO_CHECK_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "20x0") && x == 0
+                      && y == 0 && w == 20 && h == 10);
+    w = 100, h = 50, x = y = 0;
+    OIIO_CHECK_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "0x20") && x == 0
+                      && y == 0 && w == 40 && h == 20);
+    OIIO_CHECK_ASSERT(
+        !ot.adjust_geometry("foo", w, h, x, y, "10x20", true, false));
+
+    // scale by percentage
     w = h = 100;
     x = y = -42;
-    OIIO_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "200%x50%", true)
-                && x == -42 && y == -42 && w == 200 && h == 50);
+    OIIO_CHECK_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "200%x50%", true)
+                      && x == -42 && y == -42 && w == 200 && h == 50);
     w = h = 100;
     x = y = -42;
-    OIIO_ASSERT(!ot.adjust_geometry("foo", w, h, x, y, "200%x50%"));
+    OIIO_CHECK_ASSERT(!ot.adjust_geometry("foo", w, h, x, y, "200%x50%"));
     w = 640;
     h = 480;
     x = y = -42;
-    OIIO_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "200%", true) && x == -42
-                && y == -42 && w == 1280 && h == 960);
-    OIIO_ASSERT(!ot.adjust_geometry("foo", w, h, x, y, "200%"));
+    OIIO_CHECK_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "200%", true)
+                      && x == -42 && y == -42 && w == 1280 && h == 960);
+    OIIO_CHECK_ASSERT(!ot.adjust_geometry("foo", w, h, x, y, "200%"));
+
+    // offset
     w = h = x = y = -42;
-    OIIO_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "+100+200") && x == 100
-                && y == 200 && w == -42 && h == -42);
+    OIIO_CHECK_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "+100+200")
+                      && x == 100 && y == 200 && w == -42 && h == -42);
+
+    // scale by factor
     w = 640;
     h = 480;
     x = y = -42;
-    OIIO_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "2", true) && x == -42
-                && y == -42 && w == 1280 && h == 960);
-    OIIO_ASSERT(!ot.adjust_geometry("foo", w, h, x, y, "2"));
+    OIIO_CHECK_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "2", true)
+                      && x == -42 && y == -42 && w == 1280 && h == 960);
+    OIIO_CHECK_ASSERT(!ot.adjust_geometry("foo", w, h, x, y, "2"));
     w = 640;
     h = 480;
     x = y = -42;
-    OIIO_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "0.5", true) && x == -42
-                && y == -42 && w == 320 && h == 240);
-    OIIO_ASSERT(!ot.adjust_geometry("foo", w, h, x, y, "0.5"));
+    OIIO_CHECK_ASSERT(ot.adjust_geometry("foo", w, h, x, y, "0.5", true)
+                      && x == -42 && y == -42 && w == 320 && h == 240);
+    OIIO_CHECK_ASSERT(!ot.adjust_geometry("foo", w, h, x, y, "0.5"));
+
+    // errors
     w = h = x = y = -42;
-    OIIO_ASSERT(!ot.adjust_geometry("foo", w, h, x, y, "invalid") && x == -42
-                && y == -42 && w == -42 && h == -42);
+    OIIO_CHECK_ASSERT(!ot.adjust_geometry("foo", w, h, x, y, "invalid")
+                      && x == -42 && y == -42 && w == -42 && h == -42);
 }
 #endif
 
@@ -6127,8 +6163,11 @@ oiiotool_unit_tests()
 #ifdef OIIO_UNIT_TESTS
     using Strutil::print;
     print("Running unit tests...\n");
+    auto e       = ot.noerrexit;
+    ot.noerrexit = true;
     unit_test_scan_box();
     unit_test_adjust_geometry();
+    ot.noerrexit = e;
     print("...end of unit tests\n");
 #endif
 }
@@ -6178,6 +6217,8 @@ Oiiotool::getargs(int argc, char* argv[])
       .help("Quiet mode (turn verbose off)");
     ap.arg("-n", &ot.dryrun)
       .help("No saved output (dry run)");
+    ap.arg("--no-error-exit", ot.noerrexit)
+      .help("Do not exit upon error, try to process additional comands (danger!)");
     ap.arg("-a", &ot.allsubimages)
       .help("Do operations on all subimages/miplevels");
     ap.arg("--debug", &ot.debug)
diff --git a/src/oiiotool/oiiotool.h b/src/oiiotool/oiiotool.h
@@ -99,6 +99,7 @@ class Oiiotool {
     bool eval_enable;              // Enable evaluation of expressions
     bool skip_bad_frames = false;  // Just skip a bad frame, don't exit
     bool nostderr        = false;  // If true, use stdout for errors
+    bool noerrexit       = false;  // Don't exit on error
     std::string dumpdata_C_name;
     std::string full_command_line;
     std::string printinfo_metamatch;
diff --git a/testsuite/oiiotool-control/ref/out.txt b/testsuite/oiiotool-control/ref/out.txt
@@ -74,11 +74,23 @@ Testing if/else with false cond:
   inside else clause, i=0
   done
  
+Testing else without if:
+oiiotool ERROR: -else : else without matching if
+Full command line was:
+> oiiotool -colorconfig ../common/OpenColorIO/nuke-default/config.ocio -echo "Testing else without if:" -else -echo bad -endif -echo " "
+Testing endif without if:
+oiiotool ERROR: -endif : endif without matching if
+Full command line was:
+> oiiotool -colorconfig ../common/OpenColorIO/nuke-default/config.ocio -echo "Testing endif without if:" -endif -echo " "
 Testing while (expect output 0..2):
   i = 0
   i = 1
   i = 2
  
+Testing endwhile without while:
+oiiotool ERROR: -endwhile : endwhile without matching while
+Full command line was:
+> oiiotool -colorconfig ../common/OpenColorIO/nuke-default/config.ocio -echo "Testing endwhile without while:" -endwhile -echo " "
 Testing for i 5 (expect output 0..4):
   i = 0
   i = 1
@@ -98,6 +110,14 @@ Testing for i 5,10,2 (expect output 5,7,9):
   i = 7
   i = 9
  
+Testing endfor without for:
+oiiotool ERROR: -endfor : endfor without matching for
+Full command line was:
+> oiiotool -colorconfig ../common/OpenColorIO/nuke-default/config.ocio -echo "Testing endfor without for:" -endfor -echo " "
+Testing for i 5,10,2,8 (bad range):
+oiiotool ERROR: --for : Invalid range "5,10,2,8"
+Full command line was:
+> oiiotool -colorconfig ../common/OpenColorIO/nuke-default/config.ocio -echo "Testing for i 5,10,2,8 (bad range):" --for i 5,10,2,8 --echo "  i = {i}" --endfor -echo " "
 copyA.0001.jpg       :  128 x   96, 3 channel, uint8 jpeg
 copyA.0002.jpg       :  128 x   96, 3 channel, uint8 jpeg
 copyA.0003.jpg       :  128 x   96, 3 channel, uint8 jpeg
diff --git a/testsuite/oiiotool-control/run.py b/testsuite/oiiotool-control/run.py
@@ -6,7 +6,8 @@
 # command += oiiotool ("--pattern constant:color=0.5,0.5,0.5 64x64 3 -d half -o grey64.exr")
 # command += oiiotool ("--create 256x256 3 --fill:color=1,.5,.5 256x256 --fill:color=0,1,0 80x80+100+100 -d uint8 -o filled.tif")
 # wrapper_cmd = "time"
-# redirect = "2>&1"
+redirect += " 2>&1"
+failureok = True
 
 # test expression substitution
 command += oiiotool ('-echo "42+2 = {42+2}" ' +
@@ -78,6 +79,7 @@
 command += oiiotool ('-echo "Testing --set of various explicit types:" ' +
                      '-set:type=int i 42 -set:type=float f 3.5 ' +
                      '-set:type=string s "hello world" ' +
+                     '-set:type="string[3]" sarr "hello","world","wide" ' +
                      '-set:type=timecode tc 01:02:03:04 ' +
                      '-set:type=rational rat 1/2 ' +
                      '-echo "  i = {i}, f = {f}, s = {s}, tc = {tc}, rat = {rat}"')
@@ -90,14 +92,19 @@
 command += oiiotool ('-echo "Testing if with false cond (expect NO output):" -set i 0 -if "{i}" -echo "  inside if clause, i={i}" -endif -echo "  done" -echo " "')
 command += oiiotool ('-echo "Testing if/else with true cond:" -set i 42 -if "{i}" -echo "  inside if clause, i={i}" -else -echo "  inside else clause, i={i}" -endif -echo "  done" -echo " "')
 command += oiiotool ('-echo "Testing if/else with false cond:" -set i 0 -if "{i}" -echo "  inside if clause, i={i}" -else -echo "  inside else clause, i={i}" -endif -echo "  done" -echo " "')
+command += oiiotool ('-echo "Testing else without if:" -else -echo "bad" -endif -echo " "')
+command += oiiotool ('-echo "Testing endif without if:" -endif -echo " "')
 
 # Test --while --endwhile
 command += oiiotool ('-echo "Testing while (expect output 0..2):" -set i 0 --while "{i < 3}" --echo "  i = {i}" --set i "{i+1}" --endwhile -echo " "')
+command += oiiotool ('-echo "Testing endwhile without while:" -endwhile -echo " "')
 
 # Test --for --endfor
 command += oiiotool ('-echo "Testing for i 5 (expect output 0..4):" --for i 5 --echo "  i = {i}" --endfor -echo " "')
 command += oiiotool ('-echo "Testing for i 5,10 (expect output 5..9):" --for i 5,10 --echo "  i = {i}" --endfor -echo " "')
 command += oiiotool ('-echo "Testing for i 5,10,2 (expect output 5,7,9):" --for i 5,10,2 --echo "  i = {i}" --endfor -echo " "')
+command += oiiotool ('-echo "Testing endfor without for:" -endfor -echo " "')
+command += oiiotool ('-echo "Testing for i 5,10,2,8 (bad range):" --for i 5,10,2,8 --echo "  i = {i}" --endfor -echo " "')
 
 # test sequences
 command += oiiotool ("../common/tahoe-tiny.tif -o copyA.1-10#.jpg")

Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,9 @@ class UnitTestFailureCounter {`
`44`	`44`	`std::cout << Sysutil::Term(std::cout).ansi("red", "ERRORS!\n");`
`45`	`45`	`std::exit(m_failures != 0);`
`46`	`46`	`} else {`
	`47`	`+#ifndef OIIO_UNIT_TEST_QUIET_SUCCESS`
`47`	`48`	`std::cout << Sysutil::Term(std::cout).ansi("green", "OK\n");`
	`49`	`+#endif`
`48`	`50`	`}`
`49`	`51`	`}`
`50`	`52`	`const UnitTestFailureCounter& operator++() noexcept // prefix`