mbox series

[v1,0/5] selftests/nolibc: report: print test status

Message ID cover.1688633188.git.falcon@tinylab.org (mailing list archive)
Headers show
Series selftests/nolibc: report: print test status | expand

Message

Zhangjin Wu July 6, 2023, 9:02 a.m. UTC
Hi, Willy

As you suggested, the 'status: [success|warning|failure]' info is added
to the summary line, with additional newlines around this line to
extrude the status info. at the same time, the total tests is printed,
the passed, skipped and failed values are aligned with '%03d'.

This patchset is based on 20230705-nolibc-series2 of nolibc repo[1].

The test result looks like:

    ...

    138 test(s): 135 passed, 002 skipped, 001 failed => status: failure

    See all results in /labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/run.out

Or:

    ...

    137 test(s): 134 passed, 003 skipped, 000 failed => status: warning

    See all results in /labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/run.out

Best regards,
Zhangjin
---
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/nolibc.git

Zhangjin Wu (5):
  selftests/nolibc: report: print a summarized test status
  selftests/nolibc: report: print total tests
  selftests/nolibc: report: align passed, skipped and failed
  selftests/nolibc: report: extrude the test status line
  selftests/nolibc: report: add newline before test failures

 tools/testing/selftests/nolibc/Makefile | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Willy Tarreau July 9, 2023, 8:58 a.m. UTC | #1
Hi Zhangjin,

On Thu, Jul 06, 2023 at 05:02:26PM +0800, Zhangjin Wu wrote:
> Hi, Willy
> 
> As you suggested, the 'status: [success|warning|failure]' info is added
> to the summary line, with additional newlines around this line to
> extrude the status info. at the same time, the total tests is printed,
> the passed, skipped and failed values are aligned with '%03d'.

So as I mentioned with some commits, I *do* find it important to
preserve the convenience of grepping for a single word to from 20
test reports at once and visually check all statuses (and in this
sense I like your preference for aligning the words to make them
more readable). But having to guess some grep context and see the
output garbled clearly does the opposite of what we were looking
for in my opinion. Also, I think there's no need for having 5
separate patches to add/remove a line feed. Better discuss an
output format that matches everyone's needs and change it at once,
this will make the patch more reviewable than having individual
changes like this.

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

> Hi Zhangjin,
> 
> On Thu, Jul 06, 2023 at 05:02:26PM +0800, Zhangjin Wu wrote:
> > Hi, Willy
> > 
> > As you suggested, the 'status: [success|warning|failure]' info is added
> > to the summary line, with additional newlines around this line to
> > extrude the status info. at the same time, the total tests is printed,
> > the passed, skipped and failed values are aligned with '%03d'.
> 
> So as I mentioned with some commits, I *do* find it important to
> preserve the convenience of grepping for a single word to from 20
> test reports at once and visually check all statuses (and in this
> sense I like your preference for aligning the words to make them
> more readable). But having to guess some grep context and see the
> output garbled clearly does the opposite of what we were looking
> for in my opinion.

Sorry for confusing you, hope my just reply [1] explained the 'newlines'
patch, as you pointed out in another reply, perhaps I need to write more
about the deeper 'background' idea of the patch, but sometimes, I'm also
worried about writing too much, for example, some info may be 'obvious'
but I spent too much statements, I will improve as possible as I can,
thanks.

[1]: https://lore.kernel.org/lkml/20230709192652.97668-1-falcon@tinylab.org/

> Also, I think there's no need for having 5
> separate patches to add/remove a line feed. Better discuss an
> output format that matches everyone's needs and change it at once,
> this will make the patch more reviewable than having individual
> changes like this.

That's right, the patches are split here is just for the last three are
new to our previous discuss, perhaps need more discuss, in the future, I
will propose the ideas before send the patches, just as we did for some
other patches.

Thanks,
Zhangjin

> 
> thanks,
> willy
Willy Tarreau July 10, 2023, 6:32 a.m. UTC | #3
Series finally queued as well with your explanation :-)

Thanks,
Willy