Merge branch 'testing-pylint'

Fixed several pylint warnings.

* testing-pylint:
  cleaning up pylint reports
  replaced test.sh with the Jenkins version
  renamed "_utils" to "Utils"
  fixed doctest
  cleaning up pylint reports
  fixed tests after refactoring
  cleaning up pylint reports
  some refactoring and grouped the checkout function in a module
  cleaning up pylint reports
  tuning conventions for pylint
  fixed some pylint reports
  added preliminary pylint configuration file

docs/pylint.rc

+[MASTER]
+
+# Specify a configuration file.
+#rcfile=
+
+# Python code to execute, usually for sys.path manipulation such as
+# pygtk.require().
+#init-hook=
+
+# Profiled execution.
+profile=no
+
+# Add files or directories to the blacklist. They should be base names, not
+# paths.
+ignore=CVS,tests
+
+# Pickle collected data for later comparisons.
+persistent=yes
+
+# List of plugins (as comma separated values of python modules names) to load,
+# usually to register additional checkers.
+load-plugins=
+
+
+[MESSAGES CONTROL]
+
+# Enable the message, report, category or checker with the given id(s). You can
+# either give multiple identifier separated by comma (,) or put this option
+# multiple time.
+#enable=
+
+# Disable the message, report, category or checker with the given id(s). You
+# can either give multiple identifier separated by comma (,) or put this option
+# multiple time (only on the command line, not in the configuration file where
+# it should appear only once).
+#disable=
+# I####: messages about enabled/disabled warnings
+# W0142 (star-args): Used * or ** magic
+disable=I,W0142
+
+
+[REPORTS]
+
+# Set the output format. Available formats are text, parseable, colorized, msvs
+# (visual studio) and html. You can also give a reporter class, eg
+# mypackage.mymodule.MyReporterClass.
+output-format=parseable
+
+# Include message's id in output
+include-ids=no
+
+# Include symbolic ids of messages in output
+symbols=no
+
+# Put messages in a separate file for each module / package specified on the
+# command line instead of printing them on stdout. Reports (if any) will be
+# written in a file name "pylint_global.[txt|html]".
+files-output=no
+
+# Tells whether to display a full report or only the messages
+reports=yes
+
+# Python expression which should return a note less than 10 (10 is the highest
+# note). You have access to the variables errors warning, statement which
+# respectively contain the number of errors / warnings messages and the total
+# number of statements analyzed. This is used by the global evaluation report
+# (RP0004).
+evaluation=10.0 - ((float(5 * error + warning + refactor + convention) / statement) * 10)
+
+# Add a comment according to your evaluation note. This is used by the global
+# evaluation report (RP0004).
+comment=no
+
+
+[FORMAT]
+
+# Maximum number of characters on a single line.
+max-line-length=80
+
+# Maximum number of lines in a module
+max-module-lines=1000
+
+# String used as indentation unit. This is usually " " (4 spaces) or "\t" (1
+# tab).
+indent-string='    '
+
+
+[VARIABLES]
+
+# Tells whether we should check for unused import in __init__ files.
+init-import=no
+
+# A regular expression matching the beginning of the name of dummy variables
+# (i.e. not used).
+dummy-variables-rgx=_|dummy
+
+# List of additional names supposed to be defined in builtins. Remember that
+# you should avoid to define new builtins when possible.
+additional-builtins=
+
+
+[TYPECHECK]
+
+# Tells whether missing members accessed in mixin class should be ignored. A
+# mixin class is detected if its name ends with "mixin" (case insensitive).
+ignore-mixin-members=yes
+
+# List of classes names for which member attributes should not be checked
+# (useful for classes with attributes dynamically set).
+ignored-classes=SQLObject
+
+# When zope mode is activated, add a predefined set of Zope acquired attributes
+# to generated-members.
+zope=no
+
+# List of members which are set dynamically and missed by pylint inference
+# system, and so shouldn't trigger E0201 when accessed. Python regular
+# expressions are accepted.
+generated-members=REQUEST,acl_users,aq_parent
+
+
+[MISCELLANEOUS]
+
+# List of note tags to take in consideration, separated by a comma.
+notes=FIXME,XXX,TODO
+
+
+[SIMILARITIES]
+
+# Minimum lines number of a similarity.
+min-similarity-lines=4
+
+# Ignore comments when computing similarities.
+ignore-comments=yes
+
+# Ignore docstrings when computing similarities.
+ignore-docstrings=yes
+
+# Ignore imports when computing similarities.
+ignore-imports=no
+
+
+[BASIC]
+
+# Required attributes for module, separated by a comma
+required-attributes=
+
+# List of builtins function names that should not be used, separated by a comma
+#bad-functions=map,filter,apply,input
+bad-functions=input
+
+# Regular expression which should only match correct module names
+module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$
+
+# Regular expression which should only match correct module level names
+const-rgx=(([A-Z_][A-Z0-9_]*)|(__.*__))$
+
+# Regular expression which should only match correct class names
+class-rgx=[A-Z_][a-zA-Z0-9]+$
+
+# Regular expression which should only match correct function names
+#function-rgx=[a-z_][a-z0-9_]{2,30}$
+function-rgx=(([a-z_][a-z0-9_]{2,30})|(_?[a-z][a-zA-Z0-9]{2,30}))$
+
+# Regular expression which should only match correct method names
+#method-rgx=[a-z_][a-z0-9_]{2,30}$
+method-rgx=(([a-z_][a-z0-9_]{2,30})|(_{0,2}[a-z][a-zA-Z0-9]{2,30})|(__.*__))$
+
+# Regular expression which should only match correct instance attribute names
+attr-rgx=[a-z_][a-z0-9_]{2,30}$
+
+# Regular expression which should only match correct argument names
+argument-rgx=[a-z_][a-z0-9_]{2,30}$
+
+# Regular expression which should only match correct variable names
+variable-rgx=[a-z_][a-z0-9_]{2,30}$
+
+# Regular expression which should only match correct list comprehension /
+# generator expression variable names
+inlinevar-rgx=[A-Za-z_][A-Za-z0-9_]*$
+
+# Good variable names which should always be accepted, separated by a comma
+#good-names=i,j,k,ex,Run,_
+good-names=i,j,k,f,m,s,ex,Run,_
+
+# Bad variable names which should always be refused, separated by a comma
+bad-names=foo,bar,baz,toto,tutu,tata
+
+# Regular expression which should only match functions or classes name which do
+# not require a docstring
+no-docstring-rgx=__.*__
+
+
+[CLASSES]
+
+# List of interface methods to ignore, separated by a comma. This is used for
+# instance to not check methods defines in Zope's Interface base class.
+ignore-iface-methods=isImplementedBy,deferred,extends,names,namesAndDescriptions,queryDescriptionFor,getBases,getDescriptionFor,getDoc,getName,getTaggedValue,getTaggedValueTags,isEqualOrExtendedBy,setTaggedValue,isImplementedByInstancesOf,adaptWith,is_implemented_by
+
+# List of method names used to declare (i.e. assign) instance attributes.
+defining-attr-methods=__init__,__new__,setUp
+
+# List of valid names for the first argument in a class method.
+valid-classmethod-first-arg=cls
+
+# List of valid names for the first argument in a metaclass class method.
+valid-metaclass-classmethod-first-arg=mcs
+
+
+[IMPORTS]
+
+# Deprecated modules which should not be used, separated by a comma
+#deprecated-modules=regsub,string,TERMIOS,Bastion,rexec
+deprecated-modules=regsub,TERMIOS,Bastion,rexec
+
+# Create a graph of every (i.e. internal and external) dependencies in the
+# given file (report RP0402 must not be disabled)
+import-graph=
+
+# Create a graph of external dependencies in the given file (report RP0402 must
+# not be disabled)
+ext-import-graph=
+
+# Create a graph of internal dependencies in the given file (report RP0402 must
+# not be disabled)
+int-import-graph=
+
+
+[DESIGN]
+
+# Maximum number of arguments for function / method
+max-args=5
+
+# Argument names that match this expression will be ignored. Default to name
+# with leading underscore
+ignored-argument-names=_.*
+
+# Maximum number of locals for function / method body
+max-locals=15
+
+# Maximum number of return / yield for function / method body
+max-returns=6
+
+# Maximum number of branch for function / method body
+max-branchs=12
+
+# Maximum number of statements in function / method body
+max-statements=50
+
+# Maximum number of parents for a class (see R0901).
+max-parents=7
+
+# Maximum number of attributes for a class (see R0902).
+max-attributes=7
+
+# Minimum number of public methods for a class (see R0903).
+min-public-methods=2
+
+# Maximum number of public methods for a class (see R0904).
+max-public-methods=20
+
+
+[EXCEPTIONS]
+
+# Exceptions that will emit a warning when being caught. Defaults to
+# "Exception"
+overgeneral-exceptions=Exception

test.sh → jenkins/test.sh

-#!/bin/sh
+#!/bin/bash
 ###############################################################################
 # (c) Copyright 2013 CERN                                                     #
 #                                                                             #
@@ -10,6 +10,16 @@
 # or submit itself to any jurisdiction.                                       #
 ###############################################################################
 
-cd $(dirname $0)
+# prepare the environment for testing
+. /cvmfs/lhcb.cern.ch/lib/lhcb/LBSCRIPTS/prod/InstallArea/scripts/LbLogin.sh
+. SetupProject.sh LCGCMT Python pytools
+set -ex
+
+cd $(dirname $0)/..
+. ./setup.sh
 
 nosetests -v --with-doctest --with-xunit --with-coverage --cover-erase --cover-inclusive --cover-package LbNightlyTools python
+coverage xml --include="python/*"
+
+# Ignoring pylint return code (to avoid failure of the test).
+pylint --rcfile=docs/pylint.rc LbNightlyTools > pylint.txt || true

python/LbNightlyTools/CheckoutMethods.py

+###############################################################################
+# (c) Copyright 2013 CERN                                                     #
+#                                                                             #
+# This software is distributed under the terms of the GNU General Public      #
+# Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING".   #
+#                                                                             #
+# In applying this licence, CERN does not waive the privileges and immunities #
+# granted to it by virtue of its status as an Intergovernmental Organization  #
+# or submit itself to any jurisdiction.                                       #
+###############################################################################
+'''
+Module grouping the common checkout functions.
+'''
+__author__ = 'Marco Clemencic <marco.clemencic@cern.ch>'
+
+import logging
+import shutil
+import os
+
+from LbNightlyTools.Utils import retry_call as call
+
+__log__ = logging.getLogger(__name__)
+
+def default(desc, rootdir='.'):
+    '''
+    Checkout the project described by the ProjectDesc 'desc'.
+    '''
+    from os.path import normpath, join
+    getpack = ['getpack', '--batch', '--no-config']
+    __log__.debug('checking out %s', desc)
+    cmd = getpack + ['-P',
+                     '-H' if desc.version == 'HEAD' else '-r',
+                     desc.name, desc.version]
+    call(cmd, cwd=rootdir, retry=3)
+
+    prjroot = normpath(join(rootdir, desc.projectDir))
+
+    overrides = desc.overrides
+    if overrides:
+        __log__.debug('overriding packages')
+        for package, version in overrides.items():
+            if version:
+                cmd = getpack + [package, version]
+                call(cmd, cwd=prjroot, retry=3)
+            else:
+                print 'Removing', package
+                shutil.rmtree(join(prjroot, package), ignore_errors=True)
+
+    __log__.debug('checkout of %s completed in %s', desc, prjroot)
+
+def ignore(desc, rootdir='.'): # pylint: disable=W0613
+    '''
+    Special checkout function used to just declare a project version in the
+    configuration but do not perform the checkout, so that it's picked up from
+    the release area.
+    '''
+    __log__.info('checkout not requested for %s', desc)
+
+def git(desc, rootdir='.'):
+    '''
+    Checkout from a git repository.
+
+    This function requires mandatory 'url' field in the 'checkout_opts' of the
+    project description.
+    '''
+    if 'url' not in desc.checkout_opts:
+        raise RuntimeError('mandatory checkout_opts "url" is missing')
+    url = desc.checkout_opts['url']
+    commit = desc.checkout_opts.get('commit', 'master')
+    __log__.debug('checking out %s from %s (%s)', desc, url, commit)
+    dest = os.path.join(rootdir, desc.projectDir)
+    call(['git', 'clone', url, dest])
+    call(['git', 'checkout', commit], cwd=dest)
+    f = open(os.path.join(dest, 'Makefile'), 'w')
+    f.write('include $(LBCONFIGURATIONROOT)/data/Makefile\n')
+    f.close()
+    __log__.debug('checkout of %s completed in %s', desc, dest)

python/LbNightlyTools/Configuration.py

@@ -27,6 +27,9 @@ def loadFromOldXML(source, slot):
     doc = parse(source)
 
     def fixPlaceHolders(s):
+        '''
+        Replace the old placeholders with the new ones.
+        '''
         s = s.replace('%DAY%', '${TODAY}')
         s = s.replace('%YESTERDAY%', '${YESTERDAY}')
         s = s.replace('%PLATFORM%', '${CMTCONFIG}')
@@ -35,84 +38,88 @@ def loadFromOldXML(source, slot):
     data = {'slot': slot,
             'env': []}
     try:
-        slotEl = (el for el in doc.findall('slot')
-                  if el.attrib.get('name') == slot).next()
+        slot_el = (el for el in doc.findall('slot')
+                   if el.attrib.get('name') == slot).next()
 
-        cmtProjPath = ':'.join([fixPlaceHolders(el.attrib['value'])
-                                for el in slotEl.findall('cmtprojectpath/path')])
-        if cmtProjPath:
-            data['env'].append('CMTPROJECTPATH=' + cmtProjPath)
+        cmt_proj_path = ':'.join([fixPlaceHolders(el.attrib['value'])
+                                  for el in
+                                      slot_el.findall('cmtprojectpath/path')])
+        if cmt_proj_path:
+            data['env'].append('CMTPROJECTPATH=' + cmt_proj_path)
 
-        d = slotEl.attrib.get('description', '(no description)')
-        m = re.match(r'%s(:| -|\.)\s+' % slot, d)
+        desc = slot_el.attrib.get('description', '(no description)')
+        m = re.match(r'%s(:| -|\.)\s+' % slot, desc)
         if m:
-            d = d[:m.start()] + d[m.end():]
-        data['description'] = d
+            desc = desc[:m.start()] + desc[m.end():]
+        data['description'] = desc
 
-        el = slotEl.find('cmtextratags')
-        if el is not None:
-            data['env'].append('CMTEXTRATAGS=' + el.attrib['value'])
+        elem = slot_el.find('cmtextratags')
+        if elem is not None:
+            data['env'].append('CMTEXTRATAGS=' + elem.attrib['value'])
 
-        el = slotEl.find('waitfor')
-        if el is not None:
-            path = fixPlaceHolders(el.attrib['flag'])
+        elem = slot_el.find('waitfor')
+        if elem is not None:
+            path = fixPlaceHolders(elem.attrib['flag'])
             data['preconditions'] = [{"name": "waitForFile",
                                       "args": {"path": path}}]
 
         data['default_platforms'] = [p.attrib['name']
-                                     for p in slotEl.findall('platforms/platform')
+                                     for p in
+                                         slot_el.findall('platforms/platform')
                                      if 'name' in p.attrib]
 
-        allProjs = []
-        for proj in slotEl.findall('projects/project'):
+        projects = []
+        for proj in slot_el.findall('projects/project'):
             name = proj.attrib['name']
             version = proj.attrib['tag'].split('_', 1)[1]
             overrides = {}
-            for el in proj.findall('addon') + proj.findall('change'):
-                overrides[el.attrib['package']] = el.attrib['value']
+            for elem in proj.findall('addon') + proj.findall('change'):
+                overrides[elem.attrib['package']] = elem.attrib['value']
             # since dependencies are declared only to override versions, but the
             # new config needs them for the ordering, we fake dependencies on
             # all the projects encountered so far
-            dependencies = [p['name'] for p in allProjs]
+            dependencies = [p['name'] for p in projects]
             # check if we have dep overrides
-            for el in proj.findall('dependence'):
-                depName = el.attrib['project']
-                if depName not in dependencies:
-                    dependencies.append(depName)
-                    depVer = el.attrib['tag']
-                    if depVer == 'LCGCMT-preview':
-                        depVer = 'preview'
+            for elem in proj.findall('dependence'):
+                dep_name = elem.attrib['project']
+                if dep_name not in dependencies:
+                    dependencies.append(dep_name)
+                    dep_vers = elem.attrib['tag']
+                    if dep_vers == 'LCGCMT-preview':
+                        dep_vers = 'preview'
                     else:
-                        depVer = depVer.split('_', 1)[1]
-                    allProjs.append({'name': depName,
-                                     'version': depVer,
+                        dep_vers = dep_vers.split('_', 1)[1]
+                    projects.append({'name': dep_name,
+                                     'version': dep_vers,
                                      'overrides': {},
                                      'dependencies': [],
-                                     'checkout': 'noCheckout'})
+                                     'checkout': 'ignore'})
 
-            projData = {'name': name,
-                        'version': version,
-                        'overrides': overrides,
-                        'dependencies': dependencies}
+            proj_data = {'name': name,
+                         'version': version,
+                         'overrides': overrides,
+                         'dependencies': dependencies}
             if proj.attrib.get('disabled', 'false').lower() != 'false':
-                projData['checkout'] = 'noCheckout'
+                proj_data['checkout'] = 'ignore'
 
-            allProjs.append(projData)
+            projects.append(proj_data)
 
-        data['projects'] = allProjs
+        data['projects'] = projects
 
         # we assume that all slots from old config use CMT
         data['USE_CMT'] = True
 
-        def el2re(el):
+        def el2re(elem):
             '''Regex string for ignored warning or error.'''
-            v = el.attrib['value']
-            if el.attrib.get('type', 'string') == 'regex':
-                return v
+            val = elem.attrib['value']
+            if elem.attrib.get('type', 'string') == 'regex':
+                return val
             else:
-                return re.escape(v)
-        data['error_exceptions'] = map(el2re, doc.findall('general/ignore/error'))
-        data['warning_exceptions'] = map(el2re, doc.findall('general/ignore/warning'))
+                return re.escape(val)
+        data['error_exceptions'] = map(el2re,
+                                       doc.findall('general/ignore/error'))
+        data['warning_exceptions'] = map(el2re,
+                                         doc.findall('general/ignore/warning'))
 
         return data
     except StopIteration:

python/LbNightlyTools/InstallSlot.py

@@ -28,6 +28,7 @@ from subprocess import Popen, PIPE, call
 from tempfile import mkstemp
 from datetime import datetime
 
+ARTIFACTS_URL = 'https://buildlhcb.cern.ch/artifacts'
 
 def _list_http(url):
     '''
@@ -37,13 +38,19 @@ def _list_http(url):
     format.
     '''
     class ListHTMLParser(HTMLParser.HTMLParser):
+        '''
+        Specialized HTML parser to extract the list of files from standard
+        Apache directory listing.
+        '''
+        # pylint: disable=R0904
         def __init__(self):
             HTMLParser.HTMLParser.__init__(self)
             self.data = []
         def handle_starttag(self, tag, attrs):
             if tag == 'a':
                 attrs = dict(attrs)
-                href = attrs.get('href', '?') # avoid check for '' in the following 'if'
+                # avoid check for '' in the following 'if'
+                href = attrs.get('href', '?')
                 # ignore special entries like sorting links ("?...") or link to
                 # parent directory
                 if '?' not in href and href not in url:
@@ -57,8 +64,8 @@ def _list_ssh(url):
     Implementation of listdir for SSH.
     '''
     host, path = url.split(':', 1)
-    p = Popen(['ssh', host, 'ls -a1 %r' % path], stdout=PIPE)
-    return p.communicate()[0].splitlines()
+    proc = Popen(['ssh', host, 'ls -a1 %r' % path], stdout=PIPE)
+    return proc.communicate()[0].splitlines()
 
 def _url_protocol(url):
     '''
@@ -73,19 +80,24 @@ def _url_protocol(url):
 
 def listdir(url):
     '''
-    @return the list of entries in a directory, being it over HTTP, ssh or local filesystem.
+    @return the list of entries in a directory, being it over HTTP, ssh or
+            local filesystem.
     '''
     protocol = _url_protocol(url)
-    l = {'http': _list_http,
-         'ssh': _list_ssh,
-         'file': os.listdir}[protocol](url)
-    l.sort()
-    return l
+    listing = {'http': _list_http,
+               'ssh': _list_ssh,
+               'file': os.listdir}[protocol](url)
+    return sorted(listing)
 
 
 def getURL(url, dst):
+    '''
+    Generic URL retriever with suppotr for 'http:', 'file:' and 'ssh:'
+    protocols.
+    '''
     protocol = _url_protocol(url)
     def getHTTP(url, dst):
+        '''Retrieve from 'http:'.'''
         # code copied from shutil.copyfile
         fsrc = None
         fdst = None
@@ -99,12 +111,16 @@ def getURL(url, dst):
             if fsrc:
                 fsrc.close()
     def getSSH(url, dst):
+        '''Retrieve from 'ssh:'.'''
         call(['scp', '-q', url, dst])
     return {'http': getHTTP,
             'ssh': getSSH,
             'file': shutil.copy2}[protocol](url, dst)
 
 def unpack(url, dest):
+    '''
+    Unpack a tarball from 'url' into the directory 'dest'.
+    '''
     # download on a local file
     log = logging.getLogger('unpack')
     protocol = _url_protocol(url)
@@ -127,7 +143,7 @@ def unpack(url, dest):
 
 def install(url, dest):
     '''
-    Install the file at 'url' in the directory dest.
+    Install the file at 'url' in the directory 'dest'.
 
     If url points to a tarball, it is unpacked, otherwise it is just copied.
     '''
@@ -152,16 +168,17 @@ def requiredPackages(files, projects=None, platforms=None, skip=None):
     else:
         skip = set(skip)
     if projects:
-        projects = map(str.lower, projects) # change to lowercase to make the check case-insensitive
-    for f in files:
+        # change to lowercase to make the check case-insensitive
+        projects = map(str.lower, projects)
+    for filename in files:
         # file names have the format
         #   <project>.<version>.<tag.id>.<platform>.tar.bz2
-        d = f.split('.')
-        pr,</