[PATCH] Add some fancy colors in the test library when terminal supports it.

[PATCH] Add some fancy colors in the test library when terminal supports it.

[Pierre Habouzit]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---

Maybe this is just me, but I don't find the output of the test-suite
easy to watch while scrolling. This puts some colors in proper places.

  * end-test summaries are in green or red depending on the sucess of
    the tests.
  * errors are in red.
  * skipped tests and other things that tests `say` are in brown (now
    you can _see_ that your testsuite skips some tests on purpose, I
    only noticed recently that I missed part of the environment for
    proper testing).

I'm not 100% sure the test to see if terminal supports color is correct, and
people using emacs shell buffer or alike tools may have better ideas on how to
make it.

and yes, I know that it "depends" upon tput, but if tput isn't available, the
    [ "x$TERM" != "xdumb" ] && tput hpa 60 >/dev/null 2>&1 && tput setaf 1 >/dev/null 2>&1
expression will fail, and color will be disabled.

 t/test-lib.sh |   32 ++++++++++++++++++++++----------
 1 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index cc1253c..c6521c0 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -59,14 +59,24 @@ esac
 # '
 # . ./test-lib.sh
 
+[ "x$TERM" != "xdumb" ] && tput hpa 60 >/dev/null 2>&1 && tput setaf 1 >/dev/null 2>&1
+nocolor=$?
+
+say_color () {
+	[ "$nocolor" = 0 ] &&  [ "$1" != '-1' ] && tput setaf "$1"
+	shift
+	echo "* $*"
+	tput op
+}
+
 error () {
-	echo "* error: $*"
+	say_color 9 "* error: $*"
 	trap - exit
 	exit 1
 }
 
 say () {
-	echo "* $*"
+	say_color 3 "$*"
 }
 
 test "${test_description}" != "" ||
@@ -84,6 +94,8 @@ do
 		exit 0 ;;
 	-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
 		verbose=t; shift ;;
+	--no-color)
+	    nocolor=1; shift ;;
 	--no-python)
 		# noop now...
 		shift ;;
@@ -122,13 +134,13 @@ test_tick () {
 
 test_ok_ () {
 	test_count=$(expr "$test_count" + 1)
-	say "  ok $test_count: $@"
+	say_color -1 "  ok $test_count: $@"
 }
 
 test_failure_ () {
 	test_count=$(expr "$test_count" + 1)
 	test_failure=$(expr "$test_failure" + 1);
-	say "FAIL $test_count: $1"
+	say_color 9 "FAIL $test_count: $1"
 	shift
 	echo "$@" | sed -e 's/^/	/'
 	test "$immediate" = "" || { trap - exit; exit 1; }
@@ -158,9 +170,9 @@ test_skip () {
 	done
 	case "$to_skip" in
 	t)
-		say >&3 "skipping test: $@"
+		say_color 10 >&3 "skipping test: $@"
 		test_count=$(expr "$test_count" + 1)
-		say "skip $test_count: $1"
+		say_color 10 "skip $test_count: $1"
 		: true
 		;;
 	*)
@@ -247,11 +259,11 @@ test_done () {
 		# The Makefile provided will clean this test area so
 		# we will leave things as they are.
 
-		say "passed all $test_count test(s)"
+		say_color 2 "passed all $test_count test(s)"
 		exit 0 ;;
 
 	*)
-		say "failed $test_failure among $test_count test(s)"
+		say_color 9 "failed $test_failure among $test_count test(s)"
 		exit 1 ;;
 
 	esac
@@ -296,8 +308,8 @@ do
 	done
 	case "$to_skip" in
 	t)
-		say >&3 "skipping test $this_test altogether"
-		say "skip all tests in $this_test"
+		say_color 10 >&3 "skipping test $this_test altogether"
+		say_color 10 "skip all tests in $this_test"
 		test_done
 	esac
 done
-- 
1.5.3.4.1342.ge0cd-dirty

Re: [PATCH] Add some fancy colors in the test library when terminal supports it.

[Johannes Sixt]

Pierre Habouzit schrieb:
> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ---
> 
> Maybe this is just me, but I don't find the output of the test-suite
> easy to watch while scrolling. This puts some colors in proper places.
> 
>   * end-test summaries are in green or red depending on the sucess of
>     the tests.
>   * errors are in red.
>   * skipped tests and other things that tests `say` are in brown (now
>     you can _see_ that your testsuite skips some tests on purpose, I
>     only noticed recently that I missed part of the environment for
>     proper testing).
> 
> I'm not 100% sure the test to see if terminal supports color is correct, and
> people using emacs shell buffer or alike tools may have better ideas on how to
> make it.
> 
> and yes, I know that it "depends" upon tput, but if tput isn't available, the
>     [ "x$TERM" != "xdumb" ] && tput hpa 60 >/dev/null 2>&1 && tput setaf 1 >/dev/null 2>&1
> expression will fail, and color will be disabled.
> 
>  t/test-lib.sh |   32 ++++++++++++++++++++++----------
>  1 files changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index cc1253c..c6521c0 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -59,14 +59,24 @@ esac
>  # '
>  # . ./test-lib.sh
>  
> +[ "x$TERM" != "xdumb" ] && tput hpa 60 >/dev/null 2>&1 && tput setaf 1 >/dev/null 2>&1
> +nocolor=$?

test "x$TERM" != "xdumb" &&
	tput hpa 60 >/dev/null 2>&1 &&
	tput setaf 1 >/dev/null 2>&1 &&
	color=t

BTW, doesn't tput fail if stdout/stderr is not a terminal, like above?

> +
> +say_color () {
> +	[ "$nocolor" = 0 ] &&  [ "$1" != '-1' ] && tput setaf "$1"
> +	shift
> +	echo "* $*"
> +	tput op
> +}

What if tput is not available, like on Windows? How about this (at the end 
of the file, so it can obey --no-color):

if test "$color"; then
	say_color () {
		test "$1" != '-1' && tput setaf "$1"
		shift
		echo "* $*"
		tput op
	}
else
	say_color() {
		shift
		echo "* $*"
	}
fi

> +	--no-color)
> +	    nocolor=1; shift ;;

	    color=; shift ;;

-- Hannes "We don't need no double negation"

Re: [PATCH] Add some fancy colors in the test library when terminal supports it.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 2340 bytes --]

On Mon, Oct 22, 2007 at 08:53:36AM +0000, Johannes Sixt wrote:
> Pierre Habouzit schrieb:
> >Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> >---
> >Maybe this is just me, but I don't find the output of the test-suite
> >easy to watch while scrolling. This puts some colors in proper places.
> >  * end-test summaries are in green or red depending on the sucess of
> >    the tests.
> >  * errors are in red.
> >  * skipped tests and other things that tests `say` are in brown (now
> >    you can _see_ that your testsuite skips some tests on purpose, I
> >    only noticed recently that I missed part of the environment for
> >    proper testing).
> >I'm not 100% sure the test to see if terminal supports color is correct, 
> >and
> >people using emacs shell buffer or alike tools may have better ideas on 
> >how to
> >make it.
> >and yes, I know that it "depends" upon tput, but if tput isn't 
> >available, the
> >    [ "x$TERM" != "xdumb" ] && tput hpa 60 >/dev/null 2>&1 && tput setaf 
> >1 >/dev/null 2>&1
> >expression will fail, and color will be disabled.
> > t/test-lib.sh |   32 ++++++++++++++++++++++----------
> > 1 files changed, 22 insertions(+), 10 deletions(-)
> >diff --git a/t/test-lib.sh b/t/test-lib.sh
> >index cc1253c..c6521c0 100644
> >--- a/t/test-lib.sh
> >+++ b/t/test-lib.sh
> >@@ -59,14 +59,24 @@ esac
> > # '
> > # . ./test-lib.sh
> > +[ "x$TERM" != "xdumb" ] && tput hpa 60 >/dev/null 2>&1 && tput setaf 1 
> >>/dev/null 2>&1
> >+nocolor=$?
> 
> test "x$TERM" != "xdumb" &&
> 	tput hpa 60 >/dev/null 2>&1 &&
> 	tput setaf 1 >/dev/null 2>&1 &&
> 	color=t
> 
> BTW, doesn't tput fail if stdout/stderr is not a terminal, like above?
> 
> >+
> >+say_color () {
> >+	[ "$nocolor" = 0 ] &&  [ "$1" != '-1' ] && tput setaf "$1"
> >+	shift
> >+	echo "* $*"
> >+	tput op
> >+}
> 
> What if tput is not available, like on Windows? How about this (at the 
> end of the file, so it can obey --no-color):

  I answered to it already in my first mail: if tput isn't available,
the command fails, and $? is non 0. and nocolor is set. Or color isn't
set to 't' for your proposal.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Add some fancy colors in the test library when terminal supports it.

[Johannes Sixt]

Pierre Habouzit schrieb:
> On Mon, Oct 22, 2007 at 08:53:36AM +0000, Johannes Sixt wrote:
>> Pierre Habouzit schrieb:
>>> +say_color () {
>>> +	[ "$nocolor" = 0 ] &&  [ "$1" != '-1' ] && tput setaf "$1"
>>> +	shift
>>> +	echo "* $*"
>>> +	tput op
>>> +}
>> What if tput is not available, like on Windows? How about this (at the 
>> end of the file, so it can obey --no-color):
> 
>   I answered to it already in my first mail: if tput isn't available,
> the command fails, and $? is non 0. and nocolor is set. Or color isn't
> set to 't' for your proposal.

I was too terse, sorry. I wanted to point out that if tput is not available, 
the second invocation will leave "tput: command not found" behind on stderr. 
Therefore, I proposed to make the definition of say_color() different 
depending on whether $color is set or not. Then you don't need to test for 
$color twice inside the function.

-- Hannes

Re: [PATCH] Add some fancy colors in the test library when terminal supports it.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1388 bytes --]

On Mon, Oct 22, 2007 at 11:35:30AM +0000, Johannes Sixt wrote:
> Pierre Habouzit schrieb:
> >On Mon, Oct 22, 2007 at 08:53:36AM +0000, Johannes Sixt wrote:
> >>Pierre Habouzit schrieb:
> >>>+say_color () {
> >>>+	[ "$nocolor" = 0 ] &&  [ "$1" != '-1' ] && tput setaf "$1"
> >>>+	shift
> >>>+	echo "* $*"
> >>>+	tput op
> >>>+}
> >>What if tput is not available, like on Windows? How about this (at the 
> >>end of the file, so it can obey --no-color):
> >  I answered to it already in my first mail: if tput isn't available,
> >the command fails, and $? is non 0. and nocolor is set. Or color isn't
> >set to 't' for your proposal.
> 
> I was too terse, sorry. I wanted to point out that if tput is not 
> available, the second invocation will leave "tput: command not found" 
> behind on stderr. Therefore, I proposed to make the definition of 
> say_color() different depending on whether $color is set or not. Then you 
> don't need to test for $color twice inside the function.

  Right we can do that. I'll try to rework the patch. and no it
shouldn't leave tput: command not found as I 2>/dev/null and I think the
shell doesn't print that in that case. At least my zsh doesn't.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Add some fancy colors in the test library when terminal supports it.

[Johannes Sixt]

Pierre Habouzit schrieb:
> On Mon, Oct 22, 2007 at 11:35:30AM +0000, Johannes Sixt wrote:
>> Pierre Habouzit schrieb:
>>> On Mon, Oct 22, 2007 at 08:53:36AM +0000, Johannes Sixt wrote:
>>>> Pierre Habouzit schrieb:
>>>>> +say_color () {
>>>>> +	[ "$nocolor" = 0 ] &&  [ "$1" != '-1' ] && tput setaf "$1"
>>>>> +	shift
>>>>> +	echo "* $*"
>>>>> +	tput op
        ^^^^^^^^
I am talking about this line.

>>>>> +}
>> I wanted to point out that if tput is not 
>> available, the second invocation will leave "tput: command not found" 
>> behind on stderr. Therefore, I proposed to make the definition of 
>> say_color() different depending on whether $color is set or not. Then you 
>> don't need to test for $color twice inside the function.
> 
>   Right we can do that. I'll try to rework the patch. and no it
> shouldn't leave tput: command not found as I 2>/dev/null and I think the
> shell doesn't print that in that case. At least my zsh doesn't.

There is no 2>/dev/null. Am I missing something?

-- Hannes

Re: [PATCH] Add some fancy colors in the test library when terminal supports it.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1407 bytes --]

On Mon, Oct 22, 2007 at 12:18:22PM +0000, Johannes Sixt wrote:
> Pierre Habouzit schrieb:
> >On Mon, Oct 22, 2007 at 11:35:30AM +0000, Johannes Sixt wrote:
> >>Pierre Habouzit schrieb:
> >>>On Mon, Oct 22, 2007 at 08:53:36AM +0000, Johannes Sixt wrote:
> >>>>Pierre Habouzit schrieb:
> >>>>>+say_color () {
> >>>>>+	[ "$nocolor" = 0 ] &&  [ "$1" != '-1' ] && tput setaf "$1"
> >>>>>+	shift
> >>>>>+	echo "* $*"
> >>>>>+	tput op
>        ^^^^^^^^
> I am talking about this line.

  Oooh, good catch :P it should be guarded by a [ "$nocolor" = 0 ]
indeed :P (or use your solution).

> >>>>>+}
> >>I wanted to point out that if tput is not available, the second 
> >>invocation will leave "tput: command not found" behind on stderr. 
> >>Therefore, I proposed to make the definition of say_color() different 
> >>depending on whether $color is set or not. Then you don't need to test 
> >>for $color twice inside the function.
> >  Right we can do that. I'll try to rework the patch. and no it
> >shouldn't leave tput: command not found as I 2>/dev/null and I think the
> >shell doesn't print that in that case. At least my zsh doesn't.
> 
> There is no 2>/dev/null. Am I missing something?

  I was.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Add some fancy colors in the test library when terminal supports it.

[Christian Couder]

Hi Pierre,

Le lundi 22 octobre 2007, Pierre Habouzit a écrit :
> +
> +say_color () {
> +	[ "$nocolor" = 0 ] &&  [ "$1" != '-1' ] && tput setaf "$1"
> +	shift
> +	echo "* $*"
> +	tput op
> +}
> +
>  error () {
> -	echo "* error: $*"
> +	say_color 9 "* error: $*"

This will print something like "* * error: ..." instead of "* error: ..."

The following should work:

> +	say_color 9 "error: $*"

By the way, where do the 9 here and the 10 and the -1 below come from ?
"man 5 terminfo" says that only values form 0 to 7 are portably defined.
Maybe 9 is a bold red and 10 a bold green, or something like that, but it 
doesn't seem to work on my konsole.

Anyway, perhaps having:

_red=1
_green=2

and then using "say_color $_red stuff" might be easier to understand and 
change if needed.

Thanks for this good idea,
Christian.

Re: [PATCH] Add some fancy colors in the test library when terminal supports it.

[Johannes Sixt]

Christian Couder schrieb:
> Anyway, perhaps having:
> 
> _red=1
> _green=2
> 
> and then using "say_color $_red stuff" might be easier to understand and 
> change if needed.

Good idea. But then better name it by its purpose:

fail=1	# red
pass=2	# green

-- Hannes

Re: [PATCH] Add some fancy colors in the test library when terminal supports it.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1223 bytes --]

On Tue, Oct 23, 2007 at 04:08:14AM +0000, Christian Couder wrote:
> Hi Pierre,
> 
> Le lundi 22 octobre 2007, Pierre Habouzit a écrit :
> > +
> > +say_color () {
> > +	[ "$nocolor" = 0 ] &&  [ "$1" != '-1' ] && tput setaf "$1"
> > +	shift
> > +	echo "* $*"
> > +	tput op
> > +}
> > +
> >  error () {
> > -	echo "* error: $*"
> > +	say_color 9 "* error: $*"
> 
> This will print something like "* * error: ..." instead of "* error: ..."
> 
> The following should work:
> 
> > +	say_color 9 "error: $*"
> 
> By the way, where do the 9 here and the 10 and the -1 below come from ?
> "man 5 terminfo" says that only values form 0 to 7 are portably defined.
> Maybe 9 is a bold red and 10 a bold green, or something like that, but it 
> doesn't seem to work on my konsole.

Right I should use tput setb or sth like that to ask for bold mode
probably.

> Anyway, perhaps having:
> 
> _red=1
> _green=2
> 
> and then using "say_color $_red stuff" might be easier to understand and 
> change if needed.

Agreed.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Add some fancy colors in the test library when terminal supports it.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 466 bytes --]

> +default_color=0
> +fail_color=1    # red
> +pass_color=2    # green
> +msg_color=3     # brown
> +

  err, sorry, this part is spurious, I wasn't paying attention when I
re-read my patch. It's impressive how many mistakes you catch reading
the same patch 20 minutes later…

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]