Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <ZGUYVjJk2DhX9UrC@yoga>
In-Reply-To: <ZGUYVjJk2DhX9UrC@yoga>
From:   Ian Rogers <irogers@google.com>
Date:   Wed, 17 May 2023 12:56:52 -0700
Message-ID: <CAP-5=fX3oLRgUzt6QhN+6bybp_WsS9gCuJ1484hq16FTWc3Dyg@mail.gmail.com>
Subject: Re: [PATCH] perf: test: Add support for testing JSON generated by
 perf data command
To:     Anup Sharma <anupnewsmail@gmail.com>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Adrian Hunter <adrian.hunter@intel.com>,
        linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, May 17, 2023 at 11:09=E2=80=AFAM Anup Sharma <anupnewsmail@gmail.co=
m> wrote:
>
> This commit adds support for testing the JSON output generated
> by the perf data command's conversion to JSON functionality.
> The test script now includes a validation step to ensure that
> the resulting JSON file is contain valid data.
>
> As a newcomer to this community, any feedback on the implementation
> is highly appreciated.
>
> Signed-off-by: Anup Sharma <anupnewsmail@gmail.com>

Congratulations on sending your first patch! Anup is this summer's
Google Summer-of-Code contributor for the Linux perf tool. I'm sure
everyone on the lists is looking forward to your contributions!

> ---
>  .../shell/test_perf_data_converter_json.sh    | 64 +++++++++++++++++++
>  1 file changed, 64 insertions(+)
>  create mode 100755 tools/perf/tests/shell/test_perf_data_converter_json.=
sh
>
> diff --git a/tools/perf/tests/shell/test_perf_data_converter_json.sh b/to=
ols/perf/tests/shell/test_perf_data_converter_json.sh
> new file mode 100755
> index 000000000000..88db96e38925
> --- /dev/null
> +++ b/tools/perf/tests/shell/test_perf_data_converter_json.sh
> @@ -0,0 +1,64 @@
> +#!/bin/sh
> +# perf data json converter test
> +# SPDX-License-Identifier: GPL-2.0
> +
> +set -e
> +
> +err=3D0
> +perfdata=3D$(mktemp /tmp/__perf_test.perf.data.XXXXX)
> +result=3D$(mktemp /tmp/__perf_test.output.json.XXXXX)
> +
> +cleanup()
> +{
> +       rm -f ${perfdata}
> +       rm -f ${result}
> +       trap - exit term int
> +}
> +
> +trap_cleanup()
> +{
> +       cleanup
> +       exit ${err}
> +}
> +trap trap_cleanup exit term int
> +
> +check()
> +{
> +       if [ `id -u` !=3D 0 ]; then
> +               echo "[Skip] No root permission"
> +               err=3D2
> +               exit
> +       fi
> +}
> +
> +test_json()
> +{
> +       echo "Testing perf data convertion to JSON"
> +       perf record -o $perfdata -F 99 -a -g -- sleep 1 > /dev/null 2>&1
> +       perf data convert --to-json $result --force -i $perfdata >/dev/nu=
ll 2>&1
> +       echo "Perf Data Converter to JSON [SUCCESS]"
> +}
> +
> +validate_json_format()
> +{
> +       echo "Testing perf data converted to JSON format"
> +       if [ -f "${result}" ]; then

It can be a good idea to test for failure and early exit. It would
avoid being as indented in the following if.

> +               if jq '.' "${result}" > /dev/null 2>&1; then

This assumes the jq program is available and it isn't by default on my
distribution. We could test:
if ! which jq; then
but it may be better to depend on Python and json.loads - Python is
already a dependency for a number of the tests.

Thanks,
Ian

> +                   echo "The file contains valid JSON format [SUCCESS]"
> +               else
> +                   echo "The file does not contain valid JSON format [FA=
ILED]"
> +                   err=3D1
> +               fi
> +           else
> +               echo "File not found [FAILED]"
> +               err=3D2
> +               exit
> +           fi
> +}
> +
> +check
> +
> +test_json
> +validate_json_format
> +
> +exit ${err}
> --
> 2.34.1
>