Only comment on non-autokarma updates meeting testing requirements.

This commit fixes an issue where non-autokarma ciritcal path
updates were getting repeat e-mails every 6 hours saying that they
could be pushed to testing. The code that does these checks is far
more complicated than it needs to be, but it was possible to fix
this issue by adding a simple and onto the end of an existing if
statement so it will do for now. I intend to refactor this area of
the code in the future.

fixes fedora-infra#1009

bodhi/server/scripts/approve_testing.py

@@ -58,7 +58,7 @@ def main(argv=sys.argv):
 
             # Approval message when testing based on karma threshold
             if update.stable_karma not in (0, None) and update.karma >= update.stable_karma \
-                    and not update.autokarma:
+                    and not update.autokarma and update.meets_testing_requirements:
                 print('%s now reaches stable karma threshold' % update.title)
                 text = config.get('testing_approval_msg_based_on_karma')
                 update.comment(db, text, author='bodhi')

bodhi/tests/server/scripts/test_approve_testing.py

@@ -106,6 +106,78 @@ def test_autokarma_update_not_meeting_testing_requirments(self):
         self.assertEqual(self.db.query(models.User).filter_by(name=u'bodhi').count(), 0)
         self.assertEqual(self.db.query(models.Comment).count(), 0)
 
+    def test_non_autokarma_critpath_update_meeting_karma_requirements_gets_one_comment(self):
+        """
+        Ensure that a non-autokarma critical path update that meets the required karma threshold
+        and required time in testing gets only one comment from Bodhi to that effect, even on
+        subsequent runs of main(). There was an issue[0] where Bodhi wasn't correctly detecting when
+        it should add these comments, and with detecting that it has already commented on
+        critical path updates, and would repeatedly comment that these updates could be pushed.
+        This test ensures that issue stays fixed.
+
+        [0] https://github.com/fedora-infra/bodhi/issues/1009
+        """
+        update = self.db.query(models.Update).all()[0]
+        update.autokarma = False
+        # Make this update a critpath update to force meets_testing_requirements into a different
+        # code path.
+        update.critpath = True
+        # It's been in testing long enough to get the comment from bodhi that it can be pushed.
+        update.date_testing = datetime.now() - timedelta(days=15)
+        update.request = None
+        update.stable_karma = 1
+        update.status = models.UpdateStatus.testing
+        update.comment(self.db, 'testing', author='hunter2', anonymous=False, karma=1)
+
+        with patch(
+                'bodhi.server.scripts.approve_testing._get_db_session', return_value=self.db):
+            approve_testing.main(['nosetests', 'some_config.ini'])
+
+            # Now we will run main() again, but this time we expect Bodhi not to add any further
+            # comments.
+            approve_testing.main(['nosetests', 'some_config.ini'])
+
+        bodhi = self.db.query(models.User).filter_by(name=u'bodhi').one()
+        comment_q = self.db.query(models.Comment).filter_by(update_id=update.id, user_id=bodhi.id)
+        self.assertEqual(comment_q.count(), 1)
+        self.assertEqual(comment_q[0].text, config.get('testing_approval_msg_based_on_karma'))
+
+    def test_non_autokarma_critpath_update_not_meeting_time_requirements_gets_no_comment(self):
+        """
+        Ensure that a non-autokarma critical path update that does not meet the required time in
+        testing does not get any comment from bodhi saying it can be pushed to stable.
+        There was an issue[0] where Bodhi was incorrectly detecting that the update could be pushed
+        and was commenting to that effect. This test ensures that issue stays fixed.
+
+        [0] https://github.com/fedora-infra/bodhi/issues/1009
+        """
+        update = self.db.query(models.Update).all()[0]
+        update.autokarma = False
+        # Make this update a critpath update to force meets_testing_requirements into a different
+        # code path.
+        update.critpath = True
+        update.request = None
+        update.stable_karma = 1
+        update.status = models.UpdateStatus.testing
+        update.comment(self.db, 'testing', author='hunter2', anonymous=False, karma=1)
+
+        with patch(
+                'bodhi.server.scripts.approve_testing._get_db_session', return_value=self.db):
+            approve_testing.main(['nosetests', 'some_config.ini'])
+
+            # Now we will run main() again, but this time we expect Bodhi not to add any further
+            # comments.
+            approve_testing.main(['nosetests', 'some_config.ini'])
+
+        # The bodhi user shouldn't exist, since it shouldn't have made any comments
+        self.assertEqual(self.db.query(models.User).filter_by(name=u'bodhi').count(), 0)
+        # There are three comments, but none from the non-existing bodhi user.
+        self.assertEqual(self.db.query(models.Comment).count(), 3)
+        usernames = [
+            c.user.name
+            for c in self.db.query(models.Comment).order_by(models.Comment.timestamp).all()]
+        self.assertEqual(usernames, [u'guest', u'anonymous', u'hunter2'])
+
     def test_non_autokarma_update_meeting_karma_requirements_gets_one_comment(self):
         """
         Ensure that a non-autokarma update that meets the required karma threshold gets only one