Commit 0c2093e5 authored by zhaoyangli's avatar zhaoyangli Committed by Commit Bot

[code coverage] Merge script changes for separated test types.

- When merging profraw to profdata at shard_merge: Accept the test
target name arg from input and name the profdata file as
{target_name}.profdata.
- When merging profdata from different target to one profdata, add
support to filter the input with a regex pattern. This will be used to
merge only profdata of needed test type.

Bug: 1064785
Change-Id: I4c8ff800c7b40adef2cfe8f905204d6d3ec87ae8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2121087
Commit-Queue: Zhaoyang Li <zhaoyangli@chromium.org>
Reviewed-by: default avatarSajjad Mirza <sajjadm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754564}
parent b7d2775d
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
import logging import logging
import multiprocessing import multiprocessing
import os import os
import re
import shutil import shutil
import subprocess import subprocess
...@@ -87,14 +88,16 @@ def _call_profdata_tool(profile_input_file_paths, ...@@ -87,14 +88,16 @@ def _call_profdata_tool(profile_input_file_paths,
return [] return []
def _get_profile_paths(input_dir, input_extension): def _get_profile_paths(input_dir,
input_extension,
input_filename_pattern='.*'):
"""Finds all the profiles in the given directory (recursively).""" """Finds all the profiles in the given directory (recursively)."""
paths = [] paths = []
for dir_path, _sub_dirs, file_names in os.walk(input_dir): for dir_path, _sub_dirs, file_names in os.walk(input_dir):
paths.extend([ paths.extend([
os.path.join(dir_path, fn) os.path.join(dir_path, fn)
for fn in file_names for fn in file_names
if fn.endswith(input_extension) if fn.endswith(input_extension) and re.search(input_filename_pattern,fn)
]) ])
return paths return paths
...@@ -224,7 +227,11 @@ def merge_java_exec_files(input_dir, output_path, jacococli_path): ...@@ -224,7 +227,11 @@ def merge_java_exec_files(input_dir, output_path, jacococli_path):
logging.info('Merge succeeded with output: %r', output) logging.info('Merge succeeded with output: %r', output)
def merge_profiles(input_dir, output_file, input_extension, profdata_tool_path): def merge_profiles(input_dir,
output_file,
input_extension,
profdata_tool_path,
input_filename_pattern='.*'):
"""Merges the profiles produced by the shards using llvm-profdata. """Merges the profiles produced by the shards using llvm-profdata.
Args: Args:
...@@ -233,11 +240,15 @@ def merge_profiles(input_dir, output_file, input_extension, profdata_tool_path): ...@@ -233,11 +240,15 @@ def merge_profiles(input_dir, output_file, input_extension, profdata_tool_path):
input_extension (str): File extension to look for in the input_dir. input_extension (str): File extension to look for in the input_dir.
e.g. '.profdata' or '.profraw' e.g. '.profdata' or '.profraw'
profdata_tool_path: The path to the llvm-profdata executable. profdata_tool_path: The path to the llvm-profdata executable.
input_filename_pattern (str): The regex pattern of input filename. Should be
a valid regex pattern if present.
Returns: Returns:
The list of profiles that had to be excluded to get the merge to The list of profiles that had to be excluded to get the merge to
succeed and a list of profiles that had a counter overflow. succeed and a list of profiles that had a counter overflow.
""" """
profile_input_file_paths = _get_profile_paths(input_dir, input_extension) profile_input_file_paths = _get_profile_paths(input_dir,
input_extension,
input_filename_pattern)
invalid_profraw_files = [] invalid_profraw_files = []
counter_overflows = [] counter_overflows = []
if input_extension == '.profraw': if input_extension == '.profraw':
......
...@@ -43,6 +43,7 @@ def _MergeAPIArgumentParser(*args, **kwargs): ...@@ -43,6 +43,7 @@ def _MergeAPIArgumentParser(*args, **kwargs):
'--profdata-dir', required=True, help='where to store the merged data') '--profdata-dir', required=True, help='where to store the merged data')
parser.add_argument( parser.add_argument(
'--llvm-profdata', required=True, help='path to llvm-profdata executable') '--llvm-profdata', required=True, help='path to llvm-profdata executable')
parser.add_argument('--test-target-name', help='test target name')
parser.add_argument( parser.add_argument(
'--java-coverage-dir', help='directory for Java coverage data') '--java-coverage-dir', help='directory for Java coverage data')
parser.add_argument( parser.add_argument(
...@@ -75,6 +76,10 @@ def main(): ...@@ -75,6 +76,10 @@ def main():
coverage_merger.merge_java_exec_files( coverage_merger.merge_java_exec_files(
params.task_output_dir, output_path, params.jacococli_path) params.task_output_dir, output_path, params.jacococli_path)
# Name the output profdata file name as {test_target}.profdata or
# default.profdata.
output_prodata_filename = (params.test_target_name or 'default') + '.profdata'
# NOTE: The coverage data merge script must make sure that the profraw files # NOTE: The coverage data merge script must make sure that the profraw files
# are deleted from the task output directory after merging, otherwise, other # are deleted from the task output directory after merging, otherwise, other
# test results merge script such as layout tests will treat them as json test # test results merge script such as layout tests will treat them as json test
...@@ -82,7 +87,7 @@ def main(): ...@@ -82,7 +87,7 @@ def main():
logging.info('Merging code coverage profraw data') logging.info('Merging code coverage profraw data')
invalid_profiles, counter_overflows = coverage_merger.merge_profiles( invalid_profiles, counter_overflows = coverage_merger.merge_profiles(
params.task_output_dir, params.task_output_dir,
os.path.join(params.profdata_dir, 'default.profdata'), '.profraw', os.path.join(params.profdata_dir, output_prodata_filename), '.profraw',
params.llvm_profdata) params.llvm_profdata)
# At the moment counter overflows overlap with invalid profiles, but this is # At the moment counter overflows overlap with invalid profiles, but this is
......
...@@ -39,12 +39,13 @@ class MergeProfilesTest(unittest.TestCase): ...@@ -39,12 +39,13 @@ class MergeProfilesTest(unittest.TestCase):
}) })
task_output_dir = 'some/task/output/dir' task_output_dir = 'some/task/output/dir'
profdata_dir = '/some/different/path/to/profdata/default.profdata' profdata_dir = '/some/different/path/to/profdata/default.profdata'
profdata_file = os.path.join(profdata_dir, 'default.profdata') profdata_file = os.path.join(profdata_dir, 'base_unittests.profdata')
args = [ args = [
'script_name', '--output-json', 'output.json', '--build-properties', 'script_name', '--output-json', 'output.json', '--build-properties',
build_properties, '--summary-json', 'summary.json', '--task-output-dir', build_properties, '--summary-json', 'summary.json', '--task-output-dir',
task_output_dir, '--profdata-dir', profdata_dir, '--llvm-profdata', task_output_dir, '--profdata-dir', profdata_dir, '--llvm-profdata',
'llvm-profdata', 'a.json', 'b.json', 'c.json' 'llvm-profdata', 'a.json', 'b.json', 'c.json', '--test-target-name',
'base_unittests'
] ]
with mock.patch.object(merger, 'merge_profiles') as mock_merge: with mock.patch.object(merger, 'merge_profiles') as mock_merge:
mock_merge.return_value = None, None mock_merge.return_value = None, None
...@@ -53,7 +54,7 @@ class MergeProfilesTest(unittest.TestCase): ...@@ -53,7 +54,7 @@ class MergeProfilesTest(unittest.TestCase):
self.assertEqual( self.assertEqual(
mock_merge.call_args, mock_merge.call_args,
mock.call(task_output_dir, profdata_file, '.profraw', mock.call(task_output_dir, profdata_file, '.profraw',
'llvm-profdata')) 'llvm-profdata'), None)
def test_merge_steps_parameters(self): def test_merge_steps_parameters(self):
"""Test the build-level merge front-end.""" """Test the build-level merge front-end."""
...@@ -67,6 +68,8 @@ class MergeProfilesTest(unittest.TestCase): ...@@ -67,6 +68,8 @@ class MergeProfilesTest(unittest.TestCase):
output_file, output_file,
'--llvm-profdata', '--llvm-profdata',
'llvm-profdata', 'llvm-profdata',
'--profdata-filename-pattern',
'.*'
] ]
with mock.patch.object(merger, 'merge_profiles') as mock_merge: with mock.patch.object(merger, 'merge_profiles') as mock_merge:
mock_merge.return_value = None mock_merge.return_value = None
...@@ -74,7 +77,8 @@ class MergeProfilesTest(unittest.TestCase): ...@@ -74,7 +77,8 @@ class MergeProfilesTest(unittest.TestCase):
merge_steps.main() merge_steps.main()
self.assertEqual( self.assertEqual(
mock_merge.call_args, mock_merge.call_args,
mock.call(input_dir, output_file, '.profdata', 'llvm-profdata')) mock.call(input_dir, output_file, '.profdata', 'llvm-profdata',
'.*'))
@mock.patch.object(merger, '_validate_and_convert_profraws') @mock.patch.object(merger, '_validate_and_convert_profraws')
def test_merge_profraw(self, mock_validate_and_convert_profraws): def test_merge_profraw(self, mock_validate_and_convert_profraws):
...@@ -163,6 +167,44 @@ class MergeProfilesTest(unittest.TestCase): ...@@ -163,6 +167,44 @@ class MergeProfilesTest(unittest.TestCase):
# The mock method should only apply when merging .profraw files. # The mock method should only apply when merging .profraw files.
self.assertFalse(mock_validate_and_convert_profraws.called) self.assertFalse(mock_validate_and_convert_profraws.called)
@mock.patch.object(merger, '_validate_and_convert_profraws')
def test_merge_profdata_pattern(self, mock_validate_and_convert_profraws):
mock_input_dir_walk = [
('/b/some/path', ['base_unittests', 'url_unittests'], ['summary.json']),
('/b/some/path/base_unittests', [], ['output.json',
'base_unittests.profdata']),
('/b/some/path/url_unittests', [], ['output.json',
'url_unittests.profdata'],),
('/b/some/path/ios_chrome_smoke_eg2tests',
[], ['output.json','ios_chrome_smoke_eg2tests.profdata'],),
]
with mock.patch.object(os, 'walk') as mock_walk:
with mock.patch.object(os, 'remove'):
mock_walk.return_value = mock_input_dir_walk
with mock.patch.object(subprocess, 'check_output') as mock_exec_cmd:
input_profdata_filename_pattern = '.+_unittests\.profdata'
merger.merge_profiles('/b/some/path',
'output/dir/default.profdata',
'.profdata',
'llvm-profdata',
input_profdata_filename_pattern)
self.assertEqual(
mock.call(
[
'llvm-profdata',
'merge',
'-o',
'output/dir/default.profdata',
'-sparse=true',
'/b/some/path/base_unittests/base_unittests.profdata',
'/b/some/path/url_unittests/url_unittests.profdata',
],
stderr=-2,
), mock_exec_cmd.call_args)
# The mock method should only apply when merging .profraw files.
self.assertFalse(mock_validate_and_convert_profraws.called)
def test_retry_profdata_merge_failures(self): def test_retry_profdata_merge_failures(self):
mock_input_dir_walk = [ mock_input_dir_walk = [
('/b/some/path', ['0', '1'], ['summary.json']), ('/b/some/path', ['0', '1'], ['summary.json']),
...@@ -228,6 +270,7 @@ class MergeProfilesTest(unittest.TestCase): ...@@ -228,6 +270,7 @@ class MergeProfilesTest(unittest.TestCase):
self.assertEqual(set(['44b643576cf39f10', '44b1234567890123']), self.assertEqual(set(['44b643576cf39f10', '44b1234567890123']),
merger.get_shards_to_retry(bad_profiles)) merger.get_shards_to_retry(bad_profiles))
@mock.patch('merge_lib._JAVA_PATH', 'java')
def test_merge_java_exec_files(self): def test_merge_java_exec_files(self):
mock_input_dir_walk = [ mock_input_dir_walk = [
('/b/some/path', ['0', '1', '2', '3'], ['summary.json']), ('/b/some/path', ['0', '1', '2', '3'], ['summary.json']),
......
...@@ -18,6 +18,11 @@ def _merge_steps_argument_parser(*args, **kwargs): ...@@ -18,6 +18,11 @@ def _merge_steps_argument_parser(*args, **kwargs):
'--output-file', required=True, help='where to store the merged data') '--output-file', required=True, help='where to store the merged data')
parser.add_argument( parser.add_argument(
'--llvm-profdata', required=True, help='path to llvm-profdata executable') '--llvm-profdata', required=True, help='path to llvm-profdata executable')
parser.add_argument(
'--profdata-filename-pattern',
default='.*',
help='regex pattern of profdata filename to merge for current test type. '
'If not present, all profdata files will be merged.')
return parser return parser
...@@ -26,7 +31,7 @@ def main(): ...@@ -26,7 +31,7 @@ def main():
parser = _merge_steps_argument_parser(description=desc) parser = _merge_steps_argument_parser(description=desc)
params = parser.parse_args() params = parser.parse_args()
merger.merge_profiles(params.input_dir, params.output_file, '.profdata', merger.merge_profiles(params.input_dir, params.output_file, '.profdata',
params.llvm_profdata) params.llvm_profdata, params.profdata_filename_pattern)
if __name__ == '__main__': if __name__ == '__main__':
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment