Re: [PATCH 3/3] test: implement and document NOTMUCH_TEST_QUIET variable usage

Subject: Re: [PATCH 3/3] test: implement and document NOTMUCH_TEST_QUIET variable usage

Date: Wed, 04 Dec 2013 11:18:49 -0500

To: Tomi Ollila, notmuch@notmuchmail.org

Cc: tomi.ollila@iki.fi

From: Austin Clements


I just tried to use this and realized it hadn't been pushed yet.

This series LGTM except one minor nit below and the fact that it
introduces a lot of tab-indented code in sections of test-lib.sh that
appear to be space-indented.  Given that test-lib.sh is already a mess
of indentation styles, I don't know if we care, but it would be nice if
its entropy were at least non-increasing.

On Mon, 25 Nov 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> When NOTMUCH_TEST_QUIET environment variable is set to non-null value
> messages when new test script starts and when test PASSes are disabled.
> This eases picking the cases when tests FAIL (as those are still printed).
> ---
>  test/README      |  8 ++++++++
>  test/basic       |  4 ++--
>  test/test-lib.sh | 11 ++++++++++-
>  3 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/test/README b/test/README
> index d12cff2..79a9b1b 100644
> --- a/test/README
> +++ b/test/README
> @@ -74,10 +74,18 @@ the tests in one of the following ways.
>  
>  	TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient make test
>  	TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient ./emacs
>  	make test TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient
>  
> +Quiet Execution
> +---------------
> +
> +Normally, when new script starts and when test PASSes you get a message
> +printed on screen. This printing can be disabled by setting the
> +NOTMUCH_TEST_QUIET variable to a non-null value. Message on test
> +failures and skips are still printed.
> +
>  Skipping Tests
>  --------------
>  If, for any reason, you need to skip one or more tests, you can do so
>  by setting the NOTMUCH_SKIP_TESTS variable to the name of one or more
>  sections of tests.
> diff --git a/test/basic b/test/basic
> index 64eb7d7..f7eed32 100755
> --- a/test/basic
> +++ b/test/basic
> @@ -72,16 +72,16 @@ suppress_diff_date() {
>      sed -e 's/\(.*\-\-\- test-verbose\.4\.\expected\).*/\1/' \
>  	-e 's/\(.*\+\+\+ test-verbose\.4\.\output\).*/\1/'
>  }
>  
>  test_begin_subtest "Ensure that test output is suppressed unless the test fails"
> -output=$(cd $TEST_DIRECTORY; ./test-verbose 2>&1 | suppress_diff_date)
> +output=$(cd $TEST_DIRECTORY; NOTMUCH_TEST_QUIET= ./test-verbose 2>&1 | suppress_diff_date)
>  expected=$(cat $EXPECTED/test-verbose-no | suppress_diff_date)
>  test_expect_equal "$output" "$expected"
>  
>  test_begin_subtest "Ensure that -v does not suppress test output"
> -output=$(cd $TEST_DIRECTORY; ./test-verbose -v 2>&1 | suppress_diff_date)
> +output=$(cd $TEST_DIRECTORY; NOTMUCH_TEST_QUIET= ./test-verbose -v 2>&1 | suppress_diff_date)
>  expected=$(cat $EXPECTED/test-verbose-yes | suppress_diff_date)
>  # Do not include the results of test-verbose in totals
>  rm $TEST_DIRECTORY/test-results/test-verbose
>  rm -r $TEST_DIRECTORY/tmp.test-verbose
>  test_expect_equal "$output" "$expected"
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 34e0db6..9d4a807 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -196,11 +196,14 @@ print_test_description ()
>  	test -z "$test_description_printed" || return 0
>  	echo
>  	echo $this_test: "Testing ${test_description}"
>  	test_description_printed=1
>  }
> -print_test_description
> +if [ -z "$NOTMUCH_TEST_QUIET" ]
> +then
> +	print_test_description
> +fi
>  
>  exec 5>&1
>  
>  test_failure=0
>  test_count=0
> @@ -715,20 +718,26 @@ test_ok_ () {
>  	if test "$test_subtest_known_broken_" = "t"; then
>  		test_known_broken_ok_
>  		return
>  	fi
>  	test_success=$(($test_success + 1))
> +	if test -n "$NOTMUCH_TEST_QUIET"; then
> +		return 0
> +	fi
>  	say_color pass "%-6s" "PASS"
>  	echo " $test_subtest_name"
>  }
>  
>  test_failure_ () {
>  	if test "$test_subtest_known_broken_" = "t"; then
>  		test_known_broken_failure_ "$@"
>  		return
>  	fi
>  	test_failure=$(($test_failure + 1))
> +	if test -n "$NOTMUCH_TEST_QUIET"; then

Strictly speaking, this test isn't necessary, right?

> +		print_test_description
> +	fi
>  	test_failure_message_ "FAIL" "$test_subtest_name" "$@"
>  	test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
>  	return 1
>  }
>  
> -- 
> 1.8.4.2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Thread: