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
diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py
index a48a0c6..878ee70 100644
--- a/presubmit_canned_checks.py
+++ b/presubmit_canned_checks.py
@@ -859,7 +859,9 @@
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):
diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py
index 4658a69..d97e7fa 100755
--- a/tests/presubmit_unittest.py
+++ b/tests/presubmit_unittest.py
@@ -2572,7 +2572,8 @@
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 @@
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 @@
people = approvers
else:
people = reviewers
- change.R = ','.join(reviewers)
if issue:
input_api.rietveld.get_issue_properties(
@@ -2729,13 +2731,28 @@
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')
