Messages in this thread Patch in this message | | | From | Ian Rogers <> | Date | Wed, 29 Nov 2023 09:21:12 -0800 | Subject | Re: [PATCH v1 2/2] perf test: Add basic list test |
| |
On Wed, Nov 29, 2023 at 1:27 AM James Clark <james.clark@arm.com> wrote: > > > > On 29/11/2023 09:00, Adrian Hunter wrote: > > On 29/11/23 10:10, Ian Rogers wrote: > >> Test that json output produces valid json. > >> > >> Signed-off-by: Ian Rogers <irogers@google.com> > >> --- > >> tools/perf/tests/shell/list.sh | 29 +++++++++++++++++++++++++++++ > >> 1 file changed, 29 insertions(+) > >> create mode 100755 tools/perf/tests/shell/list.sh > >> > >> diff --git a/tools/perf/tests/shell/list.sh b/tools/perf/tests/shell/list.sh > >> new file mode 100755 > >> index 000000000000..286879a9837a > >> --- /dev/null > >> +++ b/tools/perf/tests/shell/list.sh > >> @@ -0,0 +1,29 @@ > >> +#!/bin/sh > >> +# perf list tests > >> +# SPDX-License-Identifier: GPL-2.0 > >> + > >> +set -e > >> +err=0 > >> + > >> +if [ "x$PYTHON" == "x" ] > >> +then > >> + if which python3 > /dev/null > > > > 'which' isn't always present. Maybe > > > > python3 --version >/dev/null 2>&1 && PYTHON=python3 > > > > Now that we have shellcheck integrated into the build, we could enable > the POSIX mode test which would warn against this usage of which and > suggest the alternative. > > At the moment though there are several other usages of which already in > the tests. And probably enabling POSIX mode would come with hundreds of > other warnings to fix. > > I'm not saying we shouldn't change this instance though, just adding the > info for the discussion.
Sounds good to me. Fwiw, the instance where I lifted this code was: https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/shell/stat+json_output.sh?h=perf-tools-next#n12
With this change: ``` diff --git a/tools/perf/tests/Makefile.tests b/tools/perf/tests/Makefile.tests index fdaca5f7a946..06de6d3f4842 100644 --- a/tools/perf/tests/Makefile.tests +++ b/tools/perf/tests/Makefile.tests @@ -1,7 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 # Athira Rajeev <atrajeev@linux.vnet.ibm.com>, 2023
-PROGS := $(shell find tests/shell -perm -o=x -type f -name '*.sh') +PROGS := $(shell find tests/shell -executable -type f -name '*.sh') FILE_NAME := $(notdir $(PROGS)) FILE_NAME := $(FILE_NAME:%=.%) LOGS := $(join $(dir $(PROGS)),$(FILE_NAME)) ```
shellcheck now runs for me. I'll try adding the posix check into the patch series, as well as fixing other instances I can see.
Thanks, Ian
> >> + then > >> + PYTHON=python3 > >> + elif which python > /dev/null > >> + then > >> + PYTHON=python > >> + else > >> + echo Skipping test, python not detected please set environment variable PYTHON. > >> + exit 2 > >> + fi > >> +fi > >> + > >> +test_list_json() { > >> + echo "Json output test" > >> + perf list -j | $PYTHON -m json.tool > >> + echo "Json output test [Success]" > >> +} > >> + > >> +test_list_json > >> +exit $err > >
| |