#3403 Consistence pre/postPackageListChange sequence
Merged 13 days ago by tkopecek. Opened 2 months ago by jcupova.
jcupova/koji issue-1472  into  master

file modified
+6 -6
@@ -1016,10 +1016,6 @@ 

      # validate arches before running callbacks

      extra_arches = koji.parse_arches(extra_arches, strict=True, allow_none=True)

      user = get_user(context.session.user_id)

-     koji.plugin.run_callbacks('prePackageListChange', action=action,

-                               tag=tag, package=pkg, owner=owner,

-                               block=block, extra_arches=extra_arches,

-                               force=force, update=update, user=user)

      # first check to see if package is:

      #   already present (via inheritance)

      #   blocked
@@ -1060,6 +1056,10 @@ 

              owner = context.session.user_id

          else:

              raise koji.GenericError("owner not specified")

+     koji.plugin.run_callbacks('prePackageListChange', action=action,

+                               tag=tag, package=pkg, owner=owner,

+                               block=block, extra_arches=extra_arches,

+                               force=force, update=update, user=user)

      if not previous or changed:

          _pkglist_add(tag_id, pkg['id'], owner, block, extra_arches)

      elif changed_owner:
@@ -1136,8 +1136,6 @@ 

      # don't check policy for admins using force

      assert_policy('package_list', policy_data, force=force)

      user = get_user(context.session.user_id)

-     koji.plugin.run_callbacks(

-         'prePackageListChange', action='unblock', tag=tag, package=pkg, user=user)

      tag_id = tag['id']

      pkg_id = pkg['id']

      pkglist = readPackageList(tag_id, pkgID=pkg_id, inherit=True)
@@ -1147,6 +1145,8 @@ 

                                  % (pkg['name'], tag['name']))

      if not previous['blocked']:

          raise koji.GenericError("package %s NOT blocked in tag %s" % (pkg['name'], tag['name']))

+     koji.plugin.run_callbacks(

+         'prePackageListChange', action='unblock', tag=tag, package=pkg, user=user)

      if previous['tag_id'] != tag_id:

          _pkglist_add(tag_id, pkg_id, previous['owner_id'], False, previous['extra_arches'])

      else:

file modified
+135 -204
@@ -20,7 +20,7 @@ 

      return get_user

  

  

- class TestPkglistBlock(unittest.TestCase):

+ class TestPkglist(unittest.TestCase):

      def setUp(self):

          self.context = mock.patch('kojihub.context').start()

          # It seems MagicMock will not automatically handle attributes that
@@ -29,101 +29,91 @@ 

          self.context.session.user_id = 112233

          self.context.session.user_data = {'name': 'username'}

          self.run_callbacks = mock.patch('koji.plugin.run_callbacks').start()

+         self.read_package_list = mock.patch('kojihub.readPackageList').start()

+         self.lookup_package = mock.patch('kojihub.lookup_package').start()

+         self._pkglist_add = mock.patch('kojihub._pkglist_add').start()

+         self.get_tag = mock.patch('kojihub.get_tag').start()

+         self._pkglist_remove = mock.patch('kojihub._pkglist_remove').start()

+         self.assert_policy = mock.patch('kojihub.assert_policy').start()

+         self.get_user = mock.patch('kojihub.get_user').start()

  

      def tearDown(self):

          mock.patch.stopall()

  

-     @mock.patch('kojihub.readPackageList')

-     @mock.patch('kojihub.lookup_package')

-     @mock.patch('kojihub.get_tag')

      @mock.patch('kojihub.pkglist_add')

-     def test_pkglist_block(self, pkglist_add, get_tag, lookup_package, readPackageList):

+     def test_pkglist_block(self, pkglist_add):

          force = mock.MagicMock()

-         get_tag.return_value = {'name': 'tag', 'id': 123}

-         lookup_package.return_value = {'name': 'pkg', 'id': 321}

-         readPackageList.return_value = ['pkg']

+         self.get_tag.return_value = {'name': 'tag', 'id': 123}

+         self.lookup_package.return_value = {'name': 'pkg', 'id': 321}

+         self.read_package_list.return_value = ['pkg']

  

          kojihub.pkglist_block('tag', 'pkg', force=force)

  

-         get_tag.assert_called_once_with('tag', strict=True)

-         lookup_package.assert_called_once_with('pkg', strict=True)

+         self.get_tag.assert_called_once_with('tag', strict=True)

+         self.lookup_package.assert_called_once_with('pkg', strict=True)

          pkglist_add.assert_called_once_with('tag', 'pkg', block=True, force=force)

  

-     @mock.patch('kojihub.readPackageList')

-     @mock.patch('kojihub.lookup_package')

-     @mock.patch('kojihub.get_tag')

      @mock.patch('kojihub.pkglist_add')

-     def test_pkglist_block_package_error(

-             self, pkglist_add, get_tag, lookup_package, readPackageList):

+     def test_pkglist_block_package_error(self, pkglist_add):

          pkg_name = 'pkg'

          tag_name = 'tag'

          force = mock.MagicMock()

-         get_tag.return_value = {'name': tag_name, 'id': 123}

-         lookup_package.return_value = {'name': pkg_name, 'id': 321}

-         readPackageList.return_value = []

+         self.get_tag.return_value = {'name': tag_name, 'id': 123}

+         self.lookup_package.return_value = {'name': pkg_name, 'id': 321}

+         self.read_package_list.return_value = []

  

          with self.assertRaises(koji.GenericError) as ex:

              kojihub.pkglist_block('tag', 'pkg', force=force)

          self.assertEqual("Package %s is not in tag listing for %s" % (pkg_name, tag_name),

                           str(ex.exception))

  

-         get_tag.assert_called_once_with('tag', strict=True)

-         lookup_package.assert_called_once_with('pkg', strict=True)

+         self.get_tag.assert_called_once_with('tag', strict=True)

+         self.lookup_package.assert_called_once_with('pkg', strict=True)

          pkglist_add.assert_not_called()

  

-     @mock.patch('kojihub._pkglist_remove')

-     @mock.patch('kojihub._pkglist_add')

-     @mock.patch('kojihub.readPackageList')

-     @mock.patch('kojihub.assert_policy')

-     @mock.patch('kojihub.get_tag')

-     @mock.patch('kojihub.lookup_package')

-     def test_pkglist_unblock(

-             self, lookup_package, get_tag, assert_policy, readPackageList, _pkglist_add,

-             _pkglist_remove):

+     def test_pkglist_unblock(self,):

          tag = {'id': 1, 'name': 'tag'}

          pkg = {'id': 2, 'name': 'package', 'owner_id': 3}

-         get_tag.return_value = tag

-         lookup_package.return_value = pkg

-         readPackageList.return_value = {pkg['id']: {

+         self.get_tag.return_value = tag

+         self.lookup_package.return_value = pkg

+         self.read_package_list.return_value = {pkg['id']: {

              'blocked': True,

              'tag_id': tag['id'],

              'owner_id': pkg['owner_id'],

              'extra_arches': ''}}

+         users = [

+             {'id': 3, 'name': 'user'},

+             {'id': 112233, 'name': 'user'},

+         ]

+         user = users[1]

+         self.get_user.side_effect = get_user_factory(users)

  

          kojihub.pkglist_unblock('tag', 'pkg', force=False)

  

-         get_tag.assert_called_once_with('tag', strict=True)

-         lookup_package.assert_called_once_with('pkg', strict=True)

-         assert_policy.assert_called_once_with(

+         self.get_tag.assert_called_once_with('tag', strict=True)

+         self.lookup_package.assert_called_once_with('pkg', strict=True)

+         self.assert_policy.assert_called_once_with(

              'package_list', {'tag': tag['id'], 'action': 'unblock', 'package': pkg['id'],

                               'force': False},

              force=False)

-         self.assertEqual(readPackageList.call_count, 2)

-         readPackageList.assert_has_calls([

+         self.assertEqual(self.read_package_list.call_count, 2)

+         self.read_package_list.assert_has_calls([

              mock.call(tag['id'], pkgID=pkg['id'], inherit=True),

              mock.call(tag['id'], pkgID=pkg['id'], inherit=True),

          ])

-         _pkglist_add.assert_called_once_with(tag['id'], pkg['id'], pkg['owner_id'], False, '')

-         _pkglist_remove.assert_called_once_with(tag['id'], pkg['id'])

+         self._pkglist_add.assert_called_once_with(tag['id'], pkg['id'], pkg['owner_id'], False, '')

+         self._pkglist_remove.assert_called_once_with(tag['id'], pkg['id'])

          self.assertEqual(self.run_callbacks.call_count, 2)

          self.run_callbacks.assert_has_calls([

-             mock.call('prePackageListChange', action='unblock', tag=tag, package=pkg, user=None),

-             mock.call('postPackageListChange', action='unblock', tag=tag, package=pkg, user=None),

+             mock.call('prePackageListChange', action='unblock', tag=tag, package=pkg, user=user),

+             mock.call('postPackageListChange', action='unblock', tag=tag, package=pkg, user=user),

          ])

  

-     @mock.patch('kojihub._pkglist_remove')

-     @mock.patch('kojihub._pkglist_add')

-     @mock.patch('kojihub.readPackageList')

-     @mock.patch('kojihub.assert_policy')

-     @mock.patch('kojihub.get_tag')

-     @mock.patch('kojihub.lookup_package')

-     def test_pkglist_unblock_inherited(

-             self, lookup_package, get_tag, assert_policy, readPackageList, _pkglist_add,

-             _pkglist_remove):

+     def test_pkglist_unblock_inherited(self):

          tag_id, pkg_id, owner_id = 1, 2, 3

-         get_tag.return_value = {'id': tag_id, 'name': 'tag'}

-         lookup_package.return_value = {'id': pkg_id, 'name': 'pkg'}

-         readPackageList.return_value = {pkg_id: {

+         self.get_tag.return_value = {'id': tag_id, 'name': 'tag'}

+         self.lookup_package.return_value = {'id': pkg_id, 'name': 'pkg'}

+         self.read_package_list.return_value = {pkg_id: {

              'blocked': True,

              'tag_id': 4,

              'owner_id': owner_id,
@@ -131,56 +121,40 @@ 

  

          kojihub.pkglist_unblock('tag', 'pkg', force=False)

  

-         get_tag.assert_called_once_with('tag', strict=True)

-         lookup_package.assert_called_once_with('pkg', strict=True)

-         assert_policy.assert_called_once_with(

+         self.get_tag.assert_called_once_with('tag', strict=True)

+         self.lookup_package.assert_called_once_with('pkg', strict=True)

+         self.assert_policy.assert_called_once_with(

              'package_list', {'tag': tag_id, 'action': 'unblock', 'package': pkg_id,

                               'force': False},

              force=False)

-         readPackageList.assert_called_once_with(tag_id, pkgID=pkg_id, inherit=True)

-         _pkglist_add.assert_called_once_with(tag_id, pkg_id, owner_id, False, '')

-         _pkglist_remove.assert_not_called()

- 

-     @mock.patch('kojihub._pkglist_remove')

-     @mock.patch('kojihub._pkglist_add')

-     @mock.patch('kojihub.readPackageList')

-     @mock.patch('kojihub.assert_policy')

-     @mock.patch('kojihub.get_tag')

-     @mock.patch('kojihub.lookup_package')

-     def test_pkglist_unblock_not_present(

-             self, lookup_package, get_tag, assert_policy, readPackageList, _pkglist_add,

-             _pkglist_remove):

+         self.read_package_list.assert_called_once_with(tag_id, pkgID=pkg_id, inherit=True)

+         self._pkglist_add.assert_called_once_with(tag_id, pkg_id, owner_id, False, '')

+         self._pkglist_remove.assert_not_called()

+ 

+     def test_pkglist_unblock_not_present(self):

          tag_id, pkg_id = 1, 2

-         get_tag.return_value = {'id': tag_id, 'name': 'tag'}

-         lookup_package.return_value = {'id': pkg_id, 'name': 'pkg'}

-         readPackageList.return_value = {}

+         self.get_tag.return_value = {'id': tag_id, 'name': 'tag'}

+         self.lookup_package.return_value = {'id': pkg_id, 'name': 'pkg'}

+         self.read_package_list.return_value = {}

  

          with self.assertRaises(koji.GenericError):

              kojihub.pkglist_unblock('tag', 'pkg', force=False)

  

-         get_tag.assert_called_once_with('tag', strict=True)

-         lookup_package.assert_called_once_with('pkg', strict=True)

-         assert_policy.assert_called_once_with(

+         self.get_tag.assert_called_once_with('tag', strict=True)

+         self.lookup_package.assert_called_once_with('pkg', strict=True)

+         self.assert_policy.assert_called_once_with(

              'package_list', {'tag': tag_id, 'action': 'unblock', 'package': pkg_id,

                               'force': False},

              force=False)

-         readPackageList.assert_called_once_with(tag_id, pkgID=pkg_id, inherit=True)

-         _pkglist_add.assert_not_called()

-         _pkglist_remove.assert_not_called()

- 

-     @mock.patch('kojihub._pkglist_remove')

-     @mock.patch('kojihub._pkglist_add')

-     @mock.patch('kojihub.readPackageList')

-     @mock.patch('kojihub.assert_policy')

-     @mock.patch('kojihub.get_tag')

-     @mock.patch('kojihub.lookup_package')

-     def test_pkglist_unblock_not_blocked(

-             self, lookup_package, get_tag, assert_policy, readPackageList, _pkglist_add,

-             _pkglist_remove):

+         self.read_package_list.assert_called_once_with(tag_id, pkgID=pkg_id, inherit=True)

+         self._pkglist_add.assert_not_called()

+         self._pkglist_remove.assert_not_called()

+ 

+     def test_pkglist_unblock_not_blocked(self):

          tag_id, pkg_id, owner_id = 1, 2, 3

-         get_tag.return_value = {'id': tag_id, 'name': 'tag'}

-         lookup_package.return_value = {'id': pkg_id, 'name': 'pkg'}

-         readPackageList.return_value = {pkg_id: {

+         self.get_tag.return_value = {'id': tag_id, 'name': 'tag'}

+         self.lookup_package.return_value = {'id': pkg_id, 'name': 'pkg'}

+         self.read_package_list.return_value = {pkg_id: {

              'blocked': False,

              'tag_id': tag_id,

              'owner_id': owner_id,
@@ -189,15 +163,15 @@ 

          with self.assertRaises(koji.GenericError):

              kojihub.pkglist_unblock('tag', 'pkg', force=False)

  

-         get_tag.assert_called_once_with('tag', strict=True)

-         lookup_package.assert_called_once_with('pkg', strict=True)

-         assert_policy.assert_called_once_with(

+         self.get_tag.assert_called_once_with('tag', strict=True)

+         self.lookup_package.assert_called_once_with('pkg', strict=True)

+         self.assert_policy.assert_called_once_with(

              'package_list', {'tag': tag_id, 'action': 'unblock', 'package': pkg_id,

                               'force': False},

              force=False)

-         readPackageList.assert_called_once_with(tag_id, pkgID=pkg_id, inherit=True)

-         _pkglist_add.assert_not_called()

-         _pkglist_remove.assert_not_called()

+         self.read_package_list.assert_called_once_with(tag_id, pkgID=pkg_id, inherit=True)

+         self._pkglist_add.assert_not_called()

+         self._pkglist_remove.assert_not_called()

  

      @mock.patch('kojihub.pkglist_add')

      def test_pkglist_setowner(self, pkglist_add):
@@ -220,14 +194,7 @@ 

          _direct_pkglist_add.assert_called_once_with('tag', 'pkg', 'owner', 'block', 'extra_arches',

                                                      'force', 'update', policy=True)

  

-     @mock.patch('kojihub._pkglist_add')

-     @mock.patch('kojihub.readPackageList')

-     @mock.patch('kojihub.assert_policy')

-     @mock.patch('kojihub.get_user')

-     @mock.patch('kojihub.get_tag')

-     @mock.patch('kojihub.lookup_package')

-     def test_direct_pkglist_add(

-             self, lookup_package, get_tag, get_user, assert_policy, readPackageList, _pkglist_add):

+     def test_direct_pkglist_add(self):

          block = False

          extra_arches = 'arch123'

          force = False
@@ -240,22 +207,22 @@ 

              {'id': 112233, 'name': 'user'},

          ]

          user = users[0]

-         get_tag.return_value = tag

-         lookup_package.return_value = pkg

-         get_user.side_effect = get_user_factory(users)

-         readPackageList.return_value = {}

+         self.get_tag.return_value = tag

+         self.lookup_package.return_value = pkg

+         self.get_user.side_effect = get_user_factory(users)

+         self.read_package_list.return_value = {}

  

          kojihub._direct_pkglist_add(tag['name'], pkg['name'], user['name'], block=block,

                                      extra_arches=extra_arches, force=force, update=update,

                                      policy=policy)

  

-         get_tag.assert_called_once_with(tag['name'], strict=True)

-         lookup_package.assert_called_once_with(pkg['name'], strict=False)

-         get_user.assert_has_calls([

+         self.get_tag.assert_called_once_with(tag['name'], strict=True)

+         self.lookup_package.assert_called_once_with(pkg['name'], strict=False)

+         self.get_user.assert_has_calls([

              mock.call(user['name'], strict=True),

              mock.call(112233),

          ])

-         assert_policy.assert_called_once_with(

+         self.assert_policy.assert_called_once_with(

              'package_list', {'tag': tag['id'], 'action': 'add', 'package': pkg['name'],

                               'force': False},

              force=False)
@@ -268,16 +235,10 @@ 

                        owner=user['id'], block=block, extra_arches=extra_arches, force=force,

                        update=update, user=users[1]),

          ])

-         _pkglist_add.assert_called_once_with(tag['id'], pkg['id'], user['id'], block, extra_arches)

- 

-     @mock.patch('kojihub._pkglist_add')

-     @mock.patch('kojihub.readPackageList')

-     @mock.patch('kojihub.assert_policy')

-     @mock.patch('kojihub.get_user')

-     @mock.patch('kojihub.get_tag')

-     @mock.patch('kojihub.lookup_package')

-     def test_direct_pkglist_add_no_package(

-             self, lookup_package, get_tag, get_user, assert_policy, readPackageList, _pkglist_add):

+         self._pkglist_add.assert_called_once_with(

+             tag['id'], pkg['id'], user['id'], block, extra_arches)

+ 

+     def test_direct_pkglist_add_no_package(self):

          block = False

          extra_arches = 'arch123'

          force = False
@@ -286,10 +247,10 @@ 

          tag = {'id': 1, 'name': 'tag'}

          pkg = {'id': 2, 'name': 'pkg', 'owner_id': 3}

          user = {'id': 3, 'name': 'user'}

-         get_tag.return_value = tag

-         lookup_package.return_value = None

-         get_user.return_value = user

-         readPackageList.return_value = {}

+         self.get_tag.return_value = tag

+         self.lookup_package.return_value = None

+         self.get_user.return_value = user

+         self.read_package_list.return_value = {}

  

          # package needs to be name, not dict

          with self.assertRaises(koji.GenericError) as ex:
@@ -298,29 +259,20 @@ 

                                          policy=policy)

          self.assertEqual("No such package: %s" % pkg, str(ex.exception))

  

-     @mock.patch('kojihub.get_tag')

-     @mock.patch('kojihub.lookup_package')

-     def test_direct_pkglist_add_pkginfo_dict(self, lookup_package, get_tag):

+     def test_direct_pkglist_add_pkginfo_dict(self):

          pkg = {'id': 2, 'name': 'pkg', 'owner_id': 3}

          user = 'user'

          tag = {'id': 1, 'name': 'tag'}

          expected = "Invalid type for id lookup: %s" % pkg

  

-         get_tag.return_value = tag

-         lookup_package.side_effect = koji.GenericError(expected)

+         self.get_tag.return_value = tag

+         self.lookup_package.side_effect = koji.GenericError(expected)

          with self.assertRaises(koji.GenericError) as ex:

              kojihub._direct_pkglist_add(tag['name'], pkg, user, block=False, extra_arches='arch',

                                          force=False, update=True)

          self.assertEqual(expected, str(ex.exception))

  

-     @mock.patch('kojihub._pkglist_add')

-     @mock.patch('kojihub.readPackageList')

-     @mock.patch('kojihub.assert_policy')

-     @mock.patch('kojihub.get_user')

-     @mock.patch('kojihub.get_tag')

-     @mock.patch('kojihub.lookup_package')

-     def test_direct_pkglist_add_no_user(

-             self, lookup_package, get_tag, get_user, assert_policy, readPackageList, _pkglist_add):

+     def test_direct_pkglist_add_no_user(self):

          block = False

          extra_arches = 'arch123'

          force = False
@@ -329,28 +281,21 @@ 

          tag = {'id': 1, 'name': 'tag'}

          pkg = {'id': 2, 'name': 'pkg', 'owner_id': 3}

          user = {'id': 3, 'name': 'user'}

-         get_tag.return_value = tag

-         lookup_package.return_value = None

-         get_user.side_effect = koji.GenericError

-         readPackageList.return_value = {}

+         self.get_tag.return_value = tag

+         self.lookup_package.return_value = None

+         self.get_user.side_effect = koji.GenericError

+         self.read_package_list.return_value = {}

  

          with self.assertRaises(koji.GenericError):

              kojihub._direct_pkglist_add(tag['name'], pkg, user['name'], block=block,

                                          extra_arches=extra_arches, force=force, update=update,

                                          policy=policy)

  

-         lookup_package.assert_called_once_with(pkg, strict=False)

+         self.lookup_package.assert_called_once_with(pkg, strict=False)

          self.assertEqual(self.run_callbacks.call_count, 0)

-         _pkglist_add.assert_not_called()

- 

-     @mock.patch('kojihub._pkglist_add')

-     @mock.patch('kojihub.readPackageList')

-     @mock.patch('kojihub.assert_policy')

-     @mock.patch('kojihub.get_user')

-     @mock.patch('kojihub.get_tag')

-     @mock.patch('kojihub.lookup_package')

-     def test_direct_pkglist_add_new_package(

-             self, lookup_package, get_tag, get_user, assert_policy, readPackageList, _pkglist_add):

+         self._pkglist_add.assert_not_called()

+ 

+     def test_direct_pkglist_add_new_package(self):

          block = False

          extra_arches = 'arch123'

          force = False
@@ -363,26 +308,26 @@ 

              {'id': 112233, 'name': 'user1'},

          ]

          user = users[0]

-         get_tag.return_value = tag

-         lookup_package.side_effect = [None, pkg]

-         get_user.side_effect = get_user_factory(users)

-         readPackageList.return_value = {}

+         self.get_tag.return_value = tag

+         self.lookup_package.side_effect = [None, pkg]

+         self.get_user.side_effect = get_user_factory(users)

+         self.read_package_list.return_value = {}

  

          kojihub._direct_pkglist_add(tag['name'], pkg['name'], user['name'], block=block,

                                      extra_arches=extra_arches, force=force, update=update,

                                      policy=policy)

  

-         get_tag.assert_called_once_with(tag['name'], strict=True)

-         self.assertEqual(lookup_package.call_count, 2)

-         lookup_package.has_calls(

+         self.get_tag.assert_called_once_with(tag['name'], strict=True)

+         self.assertEqual(self.lookup_package.call_count, 2)

+         self.lookup_package.has_calls(

              mock.call(pkg['name'], strict=False),

              mock.call(pkg['name'], create=True),

          )

-         get_user.assert_has_calls([

+         self.get_user.assert_has_calls([

              mock.call(user['name'], strict=True),

              mock.call(112233),

          ])

-         assert_policy.assert_called_once_with(

+         self.assert_policy.assert_called_once_with(

              'package_list', {'tag': tag['id'], 'action': 'add', 'package': pkg['name'],

                               'force': False},

              force=False)
@@ -395,16 +340,10 @@ 

                        owner=user['id'], block=block, extra_arches=extra_arches, force=force,

                        update=update, user=users[1]),

          ])

-         _pkglist_add.assert_called_once_with(tag['id'], pkg['id'], user['id'], block, extra_arches)

- 

-     @mock.patch('kojihub._pkglist_add')

-     @mock.patch('kojihub.readPackageList')

-     @mock.patch('kojihub.assert_policy')

-     @mock.patch('kojihub.get_user')

-     @mock.patch('kojihub.get_tag')

-     @mock.patch('kojihub.lookup_package')

-     def test_direct_pkglist_add_blocked_previously(

-             self, lookup_package, get_tag, get_user, assert_policy, readPackageList, _pkglist_add):

+         self._pkglist_add.assert_called_once_with(

+             tag['id'], pkg['id'], user['id'], block, extra_arches)

+ 

+     def test_direct_pkglist_add_blocked_previously(self):

          block = False

          extra_arches = 'arch123'

          force = False
@@ -417,10 +356,10 @@ 

              {'id': 112233, 'name': 'user1'},

          ]

          user = users[0]

-         get_tag.return_value = tag

-         lookup_package.return_value = pkg

-         get_user.side_effect = get_user_factory(users)

-         readPackageList.return_value = {pkg['id']: {

+         self.get_tag.return_value = tag

+         self.lookup_package.return_value = pkg

+         self.get_user.side_effect = get_user_factory(users)

+         self.read_package_list.return_value = {pkg['id']: {

              'owner_id': pkg['owner_id'],

              'blocked': True,

              'extra_arches': ''}
@@ -431,29 +370,20 @@ 

                                          extra_arches=extra_arches, force=force, update=update,

                                          policy=policy)

  

-         get_tag.assert_called_once_with(tag['name'], strict=True)

-         lookup_package.assert_called_once_with(pkg['name'], strict=False)

-         get_user.assert_has_calls([

+         self.get_tag.assert_called_once_with(tag['name'], strict=True)

+         self.lookup_package.assert_called_once_with(pkg['name'], strict=False)

+         self.get_user.assert_has_calls([

              mock.call(user['name'], strict=True),

              mock.call(112233),

          ])

-         assert_policy.assert_called_once_with(

+         self.assert_policy.assert_called_once_with(

              'package_list', {'tag': tag['id'], 'action': 'add', 'package': pkg['name'],

                               'force': False},

              force=False)

-         self.run_callbacks.assert_called_once_with(

-             'prePackageListChange', action='add', tag=tag, package=pkg, owner=user['id'],

-             block=block, extra_arches=extra_arches, force=force, update=update, user=users[1])

-         _pkglist_add.assert_not_called()

- 

-     @mock.patch('kojihub._pkglist_add')

-     @mock.patch('kojihub.readPackageList')

-     @mock.patch('kojihub.assert_policy')

-     @mock.patch('kojihub.get_user')

-     @mock.patch('kojihub.get_tag')

-     @mock.patch('kojihub.lookup_package')

-     def test_direct_pkglist_add_blocked_previously_force(

-             self, lookup_package, get_tag, get_user, assert_policy, readPackageList, _pkglist_add):

+         self.run_callbacks.assert_not_called()

+         self._pkglist_add.assert_not_called()

+ 

+     def test_direct_pkglist_add_blocked_previously_force(self):

          block = False

          extra_arches = 'arch123'

          force = True
@@ -466,10 +396,10 @@ 

              {'id': 112233, 'name': 'user1'},

          ]

          user = users[0]

-         get_tag.return_value = tag

-         lookup_package.return_value = pkg

-         get_user.side_effect = get_user_factory(users)

-         readPackageList.return_value = {pkg['id']: {

+         self.get_tag.return_value = tag

+         self.lookup_package.return_value = pkg

+         self.get_user.side_effect = get_user_factory(users)

+         self.read_package_list.return_value = {pkg['id']: {

              'owner_id': pkg['owner_id'],

              'blocked': True,

              'extra_arches': ''}
@@ -479,14 +409,14 @@ 

                                      extra_arches=extra_arches, force=force, update=update,

                                      policy=policy)

  

-         get_tag.assert_called_once_with(tag['name'], strict=True)

-         lookup_package.assert_called_once_with(pkg['name'], strict=False)

-         get_user.assert_has_calls([

+         self.get_tag.assert_called_once_with(tag['name'], strict=True)

+         self.lookup_package.assert_called_once_with(pkg['name'], strict=False)

+         self.get_user.assert_has_calls([

              mock.call(user['name'], strict=True),

              mock.call(112233),

          ])

          # force + admin

-         assert_policy.assert_called_once_with(

+         self.assert_policy.assert_called_once_with(

              'package_list', {'tag': 1, 'action': 'add', 'package': 'pkg', 'force': True},

              force=True)

  
@@ -499,4 +429,5 @@ 

                        owner=user['id'], block=block, extra_arches=extra_arches, force=force,

                        update=update, user=users[1]),

          ])

-         _pkglist_add.assert_called_once_with(tag['id'], pkg['id'], user['id'], block, extra_arches)

+         self._pkglist_add.assert_called_once_with(

+             tag['id'], pkg['id'], user['id'], block, extra_arches)

rebased onto 7d71558

2 months ago

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

2 months ago

Metadata Update from @mfilip:
- Pull-request tagged with: testing-done

17 days ago

Commit c1643da fixes this pull-request

Pull-Request has been merged by tkopecek

13 days ago