Re: [PATCH kvm-unit-tests v4 5/5] scripts: automatically pretty print stacks

[Peter Feiner <pfeiner@google.com>]

On Sat, Mar 05, 2016 at 06:29:50AM -0500, Andrew Jones wrote:
> 
> 
> ----- Original Message -----
> > Now run_tests.sh automatically pipes test output through
> > scripts/pretty_print_stacks.py. Can be disabled with
> > ./configure --disable-pretty-print-stacks.
> > 
> > Pretty printing disabled on standalone tests because there's no
> > binary.
> > 
> > Signed-off-by: Peter Feiner <pfeiner@google.com>
> > ---
> >  configure               | 10 ++++++++++
> >  run_tests.sh            | 10 ++++++++--
> >  scripts/mkstandalone.sh |  4 ++++
> >  3 files changed, 22 insertions(+), 2 deletions(-)
> > 
> > diff --git a/configure b/configure
> > index 958321d..ba6c55b 100755
> > --- a/configure
> > +++ b/configure
> > @@ -11,6 +11,7 @@ arch=`uname -m | sed -e
> > 's/i.86/i386/;s/arm.*/arm/;s/ppc64.*/ppc64/'`
> >  host=$arch
> >  cross_prefix=
> >  endian=""
> > +pretty_print_stacks=yes
> >  
> >  usage() {
> >      cat <<-EOF
> > @@ -25,6 +26,8 @@ usage() {
> >  	    --prefix=PREFIX        where to install things ($prefix)
> >  	    --kerneldir=DIR        kernel build directory for kvm.h ($kerneldir)
> >  	    --endian=ENDIAN        endianness to compile for (little or big, ppc64
> >  	    only)
> > +	    --[enable|disable]-pretty-print-stacks
> > +	                           enable or disable pretty stack printing (enabled
> > by default)
> >  EOF
> >      exit 1
> >  }
> > @@ -61,6 +64,12 @@ while [[ "$1" = -* ]]; do
> >  	--ld)
> >  	    ld="$arg"
> >  	    ;;
> > +	--enable-pretty-print-stacks)
> > +	    pretty_print_stacks=yes
> > +	    ;;
> > +	--disable-pretty-print-stacks)
> > +	    pretty_print_stacks=no
> > +	    ;;
> >  	--help)
> >  	    usage
> >  	    ;;
> > @@ -149,4 +158,5 @@ API=$api
> >  TEST_DIR=$testdir
> >  FIRMWARE=$firmware
> >  ENDIAN=$endian
> > +PRETTY_PRINT_STACKS=$pretty_print_stacks
> >  EOF
> > diff --git a/run_tests.sh b/run_tests.sh
> > index 89e8f84..9589252 100755
> > --- a/run_tests.sh
> > +++ b/run_tests.sh
> > @@ -28,7 +28,7 @@ EOF
> >  RUNTIME_arch_run="./$TEST_DIR/run"
> >  source scripts/runtime.bash
> >  
> > -while getopts "g:hv" opt; do
> > +while getopts "g:hvp" opt; do
> 
> Forgot to remove the 'p' here.
> 
> >      case $opt in
> >          g)
> >              only_group=$OPTARG
> > @@ -46,7 +46,13 @@ while getopts "g:hv" opt; do
> >      esac
> >  done
> >  
> > -RUNTIME_arch_run="./$TEST_DIR/run >> test.log"
> > +if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
> > +	log_redir="> >(./scripts/pretty_print_stacks.py \$kernel >> test.log)"
> > +else
> > +	log_redir=">> test.log"
> > +fi
> > +
> > +RUNTIME_arch_run="./$TEST_DIR/run $log_redir"
> >  config=$TEST_DIR/unittests.cfg
> >  rm -f test.log
> >  printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
> > diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> > index 1d3a20d..368fa4a 100755
> > --- a/scripts/mkstandalone.sh
> > +++ b/scripts/mkstandalone.sh
> > @@ -41,6 +41,10 @@ generate_test ()
> >  	echo "#!/bin/bash"
> >  	echo "export STANDALONE=yes"
> >  	echo "export HOST=\$(uname -m | sed -e s/i.86/i386/ | sed -e
> >  	's/arm.*/arm/')"
> > +
> > +	# Can't pretty print without binary.
> > +	echo "export PRETTY_PRINT_STACKS=no"
> 
> To be precise, "without the elf", because we do have the flat file.
> (And, for powerpc we actually even have the elf, so I guess we should
>  say "can't pretty print without the pretty print script" or something)
> 
> However, we don't even really need this right now, because the
> PRETTY_PRINT_STACKS variable is only consumed by run_tests.sh, which
> isn't used by standalone scripts. I don't mind adding the explicit =no
> here though, just in case something changes.

All good points. I just removed the comment altogether :-)

Peter