Add PRESUBMIT for android/javatests to encourage @Batch.

If tests are not safe to run in a batch, it should explicitly have
@DoNotBatch annotation with reasons.

Bug: 1330328
Change-Id: If94c5c3de521b5248813bc98b4de0740b43429e5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3699290
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: James Shen <zhiyuans@google.com>
Cr-Commit-Position: refs/heads/main@{#1014636}

PRESUBMIT.py

@@ -6014,3 +6014,56 @@ def CheckPythonShebang(input_api, output_api):
                 "Please use '#!/usr/bin/env python/2/3' as the shebang of %s" %
                 file))
     return result
+
+
+def CheckBatchAnnotation(input_api, output_api):
+    """Checks that tests have either @Batch or @DoNotBatch annotation. If this
+    is not an instrumentation test, disregard."""
+
+    batch_annotation = input_api.re.compile(r'^\s*@Batch')
+    do_not_batch_annotation = input_api.re.compile(r'^\s*@DoNotBatch')
+    robolectric_test = input_api.re.compile(r'[rR]obolectric')
+    test_class_declaration = input_api.re.compile(r'^\s*public\sclass.*Test')
+    uiautomator_test = input_api.re.compile(r'[uU]i[aA]utomator')
+
+    errors = []
+
+    def _FilterFile(affected_file):
+        return input_api.FilterSourceFile(
+            affected_file,
+            files_to_skip=input_api.DEFAULT_FILES_TO_SKIP,
+            files_to_check=[r'.*Test\.java$'])
+
+    for f in input_api.AffectedSourceFiles(_FilterFile):
+        batch_matched = None
+        do_not_batch_matched = None
+        is_instrumentation_test = True
+        for line in f.NewContents():
+            if robolectric_test.search(line) or uiautomator_test.search(line):
+                # Skip Robolectric and UiAutomator tests.
+                is_instrumentation_test = False
+                break
+            if not batch_matched:
+                batch_matched = batch_annotation.search(line)
+            if not do_not_batch_matched:
+                do_not_batch_matched = do_not_batch_annotation.search(line)
+            test_class_declaration_matched = test_class_declaration.search(
+                line)
+            if test_class_declaration_matched:
+                break
+        if (is_instrumentation_test and
+            not batch_matched and
+            not do_not_batch_matched):
+          errors.append(str(f.LocalPath()))
+
+    results = []
+
+    if errors:
+        results.append(
+            output_api.PresubmitPromptWarning(
+                """
+Instrumentation tests should use either @Batch or @DoNotBatch. If tests are not
+safe to run in batch, please use @DoNotBatch with reasons.
+""", errors))
+
+    return results

PRESUBMIT_test.py

@@ -4546,5 +4546,68 @@ def testMissingParentheses(self):
     for error in errors:
       self.assertRegex(error.message, r'DCHECK_IS_ON().+parentheses')
 
+class CheckBatchAnnotation(unittest.TestCase):
+  """Test the CheckBatchAnnotation presubmit check."""
+
+  def testTruePositives(self):
+    """Examples of when there is no @Batch or @DoNotBatch is correctly flagged.
+"""
+    mock_input = MockInputApi()
+    mock_input.files = [
+        MockFile('path/OneTest.java', ['public class OneTest']),
+        MockFile('path/TwoTest.java', ['public class TwoTest']),
+    ]
+    errors = PRESUBMIT.CheckBatchAnnotation(mock_input, MockOutputApi())
+    self.assertEqual(1, len(errors))
+    self.assertEqual(2, len(errors[0].items))
+    self.assertIn('OneTest.java', errors[0].items[0])
+    self.assertIn('TwoTest.java', errors[0].items[1])
+
+  def testAnnotationsPresent(self):
+    """Examples of when there is @Batch or @DoNotBatch is correctly flagged."""
+    mock_input = MockInputApi()
+    mock_input.files = [
+        MockFile('path/OneTest.java',
+                 ['@Batch(Batch.PER_CLASS)', 'public class One {']),
+        MockFile('path/TwoTest.java',
+                 ['@DoNotBatch(reason = "dummy reasons.")', 'public class Two {'
+                 ]),
+        MockFile('path/ThreeTest.java',
+                 ['@Batch(Batch.PER_CLASS)',
+                  'public class Three extends BaseTestA {'],
+                 ['@Batch(Batch.PER_CLASS)',
+                  'public class Three extends BaseTestB {']),
+        MockFile('path/FourTest.java',
+                 ['@DoNotBatch(reason = "dummy reason 1")',
+                  'public class Four extends BaseTestA {'],
+                 ['@DoNotBatch(reason = "dummy reason 2")',
+                  'public class Four extends BaseTestB {']),
+        MockFile('path/FiveTest.java',
+                 ['import androidx.test.uiautomator.UiDevice;',
+                  'public class Five extends BaseTestA {'],
+                 ['import androidx.test.uiautomator.UiDevice;',
+                  'public class Five extends BaseTestB {']),
+        MockFile('path/SixTest.java',
+                 ['import org.chromium.base.test.BaseRobolectricTestRunner;',
+                  'public class Six extends BaseTestA {'],
+                 ['import org.chromium.base.test.BaseRobolectricTestRunner;',
+                  'public class Six extends BaseTestB {']),
+        MockFile('path/SevenTest.java',
+                 ['import org.robolectric.annotation.Config;',
+                  'public class Seven extends BaseTestA {'],
+                 ['import org.robolectric.annotation.Config;',
+                  'public class Seven extends BaseTestB {']),
+        MockFile(
+            'path/OtherClass.java',
+            ['public class OtherClass {'],
+        ),
+        MockFile('path/PRESUBMIT.py',
+                 ['@Batch(Batch.PER_CLASS)',
+                  '@DoNotBatch(reason = "dummy reason)']),
+    ]
+    errors = PRESUBMIT.CheckBatchAnnotation(mock_input, MockOutputApi())
+    self.assertEqual(0, len(errors))
+
+
 if __name__ == '__main__':
   unittest.main()