Bug 1007235 - Search for all occurrences of the reviewers regex in prevent_webidl_hooks, not just the first one; r=ted
authorEhsan Akhgari <ehsan@mozilla.com>
Wed, 07 May 2014 14:56:45 -0400
changeset 191 6f53da7dc8d3904653342d7a578fe4c28b991ca0
parent 190 2d54ca1669efd8ecb1495b3cfeaa3f16702fdcd4
child 192 b8bb1b75f0725bf39651927628900ece1c07f2ab
push id119
push usereakhgari@mozilla.com
push dateWed, 07 May 2014 19:26:16 +0000
reviewersted
bugs1007235
Bug 1007235 - Search for all occurrences of the reviewers regex in prevent_webidl_hooks, not just the first one; r=ted
mozhghooks/prevent_webidl_changes.py
runtests.py
--- a/mozhghooks/prevent_webidl_changes.py
+++ b/mozhghooks/prevent_webidl_changes.py
@@ -59,25 +59,25 @@ def hook(ui, repo, hooktype, node, **kwa
             # 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'):
                 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
-                webidlReviewed = validReview
-                if not validReview and not isBackout(message):
+                def search():
+                  matches = re.findall('\Wr\s*=\s*(\w+(?:,\w+)*)', message)
+                  for match in matches:
+                      for reviewer in match.split(','):
+                          if reviewer in DOM_peers:
+                              return True
+                  return False
+                webidlReviewed = search()
+                if not webidlReviewed 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
@@ -910,16 +910,24 @@ class TestPreventWebIDLHook(unittest.Tes
 
   def testWebIDLEditWithProperReviewShouldPass(self):
     """ Test that editing .webidl file with proper DOM peer review should pass """
     u = self.ui
     editFile(join(self.clonedir, "original.webidl"), "interface Foo{};", "interface Bar{};")
     commit(u, self.clonerepo, message="checkin 1 bug 12345; r=foobar,jst")
     result = push(u, self.clonerepo, dest=self.repodir)
     self.assertEqual(result, 0)
+    editFile(join(self.clonedir, "original.webidl"), "interface Bar{};", "interface Baz{};")
+    commit(u, self.clonerepo, message="checkin 1 bug 67890; r=foobar r=jst")
+    result = push(u, self.clonerepo, dest=self.repodir)
+    self.assertEqual(result, 0)
+    editFile(join(self.clonedir, "original.webidl"), "interface Baz{};", "interface Bizarre{};")
+    commit(u, self.clonerepo, message="checkin 1 bug 123456; r=foobar r=lumpy,jst")
+    result = push(u, self.clonerepo, dest=self.repodir)
+    self.assertEqual(result, 0)
 
   def testWebIDLEditWithProperReviewDuringMergeShouldPass(self):
     """ Test that editing .webidl file with proper DOM peer review should pass """
     u = self.ui
     parentrev = short(self.clonerepo.changectx('tip').node())
     editFile(join(self.clonedir, "original.webidl"), "interface Foo{};", "interface Bar{};")
     commit(u, self.clonerepo, message="checkin 1 bug 12345; r=foobar,jst")
     update(u, self.clonerepo, rev=parentrev)