Messages in this thread | | | From | Ian Rogers <> | Date | Wed, 29 Nov 2023 13:37:53 -0800 | Subject | Re: [PATCH v1 2/2] perf test: Add basic list test |
| |
On Wed, Nov 29, 2023 at 12:30 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > 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?
Yep, sent: https://lore.kernel.org/lkml/20231129213428.2227448-1-irogers@google.com/
There are 2 fixes, one for perf list and the other for the shellcheck log file building stuff. The shellcheck stuff took a little longer PTAL.
Thanks, Ian
> - Arnaldo
| |