[iOS][test runner] Copy artifacts, zip xcresult folder for GTests.

Move some private methods of Xcode(11)LogParser to public, and call
these at tear down of test_runner module for GTests running as XCTests.

To avoid duplicate logic where touched, this change also:
- Added using_xcode_11_or_higher() to xcode_util module.
- Added get_parser() to xcode_log_parser module.
- Renamed coverage_util to file_util and added in an archiving utility.

Bug: 1047704
Change-Id: Id66ebd7a4ca1b2f7ea494eda66c015240f581c13
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2496345
Commit-Queue: Zhaoyang Li <zhaoyangli@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823380}

ios/build/bots/scripts/PRESUBMIT.py

@@ -13,7 +13,7 @@ def _RunTestRunnerUnitTests(input_api, output_api):
   # TODO(crbug.com/1056457): Replace the list with regex ".*_test.py" once
   # all test files are fixed.
   files = [
-      'coverage_util_test.py',
+      'file_util_test.py',
       'iossim_util_test.py',
       'result_sink_util_test.py',
       'run_test.py',

ios/build/bots/scripts/coverage_util.py → ios/build/bots/scripts/file_util.py

 # Copyright 2020 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
-"""Utility functions related with code coverage."""
+"""Utility functions operating with files."""
 
 import glob
 import os
@@ -26,3 +26,15 @@ def move_raw_coverage_data(udid, isolated_output_dir):
     os.mkdir(profraw_destination_dir)
   for profraw_file in glob.glob(os.path.join(profraw_origin_dir, '*.profraw')):
     shutil.move(profraw_file, profraw_destination_dir)
+
+
+def zip_and_remove_folder(dir_path):
+  """Zips folder storing in the parent folder and then removes original folder.
+
+  Args:
+    dir_path: (str) An absolute path to directory.
+  """
+  shutil.make_archive(
+      os.path.join(os.path.dirname(dir_path), os.path.basename(dir_path)),
+      'zip', dir_path)
+  shutil.rmtree(dir_path)

ios/build/bots/scripts/coverage_util_test.py → ios/build/bots/scripts/file_util_test.py

 # Copyright 2020 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
-"""Tests of coverage_util functions."""
+"""Tests of file_util functions."""
 
 import os
 import shutil
 import unittest
 
-import coverage_util
+import file_util
 import test_runner_test
 
 
 class TestCoverageUtil(test_runner_test.TestCase):
-  """Test cases for coverage_util.py"""
+  """Test cases for file_util.py"""
 
   def create_origin_profraw_file_if_not_exist(self):
     """Creates the profraw file in the correct udid data folder to move if it
@@ -25,7 +25,7 @@ class TestCoverageUtil(test_runner_test.TestCase):
   def setUp(self):
     super(TestCoverageUtil, self).setUp()
 
-    self.test_folder = os.path.join(os.getcwd(), "coverage_util_test_data")
+    self.test_folder = os.path.join(os.getcwd(), "file_util_test_data")
     self.simulators_folder = os.path.join(self.test_folder, "Devices")
     self.existing_udid = "existing-udid"
     self.existing_udid_folder = os.path.join(self.simulators_folder,
@@ -40,8 +40,9 @@ class TestCoverageUtil(test_runner_test.TestCase):
                                                  self.profraw_file_name)
 
     self.not_existing_udid = "not-existing-udid"
-    self.not_existing_udid_data_folder = os.path.join(
-        self.simulators_folder, self.not_existing_udid, "data")
+    self.not_existing_udid_data_folder = os.path.join(self.simulators_folder,
+                                                      self.not_existing_udid,
+                                                      "data")
     if os.path.exists(self.not_existing_udid_data_folder):
       shutil.rmtree(self.not_existing_udid_data_folder)
 
@@ -49,31 +50,31 @@ class TestCoverageUtil(test_runner_test.TestCase):
     if not os.path.exists(self.output_folder):
       os.makedirs(self.output_folder)
 
-    self.expected_profraw_output_path = os.path.join(
-        self.output_folder, "profraw", self.profraw_file_name)
+    self.expected_profraw_output_path = os.path.join(self.output_folder,
+                                                     "profraw",
+                                                     self.profraw_file_name)
 
-    self.mock(coverage_util, 'SIMULATORS_FOLDER', self.simulators_folder)
+    self.mock(file_util, 'SIMULATORS_FOLDER', self.simulators_folder)
 
   def tearDown(self):
     shutil.rmtree(self.test_folder)
 
   def test_move_raw_coverage_data(self):
-    """Tests if coverage_util can correctly move raw coverage data"""
+    """Tests if file_util can correctly move raw coverage data"""
     self.create_origin_profraw_file_if_not_exist()
     self.assertTrue(os.path.exists(self.origin_profraw_file_path))
     self.assertFalse(os.path.exists(self.expected_profraw_output_path))
-    coverage_util.move_raw_coverage_data(self.existing_udid, self.output_folder)
+    file_util.move_raw_coverage_data(self.existing_udid, self.output_folder)
     self.assertFalse(os.path.exists(self.origin_profraw_file_path))
     self.assertTrue(os.path.exists(self.expected_profraw_output_path))
     os.remove(self.expected_profraw_output_path)
 
   def test_move_raw_coverage_data_origin_not_exist(self):
-    """Ensures that coverage_util won't break when raw coverage data folder or
+    """Ensures that file_util won't break when raw coverage data folder or
     file doesn't exist
     """
     # Tests origin directory doesn't exist.
-    coverage_util.move_raw_coverage_data(self.not_existing_udid,
-                                         self.output_folder)
+    file_util.move_raw_coverage_data(self.not_existing_udid, self.output_folder)
     self.assertFalse(os.path.exists(self.expected_profraw_output_path))
 
     # Tests profraw file doesn't exist.
@@ -81,7 +82,7 @@ class TestCoverageUtil(test_runner_test.TestCase):
       os.remove(self.origin_profraw_file_path)
     self.assertFalse(os.path.exists(self.origin_profraw_file_path))
     self.assertFalse(os.path.exists(self.expected_profraw_output_path))
-    coverage_util.move_raw_coverage_data(self.existing_udid, self.output_folder)
+    file_util.move_raw_coverage_data(self.existing_udid, self.output_folder)
     self.assertFalse(os.path.exists(self.expected_profraw_output_path))
 
 

ios/build/bots/scripts/test_runner.py

@@ -9,7 +9,6 @@ import signal
 import sys
 
 import collections
-import distutils.version
 import logging
 import os
 import psutil
@@ -19,11 +18,13 @@ import subprocess
 import threading
 import time
 
-import coverage_util
+import file_util
 import gtest_utils
 import iossim_util
 import standard_json_util as sju
 import test_apps
+import xcode_log_parser
+import xcode_util
 import xctest_utils
 
 LOGGER = logging.getLogger(__name__)
@@ -448,6 +449,33 @@ class TestRunner(object):
       shutil.rmtree(DERIVED_DATA)
       os.mkdir(DERIVED_DATA)
 
+  def process_xcresult_dir(self):
+    """Copies artifacts & diagnostic logs, zips and removes .xcresult dir."""
+    # .xcresult dir only exists when using Xcode 11+ and running as XCTest.
+    if not xcode_util.using_xcode_11_or_higher() or not self.xctest:
+      LOGGER.info('Skip processing xcresult directory.')
+
+    xcresult_paths = []
+    # Warning: This piece of code assumes .xcresult folder is directly under
+    # self.out_dir. This is true for TestRunner subclasses in this file.
+    # xcresult folder path is whatever passed in -resultBundlePath to xcodebuild
+    # command appended with '.xcresult' suffix.
+    for filename in os.listdir(self.out_dir):
+      full_path = os.path.join(self.out_dir, filename)
+      if full_path.endswith('.xcresult') and os.path.isdir(full_path):
+        xcresult_paths.append(full_path)
+
+    log_parser = xcode_log_parser.get_parser()
+    for xcresult in xcresult_paths:
+      # This is what was passed in -resultBundlePath to xcodebuild command.
+      result_bundle_path = os.path.splitext(xcresult)[0]
+      log_parser.copy_artifacts(result_bundle_path)
+      log_parser.export_diagnostic_data(result_bundle_path)
+      # result_bundle_path is a symlink to xcresult directory.
+      if os.path.islink(result_bundle_path):
+        os.unlink(result_bundle_path)
+      file_util.zip_and_remove_folder(xcresult)
+
   def run_tests(self, cmd=None):
     """Runs passed-in tests.
 
@@ -787,7 +815,7 @@ class SimulatorTestRunner(TestRunner):
   def extract_test_data(self):
     """Extracts data emitted by the test."""
     if hasattr(self, 'use_clang_coverage') and self.use_clang_coverage:
-      coverage_util.move_raw_coverage_data(self.udid, self.out_dir)
+      file_util.move_raw_coverage_data(self.udid, self.out_dir)
 
     # Find the Documents directory of the test app. The app directory names
     # don't correspond with any known information, so we have to examine them
@@ -842,6 +870,8 @@ class SimulatorTestRunner(TestRunner):
     self.retrieve_crash_reports()
     LOGGER.debug('Retrieving derived data.')
     self.retrieve_derived_data()
+    LOGGER.debug('Processing xcresult folder.')
+    self.process_xcresult_dir()
     LOGGER.debug('Making desktop screenshots.')
     self.screenshot_desktop()
     LOGGER.debug('Killing simulators.')
@@ -1037,6 +1067,7 @@ class DeviceTestRunner(TestRunner):
     self.screenshot_desktop()
     self.retrieve_derived_data()
     self.extract_test_data()
+    self.process_xcresult_dir()
     self.retrieve_crash_reports()
     self.uninstall_apps()
 

ios/build/bots/scripts/xcode_log_parser.py

@@ -12,7 +12,9 @@ import re
 import shutil
 import subprocess
 
+import file_util
 import test_runner
+import xcode_util
 
 
 # Some system errors are reported as failed tests in Xcode test result log in
@@ -22,6 +24,15 @@ import test_runner
 SYSTEM_ERROR_TEST_NAME_SUFFIXES = ['encountered an error']
 LOGGER = logging.getLogger(__name__)
 
+_XCRESULT_SUFFIX = '.xcresult'
+
+
+def get_parser():
+  """Returns correct parser from version of Xcode installed."""
+  if xcode_util.using_xcode_11_or_higher():
+    return Xcode11LogParser()
+  return XcodeLogParser()
+
 
 def parse_passed_tests_for_interrupted_run(output):
   """Parses xcode runner output to get passed tests only.
@@ -207,7 +218,7 @@ class Xcode11LogParser(object):
 
   @staticmethod
   def collect_test_results(output_path, output):
-    """Gets test result, diagnostic data & artifacts from xcresult.
+    """Gets XCTest results, diagnostic data & artifacts from xcresult.
 
     Args:
       output_path: (str) An output path passed in --resultBundlePath when
@@ -245,7 +256,7 @@ class Xcode11LogParser(object):
     # is symlink to the `output_path.xcresult` folder.
     # `xcresulttool` with folder/symlink behaves in different way on laptop and
     # on bots. This piece of code uses .xcresult folder.
-    xcresult = output_path + '.xcresult'
+    xcresult = output_path + _XCRESULT_SUFFIX
 
     # |output_path|.xcresult folder is created at the end of tests. If
     # |output_path| folder exists but |output_path|.xcresult folder doesn't
@@ -268,21 +279,23 @@ class Xcode11LogParser(object):
       # For some crashed tests info about error contained only in root node.
       test_results['failed'] = Xcode11LogParser._list_of_failed_tests(root)
       Xcode11LogParser._get_test_statuses(xcresult, test_results)
-    Xcode11LogParser._export_diagnostic_data(xcresult)
-    Xcode11LogParser._copy_artifacts(xcresult)
+    Xcode11LogParser.export_diagnostic_data(xcresult)
+    Xcode11LogParser.copy_artifacts(xcresult)
     # Remove the symbol link file.
     if os.path.islink(output_path):
       os.unlink(output_path)
-    Xcode11LogParser._zip_and_remove_folder(xcresult)
+    file_util.zip_and_remove_folder(xcresult)
     return test_results
 
   @staticmethod
-  def _copy_artifacts(xcresult):
+  def copy_artifacts(output_path):
     """Copy screenshots, crash logs of failed tests to output folder.
 
     Args:
-      xcresult: (str) A path to xcresult directory.
+      output_path: (str) An output path passed in --resultBundlePath when
+          running xcodebuild.
     """
+    xcresult = output_path + _XCRESULT_SUFFIX
     if not os.path.exists(xcresult):
       LOGGER.warn('%s does not exist.' % xcresult)
       return
@@ -384,7 +397,7 @@ class Xcode11LogParser(object):
           attachment_index=index)
 
   @staticmethod
-  def _export_diagnostic_data(xcresult):
+  def export_diagnostic_data(output_path):
     """Exports diagnostic data from xcresult to xcresult_diagnostic.zip.
 
     Since Xcode 11 format of result bundles changed, to get diagnostic data
@@ -393,8 +406,10 @@ class Xcode11LogParser(object):
     ./export_folder --path ./RB.xcresult
 
     Args:
-      xcresult: (str) A path to xcresult directory.
+      output_path: (str) An output path passed in --resultBundlePath when
+          running xcodebuild.
     """
+    xcresult = output_path + _XCRESULT_SUFFIX
     if not os.path.exists(xcresult):
       LOGGER.warn('%s does not exist.' % xcresult)
       return
@@ -405,7 +420,7 @@ class Xcode11LogParser(object):
       diagnostic_folder = '%s_diagnostic' % xcresult
       Xcode11LogParser._export_data(xcresult, diagnostics_ref, 'directory',
                                     diagnostic_folder)
-      Xcode11LogParser._zip_and_remove_folder(diagnostic_folder)
+      file_util.zip_and_remove_folder(diagnostic_folder)
     except KeyError:
       LOGGER.warn('Did not parse diagnosticsRef from %s!' % xcresult)
 
@@ -430,18 +445,6 @@ class Xcode11LogParser(object):
     ]
     subprocess.check_output(export_command).strip()
 
-  @staticmethod
-  def _zip_and_remove_folder(dir_path):
-    """Zips folder to the parent folder and then removes original folder.
-
-    Args:
-      dir_path: (str) A path to directory.
-    """
-    shutil.make_archive(
-        os.path.join(os.path.dirname(dir_path), os.path.basename(dir_path)),
-        'zip', dir_path)
-    shutil.rmtree(dir_path)
-
 
 class XcodeLogParser(object):
   """Xcode log parser. Parses logs for Xcode until version 11."""
@@ -492,7 +495,7 @@ class XcodeLogParser(object):
 
   @staticmethod
   def collect_test_results(output_folder, output):
-    """Gets test result data from Info.plist and copies artifacts.
+    """Gets XCtest result data from Info.plist and copies artifacts.
 
     Args:
       output_folder: (str) A path to output folder.
@@ -560,3 +563,15 @@ class XcodeLogParser(object):
       test_case_folder = os.path.join(output_folder, 'failures', test_case_id)
       copy_screenshots_for_failed_test(failure_summary['Message'],
                                        test_case_folder)
+
+  @staticmethod
+  def copy_artifacts(output_path):
+    """Invokes _copy_screenshots(). To make public methods consistent."""
+    LOGGER.info('Invoking _copy_screenshots call for copy_artifacts in'
+                'XcodeLogParser')
+    XcodeLogParser._copy_screenshots(output_path)
+
+  @staticmethod
+  def export_diagnostic_data(output_path):
+    """No-op. To make parser public methods consistent."""
+    LOGGER.warn('Exporting diagnostic data only supported in Xcode 11+')

ios/build/bots/scripts/xcode_log_parser_test.py

@@ -385,6 +385,15 @@ class XCode11LogParserTest(test_runner_test.TestCase):
     super(XCode11LogParserTest, self).setUp()
     self.mock(test_runner, 'get_current_xcode_info', lambda: XCODE11_DICT)
 
+  @mock.patch('xcode_util.version', autospec=True)
+  def testGetParser(self, mock_xcode_version):
+    mock_xcode_version.return_value = ('12.0', '12A7209')
+    self.assertEqual(xcode_log_parser.get_parser().__class__.__name__, 'Xcode11LogParser')
+    mock_xcode_version.return_value = ('11.4', '11E146')
+    self.assertEqual(xcode_log_parser.get_parser().__class__.__name__, 'Xcode11LogParser')
+    mock_xcode_version.return_value = ('10.3', '10G8')
+    self.assertEqual(xcode_log_parser.get_parser().__class__.__name__, 'XcodeLogParser')
+
   @mock.patch('subprocess.check_output', autospec=True)
   def testXcresulttoolGetRoot(self, mock_process):
     mock_process.return_value = '%JSON%'
@@ -430,9 +439,9 @@ class XCode11LogParserTest(test_runner_test.TestCase):
     xcode_log_parser.Xcode11LogParser()._get_test_statuses(OUTPUT_PATH, results)
     self.assertEqual(expected, results['passed'])
 
-  @mock.patch('xcode_log_parser.Xcode11LogParser._zip_and_remove_folder')
-  @mock.patch('xcode_log_parser.Xcode11LogParser._copy_artifacts')
-  @mock.patch('xcode_log_parser.Xcode11LogParser._export_diagnostic_data')
+  @mock.patch('file_util.zip_and_remove_folder')
+  @mock.patch('xcode_log_parser.Xcode11LogParser.copy_artifacts')
+  @mock.patch('xcode_log_parser.Xcode11LogParser.export_diagnostic_data')
   @mock.patch('os.path.exists', autospec=True)
   @mock.patch('xcode_log_parser.Xcode11LogParser._xcresulttool_get')
   @mock.patch('xcode_log_parser.Xcode11LogParser._list_of_failed_tests')
@@ -468,9 +477,9 @@ class XCode11LogParserTest(test_runner_test.TestCase):
         xcode_log_parser.Xcode11LogParser().collect_test_results(
             OUTPUT_PATH, []))
 
-  @mock.patch('xcode_log_parser.Xcode11LogParser._zip_and_remove_folder')
-  @mock.patch('xcode_log_parser.Xcode11LogParser._copy_artifacts')
-  @mock.patch('xcode_log_parser.Xcode11LogParser._export_diagnostic_data')
+  @mock.patch('file_util.zip_and_remove_folder')
+  @mock.patch('xcode_log_parser.Xcode11LogParser.copy_artifacts')
+  @mock.patch('xcode_log_parser.Xcode11LogParser.export_diagnostic_data')
   @mock.patch('os.path.exists', autospec=True)
   @mock.patch('xcode_log_parser.Xcode11LogParser._xcresulttool_get')
   def testCollectTestsRanZeroTests(self, mock_root, mock_exist_file, *args):
@@ -524,7 +533,7 @@ class XCode11LogParserTest(test_runner_test.TestCase):
                           mock_process):
     mock_path_exists.return_value = True
     mock_xcresulttool_get.side_effect = _xcresulttool_get_side_effect
-    xcode_log_parser.Xcode11LogParser()._copy_artifacts(XCRESULT_PATH)
+    xcode_log_parser.Xcode11LogParser().copy_artifacts(OUTPUT_PATH)
     mock_process.assert_any_call([
         'xcresulttool', 'export', '--type', 'file', '--id',
         'SCREENSHOT_REF_ID_IN_FAILURE_SUMMARIES', '--path', XCRESULT_PATH,
@@ -541,7 +550,7 @@ class XCode11LogParserTest(test_runner_test.TestCase):
     # Ensures screenshots in activitySummaries are not copied.
     self.assertEqual(2, mock_process.call_count)
 
-  @mock.patch('xcode_log_parser.Xcode11LogParser._zip_and_remove_folder')
+  @mock.patch('file_util.zip_and_remove_folder')
   @mock.patch('subprocess.check_output', autospec=True)
   @mock.patch('os.path.exists', autospec=True)
   @mock.patch('xcode_log_parser.Xcode11LogParser._xcresulttool_get')
@@ -549,7 +558,7 @@ class XCode11LogParserTest(test_runner_test.TestCase):
                                mock_process, _):
     mock_path_exists.return_value = True
     mock_xcresulttool_get.side_effect = _xcresulttool_get_side_effect
-    xcode_log_parser.Xcode11LogParser._export_diagnostic_data(XCRESULT_PATH)
+    xcode_log_parser.Xcode11LogParser.export_diagnostic_data(OUTPUT_PATH)
     mock_process.assert_called_with([
         'xcresulttool', 'export', '--type', 'directory', '--id',
         'DIAGNOSTICS_REF_ID', '--path', XCRESULT_PATH, '--output-path',

ios/build/bots/scripts/xcode_util.py

@@ -2,6 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import distutils.version
 import logging
 import subprocess
 
@@ -84,3 +85,9 @@ def version():
   build_version = output[1].decode('UTF-8').split(' ')[2].lower()
 
   return version, build_version
+
+def using_xcode_11_or_higher():
+  """Returns true if using Xcode version 11 or higher."""
+  LOGGER.debug("Checking if Xcode version is 11 or higher")
+  return distutils.version.LooseVersion(
+      '11.0') <= distutils.version.LooseVersion(version()[0])

ios/build/bots/scripts/xcodebuild_runner.py

@@ -12,7 +12,7 @@ import os
 import subprocess
 import time
 
-import coverage_util
+import file_util
 import iossim_util
 import standard_json_util as sju
 import test_apps
@@ -135,11 +135,7 @@ class LaunchCommand(object):
     self.test_results = collections.OrderedDict()
     self.use_clang_coverage = use_clang_coverage
     self.env = env
-    if distutils.version.LooseVersion('11.0') <= distutils.version.LooseVersion(
-        test_runner.get_current_xcode_info()['version']):
-      self._log_parser = xcode_log_parser.Xcode11LogParser()
-    else:
-      self._log_parser = xcode_log_parser.XcodeLogParser()
+    self._log_parser = xcode_log_parser.get_parser()
 
   def summary_log(self):
     """Calculates test summary - how many passed, failed and error tests.
@@ -207,8 +203,8 @@ class LaunchCommand(object):
       if hasattr(self, 'use_clang_coverage') and self.use_clang_coverage:
         # out_dir of LaunchCommand object is the TestRunner out_dir joined with
         # UDID. Use os.path.dirname to retrieve the TestRunner out_dir.
-        coverage_util.move_raw_coverage_data(self.udid,
-                                             os.path.dirname(self.out_dir))
+        file_util.move_raw_coverage_data(self.udid,
+                                         os.path.dirname(self.out_dir))
       self.test_results['attempts'].append(
           self._log_parser.collect_test_results(outdir_attempt, output))