Simple way to run client/server autotest(s) from server

Simple way to run client/server autotest(s) from server.

This is a script to run client or server autotests on a live Chromium OS instance, collect results, and optionally upload the to an autotest database.  This includes functional and performance tests.  We assume the remote instance is running an appropriate image installed (one created using mod_image_for_test.sh and possibly installed using image_to_live.sh).

An example run might be

run_remote_tests.sh --remote=192.168.1.5 BootPerfServer -o results.txt

This example will run src/platform/testing/server_tests/system_BootPerfServer
5 times on instance at 192.168.1.5 and collect results in result.txt.

Also refactors and improves readability in image_to_live.sh.

[sosa, 2010-01-05 22:31:28]

Sorry about missing this.  Overall looks good, I have a few questions inlined.

http://codereview.chromium.org/519041/diff/4001/5003
File src/scripts/run_remote_tests.sh (right):

http://codereview.chromium.org/519041/diff/4001/5003#newcode29
src/scripts/run_remote_tests.sh:29: # Return if bad function was found.
Add usage doc since it expects an arg

http://codereview.chromium.org/519041/diff/4001/5003#newcode43
src/scripts/run_remote_tests.sh:43: function remove_quotes() {
Ditto

http://codereview.chromium.org/519041/diff/4001/5003#newcode117
src/scripts/run_remote_tests.sh:117: type=$(python -c "${type}; print
TEST_TYPE")
Why not just do a split here?  Running python seems like overkill

http://codereview.chromium.org/519041/diff/4001/5003#newcode119
src/scripts/run_remote_tests.sh:119: if [ "${type}" == "client" ]; then
Is this case sensitive?

http://codereview.chromium.org/519041/diff/4001/5003#newcode143
src/scripts/run_remote_tests.sh:143: echo "Keyval was:" | tee -a
"${FLAGS_output_file}"
Is it cleaner to just use >> versus tee?

[petkov, 2010-01-05 22:32:38]

See comments below. Also, image_to_live could be cleaned up further by using the
force update option for the forced update and boot_id for reboot.
Again, nothing critical.

http://codereview.chromium.org/519041/diff/4001/5002
File src/scripts/remote_access.sh (right):

http://codereview.chromium.org/519041/diff/4001/5002#newcode7
src/scripts/remote_access.sh:7: . "$(dirname "$0")/common.sh"
Is there an issue with executing common.sh from scripts that include this
library?

http://codereview.chromium.org/519041/diff/4001/5003
File src/scripts/run_remote_tests.sh (right):

http://codereview.chromium.org/519041/diff/4001/5003#newcode44
src/scripts/run_remote_tests.sh:44: echo "$1" | sed -e "s/^'//" | sed -e
"s/'$//"
echo "$1" | sed -e "s/^'//; s/'$//"

http://codereview.chromium.org/519041/diff/4001/5003#newcode68
src/scripts/run_remote_tests.sh:68: # TODO(kmixter@chromium.org): This is only
necessary because we have
The tests should be reorganized and moved to the autotest repo real soon.

http://codereview.chromium.org/519041/diff/4001/5003#newcode87
src/scripts/run_remote_tests.sh:87: # Now search for tests which unambiguously
include the given identifier
There're only 4 locations that autotest can run tests from. site_tests have
priority over tests (for tests with the same name). And it's a single level
search. It may be better to just check these locations because control files
also end up in other places (e.g., the results folders).

autotest/server/site_tests
autotest/server/tests
autotest/client/site_test
autotest/client/tests

http://codereview.chromium.org/519041/diff/4001/5003#newcode114
src/scripts/run_remote_tests.sh:114: # Assume a line starts with TEST_TYPE =
It may be better to just check if the test comes from server/[site_]tests or
client/[site_]tests.

[kmixter1, 2010-01-07 23:29:16]

Responded to all comments.

http://codereview.chromium.org/519041/diff/4001/5002
File src/scripts/remote_access.sh (right):

http://codereview.chromium.org/519041/diff/4001/5002#newcode7
src/scripts/remote_access.sh:7: . "$(dirname "$0")/common.sh"
Think it's better to reduce redundancy, but not strongly enough to push it
further.  Moving back.
On 2010/01/05 22:32:38, petkov wrote:
> Is there an issue with executing common.sh from scripts that include this
> library?
>

http://codereview.chromium.org/519041/diff/4001/5003
File src/scripts/run_remote_tests.sh (right):

http://codereview.chromium.org/519041/diff/4001/5003#newcode29
src/scripts/run_remote_tests.sh:29: # Return if bad function was found.
On 2010/01/05 22:31:28, sosa wrote:
> Add usage doc since it expects an arg

Done.

http://codereview.chromium.org/519041/diff/4001/5003#newcode43
src/scripts/run_remote_tests.sh:43: function remove_quotes() {
On 2010/01/05 22:31:28, sosa wrote:
> Ditto

Done.

http://codereview.chromium.org/519041/diff/4001/5003#newcode44
src/scripts/run_remote_tests.sh:44: echo "$1" | sed -e "s/^'//" | sed -e
"s/'$//"
Thanks, I thought there was a way to string these together.
On 2010/01/05 22:32:38, petkov wrote:
> echo "$1" | sed -e "s/^'//; s/'$//"
>

http://codereview.chromium.org/519041/diff/4001/5003#newcode68
src/scripts/run_remote_tests.sh:68: # TODO(kmixter@chromium.org): This is only
necessary because we have
I'll compensate when it's landed.
On 2010/01/05 22:32:38, petkov wrote:
> The tests should be reorganized and moved to the autotest repo real soon.
>

http://codereview.chromium.org/519041/diff/4001/5003#newcode87
src/scripts/run_remote_tests.sh:87: # Now search for tests which unambiguously
include the given identifier
On 2010/01/05 22:32:38, petkov wrote:
> There're only 4 locations that autotest can run tests from. site_tests have
> priority over tests (for tests with the same name). And it's a single level
> search. It may be better to just check these locations because control files
> also end up in other places (e.g., the results folders).
> 
> autotest/server/site_tests
> autotest/server/tests
> autotest/client/site_test
> autotest/client/tests
> 

Done.

http://codereview.chromium.org/519041/diff/4001/5003#newcode114
src/scripts/run_remote_tests.sh:114: # Assume a line starts with TEST_TYPE =
Yet the autoserv logic is probably respecting this over the directory it comes
from.  Rather keep this.
On 2010/01/05 22:32:38, petkov wrote:
> It may be better to just check if the test comes from server/[site_]tests or
> client/[site_]tests.
>

http://codereview.chromium.org/519041/diff/4001/5003#newcode117
src/scripts/run_remote_tests.sh:117: type=$(python -c "${type}; print
TEST_TYPE")
This runs in about 20ms and avoids having to approximate a python interpreter
with comment characters, whitespace, etc.
On 2010/01/05 22:31:28, sosa wrote:
> Why not just do a split here?  Running python seems like overkill

http://codereview.chromium.org/519041/diff/4001/5003#newcode119
src/scripts/run_remote_tests.sh:119: if [ "${type}" == "client" ]; then
No, but I made python return lower-case.
On 2010/01/05 22:31:28, sosa wrote:
> Is this case sensitive?

http://codereview.chromium.org/519041/diff/4001/5003#newcode143
src/scripts/run_remote_tests.sh:143: echo "Keyval was:" | tee -a
"${FLAGS_output_file}"
On 2010/01/05 22:31:28, sosa wrote:
> Is it cleaner to just use >> versus tee?  

I like to see the output inline with the autoserv output in addition to logging
to a summary results file.  So I think this is the most concise way to do that.

[sosa, 2010-01-08 00:31:10]

LGTM

On 2010/01/07 23:29:16, kmixter1 wrote:
> Responded to all comments.
> 
> http://codereview.chromium.org/519041/diff/4001/5002
> File src/scripts/remote_access.sh (right):
> 
> http://codereview.chromium.org/519041/diff/4001/5002#newcode7
> src/scripts/remote_access.sh:7: . "$(dirname "$0")/common.sh"
> Think it's better to reduce redundancy, but not strongly enough to push it
> further.  Moving back.
> On 2010/01/05 22:32:38, petkov wrote:
> > Is there an issue with executing common.sh from scripts that include this
> > library?
> >
> 
> http://codereview.chromium.org/519041/diff/4001/5003
> File src/scripts/run_remote_tests.sh (right):
> 
> http://codereview.chromium.org/519041/diff/4001/5003#newcode29
> src/scripts/run_remote_tests.sh:29: # Return if bad function was found.
> On 2010/01/05 22:31:28, sosa wrote:
> > Add usage doc since it expects an arg
> 
> Done.
> 
> http://codereview.chromium.org/519041/diff/4001/5003#newcode43
> src/scripts/run_remote_tests.sh:43: function remove_quotes() {
> On 2010/01/05 22:31:28, sosa wrote:
> > Ditto
> 
> Done.
> 
> http://codereview.chromium.org/519041/diff/4001/5003#newcode44
> src/scripts/run_remote_tests.sh:44: echo "$1" | sed -e "s/^'//" | sed -e
> "s/'$//"
> Thanks, I thought there was a way to string these together.
> On 2010/01/05 22:32:38, petkov wrote:
> > echo "$1" | sed -e "s/^'//; s/'$//"
> >
> 
> http://codereview.chromium.org/519041/diff/4001/5003#newcode68
> src/scripts/run_remote_tests.sh:68: # TODO(kmixter@chromium.org): This is only
> necessary because we have
> I'll compensate when it's landed.
> On 2010/01/05 22:32:38, petkov wrote:
> > The tests should be reorganized and moved to the autotest repo real soon.
> >
> 
> http://codereview.chromium.org/519041/diff/4001/5003#newcode87
> src/scripts/run_remote_tests.sh:87: # Now search for tests which unambiguously
> include the given identifier
> On 2010/01/05 22:32:38, petkov wrote:
> > There're only 4 locations that autotest can run tests from. site_tests have
> > priority over tests (for tests with the same name). And it's a single level
> > search. It may be better to just check these locations because control files
> > also end up in other places (e.g., the results folders).
> > 
> > autotest/server/site_tests
> > autotest/server/tests
> > autotest/client/site_test
> > autotest/client/tests
> > 
> 
> Done.
> 
> http://codereview.chromium.org/519041/diff/4001/5003#newcode114
> src/scripts/run_remote_tests.sh:114: # Assume a line starts with TEST_TYPE =
> Yet the autoserv logic is probably respecting this over the directory it comes
> from.  Rather keep this.
> On 2010/01/05 22:32:38, petkov wrote:
> > It may be better to just check if the test comes from server/[site_]tests or
> > client/[site_]tests.
> >
> 
> http://codereview.chromium.org/519041/diff/4001/5003#newcode117
> src/scripts/run_remote_tests.sh:117: type=$(python -c "${type}; print
> TEST_TYPE")
> This runs in about 20ms and avoids having to approximate a python interpreter
> with comment characters, whitespace, etc.
> On 2010/01/05 22:31:28, sosa wrote:
> > Why not just do a split here?  Running python seems like overkill
> 
> http://codereview.chromium.org/519041/diff/4001/5003#newcode119
> src/scripts/run_remote_tests.sh:119: if [ "${type}" == "client" ]; then
> No, but I made python return lower-case.
> On 2010/01/05 22:31:28, sosa wrote:
> > Is this case sensitive?
> 
> http://codereview.chromium.org/519041/diff/4001/5003#newcode143
> src/scripts/run_remote_tests.sh:143: echo "Keyval was:" | tee -a
> "${FLAGS_output_file}"
> On 2010/01/05 22:31:28, sosa wrote:
> > Is it cleaner to just use >> versus tee?  
> 
> I like to see the output inline with the autoserv output in addition to
logging
> to a summary results file.  So I think this is the most concise way to do
that.

[ericli, 2010-01-08 01:28:24]

Hi Ken,

Sorry to jump on this CL late.  I am interested in this CL and like to see how
to combine my work (test case pre-build) into your script.

Also I had some other comments into your code.

This is just a heads up, I might continue working on your script after you
checked in.

Thanks.




On 2010/01/08 00:31:10, sosa wrote:
> LGTM
> 
> On 2010/01/07 23:29:16, kmixter1 wrote:
> > Responded to all comments.
> > 
> > http://codereview.chromium.org/519041/diff/4001/5002
> > File src/scripts/remote_access.sh (right):
> > 
> > http://codereview.chromium.org/519041/diff/4001/5002#newcode7
> > src/scripts/remote_access.sh:7: . "$(dirname "$0")/common.sh"
> > Think it's better to reduce redundancy, but not strongly enough to push it
> > further.  Moving back.
> > On 2010/01/05 22:32:38, petkov wrote:
> > > Is there an issue with executing common.sh from scripts that include this
> > > library?
> > >
> > 
> > http://codereview.chromium.org/519041/diff/4001/5003
> > File src/scripts/run_remote_tests.sh (right):
> > 
> > http://codereview.chromium.org/519041/diff/4001/5003#newcode29
> > src/scripts/run_remote_tests.sh:29: # Return if bad function was found.
> > On 2010/01/05 22:31:28, sosa wrote:
> > > Add usage doc since it expects an arg
> > 
> > Done.
> > 
> > http://codereview.chromium.org/519041/diff/4001/5003#newcode43
> > src/scripts/run_remote_tests.sh:43: function remove_quotes() {
> > On 2010/01/05 22:31:28, sosa wrote:
> > > Ditto
> > 
> > Done.
> > 
> > http://codereview.chromium.org/519041/diff/4001/5003#newcode44
> > src/scripts/run_remote_tests.sh:44: echo "$1" | sed -e "s/^'//" | sed -e
> > "s/'$//"
> > Thanks, I thought there was a way to string these together.
> > On 2010/01/05 22:32:38, petkov wrote:
> > > echo "$1" | sed -e "s/^'//; s/'$//"
> > >
> > 
> > http://codereview.chromium.org/519041/diff/4001/5003#newcode68
> > src/scripts/run_remote_tests.sh:68: # TODO(kmixter@chromium.org): This is
only
> > necessary because we have
> > I'll compensate when it's landed.
> > On 2010/01/05 22:32:38, petkov wrote:
> > > The tests should be reorganized and moved to the autotest repo real soon.
> > >
> > 
> > http://codereview.chromium.org/519041/diff/4001/5003#newcode87
> > src/scripts/run_remote_tests.sh:87: # Now search for tests which
unambiguously
> > include the given identifier
> > On 2010/01/05 22:32:38, petkov wrote:
> > > There're only 4 locations that autotest can run tests from. site_tests
have
> > > priority over tests (for tests with the same name). And it's a single
level
> > > search. It may be better to just check these locations because control
files
> > > also end up in other places (e.g., the results folders).
> > > 
> > > autotest/server/site_tests
> > > autotest/server/tests
> > > autotest/client/site_test
> > > autotest/client/tests
> > > 
> > 
> > Done.
> > 
> > http://codereview.chromium.org/519041/diff/4001/5003#newcode114
> > src/scripts/run_remote_tests.sh:114: # Assume a line starts with TEST_TYPE =
> > Yet the autoserv logic is probably respecting this over the directory it
comes
> > from.  Rather keep this.
> > On 2010/01/05 22:32:38, petkov wrote:
> > > It may be better to just check if the test comes from server/[site_]tests
or
> > > client/[site_]tests.
> > >
> > 
> > http://codereview.chromium.org/519041/diff/4001/5003#newcode117
> > src/scripts/run_remote_tests.sh:117: type=$(python -c "${type}; print
> > TEST_TYPE")
> > This runs in about 20ms and avoids having to approximate a python
interpreter
> > with comment characters, whitespace, etc.
> > On 2010/01/05 22:31:28, sosa wrote:
> > > Why not just do a split here?  Running python seems like overkill
> > 
> > http://codereview.chromium.org/519041/diff/4001/5003#newcode119
> > src/scripts/run_remote_tests.sh:119: if [ "${type}" == "client" ]; then
> > No, but I made python return lower-case.
> > On 2010/01/05 22:31:28, sosa wrote:
> > > Is this case sensitive?
> > 
> > http://codereview.chromium.org/519041/diff/4001/5003#newcode143
> > src/scripts/run_remote_tests.sh:143: echo "Keyval was:" | tee -a
> > "${FLAGS_output_file}"
> > On 2010/01/05 22:31:28, sosa wrote:
> > > Is it cleaner to just use >> versus tee?  
> > 
> > I like to see the output inline with the autoserv output in addition to
> logging
> > to a summary results file.  So I think this is the most concise way to do
> that.

[ericli, 2010-01-08 01:28:57]

publishing my inline comments.

http://codereview.chromium.org/519041/diff/6002/7003
File src/scripts/run_remote_tests.sh (right):

http://codereview.chromium.org/519041/diff/6002/7003#newcode163
src/scripts/run_remote_tests.sh:163: ${autoserv} -m "${FLAGS_remote}"
"${option}" "${control_file}" \
the way to figure out options and results_dir is a duplication of autoserv
function itself.

http://codereview.chromium.org/519041/diff/6002/7003#newcode182
src/scripts/run_remote_tests.sh:182: if [[ ${FLAGS_update_db} -eq ${FLAGS_TRUE}
]]; then
This is also a dup.