When writing functions (and modules) to set up a specific type of thing, can we please not call it "setup"? Seeing a "setup.setup" call tells me nothing about what that call is doing. If a function is setting up something, I should at least be able to guess at what it's setting up from the name.
-- John On Mon, Mar 29, 2010 at 2:58 PM, Eric Li(李咏竹) <[email protected]> wrote: > Finished code review with Martin. Attached is the patch set generated on > codereview.chromium.org. > global_config.ini is excluded in this patch. > > This change does not fully work with Martin's experimental feature > "hierarchical test structure". More test and code change is needed. But for > plain test directory it should work. > > Risk: Low. > Visibility: The existing user should not see any regression from this > change. > > > > Index: client/bin/base_utils.py > diff --git a/client/bin/base_utils.py b/client/bin/base_utils.py > index > 96d1fc06fa4f616433bae03cff85ec0053cb4279..7c5d26b11996363277180757c9049b9559b95cd1 > 100644 > --- a/client/bin/base_utils.py > +++ b/client/bin/base_utils.py > @@ -63,7 +63,10 @@ def extract_tarball_to_dir(tarball, dir): > the top level of a tarball is - useful for versioned directory names, > etc > """ > if os.path.exists(dir): > - raise NameError, 'target %s already exists' % dir > + if os.path.isdir(dir): > + shutil.rmtree(dir) > + else: > + os.remove(dir) > pwd = os.getcwd() > os.chdir(os.path.dirname(os.path.abspath(dir))) > newdir = extract_tarball(tarball) > Index: client/bin/setup_job.py > diff --git a/client/bin/setup_job.py b/client/bin/setup_job.py > index > bf07fdb16bc65df0916892eb53abfaf009a23e1f..a7088b22196b7fa1247622e5d6b8c5441ed0b819 > 100644 > --- a/client/bin/setup_job.py > +++ b/client/bin/setup_job.py > @@ -30,15 +30,62 @@ class setup_job(client_job.job): > See all options defined on client/bin/autotest. > """ > base_job.base_job.__init__(self, options=options) > - logging_manager.configure_logging( > - client_logging_config.ClientLoggingConfig(), > - results_dir=self.resultdir, > - verbose=options.verbose) > self._cleanup_results_dir() > self.pkgmgr = packages.PackageManager( > self.autodir, run_function_dargs={'timeout':3600}) > > > +def init_test(options, testdir): > + """ > + Instantiate a client test object from a given test directory. > + > + @param options Command line options passed in to instantiate a > setup_job > + which associates with this test. > + @param testdir The test directory. > + @returns A test object or None if failed to instantiate. > + """ > + > + locals_dict = locals().copy() > + globals_dict = globals().copy() > + > + locals_dict['testdir'] = testdir > + > + job = setup_job(options=options) > + locals_dict['job'] = job > + > + test_name = os.path.split(testdir)[-1] > + outputdir = os.path.join(job.resultdir, test_name) > + try: > + os.makedirs(outputdir) > + except OSError: > + pass > + locals_dict['outputdir'] = outputdir > + > + sys.path.insert(0, testdir) > + client_test = None > + try: > + try: > + import_stmt = 'import %s' % test_name > + init_stmt = ('auto_test = %s.%s(job, testdir, outputdir)' % > + (test_name, test_name)) > + exec import_stmt + '\n' + init_stmt in locals_dict, > globals_dict > + client_test = globals_dict['auto_test'] > + except ImportError, e: > + # skips error if test is control file without python test > + if re.search(test_name, str(e)): > + pass > + # give the user a warning if there is an import error. > + else: > + logging.error('%s import error: %s. Skipping %s' % > + (test_name, e, test_name)) > + except Exception, e: > + # Log other errors (e.g., syntax errors) and collect the test. > + logging.error("%s: %s", test_name, e) > + finally: > + sys.path.pop(0) # pop up testbindir > + return client_test > + > + > def load_all_client_tests(options): > """ > Load and instantiate all client tests. > @@ -60,46 +107,51 @@ def load_all_client_tests(options): > for test_base_dir in ['tests', 'site_tests']: > testdir = os.path.join(os.environ['AUTODIR'], test_base_dir) > for test_name in os.listdir(testdir): > - job = setup_job(options=options) > - testbindir = os.path.join(testdir, test_name) > - local_namespace['testbindir'] = testbindir > - > - outputdir = os.path.join(job.resultdir, test_name) > - try: > - os.makedirs(outputdir) > - except OSError: > - pass > - > - local_namespace['job'] = job > - local_namespace['outputdir'] = outputdir > - > - sys.path.insert(0, testbindir) > - try: > - try: > - exec("import %s" % test_name, local_namespace, > - global_namespace) > - exec("auto_test = %s.%s(job, testbindir, outputdir)" % > - (test_name, test_name), local_namespace, > - global_namespace) > - client_test = global_namespace['auto_test'] > - all_tests.append(client_test) > - except ImportError, e: > - # skips error if test is control file without python > test > - if re.search(test_name, str(e)): > - pass > - # give the user a warning if there is an import error. > - else: > - logging.warning("%s import error: %s. Skipping > %s" \ > - % (test_name, e, test_name)) > - except Exception, e: > - # Log other errors (e.g., syntax errors) and collect the > test. > - logging.error("%s: %s", test_name, e) > - broken_tests.append(test_name) > - finally: > - sys.path.pop(0) # pop up testbindir > + client_test = init_test(options, os.path.join(testdir, > test_name)) > + if client_test: > + all_tests.append(client_test) > + else: > + broken_tests.append(test_name) > return all_tests, broken_tests > > > +def setup_test(client_test): > + """ > + Direct invoke test.setup() method. > + > + @returns A boolean to represent success or not. > + """ > + > + # TODO: check if its already build. .version? hash? > + test_name = client_test.__class__.__name__ > + cwd = os.getcwd() > + good_setup = False > + try: > + outputdir = os.path.join(client_test.job.resultdir, test_name) > + try: > + os.makedirs(outputdir) > + os.chdir(outputdir) > + except OSError: > + pass > + logging.info('setup %s.' % test_name) > + client_test.setup() > + > + # Touch .version file under src to prevent further setup on client > + # host. See client/common_lib/utils.py update_version() > + if os.path.exists(client_test.srcdir): > + versionfile = os.path.join(client_test.srcdir, '.version') > + pickle.dump(client_test.version, open(versionfile, 'w')) > + good_setup = True > + except Exception, err: > + logging.error(err) > + raise error.AutoservError('Failed to build client test %s on > server.' % > + test_name) > + finally: > + # back to original working dir > + os.chdir(cwd) > + return good_setup > + > + > def setup_tests(options): > """ > Load and instantiate all client tests. > @@ -138,25 +190,10 @@ def setup_tests(options): > logging.error('### No test setup candidates ###') > raise error.AutoservError('No test setup candidates.') > > - for setup_test in need_to_setup: > - test_name = setup_test.__class__.__name__ > - try: > - outputdir = os.path.join(setup_test.job.resultdir, test_name) > - try: > - os.makedirs(outputdir) > - os.chdir(outputdir) > - except OSError: > - pass > - logging.info('setup %s.' % test_name) > - setup_test.setup() > - # Touch .version file under src to prevent further setup on > client > - # host. See client/common_lib/utils.py update_version() > - if os.path.exists(setup_test.srcdir): > - versionfile = os.path.join(setup_test.srcdir, '.version') > - pickle.dump(setup_test.version, open(versionfile, 'w')) > - except Exception, err: > - logging.error(err) > - failed_tests.append(test_name) > + for client_test in need_to_setup: > + good_setup = setup_test(client_test) > + if not good_setup: > + failed_tests.append(client_test.__class__.__name__) > > logging.info('############################# SUMMARY ' > '#############################') > Index: server/autotest.py > diff --git a/server/autotest.py b/server/autotest.py > index > c35f4a1ed1e3b695ef4218170d0466df5d7d5d81..250a83dcb132f7c726ce8320dafd182ba49c997d > 100644 > --- a/server/autotest.py > +++ b/server/autotest.py > @@ -2,7 +2,7 @@ > > import re, os, sys, traceback, subprocess, time, pickle, glob, tempfile > import logging, getpass > -from autotest_lib.server import installable_object, utils > +from autotest_lib.server import installable_object, setup, utils > from autotest_lib.client.common_lib import log, error, autotemp > from autotest_lib.client.common_lib import global_config, packages > from autotest_lib.client.common_lib import utils as client_utils > @@ -16,6 +16,11 @@ BOOT_TIME = 1800 > CRASH_RECOVERY_TIME = 9000 > > > +get_value = global_config.global_config.get_config_value > +autoserv_prebuild = get_value('AUTOSERV', 'enable_server_prebuild', > + type=bool, default=False) > + > + > class AutodirNotFoundError(Exception): > """No Autotest installation could be found.""" > > @@ -971,10 +976,17 @@ class client_logger(object): > name, pkg_type = self.job.pkgmgr.parse_tarball_name(pkg_name) > src_dirs = [] > if pkg_type == 'test': > - src_dirs += [os.path.join(self.job.clientdir, 'site_tests', > name), > - os.path.join(self.job.clientdir, 'tests', name)] > + for test_dir in ['site_tests', 'tests']: > + src_dir = os.path.join(self.job.clientdir, test_dir, name) > + if os.path.exists(src_dir): > + src_dirs += [src_dir] > + if autoserv_prebuild: > + setup.setup(self.job.clientdir, src_dir) > + break > elif pkg_type == 'profiler': > src_dirs += [os.path.join(self.job.clientdir, 'profilers', > name)] > + if autoserv_prebuild: > + setup.setup(self.job.clientdir, src_dir) > elif pkg_type == 'dep': > src_dirs += [os.path.join(self.job.clientdir, 'deps', name)] > elif pkg_type == 'client': > Index: server/setup.py > diff --git a/server/setup.py b/server/setup.py > new file mode 100644 > index > 0000000000000000000000000000000000000000..4d786050e0128315c6b076926f5fed20c40173ee > --- /dev/null > +++ b/server/setup.py > @@ -0,0 +1,64 @@ > +# Copyright 2010 Google Inc. Released under the GPL v2 > +# > +# Eric Li <[email protected]> > + > +import logging, os, pickle, re, sys > +import common > +from autotest_lib.client.bin import setup_job as client_setup_job > + > + > +def touch_init(parent_dir, child_dir): > + """ > + Touch __init__.py file all alone through from dir_patent to child_dir. > + > + So client tests could be loaded as Python modules. Assume child_dir is > a > + subdirectory of parent_dir. > + """ > + > + if not child_dir.startswith(parent_dir): > + logging.error('%s is not a subdirectory of %s' % (child_dir, > + parent_dir)) > + return > + sub_parent_dirs = parent_dir.split(os.path.sep) > + sub_child_dirs = child_dir.split(os.path.sep) > + for sub_dir in sub_child_dirs[len(sub_parent_dirs):]: > + sub_parent_dirs.append(sub_dir) > + path = os.path.sep.join(sub_parent_dirs) > + init_py = os.path.join(path, '__init__.py') > + open(init_py, 'a').close() > + > + > +def init_test(testdir): > + """ > + Instantiate a client test object from a given test directory. > + > + @param testdir The test directory. > + @returns A test object or None if failed to instantiate. > + """ > + > + class options: > + tag = '' > + verbose = None > + cont = False > + harness = 'autoserv' > + hostname = None > + user = None > + log = True > + return client_setup_job.init_test(options, testdir) > + > + > +def setup(autotest_client_dir, client_test_dir): > + """ > + Setup prebuild of a client test. > + > + @param autotest_client_dir: The autotest/client base directory. > + @param client_test_dir: The actual test directory under client. > + """ > + > + os.environ['AUTODIR'] = autotest_client_dir > + touch_init(autotest_client_dir, client_test_dir) > + > + # instantiate a client_test instance. > + client_test = init_test(client_test_dir) > + client_setup_job.setup_test(client_test) > + > > > > > > > > > > > > > > > > > On Thu, Mar 25, 2010 at 10:50 AM, Eric Li(李咏竹) <[email protected]>wrote: > >> I kept working on server side test prebuild. Now it becomes a part of >> autoserv, and a flag is introduced so prebuild will be triggered before >> client tests/deps/profilers would be tarballed on the server side. >> >> http://codereview.chromium.org/1317002 >> >> Please code review it first and after I got LTGM, I will send a diff patch >> to Martin to apply. >> >> I had passed util/run_pylint.py and utils/unittest_suite.py, and I am not >> expecting this change should introduce any regression or behavior change for >> existing use cases. >> >> Thanks. >> -- >> Eric Li >> 李咏竹 >> Google Kirkland >> >> >> > > > -- > Eric Li > 李咏竹 > Google Kirkland > > > > _______________________________________________ > Autotest mailing list > [email protected] > http://test.kernel.org/cgi-bin/mailman/listinfo/autotest > >
_______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
