diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index 36ccfe9a3..09439fb88 100755 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -125,17 +125,23 @@ def _RunPythonUnitTests_LoadTests(input_api, module_name): def RunPythonUnitTests(input_api, output_api, unit_tests): """Imports the unit_tests modules and run them.""" + # We don't want to hinder users from uploading incomplete patches. + if input_api.is_committing: + message_type = output_api.PresubmitError + else: + message_type = output_api.PresubmitNotifyResult tests_suite = [] outputs = [] for unit_test in unit_tests: try: tests_suite.extend(_RunPythonUnitTests_LoadTests(unit_test)) except ImportError: - outputs.append(output_api.PresubmitError("Failed to load %s" % unit_test)) + outputs.append(message_type("Failed to load %s" % unit_test, + long_text=input_api.traceback.format_exc())) results = input_api.unittest.TextTestRunner(verbosity=0).run( input_api.unittest.TestSuite(tests_suite)) if not results.wasSuccessful(): - outputs.append(output_api.PresubmitError( + outputs.append(message_type( "%d unit tests failed." % (results.failures + results.errors))) return outputs diff --git a/presubmit_support.py b/presubmit_support.py index 574e4e61c..1a73d8ead 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -6,7 +6,7 @@ """Enables directory-specific presubmit checks to run at upload and/or commit. """ -__version__ = '1.2' +__version__ = '1.3' # TODO(joi) Add caching where appropriate/needed. The API is designed to allow # caching (between all different invocations of presubmit scripts for a given @@ -25,6 +25,7 @@ import re # Exposed through the API. import subprocess # Exposed through the API. import sys # Parts exposed through API. import tempfile # Exposed through the API. +import traceback # Exposed through the API. import types import unittest # Exposed through the API. import urllib2 # Exposed through the API. @@ -56,21 +57,6 @@ def normpath(path): return os.path.normpath(path) -def deprecated(func): - """This is a decorator which can be used to mark functions as deprecated. - - It will result in a warning being emmitted when the function is used.""" - def newFunc(*args, **kwargs): - warnings.warn("Call to deprecated function %s." % func.__name__, - category=DeprecationWarning, - stacklevel=2) - return func(*args, **kwargs) - newFunc.__name__ = func.__name__ - newFunc.__doc__ = func.__doc__ - newFunc.__dict__.update(func.__dict__) - return newFunc - - class OutputApi(object): """This class (more like a module) gets passed to presubmit scripts so that they can specify various types of results. @@ -151,16 +137,18 @@ class InputApi(object): know stuff about the change they're looking at. """ - def __init__(self, change, presubmit_path): + def __init__(self, change, presubmit_path, is_committing): """Builds an InputApi object. Args: change: A presubmit.GclChange object. presubmit_path: The path to the presubmit script being processed. + is_committing: True if the change is about to be committed. """ # Version number of the presubmit_support script. self.version = [int(x) for x in __version__.split('.')] self.change = change + self.is_committing = is_committing # We expose various modules and functions as attributes of the input_api # so that presubmit scripts don't have to import them. @@ -173,6 +161,7 @@ class InputApi(object): self.re = re self.subprocess = subprocess self.tempfile = tempfile + self.traceback = traceback self.unittest = unittest self.urllib2 = urllib2 @@ -603,7 +592,7 @@ class PresubmitExecuter(object): Return: A list of result objects, empty if no problems. """ - input_api = InputApi(self.change, presubmit_path) + input_api = InputApi(self.change, presubmit_path, self.committing) context = {} exec script_text in context diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index be3cd74c1..1b020260b 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -104,10 +104,10 @@ class PresubmitUnittest(PresubmitTestsBase): 'ListRelevantPresubmitFiles', 'Main', 'NotImplementedException', 'OutputApi', 'ParseFiles', 'PresubmitExecuter', 'ScanSubDirs', 'SvnAffectedFile', - 'cPickle', 'cStringIO', 'deprecated', 'exceptions', + 'cPickle', 'cStringIO', 'exceptions', 'fnmatch', 'gcl', 'gclient', 'glob', 'marshal', 'normpath', 'optparse', 'os', 'pickle', 'presubmit_canned_checks', 're', 'subprocess', 'sys', - 'tempfile', 'types', 'unittest', 'urllib2', 'warnings', + 'tempfile', 'traceback', 'types', 'unittest', 'urllib2', 'warnings', ] # If this test fails, you should add the relevant test. self.compareMembers(presubmit, members) @@ -476,6 +476,10 @@ def CheckChangeOnUpload(input_api, output_api): return [output_api.PresubmitError('Tag parsing failed. 1')] if input_api.change.tags['STORY'] != 'http://tracker.com/42': return [output_api.PresubmitError('Tag parsing failed. 2')] + if input_api.change.BUG != 'boo': + return [output_api.PresubmitError('Tag parsing failed. 6')] + if input_api.change.STORY != 'http://tracker.com/42': + return [output_api.PresubmitError('Tag parsing failed. 7')] if 'TEST' in input_api.change.tags: return [output_api.PresubmitError('Tag parsing failed. 3')] if input_api.change.DescriptionText() != 'Blah Blah': @@ -517,11 +521,12 @@ class InputApiUnittest(PresubmitTestsBase): 'DepotToLocalPath', 'LocalPaths', 'LocalToDepotPath', 'PresubmitLocalPath', 'RightHandSideLines', 'ServerPaths', 'basename', 'cPickle', 'cStringIO', 'canned_checks', 'change', - 'marshal', 'os_path', 'pickle', 'platform', - 're', 'subprocess', 'tempfile', 'unittest', 'urllib2', 'version', + 'is_committing', 'marshal', 'os_path', 'pickle', 'platform', + 're', 'subprocess', 'tempfile', 'traceback', 'unittest', 'urllib2', + 'version', ] # If this test fails, you should add the relevant test. - self.compareMembers(presubmit.InputApi(None, './.'), members) + self.compareMembers(presubmit.InputApi(None, './.', False), members) def testDepotToLocalPath(self): presubmit.gclient.CaptureSVNInfo('svn://foo/smurf').AndReturn( @@ -529,9 +534,10 @@ class InputApiUnittest(PresubmitTestsBase): presubmit.gclient.CaptureSVNInfo('svn:/foo/notfound/burp').AndReturn({}) self.mox.ReplayAll() - path = presubmit.InputApi(None, './p').DepotToLocalPath('svn://foo/smurf') + path = presubmit.InputApi(None, './p', False).DepotToLocalPath( + 'svn://foo/smurf') self.failUnless(path == 'prout') - path = presubmit.InputApi(None, './p').DepotToLocalPath( + path = presubmit.InputApi(None, './p', False).DepotToLocalPath( 'svn:/foo/notfound/burp') self.failUnless(path == None) @@ -540,15 +546,17 @@ class InputApiUnittest(PresubmitTestsBase): presubmit.gclient.CaptureSVNInfo('notfound-food').AndReturn({}) self.mox.ReplayAll() - path = presubmit.InputApi(None, './p').LocalToDepotPath('smurf') + path = presubmit.InputApi(None, './p', False).LocalToDepotPath('smurf') self.assertEqual(path, 'svn://foo') - path = presubmit.InputApi(None, './p').LocalToDepotPath('notfound-food') + path = presubmit.InputApi(None, './p', False).LocalToDepotPath( + 'notfound-food') self.failUnless(path == None) def testInputApiConstruction(self): self.mox.ReplayAll() # Fudge the change object, it's not used during construction anyway - api = presubmit.InputApi(change=42, presubmit_path='foo/path/PRESUBMIT.py') + api = presubmit.InputApi(change=42, presubmit_path='foo/path/PRESUBMIT.py', + is_committing=False) self.assertEquals(api.PresubmitLocalPath(), 'foo/path') self.assertEquals(api.change, 42) @@ -593,7 +601,7 @@ class InputApiUnittest(PresubmitTestsBase): description='\n'.join(description_lines), files=files) change = presubmit.GclChange(ci) - api = presubmit.InputApi(change, 'foo/PRESUBMIT.py') + api = presubmit.InputApi(change, 'foo/PRESUBMIT.py', False) affected_files = api.AffectedFiles() self.assertEquals(len(affected_files), 3) self.assertEquals(affected_files[0].LocalPath(), @@ -643,7 +651,9 @@ class InputApiUnittest(PresubmitTestsBase): paths_from_change = change.AbsoluteLocalPaths(include_dirs=True) self.assertEqual(len(paths_from_change), 3) presubmit_path = join(self.fake_root_dir, 'isdir', 'PRESUBMIT.py') - api = presubmit.InputApi(change=change, presubmit_path=presubmit_path) + api = presubmit.InputApi(change=change, + presubmit_path=presubmit_path, + is_committing=True) paths_from_api = api.AbsoluteLocalPaths(include_dirs=True) self.assertEqual(len(paths_from_api), 2) for absolute_paths in [paths_from_change, paths_from_api]: @@ -659,7 +669,7 @@ class InputApiUnittest(PresubmitTestsBase): change = presubmit.GclChange( presubmit.gcl.ChangeInfo(name='mychange', issue=0, patchset=0, description='Bleh\n', files=None)) - api = presubmit.InputApi(change, 'foo/PRESUBMIT.py') + api = presubmit.InputApi(change, 'foo/PRESUBMIT.py', True) api.AffectedTextFiles(include_deletes=False) @@ -821,6 +831,7 @@ class CannedChecksUnittest(PresubmitTestsBase): def MockInputApi(self): input_api = self.mox.CreateMock(presubmit.InputApi) input_api.re = presubmit.re + input_api.traceback = presubmit.traceback input_api.urllib2 = self.mox.CreateMock(presubmit.urllib2) input_api.unittest = unittest return input_api @@ -945,15 +956,29 @@ class CannedChecksUnittest(PresubmitTestsBase): self.assertEquals(results[0].__class__, presubmit.OutputApi.PresubmitError) - def testRunPythonUnitTests1(self): + def testRunPythonUnitTestsNoTest(self): input_api = self.MockInputApi() + input_api.is_committing = False self.mox.ReplayAll() results = presubmit_canned_checks.RunPythonUnitTests( input_api, presubmit.OutputApi, []) self.assertEquals(results, []) - def testRunPythonUnitTests2(self): + def testRunPythonUnitTestsNonExistent1(self): input_api = self.MockInputApi() + input_api.is_committing = False + presubmit_canned_checks._RunPythonUnitTests_LoadTests('_non_existent_module' + ).AndRaise(exceptions.ImportError('Blehh')) + self.mox.ReplayAll() + results = presubmit_canned_checks.RunPythonUnitTests( + input_api, presubmit.OutputApi, ['_non_existent_module']) + self.assertEquals(len(results), 1) + self.assertEquals(results[0].__class__, + presubmit.OutputApi.PresubmitNotifyResult) + + def testRunPythonUnitTestsNonExistent2(self): + input_api = self.MockInputApi() + input_api.is_committing = True presubmit_canned_checks._RunPythonUnitTests_LoadTests('_non_existent_module' ).AndRaise(exceptions.ImportError('Blehh')) self.mox.ReplayAll() @@ -962,8 +987,21 @@ class CannedChecksUnittest(PresubmitTestsBase): self.assertEquals(len(results), 1) self.assertEquals(results[0].__class__, presubmit.OutputApi.PresubmitError) - def testRunPythonUnitTests3(self): + def testRunPythonUnitTestsEmpty1(self): + input_api = self.MockInputApi() + input_api.is_committing = False + test_module = self.mox.CreateMockAnything() + presubmit_canned_checks._RunPythonUnitTests_LoadTests('test_module' + ).AndReturn([]) + self.mox.ReplayAll() + + results = presubmit_canned_checks.RunPythonUnitTests( + input_api, presubmit.OutputApi, ['test_module']) + self.assertEquals(results, []) + + def testRunPythonUnitTestsEmpty2(self): input_api = self.MockInputApi() + input_api.is_committing = True test_module = self.mox.CreateMockAnything() presubmit_canned_checks._RunPythonUnitTests_LoadTests('test_module' ).AndReturn([]) @@ -973,8 +1011,33 @@ class CannedChecksUnittest(PresubmitTestsBase): input_api, presubmit.OutputApi, ['test_module']) self.assertEquals(results, []) - def testRunPythonUnitTests4(self): + def testRunPythonUnitTestsFailure1(self): + input_api = self.MockInputApi() + input_api.is_committing = False + input_api.unittest = self.mox.CreateMock(unittest) + test = self.mox.CreateMockAnything() + presubmit_canned_checks._RunPythonUnitTests_LoadTests('test_module' + ).AndReturn([test]) + runner = self.mox.CreateMockAnything() + input_api.unittest.TextTestRunner(verbosity=0).AndReturn(runner) + suite = self.mox.CreateMockAnything() + input_api.unittest.TestSuite([test]).AndReturn(suite) + test_result = self.mox.CreateMockAnything() + runner.run(suite).AndReturn(test_result) + test_result.wasSuccessful().AndReturn(False) + test_result.failures = 2 + test_result.errors = 3 + self.mox.ReplayAll() + + results = presubmit_canned_checks.RunPythonUnitTests( + input_api, presubmit.OutputApi, ['test_module']) + self.assertEquals(len(results), 1) + self.assertEquals(results[0].__class__, + presubmit.OutputApi.PresubmitNotifyResult) + + def testRunPythonUnitTestsFailure2(self): input_api = self.MockInputApi() + input_api.is_committing = True input_api.unittest = self.mox.CreateMock(unittest) test = self.mox.CreateMockAnything() presubmit_canned_checks._RunPythonUnitTests_LoadTests('test_module' @@ -995,8 +1058,9 @@ class CannedChecksUnittest(PresubmitTestsBase): self.assertEquals(len(results), 1) self.assertEquals(results[0].__class__, presubmit.OutputApi.PresubmitError) - def testRunPythonUnitTests5(self): + def testRunPythonUnitTestsSuccess(self): input_api = self.MockInputApi() + input_api.is_committing = False input_api.unittest = self.mox.CreateMock(unittest) test = self.mox.CreateMockAnything() presubmit_canned_checks._RunPythonUnitTests_LoadTests('test_module'