I admit this is not the best naming. I think that is because: 1. The setup module is some wrapper around test.setup(), and I just want to follow that name. 2. There is enough module.class naming inside the code: base. base_job.base_job, harness.harness, profiler.profiler, test.test. I think I simply followed the style.
John, I am open and what is your suggestion? On Mon, Mar 29, 2010 at 3:15 PM, John Admanski <[email protected]> wrote: > 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 >> >> > -- Eric Li 李咏竹 Google Kirkland
_______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
