From 1487d53b6c8e1bd137a40cebdd57be6c03085613 Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Sat, 6 Jun 2009 00:22:57 +0000 Subject: [PATCH] Improve the presubmit_canned_checks testing by using a real mock and testing for more cases. Remove a superfluous check in CheckLongLines(). Add unittest to InputApi. Review URL: http://codereview.chromium.org/114082 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@17805 0039d316-1c4b-4281-b951-d872f2087c98 --- presubmit_canned_checks.py | 30 ++-- presubmit_support.py | 7 +- tests/presubmit_unittest.py | 302 +++++++++++++++++++++--------------- 3 files changed, 194 insertions(+), 145 deletions(-) diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index faf290ae1..36ccfe9a3 100755 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -84,16 +84,12 @@ def CheckLongLines(input_api, output_api, maxlen=80): """Checks that there aren't any lines longer than maxlen characters in any of the text files to be submitted. """ - basename = input_api.basename - bad = [] for f, line_num, line in input_api.RightHandSideLines(): - if line.endswith('\n'): - line = line[:-1] if len(line) > maxlen: bad.append( '%s, line %s, %s chars' % - (basename(f.LocalPath()), line_num, len(line))) + (f.LocalPath(), line_num, len(line))) if len(bad) == 5: # Just show the first 5 errors. break @@ -120,27 +116,25 @@ def CheckTreeIsOpen(input_api, output_api, url, closed): return [] +def _RunPythonUnitTests_LoadTests(input_api, module_name): + """Meant to be stubbed out during unit testing.""" + module = __import__(module_name) + for part in module_name.split('.')[1:]: + module = getattr(module, part) + return input_api.unittest.TestLoader().loadTestsFromModule(module)._tests + def RunPythonUnitTests(input_api, output_api, unit_tests): """Imports the unit_tests modules and run them.""" - import unittest tests_suite = [] - test_loader = unittest.TestLoader() - def LoadTests(module_name): - module = __import__(module_name) - for part in module_name.split('.')[1:]: - module = getattr(module, part) - tests_suite.extend(test_loader.loadTestsFromModule(module)._tests) - outputs = [] for unit_test in unit_tests: try: - LoadTests(unit_test) + tests_suite.extend(_RunPythonUnitTests_LoadTests(unit_test)) except ImportError: - outputs.Append(output_api.PresubmitError("Failed to load %s" % unit_test)) - raise + outputs.append(output_api.PresubmitError("Failed to load %s" % unit_test)) - results = unittest.TextTestRunner(verbosity=0).run(unittest.TestSuite( - tests_suite)) + results = input_api.unittest.TextTestRunner(verbosity=0).run( + input_api.unittest.TestSuite(tests_suite)) if not results.wasSuccessful(): outputs.append(output_api.PresubmitError( "%d unit tests failed." % (results.failures + results.errors))) diff --git a/presubmit_support.py b/presubmit_support.py index f12d31294..928929e35 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.1' +__version__ = '1.2' # TODO(joi) Add caching where appropriate/needed. The API is designed to allow # caching (between all different invocations of presubmit scripts for a given @@ -26,6 +26,7 @@ import subprocess # Exposed through the API. import sys # Parts exposed through API. import tempfile # Exposed through the API. import types +import unittest # Exposed through the API. import urllib2 # Exposed through the API. import warnings @@ -172,6 +173,7 @@ class InputApi(object): self.re = re self.subprocess = subprocess self.tempfile = tempfile + self.unittest = unittest self.urllib2 = urllib2 # InputApi.platform is the platform you're currently running on. @@ -273,6 +275,8 @@ class InputApi(object): the AffectedFile instance of the current file; integer line number (1-based); and the contents of the line as a string. + + Note: The cariage return (LF or CR) is stripped off. """ return InputApi._RightHandSideLinesImpl( filter(lambda x: x.IsTextFile(), @@ -349,6 +353,7 @@ class AffectedFile(object): side". Contents will be empty if the file is a directory or does not exist. + Note: The cariage returns (LF or CR) are stripped off. """ if self.IsDirectory(): return [] diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 05e746c57..00ea0f612 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -86,12 +86,6 @@ def CheckChangeOnUpload(input_api, output_api): self.mox.StubOutWithMock(presubmit.gcl, 'GetSVNFileProperty') self.mox.StubOutWithMock(presubmit.gcl, 'ReadFile') - @staticmethod - def MakeBasicChange(name, description): - ci = presubmit.gcl.ChangeInfo(name=name, description=description) - change = presubmit.GclChange(ci) - return change - def compareMembers(self, object, members): """If you add a member, be sure to add the relevant test!""" # Skip over members starting with '_' since they are usually not meant to @@ -113,7 +107,7 @@ class PresubmitUnittest(PresubmitTestsBase): 'cPickle', 'cStringIO', 'deprecated', 'exceptions', 'fnmatch', 'gcl', 'gclient', 'glob', 'marshal', 'normpath', 'optparse', 'os', 'pickle', 'presubmit_canned_checks', 're', 'subprocess', 'sys', - 'tempfile', 'types', 'urllib2', 'warnings', + 'tempfile', 'types', 'unittest', 'urllib2', 'warnings', ] # If this test fails, you should add the relevant test. self.compareMembers(presubmit, members) @@ -522,7 +516,7 @@ class InputApiUnittest(PresubmitTestsBase): 'PresubmitLocalPath', 'RightHandSideLines', 'ServerPaths', 'basename', 'cPickle', 'cStringIO', 'canned_checks', 'change', 'marshal', 'os_path', 'pickle', 'platform', - 're', 'subprocess', 'tempfile', 'urllib2', 'version', + 're', 'subprocess', 'tempfile', 'unittest', 'urllib2', 'version', ] # If this test fails, you should add the relevant test. self.compareMembers(presubmit.InputApi(None, './.'), members) @@ -800,27 +794,22 @@ class AffectedFileUnittest(PresubmitTestsBase): class CannedChecksUnittest(PresubmitTestsBase): """Tests presubmit_canned_checks.py.""" - class MockInputApi(object): - class MockUrllib2(object): - class urlopen(object): - def __init__(self, url): - if url == 'url_to_open': - self.result = '1' - else: - self.result = '0' - def read(self): - return self.result - def close(self): - pass - def __init__(self, lines=None): - self.lines = lines - self.basename = lambda x: x - self.urllib2 = self.MockUrllib2() - self.re = presubmit.re - - def RightHandSideLines(self): - for line in self.lines: - yield (presubmit.AffectedFile('bingo', 'M'), 1, line) + + def setUp(self): + PresubmitTestsBase.setUp(self) + self.mox.StubOutWithMock(presubmit_canned_checks, + '_RunPythonUnitTests_LoadTests') + + def MockInputApi(self): + input_api = self.mox.CreateMock(presubmit.InputApi) + input_api.re = presubmit.re + input_api.urllib2 = self.mox.CreateMock(presubmit.urllib2) + input_api.unittest = unittest + return input_api + + def MakeBasicChange(self, name, description): + ci = presubmit.gcl.ChangeInfo(name=name, description=description) + return presubmit.GclChange(ci, self.fake_root_dir) def testMembersChanged(self): self.mox.ReplayAll() @@ -834,120 +823,181 @@ class CannedChecksUnittest(PresubmitTestsBase): # If this test fails, you should add the relevant test. self.compareMembers(presubmit_canned_checks, members) - def testCannedCheckChangeHasBugField(self): + def TestDescription(self, check, description1, description2, error_type): + input_api1 = self.MockInputApi() + input_api1.change = self.MakeBasicChange('foo', 'Foo\n' + description1) + input_api2 = self.MockInputApi() + input_api2.change = self.MakeBasicChange('foo', 'Foo\n' + description2) + self.mox.ReplayAll() + + results1 = check(input_api1, presubmit.OutputApi) + self.assertEquals(results1, []) + results2 = check(input_api2, presubmit.OutputApi) + self.assertEquals(len(results2), 1) + self.assertEquals(results2[0].__class__, error_type) + + def TestContent(self, check, content1, content2, error_type): + input_api1 = self.MockInputApi() + input_api1.change = self.MakeBasicChange('foo', 'Foo\n') + affected_file = self.mox.CreateMock(presubmit.SvnAffectedFile) + affected_file.LocalPath().AndReturn('foo.cc') + output1 = [ + (affected_file, 42, 'yo, ' + content1), + (affected_file, 43, 'yer'), + (affected_file, 23, 'ya'), + ] + input_api1.RightHandSideLines().AndReturn(output1) + input_api2 = self.MockInputApi() + input_api2.change = self.MakeBasicChange('foo', 'Foo\n') + output2 = [ + (affected_file, 42, 'yo, ' + content2), + (affected_file, 43, 'yer'), + (affected_file, 23, 'ya'), + ] + input_api2.RightHandSideLines().AndReturn(output2) self.mox.ReplayAll() - change = self.MakeBasicChange('foo', - 'Foo\nBUG=1234') - api = presubmit.InputApi(change, './PRESUBMIT.py') - self.failIf(presubmit_canned_checks.CheckChangeHasBugField( - api, presubmit.OutputApi)) - change = self.MakeBasicChange('foo', - 'Foo\nNEVERTESTED=did some stuff') - api = presubmit.InputApi(change, './PRESUBMIT.py') - self.failUnless(presubmit_canned_checks.CheckChangeHasBugField( - api, presubmit.OutputApi)) + results1 = check(input_api1, presubmit.OutputApi) + self.assertEquals(results1, []) + results2 = check(input_api2, presubmit.OutputApi) + self.assertEquals(len(results2), 1) + self.assertEquals(results2[0].__class__, error_type) - def testCannedCheckChangeHasTestField(self): - self.mox.ReplayAll() - change = self.MakeBasicChange('foo', - 'Foo\nTEST=did some stuff') - api = presubmit.InputApi(change, './PRESUBMIT.py') - self.failIf(presubmit_canned_checks.CheckChangeHasTestField( - api, presubmit.OutputApi)) + def testCannedCheckChangeHasBugField(self): + self.TestDescription(presubmit_canned_checks.CheckChangeHasBugField, + 'BUG=1234', '', + presubmit.OutputApi.PresubmitNotifyResult) - change = self.MakeBasicChange('foo', - 'Foo\nNOTEST=did some stuff') - api = presubmit.InputApi(change, './PRESUBMIT.py') - self.failUnless(presubmit_canned_checks.CheckChangeHasTestField( - api, presubmit.OutputApi)) + def testCannedCheckChangeHasTestField(self): + self.TestDescription(presubmit_canned_checks.CheckChangeHasTestField, + 'TEST=did some stuff', '', + presubmit.OutputApi.PresubmitNotifyResult) def testCannedCheckChangeHasTestedField(self): - self.mox.ReplayAll() - change = self.MakeBasicChange('foo', - 'Foo\nTESTED=did some stuff') - api = presubmit.InputApi(change, './PRESUBMIT.py') - self.failIf(presubmit_canned_checks.CheckChangeHasTestedField( - api, presubmit.OutputApi)) - - change = self.MakeBasicChange('foo', - 'Foo\nNEVERTESTED=did some stuff') - api = presubmit.InputApi(change, './PRESUBMIT.py') - self.failUnless(presubmit_canned_checks.CheckChangeHasTestedField( - api, presubmit.OutputApi)) + self.TestDescription(presubmit_canned_checks.CheckChangeHasTestedField, + 'TESTED=did some stuff', '', + presubmit.OutputApi.PresubmitError) def testCannedCheckChangeHasQAField(self): - self.mox.ReplayAll() - change = self.MakeBasicChange('foo', - 'Foo\nQA=test floop feature very well') - api = presubmit.InputApi(change, './PRESUBMIT.py') - self.failIf(presubmit_canned_checks.CheckChangeHasQaField( - api, presubmit.OutputApi)) - - change = self.MakeBasicChange('foo', - 'Foo\nNOTFORQA=test floop feature very well') - api = presubmit.InputApi(change, './PRESUBMIT.py') - self.failUnless(presubmit_canned_checks.CheckChangeHasQaField( - api, presubmit.OutputApi)) + self.TestDescription(presubmit_canned_checks.CheckChangeHasQaField, + 'QA=BSOD your machine', '', + presubmit.OutputApi.PresubmitError) def testCannedCheckDoNotSubmitInDescription(self): - self.mox.ReplayAll() - change = self.MakeBasicChange('foo', 'hello') - api = presubmit.InputApi(change, './PRESUBMIT.py') - self.failIf(presubmit_canned_checks.CheckDoNotSubmitInDescription( - api, presubmit.OutputApi)) - - change = self.MakeBasicChange('foo', - 'DO NOT ' + 'SUBMIT') - api = presubmit.InputApi(change, './PRESUBMIT.py') - self.failUnless(presubmit_canned_checks.CheckDoNotSubmitInDescription( - api, presubmit.OutputApi)) + self.TestDescription(presubmit_canned_checks.CheckDoNotSubmitInDescription, + 'DO NOTSUBMIT', 'DO NOT ' + 'SUBMIT', + presubmit.OutputApi.PresubmitError) def testCannedCheckDoNotSubmitInFiles(self): - self.mox.ReplayAll() - self.failIf(presubmit_canned_checks.CheckDoNotSubmitInFiles( - self.MockInputApi(['hello', 'there']), presubmit.OutputApi - )) - self.failUnless(presubmit_canned_checks.CheckDoNotSubmitInFiles( - self.MockInputApi(['hello', 'yo, DO NOT ' + 'SUBMIT']), - presubmit.OutputApi)) + self.TestContent(presubmit_canned_checks.CheckDoNotSubmitInFiles, + 'DO NOTSUBMIT', 'DO NOT ' + 'SUBMIT', + presubmit.OutputApi.PresubmitError) def testCannedCheckChangeHasNoTabs(self): - self.mox.ReplayAll() - self.failIf(presubmit_canned_checks.CheckChangeHasNoTabs( - self.MockInputApi(['hello', 'there']), presubmit.OutputApi - )) - self.failUnless(presubmit_canned_checks.CheckChangeHasNoTabs( - self.MockInputApi(['hello', 'there\tit is']), presubmit.OutputApi - )) + self.TestContent(presubmit_canned_checks.CheckChangeHasNoTabs, + 'blah blah', 'blah\tblah', + presubmit.OutputApi.PresubmitError) def testCannedCheckLongLines(self): - self.mox.ReplayAll() - self.failIf(presubmit_canned_checks.CheckLongLines( - self.MockInputApi(['hello', 'there']), presubmit.OutputApi, 5 - )) - self.failUnless(presubmit_canned_checks.CheckLongLines( - self.MockInputApi(['hello', 'there!']), presubmit.OutputApi, 5 - )) - - def testCannedCheckTreeIsOpen(self): - self.mox.ReplayAll() - self.failIf(presubmit_canned_checks.CheckTreeIsOpen( - self.MockInputApi(), presubmit.OutputApi, url='url_to_open', closed='0' - )) - self.failUnless(presubmit_canned_checks.CheckTreeIsOpen( - self.MockInputApi(), presubmit.OutputApi, url='url_to_closed', closed='0' - )) + check = lambda x,y: presubmit_canned_checks.CheckLongLines(x, y, 10) + self.TestContent(check, '', 'blah blah blah', + presubmit.OutputApi.PresubmitPromptWarning) + + def testCannedCheckTreeIsOpenOpen(self): + input_api = self.MockInputApi() + connection = self.mox.CreateMockAnything() + input_api.urllib2.urlopen('url_to_open').AndReturn(connection) + connection.read().AndReturn('1') + connection.close() + self.mox.ReplayAll() + results = presubmit_canned_checks.CheckTreeIsOpen( + input_api, presubmit.OutputApi, url='url_to_open', closed='0') + self.assertEquals(results, []) + + def testCannedCheckTreeIsOpenClosed(self): + input_api = self.MockInputApi() + connection = self.mox.CreateMockAnything() + input_api.urllib2.urlopen('url_to_closed').AndReturn(connection) + connection.read().AndReturn('0') + connection.close() + self.mox.ReplayAll() + results = presubmit_canned_checks.CheckTreeIsOpen( + input_api, presubmit.OutputApi, url='url_to_closed', closed='0') + self.assertEquals(len(results), 1) + self.assertEquals(results[0].__class__, + presubmit.OutputApi.PresubmitError) + + def testRunPythonUnitTests1(self): + input_api = self.MockInputApi() + self.mox.ReplayAll() + results = presubmit_canned_checks.RunPythonUnitTests( + input_api, presubmit.OutputApi, []) + self.assertEquals(results, []) + + def testRunPythonUnitTests2(self): + input_api = self.MockInputApi() + 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.PresubmitError) + + def testRunPythonUnitTests3(self): + input_api = self.MockInputApi() + 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 testRunPythonUnitTests4(self): + input_api = self.MockInputApi() + 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.PresubmitError) + + def testRunPythonUnitTests5(self): + input_api = self.MockInputApi() + 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(True) + test_result.failures = 0 + test_result.errors = 0 + self.mox.ReplayAll() + + results = presubmit_canned_checks.RunPythonUnitTests( + input_api, presubmit.OutputApi, ['test_module']) + self.assertEquals(len(results), 0) - def RunPythonUnitTests(self): - self.mox.ReplayAll() - # TODO(maruel): Add real tests. - self.failIf(presubmit_canned_checks.RunPythonUnitTests( - self.MockInputApi(), - presubmit.OutputApi, [])) - self.failUnless(presubmit_canned_checks.RunPythonUnitTests( - self.MockInputApi(), - presubmit.OutputApi, ['non_existent_module'])) if __name__ == '__main__': unittest.main()