diff mbox series

kunit: tool: Print a total count of tests.

Message ID 20220408034848.2081355-1-davidgow@google.com
State New
Headers show
Series kunit: tool: Print a total count of tests. | expand

Commit Message

David Gow April 8, 2022, 3:48 a.m. UTC
Add a count of the total number of tests run (including skipped tests,
which do run a little bit until they decide to skip themselves) to the
summary line.

Signed-off-by: David Gow <davidgow@google.com>
---

This patch depends on:
https://lore.kernel.org/linux-kselftest/20220407223019.2066361-1-dlatypov@google.com/

 tools/testing/kunit/kunit_parser.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Daniel Latypov April 8, 2022, 3:59 a.m. UTC | #1
On Thu, Apr 7, 2022 at 10:48 PM 'David Gow' via KUnit Development
<kunit-dev@googlegroups.com> wrote:
>
> Add a count of the total number of tests run (including skipped tests,
> which do run a little bit until they decide to skip themselves) to the
> summary line.
>
> Signed-off-by: David Gow <davidgow@google.com>
> ---
>
> This patch depends on:
> https://lore.kernel.org/linux-kselftest/20220407223019.2066361-1-dlatypov@google.com/
>
>  tools/testing/kunit/kunit_parser.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
> index 957907105429..da01998d29b1 100644
> --- a/tools/testing/kunit/kunit_parser.py
> +++ b/tools/testing/kunit/kunit_parser.py
> @@ -96,7 +96,7 @@ class TestCounts:
>                 """
>                 statuses = [('Passed', self.passed), ('Failed', self.failed),
>                         ('Crashed', self.crashed), ('Skipped', self.skipped),
> -                       ('Errors', self.errors)]
> +                       ('Errors', self.errors), ('Total', self.total())]

Hmm, I've never really felt the need for a total to be printed out.
We've had few enough tests and different statuses that the mental
addition is easy enough.

Bikeshedding:
This current output of
  Passed: 40, Skipped: 2, Total: 42
feels a bit awkward to me.
If we did print one out, I think it should probably go first, e.g.
  Ran 42 tests: 40 passed, 2 skipped.

Wdyt?

>                 return ', '.join('{}: {}'.format(s, n) for s, n in statuses if n > 0)
David Gow April 8, 2022, 4:18 a.m. UTC | #2
On Fri, Apr 8, 2022 at 11:59 AM 'Daniel Latypov' via KUnit Development
<kunit-dev@googlegroups.com> wrote:
>
> On Thu, Apr 7, 2022 at 10:48 PM 'David Gow' via KUnit Development
> <kunit-dev@googlegroups.com> wrote:
> >
> > Add a count of the total number of tests run (including skipped tests,
> > which do run a little bit until they decide to skip themselves) to the
> > summary line.
> >
> > Signed-off-by: David Gow <davidgow@google.com>
> > ---
> >
> > This patch depends on:
> > https://lore.kernel.org/linux-kselftest/20220407223019.2066361-1-dlatypov@google.com/
> >
> >  tools/testing/kunit/kunit_parser.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
> > index 957907105429..da01998d29b1 100644
> > --- a/tools/testing/kunit/kunit_parser.py
> > +++ b/tools/testing/kunit/kunit_parser.py
> > @@ -96,7 +96,7 @@ class TestCounts:
> >                 """
> >                 statuses = [('Passed', self.passed), ('Failed', self.failed),
> >                         ('Crashed', self.crashed), ('Skipped', self.skipped),
> > -                       ('Errors', self.errors)]
> > +                       ('Errors', self.errors), ('Total', self.total())]
>
> Hmm, I've never really felt the need for a total to be printed out.
> We've had few enough tests and different statuses that the mental
> addition is easy enough.

It's useful just often enough as a sanity check (were those failures /
skipped tests fixed, or did we just stop running them): having one
number to check for "did some more tests run at all" is quite
convenient. Particularly when dealing with nasty dependency chains and
"all tests" builds.

This is also particularly useful when running on setups where
scrollback is more of a pain, as the summary line is absolutely
invaluable there.

>
> Bikeshedding:
> This current output of
>   Passed: 40, Skipped: 2, Total: 42
> feels a bit awkward to me.
> If we did print one out, I think it should probably go first, e.g.
>   Ran 42 tests: 40 passed, 2 skipped.
>
> Wdyt?

I personally don't find having "Total" at the end awkward -- putting
the sum at the end has been done on ledgers for years -- but do admit
it's even more convenient to have it first (so it's at the same place
on the screen every run, regardless of the rest). So "Ran 42 tests: 40
passed..." would emphasise the "total" over the "passed" count here.
Personally, I think that's probably a good thing: I think what most
people really want at a glance is effectively "Failed / Total" (or,
more realistically a sort-of "Problems / Total", where problems is
Failed + Error). But the combination of the colour (did it pass
overall) and the total are the things I'd usually want to look for
first.

So, tl;dr: I'd be all for the "Ran n tests: a passed, b failed, etc" wording.

Cheers,
-- David
diff mbox series

Patch

diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
index 957907105429..da01998d29b1 100644
--- a/tools/testing/kunit/kunit_parser.py
+++ b/tools/testing/kunit/kunit_parser.py
@@ -96,7 +96,7 @@  class TestCounts:
 		"""
 		statuses = [('Passed', self.passed), ('Failed', self.failed),
 			('Crashed', self.crashed), ('Skipped', self.skipped),
-			('Errors', self.errors)]
+			('Errors', self.errors), ('Total', self.total())]
 		return ', '.join('{}: {}'.format(s, n) for s, n in statuses if n > 0)
 
 	def total(self) -> int: