diff --git a/gcl.py b/gcl.py index e0d410c2b..1f1c2c307 100755 --- a/gcl.py +++ b/gcl.py @@ -1169,16 +1169,19 @@ def DoPresubmitChecks(change_info, committing, may_prompt): change_info.GetFiles(), change_info.issue, change_info.patchset) - result = presubmit_support.DoPresubmitChecks(change=change, + output = presubmit_support.DoPresubmitChecks(change=change, committing=committing, verbose=False, output_stream=sys.stdout, input_stream=sys.stdin, default_presubmit=root_presubmit, may_prompt=may_prompt) - if not result and may_prompt: + if not output.should_continue() and may_prompt: + # TODO(dpranke): move into DoPresubmitChecks(), unify cmd line args. print "\nPresubmit errors, can't continue (use --no_presubmit to bypass)" - return result + + # TODO(dpranke): Return the output object and make use of it. + return output.should_continue() @no_args diff --git a/git_cl/git_cl.py b/git_cl/git_cl.py index aaa2670e4..b43f7c835 100644 --- a/git_cl/git_cl.py +++ b/git_cl/git_cl.py @@ -482,29 +482,6 @@ def GetCodereviewSettingsInteractively(): # svn-based hackery. -class HookResults(object): - """Contains the parsed output of the presubmit hooks.""" - def __init__(self, output_from_hooks=None): - self.reviewers = [] - self.output = None - self._ParseOutputFromHooks(output_from_hooks) - - def _ParseOutputFromHooks(self, output_from_hooks): - if not output_from_hooks: - return - lines = [] - reviewers = [] - reviewer_regexp = re.compile('ADD: R=(.+)') - for l in output_from_hooks.splitlines(): - m = reviewer_regexp.match(l) - if m: - reviewers.extend(m.group(1).split(',')) - else: - lines.append(l) - self.output = '\n'.join(lines) - self.reviewers = ','.join(reviewers) - - class ChangeDescription(object): """Contains a parsed form of the change description.""" def __init__(self, subject, log_desc, reviewers): @@ -772,31 +749,16 @@ def RunHook(committing, upstream_branch, rietveld_server, tbr, may_prompt): RunCommand(['git', 'config', '--replace-all', 'rietveld.extracc', ','.join(watchers)]) - output = StringIO.StringIO() - should_continue = presubmit_support.DoPresubmitChecks(change, committing, - verbose=None, output_stream=output, input_stream=sys.stdin, - default_presubmit=None, may_prompt=False, tbr=tbr, + output = presubmit_support.DoPresubmitChecks(change, committing, + verbose=False, output_stream=sys.stdout, input_stream=sys.stdin, + default_presubmit=None, may_prompt=may_prompt, tbr=tbr, host_url=cl.GetRietveldServer()) - hook_results = HookResults(output.getvalue()) - if hook_results.output: - print hook_results.output # TODO(dpranke): We should propagate the error out instead of calling exit(). - if should_continue and hook_results.output and ( - '** Presubmit ERRORS **\n' in hook_results.output or - '** Presubmit WARNINGS **\n' in hook_results.output): - should_continue = False - - if not should_continue: - if may_prompt: - response = raw_input('Are you sure you want to continue? (y/N): ') - if not response.lower().startswith('y'): - sys.exit(1) - else: - sys.exit(1) - + if not output.should_continue(): + sys.exit(1) - return hook_results + return output def CMDpresubmit(parser, args): @@ -870,15 +832,13 @@ def CMDupload(parser, args): base_branch = cl.GetUpstreamBranch() args = [base_branch + "..."] - if not options.bypass_hooks: + if not options.bypass_hooks and not options.force: hook_results = RunHook(committing=False, upstream_branch=base_branch, rietveld_server=cl.GetRietveldServer(), tbr=False, - may_prompt=(not options.force)) - else: - hook_results = HookResults() + may_prompt=True) + if not options.reviewers and hook_results.reviewers: + options.reviewers = hook_results.reviewers - if not options.reviewers and hook_results.reviewers: - options.reviewers = hook_results.reviewers # --no-ext-diff is broken in some versions of Git, so try to work around # this by overriding the environment (but there is still a problem if the @@ -1023,12 +983,11 @@ def SendUpstream(parser, args, cmd): 'before attempting to %s.' % (base_branch, cmd)) return 1 - if not options.bypass_hooks: + if not options.bypass_hooks and not options.force: RunHook(committing=True, upstream_branch=base_branch, rietveld_server=cl.GetRietveldServer(), tbr=options.tbr, - may_prompt=(not options.force)) + may_prompt=True) - if not options.force and not options.bypass_hooks: if cmd == 'dcommit': # Check the tree status if the tree status URL is set. status = GetTreeStatus() diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index ae6ec9760..0a78af3c5 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -648,7 +648,7 @@ def CheckOwners(input_api, output_api, email_regexp=None, return [] suggested_reviewers = owners_db.reviewers_for(affected_files) - return [output_api.PresubmitAddText('R=%s' % ','.join(suggested_reviewers))] + return [output_api.PresubmitAddReviewers(suggested_reviewers)] def _Approvers(input_api, email_regexp): diff --git a/presubmit_support.py b/presubmit_support.py index fc1c7f23b..7ee2b0d13 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -78,12 +78,6 @@ def normpath(path): return os.path.normpath(path) -def PromptYesNo(input_stream, output_stream, prompt): - output_stream.write(prompt) - response = input_stream.readline().strip().lower() - return response == 'y' or response == 'yes' - - def _RightHandSideLinesImpl(affected_files): """Implements RightHandSideLines for InputApi and GclChange.""" for af in affected_files: @@ -92,14 +86,46 @@ def _RightHandSideLinesImpl(affected_files): yield (af, line[0], line[1]) +class PresubmitOutput(object): + def __init__(self, input_stream=None, output_stream=None): + self.input_stream = input_stream + self.output_stream = output_stream + self.reviewers = [] + self.written_output = [] + self.error_count = 0 + + def prompt_yes_no(self, prompt_string): + self.write(prompt_string) + if self.input_stream: + response = self.input_stream.readline().strip().lower() + if response not in ('y', 'yes'): + self.fail() + else: + self.fail() + + def fail(self): + self.error_count += 1 + + def should_continue(self): + return not self.error_count + + def write(self, s): + self.written_output.append(s) + if self.output_stream: + self.output_stream.write(s) + + def getvalue(self): + return ''.join(self.written_output) + + class OutputApi(object): """This class (more like a module) gets passed to presubmit scripts so that they can specify various types of results. """ - # Method could be a function - # pylint: disable=R0201 class PresubmitResult(object): """Base class for result objects.""" + fatal = False + should_prompt = False def __init__(self, message, items=None, long_text=''): """ @@ -113,19 +139,11 @@ class OutputApi(object): self._items = items self._long_text = long_text.rstrip() - def _Handle(self, output_stream, input_stream, may_prompt=True): - """Writes this result to the output stream. - - Args: - output_stream: Where to write - - Returns: - True if execution may continue, False otherwise. - """ - output_stream.write(self._message) - output_stream.write('\n') + def handle(self, output): + output.write(self._message) + output.write('\n') if len(self._items) > 0: - output_stream.write(' ' + ' \\\n '.join(map(str, self._items)) + '\n') + output.write(' ' + ' \\\n '.join(map(str, self._items)) + '\n') if self._long_text: # Sometimes self._long_text is a ascii string, a codepage string # (on windows), or a unicode object. @@ -134,41 +152,27 @@ class OutputApi(object): except UnicodeDecodeError: long_text = self._long_text.decode('ascii', 'replace') - output_stream.write('\n***************\n%s\n***************\n' % + output.write('\n***************\n%s\n***************\n' % long_text) + if self.fatal: + output.fail() - if self.ShouldPrompt() and may_prompt: - if not PromptYesNo(input_stream, output_stream, - 'Are you sure you want to continue? (y/N): '): - return False + class PresubmitAddReviewers(PresubmitResult): + """Add some suggested reviewers to the change.""" + def __init__(self, reviewers): + super(OutputApi.PresubmitAddReviewers, self).__init__('') + self.reviewers = reviewers - return not self.IsFatal() - - def IsFatal(self): - """An error that is fatal stops g4 mail/submit immediately, i.e. before - other presubmit scripts are run. - """ - return False - - def ShouldPrompt(self): - """Whether this presubmit result should result in a prompt warning.""" - return False - - class PresubmitAddText(PresubmitResult): - """Propagates a line of text back to the caller.""" - def __init__(self, message, items=None, long_text=''): - super(OutputApi.PresubmitAddText, self).__init__("ADD: " + message, - items, long_text) + def handle(self, output): + output.reviewers.extend(self.reviewers) class PresubmitError(PresubmitResult): """A hard presubmit error.""" - def IsFatal(self): - return True + fatal = True class PresubmitPromptWarning(PresubmitResult): """An warning that prompts the user if they want to continue.""" - def ShouldPrompt(self): - return True + should_prompt = True class PresubmitNotifyResult(PresubmitResult): """Just print something to the screen -- but it's not even a warning.""" @@ -993,6 +997,7 @@ class PresubmitExecuter(object): os.chdir(main_path) return result + # TODO(dpranke): make all callers pass in tbr, host_url? def DoPresubmitChecks(change, committing, @@ -1029,25 +1034,27 @@ def DoPresubmitChecks(change, SHOULD be sys.stdin. Return: - True if execution can continue, False if not. + A PresubmitOutput object. Use output.should_continue() to figure out + if there were errors or warnings and the caller should abort. """ - print "Running presubmit hooks..." + output = PresubmitOutput(input_stream, output_stream) + output.write("Running presubmit hooks...\n") start_time = time.time() presubmit_files = ListRelevantPresubmitFiles(change.AbsoluteLocalPaths(True), change.RepositoryRoot()) if not presubmit_files and verbose: - output_stream.write("Warning, no presubmit.py found.\n") + output.write("Warning, no presubmit.py found.\n") results = [] executer = PresubmitExecuter(change, committing, tbr, host_url) if default_presubmit: if verbose: - output_stream.write("Running default presubmit script.\n") + output.write("Running default presubmit script.\n") fake_path = os.path.join(change.RepositoryRoot(), 'PRESUBMIT.py') results += executer.ExecPresubmitScript(default_presubmit, fake_path) for filename in presubmit_files: filename = os.path.abspath(filename) if verbose: - output_stream.write("Running %s\n" % filename) + output.write("Running %s\n" % filename) # Accept CRLF presubmit script. presubmit_script = gclient_utils.FileRead(filename, 'rU') results += executer.ExecPresubmitScript(presubmit_script, filename) @@ -1056,44 +1063,37 @@ def DoPresubmitChecks(change, notifications = [] warnings = [] for result in results: - if not result.IsFatal() and not result.ShouldPrompt(): - notifications.append(result) - elif result.ShouldPrompt(): + if result.fatal: + errors.append(result) + elif result.should_prompt: warnings.append(result) else: - errors.append(result) + notifications.append(result) - error_count = 0 for name, items in (('Messages', notifications), ('Warnings', warnings), ('ERRORS', errors)): if items: - output_stream.write('** Presubmit %s **\n' % name) + output.write('** Presubmit %s **\n' % name) for item in items: - # Access to a protected member XXX of a client class - # pylint: disable=W0212 - if not item._Handle(output_stream, input_stream, - may_prompt=False): - error_count += 1 - output_stream.write('\n') + item.handle(output) + output.write('\n') total_time = time.time() - start_time if total_time > 1.0: - print "Presubmit checks took %.1fs to calculate." % total_time + output.write("Presubmit checks took %.1fs to calculate.\n" % total_time) if not errors and warnings and may_prompt: - if not PromptYesNo(input_stream, output_stream, - 'There were presubmit warnings. ' - 'Are you sure you wish to continue? (y/N): '): - error_count += 1 + output.prompt_yes_no('There were presubmit warnings. ' + 'Are you sure you wish to continue? (y/N): ') global _ASKED_FOR_FEEDBACK # Ask for feedback one time out of 5. if (len(results) and random.randint(0, 4) == 0 and not _ASKED_FOR_FEEDBACK): - output_stream.write("Was the presubmit check useful? Please send feedback " - "& hate mail to maruel@chromium.org!\n") + output.write("Was the presubmit check useful? Please send feedback " + "& hate mail to maruel@chromium.org!\n") _ASKED_FOR_FEEDBACK = True - return (error_count == 0) + return output def ScanSubDirs(mask, recursive): @@ -1179,18 +1179,19 @@ def Main(argv): print "Found %d files." % len(options.files) else: print "Found 1 file." - return not DoPresubmitChecks(change_class(options.name, - options.description, - options.root, - options.files, - options.issue, - options.patchset), - options.commit, - options.verbose, - sys.stdout, - sys.stdin, - options.default_presubmit, - options.may_prompt) + results = DoPresubmitChecks(change_class(options.name, + options.description, + options.root, + options.files, + options.issue, + options.patchset), + options.commit, + options.verbose, + sys.stdout, + sys.stdin, + options.default_presubmit, + options.may_prompt) + return not results.should_continue() if __name__ == '__main__': diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 8f39b39b9..1d390bd8d 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -138,7 +138,7 @@ class PresubmitUnittest(PresubmitTestsBase): 'GetTrySlavesExecuter', 'GitAffectedFile', 'GitChange', 'InputApi', 'ListRelevantPresubmitFiles', 'Main', 'NotImplementedException', 'OutputApi', 'ParseFiles', - 'PresubmitExecuter', 'PromptYesNo', 'ScanSubDirs', + 'PresubmitExecuter', 'PresubmitOutput', 'ScanSubDirs', 'SvnAffectedFile', 'SvnChange', 'cPickle', 'cStringIO', 'exceptions', 'fnmatch', 'gclient_utils', 'glob', 'json', 'logging', 'marshal', 'normpath', 'optparse', 'os', 'owners', 'pickle', @@ -411,14 +411,14 @@ class PresubmitUnittest(PresubmitTestsBase): presubmit.random.randint(0, 4).AndReturn(1) self.mox.ReplayAll() - output = StringIO.StringIO() input_buf = StringIO.StringIO('y\n') change = presubmit.Change('mychange', '\n'.join(description_lines), self.fake_root_dir, files, 0, 0) - self.failIf(presubmit.DoPresubmitChecks( - change, False, True, output, input_buf, None, False)) + output = presubmit.DoPresubmitChecks( + change, False, True, None, input_buf, None, False) + self.failIf(output.should_continue()) self.assertEqual(output.getvalue().count('!!'), 2) - self.checkstdout('Running presubmit hooks...\n') + self.assertEqual(output.getvalue().count('Running presubmit hooks...\n'), 1) def testDoPresubmitChecksPromptsAfterWarnings(self): join = presubmit.os.path.join @@ -444,20 +444,20 @@ class PresubmitUnittest(PresubmitTestsBase): presubmit.random.randint(0, 4).AndReturn(1) self.mox.ReplayAll() - output = StringIO.StringIO() input_buf = StringIO.StringIO('n\n') # say no to the warning change = presubmit.Change('mychange', '\n'.join(description_lines), self.fake_root_dir, files, 0, 0) - self.failIf(presubmit.DoPresubmitChecks( - change, False, True, output, input_buf, None, True)) + output = presubmit.DoPresubmitChecks( + change, False, True, None, input_buf, None, True) + self.failIf(output.should_continue()) self.assertEqual(output.getvalue().count('??'), 2) - output = StringIO.StringIO() input_buf = StringIO.StringIO('y\n') # say yes to the warning - self.failUnless(presubmit.DoPresubmitChecks( - change, False, True, output, input_buf, None, True)) + output = presubmit.DoPresubmitChecks( + change, False, True, None, input_buf, None, True) + self.failUnless(output.should_continue()) self.assertEquals(output.getvalue().count('??'), 2) - self.checkstdout('Running presubmit hooks...\nRunning presubmit hooks...\n') + self.assertEqual(output.getvalue().count('Running presubmit hooks...\n'), 1) def testDoPresubmitChecksNoWarningPromptIfErrors(self): join = presubmit.os.path.join @@ -483,16 +483,14 @@ class PresubmitUnittest(PresubmitTestsBase): presubmit.random.randint(0, 4).AndReturn(1) self.mox.ReplayAll() - output = StringIO.StringIO() - input_buf = StringIO.StringIO() # should be unused change = presubmit.Change('mychange', '\n'.join(description_lines), self.fake_root_dir, files, 0, 0) - self.failIf(presubmit.DoPresubmitChecks( - change, False, True, output, input_buf, None, False)) + output = presubmit.DoPresubmitChecks(change, False, True, None, None, + None, False) self.assertEqual(output.getvalue().count('??'), 2) self.assertEqual(output.getvalue().count('XX!!XX'), 2) self.assertEqual(output.getvalue().count('(y/N)'), 0) - self.checkstdout('Running presubmit hooks...\n') + self.assertEqual(output.getvalue().count('Running presubmit hooks...\n'), 1) def testDoDefaultPresubmitChecksAndFeedback(self): join = presubmit.os.path.join @@ -519,20 +517,20 @@ def CheckChangeOnCommit(input_api, output_api): presubmit.random.randint(0, 4).AndReturn(0) self.mox.ReplayAll() - output = StringIO.StringIO() input_buf = StringIO.StringIO('y\n') # Always fail. change = presubmit.Change('mychange', '\n'.join(description_lines), self.fake_root_dir, files, 0, 0) - self.failIf(presubmit.DoPresubmitChecks( - change, False, True, output, input_buf, DEFAULT_SCRIPT, False)) - text = ('Warning, no presubmit.py found.\n' + output = presubmit.DoPresubmitChecks( + change, False, True, None, input_buf, DEFAULT_SCRIPT, False) + self.failIf(output.should_continue()) + text = ('Running presubmit hooks...\n' + 'Warning, no presubmit.py found.\n' 'Running default presubmit script.\n' '** Presubmit ERRORS **\n!!\n\n' 'Was the presubmit check useful? Please send feedback & hate mail ' 'to maruel@chromium.org!\n') self.assertEquals(output.getvalue(), text) - self.checkstdout('Running presubmit hooks...\n') def testDirectoryHandling(self): files = [ @@ -600,11 +598,11 @@ def CheckChangeOnCommit(input_api, output_api): self.failUnless(presubmit.DoPresubmitChecks( change, False, True, output, input_buf, DEFAULT_SCRIPT, False)) self.assertEquals(output.getvalue(), - ('Warning, no presubmit.py found.\n' + ('Running presubmit hooks...\n' + 'Warning, no presubmit.py found.\n' 'Running default presubmit script.\n' '** Presubmit Messages **\n' 'http://tracker.com/42\n\n')) - self.checkstdout('Running presubmit hooks...\n') def testGetTrySlavesExecuter(self): self.mox.ReplayAll() @@ -677,10 +675,13 @@ def CheckChangeOnCommit(input_api, output_api): ['git', 'rev-parse', '--show-cdup'], cwd=self.fake_root_dir, stdout=presubmit.subprocess.PIPE).AndReturn(1) + output = self.mox.CreateMock(presubmit.PresubmitOutput) + output.should_continue().AndReturn(False) + presubmit.DoPresubmitChecks(mox.IgnoreArg(), False, False, mox.IgnoreArg(), mox.IgnoreArg(), - None, False).AndReturn(False) + None, False).AndReturn(output) self.mox.ReplayAll() self.assertEquals(True, @@ -1046,7 +1047,7 @@ class OuputApiUnittest(PresubmitTestsBase): def testMembersChanged(self): self.mox.ReplayAll() members = [ - 'MailTextResult', 'PresubmitAddText', 'PresubmitError', + 'MailTextResult', 'PresubmitAddReviewers', 'PresubmitError', 'PresubmitNotifyResult', 'PresubmitPromptWarning', 'PresubmitResult', ] # If this test fails, you should add the relevant test. @@ -1054,57 +1055,58 @@ class OuputApiUnittest(PresubmitTestsBase): def testOutputApiBasics(self): self.mox.ReplayAll() - self.failUnless(presubmit.OutputApi.PresubmitError('').IsFatal()) - self.failIf(presubmit.OutputApi.PresubmitError('').ShouldPrompt()) + self.failUnless(presubmit.OutputApi.PresubmitError('').fatal) + self.failIf(presubmit.OutputApi.PresubmitError('').should_prompt) - self.failIf(presubmit.OutputApi.PresubmitPromptWarning('').IsFatal()) + self.failIf(presubmit.OutputApi.PresubmitPromptWarning('').fatal) self.failUnless( - presubmit.OutputApi.PresubmitPromptWarning('').ShouldPrompt()) + presubmit.OutputApi.PresubmitPromptWarning('').should_prompt) - self.failIf(presubmit.OutputApi.PresubmitNotifyResult('').IsFatal()) - self.failIf(presubmit.OutputApi.PresubmitNotifyResult('').ShouldPrompt()) + self.failIf(presubmit.OutputApi.PresubmitNotifyResult('').fatal) + self.failIf(presubmit.OutputApi.PresubmitNotifyResult('').should_prompt) - self.failIf(presubmit.OutputApi.PresubmitAddText('foo').IsFatal()) - self.failIf(presubmit.OutputApi.PresubmitAddText('foo').ShouldPrompt()) + self.failIf(presubmit.OutputApi.PresubmitAddReviewers( + ['foo']).fatal) + self.failIf(presubmit.OutputApi.PresubmitAddReviewers( + ['foo']).should_prompt) # TODO(joi) Test MailTextResult once implemented. def testOutputApiHandling(self): self.mox.ReplayAll() - output = StringIO.StringIO() - unused_input = StringIO.StringIO() - added_text = presubmit.OutputApi.PresubmitAddText('R=ben@example.com') - self.failUnless(added_text._Handle(output, unused_input)) - self.failUnlessEqual(output.getvalue(), 'ADD: R=ben@example.com\n') + output = presubmit.PresubmitOutput() + presubmit.OutputApi.PresubmitAddReviewers( + ['ben@example.com']).handle(output) + self.failUnless(output.should_continue()) + self.failUnlessEqual(output.reviewers, ['ben@example.com']) - output = StringIO.StringIO() - unused_input = StringIO.StringIO() - error = presubmit.OutputApi.PresubmitError('!!!') - self.failIf(error._Handle(output, unused_input)) + output = presubmit.PresubmitOutput() + presubmit.OutputApi.PresubmitError('!!!').handle(output) + self.failIf(output.should_continue()) self.failUnless(output.getvalue().count('!!!')) - output = StringIO.StringIO() - notify = presubmit.OutputApi.PresubmitNotifyResult('?see?') - self.failUnless(notify._Handle(output, unused_input)) + output = presubmit.PresubmitOutput() + presubmit.OutputApi.PresubmitNotifyResult('?see?').handle(output) + self.failUnless(output.should_continue()) self.failUnless(output.getvalue().count('?see?')) - output = StringIO.StringIO() - input_buf = StringIO.StringIO('y') - warning = presubmit.OutputApi.PresubmitPromptWarning('???') - self.failUnless(warning._Handle(output, input_buf)) + output = presubmit.PresubmitOutput(input_stream=StringIO.StringIO('y')) + presubmit.OutputApi.PresubmitPromptWarning('???').handle(output) + output.prompt_yes_no('prompt: ') + self.failUnless(output.should_continue()) self.failUnless(output.getvalue().count('???')) - output = StringIO.StringIO() - input_buf = StringIO.StringIO('n') - warning = presubmit.OutputApi.PresubmitPromptWarning('???') - self.failIf(warning._Handle(output, input_buf)) + output = presubmit.PresubmitOutput(input_stream=StringIO.StringIO('y')) + presubmit.OutputApi.PresubmitPromptWarning('???').handle(output) + output.prompt_yes_no('prompt: ') + self.failUnless(output.should_continue()) self.failUnless(output.getvalue().count('???')) - output = StringIO.StringIO() - input_buf = StringIO.StringIO('\n') - warning = presubmit.OutputApi.PresubmitPromptWarning('???') - self.failIf(warning._Handle(output, input_buf)) + output = presubmit.PresubmitOutput(input_stream=StringIO.StringIO('\n')) + presubmit.OutputApi.PresubmitPromptWarning('???').handle(output) + output.prompt_yes_no('prompt: ') + self.failIf(output.should_continue()) self.failUnless(output.getvalue().count('???')) @@ -1865,7 +1867,7 @@ mac|success|blew def OwnersTest(self, is_committing, tbr=False, change_tags=None, suggested_reviewers=None, approvers=None, - uncovered_files=None, expected_results=None): + uncovered_files=None, expected_reviewers=None, expected_output=''): affected_file = self.mox.CreateMock(presubmit.SvnAffectedFile) affected_file.LocalPath().AndReturn('foo.cc') change = self.mox.CreateMock(presubmit.Change) @@ -1896,46 +1898,44 @@ mac|success|blew fake_db.reviewers_for(set(['foo.cc'])).AndReturn(suggested_reviewers) self.mox.ReplayAll() + output = presubmit.PresubmitOutput() results = presubmit_canned_checks.CheckOwners(input_api, presubmit.OutputApi) - self.assertEquals(len(results), len(expected_results)) - if results and expected_results: - output = StringIO.StringIO() - unused_input = StringIO.StringIO() - results[0]._Handle(output, unused_input) - self.assertEquals(output.getvalue(), expected_results[0]) + if results: + results[0].handle(output) + if expected_reviewers is not None: + self.assertEquals(output.reviewers, expected_reviewers) + self.assertEquals(output.getvalue(), expected_output) def testCannedCheckOwners_WithReviewer(self): - self.OwnersTest(is_committing=False, change_tags={'R': 'ben@example.com'}, - expected_results=[]) + self.OwnersTest(is_committing=False, change_tags={'R': 'ben@example.com'}) self.OwnersTest(is_committing=False, tbr=True, - change_tags={'R': 'ben@example.com'}, expected_results=[]) + change_tags={'R': 'ben@example.com'}) def testCannedCheckOwners_NoReviewer(self): self.OwnersTest(is_committing=False, change_tags={}, suggested_reviewers=['ben@example.com'], - expected_results=['ADD: R=ben@example.com\n']) + expected_reviewers=['ben@example.com']) self.OwnersTest(is_committing=False, tbr=True, change_tags={}, suggested_reviewers=['ben@example.com'], - expected_results=['ADD: R=ben@example.com\n']) + expected_reviewers=['ben@example.com']) def testCannedCheckOwners_CommittingWithoutOwnerLGTM(self): self.OwnersTest(is_committing=True, approvers=set(), uncovered_files=set(['foo.cc']), - expected_results=['Missing LGTM from an OWNER for: foo.cc\n']) + expected_output='Missing LGTM from an OWNER for: foo.cc\n') def testCannedCheckOwners_CommittingWithLGTMs(self): self.OwnersTest(is_committing=True, approvers=set(['ben@example.com']), - uncovered_files=set(), - expected_results=[]) + uncovered_files=set()) def testCannedCheckOwners_TBR(self): self.OwnersTest(is_committing=True, tbr=True, approvers=set(), uncovered_files=set(), - expected_results=['--tbr was specified, skipping OWNERS check\n']) + expected_output='--tbr was specified, skipping OWNERS check\n') if __name__ == '__main__': import unittest