)On Thu, Apr 7, 2022 at 11:18 PM David Gow <davidgow@xxxxxxxxxx> wrote: > > On Fri, Apr 8, 2022 at 11:59 AM 'Daniel Latypov' via KUnit Development > <kunit-dev@xxxxxxxxxxxxxxxx> wrote: > > > > On Thu, Apr 7, 2022 at 10:48 PM 'David Gow' via KUnit Development > > <kunit-dev@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxx> > > > --- > > > > > > This patch depends on: > > > https://lore.kernel.org/linux-kselftest/20220407223019.2066361-1-dlatypov@xxxxxxxxxx/ > > > > > > 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. Ack. My point was about the summary line. We have so few tests that only a handful statuses, that adding up 2-3 small numbers always felt simple enough. Esp. since the previous patch skips printing out the statues with 0s, that becomes even easier. But I'm not against having the total. I just personally find the current output looks very awkward and would prefer the status-quo over that specific output format. > > > > > 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, I think a reader can already easily note the difference between Ran 42 tests: 40 passed, 2 skipped. and Ran 42 tests: 20 passed, 20 failed, 2 skipped. I.e. the mere existence of a second or third number in the breakdown after "XX passed" feels like enough of an affordance. (This is perhaps naively assuming that tests are kept healthy) > 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. Ack. Let's see if Brendan or others on the list have a preference. If we want to go down that route, it might be easier if I combine this in the previous patch (https://lore.kernel.org/linux-kselftest/20220407223019.2066361-1-dlatypov@xxxxxxxxxx/ E.g. I get this output Ran 173 tests: passed: 137, skipped: 36 with a new combined patch of diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py index 807ed2bd6832..de1c0b7e14ed 100644 --- a/tools/testing/kunit/kunit_parser.py +++ b/tools/testing/kunit/kunit_parser.py @@ -94,11 +94,11 @@ class TestCounts: def __str__(self) -> str: """Returns the string representation of a TestCounts object. """ - return ('Passed: ' + str(self.passed) + - ', Failed: ' + str(self.failed) + - ', Crashed: ' + str(self.crashed) + - ', Skipped: ' + str(self.skipped) + - ', Errors: ' + str(self.errors)) + statuses = [('passed', self.passed), ('failed', self.failed), + ('crashed', self.crashed), ('skipped', self.skipped), + ('errors', self.errors)] + return f'Ran {self.total()} tests: ' + \ + ', '.join(f'{s}: {n}' for s, n in statuses if n > 0) def total(self) -> int: """Returns the total number of test cases within a test > > Cheers, > -- David
