Re: [PATCH 8/5] tap: support colorization of testsuite progress output
On Tuesday 19 July 2011, Ralf Wildenhues wrote:
* Stefano Lattarini wrote on Tue, Jul 19, 2011 at 11:47:21AM CEST:
On Monday 18 July 2011, Ralf Wildenhues wrote:
+ grn = '[0;32m',
+ lgn = '[1;32m',
+ blu = '[1;34m',
+ mgn = '[0;35m',
+ brg = '[1m',
+ std = '[m',
+);
+sub decorate_result ($)
+{
+ return $_[0] unless $cfg{color-tests};
+ # Best way to simulate a 'switch' construct here.
Please don't, that only obfuscates the code. automake.in uses long
if ... else lists. If you don't like that,
I don't, at least not when using the GNU Coding Standards (as too much
vertical space gets wasted IMO).
Maybe, but vertical space is less important than clear code. Your
version made me stumble and think for a moment whether all code paths
are clear.
use a map.
How would that be clearer? Honest question.
Oh, or myabe you were suggesting to use an hash?
Yes, of course. Sorry for the lax terminology there.
Thanks,
Ralf
Below is what I've squashed in. OK?
Regards,
Stefano
-*-*-*-
diff --git a/lib/tap-driver b/lib/tap-driver
index f39d92b..69533b4 100755
--- a/lib/tap-driver
+++ b/lib/tap-driver
@@ -230,17 +230,24 @@ sub colored ($$)
sub decorate_result ($)
{
- return $_[0] unless $cfg{color-tests};
- # Best way to simulate a 'switch' construct here.
- for (@_)
+ my $result = shift;
+ return $result unless $cfg{color-tests};
+ my %color_for_result =
+(
+ ERROR = 'mgn',
+ PASS = 'grn',
+ XPASS = 'red',
+ FAIL = 'red',
+ XFAIL = 'lgn',
+ SKIP = 'blu',
+);
+ if (my $color = $color_for_result{$result})
+{
+ return colored ($color, $result);
+}
+ else
{
- $_ eq ERROR and return colored ('mgn', $_);
- $_ eq PASS and return colored ('grn', $_);
- $_ eq XPASS and return colored ('red', $_);
- $_ eq FAIL and return colored ('red', $_);
- $_ eq XFAIL and return colored ('lgn', $_);
- $_ eq SKIP and return colored ('blu', $_);
- return $_; # Don't colorize unknown stuff.
+ return $result; # Don't colorize unknown stuff.
}
}
Re: [PATCH 8/5] tap: support colorization of testsuite progress output
On Tuesday 19 July 2011, Ralf Wildenhues wrote:
Below is what I've squashed in. OK?
Sure, thanks!
--- a/lib/tap-driver
+++ b/lib/tap-driver
@@ -230,17 +230,24 @@ sub colored ($$)
sub decorate_result ($)
{
- return $_[0] unless $cfg{color-tests};
- # Best way to simulate a 'switch' construct here.
- for (@_)
+ my $result = shift;
+ return $result unless $cfg{color-tests};
+ my %color_for_result =
+(
+ ERROR = 'mgn',
+ PASS = 'grn',
+ XPASS = 'red',
+ FAIL = 'red',
+ XFAIL = 'lgn',
+ SKIP = 'blu',
+);
+ if (my $color = $color_for_result{$result})
+{
+ return colored ($color, $result);
+}
+ else
{
- $_ eq ERROR and return colored ('mgn', $_);
- $_ eq PASS and return colored ('grn', $_);
- $_ eq XPASS and return colored ('red', $_);
- $_ eq FAIL and return colored ('red', $_);
- $_ eq XFAIL and return colored ('lgn', $_);
- $_ eq SKIP and return colored ('blu', $_);
- return $_; # Don't colorize unknown stuff.
+ return $result; # Don't colorize unknown stuff.
}
}
Re: [PATCH 8/5] tap: support colorization of testsuite progress output
On Monday 18 July 2011, Ralf Wildenhues wrote:
* Stefano Lattarini wrote on Mon, Jul 18, 2011 at 10:30:56AM CEST:
* lib/tap-driver (%COLORS): New variable (definition extracted
from `lib/am/check.am:$(am__tty_colors)', with some obvious
adjustments.
(report): Adjust to colorize console output when required,
using ...
(decorate_result): ... this new function.
(colored): New function, used by the one above.
* tests/tap-summary.test: Also run the checks when `color-tests'
is in use.
* tests/Makefile.am (XFAIL_TESTS): Remove `tap-color.test'.
OK once dependent changes are in, and with nits below addressed.
Thanks,
Ralf
--- a/lib/tap-driver
+++ b/lib/tap-driver
+# Keep this in sync with `lib/am/check.am:$(am__tty_colors)'.
+my %COLOR = (
+ red = '[0;31m',
I'm sure perl has a way to encode ESC without a literal ESC.
Good observation, I should use \e instead of a literal esc.
Consider that done.
+ grn = '[0;32m',
+ lgn = '[1;32m',
+ blu = '[1;34m',
+ mgn = '[0;35m',
+ brg = '[1m',
+ std = '[m',
+);
@@ -211,17 +222,39 @@ sub stringify_test_result ($)
+sub decorate_result ($)
+{
+ return $_[0] unless $cfg{color-tests};
+ # Best way to simulate a 'switch' construct here.
Please don't, that only obfuscates the code. automake.in uses long
if ... else lists. If you don't like that,
I don't, at least not when using the GNU Coding Standards (as too much
vertical space gets wasted IMO).
use a map.
How would that be clearer? Honest question.
Oh, or myabe you were suggesting to use an hash?
+ for (@_)
+{
+ $_ eq ERROR and return colored ('mgn', $_);
+ $_ eq PASS and return colored ('grn', $_);
+ $_ eq XPASS and return colored ('red', $_);
+ $_ eq FAIL and return colored ('red', $_);
+ $_ eq XFAIL and return colored ('lgn', $_);
+ $_ eq SKIP and return colored ('blu', $_);
+ return $_; # Don't colorize unknown stuff.
+}
+}
+
sub report ($;$)
{
my ($msg, $result, $explanation) = (undef, @_);
if ($result =~ /^(?:X?(?:PASS|FAIL)|SKIP|ERROR)/)
{
- $msg = $result: $test_script_name;
+ $msg = : $test_script_name;
add_test_result $result;
}
elsif ($result eq #)
{
- $msg = # $test_script_name:;
+ $msg = $test_script_name:;
}
else
{
@@ -229,10 +262,11 @@ sub report ($;$)
}
$msg .= $explanation if defined $explanation;
$msg .= \n;
- print OLDOUT $msg;
+ # Output on console might be colorized.
+ print OLDOUT decorate_result ($result) . $msg;
# Log the result in the log file too, to help debugging (this is
# especially true when said result is a TAP error or Bail out!).
- print $msg;
+ print $result . $msg;
}
sub testuite_error ($)
[...]
Regards,
Stefano
Re: [PATCH 8/5] tap: support colorization of testsuite progress output
* Stefano Lattarini wrote on Mon, Jul 18, 2011 at 10:30:56AM CEST:
* lib/tap-driver (%COLORS): New variable (definition extracted
from `lib/am/check.am:$(am__tty_colors)', with some obvious
adjustments.
(report): Adjust to colorize console output when required,
using ...
(decorate_result): ... this new function.
(colored): New function, used by the one above.
* tests/tap-summary.test: Also run the checks when `color-tests'
is in use.
* tests/Makefile.am (XFAIL_TESTS): Remove `tap-color.test'.
OK once dependent changes are in, and with nits below addressed.
Thanks,
Ralf
--- a/lib/tap-driver
+++ b/lib/tap-driver
+# Keep this in sync with `lib/am/check.am:$(am__tty_colors)'.
+my %COLOR = (
+ red = '[0;31m',
I'm sure perl has a way to encode ESC without a literal ESC.
+ grn = '[0;32m',
+ lgn = '[1;32m',
+ blu = '[1;34m',
+ mgn = '[0;35m',
+ brg = '[1m',
+ std = '[m',
+);
@@ -211,17 +222,39 @@ sub stringify_test_result ($)
+sub decorate_result ($)
+{
+ return $_[0] unless $cfg{color-tests};
+ # Best way to simulate a 'switch' construct here.
Please don't, that only obfuscates the code. automake.in uses long
if ... else lists. If you don't like that, use a map.
+ for (@_)
+{
+ $_ eq ERROR and return colored ('mgn', $_);
+ $_ eq PASS and return colored ('grn', $_);
+ $_ eq XPASS and return colored ('red', $_);
+ $_ eq FAIL and return colored ('red', $_);
+ $_ eq XFAIL and return colored ('lgn', $_);
+ $_ eq SKIP and return colored ('blu', $_);
+ return $_; # Don't colorize unknown stuff.
+}
+}
+
sub report ($;$)
{
my ($msg, $result, $explanation) = (undef, @_);
if ($result =~ /^(?:X?(?:PASS|FAIL)|SKIP|ERROR)/)
{
- $msg = $result: $test_script_name;
+ $msg = : $test_script_name;
add_test_result $result;
}
elsif ($result eq #)
{
- $msg = # $test_script_name:;
+ $msg = $test_script_name:;
}
else
{
@@ -229,10 +262,11 @@ sub report ($;$)
}
$msg .= $explanation if defined $explanation;
$msg .= \n;
- print OLDOUT $msg;
+ # Output on console might be colorized.
+ print OLDOUT decorate_result ($result) . $msg;
# Log the result in the log file too, to help debugging (this is
# especially true when said result is a TAP error or Bail out!).
- print $msg;
+ print $result . $msg;
}
sub testuite_error ($)
[...]
