testnode: some code clean up

A lot was found with pylint.

erp5/tests/testERP5TestNode.py

@@ -237,7 +237,7 @@ class ERP5TestNode(TestCase):
                       node_test_suite.custom_profile_path)
     profile = open(node_test_suite.custom_profile_path, 'r')
     if my_test_type=='UnitTest':
-      expected_profile = """
+      expected_profile = """\
 [buildout]
 extends = %(temp_dir)s/testnode/foo/rep0/software.cfg
 
@@ -258,7 +258,7 @@ shared = true
     else:
       revision1 = "azerty"
       revision2 = "qwerty"
-      expected_profile = """
+      expected_profile = """\
 [buildout]
 extends = %(temp_dir)s/testnode/foo/rep0/software.cfg
 
@@ -802,17 +802,17 @@ shared = true
       test_node.node_test_suite_dict
       rand_part_set = set()
       self.assertEquals(2, len(test_node.node_test_suite_dict))
-      assert(test_node.suite_log is not None)
-      assert(isinstance(test_node.suite_log, types.MethodType))
+      self.assertIsNot(test_node.suite_log, None)
+      self.assertTrue(isinstance(test_node.suite_log, types.MethodType))
       for ref, suite in test_node.node_test_suite_dict.items():
         self.assertTrue('var/log/testnode/%s' % suite.reference in \
                          suite.suite_log_path,
                          "Incorrect suite log path : %r" % suite.suite_log_path)
-        assert(suite.suite_log_path.endswith('suite.log'))
+        self.assertTrue(suite.suite_log_path.endswith('suite.log'))
         m = re.match('.*\-(.*)\/suite.log', suite.suite_log_path)
         rand_part = m.groups()[0]
-        assert(len(rand_part) == 32)
-        assert(rand_part not in rand_part_set)
+        self.assertEqual(len(rand_part), 32)
+        self.assertNotIn(rand_part, rand_part_set)
         rand_part_set.add(rand_part)
         suite_log = open(suite.suite_log_path, 'r')
         self.assertEquals(1, len([x for x in suite_log.readlines() \

erp5/util/testnode/NodeTestSuite.py

@@ -24,23 +24,13 @@
 # Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
 #
 ##############################################################################
-from datetime import datetime,timedelta
+import errno
 import os
-import subprocess
-import sys
-import time
 import glob
-import SlapOSControler
-import json
-import time
 import shutil
-import logging
 import string
 import random
-from ProcessManager import SubprocessError, ProcessManager, CancellationError
-from subprocess import CalledProcessError
-from Updater import Updater
-from erp5.util import taskdistribution
+from .Utils import createFolder
 
 class SlapOSInstance(object):
   """
@@ -67,22 +57,19 @@ class NodeTestSuite(SlapOSInstance):
     self.reference = reference
     self.cluster_configuration = {}
 
-  def edit(self, **kw):
-    super(NodeTestSuite, self).edit(**kw)
-
   def _checkData(self):
     if getattr(self, "working_directory", None) is not None:
       if not(self.working_directory.endswith(os.path.sep + self.reference)):
         self.working_directory = os.path.join(self.working_directory,
                                              self.reference)
-      SlapOSControler.createFolder(self.working_directory)
+      createFolder(self.working_directory)
       self.test_suite_directory = os.path.join(
                                    self.working_directory, "test_suite")
       self.custom_profile_path = os.path.join(self.working_directory,
                                  'software.cfg')
     if getattr(self, "vcs_repository_list", None) is not None:
       for vcs_repository in self.vcs_repository_list:
-        buildout_section_id = vcs_repository.get('buildout_section_id', None)
+        buildout_section_id = vcs_repository.get('buildout_section_id')
         repository_id = buildout_section_id or \
                         vcs_repository.get('url').split('/')[-1].split('.')[0]
         repository_path = os.path.join(self.working_directory,repository_id)
@@ -92,20 +79,22 @@ class NodeTestSuite(SlapOSInstance):
   def createSuiteLog(self):
     # /srv/slapgrid/slappartXX/srv/var/log/testnode/az-mlksjfmlk234Sljssdflkj23KSdfslj/suite.log
     alphabets = string.digits + string.letters
-    rand_part = ''.join(random.choice(alphabets) for i in xrange(32))
-    random_suite_folder_id = '%s-%s' % (self.reference, rand_part)
-    suite_log_directory = os.path.join(self.log_directory,
-                                       random_suite_folder_id)
-    SlapOSControler.createFolders(suite_log_directory)
+    while 1:
+      log_folder_name = '%s-%s' % (self.reference,
+        ''.join(random.choice(alphabets) for i in xrange(32)))
+      log_folder_path = os.path.join(self.log_directory, log_folder_name)
+      try:
+        os.makedirs(log_folder_path)
+      except OSError, e:
+        if e.errno != errno.EEXIST:
+          raise
+      else:
+        break
     # XXX copy the whole content of the log viewer app
     for fname in glob.glob(os.path.join(os.path.dirname(__file__), 'js-logtail', '*')):
-      shutil.copy(fname, suite_log_directory)
-    self.suite_log_path = os.path.join(suite_log_directory,
-                                       'suite.log')
-    return self.getSuiteLogPath(), random_suite_folder_id
-
-  def getSuiteLogPath(self):
-    return getattr(self,"suite_log_path", None)
+      shutil.copy(fname, log_folder_path)
+    self.suite_log_path = os.path.join(log_folder_path, 'suite.log')
+    return self.suite_log_path, log_folder_name
 
   @property
   def revision(self):

erp5/util/testnode/ProcessManager.py

@@ -131,14 +131,14 @@ class ProcessManager(object):
 
   stdin = file(os.devnull)
 
-  def __init__(self, log, *args, **kw):
+  def __init__(self, log, max_timeout=MAX_TIMEOUT):
     self.log = log
     self.process_pid_set = set()
     signal.signal(signal.SIGTERM, self.sigterm_handler)
     self.under_cancellation = False
     self.p = None
     self.result = None
-    self.max_timeout = kw.get("max_timeout") or MAX_TIMEOUT
+    self.max_timeout = max_timeout
     self.timer_set = set()
 
   def spawn(self, *args, **kw):
@@ -188,7 +188,7 @@ class ProcessManager(object):
     return result
 
   def getSupportedParameterList(self, program_path):
-    return re.findall('^  (--\w+)',
+    return re.findall(r'^  (--\w+)',
       self.spawn(program_path, '--help')['stdout'], re.M)
 
   def killall(self, name):
@@ -212,13 +212,15 @@ class ProcessManager(object):
             continue
       except (psutil.AccessDenied, psutil.NoSuchProcess):
         continue
-      self.log('ProcesssManager, killall on %s having pid %s' % (name, process.pid))
+      self.log('ProcesssManager, killall on %s having pid %s',
+               name, process.pid)
       to_kill_list.append(process.pid)
     for pid in to_kill_list:
       killCommand(pid, self.log)
 
   def killPreviousRun(self, cancellation=False):
-    self.log('ProcessManager killPreviousRun, going to kill %r' % (self.process_pid_set,))
+    self.log('ProcessManager killPreviousRun, going to kill %r',
+             self.process_pid_set)
     if cancellation:
       self.under_cancellation = True
     for timer in self.timer_set:
@@ -227,12 +229,13 @@ class ProcessManager(object):
       killCommand(pgpid, self.log)
     try:
       if os.path.exists(self.supervisord_pid_file):
-        supervisor_pid = int(open(self.supervisord_pid_file).read().strip())
-        self.log('ProcessManager killPreviousRun, going to kill supervisor with pid %r' % supervisor_pid)
+        with open(self.supervisord_pid_file) as f:
+          supervisor_pid = int(f.read().strip())
+        self.log('ProcessManager killPreviousRun, going to kill supervisor with pid %r',
+                 supervisor_pid)
         os.kill(supervisor_pid, signal.SIGTERM)
-    except:
+    except Exception:
       self.log('ProcessManager killPreviousRun, exception when killing supervisor')
-      pass
     self.process_pid_set.clear()
 
   def sigterm_handler(self, signal, frame):

erp5/util/testnode/SlapOSMasterCommunicator.py

 import datetime
 import json
-import sys
 import traceback
 import time
 #import feedparser
+from functools import wraps
 from uritemplate import expand
 
 import slapos.slap
 from slapos.slap import SoftwareProductCollection
-
-from slapos.slap.slap import ConnectionError
 from requests.exceptions import HTTPError
-from erp5.util.taskdistribution import SAFE_RPC_EXCEPTION_LIST
+from ..taskdistribution import SAFE_RPC_EXCEPTION_LIST
 
 # max time to instance changing state: 2 hour
 MAX_INSTANCE_TIME = 60*60*2
@@ -41,36 +39,29 @@ TESTER_STATE_INSTANCE_UNINSTALLED = "TESTER_STATE_INSTANCE_UNINSTALLED"
 
 # Simple decorator to prevent raise due small
 # network failures.
-def retryOnNetworkFailure(func):
-  def wrapper(*args, **kwargs):
+def retryOnNetworkFailure(func,
+    _except_list = SAFE_RPC_EXCEPTION_LIST + (
+      HTTPError, slapos.slap.ConnectionError),
+    ):
+  def wrapper(*args, **kw):
     retry_time = 64
     while True:
       try:
-        return func(*args, **kwargs)
-      except SAFE_RPC_EXCEPTION_LIST, e:
-        print 'Network failure: %s , %s' % (sys.exc_info(), e)
-      except HTTPError, e:
-        print 'Network failure: %s , %s' % (sys.exc_info(), e)
-      except ConnectionError, e:
-        print 'Network failure: %s , %s' % (sys.exc_info(), e)
-      except slapos.slap.ConnectionError, e:
-        print 'Network failure: %s , %s' % (sys.exc_info(), e)
-
-      print 'Retry method %s in %i seconds' % (func, retry_time)
+        return func(*args, **kw)
+      except _except_list:
+        traceback.print_exc()
+
+      print 'Network failure. Retry method %s in %i seconds' % (func, retry_time)
       time.sleep(retry_time)
       retry_time = min(retry_time*1.5, 640)
 
-
-  wrapper.__name__ = func.__name__
-  wrapper.__doc__ = func.__doc__
-  return wrapper
+  return wraps(func)(wrapper)
 
 
 class SlapOSMasterCommunicator(object):
   latest_state = None
 
   def __init__(self, slap, slap_supply, slap_order, url, logger):
-
     self._logger = logger
     self.slap = slap
     self.slap_order = slap_order
@@ -102,8 +93,7 @@ class SlapOSMasterCommunicator(object):
     if instance_title is not None:
       self.name = instance_title 
     if request_kw is not None:
-      if isinstance(request_kw, str) or \
-        isinstance(request_kw, unicode):
+      if isinstance(request_kw, basestring):
         self.request_kw = json.loads(request_kw)
       else:
         self.request_kw = request_kw
@@ -116,12 +106,11 @@ class SlapOSMasterCommunicator(object):
             **self.request_kw)
 
   def isInstanceRequested(self, instance_title):
-    hateoas = getattr(self.slap, '_hateoas_navigator', None)
+    hateoas = self._hateoas_navigator
     return instance_title in hateoas.getHostingSubscriptionDict()
 
   @retryOnNetworkFailure
   def _hateoas_getComputer(self, reference):
-
     root_document = self.hateoas_navigator.getRootDocument()
     search_url = root_document["_links"]['raw_search']['href']
 
@@ -147,7 +136,6 @@ class SlapOSMasterCommunicator(object):
   @retryOnNetworkFailure
   def getSoftwareInstallationList(self):
     # XXX Move me to slap.py API 
-
     computer = self._hateoas_getComputer(self.computer_guid)
 
     # Not a list ?
@@ -191,7 +179,6 @@ class SlapOSMasterCommunicator(object):
 
   @retryOnNetworkFailure
   def getInstanceUrlList(self):
-
     if self.hosting_subscription_url is None:
       hosting_subscription_dict = self.hateoas_navigator._hateoas_getHostingSubscriptionDict()
       for hs in hosting_subscription_dict:
@@ -207,7 +194,6 @@ class SlapOSMasterCommunicator(object):
 
   @retryOnNetworkFailure
   def getNewsFromInstance(self, url):
-
     result = self.hateoas_navigator.GET(url)
     result = json.loads(result)
     if result['_links'].get('action_object_slap', None) is None:
@@ -221,7 +207,6 @@ class SlapOSMasterCommunicator(object):
 
   @retryOnNetworkFailure
   def getInformationFromInstance(self, url):
-
     result = self.hateoas_navigator.GET(url)
     result = json.loads(result)
     if result['_links'].get('action_object_slap', None) is None:
@@ -329,7 +314,7 @@ class SlapOSMasterCommunicator(object):
       self._logger('Got an error requesting partition for '
             'its state')
       return INSTANCE_STATE_UNKNOWN
-    except:
+    except Exception:
       self._logger("ERROR getting instance state")
       return INSTANCE_STATE_UNKNOWN
 
@@ -377,6 +362,7 @@ class SlapOSMasterCommunicator(object):
     return {'error_message' : None}
 
 class SlapOSTester(SlapOSMasterCommunicator):
+
   def __init__(self,
               name,
               logger,
@@ -387,7 +373,6 @@ class SlapOSTester(SlapOSMasterCommunicator):
               computer_guid=None, # computer for supply if desired
               request_kw=None
           ):
-
     super(SlapOSTester, self).__init__(
       slap, slap_supply, slap_order, url, logger)
 
@@ -464,7 +449,6 @@ class SoftwareReleaseTester(SlapOSTester):
               software_timeout=3600,
               instance_timeout=3600,
           ):
-
     super(SoftwareReleaseTester, self).__init__(
       name, logger, slap, slap_order, slap_supply, url, computer_guid, request_kw)
 

erp5/util/testnode/UnitTestRunner.py

@@ -24,28 +24,22 @@
 # Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
 #
 ##############################################################################
-from datetime import datetime,timedelta
 import os
-import subprocess
-import sys
-import time
 import glob
-import SlapOSControler
 import json
-import time
-import shutil
-import logging
-import string
-import random
-from ProcessManager import SubprocessError, ProcessManager, CancellationError
-from subprocess import CalledProcessError
-from NodeTestSuite import SlapOSInstance
-from Updater import Updater
-from Utils import dealShebang
-from erp5.util import taskdistribution
+from .ProcessManager import SubprocessError
+from .SlapOSControler import SlapOSControler
+from .Utils import createFolder
 from slapos.grid.utils import md5digest
 
-class UnitTestRunner():
+def dealShebang(run_test_suite_path):
+  with open(run_test_suite_path) as f:
+    if f.read(2) == '#!':
+      return f.readline().split(None, 1)
+  return []
+
+class UnitTestRunner(object):
+
   def __init__(self, testnode):
     self.testnode = testnode
 
@@ -53,12 +47,11 @@ class UnitTestRunner():
     """
     Create a SlapOSControler
     """
-    return SlapOSControler.SlapOSControler(
+    return SlapOSControler(
                working_directory,
                self.testnode.config,
                self.testnode.log)
  
-
   def _prepareSlapOS(self, working_directory, slapos_instance, log,
           create_partition=1, software_path_list=None, **kw):
     """
@@ -66,11 +59,11 @@ class UnitTestRunner():
     """
     slapproxy_log = os.path.join(self.testnode.config['log_directory'],
                                   'slapproxy.log')
-    log('Configured slapproxy log to %r' % slapproxy_log)
+    log('Configured slapproxy log to %r', slapproxy_log)
     reset_software = slapos_instance.retry_software_count > 10
     if reset_software:
       slapos_instance.retry_software_count = 0
-    log('testnode, retry_software_count : %r' % \
+    log('testnode, retry_software_count : %r',
              slapos_instance.retry_software_count)
 
     # XXX Create a new controler because working_directory can be
@@ -108,26 +101,22 @@ class UnitTestRunner():
     """
     # report-url, report-project and suite-url are required to seleniumrunner
     # instance. This is a hack which must be removed.
-    cluster_configuration = {}
     config = self.testnode.config
-    cluster_configuration['report-url'] = config.get("report-url", "")
-    cluster_configuration['report-project'] = config.get("report-project", "")
-    cluster_configuration['suite-url'] = config.get("suite-url", "")
-    return self._prepareSlapOS(self.testnode.config['slapos_directory'],
+    return self._prepareSlapOS(config['slapos_directory'],
               test_node_slapos, self.testnode.log, create_partition=0,
-              software_path_list=self.testnode.config.get("software_list"),
-              cluster_configuration=cluster_configuration
-              )
+              software_path_list=config.get("software_list"),
+              cluster_configuration={
+                'report-url': config.get("report-url", ""),
+                'report-project': config.get("report-project", ""),
+                'suite-url': config.get("suite-url", ""),
+              })
 
   def prepareSlapOSForTestSuite(self, node_test_suite):
     """
     Build softwares needed by testsuites
     """
-    log = self.testnode.log
-    if log is None:
-      log = self.testnode.log
     return self._prepareSlapOS(node_test_suite.working_directory,
-              node_test_suite, log,
+              node_test_suite, self.testnode.log,
               software_path_list=[node_test_suite.custom_profile_path],
               cluster_configuration={'_': json.dumps(node_test_suite.cluster_configuration)})
 
@@ -171,8 +160,7 @@ class UnitTestRunner():
     # From this point, test runner becomes responsible for updating test
     # result. We only do cleanup if the test runner itself is not able
     # to run.
-    SlapOSControler.createFolder(node_test_suite.test_suite_directory,
-                                 clean=True)
+    createFolder(node_test_suite.test_suite_directory, clean=True)
     self.testnode.process_manager.spawn(*invocation_list,
                           cwd=node_test_suite.test_suite_directory,
                           log_prefix='runTestSuite', get_output=False)

erp5/util/testnode/Updater.py

@@ -28,11 +28,8 @@ import errno
 import os
 import re
 import shutil
-import subprocess
 import sys
-import threading
-
-from ProcessManager import SubprocessError
+from .ProcessManager import SubprocessError
 
 SVN_UP_REV = re.compile(r'^(?:At|Updated to) revision (\d+).$')
 SVN_CHANGED_REV = re.compile(r'^Last Changed Rev.*:\s*(\d+)', re.MULTILINE)
@@ -44,9 +41,8 @@ SVN_TYPE = 'svn'
 class Updater(object):
 
   _git_cache = {}
-  stdin = file(os.devnull)
 
-  def __init__(self, repository_path, log, revision=None, git_binary=None,
+  def __init__(self, repository_path, log, revision=None, git_binary='git',
       branch=None, realtime_output=True, process_manager=None, url=None,
       working_directory=None):
     self.log = log
@@ -81,7 +77,7 @@ class Updater(object):
 
   def deletePycFiles(self, path):
     """Delete *.pyc files so that deleted/moved files can not be imported"""
-    for path, dir_list, file_list in os.walk(path):
+    for path, _, file_list in os.walk(path):
       for file in file_list:
         if file[-4:] in ('.pyc', '.pyo'):
           # allow several processes clean the same folder at the same time
@@ -115,17 +111,17 @@ class Updater(object):
     git_repository_path = os.path.join(self.getRepositoryPath(), '.git')
     name = os.path.basename(os.path.normpath(self.getRepositoryPath()))
     git_repository_link_path = os.path.join(self.getRepositoryPath(), '%s.git' %name)
-    self.log("checking link %s -> %s.."
-                %(git_repository_link_path,git_repository_path))
+    self.log("checking link %s -> %s..",
+             git_repository_link_path, git_repository_path)
     if ( not os.path.lexists(git_repository_link_path) and \
          not os.path.exists(git_repository_link_path) ):
       try:
         os.symlink(git_repository_path, git_repository_link_path)
-        self.log("link: %s -> %s created"
-                %(git_repository_link_path,git_repository_path))
-      except:
-        self.log("Cannot create link from %s -> %s"
-                %(git_repository_link_path,git_repository_path))
+        self.log("link: %s -> %s created",
+          git_repository_link_path, git_repository_path)
+      except OSError:
+        self.log("Cannot create link from %s -> %s",
+          git_repository_link_path, git_repository_path)
   
   def _git_find_rev(self, ref):
     try:
@@ -152,7 +148,7 @@ class Updater(object):
     raise NotImplementedError
 
   def deleteRepository(self):
-    self.log("Wrong repository or wrong url, deleting repos %s" % \
+    self.log("Wrong repository or wrong url, deleting repos %s",
              self.repository_path)
     shutil.rmtree(self.repository_path)
 
@@ -165,7 +161,7 @@ class Updater(object):
           remote_url = self._git("config", "--get", "remote.origin.url")
           if remote_url == self.url:
             correct_url = True
-        except (SubprocessError,) as e:
+        except SubprocessError:
           self.log("SubprocessError", exc_info=sys.exc_info())
         if not(correct_url):
           self.deleteRepository()

erp5/util/testnode/Utils.py

-import sys
-import json
+import os
 import shutil
-import string
-from random import choice
+
+def createFolder(folder, clean=False):
+  if os.path.exists(folder):
+    if not clean:
+      return
+    shutil.rmtree(folder)
+  os.mkdir(folder)
 
 def deunicodeData(data):
   if isinstance(data, list):
-    new_data = []
-    for sub_data in data:
-      new_data.append(deunicodeData(sub_data))
-  elif isinstance(data, unicode):
-    new_data = data.encode('utf8')
-  elif isinstance(data, dict):
-    new_data = {}
-    for key, value in data.iteritems():
-      key = deunicodeData(key)
-      value = deunicodeData(value)
-      new_data[key] = value
-  else:
-    new_data = data
-  return new_data
-
-def dealShebang(run_test_suite_path):
-  line = open(run_test_suite_path, 'r').readline()
-  invocation_list = []
-  if line[:2] == '#!':
-    invocation_list = line[2:].split()
-  return invocation_list
+    return map(deunicodeData, data)
+  if isinstance(data, unicode):
+    return data.encode('utf8')
+  if isinstance(data, dict):
+    return {deunicodeData(key): deunicodeData(value)
+            for key, value in data.iteritems()}
+  return data