2023-05-17 18:20:19

by Anup Sharma

[permalink] [raw]
Subject: [PATCH] perf: test: Add support for testing JSON generated by perf data command

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 <[email protected]>
---
.../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/tools/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=0
+perfdata=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
+result=$(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` != 0 ]; then
+ echo "[Skip] No root permission"
+ err=2
+ 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/null 2>&1
+ echo "Perf Data Converter to JSON [SUCCESS]"
+}
+
+validate_json_format()
+{
+ echo "Testing perf data converted to JSON format"
+ if [ -f "${result}" ]; then
+ if jq '.' "${result}" > /dev/null 2>&1; then
+ echo "The file contains valid JSON format [SUCCESS]"
+ else
+ echo "The file does not contain valid JSON format [FAILED]"
+ err=1
+ fi
+ else
+ echo "File not found [FAILED]"
+ err=2
+ exit
+ fi
+}
+
+check
+
+test_json
+validate_json_format
+
+exit ${err}
--
2.34.1



2023-05-17 20:05:34

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH] perf: test: Add support for testing JSON generated by perf data command

On Wed, May 17, 2023 at 11:09 AM Anup Sharma <[email protected]> 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 <[email protected]>

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/tools/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=0
> +perfdata=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
> +result=$(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` != 0 ]; then
> + echo "[Skip] No root permission"
> + err=2
> + 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/null 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 [FAILED]"
> + err=1
> + fi
> + else
> + echo "File not found [FAILED]"
> + err=2
> + exit
> + fi
> +}
> +
> +check
> +
> +test_json
> +validate_json_format
> +
> +exit ${err}
> --
> 2.34.1
>

2023-05-18 06:15:17

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH] perf: test: Add support for testing JSON generated by perf data command

Hello,

On Wed, May 17, 2023 at 12:57 PM Ian Rogers <[email protected]> wrote:
>
> On Wed, May 17, 2023 at 11:09 AM Anup Sharma <[email protected]> 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 <[email protected]>
>
> 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!

+1

>
> > ---
> > .../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/tools/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

You may want to run shellcheck when you write a shell script.

> > +
> > +set -e
> > +
> > +err=0
> > +perfdata=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
> > +result=$(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` != 0 ]; then
> > + echo "[Skip] No root permission"

I don't think we actually need root permission for this test.

> > + err=2
> > + 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

The -a/--system-wide option requires root permission usually.
We can run perf record in per-task mode instead. Please remove the -a
option and run some other command instead of 'sleep 1'. The perf tool
has a couple of test workloads and benchmark programs. So you can try
something like

perf record -o $perfdata -F 99 -g -- perf test -w noploop

or

perf record -o $perfdata -F 99 -g -- perf bench sched messaging

Thanks,
Namhyung


> > + perf data convert --to-json $result --force -i $perfdata >/dev/null 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 [FAILED]"
> > + err=1
> > + fi
> > + else
> > + echo "File not found [FAILED]"
> > + err=2
> > + exit
> > + fi
> > +}
> > +
> > +check
> > +
> > +test_json
> > +validate_json_format
> > +
> > +exit ${err}
> > --
> > 2.34.1
> >