From c3923114ba0202f1d2cfd5b6909f6448c945fdc0 Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Nov 10 2025 12:56:48 +0000 Subject: [PATCH 1/3] SidetagTest: treat None tag(untagging) a false assertion rather than an error --- diff --git a/plugins/hub/sidetag_hub.py b/plugins/hub/sidetag_hub.py index 0381311..68fc67c 100644 --- a/plugins/hub/sidetag_hub.py +++ b/plugins/hub/sidetag_hub.py @@ -56,7 +56,10 @@ class SidetagTest(koji.policy.MatchTest): name = 'is_sidetag' def run(self, data): - tag = get_tag(data['tag']) + tag = data.get('tag') + if not tag: + return False + tag = get_tag(tag) return is_sidetag(tag) From dec5466f81c65ce007489a88483735f14d39ee23 Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Nov 10 2025 13:08:20 +0000 Subject: [PATCH 2/3] SidetagOwnerTest: fix none value of tag/fromtag as well --- diff --git a/plugins/hub/sidetag_hub.py b/plugins/hub/sidetag_hub.py index 68fc67c..e3d5718 100644 --- a/plugins/hub/sidetag_hub.py +++ b/plugins/hub/sidetag_hub.py @@ -85,7 +85,9 @@ class SidetagOwnerTest(koji.policy.MatchTest): for field in fields: if field not in data: return False - tag = get_tag(data[field]) + tag = data.get(field) + if tag: + tag = get_tag(tag) if not tag or not is_sidetag_owner(tag, user): return False return True From 55c5260669b988149b79686947bb9e10df3d6cd6 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 11 2025 09:07:20 +0000 Subject: [PATCH 3/3] update unit tests --- diff --git a/tests/test_plugins/test_sidetag_hub.py b/tests/test_plugins/test_sidetag_hub.py index e8c22a2..31857d5 100644 --- a/tests/test_plugins/test_sidetag_hub.py +++ b/tests/test_plugins/test_sidetag_hub.py @@ -709,3 +709,101 @@ class TestSideTagUntagHub(unittest.TestCase): self.get_tag.assert_called_once_with(self.tag_input['id'], strict=False) self.is_sidetag.assert_called_once_with(self.tag_info) self._remove_sidetag.assert_not_called() + + +class TestTestHandlers(unittest.TestCase): + + def setUp(self): + self.context = mock.patch('sidetag_hub.context').start() + self.context.session.hasPerm.return_value = False # no admin by defaul + self.get_tag = mock.patch('sidetag_hub.get_tag').start() + self.basetag = { + 'id': 32, + 'name': 'base_tag', + 'arches': ['x86_64', 'i686'], + 'extra': {'sidetag': True, 'sidetag_user_id': 23}, + } + self.get_tag.return_value = self.basetag + self.policy_get_user = mock.patch('sidetag_hub.policy_get_user').start() + self.user = { + 'id': 23, + 'name': 'username', + } + self.policy_get_user.return_value = self.user + + def tearDown(self): + mock.patch.stopall() + + def test_is_sidetag(self): + obj = sidetag_hub.SidetagTest('is_sidetag') + self.assertTrue(obj.run({'tag': 'base_tag'})) + + self.get_tag.assert_called_once() + + def test_is_not_sidetag(self): + obj = sidetag_hub.SidetagTest('is_sidetag') + self.basetag['extra'] = {} + self.get_tag.return_value = self.basetag + self.assertFalse(obj.run({'tag': 'base_tag'})) + + self.get_tag.assert_called_once() + + def test_is_sidetag_no_tag(self): + obj = sidetag_hub.SidetagTest('is_sidetag') + self.assertFalse(obj.run({})) + + def test_is_sidetag_owner(self): + obj = sidetag_hub.SidetagOwnerTest('is_sidetag_owner') + # defaults in setUp should yield true + self.assertTrue(obj.run({'tag': 'base_tag'})) + + self.get_tag.assert_called_once() + self.policy_get_user.assert_called_once() + + def test_is_sidetag_owner_badargs1(self): + obj = sidetag_hub.SidetagOwnerTest('is_sidetag_owner INVALID ARGS') + # defaults in setUp should yield true + with self.assertRaises(koji.GenericError): + obj.run({'tag': 'base_tag'}) + + self.get_tag.assert_not_called() + + def test_is_sidetag_owner_badargs2(self): + obj = sidetag_hub.SidetagOwnerTest('is_sidetag_owner INVALID_ARGS') + # defaults in setUp should yield true + with self.assertRaises(koji.GenericError): + obj.run({'tag': 'base_tag'}) + + self.get_tag.assert_not_called() + + def test_is_not_sidetag_owner(self): + obj = sidetag_hub.SidetagOwnerTest('is_sidetag_owner') + self.user['id'] = 42 + self.assertFalse(obj.run({'tag': 'base_tag'})) + + self.get_tag.assert_called_once() + self.policy_get_user.assert_called_once() + + def test_is_sidetag_owner_notag(self): + obj = sidetag_hub.SidetagOwnerTest('is_sidetag_owner') + self.assertFalse(obj.run({})) + + self.get_tag.assert_not_called() + + def test_is_sidetag_owner_nofromtag(self): + obj = sidetag_hub.SidetagOwnerTest('is_sidetag_owner both') + self.assertFalse(obj.run({'tag': 'base_tag'})) + + def test_is_sidetag_owner_both(self): + obj = sidetag_hub.SidetagOwnerTest('is_sidetag_owner both') + othertag = self.basetag.copy() + othertag.update(id=42, name='other_tag') + self.get_tag.side_effect = [self.basetag, othertag] + self.assertTrue(obj.run({'tag': 'base_tag', 'fromtag': 'other_tag'})) + + # get tag should be called for both tags + self.assertEqual(len(self.get_tag.call_args), 2) + self.policy_get_user.assert_called_once() + + +# the end