linux-kernel - Re: [RFC PATCH kunit-next] kunit: kunit_tool: Separate out config/build/exec/parse

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] [day] [month] [year] [list]

Message-ID: <CAFd5g47c+g_ufcOKEkKtCtj71nQtFwp40JuWajh_jPhQt50B_w@mail.gmail.com>
Date:   Wed, 25 Mar 2020 11:09:57 -0700
From:   Brendan Higgins <brendanhiggins@...gle.com>
To:     David Gow <davidgow@...gle.com>
Cc:     "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>,
        KUnit Development <kunit-dev@...glegroups.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH kunit-next] kunit: kunit_tool: Separate out config/build/exec/parse

On Wed, Mar 11, 2020 at 3:23 PM David Gow <davidgow@...gle.com> wrote:
>
> Add new subcommands to kunit.py to allow stages of the existing 'run'
> subcommand to be run independently:
> - 'config': Verifies that .config is a subset of .kunitconfig
> - 'build': Compiles a UML kernel for KUnit
> - 'exec': Runs the kernel, and outputs the test results.
> - 'parse': Parses test results from a file or stdin

I think all the names are fine. I like the verb-noun pattern.

>
> Most of these are not hugely useful by themselves yet, and there's room
> for plenty of bikeshedding on the names, but this hopefully can form a
> foundation for future improvements.
>
> Signed-off-by: David Gow <davidgow@...gle.com>

This looks really good! I really only have one minor comment right
now, see below.

Reviewed-by: Brendan Higgins <brendanhiggins@...gle.com>

> ---
> [Whoops: typo-ed Brendan's email. Sorry about that!]
>
> As was briefly disccussed in [1], this change is part of a "separation
> of concerns" in kunit_tool. This should make it easier to integrate
> kunit_tool into other setups.
>
> Of particular intrest is probably 'kunit.py parse', which should allow
> KUnit results to be parsed from other sources, such as after loading a
> module, or from a non-UML kernel, or from debugfs when that's
> supported[2].
>
> [1]: https://lkml.org/lkml/2020/2/5/552
> [2]: https://patchwork.kernel.org/patch/11419901/
>
>  tools/testing/kunit/kunit.py           | 236 ++++++++++++++++++++-----
>  tools/testing/kunit/kunit_tool_test.py |  55 ++++++
>  2 files changed, 242 insertions(+), 49 deletions(-)
>
> diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> index 180ad1e1b04f..92a634594cf6 100755
> --- a/tools/testing/kunit/kunit.py
> +++ b/tools/testing/kunit/kunit.py

[...]

> @@ -147,6 +244,47 @@ def main(argv, linux=None):
>                 result = run_tests(linux, request)
>                 if result.status != KunitStatus.SUCCESS:
>                         sys.exit(1)
> +       elif cli_args.subcommand == 'config':
> +               request = KunitConfigRequest(cli_args.build_dir,
> +                                            cli_args.defconfig)
> +               result = config_tests(linux, request)
> +               kunit_parser.print_with_timestamp((
> +                       'Elapsed time: %.3fs\n') % (
> +                               result.elapsed_time))
> +               if result.status != KunitStatus.SUCCESS:
> +                       sys.exit(1)
> +       elif cli_args.subcommand == 'build':
> +               request = KunitBuildRequest(cli_args.jobs,
> +                                           cli_args.build_dir)
> +               result = build_tests(linux, request)
> +               kunit_parser.print_with_timestamp((
> +                       'Elapsed time: %.3fs\n') % (
> +                               result.elapsed_time))
> +               if result.status != KunitStatus.SUCCESS:
> +                       sys.exit(1)
> +       elif cli_args.subcommand == 'exec':
> +               exec_request = KunitExecRequest(cli_args.timeout,
> +                                               cli_args.build_dir)
> +               exec_result = exec_tests(linux, exec_request)
> +               parse_request = KunitParseRequest(cli_args.raw_output,
> +                                                 exec_result.result)
> +               result = parse_tests(parse_request)
> +               kunit_parser.print_with_timestamp((
> +                       'Elapsed time: %.3fs\n') % (
> +                               exec_result.elapsed_time))
> +               if result.status != KunitStatus.SUCCESS:
> +                       sys.exit(1)
> +       elif cli_args.subcommand == 'parse':
> +               if cli_args.file == '-':
> +                       kunit_output = sys.stdin

Could you make it so parse accepts the dmesg log from stdin if no file
is specified instead of a '-'?

> +               else:
> +                       with open(cli_args.file, 'r') as f:
> +                               kunit_output = f.read().splitlines()
> +               request = KunitParseRequest(cli_args.raw_output,
> +                                           kunit_output)
> +               result = parse_tests(request)
> +               if result.status != KunitStatus.SUCCESS:
> +                       sys.exit(1)
>         else:
>                 parser.print_help()
>

[...]