[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [kvm-unit-tests PATCH v3 1/2] run_tests: put logs into
From: |
Andrew Jones |
Subject: |
Re: [Qemu-devel] [kvm-unit-tests PATCH v3 1/2] run_tests: put logs into per-test file |
Date: |
Fri, 6 Jan 2017 14:51:58 +0100 |
User-agent: |
Mutt/1.6.0.1 (2016-04-01) |
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 <address@hidden>
> ---
> 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