From e557addff80f826a44b887baf6c9fd4a41eeb048 Mon Sep 17 00:00:00 2001 From: Alexander Loechel Date: Mon, 31 Jul 2017 22:39:20 +0200 Subject: [PATCH] enhance readability --- src/plone/api/content.py | 2 +- src/plone/api/portal.py | 11 +- src/plone/api/tests/test_doctests.py | 6 +- src/plone/api/tests/test_env.py | 14 ++ src/plone/api/tests/test_portal.py | 5 +- src/plone/api/tests/test_user.py | 210 +++++++++++++++++-------- src/plone/api/tests/test_validation.py | 59 +++++-- 7 files changed, 219 insertions(+), 88 deletions(-) diff --git a/src/plone/api/content.py b/src/plone/api/content.py index 9ce93732..7bfb2192 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 85a515b5..5dae7b44 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 6470065b..a3ba8845 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 44bb3536..bf64e7f8 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 141b7989..b4ad86fd 100644 --- a/src/plone/api/tests/test_portal.py +++ b/src/plone/api/tests/test_portal.py @@ -77,7 +77,10 @@ def setUp(self): 'sender@example.org', ) else: - self.portal._updateProperty('email_from_name', 'Portal Owner') + self.portal._updateProperty( + 'email_from_name', + 'Portal Owner', + ) self.portal._updateProperty( 'email_from_address', 'sender@example.org', diff --git a/src/plone/api/tests/test_user.py b/src/plone/api/tests/test_user.py index 68f37965..0f49ef24 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@norris.org', 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@norris.org', 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 14f0edeb..f3e19bb9 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