Bug 1005920 - Make the prevent_webidl_changes hook ignore backouts; r=ted
authorEhsan Akhgari <ehsan@mozilla.com>
Mon, 05 May 2014 11:23:12 -0400
changeset 188 94e6623eb57646b4e7fd4c13b1eb520ffbe2c956
parent 187 6a34259d022e4d301b7b962b997dd095ff1eef44
child 189 2a3754c442aec1ffb6cea57b1e1062abf43ffd29
push id116
push usereakhgari@mozilla.com
push dateTue, 06 May 2014 00:34:20 +0000
reviewersted
bugs1005920
Bug 1005920 - Make the prevent_webidl_changes hook ignore backouts; r=ted
mozhghooks/prevent_webidl_changes.py
runtests.py
--- a/mozhghooks/prevent_webidl_changes.py
+++ b/mozhghooks/prevent_webidl_changes.py
@@ -16,16 +16,28 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 """
 This hook is to prevent changes to .webidl files in pushes without proper DOM peer review.
 """
 
 import re
 from mercurial.node import short
 
+# Stolen from commit-message.py
+backoutMessage = [re.compile(x) for x in [
+    r'^(back(ing|ed)?\s+out|backout).*(\s+|\:)[0-9a-f]{12}',
+    r'^(revert(ed|ing)?).*(\s+|\:)[0-9a-f]{12}'
+]]
+
+def isBackout(message):
+    for r in backoutMessage:
+        if r.search(message):
+            return True
+    return False
+
 def hook(ui, repo, hooktype, node, **kwargs):
     DOM_peers = [
         'jst',              # Johnny Stenback
         'peterv',           # Peter Van der Beken
         'bz', 'bzbarsky',   # Boris Zbarsky
         'sicking', 'jonas', # Jonas Sicking
         'smaug',            # Olli Pettay
         'bent',             # Ben Turner
@@ -46,25 +58,26 @@ def hook(ui, repo, hooktype, node, **kwa
         if len(c.parents()) > 1:
             # Skip merge changesets
             continue
 
         # Loop through each file for the current changeset
         for file in c.files():
             # Only Check WebIDL Files
             if file.endswith('.webidl'):
-                webidlReviewed = True
-                match = re.search('\Wr\s*=\s*(\w+(?:,\w+)*)', c.description().lower())
+                message = c.description().lower()
+                match = re.search('\Wr\s*=\s*(\w+(?:,\w+)*)', message)
                 validReview = False
                 if match:
                     for reviewer in match.group(1).split(','):
                         if reviewer in DOM_peers:
                             validReview = True
                             break
-                if not validReview:
+                webidlReviewed = validReview
+                if not validReview and not isBackout(message):
                         error += "WebIDL file %s altered in changeset %s without DOM peer review\n" % (file, short(c.node()))
     # Check if an error occured in any of the files that were changed
     if error != "":
         print "\n\n************************** ERROR ****************************"
         ui.warn("\n" + error + "\n")
         print "\n\rChanges to WebIDL files in this repo require review from a DOM peer in the form of r=...\n\rThis is to ensure that we behave responsibly with exposing new Web APIs. We appreciate your understanding..\n\r"
         print "*************************************************************\n\n"
         # Reject the changesets
--- a/runtests.py
+++ b/runtests.py
@@ -920,10 +920,30 @@ class TestPreventWebIDLHook(unittest.Tes
     update(u, self.clonerepo, rev=parentrev)
     editFile(join(self.clonedir, "dummy"), "foo", "bar")
     commit(u, self.clonerepo, message="dummy")
     merge(u, self.clonerepo)
     commit(u, self.clonerepo, message="merge")
     result = push(u, self.clonerepo, dest=self.repodir)
     self.assertEqual(result, 0)
 
+  def testWebIDLEditsInBackoutsWithoutProperReviewShouldPass(self):
+    """ Test that editing .webidl file without proper DOM peer review in backouts should pass """
+    u = self.ui
+    # Copied from testBackout above.
+    messages = [
+      "Backed out changeset 593d94e9492e",
+      "Backout changesets 9e4ab3907b29, 3abc0dbbf710 due to m-oth permaorange",
+      "Backout of 35a679df430b due to bustage",
+      "backout 68941:5b8ade677818", # including the local numeric ID is silly but harmless
+      # we do not have a lot of reverts "hg log | grep revert" without a bug #
+      "Revert to changeset a87ee7550f6a due to incomplete backout" 
+    ]
+    for message in messages:
+      name = "new%d.webidl" % len(message)
+      appendFile(join(self.clonedir, name), "interface Test{};")
+      add(u, self.clonerepo, join(self.clonedir, name))
+      commit(u, self.clonerepo, message=message)
+      result = push(u, self.clonerepo, dest=self.repodir)
+      self.assertEqual(result, 0)
+
 if __name__ == '__main__':
   unittest.main()