| Message ID | 20250213212858.710884-1-marcelomoreira1905@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Adds colors to kselftest output | expand |
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
>
> -----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
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 >
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:+ ${*}}"
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(-)