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: <87vbz437yu.fsf@awakening.csail.mit.edu>
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>
	<87vbz437yu.fsf@awakening.csail.mit.edu>
User-Agent: Notmuch/0.16+154~g96c0ce2 (http://notmuchmail.org) Emacs/23.4.1
	(i486-pc-linux-gnu)
Date: Wed, 04 Dec 2013 15:15:07 -0500
Message-ID: <87siu82x10.fsf@awakening.csail.mit.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: tomi.ollila@iki.fi
Precedence: list

On Wed, 04 Dec 2013, Austin Clements <amdragon@MIT.EDU> wrote:
> 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.

Ignore this comment.  Apparently I was confused by show-mode
transforming leading tabs into spaces.

> 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