On Wed, Jan 11, 2017 at 06:12:44PM +0800, Peter Xu wrote:
> On Wed, Jan 11, 2017 at 10:51:12AM +0100, Andrew Jones wrote:
> > On Wed, Jan 11, 2017 at 10:06:07AM +0100, Andrew Jones wrote:
> > > On Wed, Jan 11, 2017 at 01:29:34PM +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". Meanwhile, we will keep the last time log into
> > > > logs.old/.
> > > >
> > > > Renaming scripts/functions.bash into scripts/common.bash to store some
> > > > more global variables.
> > > >
> > > > Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>
> > > > ---
> > > > .gitignore | 3 ++-
> > > > Makefile | 5 ++---
> > > > run_tests.sh | 18 +++++++++++-------
> > > > scripts/{functions.bash => common.bash} | 13 +++++++++++--
> > > > scripts/mkstandalone.sh | 2 +-
> > > > 5 files changed, 27 insertions(+), 14 deletions(-)
> > > > rename scripts/{functions.bash => common.bash} (75%)
> > > >
> > > > diff --git a/.gitignore b/.gitignore
> > > > index 3155418..2213b9b 100644
> > > > --- a/.gitignore
> > > > +++ b/.gitignore
> > > > @@ -12,7 +12,8 @@ cscope.*
> > > > /lib/asm
> > > > /config.mak
> > > > /*-run
> > > > -/test.log
> > > > /msr.out
> > > > /tests
> > > > /build-head
> > > > +/logs/
> > > > +/logs.old/
> > >
> > > We should send a cleanup patch fixing the other dirs too someday,
> > > patches, /lib/asm, and /tests
> > >
> > > > diff --git a/Makefile b/Makefile
> > > > index a32333b..844bacc 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -94,9 +94,8 @@ libfdt_clean:
> > > > $(LIBFDT_objdir)/.*.d
> > > >
> > > > distclean: clean libfdt_clean
> > > > - $(RM) lib/asm config.mak $(TEST_DIR)-run test.log msr.out cscope.* \
> > > > - build-head
> > > > - $(RM) -r tests
> > > > + $(RM) lib/asm config.mak $(TEST_DIR)-run msr.out cscope.* build-head
> > > > + $(RM) -r tests logs logs.old
> > > >
> > > > 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..b6a1059 100755
> > > > --- a/run_tests.sh
> > > > +++ b/run_tests.sh
> > > > @@ -7,7 +7,7 @@ if [ ! -f config.mak ]; then
> > > > exit 1
> > > > fi
> > > > source config.mak
> > > > -source scripts/functions.bash
> > > > +source scripts/common.bash
> > > >
> > > > function usage()
> > > > {
> > > > @@ -46,17 +46,21 @@ 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 $unittest_log_dir.old
> > > > +mv $unittest_log_dir $unittest_log_dir.old
> >
> > Actually this mv can fail, but we definitely wouldn't want to error-out
> > like v4 did, we want to ignore it. The error is that /logs doesn't exist,
> > as it won't the first time run_tests.sh is run. So, we need to redirect
> > stderr to /dev/null on this mv to avoid the message
> > "mv: cannot stat 'logs': No such file or directory"
> > on that first run.
>
> Hmm, yes.
>
> After a second thought, I think the mv can still fail even if this is
> not the first time we run the script - e.g. if the directory belongs
> to user1 but user2 runs run_tests.sh under it (while user2 don't have
> write permission on this directory). So I think we still need some way
> to stop the script if we detected that we might encounter issue.
I'm not really concerned about the user1/user2 scenario. The general use
of kvm-unit-tests is to build and run, meaning the user must have write
access to the dir in order to compile. But, ignoring the errors, as I
suggested, probably isn't a great idea either...
>
> We can do explicit check before the mv, making sure that the directory
> is there.
>
> So, how about this:
>
> rm -rf $unittest_log_dir.old || err "Failed remove old logs"
> if [[ -d $unittest_log_dir ]]; then
Only [ ... ] for tests like these
> mv $unittest_log_dir $unittest_log_dir.old ||
> err "Failed backup logs"
> fi
> mkdir $unittest_log_dir || err "Failed to create log dir"
>
> And define err() in common.bash:
>
> function err()
> {
> echo "$@"
> exit 1
> }
The above is mostly just translating rm/mv/mkdir stderr messages to
new messages. We can do it much more simply like
rm -rf logs.old
[ -d logs ] && mv logs logs.old
mkdir logs || exit 2
Thanks,
drew
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
