Run pylint on tests

Drop support for pylint < 1.0

Enable ignoring unknown attributes on modules (both nose and pytest
use advanced techniques, support for which only made it to pylint
recently)

Fix some bugs revealed by pylint

Do minor refactoring or add pylint:disable directives where the
linter complains.

Reviewed-By: Tomas Babej <tbabej@redhat.com>

freeipa.spec.in

@@ -67,7 +67,7 @@ BuildRequires:  python-netaddr
 BuildRequires:  python-kerberos >= 1.1-14
 BuildRequires:  python-rhsm
 BuildRequires:  pyOpenSSL
-BuildRequires:  pylint
+BuildRequires:  pylint >= 1.0
 BuildRequires:  python-polib
 BuildRequires:  libipa_hbac-python
 BuildRequires:  python-memcached

ipatests/i18n.py

@@ -678,7 +678,7 @@ def po_file_iterate(po_file, get_msgstr, get_msgstr_plural):
         return 1
 
     if not n_entries:
-        print >> sys.stderr, "ERROR: no translations found in %s" % (po_filename)
+        print >> sys.stderr, "ERROR: no translations found in %s" % (po_file)
         return 1
 
     if n_fail:

ipatests/test_integration/test_legacy_clients.py

@@ -17,6 +17,9 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+# FIXME: Pylint errors
+# pylint: disable=no-member
+
 import os
 import re
 
@@ -43,6 +46,10 @@ class BaseTestLegacyClient(object):
                     '/etc/nsswitch.conf',
                     paths.SSSD_CONF]
 
+    homedir_template = "/home/{domain}/{username}"
+    required_extra_roles = ()
+    optional_extra_roles = ()
+
     # Actual test classes need to override these attributes to set the expected
     # values on the UID and GID results, since this varies with the usage of the
     # POSIX and non-POSIX ID ranges
@@ -407,7 +414,6 @@ class BaseTestLegacyClientPosix(BaseTestLegacyClient,
     testuser_gid_regex = '10047'
     subdomain_testuser_uid_regex = '10142'
     subdomain_testuser_gid_regex = '10147'
-    homedir_template = "/home/{domain}/{username}"
     posix_trust = True
 
     def test_remove_trust_with_posix_attributes(self):
@@ -421,7 +427,6 @@ class BaseTestLegacyClientNonPosix(BaseTestLegacyClient,
     testuser_gid_regex = '(?!10047)(\d+)'
     subdomain_testuser_uid_regex = '(?!10142)(\d+)'
     subdomain_testuser_gid_regex = '(?!10147)(\d+)'
-    homedir_template = '/home/{domain}/{username}'
 
     def test_remove_nonposix_trust(self):
         pass

ipatests/test_integration/test_testconfig.py

@@ -129,6 +129,12 @@ class CheckConfig(object):
         assert_deepequal(self.get_output_dict(), conf.to_dict())
         self.check_config(conf)
 
+    # Settings to override:
+    extra_input_dict = {}
+    extra_input_env = {}
+    extra_output_dict = {}
+    extra_output_env = {}
+
 
 class TestEmptyConfig(CheckConfig):
     extra_input_dict = {}

ipatests/test_ipalib/test_backend.py

@@ -21,6 +21,10 @@
 Test the `ipalib.backend` module.
 """
 
+# FIXME: Pylint errors
+# pylint: disable=no-member
+# pylint: disable=maybe-no-member
+
 import threading
 from ipatests.util import ClassChecker, raises, create_test_api
 from ipatests.data import unicode_str

ipatests/test_ipalib/test_errors.py

@@ -21,6 +21,9 @@
 Test the `ipalib.errors` module.
 """
 
+# FIXME: Pylint errors
+# pylint: disable=no-member
+
 import re
 import inspect
 

ipatests/test_ipalib/test_frontend.py

@@ -21,8 +21,11 @@
 Test the `ipalib.frontend` module.
 """
 
-from ipatests.util import raises, getitem, no_set, no_del, read_only
-from ipatests.util import check_TypeError, ClassChecker, create_test_api
+# FIXME: Pylint errors
+# pylint: disable=no-member
+
+from ipatests.util import raises, read_only
+from ipatests.util import ClassChecker, create_test_api
 from ipatests.util import assert_equal
 from ipalib.constants import TYPE_ERROR
 from ipalib.base import NameSpace
@@ -444,7 +447,7 @@ class test_Command(ClassChecker):
         o = my_cmd()
         o.set_api(api)
         o.finalize()
-        e = o(**kw)
+        e = o(**kw)  # pylint: disable=not-callable
         assert type(e) is dict
         assert 'result' in e
         assert 'option2' in e['result']

ipatests/test_ipalib/test_parameters.py

@@ -22,6 +22,9 @@
 Test the `ipalib.parameters` module.
 """
 
+# FIXME: Pylint errors
+# pylint: disable=no-member
+
 import datetime
 import re
 import sys
@@ -219,12 +222,12 @@ class test_Param(ClassChecker):
 
         # Test that ValueError is raised when a kwarg from a subclass
         # conflicts with an attribute:
-        class Subclass(self.cls):
+        class Subclass1(self.cls):
             kwargs = self.cls.kwargs + (
                 ('convert', callable, None),
             )
-        e = raises(ValueError, Subclass, name)
-        assert str(e) == "kwarg 'convert' conflicts with attribute on Subclass"
+        e = raises(ValueError, Subclass1, name)
+        assert str(e) == "kwarg 'convert' conflicts with attribute on Subclass1"
 
         # Test type validation of keyword arguments:
         class Subclass(self.cls):
@@ -593,6 +596,8 @@ class test_Param(ClassChecker):
             type = unicode
 
             def __init__(self, name, **kw):
+                # (Pylint complains because the superclass is unknowm)
+                # pylint: disable=bad-super-call, super-on-old-class
                 self._convert_scalar = PassThrough()
                 super(Str, self).__init__(name, **kw)
 

ipatests/test_ipalib/test_plugable.py

@@ -21,6 +21,9 @@
 Test the `ipalib.plugable` module.
 """
 
+# FIXME: Pylint errors
+# pylint: disable=no-member
+
 import inspect
 from ipatests.util import raises, no_set, no_del, read_only
 from ipatests.util import getitem, setitem, delitem
@@ -343,7 +346,7 @@ def test_Registrar():
     orig1 = plugin1
     class base1_extended(Base1):
         pass
-    class plugin1(base1_extended):
+    class plugin1(base1_extended):  # pylint: disable=function-redefined
         pass
     e = raises(errors.PluginOverrideError, r, plugin1)
     assert e.base == 'Base1'

ipatests/test_ipapython/test_ipautil.py

@@ -128,6 +128,11 @@ class TestCIDict(object):
         nose.tools.assert_equal(3, len(self.cidict))
         self.cidict.clear()
         nose.tools.assert_equal(0, len(self.cidict))
+        assert self.cidict == {}
+        assert self.cidict.keys() == []
+        assert self.cidict.values() == []
+        assert self.cidict.items() == []
+        assert self.cidict._keys == {}
 
     def test_copy(self):
         copy = self.cidict.copy()
@@ -309,14 +314,6 @@ class TestCIDict(object):
         assert sorted(dct.keys()) == sorted(['A', 'b', 'C'])
         assert sorted(dct.values()) == [None] * 3
 
-    def test_clear(self):
-        self.cidict.clear()
-        assert self.cidict == {}
-        assert self.cidict.keys() == []
-        assert self.cidict.values() == []
-        assert self.cidict.items() == []
-        assert self.cidict._keys == {}
-
 
 class TestTimeParser(object):
     def test_simple(self):

ipatests/test_pkcs10/test_pkcs10.py

@@ -20,6 +20,9 @@
 Test the `pkcs10.py` module.
 """
 
+# FIXME: Pylint errors
+# pylint: disable=no-member
+
 import os
 import sys
 import nose

ipatests/test_xmlrpc/test_automount_plugin.py

@@ -45,6 +45,10 @@ class MockTextui(list):
 
 class AutomountTest(XMLRPC_test):
     """Provides common functionality for automount tests"""
+
+    locname = u'testlocation'
+    tofiles_output = ''  # To be overridden
+
     def check_tofiles(self):
         """Check automountlocation_tofiles output against self.tofiles_output
         """
@@ -111,7 +115,6 @@ class test_automount(AutomountTest):
     """
     Test the `automount` plugin.
     """
-    locname = u'testlocation'
     mapname = u'testmap'
     keyname = u'testkey'
     keyname_rename = u'testkey_rename'
@@ -320,7 +323,6 @@ class test_automount_direct(AutomountTest):
     """
     Test the `automount` plugin indirect map functionality.
     """
-    locname = u'testlocation'
     mapname = u'auto.direct2'
     keyname = u'/-'
     direct_kw = { 'key' : keyname }
@@ -386,7 +388,6 @@ class test_automount_indirect(AutomountTest):
     """
     Test the `automount` plugin indirect map functionality.
     """
-    locname = u'testlocation'
     mapname = u'auto.home'
     keyname = u'/home'
     parentmap = u'auto.master'
@@ -485,7 +486,6 @@ class test_automount_indirect_no_parent(AutomountTest):
     """
     Test the `automount` plugin Indirect map function.
     """
-    locname = u'testlocation'
     mapname = u'auto.home'
     keyname = u'/home'
     mapname2 = u'auto.direct2'

ipatests/test_xmlrpc/test_baseldap_plugin.py

@@ -153,7 +153,7 @@ def test_exc_callback_registration():
 
 
     @callbacktest_base.register_exc_callback
-    def exc_callback(self, keys, options, exc, call_func, *args, **kwargs):
+    def exc_callback_2(self, keys, options, exc, call_func, *args, **kwargs):
         """Callback on super class; doesn't affect the subclass"""
         messages.append('Superclass registered callback')
         raise exc

ipatests/test_xmlrpc/test_host_plugin.py

@@ -29,7 +29,7 @@ from ipalib import api, errors, x509
 from ipalib.util import normalize_zone
 from ipapython.dn import DN
 from ipapython.dnsutil import DNSName
-from nose.tools import raises, assert_raises
+from nose.tools import raises, assert_raises  # pylint: disable=E0611
 from nose.plugins.skip import SkipTest
 from ipatests.test_xmlrpc.xmlrpc_test import (Declarative, XMLRPC_test,
     fuzzy_uuid, fuzzy_digits, fuzzy_hash, fuzzy_date, fuzzy_issuer,

ipatests/util.py

@@ -430,7 +430,7 @@ class ClassChecker(object):
 
     def __get_cls(self):
         if self.__cls is None:
-            self.__cls = self._cls
+            self.__cls = self._cls  # pylint: disable=E1101
         assert inspect.isclass(self.__cls)
         return self.__cls
     cls = property(__get_cls)
@@ -455,19 +455,6 @@ class ClassChecker(object):
         context.__dict__.clear()
 
 
-def check_TypeError(value, type_, name, callback, *args, **kw):
-    """
-    Tests a standard TypeError raised with `errors.raise_TypeError`.
-    """
-    e = raises(TypeError, callback, *args, **kw)
-    assert e.value is value
-    assert e.type is type_
-    assert e.name == name
-    assert type(e.name) is str
-    assert str(e) == ipalib.errors.TYPE_ERROR % (name, type_, value)
-    return e
-
-
 def get_api(**kw):
     """
     Returns (api, home) tuple.
@@ -503,7 +490,7 @@ class PluginTester(object):
 
     def __get_plugin(self):
         if self.__plugin is None:
-            self.__plugin = self._plugin
+            self.__plugin = self._plugin  # pylint: disable=E1101
         assert issubclass(self.__plugin, Plugin)
         return self.__plugin
     plugin = property(__get_plugin)

make-lint

@@ -29,24 +29,16 @@ try:
     from pylint import checkers
     from pylint.lint import PyLinter
     from pylint.checkers.typecheck import TypeChecker
-    try:
-        # Pylint 1.0
-        from astroid import Class, Instance, InferenceError
-        from pylint.reporters.text import TextReporter
-        have_pylint_1_0 = True
-    except ImportError:
-        # Older Pylint
-        from logilab.astng import Class, Instance, InferenceError
-        from pylint.reporters.text import ParseableTextReporter
-        have_pylint_1_0 = False
+    from astroid import Class, Instance, Module, InferenceError
+    from pylint.reporters.text import TextReporter
 except ImportError:
     print >> sys.stderr, "To use {0}, please install pylint.".format(sys.argv[0])
     sys.exit(32)
 
 # File names to ignore when searching for python source files
 IGNORE_FILES = ('.*', '*~', '*.in', '*.pyc', '*.pyo')
-IGNORE_PATHS = ('build', 'rpmbuild', 'dist', 'install/po/test_i18n.py',
-    'lite-server.py', 'make-lint', 'make-test', 'ipatests')
+IGNORE_PATHS = (
+    'build', 'rpmbuild', 'dist', 'install/po/test_i18n.py', 'lite-server.py')
 
 class IPATypeChecker(TypeChecker):
     NAMESPACE_ATTRS = ['Command', 'Object', 'Method', 'Backend', 'Updater',
@@ -54,7 +46,7 @@ class IPATypeChecker(TypeChecker):
     LOGGING_ATTRS = ['log', 'debug', 'info', 'warning', 'error', 'exception',
         'critical']
 
-    # 'class': ['generated', 'properties']
+    # 'class or module': ['generated', 'properties']
     ignore = {
         # Python standard library & 3rd party classes
         'krbV.Principal': ['name'],
@@ -65,6 +57,9 @@ class IPATypeChecker(TypeChecker):
         'urlparse.ResultMixin': ['scheme', 'netloc', 'path', 'query',
             'fragment', 'username', 'password', 'hostname', 'port'],
         'urlparse.ParseResult': ['params'],
+        'pytest': ['fixture', 'raises', 'skip', 'yield_fixture', 'mark'],
+        'nose.tools': ['assert_equal', 'assert_raises'],
+        'datetime.tzinfo': ['houroffset', 'minoffset', 'utcoffset', 'dst'],
 
         # IPA classes
         'ipalib.base.NameSpace': ['add', 'mod', 'del', 'show', 'find'],
@@ -96,6 +91,8 @@ class IPATypeChecker(TypeChecker):
         'ipalib.session.SessionManager': LOGGING_ATTRS,
         'ipaserver.install.ldapupdate.LDAPUpdate': LOGGING_ATTRS,
         'ipaserver.rpcserver.KerberosSession': ['api'] + LOGGING_ATTRS,
+        'ipatests.test_integration.base.IntegrationTest': [
+            'domain', 'master', 'replicas', 'clients', 'ad_domains']
     }
 
     def _related_classes(self, klass):
@@ -121,14 +118,16 @@ class IPATypeChecker(TypeChecker):
             inferred = []
 
         for owner in inferred:
-            if not isinstance(owner, Class) and type(owner) is not Instance:
-                continue
-
-            ignored = self._find_ignored_attrs(owner)
-            for pattern in ignored:
-                if fnmatchcase(node.attrname, pattern):
+            if isinstance(owner, Module):
+                if node.attrname in self.ignore.get(owner.name, ()):
                     return
 
+            elif isinstance(owner, Class) or type(owner) is Instance:
+                ignored = self._find_ignored_attrs(owner)
+                for pattern in ignored:
+                    if fnmatchcase(node.attrname, pattern):
+                        return
+
         super(IPATypeChecker, self).visit_getattr(node)
 
 class IPALinter(PyLinter):
@@ -231,13 +230,9 @@ def main():
     if options.errors_only:
         linter.disable_noerror_messages()
         linter.enable('F')
-    if have_pylint_1_0:
-        linter.set_reporter(TextReporter())
-        linter.set_option('msg-template',
-                          '{path}:{line}: [{msg_id}({symbol}), {obj}] {msg})')
-    else:
-        linter.set_reporter(ParseableTextReporter())
-        linter.set_option('include-ids', True)
+    linter.set_reporter(TextReporter())
+    linter.set_option('msg-template',
+                        '{path}:{line}: [{msg_id}({symbol}), {obj}] {msg})')
     linter.set_option('reports', False)
     linter.set_option('persistent', False)