Continue to refactor autoserv to build client test at run time.

Continue to refactor autoserv to build client test at run time.

The build will happen right before it is packaged and sent to a client host.

[mbligh, 2010-03-26 18:03:44]

http://codereview.chromium.org/1317002/diff/12002/18002
File server/autoserv_parser.py (right):

http://codereview.chromium.org/1317002/diff/12002/18002#newcode137
server/autoserv_parser.py:137: help="")
Can we do this as a per-host flag, or dynamically detect distro? 

Either that or global_config.ini option, which is probably much simpler for now.

http://codereview.chromium.org/1317002/diff/12002/18003
File server/autotest.py (right):

http://codereview.chromium.org/1317002/diff/12002/18003#newcode983
server/autotest.py:983: setup.setup(self.job.clientdir, src_dir)
do we want to break here, after we've done site_tests/foo but before tests/foo?
Not a big deal, but probably cleaner.

[ericli, 2010-03-26 23:45:09]

take another look.
Its not well tested with your new hierarchical structure to tests/ directory
change.

Testing it for now.

[mbligh_google.com, 2010-03-26 23:58:12]

LGTM

It looks like this will make it transparent? or are there still pieces missing?

On Fri, Mar 26, 2010 at 4:45 PM,  <ericli@chromium.org> wrote:
> Reviewers: mbligh,
>
> Message:
> take another look.
> Its not well tested with your new hierarchical structure to tests/ directory
> change.
>
> Testing it for now.
>
> Description:
> Continue to refactor autoserv to build client test at run time.
>
> The build will happen right before it is packaged and sent to a client host.
>
> Please review this at http://codereview.chromium.org/1317002
>
> Affected files:
>  M client/bin/setup_job.py
>  M global_config.ini
>  M server/autotest.py
>  A server/setup.py
>
>
>

[ericli, 2010-03-27 00:09:28]

Hopefully no.


On Fri, Mar 26, 2010 at 4:58 PM, Martin Bligh <mbligh@google.com> wrote:

> LGTM
>
> It looks like this will make it transparent? or are there still pieces
> missing?
>
> On Fri, Mar 26, 2010 at 4:45 PM,  <ericli@chromium.org> wrote:
> > Reviewers: mbligh,
> >
> > Message:
> > take another look.
> > Its not well tested with your new hierarchical structure to tests/
> directory
> > change.
> >
> > Testing it for now.
> >
> > Description:
> > Continue to refactor autoserv to build client test at run time.
> >
> > The build will happen right before it is packaged and sent to a client
> host.
> >
> > Please review this at http://codereview.chromium.org/1317002
> >
> > Affected files:
> >  M client/bin/setup_job.py
> >  M global_config.ini
> >  M server/autotest.py
> >  A server/setup.py
> >
> >
> >
>



-- 
Eric Li
李咏竹
Google Kirkland

[petkov, 2010-03-29 21:23:20]

lgtm2

http://codereview.chromium.org/1317002/diff/35001/36001
File client/bin/base_utils.py (right):

http://codereview.chromium.org/1317002/diff/35001/36001#newcode10
client/bin/base_utils.py:10: import os, shutil, sys, signal, commands, pickle,
glob, shutil, statvfs
shutil is imported twice?

http://codereview.chromium.org/1317002/diff/35001/36004
File server/autotest.py (right):

http://codereview.chromium.org/1317002/diff/35001/36004#newcode20
server/autotest.py:20: autoserv_precompile = get_value('AUTOSERV',
'enable_server_precompile',
I would substitute compile with build everywhere...

http://codereview.chromium.org/1317002/diff/35001/36005
File server/setup.py (right):

http://codereview.chromium.org/1317002/diff/35001/36005#newcode5
server/setup.py:5: import logging, os, pickle, re, sys
imports on one line?

[ericli, 2010-03-29 21:40:54]

Darin,
Please take another look.

http://codereview.chromium.org/1317002/diff/35001/36001
File client/bin/base_utils.py (right):

http://codereview.chromium.org/1317002/diff/35001/36001#newcode10
client/bin/base_utils.py:10: import os, shutil, sys, signal, commands, pickle,
glob, shutil, statvfs
good catch.

On 2010/03/29 21:23:20, petkov wrote:
> shutil is imported twice?
>

http://codereview.chromium.org/1317002/diff/35001/36004
File server/autotest.py (right):

http://codereview.chromium.org/1317002/diff/35001/36004#newcode20
server/autotest.py:20: autoserv_precompile = get_value('AUTOSERV',
'enable_server_precompile',
changed.

On 2010/03/29 21:23:20, petkov wrote:
> I would substitute compile with build everywhere...
>

http://codereview.chromium.org/1317002/diff/35001/36005
File server/setup.py (right):

http://codereview.chromium.org/1317002/diff/35001/36005#newcode5
server/setup.py:5: import logging, os, pickle, re, sys
import common is very special.
And this style is consistent across the code base.

On 2010/03/29 21:23:20, petkov wrote:
> imports on one line?

[petkov, 2010-03-29 21:45:25]

still lgtm

[mbligh, 2010-03-30 02:00:04]

LGTM

[petkov, 2010-03-31 04:50:15]

no intent to stop progress, just a nit -- lgtm otherwise :-)

http://codereview.chromium.org/1317002/diff/49001/50004
File global_config.ini (right):

http://codereview.chromium.org/1317002/diff/49001/50004#newcode97
global_config.ini:97: enable_server_precompile: False
any chance you could change this one to prebuild too?

[ericli, 2010-03-31 15:24:00]

Darin,

This is OK. global_config.ini is not part of the upstream change. It will go
to our git with another change after all upstream change settled down.

Eric

On Tue, Mar 30, 2010 at 9:50 PM, <petkov@chromium.org> wrote:

> no intent to stop progress, just a nit -- lgtm otherwise :-)
>
>
>
> http://codereview.chromium.org/1317002/diff/49001/50004
> File global_config.ini (right):
>
> http://codereview.chromium.org/1317002/diff/49001/50004#newcode97
> global_config.ini:97: enable_server_precompile: False
> any chance you could change this one to prebuild too?
>
>
> http://codereview.chromium.org/1317002
>



-- 
Eric Li
李咏竹
Google Kirkland