Revert of CQ to always post a message when Commit box is unchecked. (https://codereview.chromium.org/138173005/)

Reason for revert:
This created a lot of spam at commit event for some obscure reason. I couldn't fix it fast, so reverting.

Original issue's description:
> Made _discard_pending() always post a message. Added meaningful
> default messages to all the places where it is called in the code.  In
> particular, CQ will now always post a message whenever the Commit
> checkbox is unchecked, for any reason.
> 
> Updated unit tests accordingly, and made them a bit more robust by
> adding an error message to the failing FakeVerifier.
> 
> This does NOT fix the bug, but will hopefully give more visibility to
> developers when a CL is dropped.
> 
> BUG=238283
> 
> Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=247488

TBR=phajdan.jr@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=238283

Review URL: https://codereview.chromium.org/138133014

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/commit-queue@248624 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/pending_manager.py b/pending_manager.py
index 6f61d4b..4ef64d6 100644
--- a/pending_manager.py
+++ b/pending_manager.py
@@ -227,8 +227,13 @@
       # are str due to json support.
       if pending.issue not in new_issues:
         logging.info('Flushing issue %d' % pending.issue)
+        self.context.status.send(
+            pending,
+            { 'verification': 'abort',
+              'payload': {
+                'output': 'CQ bit was unchecked on CL. Ignoring.' }})
         pending.get_state = lambda: base.IGNORED
-        self._discard_pending(pending, 'CQ bit was unchecked on CL. Ignoring.')
+        self._discard_pending(pending, None)
 
     # Find new issues.
     for issue_id in new_issues:
@@ -298,10 +303,7 @@
                 pending.issue, pending.sort_key, missing, pending.get_state()))
         self._verify_pending(pending)
       except base.DiscardPending as e:
-        message = e.status
-        if not message:
-          message = 'process_new_pending_commit: ' + self.FAILED_NO_MESSAGE
-        self._discard_pending(e.pending, message)
+        self._discard_pending(e.pending, e.status)
 
   def update_status(self):
     """Updates the status for each pending commit verifier."""
@@ -313,10 +315,7 @@
       except base.DiscardPending as e:
         # It's not efficient since it takes a full loop for each pending
         # commit to discard.
-        message = e.status
-        if not message:
-          message = 'update_status: ' + self.FAILED_NO_MESSAGE
-        self._discard_pending(e.pending, message)
+        self._discard_pending(e.pending, e.status)
 
     for pending in self.queue.iterate():
       why_not = pending.why_not()
@@ -332,10 +331,8 @@
     for pending in self.queue.iterate():
       state = pending.get_state()
       if state == base.FAILED:
-        message = pending.error_message()
-        if not message:
-          message = 'scan_results(FAILED): ' + self.FAILED_NO_MESSAGE
-        self._discard_pending(pending, message)
+        self._discard_pending(
+            pending, pending.error_message() or self.FAILED_NO_MESSAGE)
       elif state == base.SUCCEEDED:
         if self._throttle(pending):
           continue
@@ -351,13 +348,9 @@
 
           self._commit_patch(pending)
         except base.DiscardPending as e:
-          message = e.status
-          if not message:
-            message = 'scan_results(discard): ' + self.FAILED_NO_MESSAGE
-          self._discard_pending(e.pending, message)
+          self._discard_pending(e.pending, e.status)
         except Exception as e:
-          message = 'scan_result(Exception): ' + self.INTERNAL_EXCEPTION
-          self._discard_pending(pending, message)
+          self._discard_pending(pending, self.INTERNAL_EXCEPTION)
           raise
       else:
         # When state is IGNORED, we need to keep this issue so it's not fetched
@@ -462,7 +455,6 @@
   def _discard_pending(self, pending, message):
     """Discards a pending commit. Attach an optional message to the review."""
     logging.debug('_discard_pending(%s, %s)', pending.issue, message)
-    msg = message or self.FAILED_NO_MESSAGE
     try:
       try:
         if pending.get_state() != base.IGNORED:
@@ -471,23 +463,24 @@
       except urllib2.HTTPError as e:
         logging.error(
             'Failed to set the flag to False for %s with message %s' % (
-              pending.pending_name(), msg))
+              pending.pending_name(), message))
         traceback.print_stack()
         logging.error(str(e))
         errors.send_stack(e)
-      try:
-        self.context.rietveld.add_comment(pending.issue, msg)
-      except urllib2.HTTPError as e:
-        logging.error(
-            'Failed to add comment for %s with message %s' % (
-              pending.pending_name(), msg))
-        traceback.print_stack()
-        errors.send_stack(e)
-      self.context.status.send(
-          pending,
-          { 'verification': 'abort',
-            'payload': {
-              'output': msg }})
+      if message:
+        try:
+          self.context.rietveld.add_comment(pending.issue, message)
+        except urllib2.HTTPError as e:
+          logging.error(
+              'Failed to add comment for %s with message %s' % (
+                pending.pending_name(), message))
+          traceback.print_stack()
+          errors.send_stack(e)
+        self.context.status.send(
+            pending,
+            { 'verification': 'abort',
+              'payload': {
+                'output': message }})
     finally:
       # Most importantly, remove the PendingCommit from the queue.
       self.queue.remove(pending.issue)
@@ -566,10 +559,7 @@
           out += '\n%s' % stdout
         raise base.DiscardPending(pending, out)
     except base.DiscardPending as e:
-      message = e.status
-      if not message:
-        message = '_commit_patch: ' + self.FAILED_NO_MESSAGE
-      self._discard_pending(e.pending, message)
+      self._discard_pending(e.pending, e.status)
 
   def _throttle(self, pending):
     """Returns True if a commit should be delayed."""
diff --git a/tests/pending_manager_test.py b/tests/pending_manager_test.py
index 12f73d4..0407eae 100755
--- a/tests/pending_manager_test.py
+++ b/tests/pending_manager_test.py
@@ -114,7 +114,7 @@
         self.context.rietveld.check_calls(
             [ _try_comment(),
               "set_flag(%d, 1, 'commit', False)" % issue,
-              "add_comment(%d, %r)" % (issue, fake.failed_message())])
+              "add_comment(%d, %r)" % (issue, pc.FAILED_NO_MESSAGE)])
     else:
       if success:
         self.context.checkout.check_calls(
@@ -131,11 +131,11 @@
           self.context.rietveld.check_calls(
               [ _try_comment(),
                 "set_flag(%d, 1, 'commit', False)" % issue,
-                "add_comment(%d, %r)" % (issue, fake.failed_message())])
+                "add_comment(%d, %r)" % (issue, pc.FAILED_NO_MESSAGE)])
         else:
           self.context.rietveld.check_calls(
               [ "set_flag(%d, 1, 'commit', False)" % issue,
-                "add_comment(%d, %r)" % (issue, fake.failed_message())])
+                "add_comment(%d, %r)" % (issue, pc.FAILED_NO_MESSAGE)])
 
   def _prepare_apply_commit(self, index, issue, server_hooks_missing=False):
     """Returns a frequent sequence of action happening on the Checkout object.
@@ -265,7 +265,7 @@
         ])
     self.context.rietveld.check_calls(
         [ _try_comment(),
-          "add_comment(%d, %r)" % (issue, fake.failed_message()),
+          "add_comment(%d, %r)" % (issue, pc.FAILED_NO_MESSAGE),
         ])
     self.context.status.check_names(['initial', 'abort'])
 
@@ -571,7 +571,7 @@
     self.context.checkout.check_calls(
         self._prepare_apply_commit(0, issue, server_hooks_missing=True))
 
-  def testDisappeared(self):
+  def testDisapeared(self):
     issue = 31337
     verifiers = [
         project_base.ProjectBaseUrlVerifier(
@@ -591,8 +591,6 @@
     pc.scan_results()
     self.assertEqual(0, len(pc.queue.iterate()))
     self.context.status.check_names(['abort'])
-    self.context.rietveld.check_calls(
-      ["add_comment(%d, 'CQ bit was unchecked on CL. Ignoring.')" % issue])
 
   def _get_pc_reviewer(self):
     verifiers = [
diff --git a/verification/fake.py b/verification/fake.py
index 9b95854..c38009e 100644
--- a/verification/fake.py
+++ b/verification/fake.py
@@ -6,8 +6,6 @@
 
 from verification import base
 
-def failed_message():
-  return 'FakeVerifier FAILED'
 
 class FakeVerifier(base.Verifier):
   name = 'fake'
@@ -17,11 +15,7 @@
     self.state = state
 
   def verify(self, pending):
-    fake = base.SimpleStatus(self.state)
-    # Make sure to leave a message, so CQ tests can reliably test for it.
-    if self.state == base.FAILED:
-      fake.error_message = failed_message()
-    pending.verifications[self.name] = fake
+    pending.verifications[self.name] = base.SimpleStatus(self.state)
 
   def update_status(self, queue):
     pass
@@ -45,6 +39,3 @@
 
     for _, fake in self.loop(queue, base.SimpleStatus, True):
       fake.state = self.state
-      # Make sure to leave a message, so CQ tests can reliably test for it.
-      if self.state == base.FAILED:
-        fake.error_message = failed_message()