improve logging slightly

Review URL: http://codereview.chromium.org/6736018

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

git_cl/git_cl.py

@@ -840,18 +840,10 @@ def CMDpresubmit(parser, args):
     # Default to diffing against the "upstream" branch.
     base_branch = cl.GetUpstreamBranch()
 
-  if options.upload:
-    print '*** Presubmit checks for UPLOAD would report: ***'
-    RunHook(committing=False, upstream_branch=base_branch,
-            rietveld_server=cl.GetRietveldServer(), tbr=False,
-            may_prompt=False)
-    return 0
-  else:
-    print '*** Presubmit checks for DCOMMIT would report: ***'
-    RunHook(committing=True, upstream_branch=base_branch,
-            rietveld_server=cl.GetRietveldServer, tbr=False,
-            may_prompt=False)
-    return 0
+  RunHook(committing=not options.upload, upstream_branch=base_branch,
+          rietveld_server=cl.GetRietveldServer(), tbr=False,
+          may_prompt=False)
+  return 0
 
 
 @usage('[args to "git diff"]')

presubmit_canned_checks.py

@@ -631,10 +631,12 @@ def CheckOwners(input_api, output_api, source_file_filter=None):
   if not input_api.is_committing:
     return []
   if input_api.tbr:
-    return [output_api.PresubmitNotifyResult('--tbr was specified, '
-                                             'skipping OWNERS check')]
+    return [output_api.PresubmitNotifyResult(
+        '--tbr was specified, skipping OWNERS check')]
   if not input_api.change.issue:
-    return [output_api.PresubmitError('Change not uploaded for review')]
+    return [output_api.PresubmitError(
+        "OWNERS check failed: this change has no Rietveld issue number, so "
+        "we can't check it for approvals.")]
 
   affected_files = set([f.LocalPath() for f in
       input_api.change.AffectedFiles(source_file_filter)])

presubmit_support.py

@@ -1040,7 +1040,10 @@ def DoPresubmitChecks(change,
     if there were errors or warnings and the caller should abort.
   """
   output = PresubmitOutput(input_stream, output_stream)
-  output.write("Running presubmit hooks...\n")
+  if committing:
+    output.write("Running presubmit commit checks ...\n")
+  else:
+    output.write("Running presubmit upload checks ...\n")
   start_time = time.time()
   presubmit_files = ListRelevantPresubmitFiles(change.AbsoluteLocalPaths(True),
                                                change.RepositoryRoot())
@@ -1072,6 +1075,7 @@ def DoPresubmitChecks(change,
     else:
       notifications.append(result)
 
+  output.write('\n')
   for name, items in (('Messages', notifications),
                       ('Warnings', warnings),
                       ('ERRORS', errors)):
@@ -1083,10 +1087,12 @@ def DoPresubmitChecks(change,
 
   total_time = time.time() - start_time
   if total_time > 1.0:
-    output.write("Presubmit checks took %.1fs to calculate.\n" % total_time)
+    output.write("Presubmit checks took %.1fs to calculate.\n\n" % total_time)
 
-  if not errors and warnings:
-    if may_prompt:
+  if not errors:
+    if not warnings:
+      output.write('Presubmit checks passed.\n')
+    elif may_prompt:
       output.prompt_yes_no('There were presubmit warnings. '
                           'Are you sure you wish to continue? (y/N): ')
     else:

tests/presubmit_unittest.py

@@ -420,7 +420,8 @@ class PresubmitUnittest(PresubmitTestsBase):
         change, False, True, None, input_buf, None, False)
     self.failIf(output.should_continue())
     self.assertEqual(output.getvalue().count('!!'), 2)
-    self.assertEqual(output.getvalue().count('Running presubmit hooks...\n'), 1)
+    self.assertEqual(output.getvalue().count(
+        'Running presubmit upload checks ...\n'), 1)
 
   def testDoPresubmitChecksPromptsAfterWarnings(self):
     join = presubmit.os.path.join
@@ -459,7 +460,8 @@ class PresubmitUnittest(PresubmitTestsBase):
         change, False, True, None, input_buf, None, True)
     self.failUnless(output.should_continue())
     self.assertEquals(output.getvalue().count('??'), 2)
-    self.assertEqual(output.getvalue().count('Running presubmit hooks...\n'), 1)
+    self.assertEqual(output.getvalue().count(
+        'Running presubmit upload checks ...\n'), 1)
 
   def testDoPresubmitChecksNoWarningPromptIfErrors(self):
     join = presubmit.os.path.join
@@ -492,7 +494,8 @@ class PresubmitUnittest(PresubmitTestsBase):
     self.assertEqual(output.getvalue().count('??'), 2)
     self.assertEqual(output.getvalue().count('XX!!XX'), 2)
     self.assertEqual(output.getvalue().count('(y/N)'), 0)
-    self.assertEqual(output.getvalue().count('Running presubmit hooks...\n'), 1)
+    self.assertEqual(output.getvalue().count(
+        'Running presubmit upload checks ...\n'), 1)
 
   def testDoDefaultPresubmitChecksAndFeedback(self):
     join = presubmit.os.path.join
@@ -526,9 +529,10 @@ def CheckChangeOnCommit(input_api, output_api):
     output = presubmit.DoPresubmitChecks(
         change, False, True, None, input_buf, DEFAULT_SCRIPT, False)
     self.failIf(output.should_continue())
-    text = ('Running presubmit hooks...\n'
+    text = ('Running presubmit upload checks ...\n'
             'Warning, no presubmit.py found.\n'
             'Running default presubmit script.\n'
+            '\n'
             '** Presubmit ERRORS **\n!!\n\n'
             'Was the presubmit check useful? Please send feedback & hate mail '
             'to maruel@chromium.org!\n')
@@ -600,11 +604,14 @@ def CheckChangeOnCommit(input_api, output_api):
     self.failUnless(presubmit.DoPresubmitChecks(
         change, False, True, output, input_buf, DEFAULT_SCRIPT, False))
     self.assertEquals(output.getvalue(),
-                      ('Running presubmit hooks...\n'
+                      ('Running presubmit upload checks ...\n'
                        'Warning, no presubmit.py found.\n'
                        'Running default presubmit script.\n'
+                       '\n'
                        '** Presubmit Messages **\n'
-                       'http://tracker.com/42\n\n'))
+                       'http://tracker.com/42\n'
+                       '\n'
+                       'Presubmit checks passed.\n'))
 
   def testGetTrySlavesExecuter(self):
     self.mox.ReplayAll()
@@ -1971,7 +1978,8 @@ mac|success|blew
 
   def testCannedCheckOwners_NoIssue(self):
     self.AssertOwnersWorks(issue=None,
-                           expected_output='Change not uploaded for review\n')
+        expected_output="OWNERS check failed: this change has no Rietveld "
+                        "issue number, so we can't check it for approvals.\n")
 
   def testCannedCheckOwners_NoLGTM(self):
     self.AssertOwnersWorks(expected_output='Missing LGTM from someone '