On Wed, Aug 08, 2012 at 01:31:34PM -0000, Javier Collado wrote: > Javier Collado has proposed merging lp:~javier.collado/utah/static_analysis_4 into lp:utah/dev. > > Requested reviews: > UTAH Dev (utah) See my comments below. I'm pulling setUp and tearDown from utah/client/tests/common.py into the tests namespace so that nosetests will find them and run them on start and finish. Joe > > For more details, see: > https://code.launchpad.net/~javier.collado/utah/static_analysis_4/+merge/118745 > > One more round of static analysis changes > > Please have a look at the commit messages for further information about the problems fixed > -- > https://code.launchpad.net/~javier.collado/utah/static_analysis_4/+merge/118745 > Your team UTAH Dev is requested to review the proposed merge of lp:~javier.collado/utah/static_analysis_4 into lp:utah/dev. > === modified file 'examples/run_install_test.py' > --- examples/run_install_test.py 2012-07-31 20:54:45 +0000 > +++ examples/run_install_test.py 2012-08-08 13:30:26 +0000 > @@ -97,7 +97,8 @@ > try: > inventory.destroy(machine.machineid) > except UTAHException as error: > - sys.stderr.write('Failed to update inventory: ' + str(error)) > + sys.stderr.write('Failed to update inventory: ' > + + str(error)) > finally: > del machine > if len(locallogs) != 0: > > === modified file 'examples/run_test_vm.py' > --- examples/run_test_vm.py 2012-07-31 20:54:02 +0000 > +++ examples/run_test_vm.py 2012-08-08 13:30:26 +0000 > @@ -77,7 +77,8 @@ > try: > inventory.destroy(machine.machineid) > except UTAHException as error: > - sys.stderr.write('Failed to update inventory: ' + str(error)) > + sys.stderr.write('Failed to update inventory: ' > + + str(error)) > finally: > del machine > if len(locallogs) != 0: > > === modified file 'utah/client/common.py' > --- utah/client/common.py 2012-08-02 18:36:14 +0000 > +++ utah/client/common.py 2012-08-08 13:30:26 +0000 > @@ -48,6 +48,7 @@ > CMD_TS_SETUP = 'testsuite_setup' > CMD_TS_CLEANUP = 'testsuite_cleanup' > > + > def do_nothing(obj=None): > """ > Do nothing method used a placeholder for save_state_callbacks. > @@ -265,6 +266,7 @@ > fp.close() > print("### End {} ###".format(artifact)) > > + > def get_media_info(): > """ > Get the contents of the media-info file if available. > @@ -279,6 +281,7 @@ > > return media_info > > + > def get_host_info(): > """ > Get host info, useful for debugging. > @@ -291,6 +294,7 @@ > > return retval > > + > def get_api_config(): > """ > Parse the client configuration file for API configuration data. > @@ -303,4 +307,3 @@ > data = json.load(fp) > > return data['API'] > - > > === modified file 'utah/client/result.py' > --- utah/client/result.py 2012-08-02 16:03:40 +0000 > +++ utah/client/result.py 2012-08-08 13:30:26 +0000 > @@ -4,6 +4,7 @@ > > from .common import get_host_info > > + > class Result(object): > """ > Result collection class. > @@ -132,7 +133,8 @@ > 'uname': list(host_info['uname']), > 'media-info': host_info['media-info'], > } > - yaml.dump(data, sys.stdout, explicit_start='---', default_flow_style=False) > + yaml.dump(data, sys.stdout, explicit_start='---', > + default_flow_style=False) > > status = self.status > self.clear_results() > > === modified file 'utah/client/runner.py' > --- utah/client/runner.py 2012-08-06 19:16:38 +0000 > +++ utah/client/runner.py 2012-08-08 13:30:26 +0000 > @@ -252,7 +252,8 @@ > > self.backup_runlist = os.path.join(self.testdir, 'master.run-reboot') > > - if os.path.exists(self.master_runlist) and self.master_runlist != self.backup_runlist: > + if (os.path.exists(self.master_runlist) > + and self.master_runlist != self.backup_runlist): > shutil.copyfile(self.master_runlist, self.backup_runlist) > > state = { > @@ -308,7 +309,7 @@ > # local filename if needed (it might be a local file already or cached) > try: > local_filename = urllib.urlretrieve(runlist)[0] > - except IOError as e: > + except IOError: > raise exceptions.MissingFile(runlist) > > data = parse_yaml_file(local_filename) > @@ -351,7 +352,8 @@ > > if name not in self.fetched_suites: > if fetch_cmd is not None: > - self.result.add_result(run_cmd(fetch_cmd, cwd=name, cmd_type=CMD_TS_FETCH)) > + self.result.add_result(run_cmd(fetch_cmd, cwd=name, > + cmd_type=CMD_TS_FETCH)) > self.fetched_suites.append(name) > > # If fetch_cmd failed move on to the next testsuite. > > === modified file 'utah/client/testcase.py' > --- utah/client/testcase.py 2012-08-02 15:25:34 +0000 > +++ utah/client/testcase.py 2012-08-08 13:30:26 +0000 > @@ -5,7 +5,6 @@ > from common import run_cmd, parse_control_file > from common import do_nothing > from common import CMD_TC_BUILD, CMD_TC_SETUP, CMD_TC_TEST, CMD_TC_CLEANUP > -from result import Result > from exceptions import MissingFile > > > @@ -106,7 +105,8 @@ > """ > if self.status == 'NOTRUN': > self.set_status('BUILD') > - result.add_result(run_cmd(self.build_cmd, cwd=self.working_dir, cmd_type=CMD_TC_BUILD)) > + result.add_result(run_cmd(self.build_cmd, cwd=self.working_dir, > + cmd_type=CMD_TC_BUILD)) > > def setup(self, result): > """ > @@ -114,7 +114,8 @@ > """ > if self.status == 'BUILD' and result.status == 'PASS': > self.set_status('SETUP') > - result.add_result(run_cmd(self.tc_setup, cwd=self.working_dir, cmd_type=CMD_TC_SETUP)) > + result.add_result(run_cmd(self.tc_setup, cwd=self.working_dir, > + cmd_type=CMD_TC_SETUP)) > > def cleanup(self, result): > """ > @@ -122,7 +123,8 @@ > """ > if self.status == 'RUN': > self.set_status('CLEANUP') > - result.add_result(run_cmd(self.tc_cleanup, cwd=self.working_dir, cmd_type=CMD_TC_CLEANUP)) > + result.add_result(run_cmd(self.tc_cleanup, cwd=self.working_dir, > + cmd_type=CMD_TC_CLEANUP)) > > def run(self): > """ > > === modified file 'utah/client/tests/common.py' > --- utah/client/tests/common.py 2012-08-06 19:40:22 +0000 > +++ utah/client/tests/common.py 2012-08-08 13:30:26 +0000 > @@ -1,7 +1,8 @@ > import os > import shutil > > -from utah.client.common import UTAH_DIR, MASTER_RUNLIST, UTAH_CLIENT > +from utah.client.common import UTAH_DIR, MASTER_RUNLIST > + > > def get_module_path(): > """ > @@ -39,6 +40,7 @@ > #examples_source = os.path.join(UTAH_CLIENT, 'examples') > examples_source = os.path.join(get_module_path(), 'client', 'examples') > > + > def setUp(): > """ > Set up a master.run file that will copy our 'examples' directory to the > @@ -61,6 +63,7 @@ > fp.write(master_runlist_content) > fp.close() > > + > def tearDown(): > """ > Cleanup after ourselves. > > === modified file 'utah/client/tests/manual_privileges.py' > --- utah/client/tests/manual_privileges.py 2012-08-01 17:25:20 +0000 > +++ utah/client/tests/manual_privileges.py 2012-08-08 13:30:26 +0000 > @@ -116,6 +116,7 @@ > print_priv("after") > _do_cmd("id") > > + > class TestPriv(baseTestPriv): > def test_drop_priv(self): > """ > @@ -170,4 +171,3 @@ > }) > > _do_cmd("id") > - > > === modified file 'utah/client/tests/test_common.py' > --- utah/client/tests/test_common.py 2012-07-31 19:40:18 +0000 > +++ utah/client/tests/test_common.py 2012-08-08 13:30:26 +0000 > @@ -10,6 +10,7 @@ > CONFIG, > ) > > + > class TestCommon(unittest.TestCase): > """ > Tests for utah.client.common. > @@ -35,7 +36,7 @@ > host_info = get_host_info() > > print("host_info: {}".format(host_info)) > - > + > self.assertTrue(host_info is not None) > > def test_run_cmd(self): > > === modified file 'utah/client/tests/test_jsonschema.py' > --- utah/client/tests/test_jsonschema.py 2012-08-06 19:16:38 +0000 > +++ utah/client/tests/test_jsonschema.py 2012-08-08 13:30:26 +0000 > @@ -27,22 +27,23 @@ > testsuties: > - name: testsuite1 > fetch_cmd: fetch1 > -""", # typo > +""", # typo > """--- > timeout: 101 > repeat_count: 99 > -""", # missing testsuites > +""", # missing testsuites > """--- > - name: blah > - name: blip > -""", # missing fetch_cmds > +""", # missing fetch_cmds > """--- > testsuites: > - name: blah > - name: blip > -""", # missing fetch_cmds > +""", # missing fetch_cmds > ] > > + > class TestJSONSchema(unittest.TestCase): > > def test_orig_schema(self): > @@ -64,7 +65,8 @@ > jsonschema.validate(data_new, Runner.MASTER_RUNLIST_SCHEMA) > > def test_example_runlist(self): > - example_runlist = os.path.join(os.path.dirname(__file__), "../examples/master.run") > + example_runlist = os.path.join(os.path.dirname(__file__), > + "../examples/master.run") > > self.assertTrue(os.path.exists(example_runlist)) > > @@ -74,7 +76,6 @@ > jsonschema.validate(data, Runner.MASTER_RUNLIST_SCHEMA) > fp.close() > > - > def test_bad_schemas(self): > """ > Test that required fields are enforced by the schema. > @@ -83,4 +84,6 @@ > for content in yaml_content_bad: > data = yaml.load(content) > print("data: {}".format(data)) > - self.assertRaises(jsonschema.ValidationError, jsonschema.validate, data, Runner.MASTER_RUNLIST_SCHEMA) > + self.assertRaises(jsonschema.ValidationError, > + jsonschema.validate, > + data, Runner.MASTER_RUNLIST_SCHEMA) > > === modified file 'utah/client/tests/test_misc.py' > --- utah/client/tests/test_misc.py 2012-08-06 19:40:22 +0000 > +++ utah/client/tests/test_misc.py 2012-08-08 13:30:26 +0000 > @@ -3,11 +3,13 @@ > > from .common import get_module_path > > + > class TestMisc(unittest.TestCase): > def test_get_module_path(self): > > print("__file__: {}".format(__file__)) > > # assumes this file is in 'utah/client/tests' > - file_path = os.path.abspath(os.path.join(os.path.dirname(__file__), "../../")) > + file_path = os.path.abspath(os.path.join(os.path.dirname(__file__), > + "../../")) > self.assertEqual(file_path, get_module_path()) > > === modified file 'utah/client/tests/test_result.py' > --- utah/client/tests/test_result.py 2012-07-30 14:27:12 +0000 > +++ utah/client/tests/test_result.py 2012-08-08 13:30:26 +0000 > @@ -6,6 +6,7 @@ > ResultYAML, > ) > > + > class TestResult(unittest.TestCase): > def setUp(self): > self.result_yaml = ResultYAML("result_yaml") > @@ -118,4 +119,3 @@ > self.assertEqual(result['returncode'], self.result['returncode']) > self.assertEqual(result['stdout'], self.result['stdout']) > self.assertEqual(result['stderr'], self.result['stderr']) > - > > === modified file 'utah/client/tests/test_runner.py' > --- utah/client/tests/test_runner.py 2012-08-07 19:26:12 +0000 > +++ utah/client/tests/test_runner.py 2012-08-08 13:30:26 +0000 > @@ -7,14 +7,8 @@ > from utah.client.common import RETURN_CODES > from utah.client import exceptions > > -from .common import setUp, tearDown setUp and tearDown are required here and are used by nosetests itself. > -from .common import ( > - master_runlist_content, > - master_runlist, > - master_runlist_bak, > - examples_dir, > - examples_source, > - ) > +from .common import master_runlist_content > + > > class TestRunner(unittest.TestCase): > def setUp(self): > > === modified file 'utah/client/tests/test_state_agent.py' > --- utah/client/tests/test_state_agent.py 2012-08-07 19:26:12 +0000 > +++ utah/client/tests/test_state_agent.py 2012-08-08 13:30:26 +0000 > @@ -6,16 +6,9 @@ > from utah.client.state_agent import StateAgentYAML > from utah.client.runner import Runner > from utah.client.result import ResultYAML > -from utah.client.common import ( > - run_cmd, > - UTAH_DIR, > - ) > +from utah.client.common import UTAH_DIR > > -from .common import ( > - setUp, > - tearDown, > - get_module_path, > - ) > +from .common import get_module_path setUp and tearDown are required here and are used by nosetests itself. > > partial_state_file_content = """master_runlist: master.run > status: RUN > @@ -70,6 +63,7 @@ > ts_setup: null > """ > > + > class TestStateAgentYAML(unittest.TestCase): > def setUp(self): > """ > > === modified file 'utah/client/tests/test_testcase.py' > --- utah/client/tests/test_testcase.py 2012-08-02 20:54:39 +0000 > +++ utah/client/tests/test_testcase.py 2012-08-08 13:30:26 +0000 > @@ -7,10 +7,6 @@ > ) > from utah.client import testcase > > -from .common import ( > - setUp, > - tearDown, > - ) setUp and tearDown are required here and are used by nosetests itself. > > class TestTestCase(unittest.TestCase): > """ > @@ -78,4 +74,3 @@ > case.run() > > self.assertTrue(case.is_done()) > - > > === modified file 'utah/client/tests/test_testsuite.py' > --- utah/client/tests/test_testsuite.py 2012-08-02 20:54:39 +0000 > +++ utah/client/tests/test_testsuite.py 2012-08-08 13:30:26 +0000 > @@ -7,14 +7,11 @@ > from utah.client.result import ResultYAML > from utah.client import testsuite > > -from .common import ( > - setUp, > - tearDown, > - ) setUp and tearDown are required here and are used by nosetests itself. > > def state_saver(): > debug_print("Saving state", force=True) > > + > class TestTestSuite(unittest.TestCase): > """ > Tests for the TestSuite class. > > === modified file 'utah/client/tests/test_yaml.py' > --- utah/client/tests/test_yaml.py 2012-07-27 19:42:26 +0000 > +++ utah/client/tests/test_yaml.py 2012-08-08 13:30:26 +0000 > @@ -1,18 +1,19 @@ > import unittest > import yaml > > + > class TestYAML(unittest.TestCase): > def test_parts(self): > content = {} > content['metadata'] = { > - 'runlist' : '/tmp/master.run', > - 'host' : 'Ubuntu 12.04 LTS', > + 'runlist': '/tmp/master.run', > + 'host': 'Ubuntu 12.04 LTS', > } > content['commands'] = [ > { > - 'command' : 'rm -f blah', > - 'stdout' : '', > - 'stderr' : '', > + 'command': 'rm -f blah', > + 'stdout': '', > + 'stderr': '', > } > ] > > @@ -26,7 +27,7 @@ > > def test_parts_2(self): > content = { > - 'file' : '/tmp/file' > + 'file': '/tmp/file' > } > > extra_args = { > @@ -63,7 +64,7 @@ > extra_args = {} > content = [ > { > - 'file' : '/tmp/file', > + 'file': '/tmp/file', > }, > [ > { > > === modified file 'utah/client/testsuite.py' > --- utah/client/testsuite.py 2012-08-02 20:54:39 +0000 > +++ utah/client/testsuite.py 2012-08-08 13:30:26 +0000 > @@ -6,7 +6,6 @@ > from common import DEFAULT_TSLIST, DEFAULT_TSCONTROL > from common import CMD_TS_BUILD, CMD_TS_SETUP, CMD_TS_CLEANUP > from common import do_nothing > -from result import Result > import jsonschema > > import os > @@ -160,7 +159,8 @@ > """ > if self.status == 'NOTRUN': > self.set_status('BUILD') > - result.add_result(run_cmd(self.build_cmd, cwd=self.name, cmd_type=CMD_TS_BUILD)) > + result.add_result(run_cmd(self.build_cmd, cwd=self.name, > + cmd_type=CMD_TS_BUILD)) > > def setup(self, result): > """ > @@ -168,7 +168,8 @@ > """ > if self.status == 'BUILD' and result.status == 'PASS': > self.set_status('SETUP') > - result.add_result(run_cmd(self.ts_setup, cwd=self.name, cmd_type=CMD_TS_SETUP)) > + result.add_result(run_cmd(self.ts_setup, cwd=self.name, > + cmd_type=CMD_TS_SETUP)) > > def cleanup(self, result): > """ > @@ -176,7 +177,8 @@ > """ > if self.status == 'INPROGRESS': > self.set_status('CLEANUP') > - result.add_result(run_cmd(self.ts_cleanup, cwd=self.name, cmd_type=CMD_TS_CLEANUP)) > + result.add_result(run_cmd(self.ts_cleanup, cwd=self.name, > + cmd_type=CMD_TS_CLEANUP)) > > def run(self): > """ > > === modified file 'utah/provisioning/baremetal/cobbler.py' > --- utah/provisioning/baremetal/cobbler.py 2012-08-03 21:44:39 +0000 > +++ utah/provisioning/baremetal/cobbler.py 2012-08-08 13:30:26 +0000 > @@ -132,7 +132,8 @@ > for name in (dirnames + filenames): > filename = os.path.join(dirpath, name) > if not os.path.islink(filename): > - self.logger.debug('Changing permissions of ' + filename) > + self.logger.debug('Changing permissions of ' > + + filename) > os.chmod(filename, 0775) > shutil.rmtree(self.tmpdir) > > > === modified file 'utah/provisioning/provisioning.py' > --- utah/provisioning/provisioning.py 2012-08-08 02:25:52 +0000 > +++ utah/provisioning/provisioning.py 2012-08-08 13:30:26 +0000 > @@ -458,14 +458,16 @@ > self.logger.info('Uploading ' + localpath > + ' from the host to ' + target > + ' on the machine') > - remotepath = os.path.join(target, os.path.basename(localpath)) > + remotepath = os.path.join(target, > + os.path.basename(localpath)) > sftp_client.put(localpath, remotepath) > else: > failed.append(localpath) > finally: > sftp_client.close() > if len(failed) > 0: > - err = UTAHProvisioningException('Files do not exist: ' + ' '.join(failed)) > + err = UTAHProvisioningException('Files do not exist: ' > + + ' '.join(failed)) > err.files = failed > raise err > > @@ -506,7 +508,8 @@ > self.ssh_client.connect(self.name, > key_filename=config.sshprivatekey) > sftp_client = self.ssh_client.open_sftp() > - files = [os.path.join(dirname, myfile) for myfile in sftp_client.listdir(dirname)] > + files = [os.path.join(dirname, myfile) > + for myfile in sftp_client.listdir(dirname)] > self.downloadfiles(files, target) > > def destroy(self, *args, **kw): > @@ -553,13 +556,15 @@ > except socket.error as err: > raise UTAHProvisioningException(str(err), retry=True) > > - def sshpoll(self, timeout=config.boottimeout, checktimeout=config.checktimeout, logmethod=None): > + def sshpoll(self, timeout=config.boottimeout, > + checktimeout=config.checktimeout, logmethod=None): > """ > Run sshcheck over and over until timeout expires. > """ > if logmethod is None: > - logmethod=self.logger.debug > - utah.timeout.timeout(timeout, retry, self.sshcheck, checktimeout, logmethod=logmethod) > + logmethod = self.logger.debug > + utah.timeout.timeout(timeout, retry, self.sshcheck, checktimeout, > + logmethod=logmethod) > > > class CustomInstallMixin(object): > > === modified file 'utah/provisioning/vm/libvirtvm.py' > --- utah/provisioning/vm/libvirtvm.py 2012-07-25 17:16:30 +0000 > +++ utah/provisioning/vm/libvirtvm.py 2012-08-08 13:30:26 +0000 > @@ -6,7 +6,6 @@ > import string > import subprocess > import tempfile > -import time > > from xml.etree import ElementTree > > @@ -24,7 +23,6 @@ > ) > from utah import config > from utah.process import ProcessChecker > -import utah.timeout > > > class LibvirtVM(VM): > @@ -334,7 +332,9 @@ > for myfile in files: > self.logger.info('Uploading ' + myfile > + ' from the host to ' + target + ' on the VM') > - if self._runargs(['vm-scp', '-f', '-p', self.name, myfile, target]) != 0: > + returncode = self._runargs(['vm-scp', '-f', '-p', > + self.name, myfile, target]) > + if returncode != 0: > success = False > return success > >