Added parsing test output for interrupted builds.

If tests run is interrupted(e.g. lost connection to testservice,
test was stuck and no output for 3 mins and was killed then),
xcodebuild_runner needs to parse test_output, collect passed tests
and re-run test_bundle by filtering already passed tests
(e.g. https://chromium-swarm.appspot.com/task?id=45c892feae862110).
Removed _make_cmd_list_for_failed_tests because now tests
will filter by passed tests for both cases
when tests were interrupted and not.

At the end of run also add a check whether all tests from test-bundle
were executed and if not add 'not executed tests' record.

Created the radar
'After primaryInstrumentsServerWithError xcodebuild did not finish its execution'
https://feedbackassistant.apple.com/feedback/6476415

Bug: 979267
Change-Id: I596945e10bd8382c41487633456a3c80a3569419
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1680973Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Maksym Onufriienko <monufriienko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#675844}

ios/build/bots/scripts/test_runner.py

@@ -334,6 +334,23 @@ def get_current_xcode_info():
   }
 
 
+def get_test_names(app_path):
+  """Gets list of tests from test app.
+
+  Args:
+     app_path: A path to test target bundle.
+
+  Returns:
+     List of tests.
+  """
+  cmd = ['otool', '-ov', app_path]
+  test_pattern = re.compile(
+      'imp (?:0[xX][0-9a-fA-F]+ )?-\['
+      '(?P<testSuite>[A-Za-z_][A-Za-z0-9_]*Test(?:Case)?)\s'
+      '(?P<testMethod>test[A-Za-z0-9_]*)\]')
+  return test_pattern.findall(subprocess.check_output(cmd))
+
+
 def shard_xctest(object_path, shards, test_cases=None):
   """Gets EarlGrey test methods inside a test target and splits them into shards
 
@@ -345,12 +362,7 @@ def shard_xctest(object_path, shards, test_cases=None):
   Returns:
     A list of test shards.
   """
-  cmd = ['otool', '-ov', object_path]
-  test_pattern = re.compile(
-    'imp -\[(?P<testSuite>[A-Za-z_][A-Za-z0-9_]*Test[Case]*) '
-    '(?P<testMethod>test[A-Za-z0-9_]*)\]')
-  test_names = test_pattern.findall(subprocess.check_output(cmd))
-
+  test_names = get_test_names(object_path)
   # If test_cases are passed in, only shard the intersection of them and the
   # listed tests.  Format of passed-in test_cases can be either 'testSuite' or
   # 'testSuite/testMethod'.  The listed tests are tuples of ('testSuite',

ios/build/bots/scripts/test_runner_test.py

@@ -8,6 +8,7 @@
 import collections
 import glob
 import logging
+import mock
 import os
 import subprocess
 import unittest
@@ -604,6 +605,37 @@ class DeviceTestRunnerTest(TestCase):
         ['a', 'b', 'c', 'd']
     )
 
+  @mock.patch('subprocess.check_output', autospec=True)
+  def test_get_test_names(self, mock_subprocess):
+    otool_output = (
+        'imp 0x102492020 -[BrowserViewControllerTestCase testJavaScript]'
+        'name 0x105ee8b84 testFixForCrbug801165'
+        'types 0x105f0c842 v16 @ 0:8'
+        'name 0x105ee8b9a testOpenURLFromNTP'
+        'types 0x105f0c842 v16 @ 0:8'
+        'imp 0x102493b30 -[BrowserViewControllerTestCase testOpenURLFromNTP]'
+        'name 0x105ee8bad testOpenURLFromTab'
+        'types 0x105f0c842 v16 @ 0:8'
+        'imp 0x102494180 -[BrowserViewControllerTestCase testOpenURLFromTab]'
+        'name 0x105ee8bc0 testOpenURLFromTabSwitcher'
+        'types 0x105f0c842 v16 @ 0:8'
+        'imp 0x102494f70 -[BrowserViewControllerTestCase testTabSwitch]'
+        'types 0x105f0c842 v16 @ 0:8'
+        'imp 0x102494f70 -[BrowserViewControllerTestCase helper]'
+        'imp 0x102494f70 -[BrowserViewControllerTestCCCCCCCCC testMethod]'
+    )
+    mock_subprocess.return_value = otool_output
+    tests = test_runner.get_test_names('')
+    self.assertEqual(
+        [
+            ('BrowserViewControllerTestCase', 'testJavaScript'),
+            ('BrowserViewControllerTestCase', 'testOpenURLFromNTP'),
+            ('BrowserViewControllerTestCase', 'testOpenURLFromTab'),
+            ('BrowserViewControllerTestCase', 'testTabSwitch')
+        ],
+        tests
+    )
+
 
 if __name__ == '__main__':
   logging.basicConfig(format='[%(asctime)s:%(levelname)s] %(message)s',

ios/build/bots/scripts/xcode_log_parser.py

@@ -18,6 +18,28 @@ import test_runner
 LOGGER = logging.getLogger(__name__)
 
 
+def parse_passed_tests_for_interrupted_run(output):
+  """Parses xcode runner output to get passed tests only.
+
+  Args:
+    output: [str] An output of test run.
+
+  Returns:
+    The list of passed tests only that will be a filter for next attempt.
+  """
+  passed_tests = []
+  # Test has format:
+  # [09:04:42:INFO] Test case '-[Test_class test_method]' passed.
+  passed_test_regex = re.compile(r'Test case \'\-\[(.+?)\s(.+?)\]\' passed')
+
+  for test_line in output:
+    m_test = passed_test_regex.search(test_line)
+    if m_test:
+      passed_tests.append('%s/%s' % (m_test.group(1), m_test.group(2)))
+  LOGGER.info('%d passed tests for interrupted build.' % len(passed_tests))
+  return passed_tests
+
+
 def format_test_case(test_case):
   """Format test case from `-[TestClass TestMethod]` to `TestClass_TestMethod`.
 
@@ -162,11 +184,12 @@ class Xcode11LogParser(object):
     return passed_tests
 
   @staticmethod
-  def collect_test_results(xcresult):
+  def collect_test_results(xcresult, output):
     """Gets test result data from xcresult.
 
     Args:
       xcresult: (str) A path to xcresult.
+      output: [str] An output of test run.
 
     Returns:
       Test result as a map:
@@ -190,7 +213,8 @@ class Xcode11LogParser(object):
     plist_path = os.path.join(xcresult + '.xcresult', 'Info.plist')
     if not os.path.exists(plist_path):
       test_results['failed']['BUILD_INTERRUPTED'] = [
-          '%s with test results does not exist.' % plist_path]
+          '%s with test results does not exist.' % plist_path] + output
+      test_results['passed'] = parse_passed_tests_for_interrupted_run(output)
       return test_results
 
     root = json.loads(Xcode11LogParser._xcresulttool_get(xcresult))
@@ -280,11 +304,12 @@ class XcodeLogParser(object):
     return status_summary
 
   @staticmethod
-  def collect_test_results(output_folder):
+  def collect_test_results(output_folder, output):
     """Gets test result data from Info.plist.
 
     Args:
       output_folder: (str) A path to output folder.
+      output: [str] An output of test run.
     Returns:
       Test result as a map:
         {
@@ -301,7 +326,8 @@ class XcodeLogParser(object):
     plist_path = os.path.join(output_folder, 'Info.plist')
     if not os.path.exists(plist_path):
       test_results['failed']['BUILD_INTERRUPTED'] = [
-          '%s with test results does not exist.' % plist_path]
+          '%s with test results does not exist.' % plist_path] + output
+      test_results['passed'] = parse_passed_tests_for_interrupted_run(output)
       return test_results
 
     root = plistlib.readPlist(plist_path)

ios/build/bots/scripts/xcode_log_parser_test.py

@@ -203,7 +203,7 @@ class XCode11LogParserTest(test_runner_test.TestCase):
     mock_exist_file.return_value = True
     self.assertEqual(expected_test_results,
                      xcode_log_parser.Xcode11LogParser().collect_test_results(
-                         _XTEST_RESULT))
+                         _XTEST_RESULT, []))
 
   @mock.patch('os.path.exists', autospec=True)
   @mock.patch('xcode_log_parser.Xcode11LogParser._xcresulttool_get')
@@ -216,7 +216,7 @@ class XCode11LogParserTest(test_runner_test.TestCase):
     mock_exist_file.return_value = True
     self.assertEqual(expected_test_results,
                      xcode_log_parser.Xcode11LogParser().collect_test_results(
-                         _XTEST_RESULT))
+                         _XTEST_RESULT, []))
 
   @mock.patch('os.path.exists', autospec=True)
   def testCollectTestsDidNotRun(self, mock_exist_file):
@@ -227,7 +227,7 @@ class XCode11LogParserTest(test_runner_test.TestCase):
             '%s with test results does not exist.' % _XTEST_RESULT]}}
     self.assertEqual(expected_test_results,
                      xcode_log_parser.Xcode11LogParser().collect_test_results(
-                         _XTEST_RESULT))
+                         _XTEST_RESULT, []))
 
   @mock.patch('os.path.exists', autospec=True)
   def testCollectTestsInterruptedRun(self, mock_exist_file):
@@ -239,7 +239,7 @@ class XCode11LogParserTest(test_runner_test.TestCase):
                 _XTEST_RESULT + '.xcresult', 'Info.plist')]}}
     self.assertEqual(expected_test_results,
                      xcode_log_parser.Xcode11LogParser().collect_test_results(
-                         _XTEST_RESULT))
+                         _XTEST_RESULT, []))
 
   @mock.patch('os.path.exists', autospec=True)
   @mock.patch('xcode_log_parser.Xcode11LogParser._xcresulttool_get')
@@ -250,3 +250,22 @@ class XCode11LogParserTest(test_runner_test.TestCase):
     mock_xcresulttool_get.return_value = ACTIONS_RECORD_FAILED_TEST
     xcode_log_parser.Xcode11LogParser().copy_screenshots(_XTEST_RESULT)
     self.assertEqual(1, mock_copy.call_count)
+
+  @mock.patch('os.path.exists', autospec=True)
+  def testCollectTestResults_interruptedTests(self, mock_path_exists):
+    mock_path_exists.side_effect = [True, False]
+    output = [
+        '[09:03:42:INFO] Test case \'-[TestCase1 method1]\' passed on device.',
+        '[09:06:40:INFO] Test case \'-[TestCase2 method1]\' passed on device.',
+        '[09:09:00:INFO] Test case \'-[TestCase2 method1]\' failed on device.',
+        '** BUILD INTERRUPTED **',
+    ]
+    not_found_message = [
+        'Info.plist.xcresult/Info.plist with test results does not exist.']
+    res = xcode_log_parser.Xcode11LogParser().collect_test_results(
+        'Info.plist', output)
+    self.assertIn('BUILD_INTERRUPTED', res['failed'])
+    self.assertEqual(not_found_message + output,
+                     res['failed']['BUILD_INTERRUPTED'])
+    self.assertEqual(['TestCase1/method1', 'TestCase2/method1'],
+                     res['passed'])

ios/build/bots/scripts/xcodebuild_runner_test.py

@@ -141,7 +141,7 @@ class XCodebuildRunnerTest(test_runner_test.TestCase):
     filtered_tests = ['TestCase1/testMethod1', 'TestCase1/testMethod2',
                       'TestCase2/testMethod1', 'TestCase1/testMethod2']
     egtest_node = xcodebuild_runner.EgtestsApp(
-        _EGTESTS_APP_PATH, filtered_tests=filtered_tests).xctestrun_node()[
+        _EGTESTS_APP_PATH, included_tests=filtered_tests).xctestrun_node()[
             'any_egtests_module']
     self.assertEqual(filtered_tests, egtest_node['OnlyTestIdentifiers'])
     self.assertNotIn('SkipTestIdentifiers', egtest_node)
@@ -153,8 +153,8 @@ class XCodebuildRunnerTest(test_runner_test.TestCase):
     skipped_tests = ['TestCase1/testMethod1', 'TestCase1/testMethod2',
                      'TestCase2/testMethod1', 'TestCase1/testMethod2']
     egtest_node = xcodebuild_runner.EgtestsApp(
-        _EGTESTS_APP_PATH, filtered_tests=skipped_tests,
-        invert=True).xctestrun_node()['any_egtests_module']
+        _EGTESTS_APP_PATH, excluded_tests=skipped_tests
+        ).xctestrun_node()['any_egtests_module']
     self.assertEqual(skipped_tests, egtest_node['SkipTestIdentifiers'])
     self.assertNotIn('OnlyTestIdentifiers', egtest_node)
 
@@ -195,42 +195,6 @@ class XCodebuildRunnerTest(test_runner_test.TestCase):
       xcodebuild_runner.LaunchCommand([], 'destination', shards=1, retries=1,
                                       out_dir=_OUT_DIR).fill_xctest_run([])
 
-  @mock.patch('xcodebuild_runner.LaunchCommand.fill_xctest_run', autospec=True)
-  def testLaunchCommand_make_cmd_list_for_failed_tests(self,
-                                                       fill_xctest_run_mock):
-    fill_xctest_run_mock.side_effect = [
-        '/var/folders/tmpfile1'
-    ]
-    egtest_app = 'module_1_egtests.app'
-    egtest_app_path = '%s/%s' % (_ROOT_FOLDER_PATH, egtest_app)
-    host_app_path = '%s/%s' % (_ROOT_FOLDER_PATH, egtest_app)
-    failed_tests = {
-        egtest_app: [
-            'TestCase1_1/TestMethod1',
-            'TestCase1_1/TestMethod2',
-            'TestCase1_2/TestMethod1',
-        ]
-    }
-    expected_egtests = xcodebuild_runner.EgtestsApp(
-        egtest_app_path, filtered_tests=failed_tests[egtest_app])
-    mock_egtest = mock.MagicMock(spec=xcodebuild_runner.EgtestsApp)
-    type(mock_egtest).egtests_path = mock.PropertyMock(
-        return_value=egtest_app_path)
-    type(mock_egtest).host_app_path = mock.PropertyMock(
-        return_value=host_app_path)
-    cmd = xcodebuild_runner.LaunchCommand(
-        egtests_app=mock_egtest,
-        destination=_DESTINATION,
-        out_dir='out/dir/attempt_2/iPhone X 12.0',
-        shards=1,
-        retries=1
-    )
-    cmd._make_cmd_list_for_failed_tests(
-        failed_tests, os.path.join(_OUT_DIR, 'attempt_2'))
-    self.assertEqual(1, len(fill_xctest_run_mock.mock_calls))
-    self.assertItemsEqual(expected_egtests.__dict__,
-                          fill_xctest_run_mock.mock_calls[0][1][1].__dict__)
-
   @mock.patch('os.listdir', autospec=True)
   @mock.patch('test_runner.get_current_xcode_info', autospec=True)
   @mock.patch('xcode_log_parser.XcodeLogParser.collect_test_results')
@@ -240,7 +204,7 @@ class XCodebuildRunnerTest(test_runner_test.TestCase):
     egtests = xcodebuild_runner.EgtestsApp(_EGTESTS_APP_PATH)
     xcode_version.return_value = {'version': '10.2.1'}
     mock_collect_results.side_effect = [
-        {'failed': {'TESTS_DID_NOT_START': ['not started']}},
+        {'failed': {'TESTS_DID_NOT_START': ['not started']}, 'passed': []},
         {'failed': {}, 'passed': ['passedTest1']}
     ]
     launch_command = xcodebuild_runner.LaunchCommand(egtests,