perf test: Fix perf stat JSON output test

Message ID 20230524210600.3095830-1-namhyung@kernel.org
State New
Headers
Series perf test: Fix perf stat JSON output test |

Commit Message

Namhyung Kim May 24, 2023, 9:06 p.m. UTC
  The recent --per-cache option test caused a problem.  According to
the option name, I think it should check args.per_cache instead of
args.per_cache_instance.

  $ sudo ./perf test -v 99
   99: perf stat JSON output linter                                    :
  --- start ---
  test child forked, pid 3086101
  Checking json output: no args [Success]
  Checking json output: system wide [Success]
  Checking json output: interval [Success]
  Checking json output: event [Success]
  Checking json output: per thread [Success]
  Checking json output: per node [Success]
  Checking json output: system wide no aggregation [Success]
  Checking json output: per core [Success]
  Checking json output: per cache_instance Test failed for input:
  ...
  Traceback (most recent call last):
    File "linux/tools/perf/tests/shell/lib/perf_json_output_lint.py", line 88, in <module>
      elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache_instance:
  AttributeError: 'Namespace' object has no attribute 'per_cache_instance'
  test child finished with -1
  ---- end ----
  perf stat JSON output linter: FAILED!

Fixes: bfce728db317 ("pert tests: Add tests for new "perf stat --per-cache" aggregation option")
Cc: K Prateek Nayak <kprateek.nayak@amd.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/tests/shell/lib/perf_json_output_lint.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

K Prateek Nayak May 25, 2023, 3:01 a.m. UTC | #1
Hello Namhyung,

On 5/25/2023 2:36 AM, Namhyung Kim wrote:
> The recent --per-cache option test caused a problem.  According to
> the option name, I think it should check args.per_cache instead of
> args.per_cache_instance.
> 
>   $ sudo ./perf test -v 99
>    99: perf stat JSON output linter                                    :
>   --- start ---
>   test child forked, pid 3086101
>   Checking json output: no args [Success]
>   Checking json output: system wide [Success]
>   Checking json output: interval [Success]
>   Checking json output: event [Success]
>   Checking json output: per thread [Success]
>   Checking json output: per node [Success]
>   Checking json output: system wide no aggregation [Success]
>   Checking json output: per core [Success]
>   Checking json output: per cache_instance Test failed for input:
>   ...
>   Traceback (most recent call last):
>     File "linux/tools/perf/tests/shell/lib/perf_json_output_lint.py", line 88, in <module>
>       elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache_instance:
>   AttributeError: 'Namespace' object has no attribute 'per_cache_instance'
>   test child finished with -1
>   ---- end ----
>   perf stat JSON output linter: FAILED!
> 
> Fixes: bfce728db317 ("pert tests: Add tests for new "perf stat --per-cache" aggregation option")

Another oversight on my part. Thank you for fixing this :)

   $ sudo perf test -v 99
   99: perf stat JSON output linter                                    :
   --- start ---
   test child forked, pid 25046
   Checking json output: no args [Success]
   Checking json output: system wide [Success]
   Checking json output: interval [Success]
   Checking json output: event [Success]
   Checking json output: per thread [Success]
   Checking json output: per node [Success]
   Checking json output: system wide no aggregation [Success]
   Checking json output: per core [Success]
   Checking json output: per cache_instance [Success]
   Checking json output: per die [Success]
   Checking json output: per socket [Success]
   test child finished with 0
   ---- end ----
   perf stat JSON output linter: Ok

Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>

> Cc: K Prateek Nayak <kprateek.nayak@amd.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/tests/shell/lib/perf_json_output_lint.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/tests/shell/lib/perf_json_output_lint.py b/tools/perf/tests/shell/lib/perf_json_output_lint.py
> index 4acaaed5560d..b81582a89d36 100644
> --- a/tools/perf/tests/shell/lib/perf_json_output_lint.py
> +++ b/tools/perf/tests/shell/lib/perf_json_output_lint.py
> @@ -85,7 +85,7 @@ Lines = args.file.readlines()
>      expected_items = 7
>    elif args.interval or args.per_thread or args.system_wide_no_aggr:
>      expected_items = 8
> -  elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache_instance:
> +  elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache:
>      expected_items = 9
>    else:
>      # If no option is specified, don't check the number of items.

--
Thanks and Regards,
Prateek
  
Ian Rogers May 26, 2023, 10:41 p.m. UTC | #2
On Wed, May 24, 2023 at 8:01 PM K Prateek Nayak <kprateek.nayak@amd.com> wrote:
>
> Hello Namhyung,
>
> On 5/25/2023 2:36 AM, Namhyung Kim wrote:
> > The recent --per-cache option test caused a problem.  According to
> > the option name, I think it should check args.per_cache instead of
> > args.per_cache_instance.
> >
> >   $ sudo ./perf test -v 99
> >    99: perf stat JSON output linter                                    :
> >   --- start ---
> >   test child forked, pid 3086101
> >   Checking json output: no args [Success]
> >   Checking json output: system wide [Success]
> >   Checking json output: interval [Success]
> >   Checking json output: event [Success]
> >   Checking json output: per thread [Success]
> >   Checking json output: per node [Success]
> >   Checking json output: system wide no aggregation [Success]
> >   Checking json output: per core [Success]
> >   Checking json output: per cache_instance Test failed for input:
> >   ...
> >   Traceback (most recent call last):
> >     File "linux/tools/perf/tests/shell/lib/perf_json_output_lint.py", line 88, in <module>
> >       elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache_instance:
> >   AttributeError: 'Namespace' object has no attribute 'per_cache_instance'
> >   test child finished with -1
> >   ---- end ----
> >   perf stat JSON output linter: FAILED!
> >
> > Fixes: bfce728db317 ("pert tests: Add tests for new "perf stat --per-cache" aggregation option")
>
> Another oversight on my part. Thank you for fixing this :)
>
>    $ sudo perf test -v 99
>    99: perf stat JSON output linter                                    :
>    --- start ---
>    test child forked, pid 25046
>    Checking json output: no args [Success]
>    Checking json output: system wide [Success]
>    Checking json output: interval [Success]
>    Checking json output: event [Success]
>    Checking json output: per thread [Success]
>    Checking json output: per node [Success]
>    Checking json output: system wide no aggregation [Success]
>    Checking json output: per core [Success]
>    Checking json output: per cache_instance [Success]
>    Checking json output: per die [Success]
>    Checking json output: per socket [Success]
>    test child finished with 0
>    ---- end ----
>    perf stat JSON output linter: Ok
>
> Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>

Thanks Namhyung and Prateek, Arnaldo could we get this in
perf-tools-next so that the failing test goes away?

Acked-by: Ian Rogers <irogers@google.com>

> > Cc: K Prateek Nayak <kprateek.nayak@amd.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/tests/shell/lib/perf_json_output_lint.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/tests/shell/lib/perf_json_output_lint.py b/tools/perf/tests/shell/lib/perf_json_output_lint.py
> > index 4acaaed5560d..b81582a89d36 100644
> > --- a/tools/perf/tests/shell/lib/perf_json_output_lint.py
> > +++ b/tools/perf/tests/shell/lib/perf_json_output_lint.py
> > @@ -85,7 +85,7 @@ Lines = args.file.readlines()
> >      expected_items = 7
> >    elif args.interval or args.per_thread or args.system_wide_no_aggr:
> >      expected_items = 8
> > -  elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache_instance:
> > +  elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache:
> >      expected_items = 9
> >    else:
> >      # If no option is specified, don't check the number of items.
>
> --
> Thanks and Regards,
> Prateek
  
Arnaldo Carvalho de Melo May 27, 2023, 1:48 a.m. UTC | #3
Em Fri, May 26, 2023 at 03:41:29PM -0700, Ian Rogers escreveu:
> On Wed, May 24, 2023 at 8:01 PM K Prateek Nayak <kprateek.nayak@amd.com> wrote:
> >
> > Hello Namhyung,
> >
> > On 5/25/2023 2:36 AM, Namhyung Kim wrote:
> > > The recent --per-cache option test caused a problem.  According to
> > > the option name, I think it should check args.per_cache instead of
> > > args.per_cache_instance.
> > >
> > >   $ sudo ./perf test -v 99
> > >    99: perf stat JSON output linter                                    :
> > >   --- start ---
> > >   test child forked, pid 3086101
> > >   Checking json output: no args [Success]
> > >   Checking json output: system wide [Success]
> > >   Checking json output: interval [Success]
> > >   Checking json output: event [Success]
> > >   Checking json output: per thread [Success]
> > >   Checking json output: per node [Success]
> > >   Checking json output: system wide no aggregation [Success]
> > >   Checking json output: per core [Success]
> > >   Checking json output: per cache_instance Test failed for input:
> > >   ...
> > >   Traceback (most recent call last):
> > >     File "linux/tools/perf/tests/shell/lib/perf_json_output_lint.py", line 88, in <module>
> > >       elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache_instance:
> > >   AttributeError: 'Namespace' object has no attribute 'per_cache_instance'
> > >   test child finished with -1
> > >   ---- end ----
> > >   perf stat JSON output linter: FAILED!
> > >
> > > Fixes: bfce728db317 ("pert tests: Add tests for new "perf stat --per-cache" aggregation option")
> >
> > Another oversight on my part. Thank you for fixing this :)
> >
> >    $ sudo perf test -v 99
> >    99: perf stat JSON output linter                                    :
> >    --- start ---
> >    test child forked, pid 25046
> >    Checking json output: no args [Success]
> >    Checking json output: system wide [Success]
> >    Checking json output: interval [Success]
> >    Checking json output: event [Success]
> >    Checking json output: per thread [Success]
> >    Checking json output: per node [Success]
> >    Checking json output: system wide no aggregation [Success]
> >    Checking json output: per core [Success]
> >    Checking json output: per cache_instance [Success]
> >    Checking json output: per die [Success]
> >    Checking json output: per socket [Success]
> >    test child finished with 0
> >    ---- end ----
> >    perf stat JSON output linter: Ok
> >
> > Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>
> 
> Thanks Namhyung and Prateek, Arnaldo could we get this in
> perf-tools-next so that the failing test goes away?
> 
> Acked-by: Ian Rogers <irogers@google.com>

Applied and pushed to perf-tools-next, please continue from there.

⬢[acme@toolbox perf-tools-next]$ git log --oneline -10
540c910c65a94fb4 (HEAD -> perf-tools-next) perf test: Fix perf stat JSON output test
5cebb33fd929dc67 perf tests: Organize cpu_map tests into a single suite
237d41d4a2d7d45e perf cpumap: Add intersect function
6ac2230b55d392e6 perf vendor events intel: Add metricgroup descriptions for all models
66c6e0c100277175 perf jevents: Add support for metricgroup descriptions
bfce728db3179042 pert tests: Add tests for new "perf stat --per-cache" aggregation option
aab667ca8837e45f perf stat: Add "--per-cache" aggregation option and document it
4b87406a3b590888 perf stat record: Save cache level information
995ed074b829f293 perf stat: Setup the foundation to allow aggregation based on cache topology
2b72cec9eef19d73 perf: Extract building cache level for a CPU into separate function
⬢[acme@toolbox perf-tools-next]$

- Arnaldo
  

Patch

diff --git a/tools/perf/tests/shell/lib/perf_json_output_lint.py b/tools/perf/tests/shell/lib/perf_json_output_lint.py
index 4acaaed5560d..b81582a89d36 100644
--- a/tools/perf/tests/shell/lib/perf_json_output_lint.py
+++ b/tools/perf/tests/shell/lib/perf_json_output_lint.py
@@ -85,7 +85,7 @@  Lines = args.file.readlines()
     expected_items = 7
   elif args.interval or args.per_thread or args.system_wide_no_aggr:
     expected_items = 8
-  elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache_instance:
+  elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache:
     expected_items = 9
   else:
     # If no option is specified, don't check the number of items.