Improve tests

ckanext/privatedatasets/tests/test_auth.py

@@ -78,8 +78,7 @@ class AuthTest(unittest.TestCase):
         (1,    2,    'test', True,  'active', 'conwet', False, True,  None,        None,              True),
         (1,    2,    'test', True,  'active', 'conwet', False, False, None,        None,              False),
         (1,    2,    'test', True,  'active', 'conwet', False, False, 'google.es', '/dataset/testds', False),
-        (1,    2,    'test', True,  'active', 'conwet', False, False, 'google.es', '/',               False)
+        (1,    2,    'test', True,  'active', 'conwet', False, False, 'google.es', '/',               False)    ])
-    ])
     def test_auth_package_show(self, creator_user_id, user_obj_id, user, private, state, owner_org,
                                owner_member, db_auth, adquire_url, request_path, authorized):
 
@@ -126,19 +125,25 @@ class AuthTest(unittest.TestCase):
         # and when the dataset has not been created by the user who is asking for it
         if private and owner_org and state == 'active' and creator_user_id != user_obj_id:
             auth.new_authz.has_user_permission_for_group_or_org.assert_called_once_with(owner_org, user, 'read')
+        else:
+            self.assertEquals(0, auth.new_authz.has_user_permission_for_group_or_org.call_count)
 
         # The databse is only initialized when:
         # * the dataset is private AND
         # * the dataset is active AND
         # * the dataset has no organization OR the user does not belong to that organization AND
-        # * the dataset has not been created by the user who is asking for it
+        # * the dataset has not been created by the user who is asking for it OR the user is not specified
-        if private and state == 'active' and ((owner_org and not owner_org) or not owner_org) and creator_user_id != user_obj_id:
+        if private and state == 'active' and (not owner_org or not owner_member) and (creator_user_id != user_obj_id or user_obj_id is None):
             # Check that the database has been initialized properly
             auth.db.init_db.assert_called_once_with(context['model'])
+        else:
+            self.assertEquals(0, auth.db.init_db.call_count)
 
         # Conditions to buy a dataset; It should be private, active and should not belong to any organization
         if not authorized and state == 'active' and not owner_org and request_path.startswith('/dataset/'):
             auth.helpers.flash_error.assert_called_once()
+        else:
+            self.assertEquals(0, auth.helpers.flash_error.call_count)
 
     @parameterized.expand([
         (None, None, None,   None,     None,  False),   # Anonymous user
@@ -171,9 +176,12 @@ class AuthTest(unittest.TestCase):
         # Check the result
         self.assertEquals(authorized, result['success'])
 
-        # Check that the mock has been called properly
+        # Permissions for organization are checked when the user asking to update the dataset is not the creator
+        # and when the dataset has organization
         if creator_user_id != user_obj_id and owner_org:
             auth.new_authz.has_user_permission_for_group_or_org.assert_called_once_with(owner_org, user, 'update_dataset')
+        else:
+            self.assertEquals(0, auth.new_authz.has_user_permission_for_group_or_org.call_count)
 
     @parameterized.expand([
         (True,  True),

ckanext/privatedatasets/tests/test_converters_validators.py

@@ -38,14 +38,22 @@ class ConvertersValidatorsTest(unittest.TestCase):
         # not be taken into account anymore)
         (True,    True,  'conwet', 'test', False),
         ('True',  True,  'conwet', 'test', False),
+        (True,    False, 'conwet', 'test', False),
+        ('True',  False, 'conwet', 'test', False),
         (False,   True,  'conwet', 'test', True),
         ('False', True,  'conwet', 'test', True),
+        (False,   False, 'conwet', 'test', True),
+        ('False', False, 'conwet', 'test', True),
         (None,    True,  'conwet', 'test', False),
         (None,    False, 'conwet', 'test', True),
         (True,    True,  None,     'test', False),
         ('True',  True,  None,     'test', False),
+        (True,    False, None,     'test', False),
+        ('True',  False, None,     'test', False),
         (False,   True,  None,     'test', True),
         ('False', True,  None,     'test', True),
+        (False,   False, None,     'test', True),
+        ('False', False, None,     'test', True),
         (None,    True,  None,     'test', False),
         (None,    False, None,     'test', True),
     ])
@@ -109,17 +117,10 @@ class ConvertersValidatorsTest(unittest.TestCase):
         key = 'allowed_users'
         data = {('id',): 'package_id'}
 
-        # Each time 'AllowedUser' is called, we must get a new instance
-        # and this is the way to get this behaviour
-        def constructor():
-            return MagicMock()
-
-        conv_val.db.AllowedUser = MagicMock(side_effect=constructor)
-
         # Create the users
         db_res = []
         for user in users:
-            db_row = conv_val.db.AllowedUser()
+            db_row = MagicMock()
             db_row.package_id = 'package_id'
             db_row.user_name = user
             db_res.append(db_row)

ckanext/privatedatasets/tests/test_db.py

@@ -7,13 +7,15 @@ from mock import MagicMock
 class DBTest(unittest.TestCase):
 
     def setUp(self):
+        # Restart databse initial status
+        db.AllowedUser = None
+
         # Create mocks
         self._sa = db.sa
         db.sa = MagicMock()
 
     def tearDown(self):
         db.sa = self._sa
-        db.AllowedUser = None
 
     def test_initdb_not_initialized(self):
 

ckanext/privatedatasets/tests/test_helpers.py

@@ -29,13 +29,13 @@ class HelpersTest(unittest.TestCase):
         (False, None,   False),
         (True,  None,   False),
     ])
-    def test_is_adquired(self, db_auth, user, adquired):
+    def test_is_adquired(self, db_adquired, user, adquired):
         # Configure test
         helpers.tk.c.user = user
         pkg_dict = {'id': 'package_id'}
 
         db_response = []
-        if db_auth is True:
+        if db_adquired is True:
             out = helpers.db.AllowedUser()
             out.package_id = 'package_id'
             out.user_name = user

ckanext/privatedatasets/tests/test_plugin.py

@@ -31,15 +31,18 @@ class PluginTest(unittest.TestCase):
         (plugin.p.IPackageController,),
         (plugin.p.ITemplateHelpers,)
     ])
-    def test_implementations(self, interface):
+    def test_implementation(self, interface):
         self.assertTrue(interface.implemented_by(plugin.PrivateDatasets))
 
-    def test_auth_functions(self):
+    @parameterized.expand([
+        ('package_show',     plugin.auth.package_show),
+        ('package_update',   plugin.auth.package_update),
+        ('package_show',     plugin.auth.package_show),
+        ('package_adquired', plugin.auth.package_adquired)
+    ])
+    def test_auth_function(self, function_name, expected_function):
         auth_functions = self.privateDatasets.get_auth_functions()
-        self.assertEquals(auth_functions['package_show'], plugin.auth.package_show)
+        self.assertEquals(auth_functions[function_name], expected_function)
-        self.assertEquals(auth_functions['package_update'], plugin.auth.package_update)
-        self.assertEquals(auth_functions['resource_show'], plugin.auth.resource_show)
-        self.assertEquals(auth_functions['package_adquired'], plugin.auth.package_adquired)
 
     def test_update_config(self):
         # Call the method
@@ -60,9 +63,12 @@ class PluginTest(unittest.TestCase):
                                   controller='ckanext.privatedatasets.controllers.ui_controller:AdquiredDatasetsControllerUI',
                                   action='user_adquired_datasets', conditions=dict(method=['GET']))
 
-    def test_actions_functions(self):
+    @parameterized.expand([
+        ('package_adquired', plugin.actions.package_adquired)
+    ])
+    def test_actions_function(self, function_name, expected_function):
         actions = self.privateDatasets.get_actions()
-        self.assertEquals(actions['package_adquired'], plugin.actions.package_adquired)
+        self.assertEquals(actions[function_name], expected_function)
 
     def test_fallback(self):
         self.assertEquals(True, self.privateDatasets.is_fallback())
@@ -70,6 +76,15 @@ class PluginTest(unittest.TestCase):
     def test_package_types(self):
         self.assertEquals([], self.privateDatasets.package_types())
 
+    @parameterized.expand([
+        ('privatedatasets_adquired', plugin.helpers.is_adquired),
+        ('get_allowed_users_str',    plugin.helpers.get_allowed_users_str),
+        ('is_owner',                 plugin.helpers.is_owner)
+    ])
+    def test_helpers_functions(self, function_name, expected_function):
+        helpers_functions = self.privateDatasets.get_helpers()
+        self.assertEquals(helpers_functions[function_name], expected_function)
+
     ######################################################################
     ############################## SCHEMAS ###############################
     ######################################################################
@@ -121,10 +136,10 @@ class PluginTest(unittest.TestCase):
     ######################################################################
 
     @parameterized.expand([
-        ('after_delete',),
+        ('True'),
-        ('after_delete', 'False')
+        ('False')
     ])
-    def test_packagecontroller_after_delete(self, function, private='True'):
+    def test_packagecontroller_after_delete(self, private):
         pkg_dict = {'test': 'a', 'private': private, 'allowed_users': ['a', 'b', 'c']}
         expected_pkg_dict = pkg_dict.copy()
         result = self.privateDatasets.after_delete({}, pkg_dict)    # Call the function
@@ -156,7 +171,7 @@ class PluginTest(unittest.TestCase):
     ])
     def test_packagecontroller_after_show(self, update_via_api, creator_id, user_id, sysadmin, fields_expected):
         
-        context = {'updating_via_cb': update_via_api, }
+        context = {'updating_via_cb': update_via_api}
 
         if creator_id is not None or sysadmin is not None:
             user = MagicMock()
@@ -215,10 +230,6 @@ class PluginTest(unittest.TestCase):
 
         self.assertEquals(expected_result, self.privateDatasets.before_index(pkg_dict))
 
-    def test_helpers_functions(self):
-        helpers_functions = self.privateDatasets.get_helpers()
-        self.assertEquals(helpers_functions['privatedatasets_adquired'], plugin.helpers.is_adquired)
-
     def _aux_test_after_create_update(self, function, new_users, current_users, users_to_add, users_to_delete):
         package_id = 'package_id'