Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20220118190922.1557074-1-dlatypov@google.com> <20220118190922.1557074-2-dlatypov@google.com>
In-Reply-To: <20220118190922.1557074-2-dlatypov@google.com>
From:   David Gow <davidgow@google.com>
Date:   Thu, 20 Jan 2022 16:29:40 +0800
Message-ID: <CABVgOSmkfq0K_yNXzo82cxVCUz=-vSeT9qp2fkFo6=0Gc3wujg@mail.gmail.com>
Subject: Re: [PATCH 2/5] kunit: tool: make --json handling a bit clearer
To:     Daniel Latypov <dlatypov@google.com>
Cc:     Brendan Higgins <brendanhiggins@google.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        KUnit Development <kunit-dev@googlegroups.com>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@vger.kernel.org>,
        Shuah Khan <skhan@linuxfoundation.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Jan 19, 2022 at 3:09 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> Currently kunit_json.get_json_result() will output the JSON-ified test
> output to json_path, but iff it's not "stdout".
>
> Instead, move the responsibility entirely over to the one caller.
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
> ---

This looks good to me.

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

Cheers,
-- David


>  tools/testing/kunit/kunit.py           | 12 ++++++++----
>  tools/testing/kunit/kunit_json.py      | 12 ++----------
>  tools/testing/kunit/kunit_tool_test.py |  3 +--
>  3 files changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> index 9274c6355809..bd2f7f088c72 100755
> --- a/tools/testing/kunit/kunit.py
> +++ b/tools/testing/kunit/kunit.py
> @@ -216,13 +216,17 @@ def parse_tests(request: KunitParseRequest, input_data: Iterable[str]) -> Tuple[
>         parse_end = time.time()
>
>         if request.json:
> -               json_obj = kunit_json.get_json_result(
> +               json_str = kunit_json.get_json_result(
>                                         test=test_result,
>                                         def_config='kunit_defconfig',
> -                                       build_dir=request.build_dir,
> -                                       json_path=request.json)
> +                                       build_dir=request.build_dir)
>                 if request.json == 'stdout':
> -                       print(json_obj)
> +                       print(json_str)
> +               else:
> +                       with open(request.json, 'w') as f:
> +                               f.write(json_str)
> +                       kunit_parser.print_with_timestamp("Test results stored in %s" %
> +                               os.path.abspath(request.json))
>
>         if test_result.status != kunit_parser.TestStatus.SUCCESS:
>                 return KunitResult(KunitStatus.TEST_FAILURE, parse_end - parse_start), test_result
> diff --git a/tools/testing/kunit/kunit_json.py b/tools/testing/kunit/kunit_json.py
> index 6862671709bc..61091878f51e 100644
> --- a/tools/testing/kunit/kunit_json.py
> +++ b/tools/testing/kunit/kunit_json.py
> @@ -51,15 +51,7 @@ def _get_group_json(test: Test, def_config: str,
>         return test_group
>
>  def get_json_result(test: Test, def_config: str,
> -               build_dir: Optional[str], json_path: str) -> str:
> +               build_dir: Optional[str]) -> str:
>         test_group = _get_group_json(test, def_config, build_dir)
>         test_group["name"] = "KUnit Test Group"
> -       json_obj = json.dumps(test_group, indent=4)
> -       if json_path != 'stdout':
> -               with open(json_path, 'w') as result_path:
> -                       result_path.write(json_obj)
> -               root = __file__.split('tools/testing/kunit/')[0]
> -               kunit_parser.print_with_timestamp(
> -                       "Test results stored in %s" %
> -                       os.path.join(root, result_path.name))
> -       return json_obj
> +       return json.dumps(test_group, indent=4)
> diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
> index 352369dffbd9..f7cbc248a405 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -469,8 +469,7 @@ class KUnitJsonTest(unittest.TestCase):
>                         json_obj = kunit_json.get_json_result(
>                                 test=test_result,
>                                 def_config='kunit_defconfig',
> -                               build_dir=None,
> -                               json_path='stdout')
> +                               build_dir=None)
>                 return json.loads(json_obj)
>
>         def test_failed_test_json(self):
> --
> 2.34.1.703.g22d0c6ccf7-goog
>