Re: [Xen-devel] [PATCH 1/3] raisin: introduce tests

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

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
;)

> +        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" ?

> +        echo "I don't know distro $DISTRO. It might be missing packages."
> +    fi
> +    
> +    if [[ -n "${missing[@]}" ]]

missing looks empty to me.

> +    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?

> +        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.

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel