linux-kernel - Re: [PATCH V3] tools/perf/tests/shell/base_probe: Enhance print_overall

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <32b19139-27f2-4f39-a2ae-25027acc980e@linux.ibm.com>
Date: Wed, 11 Dec 2024 17:33:50 +0530
From: kajoljain <kjain@...ux.ibm.com>
To: Athira Rajeev <atrajeev@...ux.vnet.ibm.com>, acme@...nel.org,
        jolsa@...nel.org, adrian.hunter@...el.com, irogers@...gle.com,
        namhyung@...nel.org
Cc: linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
        linuxppc-dev@...ts.ozlabs.org, maddy@...ux.ibm.com,
        disgoel@...ux.vnet.ibm.com, mpetlan@...hat.com, vmolnaro@...hat.com
Subject: Re: [PATCH V3] tools/perf/tests/shell/base_probe: Enhance
 print_overall_results to print summary information



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@...ux.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@...ux.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
>  }