Changes to support One-Click Revert in Commit Queue

tests/pending_manager_test.py

Index: tests/pending_manager_test.py
===================================================================
--- tests/pending_manager_test.py	(revision 220012)
+++ tests/pending_manager_test.py	(working copy)
@@ -82,9 +82,11 @@
       if os.path.exists(filename + '.old'):
         os.remove(filename + '.old')
 
-  def _get_pc(self, verifiers_no_patch, verifiers):
+  def _get_pc(self, verifiers_no_patch, verifiers,
+              revert_verifiers_no_patch=None, revert_verifiers=None):
     return pending_manager.PendingManager(
-        self.context, verifiers_no_patch, verifiers)
+        self.context, verifiers_no_patch, verifiers, revert_verifiers_no_patch,
+        revert_verifiers)
 
   def _check_standard_verification(self, pc, success, defered):
     """Verifies the checkout and rietveld calls."""
@@ -95,9 +97,11 @@
       if success:
         self.context.checkout.check_calls(
             [ 'prepare(None)',
-              'apply_patch(%r)' % (self.context.rietveld.patchsets[0],),
+              'apply_patch(%r, revert=False)' % (
+                  self.context.rietveld.patchsets[0]),
               'prepare(None)',  # Will sync to HEAD/124.
-              'apply_patch(%r)' % (self.context.rietveld.patchsets[1],),
+              'apply_patch(%r, revert=False)' % (
+                  self.context.rietveld.patchsets[1]),
               (
                 "commit(u'foo\\n\\n"
                 "Review URL: http://nowhere/%d', "
@@ -110,7 +114,8 @@
       else:
         self.context.checkout.check_calls(
             [ 'prepare(None)',
-              'apply_patch(%r)' % (self.context.rietveld.patchsets[0],)])
+              'apply_patch(%r, revert=False)' % (
+                  self.context.rietveld.patchsets[0])])
         self.context.rietveld.check_calls(
             [ _try_comment(),
               "set_flag(%d, 1, 'commit', 'False')" % issue,
@@ -137,7 +142,8 @@
               [ "set_flag(%d, 1, 'commit', 'False')" % issue,
                 "add_comment(%d, %r)" % (issue, pc.FAILED_NO_MESSAGE)])
 
-  def _prepare_apply_commit(self, index, issue, server_hooks_missing=False):
+  def _prepare_apply_commit(self, index, issue, server_hooks_missing=False,
+                            revert=False):
     """Returns a frequent sequence of action happening on the Checkout object.
 
     The list returned by this function should be used as an argument to
@@ -148,17 +154,21 @@
       # present.
       'prepare(None)',
       # Applies the requested PatchSet.
-      'apply_patch(%r)' % self.context.rietveld.patchsets[index],
+      'apply_patch(%r, revert=%s)' % (self.context.rietveld.patchsets[index],
+                                      revert),
     ]
     # Commits the patch.
     author_and_reviewer = ''
     if server_hooks_missing:
       author_and_reviewer = (
           '\\n\\nR=rev@example.com\\n\\nAuthor: author@example.com')
+    desc = 'foo'
+    if revert:
+      desc = 'Revert of: ' + desc
     commit_message = (
-        "commit(u'foo%s\\n\\n"
+        "commit(u'%s%s\\n\\n"
         "Review URL: http://nowhere/%d', "
-        "u'author@example.com')") % (author_and_reviewer, issue)
+        "u'author@example.com')") % (desc, author_and_reviewer, issue)
     seq.append(commit_message)
 
     return seq
@@ -258,7 +268,8 @@
     self.assertEqual([], pc.queue.iterate())
     self.context.checkout.check_calls(
         [ 'prepare(None)',
-          'apply_patch(%r)' % (self.context.rietveld.patchsets[0],),
+          'apply_patch(%r, revert=False)' % (
+              self.context.rietveld.patchsets[0]),
         ])
     self.context.rietveld.check_calls(
         [ _try_comment(),
@@ -435,7 +446,8 @@
     self.assertEqual('sub/dir', pc.queue.get(issue).relpath)
     self.context.checkout.check_calls(
         [ 'prepare(None)',
-          'apply_patch(%r)' % (self.context.rietveld.patchsets[0],)])
+          'apply_patch(%r, revert=False)' % (
+              self.context.rietveld.patchsets[0])])
     pc.update_status()
     self.context.checkout.check_calls([])
     pc.scan_results()
@@ -530,6 +542,96 @@
           "add_comment(%d, 'Change committed as 125')" % issue])
     self.context.status.check_names(['why not', 'commit'] * 2)
 
+  def testRevertBurst(self):
+    issue = 31337
+    pc = self._get_pc(
+        verifiers_no_patch=[fake.FakeVerifier(base.SUCCEEDED)],
+        verifiers=[],
+        revert_verifiers_no_patch=[fake.FakeVerifier(base.SUCCEEDED)],
+        revert_verifiers=[])
+    self.assertEqual(4, pc.MAX_COMMIT_BURST)
+    timestamp = [1]
+    self.mock(time, 'time', lambda: timestamp[-1])
+    # Set test issues to 'revert' issues, 31337 will still be a regular commit.
+    for i in range(pc.MAX_COMMIT_BURST + 2):
+      rietveld_issue = self.context.rietveld.issues[issue].copy()
+      rietveld_issue['closed'] = True
+      rietveld_issue['commit'] = False
+      rietveld_issue['revert'] = True
+      rietveld_issue['reverted_by'] = 'xyz@example.com'
+      self.context.rietveld.issues[i] = rietveld_issue
+      self.context.rietveld.issues[i]['issue'] = i
+    pc.look_for_new_pending_commit()
+    self.assertEqual(len(pc.queue.iterate()), pc.MAX_COMMIT_BURST + 3)
+    pc.process_new_pending_commit()
+    pc.update_status()
+    pc.scan_results()
+    self.context.checkout.check_calls(
+        self._prepare_apply_commit(0, 0, False, True) +
+        self._prepare_apply_commit(1, 1, False, True) +
+        self._prepare_apply_commit(2, 2, False, True) +
+        self._prepare_apply_commit(3, 3, False, True))
+    self.context.rietveld.check_calls(
+        [ _try_comment(0),
+          _try_comment(1),
+          _try_comment(2),
+          _try_comment(3),
+          _try_comment(4),
+          _try_comment(5),
+          _try_comment(),
+          'open_issue(0)',
+          "update_description(0, u'foo')",
+          "add_comment(0, 'Change reverted as 125')",
+          'open_issue(1)',
+          "update_description(1, u'foo')",
+          "add_comment(1, 'Change reverted as 125')",
+          'open_issue(2)',
+          "update_description(2, u'foo')",
+          "add_comment(2, 'Change reverted as 125')",
+          'open_issue(3)',
+          "update_description(3, u'foo')",
+          "add_comment(3, 'Change reverted as 125')",
+        ])
+    self.assertEqual(3, len(pc.queue.iterate()))
+    total = pc.MAX_COMMIT_BURST + 3
+    self.context.status.check_names(['initial'] * total +
+                                    (['why not', 'commit'] *
+                                     pc.MAX_COMMIT_BURST) +
+                                    ['why not'] * 3)
+
+    # Dry run.
+    pc.scan_results()
+    self.context.checkout.check_calls([])
+    self.context.rietveld.check_calls([])
+    self.context.status.check_names(['why not'] * 3)
+
+    # Remove one item from the burst.
+    pc.recent_commit_timestamps.pop()
+    pc.scan_results()
+    next_item = pc.MAX_COMMIT_BURST
+    self.context.checkout.check_calls(
+        self._prepare_apply_commit(next_item, next_item, False, True))
+    self.context.rietveld.check_calls(
+        [ 'open_issue(%d)' % next_item,
+          "update_description(%d, u'foo')" % next_item,
+          "add_comment(%d, 'Change reverted as 125')" % next_item,
+        ])
+    self.context.status.check_names(['why not', 'commit'] + ['why not'] * 2)
+    # After a delay, must flush the queue.
+    timestamp.append(timestamp[-1] + pc.COMMIT_BURST_DELAY + 1)
+    pc.scan_results()
+    self.context.checkout.check_calls(
+        self._prepare_apply_commit(next_item + 1, next_item + 1, False, True) +
+        self._prepare_apply_commit(next_item + 2, issue, False, False))
+    self.context.rietveld.check_calls(
+        [ 'open_issue(%d)' % (next_item + 1),
+          "update_description(%d, u'foo')" % (next_item + 1),
+          "add_comment(%d, 'Change reverted as 125')" % (next_item + 1),
+          'close_issue(%d)' % issue,
+          "update_description(%d, u'foo')" % issue,
+          "add_comment(%d, 'Change committed as 125')" % issue])
+    self.context.status.check_names(['why not', 'commit'] * 2)
+
   def testIgnored(self):
     issue = 31337
     verifiers = [