[PATCH v2 00/10] test: (hopefully) better test prerequisites

[PATCH v2 00/10] test: (hopefully) better test prerequisites

[Dmitry Kurochkin]

New version of [1] patch series.  Changes:

v2 since v1:

* Add test_require_external_prereq function to explicitly check for
  external dependencies, use it in test_emacs.

* Indenting fixes.

* Use $binary instead of $1 in test_declare_external_prereq.

Regards,
  Dmitry

[PATCH v2 01/10] test: move subtest variables reset into a dedicated function

[Dmitry Kurochkin]

Currently, there is only one such variable test_subtest_known_broken_.
But more will be added in the future.
---
 test/test-lib.sh |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 1ea7fa9..222b5e4 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -407,41 +407,41 @@ emacs_deliver_message ()
 # number of messages.
 add_email_corpus ()
 {
     rm -rf ${MAIL_DIR}
     if [ -d $TEST_DIRECTORY/corpus.mail ]; then
 	cp -a $TEST_DIRECTORY/corpus.mail ${MAIL_DIR}
     else
 	cp -a $TEST_DIRECTORY/corpus ${MAIL_DIR}
 	notmuch new >/dev/null
 	cp -a ${MAIL_DIR} $TEST_DIRECTORY/corpus.mail
     fi
 }
 
 test_begin_subtest ()
 {
     if [ -n "$inside_subtest" ]; then
 	exec 1>&6 2>&7		# Restore stdout and stderr
 	error "bug in test script: Missing test_expect_equal in ${BASH_SOURCE[1]}:${BASH_LINENO[0]}"
     fi
     test_subtest_name="$1"
-    test_subtest_known_broken_=
+    test_reset_state_
     # Remember stdout and stderr file descriptors and redirect test
     # output to the previously prepared file descriptors 3 and 4 (see
     # below)
     if test "$verbose" != "t"; then exec 4>test.output 3>&4; fi
     exec 6>&1 7>&2 >&3 2>&4
     inside_subtest=t
 }
 
 # Pass test if two arguments match
 #
 # Note: Unlike all other test_expect_* functions, this function does
 # not accept a test name. Instead, the caller should call
 # test_begin_subtest before calling this function in order to set the
 # name.
 test_expect_equal ()
 {
 	exec 1>&6 2>&7		# Restore stdout and stderr
 	inside_subtest=
 	test "$#" = 3 && { prereq=$1; shift; } || prereq=
 	test "$#" = 2 ||
@@ -559,84 +559,84 @@ test_ok_ () {
 test_failure_ () {
 	if test "$test_subtest_known_broken_" = "t"; then
 		test_known_broken_failure_ "$@"
 		return
 	fi
 	test_failure=$(($test_failure + 1))
 	test_failure_message_ "FAIL" "$@"
 	test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
 	return 1
 }
 
 test_failure_message_ () {
 	say_color error "%-6s" "$1"
 	echo " $2"
 	shift 2
 	echo "$@" | sed -e 's/^/	/'
 	if test "$verbose" != "t"; then cat test.output; fi
 }
 
 test_known_broken_ok_ () {
-	test_subtest_known_broken_=
+	test_reset_state_
 	test_fixed=$(($test_fixed+1))
 	say_color pass "%-6s" "FIXED"
 	echo " $@"
 }
 
 test_known_broken_failure_ () {
-	test_subtest_known_broken_=
+	test_reset_state_
 	test_broken=$(($test_broken+1))
 	test_failure_message_ "BROKEN" "$@"
 	return 1
 }
 
 test_debug () {
 	test "$debug" = "" || eval "$1"
 }
 
 test_run_ () {
 	test_cleanup=:
 	if test "$verbose" != "t"; then exec 4>test.output 3>&4; fi
 	eval >&3 2>&4 "$1"
 	eval_ret=$?
 	eval >&3 2>&4 "$test_cleanup"
 	return 0
 }
 
 test_skip () {
 	test_count=$(($test_count+1))
 	to_skip=
 	for skp in $NOTMUCH_SKIP_TESTS
 	do
 		case $this_test.$test_count in
 		$skp)
 			to_skip=t
 		esac
 	done
 	if test -z "$to_skip" && test -n "$prereq" &&
 	   ! test_have_prereq "$prereq"
 	then
 		to_skip=t
 	fi
 	case "$to_skip" in
 	t)
-		test_subtest_known_broken_=
+		test_reset_state_
 		say_color skip >&3 "skipping test: $@"
 		say_color skip "%-6s" "SKIP"
 		echo " $1"
 		: true
 		;;
 	*)
 		false
 		;;
 	esac
 }
 
 test_subtest_known_broken () {
 	test_subtest_known_broken_=t
 }
 
 test_expect_success () {
 	test "$#" = 3 && { prereq=$1; shift; } || prereq=
 	test "$#" = 2 ||
 	error "bug in the test script: not 2 or 3 parameters to test-expect-success"
 	if ! test_skip "$@"
@@ -842,40 +842,44 @@ test_emacs () {
 	if [ -z "$EMACS_SERVER" ]; then
 		EMACS_SERVER="notmuch-test-suite-$$"
 		# start a detached session with an emacs server
 		# user's TERM is given to dtach which assumes a minimally
 		# VT100-compatible terminal -- and emacs inherits that
 		TERM=$ORIGINAL_TERM dtach -n "$TMP_DIRECTORY/emacs-dtach-socket.$$" \
 			sh -c "stty rows 24 cols 80; exec '$TMP_DIRECTORY/run_emacs' \
 				--no-window-system \
 				--eval '(setq server-name \"$EMACS_SERVER\")' \
 				--eval '(server-start)' \
 				--eval '(orphan-watchdog $$)'" || return
 		# wait until the emacs server is up
 		until test_emacs '()' 2>/dev/null; do
 			sleep 1
 		done
 	fi
 
 	emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
 }
 
+test_reset_state_ () {
+	test_subtest_known_broken_=
+}
+
 
 find_notmuch_path ()
 {
     dir="$1"
 
     while [ -n "$dir" ]; do
 	bin="$dir/notmuch"
 	if [ -x "$bin" ]; then
 	    echo "$dir"
 	    return
 	fi
 	dir="$(dirname "$dir")"
 	if [ "$dir" = "/" ]; then
 	    break
 	fi
     done
 }
 
 # Test the binaries we have just built.  The tests are kept in
 # test/ subdirectory and are run in 'trash directory' subdirectory.
-- 
1.7.7.2

[PATCH v2 02/10] test: set EMACS_SERVER variable only after dtach(1) was successfully started

[Dmitry Kurochkin]

Otherwise, we can set the EMACS_SERVER and return with an error.  And
subsequent calls to test_emacs would assume that emacs server is running.
---
 test/test-lib.sh |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 222b5e4..ff85848 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -823,50 +823,51 @@ export NOTMUCH_CONFIG=$NOTMUCH_CONFIG
 # Here's what we are using here:
 #
 # --no-init-file	Don't load users ~/.emacs
 #
 # --no-site-file	Don't load the site-wide startup stuff
 #
 # --directory		Ensure that the local elisp sources are found
 #
 # --load		Force loading of notmuch.el and test-lib.el
 
 exec emacs --no-init-file --no-site-file \
 	--directory "$TEST_DIRECTORY/../emacs" --load notmuch.el \
 	--directory "$TEST_DIRECTORY" --load test-lib.el \
 	"\$@"
 EOF
 	chmod a+x "$TMP_DIRECTORY/run_emacs"
 }
 
 test_emacs () {
 	if [ -z "$EMACS_SERVER" ]; then
-		EMACS_SERVER="notmuch-test-suite-$$"
+		server_name="notmuch-test-suite-$$"
 		# start a detached session with an emacs server
 		# user's TERM is given to dtach which assumes a minimally
 		# VT100-compatible terminal -- and emacs inherits that
 		TERM=$ORIGINAL_TERM dtach -n "$TMP_DIRECTORY/emacs-dtach-socket.$$" \
 			sh -c "stty rows 24 cols 80; exec '$TMP_DIRECTORY/run_emacs' \
 				--no-window-system \
-				--eval '(setq server-name \"$EMACS_SERVER\")' \
+				--eval '(setq server-name \"$server_name\")' \
 				--eval '(server-start)' \
 				--eval '(orphan-watchdog $$)'" || return
+		EMACS_SERVER="$server_name"
 		# wait until the emacs server is up
 		until test_emacs '()' 2>/dev/null; do
 			sleep 1
 		done
 	fi
 
 	emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
 }
 
 test_reset_state_ () {
 	test_subtest_known_broken_=
 }
 
 
 find_notmuch_path ()
 {
     dir="$1"
 
     while [ -n "$dir" ]; do
 	bin="$dir/notmuch"
-- 
1.7.7.2

[PATCH v2 03/10] test: add test state reset to test_expect_* functions that did not have it

[Dmitry Kurochkin]

---
 test/test-lib.sh |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index ff85848..f21e45e 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -622,82 +622,85 @@ test_skip () {
 		test_reset_state_
 		say_color skip >&3 "skipping test: $@"
 		say_color skip "%-6s" "SKIP"
 		echo " $1"
 		: true
 		;;
 	*)
 		false
 		;;
 	esac
 }
 
 test_subtest_known_broken () {
 	test_subtest_known_broken_=t
 }
 
 test_expect_success () {
 	test "$#" = 3 && { prereq=$1; shift; } || prereq=
 	test "$#" = 2 ||
 	error "bug in the test script: not 2 or 3 parameters to test-expect-success"
+	test_reset_state_
 	if ! test_skip "$@"
 	then
 		test_run_ "$2"
 		if [ "$?" = 0 -a "$eval_ret" = 0 ]
 		then
 			test_ok_ "$1"
 		else
 			test_failure_ "$@"
 		fi
 	fi
 }
 
 test_expect_code () {
 	test "$#" = 4 && { prereq=$1; shift; } || prereq=
 	test "$#" = 3 ||
 	error "bug in the test script: not 3 or 4 parameters to test-expect-code"
+	test_reset_state_
 	if ! test_skip "$@"
 	then
 		test_run_ "$3"
 		if [ "$?" = 0 -a "$eval_ret" = "$1" ]
 		then
 			test_ok_ "$2"
 		else
 			test_failure_ "$@"
 		fi
 	fi
 }
 
 # test_external runs external test scripts that provide continuous
 # test output about their progress, and succeeds/fails on
 # zero/non-zero exit code.  It outputs the test output on stdout even
 # in non-verbose mode, and announces the external script with "* run
 # <n>: ..." before running it.  When providing relative paths, keep in
 # mind that all scripts run in "trash directory".
 # Usage: test_external description command arguments...
 # Example: test_external 'Perl API' perl ../path/to/test.pl
 test_external () {
 	test "$#" = 4 && { prereq=$1; shift; } || prereq=
 	test "$#" = 3 ||
 	error >&5 "bug in the test script: not 3 or 4 parameters to test_external"
 	descr="$1"
 	shift
+	test_reset_state_
 	if ! test_skip "$descr" "$@"
 	then
 		# Announce the script to reduce confusion about the
 		# test output that follows.
 		say_color "" " run $test_count: $descr ($*)"
 		# Run command; redirect its stderr to &4 as in
 		# test_run_, but keep its stdout on our stdout even in
 		# non-verbose mode.
 		"$@" 2>&4
 		if [ "$?" = 0 ]
 		then
 			test_ok_ "$descr"
 		else
 			test_failure_ "$descr" "$@"
 		fi
 	fi
 }
 
 # Like test_external, but in addition tests that the command generated
 # no output on stderr.
-- 
1.7.7.2

[PATCH v2 04/10] test: add support for external executable dependencies

[Dmitry Kurochkin]

There is existing support for general prerequisites in the test suite.
But it is not very convenient to use: every test case has to keep
track for it's dependencies and they have to be explicitly listed.

The patch aims to add better support for a particular type of external
dependencies: external executables.  The main idea is to replace
missing external binaries with shell functions that have the same
name.  These functions always fail and keep track of missing
dependencies for a subtest.  The result reporting functions later can
check that an external binaries are missing and correctly report SKIP
result instead of FAIL.  The primary benefit is that the test cases do
not need to declare their dependencies or be changed in any way.
---
 test/test-lib.sh |   49 +++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index f21e45e..625d19b 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -526,40 +526,53 @@ notmuch_json_show_sanitize ()
 # - Implicitly by specifying the prerequisite tag in the calls to
 #   test_expect_{success,failure,code}.
 #
 # The single parameter is the prerequisite tag (a simple word, in all
 # capital letters by convention).
 
 test_set_prereq () {
 	satisfied="$satisfied$1 "
 }
 satisfied=" "
 
 test_have_prereq () {
 	case $satisfied in
 	*" $1 "*)
 		: yes, have it ;;
 	*)
 		! : nope ;;
 	esac
 }
 
+# declare prerequisite for the given external binary
+test_declare_external_prereq () {
+	binary="$1"
+	test "$#" = 2 && name=$2 || name="$binary(1)"
+
+	hash $binary 2>/dev/null || eval "
+$binary () {
+	echo -n \"\$test_subtest_missing_external_prereqs_\" | grep -e \" $name \" ||
+	test_subtest_missing_external_prereqs_=\"$test_subtest_missing_external_prereqs_ $name\"
+	false
+}"
+}
+
 # You are not expected to call test_ok_ and test_failure_ directly, use
 # the text_expect_* functions instead.
 
 test_ok_ () {
 	if test "$test_subtest_known_broken_" = "t"; then
 		test_known_broken_ok_ "$@"
 		return
 	fi
 	test_success=$(($test_success + 1))
 	say_color pass "%-6s" "PASS"
 	echo " $@"
 }
 
 test_failure_ () {
 	if test "$test_subtest_known_broken_" = "t"; then
 		test_known_broken_failure_ "$@"
 		return
 	fi
 	test_failure=$(($test_failure + 1))
 	test_failure_message_ "FAIL" "$@"
@@ -602,82 +615,101 @@ test_run_ () {
 	return 0
 }
 
 test_skip () {
 	test_count=$(($test_count+1))
 	to_skip=
 	for skp in $NOTMUCH_SKIP_TESTS
 	do
 		case $this_test.$test_count in
 		$skp)
 			to_skip=t
 		esac
 	done
 	if test -z "$to_skip" && test -n "$prereq" &&
 	   ! test_have_prereq "$prereq"
 	then
 		to_skip=t
 	fi
 	case "$to_skip" in
 	t)
-		test_reset_state_
-		say_color skip >&3 "skipping test: $@"
-		say_color skip "%-6s" "SKIP"
-		echo " $1"
-		: true
+		test_report_skip_ "$@"
 		;;
 	*)
-		false
+		test_check_missing_external_prereqs_ "$@"
 		;;
 	esac
 }
 
+test_check_missing_external_prereqs_ () {
+	if test -n "$test_subtest_missing_external_prereqs_"; then
+		say_color skip >&3 "missing prerequisites:"
+		echo "$test_subtest_missing_external_prereqs_" >&3
+		test_report_skip_ "$@"
+	else
+		false
+	fi
+}
+
+test_report_skip_ () {
+	test_reset_state_
+	say_color skip >&3 "skipping test: $@"
+	say_color skip "%-6s" "SKIP"
+	echo " $1"
+}
+
 test_subtest_known_broken () {
 	test_subtest_known_broken_=t
 }
 
 test_expect_success () {
 	test "$#" = 3 && { prereq=$1; shift; } || prereq=
 	test "$#" = 2 ||
 	error "bug in the test script: not 2 or 3 parameters to test-expect-success"
 	test_reset_state_
 	if ! test_skip "$@"
 	then
 		test_run_ "$2"
-		if [ "$?" = 0 -a "$eval_ret" = 0 ]
+		run_ret="$?"
+		# test_run_ may update missing external prerequisites
+		test_check_missing_external_prereqs_ "$@" ||
+		if [ "$run_ret" = 0 -a "$eval_ret" = 0 ]
 		then
 			test_ok_ "$1"
 		else
 			test_failure_ "$@"
 		fi
 	fi
 }
 
 test_expect_code () {
 	test "$#" = 4 && { prereq=$1; shift; } || prereq=
 	test "$#" = 3 ||
 	error "bug in the test script: not 3 or 4 parameters to test-expect-code"
 	test_reset_state_
 	if ! test_skip "$@"
 	then
 		test_run_ "$3"
-		if [ "$?" = 0 -a "$eval_ret" = "$1" ]
+		run_ret="$?"
+		# test_run_ may update missing external prerequisites,
+		test_check_missing_external_prereqs_ "$@" ||
+		if [ "$run_ret" = 0 -a "$eval_ret" = "$1" ]
 		then
 			test_ok_ "$2"
 		else
 			test_failure_ "$@"
 		fi
 	fi
 }
 
 # test_external runs external test scripts that provide continuous
 # test output about their progress, and succeeds/fails on
 # zero/non-zero exit code.  It outputs the test output on stdout even
 # in non-verbose mode, and announces the external script with "* run
 # <n>: ..." before running it.  When providing relative paths, keep in
 # mind that all scripts run in "trash directory".
 # Usage: test_external description command arguments...
 # Example: test_external 'Perl API' perl ../path/to/test.pl
 test_external () {
 	test "$#" = 4 && { prereq=$1; shift; } || prereq=
 	test "$#" = 3 ||
 	error >&5 "bug in the test script: not 3 or 4 parameters to test_external"
@@ -848,40 +880,41 @@ test_emacs () {
 		# user's TERM is given to dtach which assumes a minimally
 		# VT100-compatible terminal -- and emacs inherits that
 		TERM=$ORIGINAL_TERM dtach -n "$TMP_DIRECTORY/emacs-dtach-socket.$$" \
 			sh -c "stty rows 24 cols 80; exec '$TMP_DIRECTORY/run_emacs' \
 				--no-window-system \
 				--eval '(setq server-name \"$server_name\")' \
 				--eval '(server-start)' \
 				--eval '(orphan-watchdog $$)'" || return
 		EMACS_SERVER="$server_name"
 		# wait until the emacs server is up
 		until test_emacs '()' 2>/dev/null; do
 			sleep 1
 		done
 	fi
 
 	emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
 }
 
 test_reset_state_ () {
 	test_subtest_known_broken_=
+	test_subtest_missing_external_prereqs_=
 }
 
 
 find_notmuch_path ()
 {
     dir="$1"
 
     while [ -n "$dir" ]; do
 	bin="$dir/notmuch"
 	if [ -x "$bin" ]; then
 	    echo "$dir"
 	    return
 	fi
 	dir="$(dirname "$dir")"
 	if [ "$dir" = "/" ]; then
 	    break
 	fi
     done
 }
 
-- 
1.7.7.2

[PATCH v2 05/10] test: fix "skipping test" verbose output

[Dmitry Kurochkin]

---
 test/test-lib.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 625d19b..ce7576a 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -635,41 +635,42 @@ test_skip () {
 		test_report_skip_ "$@"
 		;;
 	*)
 		test_check_missing_external_prereqs_ "$@"
 		;;
 	esac
 }
 
 test_check_missing_external_prereqs_ () {
 	if test -n "$test_subtest_missing_external_prereqs_"; then
 		say_color skip >&3 "missing prerequisites:"
 		echo "$test_subtest_missing_external_prereqs_" >&3
 		test_report_skip_ "$@"
 	else
 		false
 	fi
 }
 
 test_report_skip_ () {
 	test_reset_state_
-	say_color skip >&3 "skipping test: $@"
+	say_color skip >&3 "skipping test:"
+	echo " $@" >&3
 	say_color skip "%-6s" "SKIP"
 	echo " $1"
 }
 
 test_subtest_known_broken () {
 	test_subtest_known_broken_=t
 }
 
 test_expect_success () {
 	test "$#" = 3 && { prereq=$1; shift; } || prereq=
 	test "$#" = 2 ||
 	error "bug in the test script: not 2 or 3 parameters to test-expect-success"
 	test_reset_state_
 	if ! test_skip "$@"
 	then
 		test_run_ "$2"
 		run_ret="$?"
 		# test_run_ may update missing external prerequisites
 		test_check_missing_external_prereqs_ "$@" ||
 		if [ "$run_ret" = 0 -a "$eval_ret" = 0 ]
-- 
1.7.7.2

[PATCH v2 06/10] test: skip all subtests if external dependencies are missing during init

[Dmitry Kurochkin]

Some tests (e.g. crypto) do a common initialization required for all
subtests.  The patch adds a check for missing external dependencies
during this initialization.  If any prerequisites are missing, all
subtests are skipped.

The check is run on the first call of test_reset_state_ function, so
no changes for the tests are needed.
---
 test/test-lib.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index ce7576a..4c73437 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -880,44 +880,54 @@ test_emacs () {
 		# start a detached session with an emacs server
 		# user's TERM is given to dtach which assumes a minimally
 		# VT100-compatible terminal -- and emacs inherits that
 		TERM=$ORIGINAL_TERM dtach -n "$TMP_DIRECTORY/emacs-dtach-socket.$$" \
 			sh -c "stty rows 24 cols 80; exec '$TMP_DIRECTORY/run_emacs' \
 				--no-window-system \
 				--eval '(setq server-name \"$server_name\")' \
 				--eval '(server-start)' \
 				--eval '(orphan-watchdog $$)'" || return
 		EMACS_SERVER="$server_name"
 		# wait until the emacs server is up
 		until test_emacs '()' 2>/dev/null; do
 			sleep 1
 		done
 	fi
 
 	emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
 }
 
 test_reset_state_ () {
+	test -z "$test_init_done_" && test_init_
+
 	test_subtest_known_broken_=
 	test_subtest_missing_external_prereqs_=
 }
 
+# called once before the first subtest
+test_init_ () {
+	test_init_done_=t
+
+	# skip all tests if there were external prerequisites missing during init
+	test_check_missing_external_prereqs_ "all tests in $this_test" && test_done
+}
+
 
 find_notmuch_path ()
 {
     dir="$1"
 
     while [ -n "$dir" ]; do
 	bin="$dir/notmuch"
 	if [ -x "$bin" ]; then
 	    echo "$dir"
 	    return
 	fi
 	dir="$(dirname "$dir")"
 	if [ "$dir" = "/" ]; then
 	    break
 	fi
     done
 }
 
 # Test the binaries we have just built.  The tests are kept in
 # test/ subdirectory and are run in 'trash directory' subdirectory.
-- 
1.7.7.2

[PATCH v2 07/10] test: declare external dependencies for the tests

[Dmitry Kurochkin]

That are: dtach(1), emacs(1), emacsclient(1), gdb(1) and gpg(1).
---
 test/test-lib.sh |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 4c73437..cf1b4f0 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -1101,20 +1101,27 @@ case $(uname -s) in
 	pwd () {
 		builtin pwd -W
 	}
 	# no POSIX permissions
 	# backslashes in pathspec are converted to '/'
 	# exec does not inherit the PID
 	;;
 *)
 	test_set_prereq POSIXPERM
 	test_set_prereq BSLASHPSPEC
 	test_set_prereq EXECKEEPSPID
 	;;
 esac
 
 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
 rm -f y
+
+# declare prerequisites for external binaries used in tests
+test_declare_external_prereq dtach
+test_declare_external_prereq emacs
+test_declare_external_prereq emacsclient
+test_declare_external_prereq gdb
+test_declare_external_prereq gpg
-- 
1.7.7.2

[PATCH v2 08/10] test: add function to explicitly check for external dependencies

[Dmitry Kurochkin]

Useful when binary is called indirectly (e.g. from emacs).
---
 test/test-lib.sh |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index cf1b4f0..5a99216 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -533,46 +533,60 @@ test_set_prereq () {
 	satisfied="$satisfied$1 "
 }
 satisfied=" "
 
 test_have_prereq () {
 	case $satisfied in
 	*" $1 "*)
 		: yes, have it ;;
 	*)
 		! : nope ;;
 	esac
 }
 
 # declare prerequisite for the given external binary
 test_declare_external_prereq () {
 	binary="$1"
 	test "$#" = 2 && name=$2 || name="$binary(1)"
 
 	hash $binary 2>/dev/null || eval "
 $binary () {
+	test_missing_external_prereq_${binary}_=t
 	echo -n \"\$test_subtest_missing_external_prereqs_\" | grep -e \" $name \" ||
 	test_subtest_missing_external_prereqs_=\"$test_subtest_missing_external_prereqs_ $name\"
 	false
 }"
 }
 
+# Explicitly require external prerequisite.  Useful when binary is
+# called indirectly (e.g. from emacs).
+# Returns success if dependency is available, failure otherwise.
+test_require_external_prereq () {
+	binary="$1"
+	if [ "$(eval echo -n \$test_missing_external_prereq_${binary}_)" = t ]; then
+		# dependency is missing, call the replacement function to note it
+		eval "$binary"
+	else
+		true
+	fi
+}
+
 # You are not expected to call test_ok_ and test_failure_ directly, use
 # the text_expect_* functions instead.
 
 test_ok_ () {
 	if test "$test_subtest_known_broken_" = "t"; then
 		test_known_broken_ok_ "$@"
 		return
 	fi
 	test_success=$(($test_success + 1))
 	say_color pass "%-6s" "PASS"
 	echo " $@"
 }
 
 test_failure_ () {
 	if test "$test_subtest_known_broken_" = "t"; then
 		test_known_broken_failure_ "$@"
 		return
 	fi
 	test_failure=$(($test_failure + 1))
 	test_failure_message_ "FAIL" "$@"
-- 
1.7.7.2

[PATCH v2 09/10] test: check if emacs is available in the beginning of test_emacs

[Dmitry Kurochkin]

Unfortunately, this is needed to avoid the emacs waiting loop.
---
 test/test-lib.sh |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 5a99216..21e3162 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -872,40 +872,44 @@ export NOTMUCH_CONFIG=$NOTMUCH_CONFIG
 
 # Here's what we are using here:
 #
 # --no-init-file	Don't load users ~/.emacs
 #
 # --no-site-file	Don't load the site-wide startup stuff
 #
 # --directory		Ensure that the local elisp sources are found
 #
 # --load		Force loading of notmuch.el and test-lib.el
 
 exec emacs --no-init-file --no-site-file \
 	--directory "$TEST_DIRECTORY/../emacs" --load notmuch.el \
 	--directory "$TEST_DIRECTORY" --load test-lib.el \
 	"\$@"
 EOF
 	chmod a+x "$TMP_DIRECTORY/run_emacs"
 }
 
 test_emacs () {
+	# test dependencies beforehand to avoid the waiting loop below
+	test_require_external_prereq emacs || return
+	test_require_external_prereq emacsclient || return
+
 	if [ -z "$EMACS_SERVER" ]; then
 		server_name="notmuch-test-suite-$$"
 		# start a detached session with an emacs server
 		# user's TERM is given to dtach which assumes a minimally
 		# VT100-compatible terminal -- and emacs inherits that
 		TERM=$ORIGINAL_TERM dtach -n "$TMP_DIRECTORY/emacs-dtach-socket.$$" \
 			sh -c "stty rows 24 cols 80; exec '$TMP_DIRECTORY/run_emacs' \
 				--no-window-system \
 				--eval '(setq server-name \"$server_name\")' \
 				--eval '(server-start)' \
 				--eval '(orphan-watchdog $$)'" || return
 		EMACS_SERVER="$server_name"
 		# wait until the emacs server is up
 		until test_emacs '()' 2>/dev/null; do
 			sleep 1
 		done
 	fi
 
 	emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
 }
-- 
1.7.7.2

[PATCH v2 10/10] test: fix "Stashing in notmuch-search" test when emacs is not available

[Dmitry Kurochkin]

If emacs is not available, test_expect_equal would be called with only
one argument.  The patch fixes this by quoting the (possibly empty)
$(cat OUTPUT) argument.
---
 test/emacs |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/test/emacs b/test/emacs
index 75a0a74..9883f40 100755
--- a/test/emacs
+++ b/test/emacs
@@ -364,41 +364,41 @@ test_emacs '(notmuch-show "id:\"bought\"")
         (switch-to-buffer
           (generate-new-buffer "*test-stashing*"))
         (dotimes (i 9)
           (yank)
           (insert "\n")
           (rotate-yank-pointer 1))
         (reverse-region (point-min) (point-max))
 	    (test-output)'
 sed -i -e 's/^.*tmp.emacs\/mail.*$/FILENAME/' OUTPUT
 test_expect_equal_file OUTPUT $EXPECTED/emacs-stashing
 
 test_begin_subtest "Stashing in notmuch-search"
 test_emacs '(notmuch-search "id:\"bought\"")
         (notmuch-test-wait)
         (notmuch-search-stash-thread-id)
         (switch-to-buffer
           (generate-new-buffer "*test-stashing*"))
         (yank)
 	    (test-output)'
 sed -i -e 's/^thread:.*$/thread:XXX/' OUTPUT
-test_expect_equal $(cat OUTPUT) "thread:XXX"
+test_expect_equal "$(cat OUTPUT)" "thread:XXX"
 
 test_begin_subtest 'Hiding message following HTML part'
 test_subtest_known_broken
 id='html-message@notmuchmail.org'
 emacs_deliver_message \
     'HTML message' \
     '' \
     "(message-goto-eoh)
      (insert \"Message-ID: <$id>\n\")
      (message-goto-body)
      (mml-insert-part \"text/html\")
      (insert \"<body>This is a test HTML message</body>\")"
 emacs_deliver_message \
     'Reply to HTML message' \
     'This is a reply to the test HTML message' \
     "(message-goto-eoh)
      (insert \"In-Reply-To: <$id>\n\")"
 test_emacs "(notmuch-show \"id:$id\") \
             (notmuch-show-next-message) \
             (command-execute (key-binding (kbd \"RET\"))) \
-- 
1.7.7.2

Re: [PATCH v2 03/10] test: add test state reset to test_expect_* functions that did not have it

[David Bremner]

On Thu, 17 Nov 2011 17:05:56 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> ---
>  test/test-lib.sh |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

I pushed the first 3, but the 4th won't apply, even with git am -3

d

[PATCH v3 00/7] test: (hopefully) better test prerequisites

[Dmitry Kurochkin]

Changes:

v3 since v2:

* rebased the remaining patches on current master

v2 since v1:

* Add test_require_external_prereq function to explicitly check for
  external dependencies, use it in test_emacs.

* Indenting fixes.

* Use $binary instead of $1 in test_declare_external_prereq.

Regards,
  Dmitry

[PATCH v3 1/7] test: add support for external executable dependencies

[Dmitry Kurochkin]

There is existing support for general prerequisites in the test suite.
But it is not very convenient to use: every test case has to keep
track for it's dependencies and they have to be explicitly listed.

The patch aims to add better support for a particular type of external
dependencies: external executables.  The main idea is to replace
missing external binaries with shell functions that have the same
name.  These functions always fail and keep track of missing
dependencies for a subtest.  The result reporting functions later can
check that an external binaries are missing and correctly report SKIP
result instead of FAIL.  The primary benefit is that the test cases do
not need to declare their dependencies or be changed in any way.
---
 test/test-lib.sh |   49 +++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 076f929..0996a74 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -548,6 +548,19 @@ test_have_prereq () {
 	esac
 }
 
+# declare prerequisite for the given external binary
+test_declare_external_prereq () {
+	binary="$1"
+	test "$#" = 2 && name=$2 || name="$binary(1)"
+
+	hash $binary 2>/dev/null || eval "
+$binary () {
+	echo -n \"\$test_subtest_missing_external_prereqs_\" | grep -e \" $name \" ||
+	test_subtest_missing_external_prereqs_=\"$test_subtest_missing_external_prereqs_ $name\"
+	false
+}"
+}
+
 # You are not expected to call test_ok_ and test_failure_ directly, use
 # the text_expect_* functions instead.
 
@@ -624,18 +637,31 @@ test_skip () {
 	fi
 	case "$to_skip" in
 	t)
-		test_reset_state_
-		say_color skip >&3 "skipping test: $@"
-		say_color skip "%-6s" "SKIP"
-		echo " $1"
-		: true
+		test_report_skip_ "$@"
 		;;
 	*)
-		false
+		test_check_missing_external_prereqs_ "$@"
 		;;
 	esac
 }
 
+test_check_missing_external_prereqs_ () {
+	if test -n "$test_subtest_missing_external_prereqs_"; then
+		say_color skip >&3 "missing prerequisites:"
+		echo "$test_subtest_missing_external_prereqs_" >&3
+		test_report_skip_ "$@"
+	else
+		false
+	fi
+}
+
+test_report_skip_ () {
+	test_reset_state_
+	say_color skip >&3 "skipping test: $@"
+	say_color skip "%-6s" "SKIP"
+	echo " $1"
+}
+
 test_subtest_known_broken () {
 	test_subtest_known_broken_=t
 }
@@ -648,7 +674,10 @@ test_expect_success () {
 	if ! test_skip "$@"
 	then
 		test_run_ "$2"
-		if [ "$?" = 0 -a "$eval_ret" = 0 ]
+		run_ret="$?"
+		# test_run_ may update missing external prerequisites
+		test_check_missing_external_prereqs_ "$@" ||
+		if [ "$run_ret" = 0 -a "$eval_ret" = 0 ]
 		then
 			test_ok_ "$1"
 		else
@@ -665,7 +694,10 @@ test_expect_code () {
 	if ! test_skip "$@"
 	then
 		test_run_ "$3"
-		if [ "$?" = 0 -a "$eval_ret" = "$1" ]
+		run_ret="$?"
+		# test_run_ may update missing external prerequisites,
+		test_check_missing_external_prereqs_ "$@" ||
+		if [ "$run_ret" = 0 -a "$eval_ret" = "$1" ]
 		then
 			test_ok_ "$2"
 		else
@@ -870,6 +902,7 @@ test_emacs () {
 
 test_reset_state_ () {
 	test_subtest_known_broken_=
+	test_subtest_missing_external_prereqs_=
 }
 
 
-- 
1.7.7.3

[PATCH v3 2/7] test: fix "skipping test" verbose output

[Dmitry Kurochkin]

---
 test/test-lib.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 0996a74..df867a5 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -657,7 +657,8 @@ test_check_missing_external_prereqs_ () {
 
 test_report_skip_ () {
 	test_reset_state_
-	say_color skip >&3 "skipping test: $@"
+	say_color skip >&3 "skipping test:"
+	echo " $@" >&3
 	say_color skip "%-6s" "SKIP"
 	echo " $1"
 }
-- 
1.7.7.3

[PATCH v3 3/7] test: skip all subtests if external dependencies are missing during init

[Dmitry Kurochkin]

Some tests (e.g. crypto) do a common initialization required for all
subtests.  The patch adds a check for missing external dependencies
during this initialization.  If any prerequisites are missing, all
subtests are skipped.

The check is run on the first call of test_reset_state_ function, so
no changes for the tests are needed.
---
 test/test-lib.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index df867a5..99b9e8b 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -902,10 +902,20 @@ test_emacs () {
 }
 
 test_reset_state_ () {
+	test -z "$test_init_done_" && test_init_
+
 	test_subtest_known_broken_=
 	test_subtest_missing_external_prereqs_=
 }
 
+# called once before the first subtest
+test_init_ () {
+	test_init_done_=t
+
+	# skip all tests if there were external prerequisites missing during init
+	test_check_missing_external_prereqs_ "all tests in $this_test" && test_done
+}
+
 
 find_notmuch_path ()
 {
-- 
1.7.7.3

[PATCH v3 4/7] test: declare external dependencies for the tests

[Dmitry Kurochkin]

That are: dtach(1), emacs(1), emacsclient(1), gdb(1) and gpg(1).
---
 test/test-lib.sh |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 99b9e8b..fe80e89 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -1123,3 +1123,10 @@ 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
 rm -f y
+
+# declare prerequisites for external binaries used in tests
+test_declare_external_prereq dtach
+test_declare_external_prereq emacs
+test_declare_external_prereq emacsclient
+test_declare_external_prereq gdb
+test_declare_external_prereq gpg
-- 
1.7.7.3

[PATCH v3 5/7] test: add function to explicitly check for external dependencies

[Dmitry Kurochkin]

Useful when binary is called indirectly (e.g. from emacs).
---
 test/test-lib.sh |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index fe80e89..2422e32 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -555,12 +555,26 @@ test_declare_external_prereq () {
 
 	hash $binary 2>/dev/null || eval "
 $binary () {
+	test_missing_external_prereq_${binary}_=t
 	echo -n \"\$test_subtest_missing_external_prereqs_\" | grep -e \" $name \" ||
 	test_subtest_missing_external_prereqs_=\"$test_subtest_missing_external_prereqs_ $name\"
 	false
 }"
 }
 
+# Explicitly require external prerequisite.  Useful when binary is
+# called indirectly (e.g. from emacs).
+# Returns success if dependency is available, failure otherwise.
+test_require_external_prereq () {
+	binary="$1"
+	if [ "$(eval echo -n \$test_missing_external_prereq_${binary}_)" = t ]; then
+		# dependency is missing, call the replacement function to note it
+		eval "$binary"
+	else
+		true
+	fi
+}
+
 # You are not expected to call test_ok_ and test_failure_ directly, use
 # the text_expect_* functions instead.
 
-- 
1.7.7.3

[PATCH v3 6/7] test: check if emacs is available in the beginning of test_emacs

[Dmitry Kurochkin]

Unfortunately, this is needed to avoid the emacs waiting loop.
---
 test/test-lib.sh |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 2422e32..11e6646 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -894,6 +894,10 @@ EOF
 }
 
 test_emacs () {
+	# test dependencies beforehand to avoid the waiting loop below
+	test_require_external_prereq emacs || return
+	test_require_external_prereq emacsclient || return
+
 	if [ -z "$EMACS_SERVER" ]; then
 		server_name="notmuch-test-suite-$$"
 		# start a detached session with an emacs server
-- 
1.7.7.3

[PATCH v3 7/7] test: fix "Stashing in notmuch-search" test when emacs is not available

[Dmitry Kurochkin]

If emacs is not available, test_expect_equal would be called with only
one argument.  The patch fixes this by quoting the (possibly empty)
$(cat OUTPUT) argument.
---
 test/emacs |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/test/emacs b/test/emacs
index 198c27b..3f8c72d 100755
--- a/test/emacs
+++ b/test/emacs
@@ -412,7 +412,7 @@ test_emacs '(notmuch-search "id:\"bought\"")
         (yank)
 	    (test-output)'
 sed -i -e 's/^thread:.*$/thread:XXX/' OUTPUT
-test_expect_equal $(cat OUTPUT) "thread:XXX"
+test_expect_equal "$(cat OUTPUT)" "thread:XXX"
 
 test_begin_subtest 'Hiding message following HTML part'
 test_subtest_known_broken
-- 
1.7.7.3

Re: [PATCH v3 00/7] test: (hopefully) better test prerequisites

[David Bremner]

On Sun, 27 Nov 2011 22:36:12 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Changes:
> 
> v3 since v2:
> 
> * rebased the remaining patches on current master

These are all pushed to master now.

d

Re: [PATCH v3 00/7] test: (hopefully) better test prerequisites

[Pieter Praet]

On Sun, 27 Nov 2011 22:36:12 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Changes:
> 
> v3 since v2:
> 
> * rebased the remaining patches on current master
> 
> v2 since v1:
> 
> * Add test_require_external_prereq function to explicitly check for
>   external dependencies, use it in test_emacs.
> 
> * Indenting fixes.
> 
> * Use $binary instead of $1 in test_declare_external_prereq.
> 
> Regards,
>   Dmitry
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Fantastic work Dmitry!  My inept dabblings are thoroughly humbled
by the elegance of this approach!

All that's left to go from three to four nines, is ensuring correct
$test_{count,success,fixed,broken,failure} reports (which can most
definitively be categorized as "non-trivial")...

  Eg. with and without emacs(1), emacsclient(1), dtach(1), gpg(1) :

    #+begin_example
      Notmuch test suite complete.
      All 347 tests behaved as expected (2 expected failures).
    #+end_example

    #+begin_example
      Notmuch test suite complete.
      266/312 tests passed.
      1 broken test failed as expected.
      5 tests failed.
      40 tests skipped.
    #+end_example

... as well as a minor issue re test_emacs(), which will be
addressed in a followup patch.


Peace

-- 
Pieter

[PATCH] test: don't bail out of `run_emacs' too early when missing prereqs

[Pieter Praet]

When running the Emacs tests in verbose mode, only the first missing
prereq is reported because the `run_emacs' function is short-circuited
early:

  #+begin_example
    emacs: Testing emacs interface
     missing prerequisites: [0]  emacs(1)
     skipping test: [0]  Basic notmuch-hello view in emacs
     SKIP   [0]  Basic notmuch-hello view in emacs
  #+end_example

This can lead to situations reminiscent of "dependency hell", so instead
of returning based on each individual `test_require_external_prereq's exit
status, we now do so by checking $test_subtest_missing_external_prereqs_:

  #+begin_example
    emacs: Testing emacs interface
     missing prerequisites: [0]  dtach(1) emacs(1) emacsclient(1)
     skipping test: [0]  Basic notmuch-hello view in emacs
     SKIP   [0]  Basic notmuch-hello view in emacs
  #+end_example

Also add missing prereq for dtach(1).

---
 test/test-lib.sh |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 82767c0..6ec3882 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -907,8 +907,10 @@ EOF
 
 test_emacs () {
 	# test dependencies beforehand to avoid the waiting loop below
-	test_require_external_prereq emacs || return
-	test_require_external_prereq emacsclient || return
+	test_require_external_prereq dtach
+	test_require_external_prereq emacs
+	test_require_external_prereq emacsclient
+	test -z "$test_subtest_missing_external_prereqs_" || return
 
 	if [ -z "$EMACS_SERVER" ]; then
 		server_name="notmuch-test-suite-$$"
-- 
1.7.8.1

Re: [PATCH] test: don't bail out of `run_emacs' too early when missing prereqs

[Dmitry Kurochkin]

On Thu, 12 Jan 2012 18:16:59 +0100, Pieter Praet <pieter@praet.org> wrote:
> When running the Emacs tests in verbose mode, only the first missing
> prereq is reported because the `run_emacs' function is short-circuited
> early:
> 
>   #+begin_example
>     emacs: Testing emacs interface
>      missing prerequisites: [0]  emacs(1)
>      skipping test: [0]  Basic notmuch-hello view in emacs
>      SKIP   [0]  Basic notmuch-hello view in emacs
>   #+end_example
> 
> This can lead to situations reminiscent of "dependency hell", so instead
> of returning based on each individual `test_require_external_prereq's exit
> status, we now do so by checking $test_subtest_missing_external_prereqs_:
> 
>   #+begin_example
>     emacs: Testing emacs interface
>      missing prerequisites: [0]  dtach(1) emacs(1) emacsclient(1)
>      skipping test: [0]  Basic notmuch-hello view in emacs
>      SKIP   [0]  Basic notmuch-hello view in emacs
>   #+end_example
> 
> Also add missing prereq for dtach(1).
> 
> ---
>  test/test-lib.sh |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 82767c0..6ec3882 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -907,8 +907,10 @@ EOF
>  
>  test_emacs () {
>  	# test dependencies beforehand to avoid the waiting loop below
> -	test_require_external_prereq emacs || return
> -	test_require_external_prereq emacsclient || return
> +	test_require_external_prereq dtach
> +	test_require_external_prereq emacs
> +	test_require_external_prereq emacsclient
> +	test -z "$test_subtest_missing_external_prereqs_" || return

There may be other missing dependencies before test_emacs() is called
and $test_subtest_missing_external_prereqs_ would not be blank.  Also, I
would like to keep the number of functions that use
$test_subtest_missing_external_prereqs_ minimal.  How about:

  missing_dependencies=
  test_require_... dtach || missing_dependencies=1
  test_require_... emacs || missing_dependencies=1
  ...
  test -z "$missing_dependencies" || return

Regards,
  Dmitry

>  
>  	if [ -z "$EMACS_SERVER" ]; then
>  		server_name="notmuch-test-suite-$$"
> -- 
> 1.7.8.1
> 

Re: [PATCH] test: don't bail out of `run_emacs' too early when missing prereqs

[Pieter Praet]

On Thu, 12 Jan 2012 21:34:29 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> On Thu, 12 Jan 2012 18:16:59 +0100, Pieter Praet <pieter@praet.org> wrote:
> > When running the Emacs tests in verbose mode, only the first missing
> > prereq is reported because the `run_emacs' function is short-circuited
> > early:
> > 
> >   #+begin_example
> >     emacs: Testing emacs interface
> >      missing prerequisites: [0]  emacs(1)
> >      skipping test: [0]  Basic notmuch-hello view in emacs
> >      SKIP   [0]  Basic notmuch-hello view in emacs
> >   #+end_example
> > 
> > This can lead to situations reminiscent of "dependency hell", so instead
> > of returning based on each individual `test_require_external_prereq's exit
> > status, we now do so by checking $test_subtest_missing_external_prereqs_:
> > 
> >   #+begin_example
> >     emacs: Testing emacs interface
> >      missing prerequisites: [0]  dtach(1) emacs(1) emacsclient(1)
> >      skipping test: [0]  Basic notmuch-hello view in emacs
> >      SKIP   [0]  Basic notmuch-hello view in emacs
> >   #+end_example
> > 
> > Also add missing prereq for dtach(1).
> > 
> > ---
> >  test/test-lib.sh |    6 ++++--
> >  1 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/test/test-lib.sh b/test/test-lib.sh
> > index 82767c0..6ec3882 100644
> > --- a/test/test-lib.sh
> > +++ b/test/test-lib.sh
> > @@ -907,8 +907,10 @@ EOF
> >  
> >  test_emacs () {
> >  	# test dependencies beforehand to avoid the waiting loop below
> > -	test_require_external_prereq emacs || return
> > -	test_require_external_prereq emacsclient || return
> > +	test_require_external_prereq dtach
> > +	test_require_external_prereq emacs
> > +	test_require_external_prereq emacsclient
> > +	test -z "$test_subtest_missing_external_prereqs_" || return
> 
> There may be other missing dependencies before test_emacs() is called
> and $test_subtest_missing_external_prereqs_ would not be blank.  [...]

True, hadn't though of that...

> [...] Also, I
> would like to keep the number of functions that use
> $test_subtest_missing_external_prereqs_ minimal.  [...]

Could you elaborate on that?

> [...] How about:
> 
>   missing_dependencies=
>   test_require_... dtach || missing_dependencies=1
>   test_require_... emacs || missing_dependencies=1
>   ...
>   test -z "$missing_dependencies" || return
> 

Agreed!  Patch follows.

> Regards,
>   Dmitry
> 
> >  
> >  	if [ -z "$EMACS_SERVER" ]; then
> >  		server_name="notmuch-test-suite-$$"
> > -- 
> > 1.7.8.1
> > 


Peace

-- 
Pieter

[PATCH] test: don't bail out of `run_emacs' too early when missing prereqs

[Pieter Praet]

When running the Emacs tests in verbose mode, only the first missing
prereq is reported because the `run_emacs' function is short-circuited
early:

  #+begin_example
    emacs: Testing emacs interface
     missing prerequisites: [0]  emacs(1)
     skipping test: [0]  Basic notmuch-hello view in emacs
     SKIP   [0]  Basic notmuch-hello view in emacs
  #+end_example

This can lead to situations reminiscent of "dependency hell", so instead
of returning based on each individual `test_require_external_prereq's exit
status, we now do so only after checking all the prereqs:

  #+begin_example
    emacs: Testing emacs interface
     missing prerequisites: [0]  dtach(1) emacs(1) emacsclient(1)
     skipping test: [0]  Basic notmuch-hello view in emacs
     SKIP   [0]  Basic notmuch-hello view in emacs
  #+end_example

Also added missing prereq for dtach(1).
---
 test/test-lib.sh |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 82767c0..d1fbc05 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -907,8 +907,11 @@ EOF
 
 test_emacs () {
 	# test dependencies beforehand to avoid the waiting loop below
-	test_require_external_prereq emacs || return
-	test_require_external_prereq emacsclient || return
+	missing_dependencies=
+	test_require_external_prereq dtach || missing_dependencies=1
+	test_require_external_prereq emacs || missing_dependencies=1
+	test_require_external_prereq emacsclient || missing_dependencies=1
+	test -z "$missing_dependencies" || return
 
 	if [ -z "$EMACS_SERVER" ]; then
 		server_name="notmuch-test-suite-$$"
-- 
1.7.8.1

Re: [PATCH] test: don't bail out of `run_emacs' too early when missing prereqs

[Dmitry Kurochkin]

On Sat, 14 Jan 2012 10:09:37 +0100, Pieter Praet <pieter@praet.org> wrote:
> When running the Emacs tests in verbose mode, only the first missing
> prereq is reported because the `run_emacs' function is short-circuited
> early:
> 
>   #+begin_example
>     emacs: Testing emacs interface
>      missing prerequisites: [0]  emacs(1)
>      skipping test: [0]  Basic notmuch-hello view in emacs
>      SKIP   [0]  Basic notmuch-hello view in emacs
>   #+end_example
> 
> This can lead to situations reminiscent of "dependency hell", so instead
> of returning based on each individual `test_require_external_prereq's exit
> status, we now do so only after checking all the prereqs:
> 
>   #+begin_example
>     emacs: Testing emacs interface
>      missing prerequisites: [0]  dtach(1) emacs(1) emacsclient(1)
>      skipping test: [0]  Basic notmuch-hello view in emacs
>      SKIP   [0]  Basic notmuch-hello view in emacs
>   #+end_example
> 
> Also added missing prereq for dtach(1).

looks good to me

Regards,
  Dmitry

> ---
>  test/test-lib.sh |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 82767c0..d1fbc05 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -907,8 +907,11 @@ EOF
>  
>  test_emacs () {
>  	# test dependencies beforehand to avoid the waiting loop below
> -	test_require_external_prereq emacs || return
> -	test_require_external_prereq emacsclient || return
> +	missing_dependencies=
> +	test_require_external_prereq dtach || missing_dependencies=1
> +	test_require_external_prereq emacs || missing_dependencies=1
> +	test_require_external_prereq emacsclient || missing_dependencies=1
> +	test -z "$missing_dependencies" || return
>  
>  	if [ -z "$EMACS_SERVER" ]; then
>  		server_name="notmuch-test-suite-$$"
> -- 
> 1.7.8.1
> 

Re: [PATCH] test: don't bail out of `run_emacs' too early when missing prereqs

[Dmitry Kurochkin]

On Sat, 14 Jan 2012 10:07:41 +0100, Pieter Praet <pieter@praet.org> wrote:
> On Thu, 12 Jan 2012 21:34:29 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > On Thu, 12 Jan 2012 18:16:59 +0100, Pieter Praet <pieter@praet.org> wrote:
> > > When running the Emacs tests in verbose mode, only the first missing
> > > prereq is reported because the `run_emacs' function is short-circuited
> > > early:
> > > 
> > >   #+begin_example
> > >     emacs: Testing emacs interface
> > >      missing prerequisites: [0]  emacs(1)
> > >      skipping test: [0]  Basic notmuch-hello view in emacs
> > >      SKIP   [0]  Basic notmuch-hello view in emacs
> > >   #+end_example
> > > 
> > > This can lead to situations reminiscent of "dependency hell", so instead
> > > of returning based on each individual `test_require_external_prereq's exit
> > > status, we now do so by checking $test_subtest_missing_external_prereqs_:
> > > 
> > >   #+begin_example
> > >     emacs: Testing emacs interface
> > >      missing prerequisites: [0]  dtach(1) emacs(1) emacsclient(1)
> > >      skipping test: [0]  Basic notmuch-hello view in emacs
> > >      SKIP   [0]  Basic notmuch-hello view in emacs
> > >   #+end_example
> > > 
> > > Also add missing prereq for dtach(1).
> > > 
> > > ---
> > >  test/test-lib.sh |    6 ++++--
> > >  1 files changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/test/test-lib.sh b/test/test-lib.sh
> > > index 82767c0..6ec3882 100644
> > > --- a/test/test-lib.sh
> > > +++ b/test/test-lib.sh
> > > @@ -907,8 +907,10 @@ EOF
> > >  
> > >  test_emacs () {
> > >  	# test dependencies beforehand to avoid the waiting loop below
> > > -	test_require_external_prereq emacs || return
> > > -	test_require_external_prereq emacsclient || return
> > > +	test_require_external_prereq dtach
> > > +	test_require_external_prereq emacs
> > > +	test_require_external_prereq emacsclient
> > > +	test -z "$test_subtest_missing_external_prereqs_" || return
> > 
> > There may be other missing dependencies before test_emacs() is called
> > and $test_subtest_missing_external_prereqs_ would not be blank.  [...]
> 
> True, hadn't though of that...
> 
> > [...] Also, I
> > would like to keep the number of functions that use
> > $test_subtest_missing_external_prereqs_ minimal.  [...]
> 
> Could you elaborate on that?
> 

This variable is supposed to be internal.  I would like to be able to
change it's meaning or replace it with something better with minimal
changes in the other code.  So I prefer it to be used only by few
"low-level" dependency functions.

This is not some strict rule, in other situation I may agree that using
$test_subtest_missing_external_prereqs_ directly is the best option.
But in this case, introducing a local variable with clean and simple
meaning is better IMO.

Regards,
  Dmitry

> > [...] How about:
> > 
> >   missing_dependencies=
> >   test_require_... dtach || missing_dependencies=1
> >   test_require_... emacs || missing_dependencies=1
> >   ...
> >   test -z "$missing_dependencies" || return
> > 
> 
> Agreed!  Patch follows.
> 
> > Regards,
> >   Dmitry
> > 
> > >  
> > >  	if [ -z "$EMACS_SERVER" ]; then
> > >  		server_name="notmuch-test-suite-$$"
> > > -- 
> > > 1.7.8.1
> > > 
> 
> 
> Peace
> 
> -- 
> Pieter

Re: [PATCH] test: don't bail out of `run_emacs' too early when missing prereqs

[David Bremner]

On Sat, 14 Jan 2012 10:09:37 +0100, Pieter Praet <pieter@praet.org> wrote:
> When running the Emacs tests in verbose mode, only the first missing
> prereq is reported because the `run_emacs' function is short-circuited
> early:

pushed.

d

Re: [PATCH] test: don't bail out of `run_emacs' too early when missing prereqs

[Pieter Praet]

On Sun, 15 Jan 2012 17:56:37 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> On Sat, 14 Jan 2012 10:07:41 +0100, Pieter Praet <pieter@praet.org> wrote:
> > On Thu, 12 Jan 2012 21:34:29 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > > On Thu, 12 Jan 2012 18:16:59 +0100, Pieter Praet <pieter@praet.org> wrote:
> > > > When running the Emacs tests in verbose mode, only the first missing
> > > > prereq is reported because the `run_emacs' function is short-circuited
> > > > early:
> > > > 
> > > >   #+begin_example
> > > >     emacs: Testing emacs interface
> > > >      missing prerequisites: [0]  emacs(1)
> > > >      skipping test: [0]  Basic notmuch-hello view in emacs
> > > >      SKIP   [0]  Basic notmuch-hello view in emacs
> > > >   #+end_example
> > > > 
> > > > This can lead to situations reminiscent of "dependency hell", so instead
> > > > of returning based on each individual `test_require_external_prereq's exit
> > > > status, we now do so by checking $test_subtest_missing_external_prereqs_:
> > > > 
> > > >   #+begin_example
> > > >     emacs: Testing emacs interface
> > > >      missing prerequisites: [0]  dtach(1) emacs(1) emacsclient(1)
> > > >      skipping test: [0]  Basic notmuch-hello view in emacs
> > > >      SKIP   [0]  Basic notmuch-hello view in emacs
> > > >   #+end_example
> > > > 
> > > > Also add missing prereq for dtach(1).
> > > > 
> > > > ---
> > > >  test/test-lib.sh |    6 ++++--
> > > >  1 files changed, 4 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/test/test-lib.sh b/test/test-lib.sh
> > > > index 82767c0..6ec3882 100644
> > > > --- a/test/test-lib.sh
> > > > +++ b/test/test-lib.sh
> > > > @@ -907,8 +907,10 @@ EOF
> > > >  
> > > >  test_emacs () {
> > > >  	# test dependencies beforehand to avoid the waiting loop below
> > > > -	test_require_external_prereq emacs || return
> > > > -	test_require_external_prereq emacsclient || return
> > > > +	test_require_external_prereq dtach
> > > > +	test_require_external_prereq emacs
> > > > +	test_require_external_prereq emacsclient
> > > > +	test -z "$test_subtest_missing_external_prereqs_" || return
> > > 
> > > There may be other missing dependencies before test_emacs() is called
> > > and $test_subtest_missing_external_prereqs_ would not be blank.  [...]
> > 
> > True, hadn't though of that...
> > 
> > > [...] Also, I
> > > would like to keep the number of functions that use
> > > $test_subtest_missing_external_prereqs_ minimal.  [...]
> > 
> > Could you elaborate on that?
> > 
> 
> This variable is supposed to be internal.  I would like to be able to
> change it's meaning or replace it with something better with minimal
> changes in the other code.  So I prefer it to be used only by few
> "low-level" dependency functions.
> 

Ok, thanks!


> This is not some strict rule, in other situation I may agree that using
> $test_subtest_missing_external_prereqs_ directly is the best option.
> But in this case, introducing a local variable with clean and simple
> meaning is better IMO.
> 

Agreed.


> Regards,
>   Dmitry
> 
> > > [...] How about:
> > > 
> > >   missing_dependencies=
> > >   test_require_... dtach || missing_dependencies=1
> > >   test_require_... emacs || missing_dependencies=1
> > >   ...
> > >   test -z "$missing_dependencies" || return
> > > 
> > 
> > Agreed!  Patch follows.
> > 
> > > Regards,
> > >   Dmitry
> > > 
> > > >  
> > > >  	if [ -z "$EMACS_SERVER" ]; then
> > > >  		server_name="notmuch-test-suite-$$"
> > > > -- 
> > > > 1.7.8.1
> > > > 
> > 
> > 
> > Peace
> > 
> > -- 
> > Pieter


Peace

-- 
Pieter