linux-kernel - Re: [PATCH 2/3] kunit: tool: make parser stop overwriting status of suites w/ no

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 28 Apr 2022 15:11:37 +0800
From:   David Gow <davidgow@...gle.com>
To:     Daniel Latypov <dlatypov@...gle.com>
Cc:     Brendan Higgins <brendanhiggins@...gle.com>,
        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>,
        Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH 2/3] kunit: tool: make parser stop overwriting status of
 suites w/ no_tests

On Wed, Apr 27, 2022 at 1:33 AM Daniel Latypov <dlatypov@...gle.com> wrote:
>
> Consider this invocation
> $ ./tools/testing/kunit/kunit.py parse <<EOF
>   TAP version 14
>   1..2
>   ok 1 - suite
>     # Subtest: no_tests_suite
>     # catastrophic error!
>   not ok 1 - no_tests_suite
> EOF
>
> It will have a 0 exit code even though there's a "not ok".
>
> Consider this one:
> $ ./tools/testing/kunit/kunit.py parse <<EOF
>   TAP version 14
>   1..2
>   ok 1 - suite
>   not ok 1 - no_tests_suite
> EOF
>
> It will a non-zero exit code.
>
> Why?
> We have this line in the kunit_parser.py
> > parent_test = parse_test_header(lines, test)
> where we have special handling when we see "# Subtest" and we ignore the
> explicit reported "not ok 1" status!
>
> Also, NO_TESTS at a suite-level only results in a non-zero status code
> where then there's only one suite atm.
>
> This change is the minimal one to make sure we don't overwrite it.
>
> Signed-off-by: Daniel Latypov <dlatypov@...gle.com>
> ---

This seems sensible to me, though it doesn't change a lot in practice
given that the in-kernel KUnit will mark empty suites as skipped:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/kunit/test.c#n180

(And I think that is probably the correct thing for it to do, as "no
tests run" seems closer to skipped than to passed or failed, which are
the other options KTAP gives us.)

That being said, it's definitely true that we don't want to override a
"not ok" needlessly in kunit_tool, so this patch is definite
improvement.

Reviewed-by: David Gow <davidgow@...gle.com>


-- David

>  tools/testing/kunit/kunit_parser.py                        | 7 +++++--
>  .../test_data/test_is_test_passed-no_tests_no_plan.log     | 2 +-
>  2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
> index 7a0faf527a98..45c2c5837281 100644
> --- a/tools/testing/kunit/kunit_parser.py
> +++ b/tools/testing/kunit/kunit_parser.py
> @@ -775,8 +775,11 @@ def parse_test(lines: LineStream, expected_num: int, log: List[str]) -> Test:
>
>         # Check for there being no tests
>         if parent_test and len(subtests) == 0:
> -               test.status = TestStatus.NO_TESTS
> -               test.add_error('0 tests run!')
> +               # Don't override a bad status if this test had one reported.
> +               # Assumption: no subtests means CRASHED is from Test.__init__()
> +               if test.status in (TestStatus.TEST_CRASHED, TestStatus.SUCCESS):
> +                       test.status = TestStatus.NO_TESTS
> +                       test.add_error('0 tests run!')
>
>         # Add statuses to TestCounts attribute in Test object
>         bubble_up_test_results(test)
> diff --git a/tools/testing/kunit/test_data/test_is_test_passed-no_tests_no_plan.log b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_no_plan.log
> index dd873c981108..4f81876ee6f1 100644
> --- a/tools/testing/kunit/test_data/test_is_test_passed-no_tests_no_plan.log
> +++ b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_no_plan.log
> @@ -3,5 +3,5 @@ TAP version 14
>    # Subtest: suite
>    1..1
>      # Subtest: case
> -  ok 1 - case # SKIP
> +  ok 1 - case
>  ok 1 - suite
> --
> 2.36.0.rc2.479.g8af0fa9b8e-goog
>

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