[PATCH v2] test: add "set -u" to test-lib.sh

Tomi Ollila tomi.ollila at iki.fi
Tue Jun 7 14:59:37 PDT 2016


This prohibits unset variables to expand to empty strings.
Without this e.g misspelled variables cause unintentional results.

Now all the test variables are either initialized, or in case of
optional arguments and user-provided environment variables (i.e.
when it is not know whethet the variable is set) the form
${variable-} is used.

Two unusable lines (leftovers?) in T360-symbol-hiding.sh were removed;
this was the only non-lib test code that needed to be edited for this
change to work.
---

This is v2 of id:1441307395-2218-1-git-send-email-tomi.ollila at iki.fi

(basically added ensuring that $TERM is set)

because:

id:1465043356-23420-2-git-send-email-david at tethera.net
id:1465043356-23420-4-git-send-email-david at tethera.net

(in *-4-* if [ "${NOTMUCH_HAVE_XAPIAN_DB_RETRY_LOCK}" = "0" ]
always returns false as $NOTMUCH_HAVE_XAPIAN_DB_RETRY_LOCK
is not defined, -> string comparison "" = "0" is false)

I created this on top of current master (6a833a6e83) plus

  id:1464212261-26892-1-git-send-email-tomi.ollila at iki.fi
  id:1464212261-26892-2-git-send-email-tomi.ollila at iki.fi
  id:1464439170-25978-1-git-send-email-tomi.ollila at iki.fi
  id:1464439170-25978-2-git-send-email-tomi.ollila at iki.fi
  id:1464439170-25978-3-git-send-email-tomi.ollila at iki.fi

(so i could test this on 2 systems) -- but this applies on
top of 6a833a6e83 (current master) too.

 test/T360-symbol-hiding.sh |  3 ---
 test/test-lib.sh           | 63 +++++++++++++++++++++++++++-------------------
 2 files changed, 37 insertions(+), 29 deletions(-)

diff --git a/test/T360-symbol-hiding.sh b/test/T360-symbol-hiding.sh
index b3dbb1b..3fec3b7 100755
--- a/test/T360-symbol-hiding.sh
+++ b/test/T360-symbol-hiding.sh
@@ -22,9 +22,6 @@ caught No backend database found at path 'CWD/nonexistent'
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
-test_begin_subtest 'checking output'
-test_expect_equal "$result" "$output"
-
 test_begin_subtest 'comparing existing to exported symbols'
 nm -P $TEST_DIRECTORY/../lib/libnotmuch.so | awk '$2 == "T" && $1 ~ "^notmuch" {print $1}' | sort | uniq > ACTUAL
 sed -n 's/[[:blank:]]*\(notmuch_[^;]*\);/\1/p' $TEST_DIRECTORY/../notmuch.sym | sort | uniq > EXPORTED
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 54c65b6..6cde453 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -23,6 +23,9 @@ if [ ${BASH_VERSINFO[0]} -lt 4 ]; then
     exit 1
 fi
 
+# Exit immediately when attempting to expand unset variable.
+set -u
+
 # Make sure echo builtin does not expand backslash-escape sequences by default.
 shopt -u xpg_echo
 
@@ -32,7 +35,7 @@ this_test_bare=${this_test#T[0-9][0-9][0-9]-}
 
 # if --tee was passed, write the output not only to the terminal, but
 # additionally to the file test-results/$BASENAME.out, too.
-case "$GIT_TEST_TEE_STARTED, $* " in
+case "${GIT_TEST_TEE_STARTED-}, $* " in
 done,*)
 	# do not redirect again
 	;;
@@ -53,7 +56,7 @@ BASH_XTRACEFD=7
 export PS4='+(${BASH_SOURCE}:${LINENO}): ${FUNCNAME[0]:+${FUNCNAME[0]}(): }'
 
 # Keep the original TERM for say_color and test_emacs
-ORIGINAL_TERM=$TERM
+ORIGINAL_TERM=${TERM:-dumb}
 
 # dtach(1) provides more capable terminal environment to anything
 # that requires more than dumb terminal...
@@ -67,8 +70,8 @@ TZ=UTC
 TERM=dumb
 export LANG LC_ALL PAGER TERM TZ
 GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
-if [[ ( -n "$TEST_EMACS" && -z "$TEST_EMACSCLIENT" ) || \
-      ( -z "$TEST_EMACS" && -n "$TEST_EMACSCLIENT" ) ]]; then
+if [[ ( -n "${TEST_EMACS-}" && -z "${TEST_EMACSCLIENT-}" ) || \
+      ( -z "${TEST_EMACS-}" && -n "${TEST_EMACSCLIENT-}" ) ]]; then
     echo "error: must specify both or neither of TEST_EMACS and TEST_EMACSCLIENT" >&2
     exit 1
 fi
@@ -110,7 +113,10 @@ _x32="$_x04$_x04$_x04$_x04$_x04$_x04$_x04$_x04"
 		tput setaf 1 >/dev/null 2>&1 &&
 		tput sgr0 >/dev/null 2>&1
 	) &&
-	color=t
+	color=t || color=
+
+debug= immediate= help= verbose= quiet=
+with_dashes= valgrind= verbose= root=
 
 while test "$#" -ne 0
 do
@@ -212,7 +218,7 @@ print_test_description ()
 	echo $this_test: "Testing ${test_description}"
 	test_description_printed=1
 }
-if [ -z "$NOTMUCH_TEST_QUIET" ]
+if [ -z "${NOTMUCH_TEST_QUIET=}" ]
 then
 	print_test_description
 fi
@@ -345,6 +351,7 @@ remove_cr () {
 gen_msg_cnt=0
 gen_msg_filename=""
 gen_msg_id=""
+test_subtest_name=
 generate_message ()
 {
     # This is our (bash-specific) magic for doing named parameters
@@ -352,48 +359,48 @@ generate_message ()
     local additional_headers
 
     gen_msg_cnt=$((gen_msg_cnt + 1))
-    if [ -z "${template[filename]}" ]; then
+    if [ -z "${template[filename]-}" ]; then
 	gen_msg_name="msg-$(printf "%03d" $gen_msg_cnt)"
     else
 	gen_msg_name=${template[filename]}
     fi
 
-    if [ -z "${template[id]}" ]; then
+    if [ -z "${template[id]-}" ]; then
 	gen_msg_id="${gen_msg_name%:2,*}@notmuch-test-suite"
     else
 	gen_msg_id="${template[id]}"
     fi
 
-    if [ -z "${template[dir]}" ]; then
+    if [ -z "${template[dir]-}" ]; then
 	gen_msg_filename="${MAIL_DIR}/$gen_msg_name"
     else
 	gen_msg_filename="${MAIL_DIR}/${template[dir]}/$gen_msg_name"
 	mkdir -p "$(dirname "$gen_msg_filename")"
     fi
 
-    if [ -z "${template[body]}" ]; then
+    if [ -z "${template[body]-}" ]; then
 	template[body]="This is just a test message (#${gen_msg_cnt})"
     fi
 
-    if [ -z "${template[from]}" ]; then
+    if [ -z "${template[from]-}" ]; then
 	template[from]="Notmuch Test Suite <test_suite at notmuchmail.org>"
     fi
 
-    if [ -z "${template[to]}" ]; then
+    if [ -z "${template[to]-}" ]; then
 	template[to]="Notmuch Test Suite <test_suite at notmuchmail.org>"
     fi
 
-    if [ -z "${template[subject]}" ]; then
+    if [ -z "${template[subject]-}" ]; then
 	if [ -n "$test_subtest_name" ]; then
 	    template[subject]="$test_subtest_name"
 	else
 	    template[subject]="Test message #${gen_msg_cnt}"
 	fi
-    elif [ "${template[subject]}" = "@FORCE_EMPTY" ]; then
+    elif [ "${template[subject]-}" = "@FORCE_EMPTY" ]; then
 	template[subject]=""
     fi
 
-    if [ -z "${template[date]}" ]; then
+    if [ -z "${template[date]-}" ]; then
 	# we use decreasing timestamps here for historical reasons;
 	# the existing test suite when we converted to unique timestamps just
 	# happened to have signicantly fewer failures with that choice.
@@ -406,42 +413,42 @@ generate_message ()
     fi
 
     additional_headers=""
-    if [ ! -z "${template[header]}" ]; then
+    if [ ! -z "${template[header]-}" ]; then
 	additional_headers="${template[header]}
 ${additional_headers}"
     fi
 
-    if [ ! -z "${template[reply-to]}" ]; then
+    if [ ! -z "${template[reply-to]-}" ]; then
 	additional_headers="Reply-To: ${template[reply-to]}
 ${additional_headers}"
     fi
 
-    if [ ! -z "${template[in-reply-to]}" ]; then
+    if [ ! -z "${template[in-reply-to]-}" ]; then
 	additional_headers="In-Reply-To: ${template[in-reply-to]}
 ${additional_headers}"
     fi
 
-    if [ ! -z "${template[cc]}" ]; then
+    if [ ! -z "${template[cc]-}" ]; then
 	additional_headers="Cc: ${template[cc]}
 ${additional_headers}"
     fi
 
-    if [ ! -z "${template[bcc]}" ]; then
+    if [ ! -z "${template[bcc]-}" ]; then
 	additional_headers="Bcc: ${template[bcc]}
 ${additional_headers}"
     fi
 
-    if [ ! -z "${template[references]}" ]; then
+    if [ ! -z "${template[references]-}" ]; then
 	additional_headers="References: ${template[references]}
 ${additional_headers}"
     fi
 
-    if [ ! -z "${template[content-type]}" ]; then
+    if [ ! -z "${template[content-type]-}" ]; then
 	additional_headers="Content-Type: ${template[content-type]}
 ${additional_headers}"
     fi
 
-    if [ ! -z "${template[content-transfer-encoding]}" ]; then
+    if [ ! -z "${template[content-transfer-encoding]-}" ]; then
 	additional_headers="Content-Transfer-Encoding: ${template[content-transfer-encoding]}
 ${additional_headers}"
     fi
@@ -559,6 +566,7 @@ add_email_corpus ()
     fi
 }
 
+inside_subtest=
 test_begin_subtest ()
 {
     if [ -n "$inside_subtest" ]; then
@@ -811,7 +819,7 @@ $binary () {
 # Returns success if dependency is available, failure otherwise.
 test_require_external_prereq () {
 	binary="$1"
-	if [[ ${test_missing_external_prereq_["${binary}"]} == t ]]; then
+	if [[ ${test_missing_external_prereq_["${binary}"]-} == t ]]; then
 		# dependency is missing, call the replacement function to note it
 		eval "$binary"
 	else
@@ -882,6 +890,7 @@ test_run_ () {
 	return 0
 }
 
+: ${NOTMUCH_SKIP_TESTS=}
 test_skip () {
 	test_count=$(($test_count+1))
 	to_skip=
@@ -1146,6 +1155,7 @@ EOF
 	chmod a+x "$TMP_DIRECTORY/run_emacs"
 }
 
+EMACS_SERVER=
 test_emacs () {
 	# test dependencies beforehand to avoid the waiting loop below
 	missing_dependencies=
@@ -1246,6 +1256,7 @@ notmuch_counter_value () {
 	echo $count
 }
 
+test_init_done_=
 test_reset_state_ () {
 	test -z "$test_init_done_" && test_init_
 
@@ -1345,8 +1356,8 @@ case $(uname -s) in
 	;;
 esac
 
-test -z "$NO_PERL" && test_set_prereq PERL
-test -z "$NO_PYTHON" && test_set_prereq PYTHON
+test -z "${NO_PERL-}" && test_set_prereq PERL
+test -z "${NO_PYTHON-}" && test_set_prereq PYTHON
 
 # test whether the filesystem supports symbolic links
 ln -s x y 2>/dev/null && test -h y 2>/dev/null && test_set_prereq SYMLINKS
-- 
2.8.2



More information about the notmuch mailing list