diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 144c5ccc855c1..a29e0a0756a4b 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -1802,6 +1802,58 @@ def _CheckAndroidCrLogUsage(input_api, output_api): return results +def _CheckAndroidTestJUnitFrameworkImport(input_api, output_api): + """Checks that junit.framework.* is no longer used.""" + deprecated_junit_framework_pattern = input_api.re.compile( + r'^import junit\.framework\..*;', + input_api.re.MULTILINE) + sources = lambda x: input_api.FilterSourceFile( + x, white_list=(r'.*\.java$',), black_list=None) + errors = [] + for f in input_api.AffectedFiles(sources): + for line_num, line in f.ChangedContents(): + if deprecated_junit_framework_pattern.search(line): + errors.append("%s:%d" % (f.LocalPath(), line_num)) + + results = [] + if errors: + results.append(output_api.PresubmitError( + 'APIs from junit.framework.* are deprecated, please use JUnit4 framework' + '(org.junit.*) from //third_party/junit. Contact yolandyan@chromium.org' + ' if you have any question.', errors)) + return results + + +def _CheckAndroidTestJUnitInheritance(input_api, output_api): + """Checks that if new Java test classes have inheritance. + Either the new test class is JUnit3 test or it is a JUnit4 test class + with a base class, either case is undesirable. + """ + class_declaration_pattern = input_api.re.compile(r'^public class \w*Test ') + + sources = lambda x: input_api.FilterSourceFile( + x, white_list=(r'.*Test\.java$',), black_list=None) + errors = [] + for f in input_api.AffectedFiles(sources): + if not f.OldContents(): + class_declaration_start_flag = False + for line_num, line in f.ChangedContents(): + if class_declaration_pattern.search(line): + class_declaration_start_flag = True + if class_declaration_start_flag and ' extends ' in line: + errors.append('%s:%d' % (f.LocalPath(), line_num)) + if '{' in line: + class_declaration_start_flag = False + + results = [] + if errors: + results.append(output_api.PresubmitPromptWarning( + 'The newly created files include Test classes that inherits from base' + ' class. Please do not use inheritance in JUnit4 tests or add new' + ' JUnit3 tests. Contact yolandyan@chromium.org if you have any' + ' questions.', errors)) + return results + def _CheckAndroidTestAnnotationUsage(input_api, output_api): """Checks that android.test.suitebuilder.annotation.* is no longer used.""" deprecated_annotation_import_pattern = input_api.re.compile( @@ -2292,6 +2344,8 @@ def _AndroidSpecificOnUploadChecks(input_api, output_api): results.extend(_CheckAndroidCrLogUsage(input_api, output_api)) results.extend(_CheckAndroidNewMdpiAssetLocation(input_api, output_api)) results.extend(_CheckAndroidToastUsage(input_api, output_api)) + results.extend(_CheckAndroidTestJUnitInheritance(input_api, output_api)) + results.extend(_CheckAndroidTestJUnitFrameworkImport(input_api, output_api)) results.extend(_CheckAndroidTestAnnotationUsage(input_api, output_api)) return results diff --git a/PRESUBMIT_test.py b/PRESUBMIT_test.py index f8feb78f1815c..649872cb9d5a5 100755 --- a/PRESUBMIT_test.py +++ b/PRESUBMIT_test.py @@ -742,7 +742,6 @@ class PydepsNeedsUpdatingTest(unittest.TestCase): self.assertTrue('File is stale' in str(results[0])) self.assertTrue('File is stale' in str(results[1])) - class AndroidDeprecatedTestAnnotationTest(unittest.TestCase): def testCheckAndroidTestAnnotationUsage(self): mock_input_api = MockInputApi() @@ -788,7 +787,95 @@ class AndroidDeprecatedTestAnnotationTest(unittest.TestCase): self.assertTrue('UsedDeprecatedSmokeAnnotation.java:1' in msgs[0].items, 'UsedDeprecatedSmokeAnnotation not found in errors') +class AndroidDeprecatedJUnitFrameworkTest(unittest.TestCase): + def testCheckAndroidTestAnnotationUsage(self): + mock_input_api = MockInputApi() + mock_output_api = MockOutputApi() + mock_input_api.files = [ + MockAffectedFile('LalaLand.java', [ + 'random stuff' + ]), + MockAffectedFile('CorrectUsage.java', [ + 'import org.junit.ABC', + 'import org.junit.XYZ;', + ]), + MockAffectedFile('UsedDeprecatedJUnit.java', [ + 'import junit.framework.*;', + ]), + MockAffectedFile('UsedDeprecatedJUnitAssert.java', [ + 'import junit.framework.Assert;', + ]), + ] + msgs = PRESUBMIT._CheckAndroidTestJUnitFrameworkImport( + mock_input_api, mock_output_api) + self.assertEqual(1, len(msgs), + 'Expected %d items, found %d: %s' + % (1, len(msgs), msgs)) + self.assertEqual(2, len(msgs[0].items), + 'Expected %d items, found %d: %s' + % (2, len(msgs[0].items), msgs[0].items)) + self.assertTrue('UsedDeprecatedJUnit.java:1' in msgs[0].items, + 'UsedDeprecatedJUnit.java not found in errors') + self.assertTrue('UsedDeprecatedJUnitAssert.java:1' + in msgs[0].items, + 'UsedDeprecatedJUnitAssert not found in errors') + +class AndroidJUnitBaseClass(unittest.TestCase): + def testCheckAndroidTestAnnotationUsage(self): + mock_input_api = MockInputApi() + mock_output_api = MockOutputApi() + + mock_input_api.files = [ + MockAffectedFile('LalaLand.java', [ + 'random stuff' + ]), + MockAffectedFile('CorrectTest.java', [ + '@RunWith(ABC.class);' + 'public class CorrectTest {', + '}', + ]), + MockAffectedFile('HistoricallyIncorrectTest.java', [ + 'public class Test extends BaseCaseA {', + '}', + ], old_contents=[ + 'public class Test extends BaseCaseB {', + '}', + ]), + MockAffectedFile('CorrectTestWithInterface.java', [ + '@RunWith(ABC.class);' + 'public class CorrectTest implement Interface {', + '}', + ]), + MockAffectedFile('IncorrectTest.java', [ + 'public class IncorrectTest extends TestCase {', + '}', + ]), + MockAffectedFile('IncorrectTestWithInterface.java', [ + 'public class Test implements X extends BaseClass {', + '}', + ]), + MockAffectedFile('IncorrectTestMultiLine.java', [ + 'public class Test implements X, Y, Z', + ' extends TestBase {', + '}', + ]), + ] + msgs = PRESUBMIT._CheckAndroidTestJUnitInheritance( + mock_input_api, mock_output_api) + self.assertEqual(1, len(msgs), + 'Expected %d items, found %d: %s' + % (1, len(msgs), msgs)) + self.assertEqual(3, len(msgs[0].items), + 'Expected %d items, found %d: %s' + % (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' + in msgs[0].items, + 'IncorrectTestWithInterface not found in errors') + self.assertTrue('IncorrectTestMultiLine.java:2' in msgs[0].items, + 'IncorrectTestMultiLine not found in errors') class LogUsageTest(unittest.TestCase): diff --git a/PRESUBMIT_test_mocks.py b/PRESUBMIT_test_mocks.py index 162570ef4c85b..c6aa84ab60027 100644 --- a/PRESUBMIT_test_mocks.py +++ b/PRESUBMIT_test_mocks.py @@ -97,13 +97,14 @@ class MockFile(object): MockInputApi for presubmit unittests. """ - def __init__(self, local_path, new_contents, action='A'): + def __init__(self, local_path, new_contents, old_contents=None, action='A'): self._local_path = local_path self._new_contents = new_contents self._changed_contents = [(i + 1, l) for i, l in enumerate(new_contents)] self._action = action self._scm_diff = "--- /dev/null\n+++ %s\n@@ -0,0 +1,%d @@\n" % (local_path, len(new_contents)) + self._old_contents = old_contents for l in new_contents: self._scm_diff += "+%s\n" % l @@ -125,6 +126,9 @@ class MockFile(object): def GenerateScmDiff(self): return self._scm_diff + def OldContents(self): + return self._old_contents + def rfind(self, p): """os.path.basename is called on MockFile so we need an rfind method.""" return self._local_path.rfind(p)