diff mbox series

[V3] tools/perf/tests/shell/base_probe: Enhance print_overall_results to print summary information

Message ID 20241206135254.35727-1-atrajeev@linux.vnet.ibm.com (mailing list archive)
State Handled Elsewhere
Headers show
Series [V3] tools/perf/tests/shell/base_probe: Enhance print_overall_results to print summary information | expand

Checks

Context Check Description
snowpatch_ozlabs/github-powerpc_perf fail 2 of 6 jobs failed.
snowpatch_ozlabs/github-powerpc_sparse success Successfully ran 4 jobs.
snowpatch_ozlabs/github-powerpc_clang success Successfully ran 5 jobs.
snowpatch_ozlabs/github-powerpc_kernel_qemu success Successfully ran 21 jobs.

Commit Message

Athira Rajeev Dec. 6, 2024, 1:52 p.m. UTC
Currently print_overall_results prints the number of
fails in the summary, example from base_probe tests in
testsuite_probe:

 ## [ FAIL ] ## perf_probe :: test_invalid_options SUMMARY ::
	11 failures found

test_invalid_options contains multiple tests and out
of that 11 failed. Sometimes it could happen that it
is due to missing dependency in the build or environment
dependency.

Example, perf probe -L requires DWARF enabled. otherwise
it fails as below:
 ./perf probe -L
  Error: switch `L' is not available because NO_DWARF=1

"-L" is tested as one of the option in :
   for opt in '-a' '-d' '-L' '-V'; do
   <<perf probe test>>
   print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "missing argument
	for $opt"

Here -a and -d doesn't require DWARF. Similarly there
are few other tests requiring DWARF. To hint the user that
missing dwarf could be one issue, update print_overall_results
to print a comment string along with summary hinting the possible
cause. Update test_invalid_options.sh and test_line_semantics.sh
to pass the info about dwarf requirement since these tests
failed when perf is built without DWARF. Use the check for
presence of dwarf with "perf check feature" and append the
hint message based on the result.

With the change:
 ## [ FAIL ] ## perf_probe :: test_invalid_options SUMMARY ::
 11 failures found :: Some of the tests need DWARF to run

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
Changelog:
v1 -> v2
 Check presence of dwarf using "perf check feature". Add
 failure hint message for dwarf based on this check

v2 -> v3
 Check for empty task comment to avoid trailing space
 at the end as suggested by Michael Petlan

 tools/perf/tests/shell/base_probe/test_invalid_options.sh | 5 ++++-
 tools/perf/tests/shell/base_probe/test_line_semantics.sh  | 5 ++++-
 tools/perf/tests/shell/common/init.sh                     | 5 ++++-
 3 files changed, 12 insertions(+), 3 deletions(-)

Comments

Kajol Jain Dec. 11, 2024, 12:03 p.m. UTC | #1
On 12/6/24 19:22, Athira Rajeev wrote:
> Currently print_overall_results prints the number of
> fails in the summary, example from base_probe tests in
> testsuite_probe:
> 
>  ## [ FAIL ] ## perf_probe :: test_invalid_options SUMMARY ::
> 	11 failures found
> 
> test_invalid_options contains multiple tests and out
> of that 11 failed. Sometimes it could happen that it
> is due to missing dependency in the build or environment
> dependency.
> 
> Example, perf probe -L requires DWARF enabled. otherwise
> it fails as below:
>  ./perf probe -L
>   Error: switch `L' is not available because NO_DWARF=1
> 
> "-L" is tested as one of the option in :
>    for opt in '-a' '-d' '-L' '-V'; do
>    <<perf probe test>>
>    print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "missing argument
> 	for $opt"
> 
> Here -a and -d doesn't require DWARF. Similarly there
> are few other tests requiring DWARF. To hint the user that
> missing dwarf could be one issue, update print_overall_results
> to print a comment string along with summary hinting the possible
> cause. Update test_invalid_options.sh and test_line_semantics.sh
> to pass the info about dwarf requirement since these tests
> failed when perf is built without DWARF. Use the check for
> presence of dwarf with "perf check feature" and append the
> hint message based on the result.
> 

Patch looks fine to me.

Reviewed-by: kjain@linux.ibm.com

Thanks,
Kajol Jain

> With the change:
>  ## [ FAIL ] ## perf_probe :: test_invalid_options SUMMARY ::
>  11 failures found :: Some of the tests need DWARF to run
> 
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
> Changelog:
> v1 -> v2
>  Check presence of dwarf using "perf check feature". Add
>  failure hint message for dwarf based on this check
> 
> v2 -> v3
>  Check for empty task comment to avoid trailing space
>  at the end as suggested by Michael Petlan
> 
>  tools/perf/tests/shell/base_probe/test_invalid_options.sh | 5 ++++-
>  tools/perf/tests/shell/base_probe/test_line_semantics.sh  | 5 ++++-
>  tools/perf/tests/shell/common/init.sh                     | 5 ++++-
>  3 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/tests/shell/base_probe/test_invalid_options.sh b/tools/perf/tests/shell/base_probe/test_invalid_options.sh
> index 1fedfd8b0d0d..c51d8d9bfdb7 100755
> --- a/tools/perf/tests/shell/base_probe/test_invalid_options.sh
> +++ b/tools/perf/tests/shell/base_probe/test_invalid_options.sh
> @@ -22,6 +22,9 @@ if ! check_kprobes_available; then
>  	exit 0
>  fi
>  
> +# Check for presence of dwarf
> +$CMD_PERF check feature -q dwarf
> +[ $? -ne 0 ] && HINT_FAIL="Some of the tests need DWARF to run"
>  
>  ### missing argument
>  
> @@ -75,5 +78,5 @@ done
>  
>  
>  # print overall results
> -print_overall_results "$TEST_RESULT"
> +print_overall_results "$TEST_RESULT" $HINT_FAIL
>  exit $?
> diff --git a/tools/perf/tests/shell/base_probe/test_line_semantics.sh b/tools/perf/tests/shell/base_probe/test_line_semantics.sh
> index d8f4bde0f585..f5d3b6e8d0d6 100755
> --- a/tools/perf/tests/shell/base_probe/test_line_semantics.sh
> +++ b/tools/perf/tests/shell/base_probe/test_line_semantics.sh
> @@ -23,6 +23,9 @@ if ! check_kprobes_available; then
>  	exit 0
>  fi
>  
> +# Check for presence of dwarf
> +$CMD_PERF check feature -q dwarf
> +[ $? -ne 0 ] && HINT_FAIL="Some of the tests need DWARF to run"
>  
>  ### acceptable --line descriptions
>  
> @@ -51,5 +54,5 @@ done
>  
>  
>  # print overall results
> -print_overall_results "$TEST_RESULT"
> +print_overall_results "$TEST_RESULT" $HINT_FAIL
>  exit $?
> diff --git a/tools/perf/tests/shell/common/init.sh b/tools/perf/tests/shell/common/init.sh
> index 075f17623c8e..259706ef5899 100644
> --- a/tools/perf/tests/shell/common/init.sh
> +++ b/tools/perf/tests/shell/common/init.sh
> @@ -46,10 +46,13 @@ print_results()
>  print_overall_results()
>  {
>  	RETVAL="$1"; shift
> +	TASK_COMMENT="$*"
> +	test -n "$TASK_COMMENT" && TASK_COMMENT=":: $TASK_COMMENT"
> +
>  	if [ $RETVAL -eq 0 ]; then
>  		_echo "$MALLPASS## [ PASS ] ##$MEND $TEST_NAME :: $THIS_TEST_NAME SUMMARY"
>  	else
> -		_echo "$MALLFAIL## [ FAIL ] ##$MEND $TEST_NAME :: $THIS_TEST_NAME SUMMARY :: $RETVAL failures found"
> +		_echo "$MALLFAIL## [ FAIL ] ##$MEND $TEST_NAME :: $THIS_TEST_NAME SUMMARY :: $RETVAL failures found $TASK_COMMENT"
>  	fi
>  	return $RETVAL
>  }
Arnaldo Carvalho de Melo Dec. 16, 2024, 5:50 p.m. UTC | #2
On Wed, Dec 11, 2024 at 05:33:50PM +0530, kajoljain wrote:
> On 12/6/24 19:22, Athira Rajeev wrote:
> > Here -a and -d doesn't require DWARF. Similarly there
> > are few other tests requiring DWARF. To hint the user that
> > missing dwarf could be one issue, update print_overall_results
> > to print a comment string along with summary hinting the possible
> > cause. Update test_invalid_options.sh and test_line_semantics.sh
> > to pass the info about dwarf requirement since these tests
> > failed when perf is built without DWARF. Use the check for
> > presence of dwarf with "perf check feature" and append the
> > hint message based on the result.
 
> Patch looks fine to me.
 
> Reviewed-by: kjain@linux.ibm.com

Thanks, applied to perf-tools-next,

- Arnaldo
diff mbox series

Patch

diff --git a/tools/perf/tests/shell/base_probe/test_invalid_options.sh b/tools/perf/tests/shell/base_probe/test_invalid_options.sh
index 1fedfd8b0d0d..c51d8d9bfdb7 100755
--- a/tools/perf/tests/shell/base_probe/test_invalid_options.sh
+++ b/tools/perf/tests/shell/base_probe/test_invalid_options.sh
@@ -22,6 +22,9 @@  if ! check_kprobes_available; then
 	exit 0
 fi
 
+# Check for presence of dwarf
+$CMD_PERF check feature -q dwarf
+[ $? -ne 0 ] && HINT_FAIL="Some of the tests need DWARF to run"
 
 ### missing argument
 
@@ -75,5 +78,5 @@  done
 
 
 # print overall results
-print_overall_results "$TEST_RESULT"
+print_overall_results "$TEST_RESULT" $HINT_FAIL
 exit $?
diff --git a/tools/perf/tests/shell/base_probe/test_line_semantics.sh b/tools/perf/tests/shell/base_probe/test_line_semantics.sh
index d8f4bde0f585..f5d3b6e8d0d6 100755
--- a/tools/perf/tests/shell/base_probe/test_line_semantics.sh
+++ b/tools/perf/tests/shell/base_probe/test_line_semantics.sh
@@ -23,6 +23,9 @@  if ! check_kprobes_available; then
 	exit 0
 fi
 
+# Check for presence of dwarf
+$CMD_PERF check feature -q dwarf
+[ $? -ne 0 ] && HINT_FAIL="Some of the tests need DWARF to run"
 
 ### acceptable --line descriptions
 
@@ -51,5 +54,5 @@  done
 
 
 # print overall results
-print_overall_results "$TEST_RESULT"
+print_overall_results "$TEST_RESULT" $HINT_FAIL
 exit $?
diff --git a/tools/perf/tests/shell/common/init.sh b/tools/perf/tests/shell/common/init.sh
index 075f17623c8e..259706ef5899 100644
--- a/tools/perf/tests/shell/common/init.sh
+++ b/tools/perf/tests/shell/common/init.sh
@@ -46,10 +46,13 @@  print_results()
 print_overall_results()
 {
 	RETVAL="$1"; shift
+	TASK_COMMENT="$*"
+	test -n "$TASK_COMMENT" && TASK_COMMENT=":: $TASK_COMMENT"
+
 	if [ $RETVAL -eq 0 ]; then
 		_echo "$MALLPASS## [ PASS ] ##$MEND $TEST_NAME :: $THIS_TEST_NAME SUMMARY"
 	else
-		_echo "$MALLFAIL## [ FAIL ] ##$MEND $TEST_NAME :: $THIS_TEST_NAME SUMMARY :: $RETVAL failures found"
+		_echo "$MALLFAIL## [ FAIL ] ##$MEND $TEST_NAME :: $THIS_TEST_NAME SUMMARY :: $RETVAL failures found $TASK_COMMENT"
 	fi
 	return $RETVAL
 }