Output missing LGTMs notification also on dry-run

On larger CLs, this can be used to see whether all needed LGTMs are
there on a dry run.

R=tandrii@chromium.org

Change-Id: I5fe4022feaca73d08ead9aed14ca270192740675
Reviewed-on: https://chromium-review.googlesource.com/426819
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: smut <smut@chromium.org>

presubmit_canned_checks.py

@@ -858,18 +858,19 @@ def CheckOwners(input_api, output_api, source_file_filter=None):
     if input_api.tbr:
       return [output_api.PresubmitNotifyResult(
           '--tbr was specified, skipping OWNERS check')]
+    needed = 'LGTM from an OWNER'
+    output_fn = output_api.PresubmitError
     if input_api.change.issue:
       if input_api.dry_run:
-        return [output_api.PresubmitNotifyResult(
+        output_fn = lambda text: output_api.PresubmitNotifyResult(
-            'This is a dry run, skipping OWNERS check')]
+            'This is a dry run, but these failures would be reported on ' +
+            'commit:\n' + text)
     else:
       return [output_api.PresubmitError("OWNERS check failed: this change has "
           "no Rietveld issue number, so we can't check it for approvals.")]
-    needed = 'LGTM from an OWNER'
-    output = output_api.PresubmitError
   else:
     needed = 'OWNER reviewers'
-    output = output_api.PresubmitNotifyResult
+    output_fn = output_api.PresubmitNotifyResult
 
   affected_files = set([f.LocalPath() for f in
       input_api.AffectedFiles(file_filter=source_file_filter)])
@@ -891,17 +892,17 @@ def CheckOwners(input_api, output_api, source_file_filter=None):
 
   if missing_files:
     output_list = [
-        output('Missing %s for these files:\n    %s' %
+        output_fn('Missing %s for these files:\n    %s' %
-               (needed, '\n    '.join(sorted(missing_files))))]
+                  (needed, '\n    '.join(sorted(missing_files))))]
     if not input_api.is_committing:
       suggested_owners = owners_db.reviewers_for(missing_files, owner_email)
-      output_list.append(output('Suggested OWNERS: ' +
+      output_list.append(output_fn('Suggested OWNERS: ' +
           '(Use "git-cl owners" to interactively select owners.)\n    %s' %
           ('\n    '.join(suggested_owners or []))))
     return output_list
 
   if input_api.is_committing and not reviewers:
-    return [output('Missing LGTM from someone other than %s' % owner_email)]
+    return [output_fn('Missing LGTM from someone other than %s' % owner_email)]
   return []
 
 def GetCodereviewOwnerAndReviewers(input_api, email_regexp, approval_needed):

tests/presubmit_unittest.py

@@ -2194,9 +2194,8 @@ class CannedChecksUnittest(PresubmitTestsBase):
     input_api.dry_run = dry_run
 
     if not is_committing or (not tbr and issue):
-      if not dry_run:
+      affected_file.LocalPath().AndReturn('foo/xyz.cc')
-        affected_file.LocalPath().AndReturn('foo/xyz.cc')
+      input_api.AffectedFiles(file_filter=None).AndReturn([affected_file])
-        input_api.AffectedFiles(file_filter=None).AndReturn([affected_file])
       if issue and not rietveld_response and not gerrit_response:
         rietveld_response = {
           "owner_email": change.author_email,
@@ -2212,21 +2211,20 @@ class CannedChecksUnittest(PresubmitTestsBase):
       else:
         people = reviewers
 
-      if not dry_run:
+      if issue:
-        if issue:
+        if rietveld_response:
-          if rietveld_response:
+          input_api.rietveld.get_issue_properties(
-            input_api.rietveld.get_issue_properties(
+              issue=int(input_api.change.issue), messages=True).AndReturn(
-                issue=int(input_api.change.issue), messages=True).AndReturn(
+                  rietveld_response)
-                    rietveld_response)
+        elif gerrit_response:
-          elif gerrit_response:
+          input_api.gerrit._FetchChangeDetail = lambda _: gerrit_response
-            input_api.gerrit._FetchChangeDetail = lambda _: gerrit_response
+
-
+      people.add(change.author_email)
-        people.add(change.author_email)
+      fake_db.files_not_covered_by(set(['foo/xyz.cc']),
-        fake_db.files_not_covered_by(set(['foo/xyz.cc']),
+          people).AndReturn(uncovered_files)
-            people).AndReturn(uncovered_files)
+      if not is_committing and uncovered_files:
-        if not is_committing and uncovered_files:
+        fake_db.reviewers_for(set(['foo']),
-          fake_db.reviewers_for(set(['foo']),
+            change.author_email).AndReturn(change.author_email)
-              change.author_email).AndReturn(change.author_email)
 
     self.mox.ReplayAll()
     output = presubmit.PresubmitOutput()
@@ -2245,7 +2243,9 @@ class CannedChecksUnittest(PresubmitTestsBase):
         dry_run=True,
         rietveld_response=response,
         reviewers=set(["ben@example.com"]),
-        expected_output='This is a dry run, skipping OWNERS check\n')
+        expected_output='This is a dry run, but these failures would be ' +
+                        'reported on commit:\nMissing LGTM from someone ' +
+                        'other than john@example.com\n')
 
     self.AssertOwnersWorks(approvers=set(['ben@example.com']),
         is_committing=False,