Add a PRESUBMIT to ensure Robolectric tests use BaseRobolectricTestRunner
Bug: 383779576 Change-Id: Ica45a4591554acb2ff2f1c7f658185de0353ca5f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6092006 Commit-Queue: Yaron Friedman <yfriedman@chromium.org> Auto-Submit: Andrew Grieve <agrieve@chromium.org> Reviewed-by: Yaron Friedman <yfriedman@chromium.org> Cr-Commit-Position: refs/heads/main@{#1395951}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
99fcffa51c
commit
5a66ae7641
27
PRESUBMIT.py
27
PRESUBMIT.py
@@ -7363,19 +7363,20 @@ def CheckPythonShebang(input_api, output_api):
|
|||||||
return result
|
return result
|
||||||
|
|
||||||
|
|
||||||
def CheckBatchAnnotation(input_api, output_api):
|
def CheckAndroidTestAnnotations(input_api, output_api):
|
||||||
"""Checks that tests have either @Batch or @DoNotBatch annotation. If this
|
"""Checks that tests have either @Batch or @DoNotBatch annotation. If this
|
||||||
is not an instrumentation test, disregard."""
|
is not an instrumentation test, disregard."""
|
||||||
|
|
||||||
batch_annotation = input_api.re.compile(r'^\s*@Batch')
|
batch_annotation = input_api.re.compile(r'^\s*@Batch')
|
||||||
do_not_batch_annotation = input_api.re.compile(r'^\s*@DoNotBatch')
|
do_not_batch_annotation = input_api.re.compile(r'^\s*@DoNotBatch')
|
||||||
robolectric_test = input_api.re.compile(r'[rR]obolectric')
|
robolectric_test = input_api.re.compile(r'@RunWith\((.*?)RobolectricTestRunner')
|
||||||
test_class_declaration = input_api.re.compile(r'^\s*public\sclass.*Test')
|
test_class_declaration = input_api.re.compile(r'^\s*public\sclass.*Test')
|
||||||
uiautomator_test = input_api.re.compile(r'[uU]i[aA]utomator')
|
uiautomator_test = input_api.re.compile(r'[uU]i[aA]utomator')
|
||||||
test_annotation_declaration = input_api.re.compile(r'^\s*public\s@interface\s.*{')
|
test_annotation_declaration = input_api.re.compile(r'^\s*public\s@interface\s.*{')
|
||||||
|
|
||||||
missing_annotation_errors = []
|
missing_annotation_errors = []
|
||||||
extra_annotation_errors = []
|
extra_annotation_errors = []
|
||||||
|
wrong_robolectric_test_runner_errors = []
|
||||||
|
|
||||||
def _FilterFile(affected_file):
|
def _FilterFile(affected_file):
|
||||||
return input_api.FilterSourceFile(
|
return input_api.FilterSourceFile(
|
||||||
@@ -7388,9 +7389,20 @@ def CheckBatchAnnotation(input_api, output_api):
|
|||||||
do_not_batch_matched = None
|
do_not_batch_matched = None
|
||||||
is_instrumentation_test = True
|
is_instrumentation_test = True
|
||||||
test_annotation_declaration_matched = None
|
test_annotation_declaration_matched = None
|
||||||
|
has_base_robolectric_rule = False
|
||||||
for line in f.NewContents():
|
for line in f.NewContents():
|
||||||
if robolectric_test.search(line) or uiautomator_test.search(line):
|
if 'BaseRobolectricTestRule' in line:
|
||||||
# Skip Robolectric and UiAutomator tests.
|
has_base_robolectric_rule = True
|
||||||
|
continue
|
||||||
|
if m := robolectric_test.search(line):
|
||||||
|
is_instrumentation_test = False
|
||||||
|
if m.group(1) == '' and not has_base_robolectric_rule:
|
||||||
|
path = str(f.LocalPath())
|
||||||
|
# These two spots cannot use it.
|
||||||
|
if 'webapk' not in path and 'build' not in path:
|
||||||
|
wrong_robolectric_test_runner_errors.append(path)
|
||||||
|
break
|
||||||
|
if uiautomator_test.search(line):
|
||||||
is_instrumentation_test = False
|
is_instrumentation_test = False
|
||||||
break
|
break
|
||||||
if not batch_matched:
|
if not batch_matched:
|
||||||
@@ -7432,6 +7444,13 @@ See https://source.chromium.org/chromium/chromium/src/+/main:docs/testing/batchi
|
|||||||
"""
|
"""
|
||||||
Robolectric tests do not need a @Batch or @DoNotBatch annotations.
|
Robolectric tests do not need a @Batch or @DoNotBatch annotations.
|
||||||
""", extra_annotation_errors))
|
""", extra_annotation_errors))
|
||||||
|
if wrong_robolectric_test_runner_errors:
|
||||||
|
results.append(
|
||||||
|
output_api.PresubmitPromptWarning(
|
||||||
|
"""
|
||||||
|
Robolectric tests should use either @RunWith(BaseRobolectricTestRule.class) (or
|
||||||
|
a subclass of it), or use "@Rule BaseRobolectricTestRule".
|
||||||
|
""", wrong_robolectric_test_runner_errors))
|
||||||
|
|
||||||
return results
|
return results
|
||||||
|
|
||||||
|
@@ -4998,10 +4998,10 @@ class VerifyDcheckParentheses(unittest.TestCase):
|
|||||||
self.assertRegex(error.message, r'DCHECK_IS_ON().+parentheses')
|
self.assertRegex(error.message, r'DCHECK_IS_ON().+parentheses')
|
||||||
|
|
||||||
|
|
||||||
class CheckBatchAnnotation(unittest.TestCase):
|
class CheckAndroidTestAnnotations(unittest.TestCase):
|
||||||
"""Test the CheckBatchAnnotation presubmit check."""
|
"""Test the CheckAndroidTestAnnotations presubmit check."""
|
||||||
|
|
||||||
def testTruePositives(self):
|
def testBatchTruePositives(self):
|
||||||
"""Examples of when there is no @Batch or @DoNotBatch is correctly flagged.
|
"""Examples of when there is no @Batch or @DoNotBatch is correctly flagged.
|
||||||
"""
|
"""
|
||||||
mock_input = MockInputApi()
|
mock_input = MockInputApi()
|
||||||
@@ -5010,16 +5010,16 @@ class CheckBatchAnnotation(unittest.TestCase):
|
|||||||
MockFile('path/TwoTest.java', ['public class TwoTest']),
|
MockFile('path/TwoTest.java', ['public class TwoTest']),
|
||||||
MockFile('path/ThreeTest.java', [
|
MockFile('path/ThreeTest.java', [
|
||||||
'@Batch(Batch.PER_CLASS)',
|
'@Batch(Batch.PER_CLASS)',
|
||||||
'import org.chromium.base.test.BaseRobolectricTestRunner;',
|
'@RunWith(BaseRobolectricTestRunner.class)',
|
||||||
'public class Three {'
|
'public class Three {'
|
||||||
]),
|
]),
|
||||||
MockFile('path/FourTest.java', [
|
MockFile('path/FourTest.java', [
|
||||||
'@DoNotBatch(reason = "placeholder reason 1")',
|
'@DoNotBatch(reason = "placeholder reason 1")',
|
||||||
'import org.chromium.base.test.BaseRobolectricTestRunner;',
|
'@RunWith(BaseRobolectricTestRunner.class)',
|
||||||
'public class Four {'
|
'public class Four {'
|
||||||
]),
|
]),
|
||||||
]
|
]
|
||||||
errors = PRESUBMIT.CheckBatchAnnotation(mock_input, MockOutputApi())
|
errors = PRESUBMIT.CheckAndroidTestAnnotations(mock_input, MockOutputApi())
|
||||||
self.assertEqual(2, len(errors))
|
self.assertEqual(2, len(errors))
|
||||||
self.assertEqual(2, len(errors[0].items))
|
self.assertEqual(2, len(errors[0].items))
|
||||||
self.assertIn('OneTest.java', errors[0].items[0])
|
self.assertIn('OneTest.java', errors[0].items[0])
|
||||||
@@ -5028,7 +5028,7 @@ class CheckBatchAnnotation(unittest.TestCase):
|
|||||||
self.assertIn('ThreeTest.java', errors[1].items[0])
|
self.assertIn('ThreeTest.java', errors[1].items[0])
|
||||||
self.assertIn('FourTest.java', errors[1].items[1])
|
self.assertIn('FourTest.java', errors[1].items[1])
|
||||||
|
|
||||||
def testAnnotationsPresent(self):
|
def testBatchAnnotationsPresent(self):
|
||||||
"""Examples of when there is @Batch or @DoNotBatch is correctly flagged."""
|
"""Examples of when there is @Batch or @DoNotBatch is correctly flagged."""
|
||||||
mock_input = MockInputApi()
|
mock_input = MockInputApi()
|
||||||
mock_input.files = [
|
mock_input.files = [
|
||||||
@@ -5060,17 +5060,17 @@ class CheckBatchAnnotation(unittest.TestCase):
|
|||||||
'public class Five extends BaseTestB {'
|
'public class Five extends BaseTestB {'
|
||||||
]),
|
]),
|
||||||
MockFile('path/SixTest.java', [
|
MockFile('path/SixTest.java', [
|
||||||
'import org.chromium.base.test.BaseRobolectricTestRunner;',
|
'@RunWith(BaseRobolectricTestRunner.class)',
|
||||||
'public class Six extends BaseTestA {'
|
'public class Six extends BaseTestA {'
|
||||||
], [
|
], [
|
||||||
'import org.chromium.base.test.BaseRobolectricTestRunner;',
|
'@RunWith(BaseRobolectricTestRunner.class)',
|
||||||
'public class Six extends BaseTestB {'
|
'public class Six extends BaseTestB {'
|
||||||
]),
|
]),
|
||||||
MockFile('path/SevenTest.java', [
|
MockFile('path/SevenTest.java', [
|
||||||
'import org.robolectric.annotation.Config;',
|
'@RunWith(BaseRobolectricTestRunner.class)',
|
||||||
'public class Seven extends BaseTestA {'
|
'public class Seven extends BaseTestA {'
|
||||||
], [
|
], [
|
||||||
'import org.robolectric.annotation.Config;',
|
'@RunWith(BaseRobolectricTestRunner.class)',
|
||||||
'public class Seven extends BaseTestB {'
|
'public class Seven extends BaseTestB {'
|
||||||
]),
|
]),
|
||||||
MockFile(
|
MockFile(
|
||||||
@@ -5086,9 +5086,35 @@ class CheckBatchAnnotation(unittest.TestCase):
|
|||||||
['public @interface SomeAnnotation {'],
|
['public @interface SomeAnnotation {'],
|
||||||
),
|
),
|
||||||
]
|
]
|
||||||
errors = PRESUBMIT.CheckBatchAnnotation(mock_input, MockOutputApi())
|
errors = PRESUBMIT.CheckAndroidTestAnnotations(mock_input, MockOutputApi())
|
||||||
self.assertEqual(0, len(errors))
|
self.assertEqual(0, len(errors))
|
||||||
|
|
||||||
|
def testWrongRobolectricTestRunner(self):
|
||||||
|
mock_input = MockInputApi()
|
||||||
|
mock_input.files = [
|
||||||
|
MockFile('path/OneTest.java', [
|
||||||
|
'@RunWith(RobolectricTestRunner.class)',
|
||||||
|
'public class ThreeTest {'
|
||||||
|
]),
|
||||||
|
MockFile('path/TwoTest.java', [
|
||||||
|
'import org.chromium.base.test.BaseRobolectricTestRule;',
|
||||||
|
'@RunWith(RobolectricTestRunner.class)',
|
||||||
|
'public class TwoTest {'
|
||||||
|
]),
|
||||||
|
MockFile('path/ThreeTest.java', [
|
||||||
|
'@RunWith(FooRobolectricTestRunner.class)',
|
||||||
|
'public class ThreeTest {'
|
||||||
|
]),
|
||||||
|
MockFile('webapks/FourTest.java', [
|
||||||
|
'@RunWith(RobolectricTestRunner.class)',
|
||||||
|
'public class ThreeTest {'
|
||||||
|
]),
|
||||||
|
]
|
||||||
|
errors = PRESUBMIT.CheckAndroidTestAnnotations(mock_input, MockOutputApi())
|
||||||
|
self.assertEqual(1, len(errors))
|
||||||
|
self.assertEqual(1, len(errors[0].items))
|
||||||
|
self.assertIn('OneTest.java', errors[0].items[0])
|
||||||
|
|
||||||
|
|
||||||
class CheckMockAnnotation(unittest.TestCase):
|
class CheckMockAnnotation(unittest.TestCase):
|
||||||
"""Test the CheckMockAnnotation presubmit check."""
|
"""Test the CheckMockAnnotation presubmit check."""
|
||||||
|
Reference in New Issue
Block a user