diff mbox

[kvm-unit-tests,v3,1/2] run_tests: put logs into per-test file

Message ID 1483673581-7843-2-git-send-email-peterx@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Xu Jan. 6, 2017, 3:33 a.m. UTC 
We were using test.log before to keep all the test logs. This patch
creates one log file per test case under logs/ directory with name
"TESTNAME.log".

A new file global.bash is added to store global informations.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 Makefile               |  4 ++--
 run_tests.sh           | 14 ++++++++------
 scripts/functions.bash | 11 +++++++++--
 scripts/global.bash    |  2 ++
 4 files changed, 21 insertions(+), 10 deletions(-)
 create mode 100644 scripts/global.bash

Comments

Andrew Jones Jan. 6, 2017, 1:51 p.m. UTC | #1 
On Fri, Jan 06, 2017 at 11:33:00AM +0800, Peter Xu wrote:
> We were using test.log before to keep all the test logs. This patch
> creates one log file per test case under logs/ directory with name
> "TESTNAME.log".
> 
> A new file global.bash is added to store global informations.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  Makefile               |  4 ++--
>  run_tests.sh           | 14 ++++++++------
>  scripts/functions.bash | 11 +++++++++--
>  scripts/global.bash    |  2 ++
>  4 files changed, 21 insertions(+), 10 deletions(-)
>  create mode 100644 scripts/global.bash
> 
> diff --git a/Makefile b/Makefile
> index a32333b..f632c6c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -94,9 +94,9 @@ libfdt_clean:
>  	$(LIBFDT_objdir)/.*.d
>  
>  distclean: clean libfdt_clean
> -	$(RM) lib/asm config.mak $(TEST_DIR)-run test.log msr.out cscope.* \
> +	$(RM) lib/asm config.mak $(TEST_DIR)-run msr.out cscope.* \
>  	      build-head
> -	$(RM) -r tests
> +	$(RM) -r tests logs

We need .gitignore changes for this.

>  
>  cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/asm-generic
>  cscope:
> diff --git a/run_tests.sh b/run_tests.sh
> index 254129d..e1bb3a6 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -7,6 +7,7 @@ if [ ! -f config.mak ]; then
>      exit 1
>  fi
>  source config.mak
> +source scripts/global.bash
>  source scripts/functions.bash

Rather than add a new file, can't we just rename functions.bash to
something less function specific (common.bash?) and then add the
globals to that?

>  
>  function usage()
> @@ -46,17 +47,18 @@ while getopts "g:hv" opt; do
>      esac
>  done
>  
> -RUNTIME_log_stderr () { cat >> test.log; }
> +# RUNTIME_log_file will be configured later
> +RUNTIME_log_stderr () { cat >> $RUNTIME_log_file; }
>  RUNTIME_log_stdout () {
>      if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
> -        ./scripts/pretty_print_stacks.py $1 >> test.log
> +        ./scripts/pretty_print_stacks.py $1 >> $RUNTIME_log_file
>      else
> -        cat >> test.log
> +        cat >> $RUNTIME_log_file
>      fi
>  }
>  
> -
>  config=$TEST_DIR/unittests.cfg
> -rm -f test.log
> -printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
> +rm -rf $ut_log_dir

Instead of the 'rm', let's do
 'mv $ut_log_dir $ut_log_dir.old || { echo [...]; exit 2; }'
as I never liked that test.log was silently overwritten.

> +mkdir $ut_log_dir
> +printf "BUILD_HEAD=$(cat build-head)\n\n" > $ut_log_summary

I'm not sure we need the ut_ prefix on these variables. If we
do think we need to start prefixing variables, then I'd prefer
not to abbreviate, i.e. 'unittest_'

>  for_each_unittest $config run
> diff --git a/scripts/functions.bash b/scripts/functions.bash
> index ee9143c..d1d2e1c 100644
> --- a/scripts/functions.bash
> +++ b/scripts/functions.bash
> @@ -1,3 +1,10 @@
> +function run_task()
> +{
> +	local testname="$2"
> +
> +	RUNTIME_log_file="${ut_log_dir}/${testname}.log"
> +	"$@"
> +}
>  
>  function for_each_unittest()
>  {
> @@ -17,7 +24,7 @@ function for_each_unittest()
>  
>  	while read -u $fd line; do
>  		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
> -			"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> +			run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
>  			testname=${BASH_REMATCH[1]}
>  			smp=1
>  			kernel=""
> @@ -45,6 +52,6 @@ function for_each_unittest()
>  			timeout=${BASH_REMATCH[1]}
>  		fi
>  	done
> -	"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> +	run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
>  	exec {fd}<&-
>  }
> diff --git a/scripts/global.bash b/scripts/global.bash
> new file mode 100644
> index 0000000..77b0b29
> --- /dev/null
> +++ b/scripts/global.bash
> @@ -0,0 +1,2 @@
> +: ${ut_log_dir:=logs}
> +: ${ut_log_summary:=${ut_log_dir}/SUMMARY}

Do we even need these variables? When/why would someone override these
defaults?

Thanks,
drew
Peter Xu Jan. 9, 2017, 3:13 a.m. UTC | #2 
On Fri, Jan 06, 2017 at 02:51:58PM +0100, Andrew Jones wrote:
> On Fri, Jan 06, 2017 at 11:33:00AM +0800, Peter Xu wrote:
> > We were using test.log before to keep all the test logs. This patch
> > creates one log file per test case under logs/ directory with name
> > "TESTNAME.log".
> > 
> > A new file global.bash is added to store global informations.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  Makefile               |  4 ++--
> >  run_tests.sh           | 14 ++++++++------
> >  scripts/functions.bash | 11 +++++++++--
> >  scripts/global.bash    |  2 ++
> >  4 files changed, 21 insertions(+), 10 deletions(-)
> >  create mode 100644 scripts/global.bash
> > 
> > diff --git a/Makefile b/Makefile
> > index a32333b..f632c6c 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -94,9 +94,9 @@ libfdt_clean:
> >  	$(LIBFDT_objdir)/.*.d
> >  
> >  distclean: clean libfdt_clean
> > -	$(RM) lib/asm config.mak $(TEST_DIR)-run test.log msr.out cscope.* \
> > +	$(RM) lib/asm config.mak $(TEST_DIR)-run msr.out cscope.* \
> >  	      build-head
> > -	$(RM) -r tests
> > +	$(RM) -r tests logs
> 
> We need .gitignore changes for this.

Yep.

> 
> >  
> >  cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/asm-generic
> >  cscope:
> > diff --git a/run_tests.sh b/run_tests.sh
> > index 254129d..e1bb3a6 100755
> > --- a/run_tests.sh
> > +++ b/run_tests.sh
> > @@ -7,6 +7,7 @@ if [ ! -f config.mak ]; then
> >      exit 1
> >  fi
> >  source config.mak
> > +source scripts/global.bash
> >  source scripts/functions.bash
> 
> Rather than add a new file, can't we just rename functions.bash to
> something less function specific (common.bash?) and then add the
> globals to that?

Sure, common.bash is a good one, will use that.

> 
> >  
> >  function usage()
> > @@ -46,17 +47,18 @@ while getopts "g:hv" opt; do
> >      esac
> >  done
> >  
> > -RUNTIME_log_stderr () { cat >> test.log; }
> > +# RUNTIME_log_file will be configured later
> > +RUNTIME_log_stderr () { cat >> $RUNTIME_log_file; }
> >  RUNTIME_log_stdout () {
> >      if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
> > -        ./scripts/pretty_print_stacks.py $1 >> test.log
> > +        ./scripts/pretty_print_stacks.py $1 >> $RUNTIME_log_file
> >      else
> > -        cat >> test.log
> > +        cat >> $RUNTIME_log_file
> >      fi
> >  }
> >  
> > -
> >  config=$TEST_DIR/unittests.cfg
> > -rm -f test.log
> > -printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
> > +rm -rf $ut_log_dir
> 
> Instead of the 'rm', let's do
>  'mv $ut_log_dir $ut_log_dir.old || { echo [...]; exit 2; }'
> as I never liked that test.log was silently overwritten.

"Move" is indeed a much better way than "remove", especially "rm -rf"
(I think I should avoid using "rm -rf" in the future scripts as
well...). Will fix it.

> 
> > +mkdir $ut_log_dir
> > +printf "BUILD_HEAD=$(cat build-head)\n\n" > $ut_log_summary
> 
> I'm not sure we need the ut_ prefix on these variables. If we
> do think we need to start prefixing variables, then I'd prefer
> not to abbreviate, i.e. 'unittest_'

Since I would prefer a prefix, I'll use "unittest_" then.

[...]

> > diff --git a/scripts/global.bash b/scripts/global.bash
> > new file mode 100644
> > index 0000000..77b0b29
> > --- /dev/null
> > +++ b/scripts/global.bash
> > @@ -0,0 +1,2 @@
> > +: ${ut_log_dir:=logs}
> > +: ${ut_log_summary:=${ut_log_dir}/SUMMARY}
> 
> Do we even need these variables? When/why would someone override these
> defaults?

I use variables when I write constant more than once. This suites the
case for log_dir. I can remove the latter log_summary, but I'll still
prefer keep the log_dir if you don't mind.

Thanks!

-- peterx
diff mbox

Patch

diff --git a/Makefile b/Makefile
index a32333b..f632c6c 100644
--- a/Makefile
+++ b/Makefile
@@ -94,9 +94,9 @@  libfdt_clean:
 	$(LIBFDT_objdir)/.*.d
 
 distclean: clean libfdt_clean
-	$(RM) lib/asm config.mak $(TEST_DIR)-run test.log msr.out cscope.* \
+	$(RM) lib/asm config.mak $(TEST_DIR)-run msr.out cscope.* \
 	      build-head
-	$(RM) -r tests
+	$(RM) -r tests logs
 
 cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/asm-generic
 cscope:
diff --git a/run_tests.sh b/run_tests.sh
index 254129d..e1bb3a6 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -7,6 +7,7 @@  if [ ! -f config.mak ]; then
     exit 1
 fi
 source config.mak
+source scripts/global.bash
 source scripts/functions.bash
 
 function usage()
@@ -46,17 +47,18 @@  while getopts "g:hv" opt; do
     esac
 done
 
-RUNTIME_log_stderr () { cat >> test.log; }
+# RUNTIME_log_file will be configured later
+RUNTIME_log_stderr () { cat >> $RUNTIME_log_file; }
 RUNTIME_log_stdout () {
     if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
-        ./scripts/pretty_print_stacks.py $1 >> test.log
+        ./scripts/pretty_print_stacks.py $1 >> $RUNTIME_log_file
     else
-        cat >> test.log
+        cat >> $RUNTIME_log_file
     fi
 }
 
-
 config=$TEST_DIR/unittests.cfg
-rm -f test.log
-printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
+rm -rf $ut_log_dir
+mkdir $ut_log_dir
+printf "BUILD_HEAD=$(cat build-head)\n\n" > $ut_log_summary
 for_each_unittest $config run
diff --git a/scripts/functions.bash b/scripts/functions.bash
index ee9143c..d1d2e1c 100644
--- a/scripts/functions.bash
+++ b/scripts/functions.bash
@@ -1,3 +1,10 @@ 
+function run_task()
+{
+	local testname="$2"
+
+	RUNTIME_log_file="${ut_log_dir}/${testname}.log"
+	"$@"
+}
 
 function for_each_unittest()
 {
@@ -17,7 +24,7 @@  function for_each_unittest()
 
 	while read -u $fd line; do
 		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
-			"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+			run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
 			testname=${BASH_REMATCH[1]}
 			smp=1
 			kernel=""
@@ -45,6 +52,6 @@  function for_each_unittest()
 			timeout=${BASH_REMATCH[1]}
 		fi
 	done
-	"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+	run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
 	exec {fd}<&-
 }
diff --git a/scripts/global.bash b/scripts/global.bash
new file mode 100644
index 0000000..77b0b29
--- /dev/null
+++ b/scripts/global.bash
@@ -0,0 +1,2 @@ 
+: ${ut_log_dir:=logs}
+: ${ut_log_summary:=${ut_log_dir}/SUMMARY}