Use author as determined from scm if we can not get it from rietveld

Until an issue is uploaded to Rietveld, we don't know the official
email address to use for an owners check. There are three ways to fix
this: we could attempt to log in to rietveld prior to doing the check
and extract the address to use, or we could use ~/.last_codereview_email_address,
or we can use the email address we can determine from the checkout.

All three options have flaws; the first is particularly awkward since there
doesn't seem to be a good way to fetch the email without posting an issue.
The second is flawed if we use different addresses for different repos,
and the third is flawed if the checkout's email address is different from
the rietveld address, or if it is anonymous.

However, since this is only being used for owners checks (in this case),
anonymous checkouts probably don't matter, and hopefully the cases where
the email addresses differ are rare.

R=maruel@chromium.org
BUG=118388, 150049


Review URL: https://chromiumcodereview.appspot.com/12377023

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

presubmit_canned_checks.py

@@ -793,25 +793,19 @@ def CheckOwners(input_api, output_api, source_file_filter=None,
       owners_db.email_regexp,
       approval_needed=input_api.is_committing)
 
-  if author_counts_as_owner:
-    if owner_email:
-      message = ''
-      reviewers_plus_owner = reviewers.union(set([owner_email]))
-    else:
-      message = ('\nUntil the issue is uploaded, this list will include '
-                 'files for which you are an OWNER.')
-      owner_email = ''
-      reviewers_plus_owner = set()
+  owner_email = owner_email or input_api.change.author_email
+
+  if author_counts_as_owner and owner_email:
+    reviewers_plus_owner = set([owner_email]).union(reviewers or set())
     missing_files = owners_db.files_not_covered_by(affected_files,
-                                                   reviewers_plus_owner)
+        reviewers_plus_owner)
   else:
-    message = ''
     missing_files = owners_db.files_not_covered_by(affected_files, reviewers)
 
   if missing_files:
     output_list = [
-        output('Missing %s for these files:\n    %s%s' %
-               (needed, '\n    '.join(missing_files), message))]
+        output('Missing %s for these files:\n    %s' %
+               (needed, '\n    '.join(missing_files)))]
     if not input_api.is_committing:
       suggested_owners = owners_db.reviewers_for(affected_files, owner_email)
       output_list.append(output('Suggested OWNERS:\n    %s' %

tests/presubmit_unittest.py

@@ -2233,6 +2233,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
 
     change = self.mox.CreateMock(presubmit.Change)
     change.issue = issue
+    change.author_email = 'john@example.com'
     affected_file = self.mox.CreateMock(presubmit.SvnAffectedFile)
     input_api = self.MockInputApi(change, False)
     fake_db = self.mox.CreateMock(owners.Database)
@@ -2244,10 +2245,9 @@ class CannedChecksUnittest(PresubmitTestsBase):
     if not is_committing or (not tbr and issue):
       affected_file.LocalPath().AndReturn('foo/xyz.cc')
       change.AffectedFiles(file_filter=None).AndReturn([affected_file])
-      owner_email = 'john@example.com'
       if not rietveld_response:
         rietveld_response = {
-          "owner_email": owner_email,
+          "owner_email": change.author_email,
           "messages": [
             {"sender": a, "text": "I approve", "approval": True}
             for a in approvers
@@ -2264,17 +2264,18 @@ class CannedChecksUnittest(PresubmitTestsBase):
         input_api.rietveld.get_issue_properties(
             issue=int(input_api.change.issue), messages=True).AndReturn(
                 rietveld_response)
-        people.add(owner_email)
 
       if author_counts_as_owner:
+        people.add(change.author_email)
         fake_db.files_not_covered_by(set(['foo/xyz.cc']),
             people).AndReturn(uncovered_files)
       else:
+        people.discard(change.author_email)
         fake_db.files_not_covered_by(set(['foo/xyz.cc']),
-            people.difference(set([owner_email]))).AndReturn(uncovered_files)
+            people).AndReturn(uncovered_files)
       if not is_committing and uncovered_files:
         fake_db.reviewers_for(set(['foo/xyz.cc']),
-            owner_email if issue else '').AndReturn(owner_email)
+            change.author_email).AndReturn(change.author_email)
 
     self.mox.ReplayAll()
     output = presubmit.PresubmitOutput()
@@ -2367,9 +2368,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
         is_committing=False,
         uncovered_files=set(['foo']),
         expected_output='Missing OWNER reviewers for these files:\n'
-                        '    foo\n'
-                        'Until the issue is uploaded, this list will include '
-                        'files for which you are an OWNER.\n')
+                        '    foo\n')
 
   def testCannedCheckOwners_NoLGTM(self):
     self.AssertOwnersWorks(expected_output='Missing LGTM from someone '