bodhi/model.py | 66 +++++++++++++++++-------- bodhi/tests/test_controllers.py | 105 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 147 insertions(+), 24 deletions(-)
New commits: commit f50cdef90823a8246614c886d332d3617576a20f Author: Luke Macken lmacken@redhat.com Date: Fri Jul 9 15:31:54 2010 -0400
Refactor the critpath approval code, and improve the tests
diff --git a/bodhi/model.py b/bodhi/model.py index 240ed01..1f4495a 100644 --- a/bodhi/model.py +++ b/bodhi/model.py @@ -740,21 +740,40 @@ class PackageUpdate(SQLObject): Comment(text=text, karma=karma, update=self, author=author, anonymous=anonymous)
- if self.stable_karma != 0 and self.stable_karma == self.karma: - # If we're a criticalpath update to a pending release that is - # not yet approved, ensure that this karma can't cause it - # to go to stable. - if self.critpath and not self.critpath_approved: - pass - else: - if 'stable' not in (self.status, self.request): - log.info("Automatically marking %s as stable" % self.title) + if self.critpath: + min_karma = config.get('critpath.min_karma') + # If we weren't approved before, but are now... + if not critpath_approved and self.critpath_approved: + self.comment('Critical path update approved', author='bodhi') + mail.send_admin('critpath_approved', self) + # Karma automatism enabled + if self.stable_karma != 0: + # If this update has a stable karma threshold that is lower + # than the critpath.min_karma, then automatically push it to + # stable once it has met the requirements. + if (self.stable_karma < min_karma and self.critpath_approved and + self.karma >= min_karma and self.pushable): + self.request = 'stable' + mail.send(self.submitter, 'stablekarma', self) + mail.send_admin('stablekarma', self) + # If we're approved and meet the minimum requirements, then + # automatically push this update to the stable repository + if (self.critpath_approved and self.pushable and + self.karma >= self.stable_karma and + self.karma >= min_karma): self.request = 'stable' - self.pushed = False - #self.date_pushed = None mail.send(self.submitter, 'stablekarma', self) mail.send_admin('stablekarma', self)
+ if self.stable_karma != 0 and self.stable_karma == self.karma: + if self.pushable: + log.info("Automatically marking %s as stable" % self.title) + self.request = 'stable' + self.pushed = False + #self.date_pushed = None + mail.send(self.submitter, 'stablekarma', self) + mail.send_admin('stablekarma', self) + if self.status == 'testing' and self.unstable_karma != 0 and \ self.karma == self.unstable_karma: log.info("Automatically unpushing %s" % self.title) @@ -762,15 +781,6 @@ class PackageUpdate(SQLObject): 'being unpushed and marked as unstable' % self.karma) mail.send(self.submitter, 'unstable', self)
- # If we're a Critical Path update - if self.critpath: - # If we weren't approved before, and now are, push to stable - if not critpath_approved and self.critpath_approved: - self.comment('Critical path update approved', author='bodhi') - mail.send_admin('critpath_approved', self) - if self.stable_karma != 0: - self.request = 'stable' - # Send a notification to everyone that has commented on this update mail.send(self.people_to_notify(), 'comment', self)
@@ -864,6 +874,8 @@ class PackageUpdate(SQLObject): request=self.request, comments=[comment.__json__() for comment in self.comments], karma=self.karma, + stable_karma=self.stable_karma, + unstable_karma=self.unstable_karma, close_bugs=self.close_bugs, nagged=self.nagged, approved=self.approved, @@ -932,6 +944,17 @@ class PackageUpdate(SQLObject): # people.add(person) return people
+ @property + def pushable(self): + """ Return whether or not this update is in a pushable state. + + Note that this does not take into account critical path or karma + policies. + """ + return self.status != 'obsolete' and 'stable' not in (self.request, + self.status) + + class Comment(SQLObject): timestamp = DateTimeCol(default=datetime.utcnow) update = ForeignKey("PackageUpdate", notNone=True) diff --git a/bodhi/tests/test_controllers.py b/bodhi/tests/test_controllers.py index f3b7777..9989790 100644 --- a/bodhi/tests/test_controllers.py +++ b/bodhi/tests/test_controllers.py @@ -1578,7 +1578,7 @@ class TestControllers(testutil.DBTest): print update.stable_karma, update.unstable_karma assert update.request == None
- def test_critpath_to_pending_release_num_approved_comments(self): + def test_critpath_num_approved_comments_with_autokarma(self): """ Ensure releng/qa can push critpath updates to stable for pending releases after 1 releng/qa karma, and 1 other karma @@ -1592,9 +1592,10 @@ class TestControllers(testutil.DBTest): 'bugs' : '', 'notes' : 'foobar', 'autokarma': True, - 'stable_karma' : 10, + 'stable_karma' : 3, 'request': None, - 'unstable_karma' : -10, + 'unstable_karma' : -3, + 'autokarma': True, } self.save_update(params, releng) update = PackageUpdate.byTitle(params['builds']) @@ -1665,6 +1666,97 @@ class TestControllers(testutil.DBTest): update = PackageUpdate.byTitle(params['builds']) assert update.request == 'stable'
+ def test_critpath_num_approved_comments_with_high_stable_threshold(self): + """ + Ensure releng/qa can push critpath updates to stable + after 1 releng/qa karma, and 1 other karma -- but ensure that + it doesn't get pushed to stable automatically because it has yet to + reach the stable_karma threshold. + """ + releng = login(group='proventesters') + create_release(locked=True) + params = { + 'builds' : 'kernel-2.6.31-1.fc7', + 'release' : 'Fedora 7', + 'type_' : 'bugfix', + 'bugs' : '', + 'notes' : 'foobar', + 'autokarma': True, + 'stable_karma' : 10, + 'request': None, + 'unstable_karma' : -10, + 'autokarma': True, + } + self.save_update(params, releng) + update = PackageUpdate.byTitle(params['builds']) + testutil.create_request('/updates/%s' % params['builds'], + method='GET', headers=releng) + + # Ensure releng/QA can't push critpath updates alone + assert "Push to Testing" in cherrypy.response.body[0] + assert "Push Critical Path update to Stable" not in cherrypy.response.body[0] + + # Have a developer +1 the update + developer = login(username='bob') + testutil.create_request('/updates/comment?text=foobar&title=%s&karma=1' % + params['builds'], method='POST', headers=developer) + testutil.create_request('/updates/%s' % params['builds'], + method='GET', headers=developer) + assert "Push Critical Path update to Stable" not in cherrypy.response.body[0] + update = PackageUpdate.byTitle(params['builds']) + assert not update.request + assert len(update.comments) == 1 + assert update.comments[0].author == 'bob' + + # Make sure not even releng can submit it to stable until it gets another + # approval + testutil.create_request('/updates/request/stable/%s' % params['builds'], + method='GET', headers=releng) + update = PackageUpdate.byTitle(params['builds']) + assert update.request == 'testing' + assert update.karma == 1 + update.request = None + + # Have another developer +1 it, so it gets up to +2 + # Ensure we can't push it to stable, until we get admin approval + testutil.create_request('/updates/comment?text=foobar&title=%s&karma=1' % + params['builds'], method='POST', + headers=login(username='foobar')) + testutil.create_request('/updates/%s' % params['builds'], + method='GET', headers=login(username='foobar')) + assert "Push Critical Path update to Stable" not in cherrypy.response.body[0] + update = PackageUpdate.byTitle(params['builds']) + assert update.karma == 2 + assert update.request != 'stable', update.request + + # Ensure we're not yet approved + assert not update.critpath_approved + + # Have releng try again, and ensure it can be pushed to stable + testutil.capture_log(['bodhi.controllers', 'bodhi.util', 'bodhi.model']) + testutil.create_request('/updates/comment?text=foobar&title=%s&karma=1' % + params['builds'], method='POST', headers=releng) + update = PackageUpdate.byTitle(params['builds']) + print update.stable_karma, update.unstable_karma + + assert update.request == None, "Auto-karma kicked in even though it didn't reach the stable threshold yet!" + + assert update.critpath + assert update.critpath_approved + + # 7 more comments + for user in range(7): + dev_user = login(username='user%d' % user) + testutil.create_request('/updates/comment?text=yay&title=%s&karma=1' + % params['builds'], method='POST', + headers=dev_user) + + update = PackageUpdate.byTitle(params['builds']) + logs = testutil.get_log() + print logs + #assert False, logs + assert update.request == 'stable', update.__json__() + def test_critpath_to_frozen_release_testing(self): """ Ensure devs can *not* push critpath updates directly to stable @@ -1742,6 +1834,7 @@ class TestControllers(testutil.DBTest): 'stable_karma' : 1, 'request': None, 'unstable_karma' : -1, + 'autokarma': True, } self.save_update(params, session) update = PackageUpdate.byTitle(params['builds']) @@ -1763,11 +1856,15 @@ class TestControllers(testutil.DBTest):
assert "Mark Critical Path update as Stable" not in cherrypy.response.body[0]
+ #testutil.capture_log(['bodhi.controllers', 'bodhi.util', 'bodhi.model']) testutil.create_request('/updates/comment?text=foobar&title=%s&karma=1' % params['builds'], method='POST', headers=login(username='bob'))
update = PackageUpdate.byTitle(params['builds']) + assert update.critpath + assert update.critpath_approved + #assert False, testutil.get_log() assert len(update.comments) == 3, update.comments assert update.comments[1].author == 'bob', update.comments assert update.comments[2].author == 'bodhi', update.comments @@ -2100,3 +2197,5 @@ class TestControllers(testutil.DBTest):
# Ensure the flag gets unset properly assert not PackageUpdate.byTitle(params['builds']).builds[0].package.suggest_reboot + +
commit da231da257ceb36201a2754fc5a58560bddc5918 Author: Luke Macken lmacken@redhat.com Date: Fri Jul 9 14:05:07 2010 -0400
When determining 'mycomments', only pay attention to authenticated ones...
diff --git a/bodhi/model.py b/bodhi/model.py index 35ad8c2..240ed01 100644 --- a/bodhi/model.py +++ b/bodhi/model.py @@ -727,7 +727,8 @@ class PackageUpdate(SQLObject): if not anonymous and karma != 0 and \ not filter(lambda c: c.author == author and c.karma == karma, self.comments): - mycomments = [c.karma for c in self.comments if c.author == author] + mycomments = [c.karma for c in self.comments if c.author == author + and not c.anonymous] if karma == 1 and -1 in mycomments: self.karma += 2 elif karma == -1 and 1 in mycomments: