Merge lp:~codehelp/lava-dispatcher/multinode-pep8 into lp:lava-dispatcher/multinode

[Neil Williams]

Remaining PEP8 changes to match tip.

Includes the pending changes for group dispatcher - so that merge needs to happen first.

https://code.launchpad.net/~codehelp/lava-dispatcher/multinode/+merge/174612

[Neil Williams (codehelp)]

The equivalent changes in trunk have already been merged, merging.

review: Approve

=== modified file 'lava/dispatcher/commands.py'
--- lava/dispatcher/commands.py	2013-06-21 15:53:31 +0000
+++ lava/dispatcher/commands.py	2013-07-18 07:48:32 +0000
@@ -82,7 +82,7 @@
             help="Test scenario file")
         parser.add_argument(
             "--target",
-            default = None,
+            default=None,
             help="Run the job on a specific target device"
         )
 
@@ -154,7 +154,8 @@
                     exit(0)
         if self.args.target is None:
             if 'target' not in json_jobdata:
-                logging.error("The job file does not specify a target device. You must specify one using the --target option.")
+                logging.error("The job file does not specify a target device. "
+                              "You must specify one using the --target option.")
                 exit(1)
         else:
             json_jobdata['target'] = self.args.target

=== modified file 'lava_dispatcher/actions/deploy.py'
--- lava_dispatcher/actions/deploy.py	2013-07-03 10:44:31 +0000
+++ lava_dispatcher/actions/deploy.py	2013-07-18 07:48:32 +0000
@@ -56,9 +56,9 @@
             'rootfstype': {'type': 'string', 'optional': True},
             'bootloader': {'type': 'string', 'optional': True, 'default': 'u_boot'},
             'role': {'type': 'string', 'optional': True},
-            },
+        },
         'additionalProperties': False,
-        }
+    }
 
     @classmethod
     def validate_parameters(cls, parameters):
@@ -85,9 +85,9 @@
             'system': {'type': 'string'},
             'data': {'type': 'string'},
             'rootfstype': {'type': 'string', 'optional': True, 'default': 'ext4'},
-            },
+        },
         'additionalProperties': False,
-        }
+    }
 
     def run(self, boot, system, data, rootfstype='ext4'):
         self.client.deploy_linaro_android(boot, system, data, rootfstype)
@@ -98,12 +98,12 @@
     parameters_schema = {
         'type': 'object',
         'properties': {
-            'type': {'type': 'string', 'enum':['ubuntu', 'oe', 'android', 'fedora']},
-            },
+            'type': {'type': 'string', 'enum': ['ubuntu', 'oe', 'android', 'fedora']},
+        },
         'additionalProperties': False,
-        }
+    }
 
-    def run(self, type):
+    def run(self, target_type):
         device = self.client.target_device
         device.boot_master_image()
-        device.deployment_data = device.target_map[type]
+        device.deployment_data = device.target_map[target_type]

=== modified file 'lava_dispatcher/actions/launch_control.py'
--- lava_dispatcher/actions/launch_control.py	2013-06-24 16:47:11 +0000
+++ lava_dispatcher/actions/launch_control.py	2013-07-18 07:48:32 +0000
@@ -37,7 +37,9 @@
 
 
 class GatherResultsError(Exception):
-    def __init__(self, msg, bundles=[]):
+    def __init__(self, msg, bundles=None):
+        if not bundles:
+            bundles = []
         super(GatherResultsError, self).__init__(msg)
         self.bundles = bundles
 
@@ -50,7 +52,8 @@
     #Fix it: it's going to be deleted after transition
     if server.endswith("dashboard/"):
         server = ''.join([server, "xml-rpc/"])
-        logging.warn("Please use whole endpoint URL not just end with 'dashboard/', 'xml-rpc/' is added automatically now!!!")
+        logging.warn("Please use whole endpoint URL not just end with 'dashboard/', "
+                     "'xml-rpc/' is added automatically now!!!")
 
     parsed_server = urlparse.urlparse(server)
     auth_backend = MemoryAuthBackend([])
@@ -96,9 +99,9 @@
             'stream': {'type': 'string'},
             'result_disk': {'type': 'string', 'optional': True},
             'token': {'type': 'string', 'optional': True},
-            },
+        },
         'additionalProperties': False,
-        }
+    }
 
     def _get_bundles(self, files):
         bundles = []
@@ -208,9 +211,9 @@
     def combine_bundles(self, all_bundles):
         if not all_bundles:
             main_bundle = {
-                     "test_runs": [],
-                     "format": "Dashboard Bundle Format 1.6"
-                   }
+                "test_runs": [],
+                "format": "Dashboard Bundle Format 1.6"
+            }
         else:
             main_bundle = all_bundles.pop(0)
             test_runs = main_bundle['test_runs']

=== modified file 'lava_dispatcher/actions/lava_test_shell.py'
--- lava_dispatcher/actions/lava_test_shell.py	2013-07-03 10:44:31 +0000
+++ lava_dispatcher/actions/lava_test_shell.py	2013-07-18 07:48:32 +0000
@@ -127,7 +127,6 @@
 from lava_dispatcher import utils
 
 from lava_dispatcher.actions import BaseAction
-from lava_dispatcher.client.base import wait_for_prompt
 from lava_dispatcher.device.target import Target
 from lava_dispatcher.downloader import download_image
 
@@ -166,6 +165,7 @@
 # 755 file permissions
 XMOD = stat.S_IRWXU | stat.S_IXGRP | stat.S_IRGRP | stat.S_IXOTH | stat.S_IROTH
 
+
 def _get_testdef_git_repo(testdef_repo, tmpdir, revision):
     cwd = os.getcwd()
     gitdir = os.path.join(tmpdir, 'gittestrepo')
@@ -251,6 +251,8 @@
 
     def load_from_repo(self, testdef_repo):
         tmpdir = utils.mkdtemp(self.tmpbase)
+        repo = None
+        info = None
         if 'git-repo' in testdef_repo:
             repo = _get_testdef_git_repo(
                 testdef_repo['git-repo'], tmpdir, testdef_repo.get('revision'))
@@ -263,6 +265,8 @@
             name = testdef_repo['bzr-repo'].replace('lp:', '').split('/')[-1]
             info = _bzr_info(testdef_repo['bzr-repo'], repo, name)
 
+        if not repo or not info:
+            logging.debug("Unable to identify specified repository. %s" % testdef_repo)
         test = testdef_repo.get('testdef', 'lavatest.yaml')
         with open(os.path.join(repo, test), 'r') as f:
             logging.info('loading test definition ...')
@@ -283,7 +287,7 @@
             'branch_vcs': 'bzr',
             'branch_revision': revno,
             'branch_url': url,
-            }
+        }
     finally:
         os.chdir(cwd)
 
@@ -298,8 +302,8 @@
             'project_name': name,
             'branch_vcs': 'git',
             'branch_revision': commit_id,
-            'branch_url': url,
-            }
+            'branch_url': url
+        }
     finally:
         os.chdir(cwd)
 
@@ -354,7 +358,8 @@
                 # have non-reproducible behavior because it may rely on
                 # bzr whoami value, presence of ssh keys, etc.
                 subprocess.check_call(['bzr', 'branch', repo],
-                    env={'BZR_HOME': '/dev/null', 'BZR_LOG': '/dev/null'})
+                                      env={'BZR_HOME': '/dev/null',
+                                           'BZR_LOG': '/dev/null'})
                 name = repo.replace('lp:', '').split('/')[-1]
                 self._sw_sources.append(_bzr_info(repo, name, name))
 
@@ -422,8 +427,8 @@
             f.write('read\n')
             steps = self.testdef['run'].get('steps', [])
             if steps:
-              for cmd in steps:
-                  f.write('%s\n' % cmd)
+                for cmd in steps:
+                    f.write('%s\n' % cmd)
             f.write('echo "<LAVA_SIGNAL_ENDRUN $TESTRUN_ID $UUID>"\n')
             f.write('#wait for an ack from the dispatcher\n')
             f.write('read\n')
@@ -466,23 +471,23 @@
             'testdef_repos': {'type': 'array',
                               'items': {'type': 'object',
                                         'properties':
-                                            {'git-repo': {'type': 'string',
-                                                          'optional': True},
-                                             'bzr-repo': {'type': 'string',
-                                                          'optional': True},
-                                             'revision': {'type': 'string',
-                                                          'optional': True},
-                                             'testdef': {'type': 'string',
-                                                         'optional': True}
-                                             },
+                                        {'git-repo': {'type': 'string',
+                                        'optional': True},
+                                        'bzr-repo': {'type': 'string',
+                                        'optional': True},
+                                        'revision': {'type': 'string',
+                                        'optional': True},
+                                        'testdef': {'type': 'string',
+                                        'optional': True}
+                                         },
                                         'additionalProperties': False},
                               'optional': True
                               },
             'timeout': {'type': 'integer', 'optional': True},
             'role': {'type': 'string', 'optional': True},
-            },
+        },
         'additionalProperties': False,
-        }
+    }
 
     def run(self, testdef_urls=None, testdef_repos=None, timeout=-1):
         target = self.client.target_device
@@ -511,12 +516,12 @@
 
     def _keep_running(self, runner, timeout, signal_director):
         patterns = [
-                '<LAVA_TEST_RUNNER>: exiting',
-                pexpect.EOF,
-                pexpect.TIMEOUT,
-                '<LAVA_SIGNAL_(\S+) ([^>]+)>',
-                '<LAVA_MULTI_NODE> <LAVA_(\S+) ([^>]+)>',
-                ]
+            '<LAVA_TEST_RUNNER>: exiting',
+            pexpect.EOF,
+            pexpect.TIMEOUT,
+            '<LAVA_SIGNAL_(\S+) ([^>]+)>',
+            '<LAVA_MULTI_NODE> <LAVA_(\S+) ([^>]+)>',
+        ]
 
         idx = runner._connection.expect(patterns, timeout=timeout)
         if idx == 0:
@@ -664,4 +669,3 @@
             prefix='lava-test-shell', suffix='.bundle', dir=rdir)
         with os.fdopen(fd, 'w') as f:
             DocumentIO.dump(f, bundle)
-

=== modified file 'lava_dispatcher/device/master.py'
--- lava_dispatcher/device/master.py	2013-07-02 14:37:47 +0000
+++ lava_dispatcher/device/master.py	2013-07-18 07:48:32 +0000
@@ -85,7 +85,7 @@
             'oe': Target.oe_deployment_data,
             'ubuntu': Target.ubuntu_deployment_data,
             'fedora': Target.fedora_deployment_data,
-            }
+        }
 
         self.master_ip = None
         self.device_version = None
@@ -128,7 +128,7 @@
             self._deploy_android_tarballs(master, boot, system, data)
 
             if master.has_partition_with_label('userdata') and \
-                   master.has_partition_with_label('sdcard'):
+                    master.has_partition_with_label('sdcard'):
                 _purge_linaro_android_sdcard(master)
 
         self.deployment_data = Target.android_deployment_data
@@ -245,8 +245,7 @@
     def _format_testpartition(self, runner, fstype):
         logging.info("Format testboot and testrootfs partitions")
         runner.run('umount /dev/disk/by-label/testrootfs', failok=True)
-        runner.run('mkfs -t %s -q /dev/disk/by-label/testrootfs -L testrootfs'
-            % fstype, timeout=1800)
+        runner.run('mkfs -t %s -q /dev/disk/by-label/testrootfs -L testrootfs' % fstype, timeout=1800)
         runner.run('umount /dev/disk/by-label/testboot', failok=True)
         runner.run('mkfs.vfat /dev/disk/by-label/testboot -n testboot')
 
@@ -288,7 +287,7 @@
                 return
             except (OperationFailed, pexpect.TIMEOUT):
                 logging.warning(("transfering %s failed. %d retry left."
-                    % (tar_url, num_retry - 1)))
+                                 % (tar_url, num_retry - 1)))
 
             if num_retry > 1:
                 # send CTRL C in case wget still hasn't exited.
@@ -299,7 +298,7 @@
                 sleep_time = 60
                 logging.info("Wait %d second before retry" % sleep_time)
                 time.sleep(sleep_time)
-            num_retry = num_retry - 1
+            num_retry -= 1
 
         raise RuntimeError('extracting %s on target failed' % tar_url)
 
@@ -321,7 +320,7 @@
     @contextlib.contextmanager
     def file_system(self, partition, directory):
         logging.info('attempting to access master filesystem %r:%s' %
-            (partition, directory))
+                     (partition, directory))
 
         assert directory != '/', "cannot mount entire partition"
 
@@ -336,7 +335,7 @@
                 parent_dir, target_name = os.path.split(targetdir)
 
                 runner.run('tar -czf /tmp/fs.tgz -C %s %s' %
-                    (parent_dir, target_name))
+                           (parent_dir, target_name))
                 runner.run('cd /tmp')  # need to be in same dir as fs.tgz
                 self.proc.sendline('python -m SimpleHTTPServer 0 2>/dev/null')
                 match_id = self.proc.expect([
@@ -545,8 +544,8 @@
 
         pattern1 = "<(\d?\d?\d?\.\d?\d?\d?\.\d?\d?\d?\.\d?\d?\d?)>"
         cmd = ("ifconfig %s | grep 'inet addr' | awk -F: '{print $2}' |"
-                "awk '{print \"<\" $1 \">\"}'" %
-                self._client.config.default_network_interface)
+               "awk '{print \"<\" $1 \">\"}'" %
+               self._client.config.default_network_interface)
         self.run(
             cmd, [pattern1, pexpect.EOF, pexpect.TIMEOUT], timeout=5)
         if self.match_id != 0:
@@ -568,7 +567,7 @@
                  "| sed 's/[^0-9-]//g; s/^-\+//')"
                  "\"",
                  [pattern, pexpect.EOF, pexpect.TIMEOUT],
-                 timeout = 5)
+                 timeout=5)
 
         device_version = None
         if self.match_id == 0:
@@ -662,11 +661,10 @@
     # delete use of cache partition
     session.run('sed -i "/\/dev\/block\/%s%s%s/d" %s'
                 % (blkorg, partition_padding_string_org, cache_part_org, rc_filename))
-    session.run('sed -i "s/%s%s%s/%s%s%s/g" %s'
-                % (blkorg, partition_padding_string_org, data_part_org, blklava, partition_padding_string_lava, data_part_lava, rc_filename))
-    session.run('sed -i "s/%s%s%s/%s%s%s/g" %s'
-                % (blkorg, partition_padding_string_org, sys_part_org, blklava, partition_padding_string_lava, sys_part_lava, rc_filename))
-
+    session.run('sed -i "s/%s%s%s/%s%s%s/g" %s' % (blkorg, partition_padding_string_org, data_part_org, blklava,
+                                                   partition_padding_string_lava, data_part_lava, rc_filename))
+    session.run('sed -i "s/%s%s%s/%s%s%s/g" %s' % (blkorg, partition_padding_string_org, sys_part_org, blklava,
+                                                   partition_padding_string_lava, sys_part_lava, rc_filename))
 
 
 def _recreate_uInitrd(session, target):
@@ -740,9 +738,9 @@
     script_path = '%s/%s' % ('/mnt/lava', '/system/bin/disablesuspend.sh')
     if not session.is_file_exist(script_path):
         session.run("sh -c 'export http_proxy=%s'" %
-            target.context.config.lava_proxy)
+                    target.context.config.lava_proxy)
         session.run('wget --no-check-certificate %s -O %s' %
-            (target.config.git_url_disablesuspend_sh, script_path))
+                    (target.config.git_url_disablesuspend_sh, script_path))
         session.run('chmod +x %s' % script_path)
         session.run('chown :2000 %s' % script_path)
 
@@ -772,9 +770,9 @@
     data_label = _android_data_label(session)
     session.run('umount /dev/disk/by-label/%s' % data_label, failok=True)
     session.run('mkfs.ext4 -q /dev/disk/by-label/%s -L %s' %
-        (data_label, data_label))
+                (data_label, data_label))
     session.run('udevadm trigger')
     session.run('mkdir -p /mnt/lava/data')
-    session.run('mount /dev/disk/by-label/%s /mnt/lava/data' % (data_label))
+    session.run('mount /dev/disk/by-label/%s /mnt/lava/data' % data_label)
     session._client.target_extract(session, datatbz2, '/mnt/lava', timeout=600)
     session.run('umount /mnt/lava/data')

=== modified file 'lava_dispatcher/device/target.py'
--- lava_dispatcher/device/target.py	2013-07-03 10:44:31 +0000
+++ lava_dispatcher/device/target.py	2013-07-18 07:48:32 +0000
@@ -24,8 +24,7 @@
 import re
 
 from lava_dispatcher.client.lmc_utils import (
-    image_partition_mounted,
-    )
+    image_partition_mounted)
 import lava_dispatcher.utils as utils
 
 
@@ -49,7 +48,7 @@
         'TESTER_PS1': ANDROID_TESTER_PS1,
         'TESTER_PS1_PATTERN': ANDROID_TESTER_PS1,
         'TESTER_PS1_INCLUDES_RC': False,
-        }
+    }
     ubuntu_deployment_data = {
         'TESTER_PS1': "linaro-test [rc=$(echo \$?)]# ",
         'TESTER_PS1_PATTERN': "linaro-test \[rc=(\d+)\]# ",

=== modified file 'lava_dispatcher/lava_test_shell.py'
--- lava_dispatcher/lava_test_shell.py	2013-04-08 04:57:50 +0000
+++ lava_dispatcher/lava_test_shell.py	2013-07-18 07:48:32 +0000
@@ -103,13 +103,12 @@
 
 
 def _get_sw_context(build, pkgs, sw_sources):
-    ctx = {}
-    ctx['image'] = {'name': build}
+    ctx = {'image': {'name': build}}
 
     pkglist = []
     pattern = re.compile(
         ("^\s*package:\s*(?P<package_name>[^:]+?)\s*:"
-        "\s*(?P<version>[^\s].+)\s*$"), re.M)
+         "\s*(?P<version>[^\s].+)\s*$"), re.M)
     for line in pkgs.split('\n'):
         match = pattern.search(line)
         if match:
@@ -121,9 +120,9 @@
     return ctx
 
 
-def _attachments_from_dir(dir):
+def _attachments_from_dir(from_dir):
     attachments = []
-    for filename, filepath in _directory_names_and_paths(dir, ignore_missing=True):
+    for filename, filepath in _directory_names_and_paths(from_dir, ignore_missing=True):
         if filename.endswith('.mimetype'):
             continue
         mime_type = _read_content(filepath + '.mimetype', ignore_missing=True).strip()
@@ -136,48 +135,47 @@
     return attachments
 
 
-def _attributes_from_dir(dir):
+def _attributes_from_dir(from_dir):
     attributes = {}
-    for filename, filepath in _directory_names_and_paths(dir, ignore_missing=True):
+    for filename, filepath in _directory_names_and_paths(from_dir, ignore_missing=True):
         if os.path.isfile(filepath):
             attributes[filename] = _read_content(filepath)
     return attributes
 
 
-def _result_to_dir(test_result, dir):
+def _result_to_dir(test_result, res_dir):
 
     def w(name, content):
-        with open(os.path.join(dir, name), 'w') as f:
+        with open(os.path.join(res_dir, name), 'w') as f:
             f.write(str(content) + '\n')
 
     for name in 'result', 'measurement', 'units', 'message', 'timestamp', 'duration':
         if name in test_result:
             w(name, test_result[name])
 
-
-    os.makedirs(os.path.join(dir, 'attachments'))
+    os.makedirs(os.path.join(res_dir, 'attachments'))
 
     for attachment in test_result.get('attachments', []):
         path = 'attachments/' + attachment['pathname']
         w(path, base64.b64decode(attachment['content']))
         w(path + '.mimetype', attachment['mime_type'])
 
-    os.makedirs(os.path.join(dir, 'attributes'))
+    os.makedirs(os.path.join(res_dir, 'attributes'))
 
     for attrname, attrvalue in test_result.get('attributes', []).items():
         path = 'attributes/' + attrname
         w(path, attrvalue)
 
 
-def _result_from_dir(dir, test_case_id=None):
+def _result_from_dir(res_dir, test_case_id=None):
     if not test_case_id:
-        test_case_id = os.path.basename(dir)
+        test_case_id = os.path.basename(res_dir)
     result = {
         'test_case_id': test_case_id
-        }
+    }
 
     for fname in 'result', 'measurement', 'units', 'message', 'timestamp', 'duration':
-        fpath = os.path.join(dir, fname)
+        fpath = os.path.join(res_dir, fname)
         if os.path.isfile(fpath):
             result[fname] = _read_content(fpath).strip()
 
@@ -185,11 +183,11 @@
         try:
             result['measurement'] = decimal.Decimal(result['measurement'])
         except decimal.InvalidOperation:
-            logging.warning("Invalid measurement for %s: %s" % (dir, result['measurement']))
+            logging.warning("Invalid measurement for %s: %s" % (res_dir, result['measurement']))
             del result['measurement']
 
-    result['attachments'] = _attachments_from_dir(os.path.join(dir, 'attachments'))
-    result['attributes'] = _attributes_from_dir(os.path.join(dir, 'attributes'))
+    result['attachments'] = _attachments_from_dir(os.path.join(res_dir, 'attachments'))
+    result['attributes'] = _attributes_from_dir(os.path.join(res_dir, 'attributes'))
 
     return result
 
@@ -214,6 +212,7 @@
 def _get_test_results(test_run_dir, testdef, stdout):
     results_from_log_file = []
     fixupdict = {}
+    pattern = None
 
     if 'parse' in testdef:
         if 'fixupdict' in testdef['parse']:
@@ -227,6 +226,8 @@
                      'UNKNOWN': 'unknown'}
         logging.warning("""Using a default pattern to parse the test result. This may lead to empty test result in certain cases.""")
 
+    if not pattern:
+        logging.debug("No pattern set")
     for lineno, line in enumerate(stdout.split('\n'), 1):
         match = pattern.match(line.strip())
         if match:
@@ -244,7 +245,7 @@
                     res['measurement'] = decimal.Decimal(res['measurement'])
                 except decimal.InvalidOperation:
                     logging.warning("Invalid measurement %s" % (
-                            res['measurement']))
+                        res['measurement']))
                     del res['measurement']
             results_from_log_file.append(res)
 
@@ -277,10 +278,8 @@
 
 
 def _get_run_attachments(test_run_dir, testdef, stdout):
-    attachments = []
-
-    attachments.append(create_attachment('stdout.log', stdout))
-    attachments.append(create_attachment('testdef.yaml', testdef))
+    attachments = [create_attachment('stdout.log', stdout),
+                   create_attachment('testdef.yaml', testdef)]
     return_code = _read_content(os.path.join(test_run_dir, 'return_code'), ignore_missing=True)
     if return_code:
         attachments.append(create_attachment('return_code', return_code))
@@ -301,7 +300,7 @@
         'os': None,
         'devices': None,
         'environment': None
-        }
+    }
 
     metadata = _read_content(os.path.join(test_run_dir, 'testdef_metadata'))
     if metadata is not '':
@@ -339,7 +338,7 @@
         'attachments': attachments,
         'attributes': attributes,
         'testdef_metadata': _get_run_testdef_metadata(test_run_dir)
-        }
+    }
 
 
 def _read_content(filepath, ignore_missing=False):