[PATCH] Adds colors to kselftest output

[PATCH] Adds colors to kselftest output

[Marcelo Moreira]

This commit improves the readability of kselftest results by adding
ANSI color codes to the [ OK ] and FAIL messages.

Signed-off-by: Marcelo Moreira <marcelomoreira1905@gmail.com>
---
 tools/testing/selftests/net/mptcp/mptcp_lib.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 051e289d7967..7b55855ff788 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -83,7 +83,7 @@ mptcp_lib_print_err() {
 
 # shellcheck disable=SC2120 # parameters are optional
 mptcp_lib_pr_ok() {
-	mptcp_lib_print_ok "[ OK ]${1:+ ${*}}"
+	mptcp_lib_print_ok "\033[32m[ OK ]\033[0m${1:+ ${*}}"
 }
 
 mptcp_lib_pr_skip() {
@@ -97,7 +97,7 @@ mptcp_lib_pr_fail() {
 		title="IGNO"
 		cmt=" (flaky)"
 	else
-		title="FAIL"
+		title="\033[31mFAIL\033[0m"
 	fi
 
 	mptcp_lib_print_err "[${title}]${cmt}${1:+ ${*}}"
-- 
2.48.1

Re: [PATCH] Adds colors to kselftest output

[Bjorn Helgaas]

I'm confused about why we need this patch, but some overall hints:

Run "git log --oneline tools/testing/selftests/net/mptcp/mptcp_lib.sh"
and make your subject line match the prefixes and overall style:

  $ git log --oneline tools/testing/selftests/net/mptcp/mptcp_lib.sh
  5fbea888f8aa ("selftests: mptcp: add -m with ss in case of errors")
  8c6bb011e188 ("selftests: mptcp: move stats info in case of errors to lib.sh")
  f58817c852e9 ("selftests: mptcp: lib: add time per subtests in TAP output")
  20ccc7c5f7a3 ("selftests: mptcp: join: validate event numbers")
  464b99e77b3d ("selftests: mptcp: lib: fix shellcheck errors")
  1af3bc912eac ("selftests: mptcp: lib: use wait_local_port_listen helper")
  f265d3119a29 ("selftests: mptcp: lib: use setup/cleanup_ns helpers")
  5597613fb3cf ("selftests: mptcp: lib: support flaky subtests")
  ...

So your subject line should probably be:

  selftests: mptcp: lib: add colors to kselftest output

One could argue for "selftests: mptcp:", but generally those touch
multiple files.

On Thu, Feb 13, 2025 at 06:28:16PM -0300, Marcelo Moreira wrote:
> This commit improves the readability of kselftest results by adding
> ANSI color codes to the [ OK ] and FAIL messages.

s/This commit improves/Improve/

See:
  https://chris.beams.io/posts/git-commit/
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/maintainer-tip.rst?id=v6.11#n134

> Signed-off-by: Marcelo Moreira <marcelomoreira1905@gmail.com>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_lib.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index 051e289d7967..7b55855ff788 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -83,7 +83,7 @@ mptcp_lib_print_err() {
>  
>  # shellcheck disable=SC2120 # parameters are optional
>  mptcp_lib_pr_ok() {
> -	mptcp_lib_print_ok "[ OK ]${1:+ ${*}}"
> +	mptcp_lib_print_ok "\033[32m[ OK ]\033[0m${1:+ ${*}}"

I'm confused.  Did you look at the implementation of
mptcp_lib_print_ok()?  It looks like this:

  mptcp_lib_print_ok() {
	  mptcp_lib_print_color "${MPTCP_LIB_COLOR_GREEN}${*}"
  }

So it looks like this should already be colored.

If there's something broken here, I would take a look at 747ba8783a33
("selftests: mptcp: print test results with colors") and cc the author
when fixing it.

>  }
>  
>  mptcp_lib_pr_skip() {
> @@ -97,7 +97,7 @@ mptcp_lib_pr_fail() {
>  		title="IGNO"
>  		cmt=" (flaky)"
>  	else
> -		title="FAIL"
> +		title="\033[31mFAIL\033[0m"
>  	fi
>  
>  	mptcp_lib_print_err "[${title}]${cmt}${1:+ ${*}}"
> -- 
> 2.48.1
> 

RE: [PATCH] Adds colors to kselftest output

[Bird, Tim]

> -----Original Message-----
> From: Marcelo Moreira <marcelomoreira1905@gmail.com>
> This commit improves the readability of kselftest results by adding
> ANSI color codes to the [ OK ] and FAIL messages.
> 
> Signed-off-by: Marcelo Moreira <marcelomoreira1905@gmail.com>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_lib.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index 051e289d7967..7b55855ff788 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -83,7 +83,7 @@ mptcp_lib_print_err() {
> 
>  # shellcheck disable=SC2120 # parameters are optional
>  mptcp_lib_pr_ok() {
> -	mptcp_lib_print_ok "[ OK ]${1:+ ${*}}"
> +	mptcp_lib_print_ok "\033[32m[ OK ]\033[0m${1:+ ${*}}"
>  }

This should not be done unconditionally, but based on the controlling
and COLOR variables for this library.  You should also re-used the existing
color print helper function.

IOW, why is this not using MPTCP_LIB_COLOR_GREEN?
or the helper function mptcp_lib_print_color()?

>  mptcp_lib_pr_skip() {
> @@ -97,7 +97,7 @@ mptcp_lib_pr_fail() {
>  		title="IGNO"
>  		cmt=" (flaky)"
>  	else
> -		title="FAIL"
> +		title="\033[31mFAIL\033[0m"

Why not use the helper function and the MPTCP_LIB_COLOR_RED here?

 -- Tim

Re: [PATCH] Adds colors to kselftest output

[Marcelo Moreira]

Guys, sorry about that. For some reason when I run sudo make kselftest
the colors don't show up, but if I run the test alone, the colors do
show up, like this: sudo
tools/testing/selftests/net/mptcp/mptcp_join.sh

I've tried sudo TERM=xterm-256color make kselftest ; sudo make kselftest | cat ;

but it didn't work.

Anyway, thanks a lot for the feedback, time and patience :D


Em qui., 13 de fev. de 2025 às 20:31, Bird, Tim <Tim.Bird@sony.com> escreveu:
>
> > -----Original Message-----
> > From: Marcelo Moreira <marcelomoreira1905@gmail.com>
> > This commit improves the readability of kselftest results by adding
> > ANSI color codes to the [ OK ] and FAIL messages.
> >
> > Signed-off-by: Marcelo Moreira <marcelomoreira1905@gmail.com>
> > ---
> >  tools/testing/selftests/net/mptcp/mptcp_lib.sh | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > index 051e289d7967..7b55855ff788 100644
> > --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > @@ -83,7 +83,7 @@ mptcp_lib_print_err() {
> >
> >  # shellcheck disable=SC2120 # parameters are optional
> >  mptcp_lib_pr_ok() {
> > -     mptcp_lib_print_ok "[ OK ]${1:+ ${*}}"
> > +     mptcp_lib_print_ok "\033[32m[ OK ]\033[0m${1:+ ${*}}"
> >  }
>
> This should not be done unconditionally, but based on the controlling
> and COLOR variables for this library.  You should also re-used the existing
> color print helper function.
>
> IOW, why is this not using MPTCP_LIB_COLOR_GREEN?
> or the helper function mptcp_lib_print_color()?
>
> >  mptcp_lib_pr_skip() {
> > @@ -97,7 +97,7 @@ mptcp_lib_pr_fail() {
> >               title="IGNO"
> >               cmt=" (flaky)"
> >       else
> > -             title="FAIL"
> > +             title="\033[31mFAIL\033[0m"
>
> Why not use the helper function and the MPTCP_LIB_COLOR_RED here?
>
>  -- Tim
>