Re: [PATCH v6 0/7] make test output coloring more intuitive

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 20, 2012, 19:21 UTC
Message-ID: <7vy5gs4jiy.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20121220161110.GA10605@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 8 quoted lines

>> Good point, I forgot to check what it looked like with -v.  Since this
>> series is already on v6, is there a more lightweight way of addressing
>> this tiny tweak than sending v7?
>
> It is ultimately up to Junio, but I suspect he would be OK if you just
> reposted patch 4/7 with the above squashed. Or even just said "I like
> this, please squash it into patch 4 (change info messages from
> yellow/brown to bold cyan).

Surely; as long as the series is not in 'next', the change to be squashed is not too big and it is not too much work (and in this case it certainly is not).

I actually wonder if "skipped test in bold blue" and "known breakage in bold yellow" should also lose the boldness. Errors and warnings in bold are good, but I would say the degree of need for attention are more like this:

	error (failed tests - you should look into it)
        skip (skipped - perhaps you need more packages?)
        warn (expected failure - you may want to look into fixing it someday)
	info
        pass

The "expected_failure" cases painted in "warn" are all long-known failures; I do not think reminding about them in "bold" over and over will help encouraging the developers take a look at them.

The "skipped" cases fall into two categories. Either you already know you choose to not to care (e.g. I do not expect to use git-p4 and decided not to install p4 anywhere, so I may have t98?? on GIT_SKIP_TESTS environment) or you haven't reached that point on a new system and haven't realized that you didn't install a package needed to run tests you care about (e.g. cvsserver tests would not run without Perl interface to SQLite). For the former, the bold output is merely distracting; for the latter, bold _might_ help in this case.

At least, I think

	GIT_SKIP_TESTS=t98?? sh t9800-git-p4-basic.sh -v

should paint "skipping test t9800 altogether" (emitted with "-v) and the last line "1..0 # SKIP skip all tests in t9800" both in the same "info" color.

How about going further to reduce "bold" a bit more, like this?

diff --git a/t/test-lib.sh b/t/test-lib.sh
index aaf013e..2bbb81d 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -182,13 +182,13 @@ then
 		error)
 			tput bold; tput setaf 1;; # bold red
 		skip)
-			tput bold; tput setaf 4;; # bold blue
+			tput setaf 4;; # bold blue
 		warn)
-			tput bold; tput setaf 3;; # bold brown/yellow
+			tput setaf 3;; # bold brown/yellow
 		pass)
 			tput setaf 2;;            # green
 		info)
-			tput bold; tput setaf 6;; # bold cyan
+			tput setaf 6;; # bold cyan
 		*)
 			test -n "$quiet" && return;;
 		esac
@@ -589,7 +589,7 @@ for skp in $GIT_SKIP_TESTS
 do
 	case "$this_test" in
 	$skp)
-		say_color skip >&3 "skipping test $this_test altogether"
+		say_color info >&3 "skipping test $this_test altogether"
 		skip_all="skip all tests in $this_test"
 		test_done
 	esac