Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Message-ID: <03b314c6-ed6d-ae17-5bc5-0170139f7feb@arm.com>
Date:   Wed, 29 Nov 2023 09:27:55 +0000
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [PATCH v1 2/2] perf test: Add basic list test
Content-Language: en-US
To:     Adrian Hunter <adrian.hunter@intel.com>,
        Ian Rogers <irogers@google.com>,
        Athira Rajeev <atrajeev@linux.vnet.ibm.com>
References: <20231129081004.1918096-1-irogers@google.com>
 <20231129081004.1918096-2-irogers@google.com>
 <a54d0a93-aacd-4c69-a34c-8628b0e18ee7@intel.com>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Kan Liang <kan.liang@linux.intel.com>,
        linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org
From:   James Clark <james.clark@arm.com>
In-Reply-To: <a54d0a93-aacd-4c69-a34c-8628b0e18ee7@intel.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk


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.

>> +	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
> 
>