Added HWQual utility script - call_script.py

Added HWQual utility script - call_script.py

Change-Id: I07722c5eac6836051dedf015593d4717bf852853

BUG=chromium-os:10197
TEST= Run HWQual scripts as - 

python call_autoserv.py --ip=<remote machine IP> --test=<server or client HWQual testname in suite_HWQual or site_tests>

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=571d1c9

[ruchic, 2011-01-12 23:14:18]

The script is executed as -
 python callscript.py ip=<IP address of machine> test=<test name>

Please review the CL for this board independent utility script that takes
minimal parameters for running any HWQual script and does following steps -

* Firstly adds testing-rsa
* Checks the ~/.ssh/known_hosts and removes the remote machine IP to avoid
identity clash.
* Sets up ssh connection to the remote IP.
* If ssh connection succeeds, starts the HWQual test. It looks for location of
test control file based on test name given as parameter. [If test name is not
found under suite_HWQual then it looks for it under site_tests directory]
* Stores the results in a unique 'test name+time stamped' directory.
* At the end of test, displays the PASS / FAIL results using the
generate_test_report script.
* It checks the status log in result directory and if there are failures, then
grabs the result logs, tars it up and displays the location.

Thanks,
Ruchi

[kmixter1, 2011-01-13 01:52:52]

http://codereview.chromium.org/6187006/diff/1/archive_hwqual
File archive_hwqual (right):

http://codereview.chromium.org/6187006/diff/1/archive_hwqual#newcode103
archive_hwqual:103: cp "${script_dir}/call_script.py
tarball/${FLAGS_output_tag}"
If you're adding this to the package, have you considered changing
suite_HWQual/README.txt to use this instead of instructing user to call
server/autoserv directly?  It does mean you'll need to support problems that may
occur with this script externally, but it would simplify how partners and we run
these tests.

http://codereview.chromium.org/6187006/diff/1/call_script.py
File call_script.py (right):

http://codereview.chromium.org/6187006/diff/1/call_script.py#newcode1
call_script.py:1: #!/usr/bin/python
High level comment is that, as we discussed, this file has very similar
functionality to src/scripts/run_remote_tests.  I think the delta between what
it does and what this does is:

1) this assumes you are in the root of an unzipped hwqual package whereas it
assumes you are in src/scripts and running on third_party/autotest/files.

2) This creates a results file with a different name format. 
results.<testname>.<timestamp> vs /tmp/run_remote_tests.<randomkey>

3) This tars up the results of failed tests.

4) run_remote_tests depends on other scripts (common.sh and remote_access.sh)

5) run_remote_tests runs inside the chroot.  It does this because it also has
the ability to build tests and requires a hermetic/chrooted environment to do
so.

6) run_remote_tests lets you run multiple tests, supports both server and client
tests (this assumes all tests are client tests), and lets you specify tests by a
unique substring of the path to the control file (which handles both the
control.%s pattern and the .../%s/control pattern)

7) different style parameters:
./run_remote_tests.sh --remote=$ip auto
./call_script.py --ip=$ip --test=auto

It would be nice to not have a separate script that's so similar in
functionality, but there are a lot of differences, and I can see this being very
handy for partners and for people running hwqual testing to have it in, so
that's why I'm not blocking this. But over time it would be nice to find a way
to merge the functionality so these features can be used in both places.

http://codereview.chromium.org/6187006/diff/1/call_script.py#newcode3
call_script.py:3: # Copyright (c) 2009 The Chromium OS Authors. All rights
reserved.
2011

http://codereview.chromium.org/6187006/diff/1/call_script.py#newcode7
call_script.py:7: """Script to run client or server tests on a live remote
image.
call_script.py is a little too generic.  How about call_autoserv.py at least?

http://codereview.chromium.org/6187006/diff/1/call_script.py#newcode17
call_script.py:17: import datetime
abc order

http://codereview.chromium.org/6187006/diff/1/call_script.py#newcode27
call_script.py:27: os.system('echo $USER > /tmp/user_name')
username = os.environ['USER']?

http://codereview.chromium.org/6187006/diff/1/call_script.py#newcode56
call_script.py:56: if ("FAIL" in line) or ("ERROR" in line):
Suggest running generate_test_report and using its return value, that is more
robust than this method (and we rely on that in other infrastructure including
run_remote_tests).

http://codereview.chromium.org/6187006/diff/1/call_script.py#newcode83
call_script.py:83: print "INFO : Starting the HWQual test : %s" % test_path
May want to consider import logging for these.

http://codereview.chromium.org/6187006/diff/1/call_script.py#newcode84
call_script.py:84: os.system('./autotest/server/autoserv -r ./autotest/%s -m %s
-c %s' % \
-c assumes the test is a client test.  Probably worth documenting that --test
argument must be a client test somewhere.  I believe all hwqual tests are
currently.

http://codereview.chromium.org/6187006/diff/1/call_script.py#newcode99
call_script.py:99: if i in ["-h", "--help", "-?"]:
I would suggest using import optparse for arg parsing unless you're doing
something really unusual here.

[ruchic, 2011-01-18 23:52:28]

Kindly review the changes made to the CL.

Thanks,
Ruchi

http://codereview.chromium.org/6187006/diff/1/archive_hwqual
File archive_hwqual (right):

http://codereview.chromium.org/6187006/diff/1/archive_hwqual#newcode103
archive_hwqual:103: cp "${script_dir}/call_script.py
tarball/${FLAGS_output_tag}"
On 2011/01/13 01:52:52, kmixter1 wrote:
> If you're adding this to the package, have you considered changing
> suite_HWQual/README.txt to use this instead of instructing user to call
> server/autoserv directly?  It does mean you'll need to support problems that
may
> occur with this script externally, but it would simplify how partners and we
run
> these tests.

Done. 
This script was initially developed mainly as a utility to speed up the HWQual
test process for the QA team.

However, in case vendors want to use this script, I have submitted the changes
to README.txt in CL http://codereview.chromium.org/6346005

In case vendors want to use the old method of running the HWQual tests, it is
still present in the README.txt

http://codereview.chromium.org/6187006/diff/1/call_script.py
File call_script.py (right):

http://codereview.chromium.org/6187006/diff/1/call_script.py#newcode3
call_script.py:3: # Copyright (c) 2009 The Chromium OS Authors. All rights
reserved.
On 2011/01/13 01:52:52, kmixter1 wrote:
> 2011

Done.

http://codereview.chromium.org/6187006/diff/1/call_script.py#newcode7
call_script.py:7: """Script to run client or server tests on a live remote
image.
On 2011/01/13 01:52:52, kmixter1 wrote:
> call_script.py is a little too generic.  How about call_autoserv.py at least?

Done. Changed the name to call_autoserv.py Thanks!

http://codereview.chromium.org/6187006/diff/1/call_script.py#newcode17
call_script.py:17: import datetime
On 2011/01/13 01:52:52, kmixter1 wrote:
> abc order

Done.

http://codereview.chromium.org/6187006/diff/1/call_script.py#newcode27
call_script.py:27: os.system('echo $USER > /tmp/user_name')
On 2011/01/13 01:52:52, kmixter1 wrote:
> username = os.environ['USER']?

Done. Thanks for the tip

http://codereview.chromium.org/6187006/diff/1/call_script.py#newcode56
call_script.py:56: if ("FAIL" in line) or ("ERROR" in line):
On 2011/01/13 01:52:52, kmixter1 wrote:
> Suggest running generate_test_report and using its return value, that is more
> robust than this method (and we rely on that in other infrastructure including
> run_remote_tests).

Done.

http://codereview.chromium.org/6187006/diff/1/call_script.py#newcode83
call_script.py:83: print "INFO : Starting the HWQual test : %s" % test_path
On 2011/01/13 01:52:52, kmixter1 wrote:
> May want to consider import logging for these.

Done.

http://codereview.chromium.org/6187006/diff/1/call_script.py#newcode84
call_script.py:84: os.system('./autotest/server/autoserv -r ./autotest/%s -m %s
-c %s' % \
Changed the code to support server tests also.

On 2011/01/13 01:52:52, kmixter1 wrote:
> -c assumes the test is a client test.  Probably worth documenting that --test
> argument must be a client test somewhere.  I believe all hwqual tests are
> currently.

http://codereview.chromium.org/6187006/diff/1/call_script.py#newcode99
call_script.py:99: if i in ["-h", "--help", "-?"]:
On 2011/01/13 01:52:52, kmixter1 wrote:
> I would suggest using import optparse for arg parsing unless you're doing
> something really unusual here.

Done.

[kmixter1, 2011-01-21 20:01:46]

Mostly style nits.

http://codereview.chromium.org/6187006/diff/8001/archive_hwqual
File archive_hwqual (right):

http://codereview.chromium.org/6187006/diff/8001/archive_hwqual#newcode102
archive_hwqual:102: # Copy call_script.py to tarball.
Wrong script name here and below.  You should probably run a test - build an
image, and then run archive_build to make sure this works, otherwise our
continuous builds will immediately fail.

http://codereview.chromium.org/6187006/diff/8001/call_autoserv.py
File call_autoserv.py (right):

http://codereview.chromium.org/6187006/diff/8001/call_autoserv.py#newcode30
call_autoserv.py:30: def connectRemoteMachine(machine_ip):
which style guide are you following?  For Google this would be
ConnectRemoteMachine.  For autotest, connect_remote_machine (though a number of
other changes would be necessary as well).  Same comment with others.

http://codereview.chromium.org/6187006/diff/8001/call_autoserv.py#newcode36
call_autoserv.py:36: logger.info("Removing machine IP entry from known hosts to
avoid identity"
Use single or double quotes consistently here and below.  Usually the second
lines of quotes should line up with the first.

http://codereview.chromium.org/6187006/diff/8001/call_autoserv.py#newcode87
call_autoserv.py:87: run_command = './autotest/server/autoserv -r ./autotest/%s
-m %s -s %s' % \
Try to avoid \ by using parentheses.

[ruchic, 2011-01-24 20:48:41]

Please review the changes made.

Thanks,
Ruchi

http://codereview.chromium.org/6187006/diff/8001/archive_hwqual
File archive_hwqual (right):

http://codereview.chromium.org/6187006/diff/8001/archive_hwqual#newcode102
archive_hwqual:102: # Copy call_script.py to tarball.
On 2011/01/21 20:01:46, kmixter1 wrote:
> Wrong script name here and below.  You should probably run a test - build an
> image, and then run archive_build to make sure this works, otherwise our
> continuous builds will immediately fail.

Done.

http://codereview.chromium.org/6187006/diff/8001/call_autoserv.py
File call_autoserv.py (right):

http://codereview.chromium.org/6187006/diff/8001/call_autoserv.py#newcode30
call_autoserv.py:30: def connectRemoteMachine(machine_ip):
On 2011/01/21 20:01:46, kmixter1 wrote:
> which style guide are you following?  For Google this would be
> ConnectRemoteMachine.  For autotest, connect_remote_machine (though a number
of
> other changes would be necessary as well).  Same comment with others.

Done.

http://codereview.chromium.org/6187006/diff/8001/call_autoserv.py#newcode36
call_autoserv.py:36: logger.info("Removing machine IP entry from known hosts to
avoid identity"
On 2011/01/21 20:01:46, kmixter1 wrote:
> Use single or double quotes consistently here and below.  Usually the second
> lines of quotes should line up with the first.

Done.

http://codereview.chromium.org/6187006/diff/8001/call_autoserv.py#newcode87
call_autoserv.py:87: run_command = './autotest/server/autoserv -r ./autotest/%s
-m %s -s %s' % \
On 2011/01/21 20:01:46, kmixter1 wrote:
> Try to avoid \ by using parentheses.

Done.

[kmixter1, 2011-01-25 00:29:45]

LGTM with one nit.

http://codereview.chromium.org/6187006/diff/16001/call_autoserv.py
File call_autoserv.py (right):

http://codereview.chromium.org/6187006/diff/16001/call_autoserv.py#newcode92
call_autoserv.py:92: run_command = "./autotest/server/autoserv -r ./autotest/%s
-m %s -s %s" % (
Sorry, I was suggesting this style:

run_command = ("./autotest/server/autoserv -r ./autotest/%s -m %s -s %s" %
               (result_name, ...))

[ruchic, 2011-01-25 23:53:33]

Done with changes. 

Can you please push the CL if it still looks good to you as generally I have
some permission issues.

Thanks,
Ruchi

http://codereview.chromium.org/6187006/diff/16001/call_autoserv.py
File call_autoserv.py (right):

http://codereview.chromium.org/6187006/diff/16001/call_autoserv.py#newcode92
call_autoserv.py:92: run_command = "./autotest/server/autoserv -r ./autotest/%s
-m %s -s %s" % (
On 2011/01/25 00:29:45, kmixter1 wrote:
> Sorry, I was suggesting this style:
> 
> run_command = ("./autotest/server/autoserv -r ./autotest/%s -m %s -s %s" %
>                (result_name, ...))
> 

Done.

http://codereview.chromium.org/6187006/diff/22001/archive_hwqual
File archive_hwqual (right):

http://codereview.chromium.org/6187006/diff/22001/archive_hwqual#newcode103
archive_hwqual:103: cp "${script_dir}/call_autoserv.py"
"tarball/${FLAGS_output_tag}/."
Correcting the statement after locally creating build and Hwqual package.