Messages in this thread | | | Date | Wed, 29 Nov 2023 17:30:40 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH v1 2/2] perf test: Add basic list test |
| |
Em Wed, Nov 29, 2023 at 09:21:12AM -0800, Ian Rogers escreveu: > 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.
So I'll wait for a v2 for this one, ok?
- Arnaldo
| |