[PATCH 1/2] selftests: ftrace: Do not failure if there is unsupported tests

[PATCH 1/2] selftests: ftrace: Do not failure if there is unsupported tests

[Masami Hiramatsu]

Do not return failure exit code (1) for unsupported testcases,
since it is expected for stable kernels.

Previously, ftracetest is expected to run only on current
release for avoiding regressions. However, nowadays we run
it on stable kernels. This means some test cases must return
unsupported result. In such case, we should NOT exit
ftracetest with error status for unsupported results so that
kselftest (upper tests wrapper) shows it passed correctly.

Note that we continue to treat unresolved results as failure,
if test writers would like to notice user that the test result
should be reviewed, they can use exit_unresolved.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/testing/selftests/ftrace/ftracetest |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
index 14a03ea..290cd42 100755
--- a/tools/testing/selftests/ftrace/ftracetest
+++ b/tools/testing/selftests/ftrace/ftracetest
@@ -187,7 +187,7 @@ eval_result() { # sigval
     $UNSUPPORTED)
       prlog "	[UNSUPPORTED]"
       UNSUPPORTED_CASES="$UNSUPPORTED_CASES $CASENO"
-      return 1 # this is not a bug, but the result should be reported.
+      return 0 # this is not a bug.
     ;;
     $XFAIL)
       prlog "	[XFAIL]"

[PATCH 2/2] selftests: ftrace: Add more verbosity for immediate log

[Masami Hiramatsu]

Add 3-level verbosity for showing traced command log
on console immediately. Since some test cases can cause
kernel pacic if there is a probrem (like regression etc.),
we can not know which command caused the problem without
traced command log. This verbosity (-vvv) solves that
because it shows the log on console immediately. User
can get continuous command/error log.

Note that this is a kind of kernel debug mode, if you
don't see any kernel related issue, you don't need this
verbosity.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/testing/selftests/ftrace/ftracetest |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
index 290cd42..e3f1521 100755
--- a/tools/testing/selftests/ftrace/ftracetest
+++ b/tools/testing/selftests/ftrace/ftracetest
@@ -15,6 +15,7 @@ echo "		-h|--help  Show help message"
 echo "		-k|--keep  Keep passed test logs"
 echo "		-v|--verbose Increase verbosity of test messages"
 echo "		-vv        Alias of -v -v (Show all results in stdout)"
+echo "		-vvv       Alias of -v -v -v (Show all commands immediately)"
 echo "		-d|--debug Debug mode (trace all shell commands)"
 echo "		-l|--logdir <dir> Save logs on the <dir>"
 exit $1
@@ -56,9 +57,10 @@ parse_opts() { # opts
       KEEP_LOG=1
       shift 1
     ;;
-    --verbose|-v|-vv)
+    --verbose|-v|-vv|-vvv)
       VERBOSE=$((VERBOSE + 1))
       [ $1 = '-vv' ] && VERBOSE=$((VERBOSE + 1))
+      [ $1 = '-vvv' ] && VERBOSE=$((VERBOSE + 2))
       shift 1
     ;;
     --debug|-d)
@@ -252,7 +254,9 @@ run_test() { # testfile
   testcase $1
   echo "execute$INSTANCE: "$1 > $testlog
   SIG_RESULT=0
-  if [ $VERBOSE -ge 2 ]; then
+  if [ $VERBOSE -ge 3 ]; then
+    __run_test $1 | tee -a $testlog 2>&1
+  elif [ $VERBOSE -eq 2 ]; then
     __run_test $1 2>> $testlog | tee -a $testlog
   else
     __run_test $1 >> $testlog 2>&1

Re: [PATCH 2/2] selftests: ftrace: Add more verbosity for immediate log

[Steven Rostedt]

On Mon,  3 Jul 2017 14:26:40 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Add 3-level verbosity for showing traced command log
> on console immediately. Since some test cases can cause
> kernel pacic if there is a probrem (like regression etc.),
> we can not know which command caused the problem without
> traced command log. This verbosity (-vvv) solves that
> because it shows the log on console immediately. User
> can get continuous command/error log.
> 
> Note that this is a kind of kernel debug mode, if you
> don't see any kernel related issue, you don't need this
> verbosity.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  tools/testing/selftests/ftrace/ftracetest |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> index 290cd42..e3f1521 100755
> --- a/tools/testing/selftests/ftrace/ftracetest
> +++ b/tools/testing/selftests/ftrace/ftracetest
> @@ -15,6 +15,7 @@ echo "		-h|--help  Show help message"
>  echo "		-k|--keep  Keep passed test logs"
>  echo "		-v|--verbose Increase verbosity of test messages"
>  echo "		-vv        Alias of -v -v (Show all results in stdout)"
> +echo "		-vvv       Alias of -v -v -v (Show all commands immediately)"
>  echo "		-d|--debug Debug mode (trace all shell commands)"
>  echo "		-l|--logdir <dir> Save logs on the <dir>"
>  exit $1
> @@ -56,9 +57,10 @@ parse_opts() { # opts
>        KEEP_LOG=1
>        shift 1
>      ;;
> -    --verbose|-v|-vv)
> +    --verbose|-v|-vv|-vvv)
>        VERBOSE=$((VERBOSE + 1))
>        [ $1 = '-vv' ] && VERBOSE=$((VERBOSE + 1))
> +      [ $1 = '-vvv' ] && VERBOSE=$((VERBOSE + 2))
>        shift 1
>      ;;
>      --debug|-d)
> @@ -252,7 +254,9 @@ run_test() { # testfile
>    testcase $1
>    echo "execute$INSTANCE: "$1 > $testlog
>    SIG_RESULT=0
> -  if [ $VERBOSE -ge 2 ]; then
> +  if [ $VERBOSE -ge 3 ]; then
> +    __run_test $1 | tee -a $testlog 2>&1

I have nothing against this patch, but I want to point out that the
above is far from printing everything out immediately. The tee command
buffers a bit. At least it has for me in the past. If the kernel does
crash, one needs to still be a bit weary of where exactly the crash
happened.

Perhaps we need an option to print not to a log, but just let the test
print out directly?

-- Steve



> +  elif [ $VERBOSE -eq 2 ]; then
>      __run_test $1 2>> $testlog | tee -a $testlog
>    else
>      __run_test $1 >> $testlog 2>&1

Re: [PATCH 2/2] selftests: ftrace: Add more verbosity for immediate log

[Masami Hiramatsu]

On Mon, 3 Jul 2017 09:19:36 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Mon,  3 Jul 2017 14:26:40 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > Add 3-level verbosity for showing traced command log
> > on console immediately. Since some test cases can cause
> > kernel pacic if there is a probrem (like regression etc.),
> > we can not know which command caused the problem without
> > traced command log. This verbosity (-vvv) solves that
> > because it shows the log on console immediately. User
> > can get continuous command/error log.
> > 
> > Note that this is a kind of kernel debug mode, if you
> > don't see any kernel related issue, you don't need this
> > verbosity.
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  tools/testing/selftests/ftrace/ftracetest |    8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> > index 290cd42..e3f1521 100755
> > --- a/tools/testing/selftests/ftrace/ftracetest
> > +++ b/tools/testing/selftests/ftrace/ftracetest
> > @@ -15,6 +15,7 @@ echo "		-h|--help  Show help message"
> >  echo "		-k|--keep  Keep passed test logs"
> >  echo "		-v|--verbose Increase verbosity of test messages"
> >  echo "		-vv        Alias of -v -v (Show all results in stdout)"
> > +echo "		-vvv       Alias of -v -v -v (Show all commands immediately)"
> >  echo "		-d|--debug Debug mode (trace all shell commands)"
> >  echo "		-l|--logdir <dir> Save logs on the <dir>"
> >  exit $1
> > @@ -56,9 +57,10 @@ parse_opts() { # opts
> >        KEEP_LOG=1
> >        shift 1
> >      ;;
> > -    --verbose|-v|-vv)
> > +    --verbose|-v|-vv|-vvv)
> >        VERBOSE=$((VERBOSE + 1))
> >        [ $1 = '-vv' ] && VERBOSE=$((VERBOSE + 1))
> > +      [ $1 = '-vvv' ] && VERBOSE=$((VERBOSE + 2))
> >        shift 1
> >      ;;
> >      --debug|-d)
> > @@ -252,7 +254,9 @@ run_test() { # testfile
> >    testcase $1
> >    echo "execute$INSTANCE: "$1 > $testlog
> >    SIG_RESULT=0
> > -  if [ $VERBOSE -ge 2 ]; then
> > +  if [ $VERBOSE -ge 3 ]; then
> > +    __run_test $1 | tee -a $testlog 2>&1
> 
> I have nothing against this patch, but I want to point out that the
> above is far from printing everything out immediately. The tee command
> buffers a bit. At least it has for me in the past. If the kernel does
> crash, one needs to still be a bit weary of where exactly the crash
> happened.

Indeed.

> Perhaps we need an option to print not to a log, but just let the test
> print out directly?

OK, then I'd like to add "--logdir -" option instead of this.
(if someone can correct console log, they may not need logfile)

Thank you,

> 
> -- Steve
> 
> 
> 
> > +  elif [ $VERBOSE -eq 2 ]; then
> >      __run_test $1 2>> $testlog | tee -a $testlog
> >    else
> >      __run_test $1 >> $testlog 2>&1
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH 1/2] selftests: ftrace: Do not failure if there is unsupported tests

[Steven Rostedt]

On Mon,  3 Jul 2017 14:25:36 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Do not return failure exit code (1) for unsupported testcases,
> since it is expected for stable kernels.
> 
> Previously, ftracetest is expected to run only on current
> release for avoiding regressions. However, nowadays we run
> it on stable kernels. This means some test cases must return
> unsupported result. In such case, we should NOT exit
> ftracetest with error status for unsupported results so that
> kselftest (upper tests wrapper) shows it passed correctly.

I wonder if we should change kselftest instead. There are case where we
want to report "unsupported" as a failure. For instance, I have tests
where I enable everything, and if a test returns "unsupported" then it
is a failure for me.

Can we add an option in kselftest, or to ftracetest that decides if
unsupported is a failure or not? Otherwise I can not ack this patch.

-- Steve


> 
> Note that we continue to treat unresolved results as failure,
> if test writers would like to notice user that the test result
> should be reviewed, they can use exit_unresolved.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  tools/testing/selftests/ftrace/ftracetest |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> index 14a03ea..290cd42 100755
> --- a/tools/testing/selftests/ftrace/ftracetest
> +++ b/tools/testing/selftests/ftrace/ftracetest
> @@ -187,7 +187,7 @@ eval_result() { # sigval
>      $UNSUPPORTED)
>        prlog "	[UNSUPPORTED]"
>        UNSUPPORTED_CASES="$UNSUPPORTED_CASES $CASENO"
> -      return 1 # this is not a bug, but the result should be reported.
> +      return 0 # this is not a bug.
>      ;;
>      $XFAIL)
>        prlog "	[XFAIL]"

Re: [PATCH 1/2] selftests: ftrace: Do not failure if there is unsupported tests

[Masami Hiramatsu]

On Mon, 3 Jul 2017 09:15:43 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Mon,  3 Jul 2017 14:25:36 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > Do not return failure exit code (1) for unsupported testcases,
> > since it is expected for stable kernels.
> > 
> > Previously, ftracetest is expected to run only on current
> > release for avoiding regressions. However, nowadays we run
> > it on stable kernels. This means some test cases must return
> > unsupported result. In such case, we should NOT exit
> > ftracetest with error status for unsupported results so that
> > kselftest (upper tests wrapper) shows it passed correctly.
> 
> I wonder if we should change kselftest instead. There are case where we
> want to report "unsupported" as a failure. For instance, I have tests
> where I enable everything, and if a test returns "unsupported" then it
> is a failure for me.

OK, that's a possible usecase. 

> 
> Can we add an option in kselftest, or to ftracetest that decides if
> unsupported is a failure or not? Otherwise I can not ack this patch.

I would rather like to add an option to ftracetest instead of
kselftest, because whether the tested feature should be supported
or not is hard to decide from testing framework. It should be
checked by manual.

Thank you,

> 
> -- Steve
> 
> 
> > 
> > Note that we continue to treat unresolved results as failure,
> > if test writers would like to notice user that the test result
> > should be reviewed, they can use exit_unresolved.
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  tools/testing/selftests/ftrace/ftracetest |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> > index 14a03ea..290cd42 100755
> > --- a/tools/testing/selftests/ftrace/ftracetest
> > +++ b/tools/testing/selftests/ftrace/ftracetest
> > @@ -187,7 +187,7 @@ eval_result() { # sigval
> >      $UNSUPPORTED)
> >        prlog "	[UNSUPPORTED]"
> >        UNSUPPORTED_CASES="$UNSUPPORTED_CASES $CASENO"
> > -      return 1 # this is not a bug, but the result should be reported.
> > +      return 0 # this is not a bug.
> >      ;;
> >      $XFAIL)
> >        prlog "	[XFAIL]"
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH 1/2] selftests: ftrace: Do not failure if there is unsupported tests

[Steven Rostedt]

On Tue, 4 Jul 2017 00:52:32 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > 
> > Can we add an option in kselftest, or to ftracetest that decides if
> > unsupported is a failure or not? Otherwise I can not ack this patch.  
> 
> I would rather like to add an option to ftracetest instead of
> kselftest, because whether the tested feature should be supported
> or not is hard to decide from testing framework. It should be
> checked by manual.

Can we do both? That is, add an option to have ftracetest not fail on
"unsupported" but have it fail by default. We can have kselftest just
pass in a parameter to ftracetest that has unsupported not fail?

But if that is too difficult, then I can live with modifying my test
case to add the option.

-- Steve

Re: [PATCH 1/2] selftests: ftrace: Do not failure if there is unsupported tests

[Masami Hiramatsu]

On Mon, 3 Jul 2017 11:59:06 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue, 4 Jul 2017 00:52:32 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > 
> > > 
> > > Can we add an option in kselftest, or to ftracetest that decides if
> > > unsupported is a failure or not? Otherwise I can not ack this patch.  
> > 
> > I would rather like to add an option to ftracetest instead of
> > kselftest, because whether the tested feature should be supported
> > or not is hard to decide from testing framework. It should be
> > checked by manual.
> 
> Can we do both? That is, add an option to have ftracetest not fail on
> "unsupported" but have it fail by default. We can have kselftest just
> pass in a parameter to ftracetest that has unsupported not fail?

I rather like to treat unsupported as success (or XFAIL) by default
and add "--fail-unsupported" option.

Thank you,

> But if that is too difficult, then I can live with modifying my test
> case to add the option.
> 
> -- Steve


-- 
Masami Hiramatsu <mhiramat@kernel.org>