Remove commented code from test_checker.py

Also run all remaining tests on all Python versions.

Trivial trailing/keyword arg whitespace cleanups.

6 files changed, 85 insertions, 99 deletions

diff --git a/src/zope/security/adapter.py b/src/zope/security/adapter.py
index c830cf4..f92ac34 100644
--- a/src/zope/security/adapter.py
+++ b/src/zope/security/adapter.py
@@ -65,9 +65,8 @@ class LocatingTrustedAdapterFactory(object):
         return assertLocation(adapter, context)
 
     def _customizeUnprotected(self, adapter, context):
-        if (ILocation.providedBy(adapter)
-            and adapter.__parent__ is None):
-                    adapter.__parent__ = context
+        if ILocation.providedBy(adapter) and adapter.__parent__ is None:
+            adapter.__parent__ = context
         return adapter
 
     def __call__(self, *args):
diff --git a/src/zope/security/interfaces.py b/src/zope/security/interfaces.py
index 9831b29..1035b77 100644
--- a/src/zope/security/interfaces.py
+++ b/src/zope/security/interfaces.py
@@ -119,7 +119,7 @@ class IChecker(Interface):
 
     def check_setattr(ob, name):
         """Check whether attribute assignment is allowed.
-        
+
         May raise Unauthorized or Forbidden.  Returns no value.
         """
 
@@ -236,7 +236,7 @@ class IInteractionManagement(Interface):
 
         Does nothing if there is no interaction.
         """
-        
+
 class IPrincipal(Interface):
     """Principals are security artifacts that execute actions in a security
     environment.
@@ -268,13 +268,13 @@ class IPrincipal(Interface):
         title=_("Description"),
         description=_("A detailed description of the principal."),
         required=False)
-        
-        
+
+
 class IGroupAwarePrincipal(IPrincipal):
     """Group aware principal interface
     Extends IPrincipal to contain group information.
     """
-    
+
     groups = Attribute(
         'An iterable of groups to which the principal directly belongs')
 
diff --git a/src/zope/security/metaconfigure.py b/src/zope/security/metaconfigure.py
index e2dd0a0..5f4884f 100644
--- a/src/zope/security/metaconfigure.py
+++ b/src/zope/security/metaconfigure.py
@@ -56,17 +56,17 @@ class ClassDirective(object):
     def implements(self, _context, interface):
         for interface in interface:
             _context.action(
-                discriminator = (
-                'ContentDirective', self.__class, object()),
-                callable = classImplements,
-                args = (self.__class, interface),
-                )
+                discriminator=(
+                    'ContentDirective', self.__class, object()),
+                callable=classImplements,
+                args=(self.__class, interface),
+            )
             _context.action(
-                discriminator = None,
-                callable = provideInterface,
-                args = (interface.__module__ + '.' + interface.getName(),
-                        interface)
-                )
+                discriminator=None,
+                callable=provideInterface,
+                args=(interface.__module__ + '.' + interface.getName(),
+                      interface)
+            )
 
     def require(self, _context,
                 permission=None, attributes=None, interface=None,
@@ -101,7 +101,7 @@ class ClassDirective(object):
             discriminator=('mimic', self.__class, object()),
             callable=protectLikeUnto,
             args=(self.__class, class_),
-            )
+        )
 
     def allow(self, _context, attributes=None, interface=None):
         """Like require, but with permission_id zope.Public"""
@@ -112,19 +112,19 @@ class ClassDirective(object):
         for n, d in sorted(interface.namesAndDescriptions(1)):
             self.__protectName(n, permission_id)
         self.__context.action(
-            discriminator = None,
-            callable = provideInterface,
-            args = (interface.__module__ + '.' + interface.getName(),
-                    interface)
-            )
+            discriminator=None,
+            callable=provideInterface,
+            args=(interface.__module__ + '.' + interface.getName(),
+                  interface)
+        )
 
     def __protectName(self, name, permission_id):
         "Set a permission on a particular name."
         self.__context.action(
-            discriminator = ('protectName', self.__class, name),
-            callable = protectName,
-            args = (self.__class, name, permission_id)
-            )
+            discriminator=('protectName', self.__class, name),
+            callable=protectName,
+            args=(self.__class, name, permission_id)
+        )
 
     def __protectNames(self, names, permission_id):
         "Set a permission on a bunch of names."
@@ -135,10 +135,10 @@ class ClassDirective(object):
         "Set a permission on a bunch of names."
         for name in names:
             self.__context.action(
-                discriminator = ('protectSetAttribute', self.__class, name),
-                callable = protectSetAttribute,
-                args = (self.__class, name, permission_id)
-                )
+                discriminator=('protectSetAttribute', self.__class, name),
+                callable=protectSetAttribute,
+                args=(self.__class, name, permission_id)
+            )
 
     def __protectSetSchema(self, schema, permission_id):
         "Set a permission on a bunch of names."
@@ -147,16 +147,16 @@ class ClassDirective(object):
             field = schema[name]
             if IField.providedBy(field) and not field.readonly:
                 _context.action(
-                    discriminator = ('protectSetAttribute', self.__class, name),
-                    callable = protectSetAttribute,
-                    args = (self.__class, name, permission_id)
-                    )
+                    discriminator=('protectSetAttribute', self.__class, name),
+                    callable=protectSetAttribute,
+                    args=(self.__class, name, permission_id)
+                )
         _context.action(
-            discriminator = None,
-            callable = provideInterface,
-            args = (schema.__module__ + '.' + schema.getName(),
-                    schema)
-            )
+            discriminator=None,
+            callable=provideInterface,
+            args=(schema.__module__ + '.' + schema.getName(),
+                  schema)
+        )
 
     def __call__(self):
         "Handle empty/simple declaration."
@@ -214,9 +214,9 @@ def allow(context, attributes=(), interface=()):
         context.action(
             discriminator=('http://namespaces.zope.org/zope:module',
                            context.module, name),
-            callable = protectModule,
-            args = (context.module, name, 'zope.Public'),
-            )
+            callable=protectModule,
+            args=(context.module, name, 'zope.Public'),
+        )
 
 
 def require(context, permission, attributes=(), interface=()):
@@ -224,6 +224,6 @@ def require(context, permission, attributes=(), interface=()):
         context.action(
             discriminator=('http://namespaces.zope.org/zope:module',
                            context.module, name),
-            callable = protectModule,
-            args = (context.module, name, permission),
-            )
+            callable=protectModule,
+            args=(context.module, name, permission),
+        )
diff --git a/src/zope/security/metadirectives.py b/src/zope/security/metadirectives.py
index 1a81658..bd1f793 100644
--- a/src/zope/security/metadirectives.py
+++ b/src/zope/security/metadirectives.py
@@ -170,8 +170,8 @@ class IAllow(Interface):
     interface = Tokens(
         title=u"Interface",
         description=(u"Interfaces whos names to provide access to. Access "
-                       u"will be provided to all of the names defined by the "
-                       u"interface(s). Multiple interfaces can be supplied."),
+                     u"will be provided to all of the names defined by the "
+                     u"interface(s). Multiple interfaces can be supplied."),
         value_type=GlobalInterface(),
         required=False)
 
diff --git a/src/zope/security/permission.py b/src/zope/security/permission.py
index 8044d4d..cfc1eee 100644
--- a/src/zope/security/permission.py
+++ b/src/zope/security/permission.py
@@ -47,9 +47,9 @@ def checkPermission(context, permission_id):
 def allPermissions(context=None):
     """Get the ids of all defined permissions
     """
-    for id, permission in getUtilitiesFor(IPermission, context):
-        if id != u'zope.Public':
-            yield id
+    for name, _permission in getUtilitiesFor(IPermission, context):
+        if name != u'zope.Public':
+            yield name
 
 def PermissionsVocabulary(context=None):
     """A vocabulary of permission IDs.
@@ -57,8 +57,8 @@ def PermissionsVocabulary(context=None):
     Term values are permissions, while term tokens are permission IDs.
     """
     terms = []
-    for id, permission in getUtilitiesFor(IPermission, context):
-        terms.append(SimpleTerm(permission, id))
+    for name, permission in getUtilitiesFor(IPermission, context):
+        terms.append(SimpleTerm(permission, name))
     return SimpleVocabulary(terms)
 
 directlyProvides(PermissionsVocabulary, IVocabularyFactory)
@@ -77,7 +77,7 @@ def PermissionIdsVocabulary(context=None):
     """
     terms = []
     has_public = False
-    for name, permission in getUtilitiesFor(IPermission, context):
+    for name, _permission in getUtilitiesFor(IPermission, context):
         if name == 'zope.Public':
             has_public = True
         else:
diff --git a/src/zope/security/tests/test_checker.py b/src/zope/security/tests/test_checker.py
index 8d38bc6..14f6002 100644
--- a/src/zope/security/tests/test_checker.py
+++ b/src/zope/security/tests/test_checker.py
@@ -17,10 +17,9 @@ import unittest
 
 from zope.security import checker as sec_checker
 from zope.security.tests import QuietWatchingChecker
+from zope.security._compat import PYTHON3 as PY3
+from zope.security._compat import PYTHON2 as PY2
 
-def _skip_if_not_Py2(testfunc):
-    import sys
-    return unittest.skipIf(sys.version_info[0] >= 3, "Needs Python 2")(testfunc)
 
 # pylint:disable=protected-access,inherit-non-class,no-method-argument,old-style-class
 # pylint:disable=blacklisted-name,no-init
@@ -629,15 +628,17 @@ class TestTracebackSupplement(unittest.TestCase):
                           '   - type: %s.C' % self.__class__.__module__,
                          ])
 
-    @_skip_if_not_Py2
     def test_getInfo_classic_instance(self):
         class C:
             pass
         tbs = self._makeOne(C())
-        self.assertEqual(tbs.getInfo().splitlines(),
-                         ['   - class: %s.C' % self.__class__.__module__,
-                          '   - type: __builtin__.instance',
-                         ])
+
+        lines = tbs.getInfo().splitlines()
+        self.assertEqual(lines[0],
+                         '   - class: %s.C' % self.__class__.__module__)
+        kind = '__builtin__.instance' if PY2 else '%s.C' % self.__class__.__module__
+        self.assertEqual(lines[1],
+                         '   - type: ' + kind)
 
 
 class TestGlobal(unittest.TestCase):
@@ -1081,7 +1082,6 @@ class Test_defineChecker(unittest.TestCase):
         self._callFUT(zope.interface, checker)
         self.assertIs(_checkers[zope.interface], checker)
 
-    @_skip_if_not_Py2
     def test_w_oldstyle_class(self):
         from zope.security.checker import _checkers
         checker = object()
@@ -1592,6 +1592,14 @@ class TestSecurityPolicy(QuietWatchingChecker,
         setSecurityPolicy(self.__oldpolicy)
         sec_checker._clear()
 
+    def _get_old_class_type(self):
+        # Py3 has no ClassType and no old-style classes
+        import types
+        old_type = getattr(types, 'ClassType', type)
+        self.assertTrue((PY2 and old_type is not type)
+                        or (PY3 and old_type is type))
+        return old_type
+
     def _makeSecurityPolicy(self):
         from zope.interface import implementer
         from zope.security.interfaces import ISecurityPolicy
@@ -1601,20 +1609,22 @@ class TestSecurityPolicy(QuietWatchingChecker,
                 return permission == 'test_allowed'
         return SecurityPolicy
 
-    @_skip_if_not_Py2
     def test_defineChecker_oldstyle_class(self):
-        import types
         from zope.security.checker import defineChecker
         from zope.security.checker import NamesChecker
+        old_type = self._get_old_class_type()
         class ClassicClass:
-            __metaclass__ = types.ClassType
+            __metaclass__ = old_type
+        self.assertIsInstance(ClassicClass, old_type)
+
         defineChecker(ClassicClass, NamesChecker())
 
     def test_defineChecker_newstyle_class(self):
         from zope.security.checker import defineChecker
         from zope.security.checker import NamesChecker
-        class NewStyleClass:
-            __metaclass__ = type
+        class NewStyleClass(object):
+            pass
+        self.assertIsInstance(NewStyleClass, type)
         defineChecker(NewStyleClass, NamesChecker())
 
     def test_defineChecker_module(self):
@@ -1631,9 +1641,8 @@ class TestSecurityPolicy(QuietWatchingChecker,
                           defineChecker, not_a_type, NamesChecker())
 
     def _makeClasses(self):
-        import types
+        old_type = self._get_old_class_type()
         class OldInst:
-            __metaclass__ = types.ClassType
             a = 1
             def b(self):
                 raise AssertionError("Never called")
@@ -1646,7 +1655,9 @@ class TestSecurityPolicy(QuietWatchingChecker,
             def __setitem__(self, x, v):
                 raise AssertionError("Never called")
 
-        class NewInst(object, OldInst):
+        self.assertIsInstance(OldInst, old_type)
+
+        class NewInst(OldInst, object):
             # This is not needed, but left in to show the change of metaclass
             # __metaclass__ = type
             def gete(self):
@@ -1655,6 +1666,7 @@ class TestSecurityPolicy(QuietWatchingChecker,
                 raise AssertionError("Never called")
             e = property(gete, sete)
 
+        self.assertIsInstance(NewInst, type)
         return OldInst, NewInst
 
     # check_getattr cases:
@@ -1662,7 +1674,6 @@ class TestSecurityPolicy(QuietWatchingChecker,
     # - no attribute there
     # - method
     # - allow and disallow by permission
-    @_skip_if_not_Py2
     def test_check_getattr(self):
         # pylint:disable=attribute-defined-outside-init
         from zope.security.interfaces import Forbidden
@@ -1714,7 +1725,6 @@ class TestSecurityPolicy(QuietWatchingChecker,
             self.assertRaises(Forbidden, checker.check_getattr, inst, 'e')
             self.assertRaises(Forbidden, checker.check_getattr, inst, 'f')
 
-    @_skip_if_not_Py2
     def test_check_setattr(self):
         # pylint:disable=attribute-defined-outside-init
         from zope.security.interfaces import Forbidden
@@ -1756,7 +1766,6 @@ class TestSecurityPolicy(QuietWatchingChecker,
             self.assertRaises(Forbidden, checker.check_setattr, inst, 'e')
             self.assertRaises(Forbidden, checker.check_setattr, inst, 'f')
 
-    @_skip_if_not_Py2
     def test_proxy(self):
         from zope.security.proxy import getChecker
         from zope.security.proxy import removeSecurityProxy
@@ -1787,29 +1796,8 @@ class TestSecurityPolicy(QuietWatchingChecker,
                     self.assertEqual(checker.permission_id('__str__'),
                                      CheckerPublic)
 
-            #No longer doing anything special for transparent proxies.
-            #A proxy needs to provide its own security checker.
-            #
-            #special = NamesChecker(['a', 'b'], 'test_allowed')
-            #defineChecker(class_, special)
-            #
-            #class TransparentProxy(object):
-            #    def __init__(self, ob):
-            #        self._ob = ob
-            #
-            #   def __getattribute__(self, name):
-            #       ob = object.__getattribute__(self, '_ob')
-            #       return getattr(ob, name)
-            #for ob in inst, TransparentProxy(inst):
-            #    proxy = checker.proxy(ob)
-            #    self.assertIs(removeSecurityProxy(proxy), ob)
-            #
-            #    checker = getChecker(proxy)
-            #    self.assertTrue(checker is special,
-            #                    checker.get_permissions)
-            #
-            #    proxy2 = checker.proxy(proxy)
-            #    self.assertIs(proxy2, proxy, [proxy, proxy2])
+            # No longer doing anything special for transparent proxies.
+            # A proxy needs to provide its own security checker.
 
     def test_iteration(self):
         from zope.security.checker import ProxyFactory
@@ -1910,7 +1898,6 @@ class TestSecurityPolicy(QuietWatchingChecker,
         self.assertEqual(checker.check(C, '__name__'), None)
         self.assertEqual(checker.check(C, '__parent__'), None)
 
-    @_skip_if_not_Py2
     def test_setattr(self):
         from zope.security.interfaces import Forbidden
         from zope.security.checker import NamesChecker