threads / patch / 10424

patchAdd some fancy colors in the test library when terminal supports it.

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

## tl;dr

10 messages between Oct 22, 2007 and Oct 23, 2007. Diffs are folded; open one to read it.

replies: 9people: 3as markdown or json

Pierre Habouzit· Oct 22, 2007, 08:13 UTC · lore
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(-)
Show changes to t/test-lib.sh +22 −10
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
Johannes Sixt· Oct 22, 2007, 08:53 UTC · re: Pierre Habouzit · lore

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

Pierre Habouzit schrieb:
Show 35 quoted lines
> 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?
Show 7 quoted lines
> +
> +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"
Pierre Habouzit· Oct 22, 2007, 11:24 UTC · re: Johannes Sixt · lore

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

On Mon, Oct 22, 2007 at 08:53:36AM +0000, Johannes Sixt wrote:
Show 52 quoted lines
> 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
Johannes Sixt· Oct 22, 2007, 11:35 UTC · re: Pierre Habouzit · lore

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

Pierre Habouzit schrieb:
Show 14 quoted lines
> 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
Pierre Habouzit· Oct 22, 2007, 12:11 UTC · re: Johannes Sixt · lore

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

On Mon, Oct 22, 2007 at 11:35:30AM +0000, Johannes Sixt wrote:
Show 20 quoted lines
> 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
Johannes Sixt· Oct 22, 2007, 12:18 UTC · re: Pierre Habouzit · lore

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

Pierre Habouzit schrieb:
Show 9 quoted lines
> 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.
Show 10 quoted lines
>>>>> +}
>> 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
Pierre Habouzit· Oct 22, 2007, 13:45 UTC · re: Johannes Sixt · lore

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

On Mon, Oct 22, 2007 at 12:18:22PM +0000, Johannes Sixt wrote:
Show 12 quoted lines
> 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).
Show 11 quoted lines
> >>>>>+}
> >>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
Christian Couder· Oct 23, 2007, 04:08 UTC · re: Pierre Habouzit · lore

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

Hi Pierre,
Le lundi 22 octobre 2007, Pierre Habouzit a écrit :
Show 11 quoted lines
> +
> +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.

Johannes Sixt· Oct 23, 2007, 06:37 UTC · re: Christian Couder · lore

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

Christian Couder schrieb:
Show 7 quoted lines
> 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
Pierre Habouzit· Oct 23, 2007, 08:13 UTC · re: Christian Couder · lore

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

On Tue, Oct 23, 2007 at 04:08:14AM +0000, Christian Couder wrote:
Show 25 quoted lines
> 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.

Show 7 quoted lines
> 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

← back to recent threads