linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] perf test: Change to use bash for daemon test
@ 2021-03-20 10:45 Leo Yan
  2021-03-24  1:45 ` Namhyung Kim
  0 siblings, 1 reply; 3+ messages in thread
From: Leo Yan @ 2021-03-20 10:45 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa, Peter Zijlstra, Ingo Molnar,
	Mark Rutland, Alexander Shishkin, Namhyung Kim, Ian Rogers,
	linux-kernel
  Cc: Leo Yan

When executed the daemon test on Arm64 and x86 with Debian (Buster)
distro, both skip the test case with the log:

  # ./perf test -v 76
  76: daemon operations                                               :
  --- start ---
  test child forked, pid 11687
  test daemon list
  trap: SIGINT: bad trap
  ./tests/shell/daemon.sh: 173: local: cpu-clock: bad variable name
  test child finished with -2
  ---- end ----
  daemon operations: Skip

So the error happens for the variable expansion when use local variable
in the shell script.  Since Debian Buster uses dash but not bash as
non-interactive shell, when execute the daemon testing, it hits a
known issue for dash which was reported [1].

To resolve this issue, one option is to add double quotes for all local
variables assignment, so need to change the code from:

  local line=`perf daemon --config ${config} -x: | head -2 | tail -1`

  ... to:

  local line="`perf daemon --config ${config} -x: | head -2 | tail -1`"

But the testing script has bunch of local variables, this leads to big
changes for whole script.

On the other hand, the testing script asks to use the "local" feature
which is bash-specific, so this patch explicitly uses "#!/bin/bash" to
ensure running the script with bash.

After:

  # ./perf test -v 76
  76: daemon operations                                               :
  --- start ---
  test child forked, pid 11329
  test daemon list
  test daemon reconfig
  test daemon stop
  test daemon signal
  signal 12 sent to session 'test [11596]'
  signal 12 sent to session 'test [11596]'
  test daemon ping
  test daemon lock
  test child finished with 0
  ---- end ----
  daemon operations: Ok

[1] https://bugs.launchpad.net/ubuntu/+source/dash/+bug/139097

Fixes: 2291bb915b55 ("perf tests: Add daemon 'list' command test")
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/tests/shell/daemon.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
index ee4a30ca3f57..45fc24af5b07 100755
--- a/tools/perf/tests/shell/daemon.sh
+++ b/tools/perf/tests/shell/daemon.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 # daemon operations
 # SPDX-License-Identifier: GPL-2.0
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] perf test: Change to use bash for daemon test
  2021-03-20 10:45 [PATCH] perf test: Change to use bash for daemon test Leo Yan
@ 2021-03-24  1:45 ` Namhyung Kim
  2021-03-24 13:50   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 3+ messages in thread
From: Namhyung Kim @ 2021-03-24  1:45 UTC (permalink / raw)
  To: Leo Yan
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Peter Zijlstra, Ingo Molnar,
	Mark Rutland, Alexander Shishkin, Ian Rogers, linux-kernel

Hi Leo,

On Sat, Mar 20, 2021 at 7:46 PM Leo Yan <leo.yan@linaro.org> wrote:
>
> When executed the daemon test on Arm64 and x86 with Debian (Buster)
> distro, both skip the test case with the log:
>
>   # ./perf test -v 76
>   76: daemon operations                                               :
>   --- start ---
>   test child forked, pid 11687
>   test daemon list
>   trap: SIGINT: bad trap
>   ./tests/shell/daemon.sh: 173: local: cpu-clock: bad variable name
>   test child finished with -2
>   ---- end ----
>   daemon operations: Skip
>
> So the error happens for the variable expansion when use local variable
> in the shell script.  Since Debian Buster uses dash but not bash as
> non-interactive shell, when execute the daemon testing, it hits a
> known issue for dash which was reported [1].
>
> To resolve this issue, one option is to add double quotes for all local
> variables assignment, so need to change the code from:
>
>   local line=`perf daemon --config ${config} -x: | head -2 | tail -1`
>
>   ... to:
>
>   local line="`perf daemon --config ${config} -x: | head -2 | tail -1`"
>
> But the testing script has bunch of local variables, this leads to big
> changes for whole script.
>
> On the other hand, the testing script asks to use the "local" feature
> which is bash-specific, so this patch explicitly uses "#!/bin/bash" to
> ensure running the script with bash.
>
> After:
>
>   # ./perf test -v 76
>   76: daemon operations                                               :
>   --- start ---
>   test child forked, pid 11329
>   test daemon list
>   test daemon reconfig
>   test daemon stop
>   test daemon signal
>   signal 12 sent to session 'test [11596]'
>   signal 12 sent to session 'test [11596]'
>   test daemon ping
>   test daemon lock
>   test child finished with 0
>   ---- end ----
>   daemon operations: Ok
>
> [1] https://bugs.launchpad.net/ubuntu/+source/dash/+bug/139097
>
> Fixes: 2291bb915b55 ("perf tests: Add daemon 'list' command test")
> Signed-off-by: Leo Yan <leo.yan@linaro.org>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> ---
>  tools/perf/tests/shell/daemon.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
> index ee4a30ca3f57..45fc24af5b07 100755
> --- a/tools/perf/tests/shell/daemon.sh
> +++ b/tools/perf/tests/shell/daemon.sh
> @@ -1,4 +1,4 @@
> -#!/bin/sh
> +#!/bin/bash
>  # daemon operations
>  # SPDX-License-Identifier: GPL-2.0
>
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] perf test: Change to use bash for daemon test
  2021-03-24  1:45 ` Namhyung Kim
@ 2021-03-24 13:50   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-03-24 13:50 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Leo Yan, Jiri Olsa, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Alexander Shishkin, Ian Rogers, linux-kernel

Em Wed, Mar 24, 2021 at 10:45:22AM +0900, Namhyung Kim escreveu:
> On Sat, Mar 20, 2021 at 7:46 PM Leo Yan <leo.yan@linaro.org> wrote:
> > [1] https://bugs.launchpad.net/ubuntu/+source/dash/+bug/139097
> >
> > Fixes: 2291bb915b55 ("perf tests: Add daemon 'list' command test")
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> 
> Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks, applied.

- Arnaldo


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-03-24 13:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-20 10:45 [PATCH] perf test: Change to use bash for daemon test Leo Yan
2021-03-24  1:45 ` Namhyung Kim
2021-03-24 13:50   ` Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).