From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Jones <drjones@redhat.com>
Subject: Re: [kvm-unit-tests PATCH 5/6] runtime: move getopts to run_tests.sh
Date: Tue, 26 Jan 2016 13:24:09 +0100
Message-ID: <20160126122409.GC10697@hawk.localdomain>
References: <1453474709-10679-1-git-send-email-drjones@redhat.com>
 <1453474709-10679-6-git-send-email-drjones@redhat.com>
 <56A75346.4000003@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org, rkrcmar@redhat.com
To: Paolo Bonzini <pbonzini@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:34119 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S933988AbcAZMYN (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 26 Jan 2016 07:24:13 -0500
Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27])
	by mx1.redhat.com (Postfix) with ESMTPS id 9D6503024BE
	for <kvm@vger.kernel.org>; Tue, 26 Jan 2016 12:24:13 +0000 (UTC)
Content-Disposition: inline
In-Reply-To: <56A75346.4000003@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Tue, Jan 26, 2016 at 12:06:46PM +0100, Paolo Bonzini wrote:
> 
> 
> On 22/01/2016 15:58, Andrew Jones wrote:
> > We don't need to check options in standalone tests, kick the
> > loop and usage() function out to run_tests.sh
> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  run_tests.sh         | 36 ++++++++++++++++++++++++++++++++++++
> >  scripts/runtime.bash | 37 +------------------------------------
> >  2 files changed, 37 insertions(+), 36 deletions(-)
> > 
> > diff --git a/run_tests.sh b/run_tests.sh
> > index 2312e031482a4..558b8e7431d8e 100755
> > --- a/run_tests.sh
> > +++ b/run_tests.sh
> > @@ -1,5 +1,7 @@
> >  #!/bin/bash
> >  
> > +verbose="no"
> > +
> >  if [ ! -f config.mak ]; then
> >      echo "run ./configure && make first. See ./configure -h"
> >      exit
> > @@ -7,9 +9,43 @@ fi
> >  source config.mak
> >  source scripts/functions.bash
> >  
> > +function usage()
> > +{
> > +cat <<EOF
> > +
> > +Usage: $0 [-g group] [-h] [-v]
> > +
> > +    -g: Only execute tests in the given group
> > +    -h: Output this help text
> > +    -v: Enables verbose mode
> > +
> > +Set the environment variable QEMU=/path/to/qemu-system-ARCH to
> > +specify the appropriate qemu binary for ARCH-run.
> > +
> > +EOF
> > +}
> > +
> >  RUNTIME_arch_run="./$TEST_DIR/run"
> >  source scripts/runtime.bash
> >  
> > +while getopts "g:hv" opt; do
> > +    case $opt in
> > +        g)
> > +            only_group=$OPTARG
> > +            ;;
> > +        h)
> > +            usage
> > +            exit
> > +            ;;
> > +        v)
> > +            verbose="yes"
> > +            ;;
> > +        *)
> > +            exit
> > +            ;;
> > +    esac
> > +done
> > +
> >  RUNTIME_arch_run="./$TEST_DIR/run >> test.log"
> >  config=$TEST_DIR/unittests.cfg
> >  echo > test.log
> > diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> > index fc878f99c977f..63d1b9653007b 100644
> > --- a/scripts/runtime.bash
> > +++ b/scripts/runtime.bash
> > @@ -1,7 +1,6 @@
> >  : "${RUNTIME_arch_run?}"
> >  
> >  qemu=${QEMU:-qemu-system-$ARCH}
> > -verbose=0
> >  
> >  function run()
> >  {
> > @@ -40,7 +39,7 @@ function run()
> >      done
> >  
> >      cmdline="TESTNAME=$testname ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
> > -    if [ $verbose != 0 ]; then
> > +    if [ "$verbose" = "yes" ]; then
> >          echo $cmdline
> >      fi
> >  
> > @@ -58,40 +57,6 @@ function run()
> >      return $ret
> >  }
> >  
> > -function usage()
> > -{
> > -cat <<EOF
> > -
> > -Usage: $0 [-g group] [-h] [-v]
> > -
> > -    -g: Only execute tests in the given group
> > -    -h: Output this help text
> > -    -v: Enables verbose mode
> > -
> > -Set the environment variable QEMU=/path/to/qemu-system-ARCH to
> > -specify the appropriate qemu binary for ARCH-run.
> > -
> > -EOF
> > -}
> > -
> > -while getopts "g:hv" opt; do
> > -    case $opt in
> > -        g)
> > -            only_group=$OPTARG
> > -            ;;
> > -        h)
> > -            usage
> > -            exit
> > -            ;;
> > -        v)
> > -            verbose=1
> > -            ;;
> > -        *)
> > -            exit
> > -            ;;
> > -    esac
> > -done
> > -
> >  MAX_SMP=$(getconf _NPROCESSORS_CONF)
> >  #
> >  # Probe for MAX_SMP, in case it's less than the number of host cpus.
> > 
> 
> Should standalone tests always set verbose=1?

atm, no. standalone tests already always echo the command line, which
is currently the only thing verbose does. Actually, turning it on would
result in a redundant echo. So, if we ever expand the scope of verbose,
then we may need to revisit this some day, but I don't see much need now.

Thanks,
drew