diff mbox series

perf test: Fixed masked error condition in perf data convert test

Message ID 20240112120737.1995575-1-james.clark@arm.com
State New
Headers
Series perf test: Fixed masked error condition in perf data convert test |

Commit Message

James Clark Jan. 12, 2024, 12:07 p.m. UTC 
  The test does set -e, so any errors go straight to the exit handler,
where it returns err=0 (success). Fix it by leaving err=1 from the
beginning and only set the success code if it ran all the way to the end
without errors.

Also remove the exit code argument from the last exit because it doesn't
do anything, it's always replaced by err in the exit handler.

Fixes: 68d124182610 ("perf test: Add test validating JSON generated by 'perf data convert --to-json'")
Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/tests/shell/test_perf_data_converter_json.sh | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Namhyung Kim Jan. 16, 2024, 9:54 p.m. UTC | #1 
On Fri, Jan 12, 2024 at 4:07 AM James Clark <james.clark@arm.com> wrote:
>
> The test does set -e, so any errors go straight to the exit handler,
> where it returns err=0 (success). Fix it by leaving err=1 from the
> beginning and only set the success code if it ran all the way to the end
> without errors.
>
> Also remove the exit code argument from the last exit because it doesn't
> do anything, it's always replaced by err in the exit handler.
>
> Fixes: 68d124182610 ("perf test: Add test validating JSON generated by 'perf data convert --to-json'")
> Signed-off-by: James Clark <james.clark@arm.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> ---
>  tools/perf/tests/shell/test_perf_data_converter_json.sh | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/tests/shell/test_perf_data_converter_json.sh b/tools/perf/tests/shell/test_perf_data_converter_json.sh
> index c4f1b59d116f..1781b7215c11 100755
> --- a/tools/perf/tests/shell/test_perf_data_converter_json.sh
> +++ b/tools/perf/tests/shell/test_perf_data_converter_json.sh
> @@ -4,7 +4,7 @@
>
>  set -e
>
> -err=0
> +err=1
>
>  shelldir=$(dirname "$0")
>  # shellcheck source=lib/setup_python.sh
> @@ -36,7 +36,6 @@ test_json_converter_command()
>                 echo "Perf Data Converter Command to JSON [SUCCESS]"
>         else
>                 echo "Perf Data Converter Command to JSON [FAILED]"
> -               err=1
>                 exit
>         fi
>  }
> @@ -49,7 +48,6 @@ validate_json_format()
>                         echo "The file contains valid JSON format [SUCCESS]"
>                 else
>                         echo "The file does not contain valid JSON format [FAILED]"
> -                       err=1
>                         exit
>                 fi
>         else
> @@ -62,4 +60,7 @@ validate_json_format()
>  test_json_converter_command
>  validate_json_format
>
> -exit ${err}
> +# Set -e is on, so it was only successful if it ran up to this point.
> +# Therefore only set err=0 here.
> +err=0
> +exit
> --
> 2.34.1
>
Ilkka Koskinen Jan. 17, 2024, 7:37 a.m. UTC | #2 
On Fri, 12 Jan 2024, James Clark wrote:

> The test does set -e, so any errors go straight to the exit handler,
> where it returns err=0 (success). Fix it by leaving err=1 from the
> beginning and only set the success code if it ran all the way to the end
> without errors.
>
> Also remove the exit code argument from the last exit because it doesn't
> do anything, it's always replaced by err in the exit handler.
>
> Fixes: 68d124182610 ("perf test: Add test validating JSON generated by 'perf data convert --to-json'")
> Signed-off-by: James Clark <james.clark@arm.com>

Looks good to me.


Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>

Cheers, Ilkka

> ---
> tools/perf/tests/shell/test_perf_data_converter_json.sh | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/tests/shell/test_perf_data_converter_json.sh b/tools/perf/tests/shell/test_perf_data_converter_json.sh
> index c4f1b59d116f..1781b7215c11 100755
> --- a/tools/perf/tests/shell/test_perf_data_converter_json.sh
> +++ b/tools/perf/tests/shell/test_perf_data_converter_json.sh
> @@ -4,7 +4,7 @@
>
> set -e
>
> -err=0
> +err=1
>
> shelldir=$(dirname "$0")
> # shellcheck source=lib/setup_python.sh
> @@ -36,7 +36,6 @@ test_json_converter_command()
> 		echo "Perf Data Converter Command to JSON [SUCCESS]"
> 	else
> 		echo "Perf Data Converter Command to JSON [FAILED]"
> -		err=1
> 		exit
> 	fi
> }
> @@ -49,7 +48,6 @@ validate_json_format()
> 			echo "The file contains valid JSON format [SUCCESS]"
> 		else
> 			echo "The file does not contain valid JSON format [FAILED]"
> -			err=1
> 			exit
> 		fi
> 	else
> @@ -62,4 +60,7 @@ validate_json_format()
> test_json_converter_command
> validate_json_format
>
> -exit ${err}
> +# Set -e is on, so it was only successful if it ran up to this point.
> +# Therefore only set err=0 here.
> +err=0
> +exit
> -- 
> 2.34.1
>
>
diff mbox series

Patch

diff --git a/tools/perf/tests/shell/test_perf_data_converter_json.sh b/tools/perf/tests/shell/test_perf_data_converter_json.sh
index c4f1b59d116f..1781b7215c11 100755
--- a/tools/perf/tests/shell/test_perf_data_converter_json.sh
+++ b/tools/perf/tests/shell/test_perf_data_converter_json.sh
@@ -4,7 +4,7 @@ 
 
 set -e
 
-err=0
+err=1
 
 shelldir=$(dirname "$0")
 # shellcheck source=lib/setup_python.sh
@@ -36,7 +36,6 @@  test_json_converter_command()
 		echo "Perf Data Converter Command to JSON [SUCCESS]"
 	else
 		echo "Perf Data Converter Command to JSON [FAILED]"
-		err=1
 		exit
 	fi
 }
@@ -49,7 +48,6 @@  validate_json_format()
 			echo "The file contains valid JSON format [SUCCESS]"
 		else
 			echo "The file does not contain valid JSON format [FAILED]"
-			err=1
 			exit
 		fi
 	else
@@ -62,4 +60,7 @@  validate_json_format()
 test_json_converter_command
 validate_json_format
 
-exit ${err}
+# Set -e is on, so it was only successful if it ran up to this point.
+# Therefore only set err=0 here.
+err=0
+exit