New autotest wrapper script and bash complete support.

New autotest wrapper script and bash complete support.

A new command line script was introduced in this CL named autotest, which is intend to replace both build_autotest.sh and run_remote_tests.sh in the future.

This change list should be reviewed with 
http://codereview.chromium.org/1513006
http://codereview.chromium.org/1595001

all together.

autotest --board x86-generic --build=all will build all client tests.

autotest --board x86-generic --build=unioxbench,ltp will build enlisted client tests.

autotest --board x86-generic -c client/tests/sleeptest/control -m 12.34.56.78 ...

will invoke autoserv inside emerge-x86-generic cross-compiling env, and do a prebuild of the client test before it got pushed onto client host.

you could also:

1. bash complete the directory/file name from autotest directory after -c or -s args.
2. supply any autoserv commandline args transparently.


Note: replace build_autotest.sh/run_remotes_test.sh is only a future plan. When all the three CLs got pushed, no existing use cases should be broken.

I need to change:
1. build_packages script from third_party/chromiumos-overlay/chromeos/scripts/build_packages.
2. add "enable_server_precompile: False " to third_party/autotest/files/global_config.ini

to activate it.

[petkov, 2010-03-31 16:56:34]

Mostly nits.

http://codereview.chromium.org/1595001/diff/4001/5001
File src/scripts/autotest (right):

http://codereview.chromium.org/1595001/diff/4001/5001#newcode18
src/scripts/autotest:18: python $(dirname $0)/autotest.py $@
|| die "autotest failed."

to add some color and make failures more visible.

http://codereview.chromium.org/1595001/diff/4001/5002
File src/scripts/autotest.py (right):

http://codereview.chromium.org/1595001/diff/4001/5002#newcode9
src/scripts/autotest.py:9: # DO NOT CALL THIS SCRIPT DIRECTLY, CALL
src/scripts/autotest INSTEAD.
Remove #!/usr/bin/python then?

http://codereview.chromium.org/1595001/diff/4001/5002#newcode11
src/scripts/autotest.py:11: import logging, optparse, os, sys, subprocess
subprocess, sys

http://codereview.chromium.org/1595001/diff/4001/5002#newcode114
src/scripts/autotest.py:114: if options.autox is False:
if not options.autox:  ?

http://codereview.chromium.org/1595001/diff/4001/5002#newcode116
src/scripts/autotest.py:116: if options.buildcheck is True:
if options.buildcheck:   ?

http://codereview.chromium.org/1595001/diff/4001/5002#newcode143
src/scripts/autotest.py:143: print 'E.g.: ./autotest
--build=disktest,system_SAT'
s/system_SAT/hardware_SAT/

http://codereview.chromium.org/1595001/diff/4001/5002#newcode158
src/scripts/autotest.py:158: if status is not 0:
just 'if status:'

http://codereview.chromium.org/1595001/diff/4001/5002#newcode164
src/scripts/autotest.py:164: environ = os.environ.copy()
Do you need to copy the environment?

http://codereview.chromium.org/1595001/diff/4001/5002#newcode166
src/scripts/autotest.py:166: environ['FEATURES'] = "${FEATURES} -digest noauto"
Use environ.get instead of ${FEATURES}?

http://codereview.chromium.org/1595001/diff/4001/5003
File src/scripts/bash_completion (right):

http://codereview.chromium.org/1595001/diff/4001/5003#newcode125
src/scripts/bash_completion:125: ls --color=never -dBFH ${autotest_dir}/$1*
2>/dev/null |\
If I'm reading this right, this adds file completion from files down?

This is certainly better than nothing but it may be nice to provide more
targeted completion for the separate options, i.e., maybe in addition to under
files, complete from files/client/{tests|site_tests} for -c and
files/server/{tests|site_tests} for -s.

http://codereview.chromium.org/1595001/diff/4001/5003#newcode138
src/scripts/bash_completion:138: COMPREPLY=($(compgen -W "$(_ls_autotest
${cur})"))
If use ${arg#-c=} here (and -s= below), you won't need ${cur}.

http://codereview.chromium.org/1595001/diff/4001/5003#newcode148
src/scripts/bash_completion:148: arg=$(_argeq --board)
It might be nice to put this in a separate function and call from here and from
_board_sysroot.

[petkov, 2010-03-31 17:07:45]

http://codereview.chromium.org/1595001/diff/4001/5003
File src/scripts/bash_completion (right):

http://codereview.chromium.org/1595001/diff/4001/5003#newcode125
src/scripts/bash_completion:125: ls --color=never -dBFH ${autotest_dir}/$1*
2>/dev/null |\
On 2010/03/31 16:56:34, petkov wrote:
> If I'm reading this right, this adds file completion from files down?
> 
> This is certainly better than nothing but it may be nice to provide more
> targeted completion for the separate options, i.e., maybe in addition to under
> files, complete from files/client/{tests|site_tests} for -c and
> files/server/{tests|site_tests} for -s.
> 

Ah, I guess you don't have run_remote_tests's functionality to search for tests
anywhere so what I'm suggesting won't actually work. NM.

[ericli, 2010-03-31 17:41:04]

Thanks for your review, PTAL.

http://codereview.chromium.org/1595001/diff/4001/5001
File src/scripts/autotest (right):

http://codereview.chromium.org/1595001/diff/4001/5001#newcode18
src/scripts/autotest:18: python $(dirname $0)/autotest.py $@
On 2010/03/31 16:56:34, petkov wrote:
> || die "autotest failed."
> 
> to add some color and make failures more visible.
> 

Done.

http://codereview.chromium.org/1595001/diff/4001/5002
File src/scripts/autotest.py (right):

http://codereview.chromium.org/1595001/diff/4001/5002#newcode11
src/scripts/autotest.py:11: import logging, optparse, os, sys, subprocess
On 2010/03/31 16:56:34, petkov wrote:
> subprocess, sys
> 

Done.

http://codereview.chromium.org/1595001/diff/4001/5002#newcode114
src/scripts/autotest.py:114: if options.autox is False:
On 2010/03/31 16:56:34, petkov wrote:
> if not options.autox:  ?
> 

Done.

http://codereview.chromium.org/1595001/diff/4001/5002#newcode116
src/scripts/autotest.py:116: if options.buildcheck is True:
On 2010/03/31 16:56:34, petkov wrote:
> if options.buildcheck:   ?

Done.

http://codereview.chromium.org/1595001/diff/4001/5002#newcode143
src/scripts/autotest.py:143: print 'E.g.: ./autotest
--build=disktest,system_SAT'
On 2010/03/31 16:56:34, petkov wrote:
> s/system_SAT/hardware_SAT/
> 
> 

Done.

http://codereview.chromium.org/1595001/diff/4001/5002#newcode158
src/scripts/autotest.py:158: if status is not 0:
On 2010/03/31 16:56:34, petkov wrote:
> just 'if status:'
> 

Done.

http://codereview.chromium.org/1595001/diff/4001/5002#newcode166
src/scripts/autotest.py:166: environ['FEATURES'] = "${FEATURES} -digest noauto"
Good catch, fixed.

On 2010/03/31 16:56:34, petkov wrote:
> Use environ.get instead of ${FEATURES}?
>

[petkov, 2010-03-31 18:11:34]

A few nits on the bash_completion, LGTM otherwise. Thanks for updating the
bash_completion script.

http://codereview.chromium.org/1595001/diff/14001/15003
File src/scripts/bash_completion (right):

http://codereview.chromium.org/1595001/diff/14001/15003#newcode76
src/scripts/bash_completion:76: _complete_board_flag()
Rename to something like _complete_board_sysroot_flag to distinguish from
overlay-based completion.

http://codereview.chromium.org/1595001/diff/14001/15003#newcode78
src/scripts/bash_completion:78: local arg=$(_argeq --board)
Assign COMPREPLY=() to make it self contained

http://codereview.chromium.org/1595001/diff/14001/15003#newcode83
src/scripts/bash_completion:83: }
return 1 if no completion.

http://codereview.chromium.org/1595001/diff/14001/15003#newcode90
src/scripts/bash_completion:90: COMPREPLY=()
Based on above comment, remove.

http://codereview.chromium.org/1595001/diff/14001/15003#newcode91
src/scripts/bash_completion:91: _complete_board_flag
_complete_board_sysroot_flag && return 0

http://codereview.chromium.org/1595001/diff/14001/15003#newcode152
src/scripts/bash_completion:152: _complete_board_flag
_complete_board_sysroot_flag && return 0

[ericli, 2010-03-31 18:20:21]

Thanks for your review. Please take a final check before I submit. This is my
first bash complete script.

[petkov, 2010-03-31 18:31:31]

still lgtm.

Btw, I assume you've considered the implications of naming the script the same
way as the client/bin/autotest binary? I can't think of anything major other
than potential confusion. If it's not too much trouble, it might be worth
picking a unique name.

[ericli, 2010-03-31 18:40:34]

I am on the other side. I want to let people know this is the only script to
invoke autotest in the future. There are multiple executables under
autotest/files: client/bin/autotest, client/bin/autotest_client,
server/autoserv, like to hide that level of information.

That is why I tried to put the fancy optionparser there to pass autotest
args.


On Wed, Mar 31, 2010 at 11:31 AM, <petkov@chromium.org> wrote:

> still lgtm.
>
> Btw, I assume you've considered the implications of naming the script the
> same
> way as the client/bin/autotest binary? I can't think of anything major
> other
> than potential confusion. If it's not too much trouble, it might be worth
> picking a unique name.
>
>
>
>
> http://codereview.chromium.org/1595001
>



-- 
Eric Li
李咏竹
Google Kirkland