From: Austin Clements <amdragon@MIT.EDU>
To: Tomi Ollila <tomi.ollila@iki.fi>, notmuch@notmuchmail.org
Subject: Re: [PATCH 3/3] test: implement and document NOTMUCH_TEST_QUIET
	variable usage
In-Reply-To: <1385399299-12936-3-git-send-email-tomi.ollila@iki.fi>
References: <1384288868-23903-1-git-send-email-tomi.ollila@iki.fi>
	<1385399299-12936-1-git-send-email-tomi.ollila@iki.fi>
	<1385399299-12936-3-git-send-email-tomi.ollila@iki.fi>
User-Agent: Notmuch/0.16+154~g96c0ce2 (http://notmuchmail.org) Emacs/23.4.1
	(i486-pc-linux-gnu)
Date: Wed, 04 Dec 2013 11:18:49 -0500
Message-ID: <87vbz437yu.fsf@awakening.csail.mit.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: tomi.ollila@iki.fi
Precedence: list

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