[v1,4/5] selftests/nolibc: report: extrude the test status line

Message ID 89f3668f48d01fdac847bdfa085867cb641bad27.1688633188.git.falcon@tinylab.org
State New
Headers
Series selftests/nolibc: report: print test status |

Commit Message

Zhangjin Wu July 6, 2023, 9:11 a.m. UTC
  two newlines are added around the test summary line to extrude the test
status.

Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

Willy Tarreau July 9, 2023, 8:54 a.m. UTC | #1
On Thu, Jul 06, 2023 at 05:11:17PM +0800, Zhangjin Wu wrote:
> two newlines are added around the test summary line to extrude the test
> status.

But then we're back to making it annoying to check, having to figure
if we need to grep -A or grep -B etc. With grep 'status:' we would get
a synthetic status and the counters together. Why do you think it's
not convenient ? Or am I the only one considering it useful to just
run grep "status:" on all output files and figure a global status at
once ?

Willy
  
Zhangjin Wu July 9, 2023, 7:26 p.m. UTC | #2
Hi, Willy

> On Thu, Jul 06, 2023 at 05:11:17PM +0800, Zhangjin Wu wrote:
> > two newlines are added around the test summary line to extrude the test
> > status.
> 
> But then we're back to making it annoying to check, having to figure
> if we need to grep -A or grep -B etc. With grep 'status:' we would get
> a synthetic status and the counters together. Why do you think it's
> not convenient ? Or am I the only one considering it useful to just
> run grep "status:" on all output files and figure a global status at
> once ?

Sorry, Willy, my commit message may mislead you a little.

The newlines are added around the whole test summary line (with the
status info), not only around the 'status info' ;-)

An example is added in our cover-letter (use '%3d' instead of '%03d'
here):

    ...
                                                 <-- newline here -->
    138 test(s): 135 passed,   2 skipped,   1 failed => status: failure
                                                 <-- newline here -->
    See all results in /labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/run.out

Or:

    ...
                                                 <-- newline here -->
    137 test(s): 134 passed,   3 skipped,   0 failed => status: warning
                                                 <-- newline here -->
    See all results in /labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/run.out

It is not for status grep, it is for developers to easily see the whole
summary line at a glance (I should add this in the commit message),
especially when we run tests for lots of architectures one by one
automatically, during the tests running, these newlines may help us to
see the status at a glance.

And further, if not consider pure-text, the colors may be more helpful,
for example, red for failed/failure, yellow for skipped/warning, green
for passed/success, for example:

    $ echo | awk 'END{printf("138 test(s): \033[32m135\033[0m passed, \033[33m  2\033[0m skipped, \033[31m  1\033[0m failed => status: \033[31mfailure\033[0m\n");}'
    138 test(s): 135 passed,   2 skipped,   1 failed => status: failure

But as we can see, the color control code is not readable and it may
break the simple "status: failure" grep, we should use something like
"status: .*failure" ;-)

It is possible to filter out the color info in the last run.out and only
reserve the colors info in the console.

    $ cat run.tmp.out | sed 's/\x1b\[[0-9;]*m//g' | col -bp > run.out

As a summary, the "status info grep" you proposed is very helpful to
summarize all of the tests after the testing finish, I do like it:

    $ grep "status: " /path/to/all/run.out
    138 test(s): 135 passed,   2 skipped,   1 failed => status: failure
    137 test(s): 134 passed,   3 skipped,   0 failed => status: warning

And these newlines (and even further with colors) are added to help
developers to see what happens during the tests running at a glance.

Thanks,
Zhangjin

> 
> Willy
  
Willy Tarreau July 10, 2023, 6:30 a.m. UTC | #3
Hi Zhangjin,

On Mon, Jul 10, 2023 at 03:26:52AM +0800, Zhangjin Wu wrote:
> > On Thu, Jul 06, 2023 at 05:11:17PM +0800, Zhangjin Wu wrote:
> > > two newlines are added around the test summary line to extrude the test
> > > status.
> > 
> > But then we're back to making it annoying to check, having to figure
> > if we need to grep -A or grep -B etc. With grep 'status:' we would get
> > a synthetic status and the counters together. Why do you think it's
> > not convenient ? Or am I the only one considering it useful to just
> > run grep "status:" on all output files and figure a global status at
> > once ?
> 
> Sorry, Willy, my commit message may mislead you a little.
> 
> The newlines are added around the whole test summary line (with the
> status info), not only around the 'status info' ;-)

Ah OK, thanks for clarifying this!

> It is not for status grep, it is for developers to easily see the whole
> summary line at a glance

I understand but both work hand-in-hand, as every time you'll perform
a slight change, you'll necessarily rerun the whole series on all archs
to confirm, which is why I'm particularly annoying about the ability to
grep!

> And further, if not consider pure-text, the colors may be more helpful,
> for example, red for failed/failure, yellow for skipped/warning, green
> for passed/success, for example:
> 
>     $ echo | awk 'END{printf("138 test(s): \033[32m135\033[0m passed, \033[33m  2\033[0m skipped, \033[31m  1\033[0m failed => status: \033[31mfailure\033[0m\n");}'
>     138 test(s): 135 passed,   2 skipped,   1 failed => status: failure
> 
> But as we can see, the color control code is not readable and it may
> break the simple "status: failure" grep, we should use something like
> "status: .*failure" ;-)

Colors may only be used when stdout is a terminal, and still, some might
find it annonying (for example some distros use unreadably dark colors
that were apparently never tested over a black background, forcing users
to highlight the text by selecting it with the mouse to read it). Better
not start to play with this IMO, that's not really needed and may be more
annoying to some than helpful to most.

Thanks,
Willy
  

Patch

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 2c53bf41967b..10e9e5c1bdd0 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -85,9 +85,9 @@  CFLAGS  ?= -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 \
 LDFLAGS := -s
 
 REPORT  ?= awk '/\[OK\][\r]*$$/{p++} /\[FAIL\][\r]*$$/{f++;print} /\[SKIPPED\][\r]*$$/{s++} \
-		END{ printf("%03d test(s): %03d passed, %03d skipped, %03d failed => status: ", p+s+f, p, s, f); \
+		END{ printf("\n%03d test(s): %03d passed, %03d skipped, %03d failed => status: ", p+s+f, p, s, f); \
 		if (f) printf("failure\n"); else if (s) printf("warning\n"); else printf("success\n");; \
-		printf("See all results in %s\n", ARGV[1]); }'
+		printf("\nSee all results in %s\n", ARGV[1]); }'
 
 help:
 	@echo "Supported targets under selftests/nolibc:"