For determining OWNERS, drop reviewers specified via '-r' that aren't in email address format.

BUG=none
R=agable@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@262641 0039d316-1c4b-4281-b951-d872f2087c98

presubmit_canned_checks.py

@@ -859,7 +859,9 @@ def _ReviewersFromChange(change):
     reviewers.update(set([r.strip() for r in change.R.split(',')]))
   if change.TBR:
     reviewers.update(set([r.strip() for r in change.TBR.split(',')]))
-  return reviewers
+
+  # Drop reviewers that aren't specified in email address format.
+  return set(reviewer for reviewer in reviewers if '@' in reviewer)
 
 
 def _RietveldOwnerAndReviewers(input_api, email_regexp, approval_needed=False):

tests/presubmit_unittest.py

@@ -2572,7 +2572,8 @@ class CannedChecksUnittest(PresubmitTestsBase):
 
   def AssertOwnersWorks(self, tbr=False, issue='1', approvers=None,
       reviewers=None, is_committing=True, rietveld_response=None,
-      uncovered_files=None, expected_output='', author_counts_as_owner=True):
+      uncovered_files=None, expected_output='', author_counts_as_owner=True,
+      manually_specified_reviewers=None):
     if approvers is None:
       # The set of people who lgtm'ed a change.
       approvers = set()
@@ -2583,11 +2584,13 @@ class CannedChecksUnittest(PresubmitTestsBase):
       reviewers = approvers
     if uncovered_files is None:
       uncovered_files = set()
+    if manually_specified_reviewers is None:
+      manually_specified_reviewers = []
 
     change = self.mox.CreateMock(presubmit.Change)
     change.issue = issue
     change.author_email = 'john@example.com'
-    change.R = ''
+    change.R = ','.join(manually_specified_reviewers)
     change.TBR = ''
     affected_file = self.mox.CreateMock(presubmit.SvnAffectedFile)
     input_api = self.MockInputApi(change, False)
@@ -2614,7 +2617,6 @@ class CannedChecksUnittest(PresubmitTestsBase):
         people = approvers
       else:
         people = reviewers
-        change.R = ','.join(reviewers)
 
       if issue:
         input_api.rietveld.get_issue_properties(
@@ -2729,13 +2731,28 @@ class CannedChecksUnittest(PresubmitTestsBase):
   def testCannedCheckOwners_NoIssueLocalReviewers(self):
     self.AssertOwnersWorks(issue=None,
         reviewers=set(['jane@example.com']),
+        manually_specified_reviewers=['jane@example.com'],
         expected_output="OWNERS check failed: this change has no Rietveld "
                         "issue number, so we can't check it for approvals.\n")
     self.AssertOwnersWorks(issue=None,
         reviewers=set(['jane@example.com']),
+        manually_specified_reviewers=['jane@example.com'],
         is_committing=False,
         expected_output='')
 
+  def testCannedCheckOwners_NoIssueLocalReviewersDontInferEmailDomain(self):
+    self.AssertOwnersWorks(issue=None,
+        reviewers=set(['jane']),
+        manually_specified_reviewers=['jane@example.com'],
+        expected_output="OWNERS check failed: this change has no Rietveld "
+                        "issue number, so we can't check it for approvals.\n")
+    self.AssertOwnersWorks(issue=None,
+        uncovered_files=set(['foo']),
+        manually_specified_reviewers=['jane'],
+        is_committing=False,
+        expected_output='Missing OWNER reviewers for these files:\n'
+                        '    foo\n')
+
   def testCannedCheckOwners_NoLGTM(self):
     self.AssertOwnersWorks(expected_output='Missing LGTM from someone '
                                            'other than john@example.com\n')