linux-kernel - Re: [PATCH] kunit: tool: Print a total count of tests.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABVgOSkKuHCkfpQhzNmzufaVw_6qO-Ve_t5oTub1JeweTiC9RQ@mail.gmail.com>
Date:   Fri, 8 Apr 2022 12:18:00 +0800
From:   David Gow <davidgow@...gle.com>
To:     Daniel Latypov <dlatypov@...gle.com>
Cc:     Brendan Higgins <brendanhiggins@...gle.com>,
        Shuah Khan <skhan@...uxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        KUnit Development <kunit-dev@...glegroups.com>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH] kunit: tool: Print a total count of tests.

On Fri, Apr 8, 2022 at 11:59 AM 'Daniel Latypov' via KUnit Development
<kunit-dev@...glegroups.com> wrote:
>
> On Thu, Apr 7, 2022 at 10:48 PM 'David Gow' via KUnit Development
> <kunit-dev@...glegroups.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@...gle.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

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4003 bytes)