run_remote_test.sh: explicitly start/stop ssh-agent

run_remote_test.sh: explicitly start/stop ssh-agent

run_remote_test.sh is implicitly depending on remote_access.sh to setup
the ssh-agent for it. This broke when I tried to remove ssh-agent starting
from remote-access.sh. Implicitly dependencies are ugly.

BUG=n0ne
TEST=Ran a test on my local machine. Tested with and without default board.

Change-Id: I50db1717cafc2565cb8bb648919430d9ce24ce80

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=9af6ba1

[petkov, 2011-01-13 22:54:45]

http://codereview.chromium.org/6333001/diff/1/run_remote_tests.sh
File run_remote_tests.sh (right):

http://codereview.chromium.org/6333001/diff/1/run_remote_tests.sh#newcode19
run_remote_tests.sh:19: DEFINE_string remote "" "remote hostname/IP of running
Chromium OS instance"
sort flags

http://codereview.chromium.org/6333001/diff/1/run_remote_tests.sh#newcode40
run_remote_tests.sh:40: function learn_board() {
any reason why you're not using the same routine from remote_access? this way
you can really claim you're setting up the ssh-agent here just for Autotest. You
might want to include this in a comment somewhere.

http://codereview.chromium.org/6333001/diff/1/run_remote_tests.sh#newcode48
run_remote_tests.sh:48: error "Board required"
this should just use "die"

http://codereview.chromium.org/6333001/diff/1/run_remote_tests.sh#newcode65
run_remote_tests.sh:65: function remote_access_init() {
rename to something that makes it clear this is specific to autotest -- e.g.,
autotest_remote_access_init. same with the cleanup function.

[Mandeep Singh Baines, 2011-01-13 23:13:01]

Fixed. PTAL.

petkov@chromium.org (petkov@chromium.org) wrote:
> 
> http://codereview.chromium.org/6333001/diff/1/run_remote_tests.sh
> File run_remote_tests.sh (right):
> 
> http://codereview.chromium.org/6333001/diff/1/run_remote_tests.sh#newcode19
> run_remote_tests.sh:19: DEFINE_string remote "" "remote hostname/IP of
> running Chromium OS instance"
> sort flags
> 

Removed.

> http://codereview.chromium.org/6333001/diff/1/run_remote_tests.sh#newcode40
> run_remote_tests.sh:40: function learn_board() {
> any reason why you're not using the same routine from remote_access?
> this way you can really claim you're setting up the ssh-agent here just
> for Autotest. You might want to include this in a comment somewhere.
> 

Fair enough. Now using same routine.

> http://codereview.chromium.org/6333001/diff/1/run_remote_tests.sh#newcode48
> run_remote_tests.sh:48: error "Board required"
> this should just use "die"
> 

Removed.

> http://codereview.chromium.org/6333001/diff/1/run_remote_tests.sh#newcode65
> run_remote_tests.sh:65: function remote_access_init() {
> rename to something that makes it clear this is specific to autotest --
> e.g., autotest_remote_access_init. same with the cleanup function.
> 

Done.

> http://codereview.chromium.org/6333001/

[kmixter1, 2011-01-13 23:16:39]

LGTM with nit

http://codereview.chromium.org/6333001/diff/5001/run_remote_tests.sh
File run_remote_tests.sh (right):

http://codereview.chromium.org/6333001/diff/5001/run_remote_tests.sh#newcode32
run_remote_tests.sh:32: function stop_ssh_agent() {
Agree it would be good to say that this is implicitly required to be set up by
autotest.

[petkov, 2011-01-13 23:19:35]

LGTM too but yes -- add a comment that the ssh-agent is required just for
autotest.

[petkov, 2011-01-13 23:22:27]

http://codereview.chromium.org/6333001/diff/5001/run_remote_tests.sh
File run_remote_tests.sh (right):

http://codereview.chromium.org/6333001/diff/5001/run_remote_tests.sh#newcode196
run_remote_tests.sh:196: remote_access_init
btw, are you setting up 2 ssh-agent's now? any conflicts with init/cleanup?