|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/3] raisin: introduce tests
On Thu, 30 Apr 2015, Anthony PERARD wrote:
> On Wed, Apr 29, 2015 at 06:00:54PM +0100, Stefano Stabellini wrote:
> > Introduce a new command to run functional tests and unit tests.
> > Introduce a generic infrastrucutre to run tests on the local machine.
> > Add a library of common functions that can be used by the test scripts
> > to setup guest VMs.
> >
> > Add a simple test script that boots a single busybox based PV guest.
> >
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > ---
> > README | 14 ++++++
> > defconfig | 5 +++
> > lib/commands.sh | 4 ++
> > lib/common-functions.sh | 74 +++++++++++++++++++++++++++++++
> > lib/common-tests.sh | 112
> > +++++++++++++++++++++++++++++++++++++++++++++++
> > raise | 8 ++--
> > tests/busybox-pv | 38 ++++++++++++++++
> > tests/series | 1 +
> > 8 files changed, 253 insertions(+), 3 deletions(-)
> > create mode 100644 lib/common-tests.sh
> > create mode 100755 tests/busybox-pv
> > create mode 100644 tests/series
> >
> > diff --git a/README b/README
> > index b7832da..42c0f4d 100644
> > --- a/README
> > +++ b/README
> > @@ -102,3 +102,17 @@ check-package
> >
> > If your component comes with additional data, maybe a config script or
> > anything else, place it under "data".
> > +
> > +
> > += Testing =
> > +
> > +Raisin can also be used for testing. Make sure to have Xen already up
> > +and running (raise build, raise install and host reboot).
> > +Ask Raisin to run tests like this:
> > +
> > +./raise test
> > +
> > +You can specify a subset of tests to run with ENABLED_TESTS in the
> > +config file, or the TESTS environmental variable:
> > +
> > +TESTS="busybox-pv" ./raise test
> > diff --git a/defconfig b/defconfig
> > index b4ed94d..e88f3d3 100644
> > --- a/defconfig
> > +++ b/defconfig
> > @@ -39,3 +39,8 @@ GRUB_REVISION="master"
> > LIBVIRT_REVISION="master"
> > OVMF_REVISION="master"
> > LINUX_REVISION="master"
> > +
> > +# Tests
> > +## All tests: busybox-pv
> > +## ENABLED_TESTS is the list of test run by raise test
> > +ENABLED_TESTS="busybox-pv"
> > diff --git a/lib/commands.sh b/lib/commands.sh
> > index 801341b..ffbadb4 100755
> > --- a/lib/commands.sh
> > +++ b/lib/commands.sh
> > @@ -103,3 +103,7 @@ function configure() {
> > for_each_component configure
> > }
> >
> > +function test() {
> > + init_tests
> > + run_tests
> > +}
> > diff --git a/lib/common-functions.sh b/lib/common-functions.sh
> > index d38788b..2edb168 100644
> > --- a/lib/common-functions.sh
> > +++ b/lib/common-functions.sh
> > @@ -39,6 +39,7 @@ function common_init() {
> > get_distro
> > get_arch
> > get_components
> > + get_tests
> >
> > verbose_echo "Distro: $DISTRO"
> > verbose_echo "Arch: $RAISIN_ARCH"
> > @@ -73,6 +74,24 @@ function get_components() {
> > export COMPONENTS
> > }
> >
> > +function get_tests() {
> > + if [[ -z "$TESTS" ]]
> > + then
> > + TESTS="$ENABLED_TESTS"
> > + fi
> > +
> > + if [[ -z "$TESTS" ]]
> > + then
> > + local t
> > + for t in `cat "$BASEDIR"/tests/series`
> > + do
> > + TESTS="$TESTS $t"
> > + verbose_echo "Found test $t"
> > + done
> > + fi
> > + export TESTS
> > +}
> > +
> > function get_distro() {
> > if [[ -x "`which lsb_release 2>/dev/null`" ]]
> > then
> > @@ -278,6 +297,61 @@ function for_each_component () {
> > done
> > }
> >
> > +function run_tests() {
> > + local t
> > + local enabled
> > + local found
> > +
> > + for t in `cat "$BASEDIR"/tests/series`
> > + do
> > + found=0
>
> found=false
>
> > + for enabled in $TESTS
> > + do
> > + if [[ $enabled = $t ]]
> > + then
> > + found=1
>
> found=true
>
> > + break
> > + fi
> > + done
> > + if [[ $found -eq 0 ]]
>
> if $found
> ;)
good suggestion, I'll make the change
> > + then
> > + verbose_echo "$t" is disabled
> > + continue
> > + fi
> > +
> > + verbose_echo running test "$t"
> > + "$BASEDIR"/tests/$t
> > + verbose_echo "test "$t" done"
> > + done
> > +}
> > +
> > +function init_tests() {
> > + local -a missing
> > +
> > + check-package bridge-utils
> > + if [[ $DISTRO = "Debian" ]]
> > + then
> > + check-package busybox-static
> > + elif [[ $DISTRO = "Fedora" ]]
> > + then
> > + check-package busybox grub2 which
> > + else
>
> Wouldn't it be easier to read with "case" instead of "if" ?
Maybe, I don't really have a preference, but we have the same pair of
ifs in many other places.
> > + echo "I don't know distro $DISTRO. It might be missing packages."
> > + fi
> > +
> > + if [[ -n "${missing[@]}" ]]
>
> missing looks empty to me.
It is filled by the check-package calls above
> > + then
> > + verbose_echo "Installing ${missing[@]}"
> > + install-package "${missing[@]}"
> > + fi
> > +
> > + if ! ifconfig xenbr1 &>/dev/null
> > + then
> > + $SUDO brctl addbr xenbr1
> > + $SUDO ifconfig xenbr1 169.254.0.1 up
> > + fi
> > +}
> > +
> > function _build_package_deb() {
> > fakeroot bash ./scripts/mkdeb "$1"
> > }
> > diff --git a/lib/common-tests.sh b/lib/common-tests.sh
> > new file mode 100644
> > index 0000000..be1c720
> > --- /dev/null
> > +++ b/lib/common-tests.sh
> > @@ -0,0 +1,112 @@
> > +#!/usr/bin/env bash
> > +
> > +source ${RAISIN_PATH}/common-functions.sh
> > +
> > +# $1 disk name
> > +# $2 disk size
> > +function allocate_disk() {
> > + local disk
> > + local size
> > +
> > + disk=$1
> > + size=$2
> > +
> > + size=$((size+511))
> > + size=$((size/512))
> > +
> > + dd if=/dev/zero of=$disk bs=512 count=$size
> > + sync
> > +}
> > +
> > +# $1 disk name
> > +# print loop device name
> > +function create_loop() {
> > + local disk
> > + local loop
> > +
> > + disk=`readlink -f $1`
> > +
> > + $SUDO losetup -f $disk
> > + loop=`$SUDO losetup -a | grep $disk | cut -d : -f 1`
> > + echo $loop
> > +}
> > +
> > +# $1 dev name
> > +function busybox_rootfs() {
> > + local dev
> > + local tmpdir
> > +
> > + dev=$1
> > +
> > + $SUDO mkfs.ext3 $dev
> > +
> > + tmpdir=`mktemp -d`
> > + $SUDO mount $dev $tmpdir
> > + mkdir -p $tmpdir/bin
> > + mkdir -p $tmpdir/sbin
> > + mkdir -p $tmpdir/dev
> > + mkdir -p $tmpdir/proc
> > + mkdir -p $tmpdir/sys
> > + mkdir -p $tmpdir/lib
> > + mkdir -p $tmpdir/var
> > + cp `which busybox` $tmpdir/bin
> > + $tmpdir/bin/busybox --install $tmpdir/bin
> > +
> > + $SUDO umount $tmpdir
> > + rmdir $tmpdir
> > +}
> > +
> > +function busybox_network_init() {
> > + local dev
> > + local tmpdir
> > +
> > + dev=$1
> > + tmpdir=`mktemp -d`
> > +
> > + $SUDO mount $dev $tmpdir
> > + rm -f $tmpdir/bin/init
> > + cat >$tmpdir/bin/init <<EOF
> > +#!/bin/sh
> > +mount -t proc proc /proc
> > +mount -t sysfs sysfs /sys
> > +ifconfig eth0 169.254.0.2 up
> > +/bin/sh
> > +EOF
> > + chmod +x $tmpdir/bin/init
> > +
> > + $SUDO umount $tmpdir
> > + rmdir $tmpdir
> > +}
> > +
> > +function check_guest_alive() {
> > + local i
> > + i=0
> > + while ! ping -c 1 169.254.0.2 &> /dev/null
> > + do
> > + sleep 1
> > + i=$((i+1))
> > + if [[ $i -gt 60 ]]
> > + then
> > + echo Timeout connecting to guest
> > + return 1
> > + fi
> > + done
> > + return 0
> > +}
> > +
> > +function get_host_kernel() {
> > + echo "/boot/vmlinuz-`uname -r`"
> > +}
> > +
> > +function get_host_initrd() {
> > + if [[ $DISTRO = "Debian" ]]
> > + then
> > + echo "/boot/initrd.img-`uname -r`"
> > + elif [[ $DISTRO = "Fedora" ]]
> > + then
> > + echo "/boot/initramfs-`uname -r`".img
> > + else
> > + echo "I don't know how to find the initrd"
>
> echo >&2 since it's an error message?
Good point
> > + exit 1
> > + fi
> > +}
>
> Also, in general, it might be better to use quotes around variables in case
> an argument of a function is missing, or a path contain a space.
Yes, you are right. I am trying to avoid quotes only on variables we
know must be populated, see README for a list. DISTRO is one of them.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |