linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCH] selftests: ftrace: Allow some tests to be run in a tracing instance
@ 2017-04-20 17:30 Steven Rostedt
  2017-04-20 18:16 ` Steven Rostedt
  2017-04-21  9:36 ` Masami Hiramatsu
  0 siblings, 2 replies; 6+ messages in thread
From: Steven Rostedt @ 2017-04-20 17:30 UTC (permalink / raw)
  To: LKML; +Cc: Shuah Khan, Masami Hiramatsu, Namhyung Kim


An tracing instance has several of the same capabilities as the top level
instance, but may be implemented slightly different. Instead of just writing
tests that duplicat the same test cases of the top level instance, allow a
test to be written for both the top level as well as for an instance.

If a test case can be run in both the top level as well as in an tracing
instance directory, then it should have a ".itc" extension instead of just
the ".tc" extension.

Cc: Shuah Khan <shuah@kernel.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/testing/selftests/ftrace/ftracetest | 41 ++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
index a8631d9..889ec67 100755
--- a/tools/testing/selftests/ftrace/ftracetest
+++ b/tools/testing/selftests/ftrace/ftracetest
@@ -40,7 +40,11 @@ abspath() {
 }
 
 find_testcases() { #directory
-  echo `find $1 -name \*.tc | sort`
+  echo `find $1 -name \*.tc -o -name \*.itc | sort`
+}
+
+find_instance_testcases() { #directory
+  echo `find $1 -name \*.itc | sort`
 }
 
 parse_opts() { # opts
@@ -69,7 +73,7 @@ parse_opts() { # opts
       LOG_DIR=$2
       shift 2
     ;;
-    *.tc)
+    *.tc|*.itc)
       if [ -f "$1" ]; then
         OPT_TEST_CASES="$OPT_TEST_CASES `abspath $1`"
         shift 1
@@ -233,18 +237,28 @@ exit_xfail () {
 }
 trap 'SIG_RESULT=$XFAIL' $SIG_XFAIL
 
+INSTANCE_DIR="."
 __run_test() { # testfile
   # setup PID and PPID, $$ is not updated.
-  (cd $TRACING_DIR; read PID _ < /proc/self/stat; set -e; set -x; initialize_ftrace; . $1)
+  (cd $TRACING_DIR; read PID _ < /proc/self/stat; set -e; set -x; initialize_ftrace; cd $INSTANCE_DIR; . $1)
   [ $? -ne 0 ] && kill -s $SIG_FAIL $SIG_PID
 }
 
 # Run one test case
-run_test() { # testfile
+run_test() { # [-i] testfile
+  local makeinstance=0
+  if [ $1 == "-i" ]; then
+    makeinstance=1
+    shift
+  fi
   local testname=`basename $1`
   local testlog=`mktemp $LOG_DIR/${testname}-log.XXXXXX`
   export TMPDIR=`mktemp -d /tmp/ftracetest-dir.XXXXXX`
   testcase $1
+  if [ $makeinstance -eq 1 ]; then
+    INSTANCE_DIR=$TRACING_DIR/instances/${testname}_test
+    mkdir $INSTANCE_DIR
+  fi
   echo "execute: "$1 > $testlog
   SIG_RESULT=0
   if [ $VERBOSE -ge 2 ]; then
@@ -260,17 +274,36 @@ run_test() { # testfile
     [ $VERBOSE -ge 1 ] && catlog $testlog
     TOTAL_RESULT=1
   fi
+  if [ $makeinstance -eq 1 ]; then
+    rmdir $INSTANCE_DIR
+    INSTANCE_DIR="."
+  fi
   rm -rf $TMPDIR
 }
 
 # load in the helper functions
 . $TEST_DIR/functions
 
+RUN_INSTANCES=0
+
 # Main loop
 for t in $TEST_CASES; do
+  if [ "${t/*./}" == "itc" ]; then
+    RUN_INSTANCES=1
+  fi
   run_test $t
 done
 
+if [ $RUN_INSTANCES -eq 1 ]; then
+    echo "Running tests in an tracing instance:"
+    for t in $TEST_CASES; do
+	if [ "${t/*./}" != "itc" ]; then
+	    continue
+	fi
+	run_test -i $t
+    done
+fi
+
 prlog ""
 prlog "# of passed: " `echo $PASSED_CASES | wc -w`
 prlog "# of failed: " `echo $FAILED_CASES | wc -w`
-- 
2.9.3

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

* Re: [RFC][PATCH] selftests: ftrace: Allow some tests to be run in a tracing instance
  2017-04-20 17:30 [RFC][PATCH] selftests: ftrace: Allow some tests to be run in a tracing instance Steven Rostedt
@ 2017-04-20 18:16 ` Steven Rostedt
  2017-04-21  9:36 ` Masami Hiramatsu
  1 sibling, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2017-04-20 18:16 UTC (permalink / raw)
  To: LKML; +Cc: Shuah Khan, Masami Hiramatsu, Namhyung Kim

On Thu, 20 Apr 2017 13:30:34 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> An tracing instance has several of the same capabilities as the top level
> instance, but may be implemented slightly different. Instead of just writing
> tests that duplicat the same test cases of the top level instance, allow a
> test to be written for both the top level as well as for an instance.
> 
> If a test case can be run in both the top level as well as in an tracing
> instance directory, then it should have a ".itc" extension instead of just
> the ".tc" extension.
> 
> Cc: Shuah Khan <shuah@kernel.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---

Updated with:

diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
index 889ec67..5e04f53 100755
--- a/tools/testing/selftests/ftrace/ftracetest
+++ b/tools/testing/selftests/ftrace/ftracetest
@@ -295,7 +295,7 @@ for t in $TEST_CASES; do
 done
 
 if [ $RUN_INSTANCES -eq 1 ]; then
-    echo "Running tests in an tracing instance:"
+    echo "Running tests in a tracing instance:"
     for t in $TEST_CASES; do
 	if [ "${t/*./}" != "itc" ]; then
 	    continue


-- Steve

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

* Re: [RFC][PATCH] selftests: ftrace: Allow some tests to be run in a tracing instance
  2017-04-20 17:30 [RFC][PATCH] selftests: ftrace: Allow some tests to be run in a tracing instance Steven Rostedt
  2017-04-20 18:16 ` Steven Rostedt
@ 2017-04-21  9:36 ` Masami Hiramatsu
  2017-04-21 10:00   ` Masami Hiramatsu
  1 sibling, 1 reply; 6+ messages in thread
From: Masami Hiramatsu @ 2017-04-21  9:36 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: LKML, Shuah Khan, Masami Hiramatsu, Namhyung Kim

On Thu, 20 Apr 2017 13:30:34 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> An tracing instance has several of the same capabilities as the top level
> instance, but may be implemented slightly different. Instead of just writing
> tests that duplicat the same test cases of the top level instance, allow a
> test to be written for both the top level as well as for an instance.
> 
> If a test case can be run in both the top level as well as in an tracing
> instance directory, then it should have a ".itc" extension instead of just
> the ".tc" extension.

Hm, I'd rather like to have a tag for each script instead of
using the extension. E.g.

#!/bin/sh
# description: Kprobe dynamic event - busy event check
# test: instance

Thank you,

> 
> Cc: Shuah Khan <shuah@kernel.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  tools/testing/selftests/ftrace/ftracetest | 41 ++++++++++++++++++++++++++++---
>  1 file changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> index a8631d9..889ec67 100755
> --- a/tools/testing/selftests/ftrace/ftracetest
> +++ b/tools/testing/selftests/ftrace/ftracetest
> @@ -40,7 +40,11 @@ abspath() {
>  }
>  
>  find_testcases() { #directory
> -  echo `find $1 -name \*.tc | sort`
> +  echo `find $1 -name \*.tc -o -name \*.itc | sort`
> +}
> +
> +find_instance_testcases() { #directory
> +  echo `find $1 -name \*.itc | sort`
>  }
>  
>  parse_opts() { # opts
> @@ -69,7 +73,7 @@ parse_opts() { # opts
>        LOG_DIR=$2
>        shift 2
>      ;;
> -    *.tc)
> +    *.tc|*.itc)
>        if [ -f "$1" ]; then
>          OPT_TEST_CASES="$OPT_TEST_CASES `abspath $1`"
>          shift 1
> @@ -233,18 +237,28 @@ exit_xfail () {
>  }
>  trap 'SIG_RESULT=$XFAIL' $SIG_XFAIL
>  
> +INSTANCE_DIR="."
>  __run_test() { # testfile
>    # setup PID and PPID, $$ is not updated.
> -  (cd $TRACING_DIR; read PID _ < /proc/self/stat; set -e; set -x; initialize_ftrace; . $1)
> +  (cd $TRACING_DIR; read PID _ < /proc/self/stat; set -e; set -x; initialize_ftrace; cd $INSTANCE_DIR; . $1)
>    [ $? -ne 0 ] && kill -s $SIG_FAIL $SIG_PID
>  }
>  
>  # Run one test case
> -run_test() { # testfile
> +run_test() { # [-i] testfile
> +  local makeinstance=0
> +  if [ $1 == "-i" ]; then
> +    makeinstance=1
> +    shift
> +  fi
>    local testname=`basename $1`
>    local testlog=`mktemp $LOG_DIR/${testname}-log.XXXXXX`
>    export TMPDIR=`mktemp -d /tmp/ftracetest-dir.XXXXXX`
>    testcase $1
> +  if [ $makeinstance -eq 1 ]; then
> +    INSTANCE_DIR=$TRACING_DIR/instances/${testname}_test
> +    mkdir $INSTANCE_DIR
> +  fi
>    echo "execute: "$1 > $testlog
>    SIG_RESULT=0
>    if [ $VERBOSE -ge 2 ]; then
> @@ -260,17 +274,36 @@ run_test() { # testfile
>      [ $VERBOSE -ge 1 ] && catlog $testlog
>      TOTAL_RESULT=1
>    fi
> +  if [ $makeinstance -eq 1 ]; then
> +    rmdir $INSTANCE_DIR
> +    INSTANCE_DIR="."
> +  fi
>    rm -rf $TMPDIR
>  }
>  
>  # load in the helper functions
>  . $TEST_DIR/functions
>  
> +RUN_INSTANCES=0
> +
>  # Main loop
>  for t in $TEST_CASES; do
> +  if [ "${t/*./}" == "itc" ]; then
> +    RUN_INSTANCES=1
> +  fi
>    run_test $t
>  done
>  
> +if [ $RUN_INSTANCES -eq 1 ]; then
> +    echo "Running tests in an tracing instance:"
> +    for t in $TEST_CASES; do
> +	if [ "${t/*./}" != "itc" ]; then
> +	    continue
> +	fi
> +	run_test -i $t
> +    done
> +fi
> +
>  prlog ""
>  prlog "# of passed: " `echo $PASSED_CASES | wc -w`
>  prlog "# of failed: " `echo $FAILED_CASES | wc -w`
> -- 
> 2.9.3
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

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

* Re: [RFC][PATCH] selftests: ftrace: Allow some tests to be run in a tracing instance
  2017-04-21  9:36 ` Masami Hiramatsu
@ 2017-04-21 10:00   ` Masami Hiramatsu
  2017-04-21 13:53     ` Steven Rostedt
  0 siblings, 1 reply; 6+ messages in thread
From: Masami Hiramatsu @ 2017-04-21 10:00 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: Steven Rostedt, LKML, Shuah Khan, Namhyung Kim

On Fri, 21 Apr 2017 18:36:17 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> On Thu, 20 Apr 2017 13:30:34 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > An tracing instance has several of the same capabilities as the top level
> > instance, but may be implemented slightly different. Instead of just writing
> > tests that duplicat the same test cases of the top level instance, allow a
> > test to be written for both the top level as well as for an instance.
> > 
> > If a test case can be run in both the top level as well as in an tracing
> > instance directory, then it should have a ".itc" extension instead of just
> > the ".tc" extension.
> 
> Hm, I'd rather like to have a tag for each script instead of
> using the extension. E.g.
> 
> #!/bin/sh
> # description: Kprobe dynamic event - busy event check
> # test: instance

BTW, I also have some comments, see below,

[...]
> > +INSTANCE_DIR="."
> >  __run_test() { # testfile
> >    # setup PID and PPID, $$ is not updated.
> > -  (cd $TRACING_DIR; read PID _ < /proc/self/stat; set -e; set -x; initialize_ftrace; . $1)
> > +  (cd $TRACING_DIR; read PID _ < /proc/self/stat; set -e; set -x; initialize_ftrace; cd $INSTANCE_DIR; . $1)

You can just change "cd $TRACING_DIR;" with "cd $TRACING_DIR/$INSTANCE_DIR;"
Then, the default value of INSTANCE_DIR can be "".

> >    [ $? -ne 0 ] && kill -s $SIG_FAIL $SIG_PID
> >  }
> >  
> >  # Run one test case
> > -run_test() { # testfile
> > +run_test() { # [-i] testfile
> > +  local makeinstance=0
> > +  if [ $1 == "-i" ]; then
> > +    makeinstance=1
> > +    shift
> > +  fi

No, don't parse argument in the function. You can just define
MAKEINSTANCE=1 global variable before calling this.

> >    local testname=`basename $1`
> >    local testlog=`mktemp $LOG_DIR/${testname}-log.XXXXXX`
> >    export TMPDIR=`mktemp -d /tmp/ftracetest-dir.XXXXXX`
> >    testcase $1

Anyway, if we use a "tag" in the script header, we can get it
in this testcase() and update TEST_TAGS global variable there.

> > +  if [ $makeinstance -eq 1 ]; then

and check whether "instance" tag in $TEST_TAGS here.

> > +    INSTANCE_DIR=$TRACING_DIR/instances/${testname}_test
> > +    mkdir $INSTANCE_DIR
> > +  fi


> >    echo "execute: "$1 > $testlog
> >    SIG_RESULT=0
> >    if [ $VERBOSE -ge 2 ]; then
> > @@ -260,17 +274,36 @@ run_test() { # testfile
> >      [ $VERBOSE -ge 1 ] && catlog $testlog
> >      TOTAL_RESULT=1
> >    fi
> > +  if [ $makeinstance -eq 1 ]; then

Here also we can just check the "$INSTANCE_DIR" is not empty.

Thank you,

> > +    rmdir $INSTANCE_DIR
> > +    INSTANCE_DIR="."
> > +  fi
> >    rm -rf $TMPDIR
> >  }
> >  
> >  # load in the helper functions
> >  . $TEST_DIR/functions
> >  
> > +RUN_INSTANCES=0
> > +
> >  # Main loop
> >  for t in $TEST_CASES; do
> > +  if [ "${t/*./}" == "itc" ]; then
> > +    RUN_INSTANCES=1
> > +  fi
> >    run_test $t
> >  done
> >  
> > +if [ $RUN_INSTANCES -eq 1 ]; then
> > +    echo "Running tests in an tracing instance:"
> > +    for t in $TEST_CASES; do
> > +	if [ "${t/*./}" != "itc" ]; then
> > +	    continue
> > +	fi
> > +	run_test -i $t
> > +    done
> > +fi
> > +
> >  prlog ""
> >  prlog "# of passed: " `echo $PASSED_CASES | wc -w`
> >  prlog "# of failed: " `echo $FAILED_CASES | wc -w`
> > -- 
> > 2.9.3
> > 
> 
> 
> -- 
> Masami Hiramatsu <mhiramat@kernel.org>


-- 
Masami Hiramatsu <mhiramat@kernel.org>

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

* Re: [RFC][PATCH] selftests: ftrace: Allow some tests to be run in a tracing instance
  2017-04-21 10:00   ` Masami Hiramatsu
@ 2017-04-21 13:53     ` Steven Rostedt
  2017-04-21 22:26       ` Masami Hiramatsu
  0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2017-04-21 13:53 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: LKML, Shuah Khan, Namhyung Kim

On Fri, 21 Apr 2017 19:00:25 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> On Fri, 21 Apr 2017 18:36:17 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > On Thu, 20 Apr 2017 13:30:34 -0400
> > Steven Rostedt <rostedt@goodmis.org> wrote:  
> > > 
> > > An tracing instance has several of the same capabilities as the top level
> > > instance, but may be implemented slightly different. Instead of just writing
> > > tests that duplicat the same test cases of the top level instance, allow a
> > > test to be written for both the top level as well as for an instance.
> > > 
> > > If a test case can be run in both the top level as well as in an tracing
> > > instance directory, then it should have a ".itc" extension instead of just
> > > the ".tc" extension.  
> > 
> > Hm, I'd rather like to have a tag for each script instead of
> > using the extension. E.g.

OK, I'll update. I just finished testing all this too :-/

> > 
> > #!/bin/sh
> > # description: Kprobe dynamic event - busy event check
> > # test: instance  

Hmm, what about "flags:" instead. As a way to denote what this test has.

 # flags: instance

> 
> BTW, I also have some comments, see below,
> 
> [...]
> > > +INSTANCE_DIR="."
> > >  __run_test() { # testfile
> > >    # setup PID and PPID, $$ is not updated.
> > > -  (cd $TRACING_DIR; read PID _ < /proc/self/stat; set -e; set -x; initialize_ftrace; . $1)
> > > +  (cd $TRACING_DIR; read PID _ < /proc/self/stat; set -e; set -x; initialize_ftrace; cd $INSTANCE_DIR; . $1)  
> 
> You can just change "cd $TRACING_DIR;" with "cd $TRACING_DIR/$INSTANCE_DIR;"
> Then, the default value of INSTANCE_DIR can be "".

I'll look into doing this. I originally had an option to run just
instance tests, and overriding TRACING_DIR wasn't an option at the time.

> 
> > >    [ $? -ne 0 ] && kill -s $SIG_FAIL $SIG_PID
> > >  }
> > >  
> > >  # Run one test case
> > > -run_test() { # testfile
> > > +run_test() { # [-i] testfile
> > > +  local makeinstance=0
> > > +  if [ $1 == "-i" ]; then
> > > +    makeinstance=1
> > > +    shift
> > > +  fi  
> 
> No, don't parse argument in the function. You can just define
> MAKEINSTANCE=1 global variable before calling this.

Again, this was due to the ftracetest option. I'll update.

> 
> > >    local testname=`basename $1`
> > >    local testlog=`mktemp $LOG_DIR/${testname}-log.XXXXXX`
> > >    export TMPDIR=`mktemp -d /tmp/ftracetest-dir.XXXXXX`
> > >    testcase $1  
> 
> Anyway, if we use a "tag" in the script header, we can get it
> in this testcase() and update TEST_TAGS global variable there.

TEST_TAGS? Or you mean something like the way to get the description.

> 
> > > +  if [ $makeinstance -eq 1 ]; then  
> 
> and check whether "instance" tag in $TEST_TAGS here.

TEST_FLAGS?

> 
> > > +    INSTANCE_DIR=$TRACING_DIR/instances/${testname}_test
> > > +    mkdir $INSTANCE_DIR
> > > +  fi  
> 
> 
> > >    echo "execute: "$1 > $testlog
> > >    SIG_RESULT=0
> > >    if [ $VERBOSE -ge 2 ]; then
> > > @@ -260,17 +274,36 @@ run_test() { # testfile
> > >      [ $VERBOSE -ge 1 ] && catlog $testlog
> > >      TOTAL_RESULT=1
> > >    fi
> > > +  if [ $makeinstance -eq 1 ]; then  
> 
> Here also we can just check the "$INSTANCE_DIR" is not empty.

Why? It never will be empty. It's a pseudo dir and files can not be
created or destroyed in it by the user. It should be created by the
test, and destroyed by the test. Hopefully it's a unique name. The name
is currently something like instances/func_event_triggers.tc_test
It's "instances/${testname}_test". I could even add a $$ just to be
more random here.

> 
> Thank you,

Thanks for you review! I'll have a new patch out soon.

-- Steve

> 
> > > +    rmdir $INSTANCE_DIR
> > > +    INSTANCE_DIR="."
> > > +  fi
> > >    rm -rf $TMPDIR
> > >  }
> > >  
> > >  # load in the helper functions
> > >  . $TEST_DIR/functions
> > >  
> > > +RUN_INSTANCES=0
> > > +
> > >  # Main loop
> > >  for t in $TEST_CASES; do
> > > +  if [ "${t/*./}" == "itc" ]; then
> > > +    RUN_INSTANCES=1
> > > +  fi
> > >    run_test $t
> > >  done
> > >  
> > > +if [ $RUN_INSTANCES -eq 1 ]; then
> > > +    echo "Running tests in an tracing instance:"
> > > +    for t in $TEST_CASES; do
> > > +	if [ "${t/*./}" != "itc" ]; then
> > > +	    continue
> > > +	fi
> > > +	run_test -i $t
> > > +    done
> > > +fi
> > > +
> > >  prlog ""
> > >  prlog "# of passed: " `echo $PASSED_CASES | wc -w`
> > >  prlog "# of failed: " `echo $FAILED_CASES | wc -w`
> > > -- 
> > > 2.9.3
> > >   
> > 
> > 
> > -- 
> > Masami Hiramatsu <mhiramat@kernel.org>  
> 
> 

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

* Re: [RFC][PATCH] selftests: ftrace: Allow some tests to be run in a tracing instance
  2017-04-21 13:53     ` Steven Rostedt
@ 2017-04-21 22:26       ` Masami Hiramatsu
  0 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2017-04-21 22:26 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: LKML, Shuah Khan, Namhyung Kim

On Fri, 21 Apr 2017 09:53:34 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Fri, 21 Apr 2017 19:00:25 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > On Fri, 21 Apr 2017 18:36:17 +0900
> > Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > 
> > > On Thu, 20 Apr 2017 13:30:34 -0400
> > > Steven Rostedt <rostedt@goodmis.org> wrote:  
> > > > 
> > > > An tracing instance has several of the same capabilities as the top level
> > > > instance, but may be implemented slightly different. Instead of just writing
> > > > tests that duplicat the same test cases of the top level instance, allow a
> > > > test to be written for both the top level as well as for an instance.

BTW, would you need "testcases only for instance" or
"testcase for top-level and instance" ? I thought latter...

> > > > 
> > > > If a test case can be run in both the top level as well as in an tracing
> > > > instance directory, then it should have a ".itc" extension instead of just
> > > > the ".tc" extension.  
> > > 
> > > Hm, I'd rather like to have a tag for each script instead of
> > > using the extension. E.g.
> 
> OK, I'll update. I just finished testing all this too :-/
> 
> > > 
> > > #!/bin/sh
> > > # description: Kprobe dynamic event - busy event check
> > > # test: instance  
> 
> Hmm, what about "flags:" instead. As a way to denote what this test has.

OK, that's better.

> 
>  # flags: instance
> 
> > 
> > BTW, I also have some comments, see below,
> > 
> > [...]
> > > > +INSTANCE_DIR="."
> > > >  __run_test() { # testfile
> > > >    # setup PID and PPID, $$ is not updated.
> > > > -  (cd $TRACING_DIR; read PID _ < /proc/self/stat; set -e; set -x; initialize_ftrace; . $1)
> > > > +  (cd $TRACING_DIR; read PID _ < /proc/self/stat; set -e; set -x; initialize_ftrace; cd $INSTANCE_DIR; . $1)  
> > 
> > You can just change "cd $TRACING_DIR;" with "cd $TRACING_DIR/$INSTANCE_DIR;"
> > Then, the default value of INSTANCE_DIR can be "".
> 
> I'll look into doing this. I originally had an option to run just
> instance tests, and overriding TRACING_DIR wasn't an option at the time.

My idea is using INSTANCE_DIR not only for instance name, but also
for a flag which indicates "instance test is running, need to be removed".


> > > >    [ $? -ne 0 ] && kill -s $SIG_FAIL $SIG_PID
> > > >  }
> > > >  
> > > >  # Run one test case
> > > > -run_test() { # testfile
> > > > +run_test() { # [-i] testfile
> > > > +  local makeinstance=0
> > > > +  if [ $1 == "-i" ]; then
> > > > +    makeinstance=1
> > > > +    shift
> > > > +  fi  
> > 
> > No, don't parse argument in the function. You can just define
> > MAKEINSTANCE=1 global variable before calling this.
> 
> Again, this was due to the ftracetest option. I'll update.

I see.

> 
> > 
> > > >    local testname=`basename $1`
> > > >    local testlog=`mktemp $LOG_DIR/${testname}-log.XXXXXX`
> > > >    export TMPDIR=`mktemp -d /tmp/ftracetest-dir.XXXXXX`
> > > >    testcase $1  
> > 
> > Anyway, if we use a "tag" in the script header, we can get it
> > in this testcase() and update TEST_TAGS global variable there.
> 
> TEST_TAGS? Or you mean something like the way to get the description.
> 
> > 
> > > > +  if [ $makeinstance -eq 1 ]; then  
> > 
> > and check whether "instance" tag in $TEST_TAGS here.
> 
> TEST_FLAGS?

Yes, that's good to me :)

> 
> > 
> > > > +    INSTANCE_DIR=$TRACING_DIR/instances/${testname}_test
> > > > +    mkdir $INSTANCE_DIR
> > > > +  fi  
> > 
> > 
> > > >    echo "execute: "$1 > $testlog
> > > >    SIG_RESULT=0
> > > >    if [ $VERBOSE -ge 2 ]; then
> > > > @@ -260,17 +274,36 @@ run_test() { # testfile
> > > >      [ $VERBOSE -ge 1 ] && catlog $testlog
> > > >      TOTAL_RESULT=1
> > > >    fi
> > > > +  if [ $makeinstance -eq 1 ]; then  
> > 
> > Here also we can just check the "$INSTANCE_DIR" is not empty.
> 
> Why? It never will be empty. It's a pseudo dir and files can not be
> created or destroyed in it by the user. It should be created by the
> test, and destroyed by the test. Hopefully it's a unique name. The name
> is currently something like instances/func_event_triggers.tc_test
> It's "instances/${testname}_test". I could even add a $$ just to be
> more random here.

As I said above, INSTANCE_DIR can be a test flag too, and that
ensures we've actually made it. (Just for make sure there is)
BTW, maybe we can use mktemp if you'd like avoid name conflict.

Thank you,

> 
> > 
> > Thank you,
> 
> Thanks for you review! I'll have a new patch out soon.
> 
> -- Steve
> 
> > 
> > > > +    rmdir $INSTANCE_DIR
> > > > +    INSTANCE_DIR="."
> > > > +  fi
> > > >    rm -rf $TMPDIR
> > > >  }
> > > >  
> > > >  # load in the helper functions
> > > >  . $TEST_DIR/functions
> > > >  
> > > > +RUN_INSTANCES=0
> > > > +
> > > >  # Main loop
> > > >  for t in $TEST_CASES; do
> > > > +  if [ "${t/*./}" == "itc" ]; then
> > > > +    RUN_INSTANCES=1
> > > > +  fi
> > > >    run_test $t
> > > >  done
> > > >  
> > > > +if [ $RUN_INSTANCES -eq 1 ]; then
> > > > +    echo "Running tests in an tracing instance:"
> > > > +    for t in $TEST_CASES; do
> > > > +	if [ "${t/*./}" != "itc" ]; then
> > > > +	    continue
> > > > +	fi
> > > > +	run_test -i $t
> > > > +    done
> > > > +fi
> > > > +
> > > >  prlog ""
> > > >  prlog "# of passed: " `echo $PASSED_CASES | wc -w`
> > > >  prlog "# of failed: " `echo $FAILED_CASES | wc -w`
> > > > -- 
> > > > 2.9.3
> > > >   
> > > 
> > > 
> > > -- 
> > > Masami Hiramatsu <mhiramat@kernel.org>  
> > 
> > 
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

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

end of thread, other threads:[~2017-04-21 22:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-20 17:30 [RFC][PATCH] selftests: ftrace: Allow some tests to be run in a tracing instance Steven Rostedt
2017-04-20 18:16 ` Steven Rostedt
2017-04-21  9:36 ` Masami Hiramatsu
2017-04-21 10:00   ` Masami Hiramatsu
2017-04-21 13:53     ` Steven Rostedt
2017-04-21 22:26       ` Masami Hiramatsu

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).