Re: [PATCH kvm-unit-tests] mkstandalone: make parsing of unittests.cfg simpler and clearer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jan 12, 2017 at 10:23:43PM +0100, Paolo Bonzini wrote:
> 
> 
> On 12/01/2017 20:25, Andrew Jones wrote:
> > On Thu, Jan 12, 2017 at 06:50:15PM +0100, Paolo Bonzini wrote:
> >> The custom configuration created when mkstandalone has a parameter
> >> does not handle the case where the .flat file and the test have
> >> mismatching name.  Case in point, intel-iommu.flat vs intel_iommu.
> >> Instead, let for_each_unittest iterate on all tests, and filter
> >> the kernel name in mkstandalone().
> >>
> >> A kernel that does not have any test in unittests.cfg still behaves
> >> as before.
> >>
> >> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> >> ---
> >>  scripts/mkstandalone.sh | 50 ++++++++++++++++++++++++-------------------------
> >>  1 file changed, 25 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> >> index 3c1938e..20e56b8 100755
> >> --- a/scripts/mkstandalone.sh
> >> +++ b/scripts/mkstandalone.sh
> >> @@ -80,49 +80,49 @@ generate_test ()
> >>  function mkstandalone()
> >>  {
> >>  	local testname="$1"
> >> +	local kernel="$4"
> >>  
> >>  	if [ -z "$testname" ]; then
> >>  		return
> >>  	fi
> >>  
> >> -	if [ -n "$one_testname" ] && [ "$testname" != "$one_testname" ]; then
> >> +	if [ "$one_kernel" ] && [ "$kernel" != "$TEST_DIR/$one_kernel" ]; then
> >>  		return
> >>  	fi
> >>  
> >> +	mkdir -p tests
> >>  	standalone=tests/$testname
> >>  
> >>  	generate_test "$@" > $standalone
> >>  
> >>  	chmod +x $standalone
> >>  	echo Written $standalone.
> >> +	ok=true
> >>  }
> >>  
> >>  trap 'rm -f $cfg' EXIT
> > 
> > no need for this trap handler anymore
> > 
> >> -cfg=$(mktemp)
> >>  
> >>  unittests=$TEST_DIR/unittests.cfg
> >>  one_kernel="$1"
> >> -
> >> -if [ "$one_kernel" ]; then
> >> -	[ ! -f $one_kernel ] && {
> >> -		echo "$one_kernel doesn't exist"
> >> -		exit 1
> >> -	}
> >> -
> >> -	one_kernel_base=$(basename $one_kernel)
> >> -	one_testname="${2:-${one_kernel_base%.*}}"
> >> -
> >> -	if grep -q "\[$one_testname\]" $unittests; then
> >> -		sed -n "/\\[$one_testname\\]/,/^\\[/p" $unittests \
> >> -			| awk '!/^\[/ || NR == 1' > $cfg
> >> -	else
> >> -		echo "[$one_testname]" > $cfg
> >> -		echo "file = $one_kernel_base" >> $cfg
> >> -	fi
> >> -else
> >> -	cp -f $unittests $cfg
> >> +ok=false
> >> +
> >> +if [ -z "$1" ]; then
> >> +    echo Usage: "$0" KERNEL-NAME
> >> +    echo
> >> +    echo Extract from $unittests into standalone scripts all tests
> >> +    echo that run KERNEL-NAME.  KERNEL-NAME is relative to $TEST_DIR.
> >> +    echo
> >> +    echo If no tests exist in $unittests, create a standalone script
> >> +    echo that runs KERNEL-NAME with no special options.
> >> +    exit 1
> >>  fi
> >> -
> >> -mkdir -p tests
> >> -
> >> -for_each_unittest $cfg mkstandalone
> >> +if ! [ -f "$TEST_DIR/$one_kernel" ]; then
> >> +    echo "$0": "$TEST_DIR/$one_kernel" not found.
> >> +    exit 1
> >> +fi
> >> +for_each_unittest $unittests mkstandalone
> >> +$ok || {
> >> +    one_kernel_base=$(basename $one_kernel)
> >> +    one_testname="${2:-${one_kernel_base%.*}}"
> >> +    mkstandalone "$one_testname" "" "" "$TEST_DIR/$one_kernel"
> >> +}
> >> -- 
> >> 1.8.3.1
> >>
> > 
> > Nice cleanup! But doesn't this output usage and exit now if run without
> > any arguments? If so, then doesn't it break 'make standalone'? To fix,
> > we just need to move the if checks into the { }, right?
> 
> Hmm, yes it does.  It's not that simple, but not much more complex
> either.  But what about just removing 'make standalone'? :) Any reason
> to use it instead of the script if you're preparing a file for a tester?

make standalone is just a convenient way to make all of them at once,
and the dependency for make install, which installs all unit tests as
standalone tests. I'm not opposed to killing both make targets, but
some flag for mkstandalone, in order to build all tests as standalone
tests at once, would still be nice, as I do that occasionally for testing
purposes.

Thanks,
drew

> 
> Paolo
> --
> 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
--
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



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux