Add tests for 'ForTesting' presubmit check
The root PRESUBMIT.py contains _CheckNoProductionCodeUsingTestOnlyFunctions which checks against including a call to a 'for testing only' method in production code. That check has not been tested. This CL adds two tests for it. Additionally, the CL also fixes two issues in the presubmit tests: * FilterSourceFile in PRESUBMIT_test_mocks.py did not match the production version from tools/depot_tools/presubmit_support.py: the production requires the filename to be in the whitelist, while the mock version was OK with just not being in the blacklist. The CL modifies the mock version to match the production. * The test for _CheckAndroidTestJUnitInheritance did not adhere to the whitelisted filename pattern (*Test.java). This CL changes the names of the fake files to match the pattern. Bug: 821981 Change-Id: I65edf07ddb2ae26ad7d08ceb7cf4d51b482b5e56 Reviewed-on: https://chromium-review.googlesource.com/966605 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Commit-Queue: Vaclav Brozek <vabr@chromium.org> Cr-Commit-Position: refs/heads/master@{#543783}
This commit is contained in:

committed by
Commit Bot

parent
dd80e340ad
commit
f01ed503dc
@ -1009,11 +1009,11 @@ class AndroidJUnitBaseClass(unittest.TestCase):
|
||||
'public class IncorrectTest extends TestCase {',
|
||||
'}',
|
||||
]),
|
||||
MockAffectedFile('IncorrectTestWithInterface.java', [
|
||||
MockAffectedFile('IncorrectWithInterfaceTest.java', [
|
||||
'public class Test implements X extends BaseClass {',
|
||||
'}',
|
||||
]),
|
||||
MockAffectedFile('IncorrectTestMultiLine.java', [
|
||||
MockAffectedFile('IncorrectMultiLineTest.java', [
|
||||
'public class Test implements X, Y, Z',
|
||||
' extends TestBase {',
|
||||
'}',
|
||||
@ -1029,11 +1029,11 @@ class AndroidJUnitBaseClass(unittest.TestCase):
|
||||
% (3, len(msgs[0].items), msgs[0].items))
|
||||
self.assertTrue('IncorrectTest.java:1' in msgs[0].items,
|
||||
'IncorrectTest not found in errors')
|
||||
self.assertTrue('IncorrectTestWithInterface.java:1'
|
||||
self.assertTrue('IncorrectWithInterfaceTest.java:1'
|
||||
in msgs[0].items,
|
||||
'IncorrectTestWithInterface not found in errors')
|
||||
self.assertTrue('IncorrectTestMultiLine.java:2' in msgs[0].items,
|
||||
'IncorrectTestMultiLine not found in errors')
|
||||
'IncorrectWithInterfaceTest not found in errors')
|
||||
self.assertTrue('IncorrectMultiLineTest.java:2' in msgs[0].items,
|
||||
'IncorrectMultiLineTest not found in errors')
|
||||
|
||||
class LogUsageTest(unittest.TestCase):
|
||||
|
||||
@ -1488,5 +1488,38 @@ class BannedFunctionCheckTest(unittest.TestCase):
|
||||
self.assertTrue('some/mac/file.mm' not in errors[0].message)
|
||||
|
||||
|
||||
class NoProductionCodeUsingTestOnlyFunctions(unittest.TestCase):
|
||||
def testTruePositives(self):
|
||||
mock_input_api = MockInputApi()
|
||||
mock_input_api.files = [
|
||||
MockFile('some/path/foo.cc', ['foo_for_testing();']),
|
||||
MockFile('some/path/foo.mm', ['FooForTesting();']),
|
||||
MockFile('some/path/foo.cxx', ['FooForTests();']),
|
||||
MockFile('some/path/foo.cpp', ['foo_for_test();']),
|
||||
]
|
||||
|
||||
results = PRESUBMIT._CheckNoProductionCodeUsingTestOnlyFunctions(
|
||||
mock_input_api, MockOutputApi())
|
||||
self.assertEqual(1, len(results))
|
||||
self.assertEqual(4, len(results[0].items))
|
||||
self.assertTrue('foo.cc' in results[0].items[0])
|
||||
self.assertTrue('foo.mm' in results[0].items[1])
|
||||
self.assertTrue('foo.cxx' in results[0].items[2])
|
||||
self.assertTrue('foo.cpp' in results[0].items[3])
|
||||
|
||||
def testFalsePositives(self):
|
||||
mock_input_api = MockInputApi()
|
||||
mock_input_api.files = [
|
||||
MockFile('some/path/foo.h', ['foo_for_testing();']),
|
||||
MockFile('some/path/foo.mm', ['FooForTesting() {']),
|
||||
MockFile('some/path/foo.cc', ['::FooForTests();']),
|
||||
MockFile('some/path/foo.cpp', ['// foo_for_test();']),
|
||||
]
|
||||
|
||||
results = PRESUBMIT._CheckNoProductionCodeUsingTestOnlyFunctions(
|
||||
mock_input_api, MockOutputApi())
|
||||
self.assertEqual(0, len(results))
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
|
@ -92,17 +92,19 @@ class MockInputApi(object):
|
||||
|
||||
def FilterSourceFile(self, file, white_list=(), black_list=()):
|
||||
local_path = file.LocalPath()
|
||||
found_in_white_list = not white_list
|
||||
if white_list:
|
||||
for pattern in white_list:
|
||||
compiled_pattern = re.compile(pattern)
|
||||
if compiled_pattern.search(local_path):
|
||||
return True
|
||||
found_in_white_list = True
|
||||
break
|
||||
if black_list:
|
||||
for pattern in black_list:
|
||||
compiled_pattern = re.compile(pattern)
|
||||
if compiled_pattern.search(local_path):
|
||||
return False
|
||||
return True
|
||||
return found_in_white_list
|
||||
|
||||
def LocalPaths(self):
|
||||
return self.files
|
||||
|
Reference in New Issue
Block a user