Re: [PATCH] selftests/ftrace: skip ftrace test if FTRACE was not enabled

[Po-Hsu Lin]

Hello,
Thanks all for your feedback!

On Wed, Jul 3, 2019 at 8:11 AM Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
>
> Hi Po-Hsu Lin,
>
> On Tue, 2 Jul 2019 13:22:26 -0600
> shuah <shuah@xxxxxxxxxx> wrote:
>
> > Hi Po-Hsu Lin,
> >
> > On 7/2/19 12:23 AM, Po-Hsu Lin wrote:
> > > The ftrace test will need to have CONFIG_FTRACE enabled to make the
> > > ftrace directory available.
> > >
> > > Add an additional check to skip this test if the CONFIG_FTRACE was not
> > > enabled.
>
> Sorry, NAK for config check.
>
> > >
> > > This will be helpful to avoid a false-positive test result when testing
> > > it directly with the following commad against a kernel that does not
> > > have CONFIG_FTRACE enabled:
>
> Would you know tools/testing/selftests/ftrace/config (and other config files
> in each tests) ?
>
> Since each selftest depends specific configurations, those configs are
> written in config file, and tester must enable it using
> "scripts/kconfig/merge_config.sh".
>
> We can not check the kernel config in some cases, e.g. distro kernel,
> cross-build kernel, remote build kernel etc. Also, the .config file
> can be a config file for another kernel build.
>
> So please take care of your kernel configuration. If you find any test
> failed even if you enable configs in config file under that test, please
> report it, since that is a bug.
>
Yes I'm aware the config file.

People can still run a test directly without enabling corresponding
configs, so I think it's better to skip it if possible, directory
checking proposed by Steven looks better for this.

And thank you the explanation about the possible issue for a config check.

>
> Thank you,
>
> > >      make -C tools/testing/selftests TARGETS=ftrace run_tests
> > >
> > > The test result on an Ubuntu KVM kernel will be changed from:
> > >      selftests: ftrace: ftracetest
> > >      ========================================
> > >      Error: No ftrace directory found
> > >      not ok 1..1 selftests: ftrace: ftracetest [FAIL]
> > > To:
> >
> > Thanks for the patch.
> >
> > Check patch fails with the above To:
> >
> > WARNING: Use a single space after To:
> > #107:
> > To:
> >
> > ERROR: Unrecognized email address: ''
> > #107:
> > To:
> >
> > total: 1 errors, 1 warnings, 23 lines checked
> >

This is an interesting error here,
it looks like this checkpatch.pl script cannot identify the "To:" is
actually located inside the commit message.
I have to change this into something else to avoid this error.


> >
> > Please fix and send v2.
> >
> > >      selftests: ftrace: ftracetest
> > >      ========================================
> > >      CONFIG_FTRACE was not enabled, test skipped.
> > >      not ok 1..1 selftests: ftrace: ftracetest [SKIP]
> > >
> > > Signed-off-by: Po-Hsu Lin <po-hsu.lin@xxxxxxxxxxxxx>
> > > ---
> > >   tools/testing/selftests/ftrace/ftracetest | 11 ++++++++++-
> > >   1 file changed, 10 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> > > index 6d5e9e8..6c8322e 100755
> > > --- a/tools/testing/selftests/ftrace/ftracetest
> > > +++ b/tools/testing/selftests/ftrace/ftracetest
> > > @@ -7,6 +7,9 @@
> > >   #  Written by Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>
> > >   #
> >
> > Hmm. You havem't cc'ed Masami on this. Adding Masami.
> >
> > I would think Masami should be on the Signed-off-by as well,
> > since he is the author.
> >
> > >
> > > +# Kselftest framework requirement - SKIP code is 4.
> > > +ksft_skip=4
> > > +
> > >   usage() { # errno [message]
> > >   [ ! -z "$2" ] && echo $2
> > >   echo "Usage: ftracetest [options] [testcase(s)] [testcase-directory(s)]"
> > > @@ -139,7 +142,13 @@ parse_opts $*
> > >
> > >   # Verify parameters
> > >   if [ -z "$TRACING_DIR" -o ! -d "$TRACING_DIR" ]; then
> > > -  errexit "No ftrace directory found"
> > > +  ftrace_enabled=`grep "^CONFIG_FTRACE=y" /lib/modules/$(uname -r)/build/.config`
> > > +  if [ -z "$ftrace_enabled" ]; then
> > > +    echo "CONFIG_FTRACE was not enabled, test skipped."
> > > +    exit $ksft_skip
> > > +  else
> > > +    errexit "No ftrace directory found"
> > > +  fi
> > >   fi
> > >
> > >   # Preparing logs
> > >
> >
> > thanks,
> > -- Shuah
>
>
> --
> Masami Hiramatsu <mhiramat@xxxxxxxxxx>