Fixed 'set -e' errors.

2 files changed, 125 insertions, 129 deletions

diff --git a/shflags b/shflags
index da09932..35fb137 100644
--- a/shflags
+++ b/shflags
@@ -357,9 +357,7 @@ _flags_define() {
   fi
 
   # Check for existing short name definition.
-  if [ ${_flags_return_} -eq ${FLAGS_TRUE} \
-      -a "${_flags_short_}" != "${__FLAGS_NULL}" ]
-  then
+  if [ ${_flags_return_} -eq ${FLAGS_TRUE} -a "${_flags_short_}" != "${__FLAGS_NULL}" ]; then
     if _flags_itemInList "${_flags_short_}" "${__flags_shortNames}"; then
       flags_error="flag short name (${_flags_short_}) already defined"
       _flags_warn "${flags_error}"
diff --git a/shflags_defines_test.sh b/shflags_defines_test.sh
index f2e1d71..2c2ad81 100755
--- a/shflags_defines_test.sh
+++ b/shflags_defines_test.sh
@@ -3,7 +3,7 @@
 #
 # shFlags unit test for the flag definition methods
 #
-# Copyright 2008-2017 Kate Ward. All Rights Reserved.
+# Copyright 2008-2020 Kate Ward. All Rights Reserved.
 # Released under the Apache 2.0 license.
 #
 # Author: kate.ward@forestent.com (Kate Ward)
@@ -14,7 +14,10 @@
 #   shellcheck disable=SC1090,SC1091
 
 # Exit immediately if a simple command exits with a non-zero status.
-#set -e
+set -e
+
+# Treat unset variables as an error when performing parameter expansion.
+set -u
 
 # These variables will be overridden by the test helpers.
 stdoutF="${TMPDIR:-/tmp}/STDOUT"
@@ -25,172 +28,165 @@ stderrF="${TMPDIR:-/tmp}/STDERR"
 
 testFlagsDefine() {
   # No arguments.
-  _flags_define >"${stdoutF}" 2>"${stderrF}"
-  assertFalse '_flags_define() with no arguments should have failed.' $?
+  if _flags_define >"${stdoutF}" 2>"${stderrF}"
+  then :; else
+    assertEquals '_flags_define() with no arguments should error' ${FLAGS_ERROR} $?
+  fi
   assertErrorMsg '' 'no arguments'
 
   # One argument.
-  _flags_define arg1 >"${stdoutF}" 2>"${stderrF}"
-  assertFalse '_flags_define() call with one argument should fail' $?
+  if _flags_define arg1 >"${stdoutF}" 2>"${stderrF}"
+  then :; else
+    assertEquals '_flags_define() call with one argument should error' ${FLAGS_ERROR} $?
+  fi
   assertErrorMsg '' 'one argument'
 
   # Two arguments.
-  _flags_define arg1 arg2 >"${stdoutF}" 2>"${stderrF}"
-  assertFalse '_flags_define() call with two arguments should fail' $?
+  if _flags_define arg1 arg2 >"${stdoutF}" 2>"${stderrF}"
+  then :; else
+    assertEquals '_flags_define() call with two arguments should error' ${FLAGS_ERROR} $?
+  fi
   assertErrorMsg '' 'two arguments'
 
   # Three arguments.
-  _flags_define arg1 arg2 arg3 >"${stdoutF}" 2>"${stderrF}"
-  assertFalse '_flags_define() call with three arguments should fail' $?
+  if _flags_define arg1 arg2 arg3 >"${stdoutF}" 2>"${stderrF}"
+  then :; else
+    assertEquals '_flags_define() call with three arguments should error' ${FLAGS_ERROR} $?
+  fi
   assertErrorMsg '' 'three arguments'
 
   # Multiple definition. Assumes working boolean definition (tested elsewhere).
-  _flags_define "${__FLAGS_TYPE_BOOLEAN}" multiDefBool true 'multi def #1' m
-  _flags_define "${__FLAGS_TYPE_BOOLEAN}" multiDefBool false 'multi def #2' m \
-      >"${stdoutF}" 2>"${stderrF}"
-  assertFalse '_flags_define() with existing flag name should fail' $?
-  assertTrue \
-      '_flags_define() should not overwrite previously defined default.' \
-      "${FLAGS_multiDefBool:-}"
+  if ! _flags_define "${__FLAGS_TYPE_BOOLEAN}" multiDefBool true 'multi def #1' m; then
+    fail "didn't expect _flags_define for 'multi def #1' to fail"
+  fi
+  if _flags_define "${__FLAGS_TYPE_BOOLEAN}" multiDefBool false 'multi def #2' m >"${stdoutF}" 2>"${stderrF}"
+  then :; else
+    assertEquals '_flags_define() with existing flag name should fail' ${FLAGS_FALSE} $?
+  fi
+  assertTrue '_flags_define() should not overwrite previously defined default.' "${FLAGS_multiDefBool:-}"
   assertWarnMsg '' 'existing flag'
 
   # Duplicate dashed and underscored definition.
-  _flags_define "${__FLAGS_TYPE_STRING}" long-name 'foo' 'dashed name' l
-  _flags_define "${__FLAGS_TYPE_STRING}" long_name 'bar' 'underscored name' l \
-      >"${stdoutF}" 2>"${stderrF}"
-  assertFalse '_flags_define() with existing flag name should fail' $?
+  if ! _flags_define "${__FLAGS_TYPE_STRING}" long-name 'foo' 'dashed name' l; then
+    fail "didn't expect _flags_define() for 'dashed name' to fail"
+  fi
+  if _flags_define "${__FLAGS_TYPE_STRING}" long_name 'bar' 'underscored name' l >"${stdoutF}" 2>"${stderrF}"
+  then :; else
+    assertEquals '_flags_define() with duplicate dashed and underscored definition should fail' ${FLAGS_FALSE} $?
+  fi
   # shellcheck disable=SC2154
-  assertEquals \
-      '_flags_define() should not overwrite previously defined default.' \
-      "${FLAGS_long_name}" 'foo'
+  assertEquals '_flags_define() should not overwrite previously defined default.' "${FLAGS_long_name}" 'foo'
   assertWarnMsg '' 'already exists'
 
   # TODO(kward): test requirement of enhanced getopt.
 
   # Invalid type.
-  _flags_define invalid arg2 arg3 arg4 i >"${stdoutF}" 2>"${stderrF}"
-  assertFalse '_flags_define() with "invalid" type should have failed.' $?
+  if _flags_define invalid arg2 arg3 arg4 i >"${stdoutF}" 2>"${stderrF}"
+  then :; else
+    assertEquals '_flags_define() with "invalid" type should have failed.' ${FLAGS_ERROR} $?
+  fi
   assertErrorMsg 'unrecognized flag type' 'invalid type'
 }
 
 testBoolean() {
-  # Test true defaults.
-  for default in 'true' 't' 0; do
+  while read -r desc ok default want; do
     flags_reset
-    DEFINE_boolean boolVal "${default}" 'my boolean' b
-    rtrn=$?
-    assertTrue \
-        "DEFINE_boolean() call with default of '${default}' failed." \
-        "${FLAGS_boolVal:-}"
-    assertTrue \
-        "DEFINE_boolean() call with default of '${default}' returned failure." \
-        ${rtrn}
-  done
 
-  # test false defaults
-  for default in 'false' 'f' 1; do
-    flags_reset
-    DEFINE_boolean boolVal "${default}" 'my boolean' b
-    rtrn=$?
-    assertFalse \
-        "DEFINE_boolean() call with default of '${default}' failed." \
-        "${FLAGS_boolVal:-}"
-    assertTrue \
-        "DEFINE_boolean() call with default of '${default}' returned failure." \
-        ${rtrn}
-  done
-
-  # Test invalid default.
-  flags_reset
-  DEFINE_boolean boolVal 'invalid' 'my boolean' b >"${stdoutF}" 2>"${stderrF}"
-  assertFalse 'DEFINE_boolean() call with invalid default did not fail.' $?
-  assertErrorMsg
+    if DEFINE_boolean boolVal "${default}" 'my boolean' b >"${stdoutF}" 2>"${stderrF}"
+    then
+      assertEquals "${desc}: incorrect FLAGS_boolVal value" "${FLAGS_boolVal:-}" "${want}"
+    else
+      got=$?
+      if [ "${ok}" -eq ${FLAGS_TRUE} ]; then
+        assertEquals "${desc}: DEFINE_boolean() failed unexpectedly" "${want}" "${got}"
+      else
+        assertEquals "${desc}: DEFINE_boolean() expected different return value" "${want}" "${got}"
+        assertErrorMsg
+      fi
+    fi
+  done <<EOF
+true_long   ${FLAGS_TRUE}  true    ${FLAGS_TRUE}
+true_short  ${FLAGS_TRUE}  t       ${FLAGS_TRUE}
+true_int    ${FLAGS_TRUE}  0       ${FLAGS_TRUE}
+false_long  ${FLAGS_TRUE}  false   ${FLAGS_FALSE}
+false_short ${FLAGS_TRUE}  f       ${FLAGS_FALSE}
+false_int   ${FLAGS_TRUE}  1       ${FLAGS_FALSE}
+invalid     ${FLAGS_FALSE} invalid ${FLAGS_ERROR}
+EOF
 }
 
 testFloat() {
-  # Test valid defaults.
+  # Valid defaults.
   for default in ${TH_FLOAT_VALID}; do
     flags_reset
-    DEFINE_float floatVal "${default}" "float: ${default}" f
-    rtrn=$?
-    assertSame "DEFINE_float() call with valid default failed." \
-        "${default}" "${FLAGS_floatVal:-}"
-    assertTrue \
-        "DEFINE_float() call with valid default of '${default}' returned failure." \
-        ${rtrn}
+    desc="valid_float_val='${default}'"
+    if DEFINE_float floatVal "${default}" 'valid float' f
+    then
+      got="${FLAGS_floatVal:-}" want="${default}"
+      assertEquals "${desc}: incorrect FLAGS_floatVal value" "${want}" "${got}"
+    else
+      assertEquals "${desc}: DEFINE_float() failed unexpectedly." ${FLAGS_TRUE} $?
+    fi
   done
 
-  # Test invalid defaults.
-  flags_reset
-  DEFINE_float floatVal 'invalid' 'invalid float: string' f \
-      >"${stdoutF}" 2>"${stderrF}"
-  assertFalse 'DEFINE_float() call with string value default did not fail.' $?
-  assertErrorMsg
+  # Invalid defaults.
+  for default in ${TH_FLOAT_INVALID}; do
+    flags_reset
+    desc="invalid_float_val='${default}'"
+    if DEFINE_float floatVal "${default}" 'invalid float' f >"${stdoutF}" 2>"${stderrF}"
+    then
+      fail "${desc}: expected DEFINE_float() to fail"
+    else
+      assertEquals "${desc}: DEFINE_float() expected error" ${FLAGS_ERROR} $?
+      assertErrorMsg
+    fi
+  done
 }
 
 testInteger() {
-  # Test valid defaults.
+  # Valid defaults.
   for default in ${TH_INT_VALID}; do
     flags_reset
-    DEFINE_integer intVal "${default}" "integer: ${default}" i
-    rtrn=$?
-    assertSame \
-        "DEFINE_integer() call with valid default failed." \
-        "${default}" "${FLAGS_intVal:-}"
-    assertTrue \
-        "DEFINE_integer() call with valid default of '${default}' returned failure." \
-        ${rtrn}
+    desc="valid_int_val='${default}'"
+    if DEFINE_integer intVal "${default}" 'valid integer' i
+    then
+      got="${FLAGS_intVal:-}" want="${default}"
+      assertEquals "${desc}: incorrect FLAGS_intVal value" "${want}" "${got}"
+    else
+      assertEquals "${desc}: DEFINE_integer() failed unexpectedly." ${FLAGS_TRUE} $?
+    fi
   done
 
-  # Test invalid defaults.
-  flags_reset
-  DEFINE_integer intVal 1.234 'invalid integer: float' i \
-      >"${stdoutF}" 2>"${stderrF}"
-  assertFalse 'DEFINE_integer() call with float value default did not fail.' $?
-  assertErrorMsg 'invalid default' 'float default'
-
-  DEFINE_integer intVal -1.234 'invalid integer: negative float' i \
-      >"${stdoutF}" 2>"${stderrF}"
-  assertFalse \
-      'DEFINE_integer() call with negative float value default did not fail.' \
-      $?
-  assertErrorMsg 'invalid default' 'negative float default'
-
-  DEFINE_integer intVal 'invalid' 'invalid integer: string' i \
-      >"${stdoutF}" 2>"${stderrF}"
-  assertFalse \
-      'DEFINE_integer() call with string value default did not fail.' \
-      $?
-  assertErrorMsg 'invalid default' 'string default'
+  # Invalid defaults.
+  for default in ${TH_INT_INVALID}; do
+    flags_reset
+    desc="invalid_int_val='${default}'"
+    if DEFINE_integer intVal "${default}" 'invalid integer' i >"${stdoutF}" 2>"${stderrF}"
+    then
+      fail "${desc}: expected DEFINE_integer() to fail"
+    else
+      assertEquals "${desc}: DEFINE_integer() expected error." ${FLAGS_ERROR} $?
+      assertErrorMsg
+    fi
+  done
 }
 
-testString()
-{
-  # test valid defaults
-  for default in \
-      ${TH_BOOL_VALID} \
-      ${TH_FLOAT_VALID} \
-      ${TH_INT_VALID} \
-      'also valid'
+testString() {
+  # Valid defaults.
+  for default in ${TH_BOOL_VALID} ${TH_FLOAT_VALID} ${TH_INT_VALID} 'also valid' ''
   do
     flags_reset
-    DEFINE_string strVal "${default}" "string: ${default}" s
-    rtrn=$?
-    assertSame \
-        "DEFINE_string() call with valid default failed." \
-        "${default}" "${FLAGS_strVal:-}"
-    assertTrue \
-        "DEFINE_string() call with valid default of '${default}' returned failure." \
-        ${rtrn}
+    desc="valid_string_val='${default}'"
+    if DEFINE_string strVal "${default}" "string: ${default}" s
+    then
+      got="${FLAGS_strVal:-}" want="${default}"
+      assertEquals "${desc}: incorrect FLAGS_strVal value" "${want}" "${got}"
+    else
+      assertEquals "${desc}: DEFINE_string() failed unexpectedly." ${FLAGS_TRUE} $?
+    fi
   done
 
-  # test "empty" strings
-  flags_reset
-  DEFINE_string str '' "string: empty single quotes" s
-  rtrn=$?
-  assertSame \
-      "DEFINE_string() call with valid default failed." \
-      '' "${FLAGS_str:-}"
+  # There are no known invalid defaults.
 }
 
 testShortNameLength() {
@@ -199,11 +195,13 @@ testShortNameLength() {
 }
 
 testFlagNameIsReserved() {
-  ( DEFINE_string TRUE '' 'true is a reserved flag name' t \
-      >"${stdoutF}" 2>"${stderrF}" )
-  rtrn=$?
-  assertEquals "${FLAGS_ERROR}" "${rtrn}"
-  assertErrorMsg 'flag name (TRUE) is reserved'
+  if DEFINE_string TRUE '' 'true is a reserved flag name' t >"${stdoutF}" 2>"${stderrF}"
+  then
+    fail "expected DEFINE with reserved flag name to fail"
+  else
+    assertEquals "expected error from DEFINE with reserved flag" ${FLAGS_ERROR} $?
+    assertErrorMsg 'flag name (TRUE) is reserved'
+  fi
 }
 
 oneTimeSetUp() {