plone.api/fix_tests: enhance readability

Alexander Loechel <jenkins-z4DKO/[email protected]> Mon, 31 Jul 2017 13:39:34 -0700 (PDT)
Newsgroups gmane.comp.web.zope.plone.cvs
Message-ID <[email protected]>
Repository: plone.api
Branch: refs/heads/fix_tests
Date: 2017-07-31T22:39:20+02:00
Author: Alexander Loechel (loechel) <[email protected]>
Commit: https://github.com/plone/plone.api/commit/e557addff80f826a44b887baf6c9fd4a41eeb048

enhance readability

Files changed:
M src/plone/api/content.py
M src/plone/api/portal.py
M src/plone/api/tests/test_doctests.py
M src/plone/api/tests/test_env.py
M src/plone/api/tests/test_portal.py
M src/plone/api/tests/test_user.py
M src/plone/api/tests/test_validation.py

diff --git a/src/plone/api/content.py b/src/plone/api/content.py
index 9ce9373..7bfb219 100644
--- a/src/plone/api/content.py
+++ b/src/plone/api/content.py
@@ -50,7 +50,7 @@ def create(
     id=None,
     title=None,
     safe_id=False,
-    **kwargs
+    **kwargs  # NOQA: C816
 ):
     """Create a new content item.
 
diff --git a/src/plone/api/portal.py b/src/plone/api/portal.py
index 85a515b..5dae7b4 100644
--- a/src/plone/api/portal.py
+++ b/src/plone/api/portal.py
@@ -260,7 +260,7 @@ def show_message(message=None, request=None, type='info'):
     :param message: [required] Message to show.
     :type message: string
     :param request: [required] Request.
-    :type request: TODO: hm?
+    :type request: HTTPRequest
     :param type: Message type. Possible values: 'info', 'warn', 'error'
     :type type: string
     :raises:
@@ -308,8 +308,8 @@ def get_registry_record(name=None, interface=None, default=MISSING):
         # Show all records on the interface.
         records = [key for key in interface.names()]
         msg = (
-            "Cannot find a record with name '{name}'"
-            " on interface {identifier}.\n"
+            'Cannot find a record with name "{name}"'
+            ' on interface {identifier}.\n'
             'Did you mean?\n'
             '{records}'.format(
                 name=name,
@@ -370,10 +370,7 @@ def set_registry_record(name=None, value=None, interface=None):
 
         from zope.schema._bootstrapinterfaces import WrongType
         try:
-            registry['{identifier}.{name}'.format(
-                identifier=interface.__identifier__,
-                name=name
-            )] = value
+            registry[interface.__identifier__ + '.' + name] = value
         except WrongType:
             field_type = [
                 field[1]
diff --git a/src/plone/api/tests/test_doctests.py b/src/plone/api/tests/test_doctests.py
index 6470065..a3ba884 100644
--- a/src/plone/api/tests/test_doctests.py
+++ b/src/plone/api/tests/test_doctests.py
@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 """Boilerplate for doctest functional tests."""
+
 from logging import getLogger
 from plone.app.testing import applyProfile
 from plone.app.testing import PLONE_INTEGRATION_TESTING
@@ -117,6 +118,9 @@ def test_suite():
         try:
             doctests.append(DocFileSuite(os.path.join(path, filename)))
         except IOError:
-            logger.warning('test_doctest.py skipping {0}'.format(filename))
+            logger.warning('test_doctest.py skipping {file}'.format(
+                    file=filename,
+                )
+            )
 
     return unittest.TestSuite(doctests)
diff --git a/src/plone/api/tests/test_env.py b/src/plone/api/tests/test_env.py
index 44bb353..bf64e7f 100644
--- a/src/plone/api/tests/test_env.py
+++ b/src/plone/api/tests/test_env.py
@@ -143,6 +143,20 @@ def test_adopt_manager_role(self):
             ])
         self.test_test_defaults()
 
+    def test_adopt_manager_string_role(self):
+        """Test that we can adopt the Manager role temporarily."""
+        with api.env.adopt_roles(roles='Manager'):
+            self.should_allow([
+                'public_method',
+                'pp_method',
+                'qq_method',
+                'rr_method',
+            ])
+            self.should_forbid([
+                'private_method',
+            ])
+        self.test_test_defaults()
+
     def test_adopt_fewers_role(self):
         """Test that we can adopt a non-Manager role temporarily."""
         with api.env.adopt_roles(roles=['Member']):
diff --git a/src/plone/api/tests/test_portal.py b/src/plone/api/tests/test_portal.py
index 141b798..b4ad86f 100644
--- a/src/plone/api/tests/test_portal.py
+++ b/src/plone/api/tests/test_portal.py
@@ -77,7 +77,10 @@ def setUp(self):
                 '[email protected]',
             )
         else:
-            self.portal._updateProperty('email_from_name', 'Portal Owner')
+            self.portal._updateProperty(
+                'email_from_name',
+                'Portal Owner',
+            )
             self.portal._updateProperty(
                 'email_from_address',
                 '[email protected]',
diff --git a/src/plone/api/tests/test_user.py b/src/plone/api/tests/test_user.py
index 68f3796..0f49ef2 100644
--- a/src/plone/api/tests/test_user.py
+++ b/src/plone/api/tests/test_user.py
@@ -136,9 +136,9 @@ def test_create_default_roles(self):
             email='chuck-69hlKehBS/[email protected]',
             password='secret',
         )
-        self.assertEqual(
-            sorted(api.user.get_roles(user=user)),
-            sorted(['Member', 'Authenticated', ]),
+        self.assertItemsEqual(
+            api.user.get_roles(user=user),
+            ['Member', 'Authenticated', ],
         )
 
     def test_create_specified_roles(self):
@@ -187,7 +187,10 @@ def test_get(self):
 
     def test_get_current(self):
         """Test getting the currently logged-in user."""
-        self.assertEqual(api.user.get_current().getUserName(), TEST_USER_NAME)
+        self.assertEqual(
+            api.user.get_current().getUserName(),
+            TEST_USER_NAME,
+        )
 
     def test_get_all_users(self):
         """Test getting all users."""
@@ -198,7 +201,10 @@ def test_get_all_users(self):
         )
         users = [user.getUserName() for user in api.user.get_users()]
 
-        self.assertEqual(users, ['chuck', TEST_USER_NAME])
+        self.assertItemsEqual(
+            users,
+            ['chuck', TEST_USER_NAME],
+        )
 
     def test_get_groups_users(self):
         """Test getting all users of a certain group."""
@@ -283,15 +289,18 @@ def test_is_anonymous(self):
 
     def test_get_roles_username(self):
         """Test get roles passing a username."""
-        ROLES = ['Reviewer', 'Editor']
+        ROLES = ['Reviewer', 'Editor', ]
         api.user.create(
             username='chuck',
             email='chuck-69hlKehBS/[email protected]',
             password='secret',
             roles=ROLES,
         )
-        ROLES = set(ROLES + ['Authenticated'])
-        self.assertEqual(ROLES, set(api.user.get_roles(username='chuck')))
+
+        self.assertItemsEqual(
+            ROLES + ['Authenticated'],
+            api.user.get_roles(username='chuck'),
+        )
 
     def test_get_roles_user(self):
         """Test get roles passing a user."""
@@ -302,8 +311,11 @@ def test_get_roles_user(self):
             password='secret',
             roles=ROLES,
         )
-        ROLES = set(ROLES + ['Authenticated'])
-        self.assertEqual(ROLES, set(api.user.get_roles(user=user)))
+
+        self.assertItemsEqual(
+            ROLES + ['Authenticated'],
+            api.user.get_roles(user=user),
+        )
 
     def test_get_roles_username_and_user(self):
         """Test get roles passing username and user."""
@@ -324,14 +336,16 @@ def test_get_roles_username_and_user(self):
 
     def test_get_roles_no_parameters(self):
         """Test get roles without any parameters."""
-        ROLES = set(['Manager', 'Authenticated'])
-        self.assertEqual(ROLES, set(api.user.get_roles()))
+        self.assertItemsEqual(
+            ['Manager', 'Authenticated'],
+            api.user.get_roles(),
+        )
 
     def test_get_permissions_no_parameters(self):
         """Test get_permissions passing no parameters."""
-        self.assertEqual(  # TODO: maybe assertItemsEqual?
-            set(p[0] for p in getPermissions()),
-            set(api.user.get_permissions().keys()),
+        self.assertItemsEqual(
+            [p[0] for p in getPermissions()],
+            api.user.get_permissions().keys(),
         )
 
     def test_get_roles_nonexistant_user(self):
@@ -370,9 +384,14 @@ def test_get_roles_in_context(self):
             id='document_one',
             title='Document One',
         )
-        api.user.grant_roles(username='chuck', roles=['Editor'], obj=folder)
+        api.user.grant_roles(
+            username='chuck',
+            roles=['Editor'],
+            obj=folder,
+        )
         self.assertIn(
-            'Editor', api.user.get_roles(username='chuck', obj=document),
+            'Editor',
+            api.user.get_roles(username='chuck', obj=document),
         )
 
     def test_get_roles_local_only(self):
@@ -396,7 +415,11 @@ def test_get_roles_local_only(self):
             id='document_one',
             title='Document One',
         )
-        api.user.grant_roles(username='chuck', roles=['Editor'], obj=folder)
+        api.user.grant_roles(
+            username='chuck',
+            roles=['Editor'],
+            obj=folder,
+        )
         self.assertNotIn(
             'Editor',
             api.user.get_roles(username='chuck', obj=document, inherit=False),
@@ -429,7 +452,11 @@ def test_get_roles_local_includes_group_roles(self):
             api.user.get_roles(username='chuck', obj=document),
             ['Member', 'Reviewer', 'Authenticated'],
         )
-        api.user.grant_roles(username='chuck', roles=['Editor'], obj=folder)
+        api.user.grant_roles(
+            username='chuck',
+            roles=['Editor'],
+            obj=folder,
+        )
         self.assertItemsEqual(
             api.user.get_roles(username='chuck', obj=document),
             ['Member', 'Reviewer', 'Authenticated', 'Editor'],
@@ -443,7 +470,9 @@ def test_get_roles_local_includes_group_roles(self):
             ['Editor'],
         )
         api.group.grant_roles(
-            groupname='foo', roles=['Contributor'], obj=document,
+            groupname='foo',
+            roles=['Contributor'],
+            obj=document,
         )
         self.assertItemsEqual(
             ['Contributor'],
@@ -598,11 +627,21 @@ def test_grant_roles(self):
         self.assertIn('Contributor', api.user.get_roles(user=user))
 
         api.user.grant_roles(username='chuck', roles=['Reader', 'Reader'])
-        ROLES = set(
-            ('Editor', 'Contributor', 'Reader', 'Authenticated', 'Member'),
+        ROLES = [
+            'Editor',
+            'Contributor',
+            'Reader',
+            'Authenticated',
+            'Member',
+        ]
+        self.assertItemsEqual(
+            ROLES,
+            api.user.get_roles(username='chuck'),
+        )
+        self.assertItemsEqual(
+            ROLES,
+            api.user.get_roles(user=user),
         )
-        self.assertEqual(ROLES, set(api.user.get_roles(username='chuck')))
-        self.assertEqual(ROLES, set(api.user.get_roles(user=user)))
 
     def test_grant_roles_username_and_user(self):
         """Test grant roles passing username and user."""
@@ -660,24 +699,30 @@ def test_revoke_roles(self):
             password='secret',
         )
 
-        api.user.grant_roles(username='chuck', roles=['Reviewer', 'Editor'])
-        api.user.revoke_roles(username='chuck', roles=['Reviewer'])
+        api.user.grant_roles(username='chuck', roles=['Reviewer', 'Editor', ])
+        api.user.revoke_roles(username='chuck', roles=['Reviewer', ])
         self.assertNotIn('Reviewer', api.user.get_roles(username='chuck'))
         self.assertNotIn('Reviewer', api.user.get_roles(user=user))
         self.assertIn('Editor', api.user.get_roles(username='chuck'))
         self.assertIn('Editor', api.user.get_roles(user=user))
 
         api.user.revoke_roles(username='chuck', roles=('Editor',))
-        ROLES = set(('Authenticated', 'Member'))
-        self.assertEqual(ROLES, set(api.user.get_roles(username='chuck')))
-        self.assertEqual(ROLES, set(api.user.get_roles(user=user)))
-        self.assertEqual(
+        ROLES = ['Authenticated', 'Member', ]
+        self.assertItemsEqual(
             ROLES,
-            set(api.user.get_roles(username='chuck', inherit=False)),
+            api.user.get_roles(username='chuck'),
         )
-        self.assertEqual(
+        self.assertItemsEqual(
+            ROLES,
+            api.user.get_roles(user=user),
+        )
+        self.assertItemsEqual(
             ROLES,
-            set(api.user.get_roles(user=user, inherit=False)),
+            api.user.get_roles(username='chuck', inherit=False),
+        )
+        self.assertItemsEqual(
+            ROLES,
+            api.user.get_roles(user=user, inherit=False),
         )
 
     def test_revoke_roles_username_and_user(self):
@@ -723,11 +768,14 @@ def test_revoke_roles_no_parameters(self):
     @unittest.skip('Getting the Anonymous user does not work like this.')
     def test_revoke_roles_from_anonymous(self):
         """Test revoking roles from an Anonymous user."""
-        api.user.revoke_roles(username='Anonymous User', roles=['Reviewer'])
-        ROLES = set(['Anonymous', ])
-        self.assertEqual(
+        api.user.revoke_roles(
+            username='Anonymous User',
+            roles=['Reviewer'],
+        )
+        ROLES = ['Anonymous', ]
+        self.assertItemsEqual(
             ROLES,
-            set(api.user.get_roles(username='Anonymous User')),
+            api.user.get_roles(username='Anonymous User'),
         )
 
     def test_revoke_roles_no_user(self):
@@ -762,7 +810,11 @@ def test_grant_roles_in_context(self):
             title='Document One',
         )
 
-        api.user.grant_roles(username='chuck', roles=['Editor'], obj=folder)
+        api.user.grant_roles(
+            username='chuck',
+            roles=['Editor'],
+            obj=folder,
+        )
         self.assertIn(
             'Editor',
             api.user.get_roles(username='chuck', obj=folder),
@@ -779,7 +831,10 @@ def test_grant_roles_in_context(self):
             'Editor',
             api.user.get_roles(username='chuck', obj=document),
         )
-        self.assertIn('Editor', api.user.get_roles(user=user, obj=document))
+        self.assertIn(
+            'Editor',
+            api.user.get_roles(user=user, obj=document),
+        )
 
         api.user.grant_roles(
             username='chuck',
@@ -803,22 +858,22 @@ def test_grant_roles_in_context(self):
             api.user.get_roles(user=user, obj=document),
         )
 
-        ROLES = set(('Editor', 'Contributor', 'Authenticated', 'Member'))
+        ROLES = ['Editor', 'Contributor', 'Authenticated', 'Member', ]
         self.assertItemsEqual(
             ROLES,
-            set(api.user.get_roles(username='chuck', obj=folder)),
+            api.user.get_roles(username='chuck', obj=folder),
         )
         self.assertItemsEqual(
             ROLES,
-            set(api.user.get_roles(user=user, obj=folder)),
+            api.user.get_roles(user=user, obj=folder),
         )
         self.assertItemsEqual(
             ROLES,
-            set(api.user.get_roles(username='chuck', obj=document)),
+            api.user.get_roles(username='chuck', obj=document),
         )
-        self.assertEqual(
+        self.assertItemsEqual(
             ROLES,
-            set(api.user.get_roles(user=user, obj=document)),
+            api.user.get_roles(user=user, obj=document),
         )
 
     def test_grant_roles_disregards_adapter(self):
@@ -878,7 +933,9 @@ def getRoles(self, principal_id):
 
         # Assign a local role
         api.user.grant_roles(
-            username='chuck', roles=['Contributor'], obj=folder,
+            username='chuck',
+            roles=['Contributor'],
+            obj=folder,
         )
         self.assertItemsEqual(
             api.user.get_roles(username='chuck', obj=folder),
@@ -888,7 +945,7 @@ def getRoles(self, principal_id):
         # The adapter role is in in the local roles but not persistent
         self.assertItemsEqual(
             api.user.get_roles(username='chuck', obj=folder, inherit=False),
-            ['Contributor', 'Reviewer'],
+            ['Contributor', 'Reviewer', ],
         )
         local_roles = getattr(folder, '__ac_local_roles__', {})
         self.assertEqual(
@@ -902,34 +959,50 @@ def getRoles(self, principal_id):
         )
         self.assertItemsEqual(
             api.user.get_roles(username='chuck', obj=document),
-            ['Member', 'Authenticated', 'Contributor', 'Reviewer'],
+            ['Member', 'Authenticated', 'Contributor', 'Reviewer', ],
         )
 
         # add a group and test mix of group and adapter and user-roles
         api.group.create('foo')
-        api.group.grant_roles(groupname='foo', roles=['Contributor'], obj=document)  # noqa
+        api.group.grant_roles(
+            groupname='foo',
+            roles=['Contributor'],
+            obj=document,
+        )
         api.group.add_user(groupname='foo', username='chuck')
 
         self.assertItemsEqual(
             api.user.get_roles(username='chuck', obj=document, inherit=False),
-            ['Contributor', 'Reviewer'],
+            ['Contributor', 'Reviewer', ],
         )
         api.group.grant_roles(groupname='foo', roles=['Manager'], obj=folder)
         self.assertItemsEqual(
             api.user.get_roles(username='chuck', obj=document, inherit=False),
-            ['Contributor', 'Reviewer'],
+            ['Contributor', 'Reviewer', ],
         )
         self.assertItemsEqual(
             api.user.get_roles(username='chuck', obj=document),
-            ['Contributor', 'Reviewer', 'Manager', 'Authenticated', 'Member'],
+            [
+                'Contributor',
+                'Reviewer',
+                'Manager',
+                'Authenticated',
+                'Member',
+            ],
         )
         self.assertItemsEqual(
             api.user.get_roles(username='chuck', obj=folder),
-            ['Contributor', 'Reviewer', 'Manager', 'Authenticated', 'Member'],
+            [
+                'Contributor',
+                'Reviewer',
+                'Manager',
+                'Authenticated',
+                'Member',
+            ],
         )
         self.assertItemsEqual(
             api.user.get_roles(username='chuck', obj=folder, inherit=False),
-            ['Contributor', 'Reviewer', 'Manager'],
+            ['Contributor', 'Reviewer', 'Manager', ],
         )
 
         # cleanup
@@ -1003,33 +1076,36 @@ def test_revoke_roles_in_context(self):
         )
         self.assertNotIn('Editor', api.user.get_roles(user=user, obj=document))
 
-        ROLES = set(('Authenticated', 'Member'))
-        self.assertEqual(
+        ROLES = ['Authenticated', 'Member', ]
+        self.assertItemsEqual(
             ROLES,
-            set(api.user.get_roles(username='chuck', obj=folder)),
+            api.user.get_roles(username='chuck', obj=folder),
         )
-        self.assertEqual(
-            ROLES, set(api.user.get_roles(user=user, obj=folder)),
+        self.assertItemsEqual(
+            ROLES,
+            api.user.get_roles(user=user, obj=folder),
         )
-        self.assertEqual(
-            ROLES, set(api.user.get_roles(username='chuck', obj=document)),
+        self.assertItemsEqual(
+            ROLES,
+            api.user.get_roles(username='chuck', obj=document),
         )
-        self.assertEqual(
-            ROLES, set(api.user.get_roles(user=user, obj=document)),
+        self.assertItemsEqual(
+            ROLES,
+            api.user.get_roles(user=user, obj=document),
         )
-        self.assertEqual(
+        self.assertItemsEqual(
             [],
             api.user.get_roles(username='chuck', obj=folder, inherit=False),
         )
-        self.assertEqual(
+        self.assertItemsEqual(
             [],
             api.user.get_roles(user=user, obj=folder, inherit=False),
         )
-        self.assertEqual(
+        self.assertItemsEqual(
             [],
             api.user.get_roles(username='chuck', obj=document, inherit=False),
         )
-        self.assertEqual(
+        self.assertItemsEqual(
             [],
             api.user.get_roles(user=user, obj=document, inherit=False),
         )
diff --git a/src/plone/api/tests/test_validation.py b/src/plone/api/tests/test_validation.py
index 14f0ede..f3e19bb 100644
--- a/src/plone/api/tests/test_validation.py
+++ b/src/plone/api/tests/test_validation.py
@@ -62,43 +62,80 @@ def test_get_supplied_args(self):
 
         # test that positional args are recognised correctly
         result = _gsa(signature, ('foo', 'wibble'), {})
-        self.assertEqual(set(result), set(('arg1', 'arg2')))
+        self.assertEqual(
+            set(result),
+            set(('arg1', 'arg2')),
+        )
 
         # test that keyword args are recognised correctly
         result = _gsa(
-            signature, (), {'arg1': 'foo', 'arg2': 'wibble'},
+            signature,
+            (),
+            {
+                'arg1': 'foo',
+                'arg2': 'wibble',
+            },
+        )
+        self.assertEqual(
+            set(result),
+            set(('arg1', 'arg2')),
         )
-        self.assertEqual(set(result), set(('arg1', 'arg2')))
 
         # test that a mixture of args are recognised correctly
-        result = _gsa(signature, ('foo',), {'arg2': 'wibble'})
-        self.assertEqual(set(result), set(('arg1', 'arg2')))
+        result = _gsa(
+            signature,
+            ('foo',),
+            {'arg2': 'wibble'},
+        )
+        self.assertEqual(
+            set(result),
+            set(('arg1', 'arg2')),
+        )
 
         # test that None-valued positional args are ignored
         result = _gsa(
-            signature, ('foo', None), {},
+            signature,
+            ('foo', None),
+            {},
+        )
+        self.assertEqual(
+            set(result),
+            set(('arg1',)),
         )
-        self.assertEqual(set(result), set(('arg1',)))
 
         # test that None-valued keyword args are ignored
         result = _gsa(
-            signature, (), {'arg1': None, 'arg2': 'wibble', },
+            signature,
+            (),
+            {
+                'arg1': None,
+                'arg2': 'wibble',
+            },
+        )
+        self.assertEqual(
+            set(result),
+            set(('arg2',)),
         )
-        self.assertEqual(set(result), set(('arg2',)))
 
     def test_single_keyword_arg_provided(self):
         """Test for passing a single required parameter
         as a keyword argument.
         """
         _func = required_parameters('arg1')(undecorated_func)
-        self.assertEqual(_func(arg1='hello'), 'foo')
+        self.assertEqual(
+            _func(arg1='hello'),
+            'foo',
+        )
 
     def test_single_positional_arg_provided(self):
         """Test for passing a single required parameter
         as a positional argument.
         """
         _func = required_parameters('arg1')(undecorated_func)
-        self.assertEqual(_func('hello'), 'foo')
+        self.assertEqual(
+            _func('hello'),
+            'foo',
+        )
 
     def test_single_arg_missing(self):
         """Test that MissingParameterError is raised if the



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot