Re: [PATCH] tools/testing/kunit/kunit.py extract handlers

[David Gow]

On Sun, 22 Jan 2023 at 05:27, Alexander Pantyukhin <apantykhin@xxxxxxxxx> wrote:
>
> The main function contains a wide if-elif block that handles different
> subcommands. It's possible to make code refactoring to extract
> subcommands handlers.
>
> Signed-off-by: Alexander Pantyukhin <apantykhin@xxxxxxxxx>
> ---

Thanks -- this is a much nicer way of doing things!

Reviewed-by: David Gow <davidgow@xxxxxxxxxx>

Cheers,
-- David

>  tools/testing/kunit/kunit.py | 167 ++++++++++++++++++++---------------
>  1 file changed, 96 insertions(+), 71 deletions(-)
>
> diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> index 43fbe96318fe..8cd818867504 100755
> --- a/tools/testing/kunit/kunit.py
> +++ b/tools/testing/kunit/kunit.py
> @@ -395,6 +395,95 @@ def tree_from_args(cli_args: argparse.Namespace) -> kunit_kernel.LinuxSourceTree
>                         extra_qemu_args=qemu_args)
>
>
> +def run_handler(cli_args):
> +       if not os.path.exists(cli_args.build_dir):
> +               os.mkdir(cli_args.build_dir)
> +
> +       linux = tree_from_args(cli_args)
> +       request = KunitRequest(build_dir=cli_args.build_dir,
> +                                       make_options=cli_args.make_options,
> +                                       jobs=cli_args.jobs,
> +                                       raw_output=cli_args.raw_output,
> +                                       json=cli_args.json,
> +                                       timeout=cli_args.timeout,
> +                                       filter_glob=cli_args.filter_glob,
> +                                       kernel_args=cli_args.kernel_args,
> +                                       run_isolated=cli_args.run_isolated)
> +       result = run_tests(linux, request)
> +       if result.status != KunitStatus.SUCCESS:
> +               sys.exit(1)
> +
> +
> +def config_handler(cli_args):
> +       if cli_args.build_dir and (
> +                       not os.path.exists(cli_args.build_dir)):
> +               os.mkdir(cli_args.build_dir)
> +
> +       linux = tree_from_args(cli_args)
> +       request = KunitConfigRequest(build_dir=cli_args.build_dir,
> +                                               make_options=cli_args.make_options)
> +       result = config_tests(linux, request)
> +       stdout.print_with_timestamp((
> +               'Elapsed time: %.3fs\n') % (
> +                       result.elapsed_time))
> +       if result.status != KunitStatus.SUCCESS:
> +               sys.exit(1)
> +
> +
> +def build_handler(cli_args):
> +       linux = tree_from_args(cli_args)
> +       request = KunitBuildRequest(build_dir=cli_args.build_dir,
> +                                       make_options=cli_args.make_options,
> +                                       jobs=cli_args.jobs)
> +       result = config_and_build_tests(linux, request)
> +       stdout.print_with_timestamp((
> +               'Elapsed time: %.3fs\n') % (
> +                       result.elapsed_time))
> +       if result.status != KunitStatus.SUCCESS:
> +               sys.exit(1)
> +
> +
> +def exec_handler(cli_args):
> +       linux = tree_from_args(cli_args)
> +       exec_request = KunitExecRequest(raw_output=cli_args.raw_output,
> +                                       build_dir=cli_args.build_dir,
> +                                       json=cli_args.json,
> +                                       timeout=cli_args.timeout,
> +                                       filter_glob=cli_args.filter_glob,
> +                                       kernel_args=cli_args.kernel_args,
> +                                       run_isolated=cli_args.run_isolated)
> +       result = exec_tests(linux, exec_request)
> +       stdout.print_with_timestamp((
> +               'Elapsed time: %.3fs\n') % (result.elapsed_time))
> +       if result.status != KunitStatus.SUCCESS:
> +               sys.exit(1)
> +
> +
> +def parse_handler(cli_args):
> +       if cli_args.file is None:
> +               sys.stdin.reconfigure(errors='backslashreplace')  # pytype: disable=attribute-error
> +               kunit_output = sys.stdin
> +       else:
> +               with open(cli_args.file, 'r', errors='backslashreplace') as f:
> +                       kunit_output = f.read().splitlines()
> +       # We know nothing about how the result was created!
> +       metadata = kunit_json.Metadata()
> +       request = KunitParseRequest(raw_output=cli_args.raw_output,
> +                                       json=cli_args.json)
> +       result, _ = parse_tests(request, metadata, kunit_output)
> +       if result.status != KunitStatus.SUCCESS:
> +               sys.exit(1)
> +
> +
> +subcommand_handlers_map = {
> +       'run': run_handler,
> +       'config': config_handler,
> +       'build': build_handler,
> +       'exec': exec_handler,
> +       'parse': parse_handler
> +}
> +
> +
>  def main(argv):
>         parser = argparse.ArgumentParser(
>                         description='Helps writing and running KUnit tests.')
> @@ -438,78 +527,14 @@ def main(argv):
>         if get_kernel_root_path():
>                 os.chdir(get_kernel_root_path())
>
> -       if cli_args.subcommand == 'run':
> -               if not os.path.exists(cli_args.build_dir):
> -                       os.mkdir(cli_args.build_dir)
> -
> -               linux = tree_from_args(cli_args)
> -               request = KunitRequest(build_dir=cli_args.build_dir,
> -                                      make_options=cli_args.make_options,
> -                                      jobs=cli_args.jobs,
> -                                      raw_output=cli_args.raw_output,
> -                                      json=cli_args.json,
> -                                      timeout=cli_args.timeout,
> -                                      filter_glob=cli_args.filter_glob,
> -                                      kernel_args=cli_args.kernel_args,
> -                                      run_isolated=cli_args.run_isolated)
> -               result = run_tests(linux, request)
> -               if result.status != KunitStatus.SUCCESS:
> -                       sys.exit(1)
> -       elif cli_args.subcommand == 'config':
> -               if cli_args.build_dir and (
> -                               not os.path.exists(cli_args.build_dir)):
> -                       os.mkdir(cli_args.build_dir)
> -
> -               linux = tree_from_args(cli_args)
> -               request = KunitConfigRequest(build_dir=cli_args.build_dir,
> -                                            make_options=cli_args.make_options)
> -               result = config_tests(linux, request)
> -               stdout.print_with_timestamp((
> -                       'Elapsed time: %.3fs\n') % (
> -                               result.elapsed_time))
> -               if result.status != KunitStatus.SUCCESS:
> -                       sys.exit(1)
> -       elif cli_args.subcommand == 'build':
> -               linux = tree_from_args(cli_args)
> -               request = KunitBuildRequest(build_dir=cli_args.build_dir,
> -                                           make_options=cli_args.make_options,
> -                                           jobs=cli_args.jobs)
> -               result = config_and_build_tests(linux, request)
> -               stdout.print_with_timestamp((
> -                       'Elapsed time: %.3fs\n') % (
> -                               result.elapsed_time))
> -               if result.status != KunitStatus.SUCCESS:
> -                       sys.exit(1)
> -       elif cli_args.subcommand == 'exec':
> -               linux = tree_from_args(cli_args)
> -               exec_request = KunitExecRequest(raw_output=cli_args.raw_output,
> -                                               build_dir=cli_args.build_dir,
> -                                               json=cli_args.json,
> -                                               timeout=cli_args.timeout,
> -                                               filter_glob=cli_args.filter_glob,
> -                                               kernel_args=cli_args.kernel_args,
> -                                               run_isolated=cli_args.run_isolated)
> -               result = exec_tests(linux, exec_request)
> -               stdout.print_with_timestamp((
> -                       'Elapsed time: %.3fs\n') % (result.elapsed_time))
> -               if result.status != KunitStatus.SUCCESS:
> -                       sys.exit(1)
> -       elif cli_args.subcommand == 'parse':
> -               if cli_args.file is None:
> -                       sys.stdin.reconfigure(errors='backslashreplace')  # pytype: disable=attribute-error
> -                       kunit_output = sys.stdin
> -               else:
> -                       with open(cli_args.file, 'r', errors='backslashreplace') as f:
> -                               kunit_output = f.read().splitlines()
> -               # We know nothing about how the result was created!
> -               metadata = kunit_json.Metadata()
> -               request = KunitParseRequest(raw_output=cli_args.raw_output,
> -                                           json=cli_args.json)
> -               result, _ = parse_tests(request, metadata, kunit_output)
> -               if result.status != KunitStatus.SUCCESS:
> -                       sys.exit(1)
> -       else:
> +       subcomand_handler = subcommand_handlers_map.get(cli_args.subcommand, None)
> +
> +       if subcomand_handler is None:
>                 parser.print_help()
> +               return
> +
> +       subcomand_handler(cli_args)
> +
>
>  if __name__ == '__main__':
>         main(sys.argv[1:])
> --
> 2.25.1
>

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature