1. Patchew
  2. linux
  3. [PATCHSET 0/8] perf test: Improve perf record tests (v2)
  4. View patch

[PATCH 8/8] perf test: Do not set TEST_SKIP for record subtests

Namhyung Kim posted 8 patches 3 years, 5 months ago
[PATCH 8/8] perf test: Do not set TEST_SKIP for record subtests
Posted by Namhyung Kim 3 years, 5 months ago 
It now has 4 sub tests and one of them should run at least.
But once TEST_SKIP (= 2) return value is set, it won't be overwritten
unless there's a failure.  I think we should return success when one
or more tested are skipped but the remaining subtests are passed.

So update the test code not to set the err variable when it skips
the test.

Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/tests/shell/record.sh | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
index 01aa9531b369..e93b3a8871fe 100755
--- a/tools/perf/tests/shell/record.sh
+++ b/tools/perf/tests/shell/record.sh
@@ -81,10 +81,6 @@ test_per_thread() {
   if ! perf record -o /dev/null --quiet ${testprog} 2> /dev/null
   then
     echo "Per-thread record [Skipped event not supported]"
-    if [ $err -ne 1 ]
-    then
-      err=2
-    fi
     return
   fi
   if ! perf record --per-thread -o "${perfdata}" ${testprog} 2> /dev/null
@@ -131,10 +127,6 @@ test_register_capture() {
   if ! perf list | grep -q 'br_inst_retired.near_call'
   then
     echo "Register capture test [Skipped missing event]"
-    if [ $err -ne 1 ]
-    then
-      err=2
-    fi
     return
   fi
   if ! perf record --intr-regs=\? 2>&1 | grep -q 'available registers: AX BX CX DX SI DI BP SP IP FLAGS CS SS R8 R9 R10 R11 R12 R13 R14 R15'
@@ -159,10 +151,6 @@ test_system_wide() {
   if ! perf record -aB --synth=no -o "${perfdata}" ${testprog} 2> /dev/null
   then
     echo "System-wide record [Skipped not supported]"
-    if [ $err -ne 1 ]
-    then
-      err=2
-    fi
     return
   fi
   if ! perf report -i "${perfdata}" -q | grep -q "${testsym}"
-- 
2.38.0.135.g90850a2211-goog
Re: [PATCH 8/8] perf test: Do not set TEST_SKIP for record subtests
Posted by Ian Rogers 3 years, 5 months ago 
On Thu, Oct 20, 2022 at 10:26 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> It now has 4 sub tests and one of them should run at least.
> But once TEST_SKIP (= 2) return value is set, it won't be overwritten
> unless there's a failure.  I think we should return success when one
> or more tested are skipped but the remaining subtests are passed.
>
> So update the test code not to set the err variable when it skips
> the test.
>
> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

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

Perhaps we can have proper sub-tests in shell tests like we do for the C ones.

Thanks,
Ian

> ---
>  tools/perf/tests/shell/record.sh | 12 ------------
>  1 file changed, 12 deletions(-)
>
> diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
> index 01aa9531b369..e93b3a8871fe 100755
> --- a/tools/perf/tests/shell/record.sh
> +++ b/tools/perf/tests/shell/record.sh
> @@ -81,10 +81,6 @@ test_per_thread() {
>    if ! perf record -o /dev/null --quiet ${testprog} 2> /dev/null
>    then
>      echo "Per-thread record [Skipped event not supported]"
> -    if [ $err -ne 1 ]
> -    then
> -      err=2
> -    fi
>      return
>    fi
>    if ! perf record --per-thread -o "${perfdata}" ${testprog} 2> /dev/null
> @@ -131,10 +127,6 @@ test_register_capture() {
>    if ! perf list | grep -q 'br_inst_retired.near_call'
>    then
>      echo "Register capture test [Skipped missing event]"
> -    if [ $err -ne 1 ]
> -    then
> -      err=2
> -    fi
>      return
>    fi
>    if ! perf record --intr-regs=\? 2>&1 | grep -q 'available registers: AX BX CX DX SI DI BP SP IP FLAGS CS SS R8 R9 R10 R11 R12 R13 R14 R15'
> @@ -159,10 +151,6 @@ test_system_wide() {
>    if ! perf record -aB --synth=no -o "${perfdata}" ${testprog} 2> /dev/null
>    then
>      echo "System-wide record [Skipped not supported]"
> -    if [ $err -ne 1 ]
> -    then
> -      err=2
> -    fi
>      return
>    fi
>    if ! perf report -i "${perfdata}" -q | grep -q "${testsym}"
> --
> 2.38.0.135.g90850a2211-goog
>
Re: [PATCH 8/8] perf test: Do not set TEST_SKIP for record subtests
Posted by Arnaldo Carvalho de Melo 3 years, 5 months ago 
Em Thu, Oct 20, 2022 at 05:05:49PM -0700, Ian Rogers escreveu:
> On Thu, Oct 20, 2022 at 10:26 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > It now has 4 sub tests and one of them should run at least.
> > But once TEST_SKIP (= 2) return value is set, it won't be overwritten
> > unless there's a failure.  I think we should return success when one
> > or more tested are skipped but the remaining subtests are passed.
> >
> > So update the test code not to set the err variable when it skips
> > the test.
> >
> > Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> 
> Acked-by: Ian Rogers <irogers@google.com>
> 
> Perhaps we can have proper sub-tests in shell tests like we do for the C ones.

Yeah, giving info about the various subtests helps in giving progress
information and overal confidence that more stuff is being tested.

- Arnaldo
 
> Thanks,
> Ian
> 
> > ---
> >  tools/perf/tests/shell/record.sh | 12 ------------
> >  1 file changed, 12 deletions(-)
> >
> > diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
> > index 01aa9531b369..e93b3a8871fe 100755
> > --- a/tools/perf/tests/shell/record.sh
> > +++ b/tools/perf/tests/shell/record.sh
> > @@ -81,10 +81,6 @@ test_per_thread() {
> >    if ! perf record -o /dev/null --quiet ${testprog} 2> /dev/null
> >    then
> >      echo "Per-thread record [Skipped event not supported]"
> > -    if [ $err -ne 1 ]
> > -    then
> > -      err=2
> > -    fi
> >      return
> >    fi
> >    if ! perf record --per-thread -o "${perfdata}" ${testprog} 2> /dev/null
> > @@ -131,10 +127,6 @@ test_register_capture() {
> >    if ! perf list | grep -q 'br_inst_retired.near_call'
> >    then
> >      echo "Register capture test [Skipped missing event]"
> > -    if [ $err -ne 1 ]
> > -    then
> > -      err=2
> > -    fi
> >      return
> >    fi
> >    if ! perf record --intr-regs=\? 2>&1 | grep -q 'available registers: AX BX CX DX SI DI BP SP IP FLAGS CS SS R8 R9 R10 R11 R12 R13 R14 R15'
> > @@ -159,10 +151,6 @@ test_system_wide() {
> >    if ! perf record -aB --synth=no -o "${perfdata}" ${testprog} 2> /dev/null
> >    then
> >      echo "System-wide record [Skipped not supported]"
> > -    if [ $err -ne 1 ]
> > -    then
> > -      err=2
> > -    fi
> >      return
> >    fi
> >    if ! perf report -i "${perfdata}" -q | grep -q "${testsym}"
> > --
> > 2.38.0.135.g90850a2211-goog
> >

-- 

- Arnaldo

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.