Fix server side autotests installing to the wrong directory.

Fix server side autotests installing to the wrong directory.

I've added two new constants to global_config.ini: chromeos_client_autodir (default: /mnt/stateful_partition/autotest) and chromeos_client_mount (default: /home/autotest_mount). Autotest will now always create these directories and then mount chromeos_client_autodir at chromeos_client_mount as executable.

A previous change did this for all client tests, but server side tests were over looked. Original CL is at http://codereview.chromium.org/3763007/show . I've merely relocated the code into autotest.py and updated unit tests and surrounding code.

Since the original variable for client installation, client_autodir_paths, has no meaning for us in Chrome OS. I've removed it and ensured there are no dependencies.

Some tests will need modification to work properly as they improperly hard coded the path to the autotest directory instead of querying the autodir variable.

Change-Id: Ic8f9171d14a754d86f38ec882006a4217eab8ec3

BUG=8336
TEST=Ran unit tests. Tested in production; ran bvt, works fine.

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=0b6843a

[DaleCurtis, 2010-10-28 21:52:14]

PTAL

Mike, since I've made changes to global_config.ini how do we merge these with
what we have on /home/build/static/... ?

[truty1, 2010-10-28 21:55:13]

The contents of hbs are not regularly updated. They're simply enough to keep
the cli alive.  I don't think we need to update hbs do you?

On Thu, Oct 28, 2010 at 2:52 PM, <dalecurtis@chromium.org> wrote:

> PTAL
>
> Mike, since I've made changes to global_config.ini how do we merge these
> with
> what we have on /home/build/static/... ?
>
>
> http://codereview.chromium.org/4176007/show
>



-- 
Thanks,
Mike

[DaleCurtis, 2010-10-28 21:59:38]

You're correct, I did not realize we kept the global_config.ini file used on
cautotest in the private autotest-tools repo. After talking with Eric, I'll
submit a separate CL with the ini additions there.

On 2010/10/28 21:55:13, truty1 wrote:
> The contents of hbs are not regularly updated. They're simply enough to keep
> the cli alive.  I don't think we need to update hbs do you?
> 
> On Thu, Oct 28, 2010 at 2:52 PM, <mailto:dalecurtis@chromium.org> wrote:
> 
> > PTAL
> >
> > Mike, since I've made changes to global_config.ini how do we merge these
> > with
> > what we have on /home/build/static/... ?
> >
> >
> > http://codereview.chromium.org/4176007/show
> >
> 
> 
> 
> -- 
> Thanks,
> Mike
>

[ericli, 2010-10-28 23:05:03]

http://codereview.chromium.org/4176007/diff/2001/3001
File global_config.ini (right):

http://codereview.chromium.org/4176007/diff/2001/3001#newcode102
global_config.ini:102: chromeos_client_autodir: /mnt/stateful_partition/autotest
I think we should reserve the entry for client_autodir_paths and keep it as it
is.

It is OK to change the default value to /mnt/stateful_partition/autotest

http://codereview.chromium.org/4176007/diff/2001/3001#newcode103
global_config.ini:103: chromeos_client_mount: /home/autotest_mount
Adding a new entry seems appropriate.
For naming consistent, I would suggest to name it as client_autodir_mount.

http://codereview.chromium.org/4176007/diff/2001/3002
File server/autotest.py (right):

http://codereview.chromium.org/4176007/diff/2001/3002#newcode59
server/autotest.py:59: 'AUTOSERV', 'chromeos_client_autodir', type=str)
Do not change this function, even the type. I dont think anyone on this project
bothered to use multiple dirs as list.
We should be OK.

Plus it give me trouble for future merge. This change definitely will trigger a
manual merge for me.

http://codereview.chromium.org/4176007/diff/2001/3002#newcode61
server/autotest.py:61: 'AUTOSERV', 'chromeos_client_mount', type=str)
Define a new function to return this value.

[DaleCurtis, 2010-10-29 01:09:57]

PTAL. I reworked the CL to minimize changes to Autotest.

On 2010/10/28 23:05:03, ericli wrote:
> http://codereview.chromium.org/4176007/diff/2001/3001
> File global_config.ini (right):
> 
> http://codereview.chromium.org/4176007/diff/2001/3001#newcode102
> global_config.ini:102: chromeos_client_autodir:
/mnt/stateful_partition/autotest
> I think we should reserve the entry for client_autodir_paths and keep it as it
> is.
> 
> It is OK to change the default value to /mnt/stateful_partition/autotest
> 
> http://codereview.chromium.org/4176007/diff/2001/3001#newcode103
> global_config.ini:103: chromeos_client_mount: /home/autotest_mount
> Adding a new entry seems appropriate.
> For naming consistent, I would suggest to name it as client_autodir_mount.
> 
> http://codereview.chromium.org/4176007/diff/2001/3002
> File server/autotest.py (right):
> 
> http://codereview.chromium.org/4176007/diff/2001/3002#newcode59
> server/autotest.py:59: 'AUTOSERV', 'chromeos_client_autodir', type=str)
> Do not change this function, even the type. I dont think anyone on this
project
> bothered to use multiple dirs as list.
> We should be OK.
> 
> Plus it give me trouble for future merge. This change definitely will trigger
a
> manual merge for me.
> 
> http://codereview.chromium.org/4176007/diff/2001/3002#newcode61
> server/autotest.py:61: 'AUTOSERV', 'chromeos_client_mount', type=str)
> Define a new function to return this value.

[DaleCurtis, 2010-10-29 01:10:10]

http://codereview.chromium.org/4176007/diff/2001/3001
File global_config.ini (right):

http://codereview.chromium.org/4176007/diff/2001/3001#newcode102
global_config.ini:102: chromeos_client_autodir: /mnt/stateful_partition/autotest
On 2010/10/28 23:05:04, ericli wrote:
> I think we should reserve the entry for client_autodir_paths and keep it as it
> is.
> 
> It is OK to change the default value to /mnt/stateful_partition/autotest
> 

Done.

http://codereview.chromium.org/4176007/diff/2001/3001#newcode103
global_config.ini:103: chromeos_client_mount: /home/autotest_mount
On 2010/10/28 23:05:04, ericli wrote:
> Adding a new entry seems appropriate.
> For naming consistent, I would suggest to name it as client_autodir_mount.

Done.

http://codereview.chromium.org/4176007/diff/2001/3002
File server/autotest.py (right):

http://codereview.chromium.org/4176007/diff/2001/3002#newcode59
server/autotest.py:59: 'AUTOSERV', 'chromeos_client_autodir', type=str)
On 2010/10/28 23:05:04, ericli wrote:
> Do not change this function, even the type. I dont think anyone on this
project
> bothered to use multiple dirs as list.
> We should be OK.
> 
> Plus it give me trouble for future merge. This change definitely will trigger
a
> manual merge for me.

Done.

http://codereview.chromium.org/4176007/diff/2001/3002#newcode61
server/autotest.py:61: 'AUTOSERV', 'chromeos_client_mount', type=str)
On 2010/10/28 23:05:04, ericli wrote:
> Define a new function to return this value.

Done.

[ericli, 2010-10-29 16:54:12]

http://codereview.chromium.org/4176007/diff/12001/13001
File global_config.ini (right):

http://codereview.chromium.org/4176007/diff/12001/13001#newcode103
global_config.ini:103: client_autodir_real_path:
/mnt/stateful_partition/autotest
for even better upstream compatibility, you could add a comment here to memtion
client_autodir_real_path could also be a list, so the value from
client_autodir_paths and client_autodir_real_path could could be 1 to 1 mapping.

But to be honest, I dont think we will need this feature so a comment should be
enough.

You code is easy enough to support list.

http://codereview.chromium.org/4176007/diff/12001/13003
File server/autotest_unittest.py (right):

http://codereview.chromium.org/4176007/diff/12001/13003#newcode292
server/autotest_unittest.py:292: self.assertEquals(install_dir, '/some/path')
can you change line 281 to /another/path and keep this line unchanged?

[truty, 2010-10-29 17:40:19]

http://codereview.chromium.org/4176007/diff/12001/13002
File server/autotest.py (right):

http://codereview.chromium.org/4176007/diff/12001/13002#newcode125
server/autotest.py:125: host.run('mkdir ' + client_autodir_real_path)
Don't these mkdir commands need to be mkdir -p in case they are comprised of
multiple nonexistant dirs.

http://codereview.chromium.org/4176007/diff/12001/13002#newcode128
server/autotest.py:128: host.run('mkdir ' + path)
Same comment for -p on this mkdir

[DaleCurtis, 2010-10-29 17:44:08]

http://codereview.chromium.org/4176007/diff/12001/13001
File global_config.ini (right):

http://codereview.chromium.org/4176007/diff/12001/13001#newcode103
global_config.ini:103: client_autodir_real_path:
/mnt/stateful_partition/autotest
On 2010/10/29 16:54:12, ericli wrote:
> for even better upstream compatibility, you could add a comment here to
memtion
> client_autodir_real_path could also be a list, so the value from
> client_autodir_paths and client_autodir_real_path could could be 1 to 1
mapping.
> 
> But to be honest, I dont think we will need this feature so a comment should
be
> enough.
> 
> You code is easy enough to support list.
> 

Done.

http://codereview.chromium.org/4176007/diff/12001/13003
File server/autotest_unittest.py (right):

http://codereview.chromium.org/4176007/diff/12001/13003#newcode292
server/autotest_unittest.py:292: self.assertEquals(install_dir, '/some/path')
No, I'd have to change the order on line 244 and that would require me changing
all of the unit tests. 

On 2010/10/29 16:54:12, ericli wrote:
> can you change line 281 to /another/path and keep this line unchanged?

[DaleCurtis, 2010-10-29 17:47:05]

http://codereview.chromium.org/4176007/diff/12001/13002
File server/autotest.py (right):

http://codereview.chromium.org/4176007/diff/12001/13002#newcode125
server/autotest.py:125: host.run('mkdir ' + client_autodir_real_path)
On 2010/10/29 17:40:20, truty wrote:
> Don't these mkdir commands need to be mkdir -p in case they are comprised of
> multiple nonexistant dirs.

Done.

http://codereview.chromium.org/4176007/diff/12001/13002#newcode128
server/autotest.py:128: host.run('mkdir ' + path)
On 2010/10/29 17:40:20, truty wrote:
> Same comment for -p on this mkdir

Done.

[ericli, 2010-10-29 17:50:27]

LGTM.

[truty, 2010-10-29 17:59:41]

lgtm