Runs our BVT suite in a VM

Runs our BVT suite in a VM

Change-Id: I758c93596d5cbdd6b52b9acc82f4d6e19a326c9f

BUG=5518
TEST=Tested using all the options. All tests listed in the file pass.

[anush, 2010-08-25 23:57:34]

Can we name it cros_run_bvt.sh ? and drop it into the crosutils.git:bin/
dir. This way it will be available from any where in the chroot and not just
src/scripts/

Thanks

On Wed, Aug 25, 2010 at 6:53 PM, <rtc@chromium.org> wrote:

> Reviewers: petkov, kmixter1,
>
> Description:
> Runs our BVT suite in a VM
>
> Change-Id: I758c93596d5cbdd6b52b9acc82f4d6e19a326c9f
>
> BUG=5518
> TEST=Tested using all the options. All tests listed in the file pass.
>
> Please review this at http://codereview.chromium.org/3107039/show
>
> SVN Base: ssh://git@gitrw.chromium.org:9222/crosutils.git
>
> Affected files:
>  A run_bvt.sh
>
>
> Index: run_bvt.sh
> diff --git a/run_bvt.sh b/run_bvt.sh
> new file mode 100755
> index
>
0000000000000000000000000000000000000000..2b791961be91d0b5687b41e3f48cd33e46d88007
> --- /dev/null
> +++ b/run_bvt.sh
> @@ -0,0 +1,90 @@
> +#!/bin/bash
> +
> +# Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
> +# Use of this source code is governed by a BSD-style license that can be
> +# found in the LICENSE file.
> +
> +# Runs most of the test cases from our BVT suite on qemu-kvm.
> +#
> +# Known Issues:
> +#
> +#  Server side tests don't seem to work properly. This is why we're not
> running
> +#  the BVT suite directly and using run_remote_tests.
> +#
> +#  Using run_remote_tests does redundant work to copy autotest to the
> +#  system under test. The entire BVT suite runs in about 10 minutes, but
> +#  this will become more of an issue as people add test cases.
> +
> +
> +. "$(dirname $0)/common.sh"
> +
> +DEFINE_string image_path "" "Full path of the VM image"
> +DEFINE_integer ssh_port 9222 "Port to tunnel ssh traffic over"
> +DEFINE_boolean no_graphics ${FLAGS_FALSE} "Runs the KVM instance silently"
> +
> +# Parse command line
> +FLAGS "$@" || exit 1
> +eval set -- "${FLAGS_ARGV}"
> +
> +set -e
> +
> +KVM_PID_FILE=/tmp/kvm.$$.pid
> +TEST_CMD="./run_remote_tests.sh --ssh_port=${FLAGS_ssh_port}
> --remote=${HOSTNAME}"
> +TEST_CASES='
> +  desktopui_ChromeFirstRender
> +  build_RootFilesystemSize
> +  desktopui_FlashSanityCheck
> +  desktopui_KillRestart
> +  login_Backdoor
> +  login_BadAuthentication
> +  login_CryptohomeIncognitoMounted
> +  login_CryptohomeMounted
> +  login_LoginSuccess
> +  login_RemoteLogin
> +  platform_FilePerms
> +  platform_OSLimits'
> +
> +# TODO(rtc): These flags assume that we'll be using KVM on Lucid and won't
> work
> +# on Hardy.
> +function start_kvm {
> +  echo "Starting the KVM instance"
> +  local nographics=""
> +  if [ ${FLAGS_no_graphics} -eq ${FLAGS_TRUE} ]; then
> +    nographics="-nographic"
> +  fi
> +  sudo kvm -m 1024 \
> +    -vga std \
> +    -pidfile ${KVM_PID_FILE} \
> +    -daemonize \
> +    -net nic \
> +    ${nographics} \
> +    -net user,hostfwd=tcp::${FLAGS_ssh_port}-:22 \
> +    -hda ${FLAGS_image_path}
> +}
> +
> +function stop_kvm {
> +  echo "Stopping the KVM instance"
> +  local pid=`sudo cat ${KVM_PID_FILE}`
> +  echo "Killing ${pid}"
> +  sudo kill ${pid}
> +  sudo rm ${KVM_PID_FILE}
> +}
> +
> +function run_tests {
> +  for i in $TEST_CASES; do
> +    echo "Running ${test_cmd} ${i}"
> +    $TEST_CMD ${i}
> +  done
> +}
> +
> +echo ${FLAGS_image_path}
> +if [ -z ${FLAGS_image_path} ]; then
> +  echo "You must specify a path to an image"
> +  exit 1
> +fi
> +
> +trap stop_kvm EXIT
> +start_kvm
> +run_tests
> +
> +echo "BVT has completed successfully"
>
>
>

[petkov, 2010-08-26 00:26:00]

http://codereview.chromium.org/3107039/diff/4001/5001
File run_bvt.sh (right):

http://codereview.chromium.org/3107039/diff/4001/5001#newcode21
run_bvt.sh:21: DEFINE_string image_path "" "Full path of the VM image"
Sort the flags alphabetically.

http://codereview.chromium.org/3107039/diff/4001/5001#newcode25
run_bvt.sh:25: # Parse command line
Move this code to after the function declarations.

http://codereview.chromium.org/3107039/diff/4001/5001#newcode32
run_bvt.sh:32: TEST_CMD="./run_remote_tests.sh --ssh_port=${FLAGS_ssh_port}
--remote=${HOSTNAME}"
any way you can limit this to 80 chars?

http://codereview.chromium.org/3107039/diff/4001/5001#newcode34
run_bvt.sh:34: desktopui_ChromeFirstRender
should these be sorted?

http://codereview.chromium.org/3107039/diff/4001/5001#newcode52
run_bvt.sh:52: if [ ${FLAGS_no_graphics} -eq ${FLAGS_TRUE} ]; then
This is OK, although I tend to prefer the

[ ... ] && nographics=...

syntax if it fits on one line.

http://codereview.chromium.org/3107039/diff/4001/5001#newcode67
run_bvt.sh:67: local pid=`sudo cat ${KVM_PID_FILE}`
Use $() rather than `` (per style)

http://codereview.chromium.org/3107039/diff/4001/5001#newcode67
run_bvt.sh:67: local pid=`sudo cat ${KVM_PID_FILE}`
Does this work OK if KVM_PID_FILE wasn't created at all for some reason?

http://codereview.chromium.org/3107039/diff/4001/5001#newcode74
run_bvt.sh:74: for i in $TEST_CASES; do
Can you issue a single run_remote_tests command instead listing all the tests?
I.e., something like:

$TEST_CMD $TEST_CASES

http://codereview.chromium.org/3107039/diff/4001/5001#newcode75
run_bvt.sh:75: echo "Running ${test_cmd} ${i}"
s/test_cmd/TEST_CMD/ ?

http://codereview.chromium.org/3107039/diff/4001/5001#newcode80
run_bvt.sh:80: echo ${FLAGS_image_path}
Either prefix with "Image path: " or something like that, or remove.

http://codereview.chromium.org/3107039/diff/4001/5001#newcode82
run_bvt.sh:82: echo "You must specify a path to an image"
die "You must specify..."

also, I tend to like:

[ -n "${FLAGS_image_path" ] || die "You must specify..."

[kmixter1, 2010-08-26 01:44:50]

http://codereview.chromium.org/3107039/diff/4001/5001
File run_bvt.sh (right):

http://codereview.chromium.org/3107039/diff/4001/5001#newcode11
run_bvt.sh:11: #  Server side tests don't seem to work properly. This is why
we're not running
What's the failure?  Last week we introduced a second place to list new BVTs,
and this would introduce yet a third.

http://codereview.chromium.org/3107039/diff/4001/5001#newcode14
run_bvt.sh:14: #  Using run_remote_tests does redundant work to copy autotest to
the
Why is it redundant?  The autotest directory isn't part of the VM image is it?

[rtc, 2010-08-26 17:29:48]

On Wed, Aug 25, 2010 at 6:44 PM, <kmixter@chromium.org> wrote:

>
> http://codereview.chromium.org/3107039/diff/4001/5001
> File run_bvt.sh (right):
>
> http://codereview.chromium.org/3107039/diff/4001/5001#newcode11
> run_bvt.sh:11: #  Server side tests don't seem to work properly. This is
>
> why we're not running
> What's the failure?  Last week we introduced a second place to list new
> BVTs, and this would introduce yet a third.
>

So, this will either resolve one of two ways. This script becomes the
canonical listing of tests we run for BVT (because it will be the only BVT
that we do). Or, we'll get server side tests to work. The issue with server
side tests is that Autotest is failing on getaddrinfo most likely because
the system under test uses the hosts network connection. There are other
issues like the system under test can't accept icmp traffic or be addressed
by IP.



>
> http://codereview.chromium.org/3107039/diff/4001/5001#newcode14
> run_bvt.sh:14: #  Using run_remote_tests does redundant work to copy
> autotest to the
> Why is it redundant?  The autotest directory isn't part of the VM image
> is it?


It's not redundant the first time, but each call to run_remote_tests causes
the autotest directory to be copied over again.


>
>
> http://codereview.chromium.org/3107039/show
>

[petkov, 2010-08-26 17:36:04]

On 2010/08/26 17:29:48, rtc wrote:
> On Wed, Aug 25, 2010 at 6:44 PM, <mailto:kmixter@chromium.org> wrote:
> 
> >
> > http://codereview.chromium.org/3107039/diff/4001/5001
> > File run_bvt.sh (right):
> >
> > http://codereview.chromium.org/3107039/diff/4001/5001#newcode11
> > run_bvt.sh:11: #  Server side tests don't seem to work properly. This is
> >
> > why we're not running
> > What's the failure?  Last week we introduced a second place to list new
> > BVTs, and this would introduce yet a third.
> >
> 
> So, this will either resolve one of two ways. This script becomes the
> canonical listing of tests we run for BVT (because it will be the only BVT

Actually, it seems that some of these issues can be resolved by creating a new
BVT client-side control file that lists these tests. This will resolve both the
multiple locations issue (assuming this really becomes _the_ BVT suite), as well
as the redundant copying of autotest (because you'd run run_remote_tests.sh on
only one control file).

> that we do). Or, we'll get server side tests to work. The issue with server
> side tests is that Autotest is failing on getaddrinfo most likely because
> the system under test uses the hosts network connection. There are other
> issues like the system under test can't accept icmp traffic or be addressed
> by IP.
> 
> 
> 
> >
> > http://codereview.chromium.org/3107039/diff/4001/5001#newcode14
> > run_bvt.sh:14: #  Using run_remote_tests does redundant work to copy
> > autotest to the
> > Why is it redundant?  The autotest directory isn't part of the VM image
> > is it?
> 
> 
> It's not redundant the first time, but each call to run_remote_tests causes
> the autotest directory to be copied over again.
> 
> 
> >
> >
> > http://codereview.chromium.org/3107039/show
> >
>

[rtc, 2010-08-26 17:37:40]

On Thu, Aug 26, 2010 at 10:36 AM, <petkov@chromium.org> wrote:

> On 2010/08/26 17:29:48, rtc wrote:
>
>  On Wed, Aug 25, 2010 at 6:44 PM, <mailto:kmixter@chromium.org> wrote:
>>
>
>  >
>> > http://codereview.chromium.org/3107039/diff/4001/5001
>> > File run_bvt.sh (right):
>> >
>> > http://codereview.chromium.org/3107039/diff/4001/5001#newcode11
>> > run_bvt.sh:11: #  Server side tests don't seem to work properly. This is
>> >
>> > why we're not running
>> > What's the failure?  Last week we introduced a second place to list new
>> > BVTs, and this would introduce yet a third.
>> >
>>
>
>  So, this will either resolve one of two ways. This script becomes the
>> canonical listing of tests we run for BVT (because it will be the only BVT
>>
>
> Actually, it seems that some of these issues can be resolved by creating a
> new
> BVT client-side control file that lists these tests. This will resolve both
> the
> multiple locations issue (assuming this really becomes _the_ BVT suite), as
> well
> as the redundant copying of autotest (because you'd run run_remote_tests.sh
> on
> only one control file).


That's a good point, let me try that and see if it makes a dent on the
speed.

>
>
>  that we do). Or, we'll get server side tests to work. The issue with
>> server
>> side tests is that Autotest is failing on getaddrinfo most likely because
>> the system under test uses the hosts network connection. There are other
>> issues like the system under test can't accept icmp traffic or be
>> addressed
>> by IP.
>>
>
>
>
>  >
>> > http://codereview.chromium.org/3107039/diff/4001/5001#newcode14
>> > run_bvt.sh:14: #  Using run_remote_tests does redundant work to copy
>> > autotest to the
>> > Why is it redundant?  The autotest directory isn't part of the VM image
>> > is it?
>>
>
>
>  It's not redundant the first time, but each call to run_remote_tests
>> causes
>> the autotest directory to be copied over again.
>>
>
>
>  >
>> >
>> > http://codereview.chromium.org/3107039/show
>> >
>>
>
>
>
>
> http://codereview.chromium.org/3107039/show
>

[petkov, 2010-08-26 17:40:25]

http://codereview.chromium.org/3107039/diff/4001/5001
File run_bvt.sh (right):

http://codereview.chromium.org/3107039/diff/4001/5001#newcode47
run_bvt.sh:47: # TODO(rtc): These flags assume that we'll be using KVM on Lucid
and won't work
sosa made a good point: most devs are running Hardy and we should make sure devs
can run BVTs; how much effort is it to make this work on Hardy?

[sosa, 2010-08-26 17:42:44]

Mostly nits.  You should make sure to put ""'s around user defined variables
because of possible spaces in names.

http://codereview.chromium.org/3107039/diff/4001/5001
File run_bvt.sh (right):

http://codereview.chromium.org/3107039/diff/4001/5001#newcode48
run_bvt.sh:48: # on Hardy.
Can you add a check to /etc/lsb-release -- DISTRIB_CODENAME and verify that it's
lucid and throw an error otherwise.

http://codereview.chromium.org/3107039/diff/4001/5001#newcode55
run_bvt.sh:55: sudo kvm -m 1024 \
What's this constant?  Should this be a variable?

http://codereview.chromium.org/3107039/diff/4001/5001#newcode62
run_bvt.sh:62: -hda ${FLAGS_image_path}
Quotes around image_path

http://codereview.chromium.org/3107039/diff/4001/5001#newcode69
run_bvt.sh:69: sudo kill ${pid}
Maybe use kill -9 to be sure it stopped.

http://codereview.chromium.org/3107039/diff/4001/5001#newcode74
run_bvt.sh:74: for i in $TEST_CASES; do
Use ${VAR} since you did so for other vars.

http://codereview.chromium.org/3107039/diff/4001/5001#newcode76
run_bvt.sh:76: $TEST_CMD ${i}
Use ${VAR} since you did so for other vars.

http://codereview.chromium.org/3107039/diff/4001/5001#newcode81
run_bvt.sh:81: if [ -z ${FLAGS_image_path} ]; then
Put quotes around image path in case it's ws delimited

[ericli, 2010-08-26 17:50:03]

http://codereview.chromium.org/3107039/diff/4001/5001
File run_bvt.sh (right):

http://codereview.chromium.org/3107039/diff/4001/5001#newcode82
run_bvt.sh:82: echo "You must specify a path to an image"
On 2010/08/26 00:26:00, petkov wrote:
> die "You must specify..."
> 
> also, I tend to like:
> 
> [ -n "${FLAGS_image_path" ] || die "You must specify..."
> 

Done.

[petkov, 2010-08-26 17:50:03]

On 2010/08/26 17:40:25, petkov wrote:
> http://codereview.chromium.org/3107039/diff/4001/5001
> File run_bvt.sh (right):
> 
> http://codereview.chromium.org/3107039/diff/4001/5001#newcode47
> run_bvt.sh:47: # TODO(rtc): These flags assume that we'll be using KVM on
Lucid
> and won't work
> sosa made a good point: most devs are running Hardy and we should make sure
devs
> can run BVTs; how much effort is it to make this work on Hardy?

Nevermind :-)

[ericli, 2010-08-26 17:53:37]

http://codereview.chromium.org/3107039/diff/4001/5001
File run_bvt.sh (right):

http://codereview.chromium.org/3107039/diff/4001/5001#newcode82
run_bvt.sh:82: echo "You must specify a path to an image"
Sorry, mistakenly hit "Done".

How this script will notify a BVT test failure? I didn't see any checks... Only
a success message at the end.


On 2010/08/26 17:50:03, ericli wrote:
> On 2010/08/26 00:26:00, petkov wrote:
> > die "You must specify..."
> > 
> > also, I tend to like:
> > 
> > [ -n "${FLAGS_image_path" ] || die "You must specify..."
> > 
> 
> Done.

[rtc, 2010-08-26 18:17:18]

It fails when run_remote_tests fails.

On Thu, Aug 26, 2010 at 10:53 AM, <ericli@chromium.org> wrote:

>
> http://codereview.chromium.org/3107039/diff/4001/5001
> File run_bvt.sh (right):
>
> http://codereview.chromium.org/3107039/diff/4001/5001#newcode82
> run_bvt.sh:82: echo "You must specify a path to an image"
> Sorry, mistakenly hit "Done".
>
> How this script will notify a BVT test failure? I didn't see any
> checks... Only a success message at the end.
>
>
>
> On 2010/08/26 17:50:03, ericli wrote:
>
>> On 2010/08/26 00:26:00, petkov wrote:
>> > die "You must specify..."
>> >
>> > also, I tend to like:
>> >
>> > [ -n "${FLAGS_image_path" ] || die "You must specify..."
>> >
>>
>
>  Done.
>>
>
> http://codereview.chromium.org/3107039/show
>

[rtc, 2010-08-30 23:07:52]

Ok, I think I've addressed everyone's comments.

PTAL

On Thu, Aug 26, 2010 at 11:17 AM, Ryan Cairns <rtc@chromium.org> wrote:

> It fails when run_remote_tests fails.
>
>
> On Thu, Aug 26, 2010 at 10:53 AM, <ericli@chromium.org> wrote:
>
>>
>> http://codereview.chromium.org/3107039/diff/4001/5001
>> File run_bvt.sh (right):
>>
>> http://codereview.chromium.org/3107039/diff/4001/5001#newcode82
>> run_bvt.sh:82: echo "You must specify a path to an image"
>> Sorry, mistakenly hit "Done".
>>
>> How this script will notify a BVT test failure? I didn't see any
>> checks... Only a success message at the end.
>>
>>
>>
>> On 2010/08/26 17:50:03, ericli wrote:
>>
>>> On 2010/08/26 00:26:00, petkov wrote:
>>> > die "You must specify..."
>>> >
>>> > also, I tend to like:
>>> >
>>> > [ -n "${FLAGS_image_path" ] || die "You must specify..."
>>> >
>>>
>>
>>  Done.
>>>
>>
>> http://codereview.chromium.org/3107039/show
>>
>
>

[petkov, 2010-08-30 23:30:51]

LGTM