Re: [PATCH 2/3] perf test: Move some ARM tests to arch/arm64/tests/shell

[James Clark <james.clark@linaro.org>]

On 22/05/2025 6:10 pm, Namhyung Kim wrote:
> The test_arm_callgraph_fp.sh checks with aarch64 so it should belong to
> there.  And IIRC ARM SPE is supported on 64-bit platforms so move the
> tests too.  But I'm not sure about coresight so left them.

Coresight is also supported on arm32 which is why 
tools/perf/arch/arm/util/cs-etm.c is built for both platforms. Having 
said that, I'm not sure if the tests pass on arm32 because we're not 
running them, but they should pass and if someone is running them this 
change could break that.

We could symlink "arch/arm64/tests/shell" to "arch/arm/tests/shell" or 
have a condition in the code that forces one folder for both. Or just 
continue with only arm64. Considering I haven't seen a test failure 
report from there and we've only really been adding tests for arm64 
stuff it could be reasonable.

> 
> Also please test it with shellcheck as I couldn't run it actually.
> 

Shellcheck is working on these scripts for me.

Reviewed-by: James Clark <james.clark@linaro.org>

> Cc: Leo Yan <leo.yan@arm.com>
> Cc: James Clark <james.clark@linaro.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>   tools/perf/arch/arm64/tests/Build                  | 14 ++++++++++++++
>   .../arm64}/tests/shell/test_arm_callgraph_fp.sh    |  4 ++--
>   .../{ => arch/arm64}/tests/shell/test_arm_spe.sh   |  0
>   .../arm64}/tests/shell/test_arm_spe_fork.sh        |  0
>   4 files changed, 16 insertions(+), 2 deletions(-)
>   rename tools/perf/{ => arch/arm64}/tests/shell/test_arm_callgraph_fp.sh (89%)
>   rename tools/perf/{ => arch/arm64}/tests/shell/test_arm_spe.sh (100%)
>   rename tools/perf/{ => arch/arm64}/tests/shell/test_arm_spe_fork.sh (100%)
> 
> diff --git a/tools/perf/arch/arm64/tests/Build b/tools/perf/arch/arm64/tests/Build
> index d44c9de92d425c62..6c73720cb0ffa99d 100644
> --- a/tools/perf/arch/arm64/tests/Build
> +++ b/tools/perf/arch/arm64/tests/Build
> @@ -3,3 +3,17 @@ perf-test-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
>   
>   perf-test-y += arch-tests.o
>   perf-test-y += cpuid-match.o
> +
> +ifdef SHELLCHECK
> +  SHELL_TESTS := $(shell find shell -executable -type f -name '*.sh')
> +  SHELL_TEST_LOGS := $(SHELL_TESTS:tests/shell/%=shell/%.shellcheck_log)
> +else
> +  SHELL_TESTS :=
> +  SHELL_TEST_LOGS :=
> +endif
> +
> +$(OUTPUT)%.shellcheck_log: %
> +	$(call rule_mkdir)
> +	$(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> +
> +perf-test-y += $(SHELL_TEST_LOGS)
> diff --git a/tools/perf/tests/shell/test_arm_callgraph_fp.sh b/tools/perf/arch/arm64/tests/shell/test_arm_callgraph_fp.sh
> similarity index 89%
> rename from tools/perf/tests/shell/test_arm_callgraph_fp.sh
> rename to tools/perf/arch/arm64/tests/shell/test_arm_callgraph_fp.sh
> index 9caa36130175964e..f59ab293d67b9f9c 100755
> --- a/tools/perf/tests/shell/test_arm_callgraph_fp.sh
> +++ b/tools/perf/arch/arm64/tests/shell/test_arm_callgraph_fp.sh
> @@ -3,8 +3,8 @@
>   # SPDX-License-Identifier: GPL-2.0
>   
>   shelldir=$(dirname "$0")
> -# shellcheck source=lib/perf_has_symbol.sh
> -. "${shelldir}"/lib/perf_has_symbol.sh
> +# shellcheck source=../../../../tests/shell/lib/perf_has_symbol.sh
> +. "${shelldir}"/../../../../tests/shell/lib/perf_has_symbol.sh
>   
>   if [ "$(uname -m)" != "aarch64" ]; then
>   	exit 2
> diff --git a/tools/perf/tests/shell/test_arm_spe.sh b/tools/perf/arch/arm64/tests/shell/test_arm_spe.sh
> similarity index 100%
> rename from tools/perf/tests/shell/test_arm_spe.sh
> rename to tools/perf/arch/arm64/tests/shell/test_arm_spe.sh
> diff --git a/tools/perf/tests/shell/test_arm_spe_fork.sh b/tools/perf/arch/arm64/tests/shell/test_arm_spe_fork.sh
> similarity index 100%
> rename from tools/perf/tests/shell/test_arm_spe_fork.sh
> rename to tools/perf/arch/arm64/tests/shell/test_arm_spe_fork.sh