From c31fc2a941d417286e1a56d6f18d5051c99d06d9 Mon Sep 17 00:00:00 2001 From: Jeremy Leconte Date: Tue, 7 Dec 2021 09:54:34 +0100 Subject: [PATCH] Reland "Use gtest_parallel with 1 worker for webrtc_perf_tests." This is a reland of 258ed1a38ad9d4f0da798c40b6976eff2dce864f Original change's description: > Use gtest_parallel with 1 worker for webrtc_perf_tests. > > This will enable test results to be uploaded to ResultDB. > > Bug: b/197492097 > Change-Id: Iec28520c4cd8f35fcff2cbd105a4b851ef41b9fc > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/239641 > Reviewed-by: Mirko Bonadei > Reviewed-by: Christoffer Jansson > Commit-Queue: Jeremy Leconte > Cr-Commit-Position: refs/heads/main@{#35458} Bug: b/197492097 No-Presubmit: True Change-Id: Iea90f5698c83791d39c0f6da666c1d1eb274edd3 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/239645 Commit-Queue: Jeremy Leconte Reviewed-by: Andrey Logvin Reviewed-by: Christoffer Jansson Reviewed-by: Jakob Ivarsson Reviewed-by: Gustaf Ullberg Reviewed-by: Mirko Bonadei Cr-Commit-Position: refs/heads/main@{#35483} --- audio/test/OWNERS | 3 ++ audio/test/low_bandwidth_audio_test.py | 24 ++++++----- test/testsupport/perf_test.cc | 2 +- tools_webrtc/mb/mb.py | 55 ++++++++++++-------------- tools_webrtc/mb/mb_unittest.py | 33 ++++++++++++---- 5 files changed, 70 insertions(+), 47 deletions(-) create mode 100644 audio/test/OWNERS diff --git a/audio/test/OWNERS b/audio/test/OWNERS new file mode 100644 index 0000000000..3754d4823a --- /dev/null +++ b/audio/test/OWNERS @@ -0,0 +1,3 @@ +# Script to launch low_bandwidth_audio_test. +per-file low_bandwidth_audio_test.py=mbonadei@webrtc.org +per-file low_bandwidth_audio_test.py=jleconte@webrtc.org diff --git a/audio/test/low_bandwidth_audio_test.py b/audio/test/low_bandwidth_audio_test.py index 9aaf30f364..d581b06977 100755 --- a/audio/test/low_bandwidth_audio_test.py +++ b/audio/test/low_bandwidth_audio_test.py @@ -31,9 +31,9 @@ NO_TOOLS_ERROR_MESSAGE = ( 'To fix this run:\n' ' python %s %s\n' '\n' - 'Note that these tools are Google-internal due to licensing, so in order to ' - 'use them you will have to get your own license and manually put them in the ' - 'right location.\n' + 'Note that these tools are Google-internal due to licensing, so in order ' + 'to use them you will have to get your own license and manually put them ' + 'in the right location.\n' 'See https://cs.chromium.org/chromium/src/third_party/webrtc/tools_webrtc/' 'download_tools.py?rcl=bbceb76f540159e2dba0701ac03c514f01624130&l=13') @@ -65,6 +65,9 @@ def _ParseArgs(): '--isolated-script-test-perf-output', default=None, help='Path to store perf results in histogram proto format.') + parser.add_argument('--dump_json_test_results', + default=None, + help='Path to store json test results.') parser.add_argument('--extra-test-args', default=[], action='append', @@ -242,14 +245,14 @@ def _ConfigurePythonPath(args): checkout_root = os.path.abspath( os.path.join(script_dir, os.pardir, os.pardir)) - # TODO(https://crbug.com/1029452): Use a copy rule and add these from the out - # dir like for the third_party/protobuf code. + # TODO(https://crbug.com/1029452): Use a copy rule and add these from the + # out dir like for the third_party/protobuf code. sys.path.insert( 0, os.path.join(checkout_root, 'third_party', 'catapult', 'tracing')) - # The low_bandwidth_audio_perf_test gn rule will build the protobuf stub for - # python, so put it in the path for this script before we attempt to import - # it. + # The low_bandwidth_audio_perf_test gn rule will build the protobuf stub + # for python, so put it in the path for this script before we attempt to + # import it. histogram_proto_path = os.path.join(os.path.abspath(args.build_dir), 'pyproto', 'tracing', 'tracing', 'proto') @@ -296,7 +299,10 @@ def main(): ] else: test_command = [ - os.path.join(args.build_dir, 'low_bandwidth_audio_test') + os.path.join('..', '..', 'tools_webrtc', + 'gtest-parallel-wrapper.py'), + os.path.join(args.build_dir, 'low_bandwidth_audio_test'), + '--dump_json_test_results=%s' % args.dump_json_test_results, ] analyzers = [Analyzer('pesq', _RunPesq, pesq_path, 16000)] diff --git a/test/testsupport/perf_test.cc b/test/testsupport/perf_test.cc index d282bf23a1..28d8e7730a 100644 --- a/test/testsupport/perf_test.cc +++ b/test/testsupport/perf_test.cc @@ -242,7 +242,7 @@ void PrintPlottableResults(const std::vector& desired_graphs) { bool WritePerfResults(const std::string& output_path) { std::string results = GetPerfResults(); CreateDir(DirName(output_path)); - FILE* output = fopen(output_path.c_str(), "wb"); + FILE* output = fopen(output_path.c_str(), "ab"); if (output == NULL) { printf("Failed to write to %s.\n", output_path.c_str()); return false; diff --git a/tools_webrtc/mb/mb.py b/tools_webrtc/mb/mb.py index 09550fabcb..ec62996961 100755 --- a/tools_webrtc/mb/mb.py +++ b/tools_webrtc/mb/mb.py @@ -906,14 +906,22 @@ class MetaBuildWrapper(object): extra_files = [ '../../.vpython', '../../testing/test_env.py', + '../../third_party/gtest-parallel/gtest-parallel', + '../../third_party/gtest-parallel/gtest_parallel.py', + '../../tools_webrtc/gtest-parallel-wrapper.py', ] vpython_exe = 'vpython' + #TODO(crbug.com/webrtc/13475) : use os.path module instead of 'sep'. + sep = '\\' if self.platform == 'win32' else '/' + output_dir = '${ISOLATED_OUTDIR}' + sep + 'test_logs' + test_results = '${ISOLATED_OUTDIR}' + sep + 'gtest_output.json' must_retry = False if test_type == 'script': cmdline += [vpython_exe, '../../' + - self.ToSrcRelPath(isolate_map[target]['script'])] + self.ToSrcRelPath(isolate_map[target]['script']), + '--dump_json_test_results=%s' % test_results] elif is_android: cmdline += [vpython_exe, '../../build/android/test_wrapper/logdog_wrapper.py', @@ -922,11 +930,6 @@ class MetaBuildWrapper(object): '--logcat-output-file', '${ISOLATED_OUTDIR}/logcats', '--store-tombstones'] else: - if test_type == 'raw': - cmdline += [vpython_exe, - '../../tools_webrtc/flags_compatibility.py'] - extra_files.append('../../tools_webrtc/flags_compatibility.py') - if isolate_map[target].get('use_webcam', False): cmdline += [vpython_exe, '../../tools_webrtc/ensure_webcam_is_running.py'] @@ -943,33 +946,25 @@ class MetaBuildWrapper(object): else: cmdline += [vpython_exe, '../../testing/test_env.py'] + cmdline += [ + '../../tools_webrtc/gtest-parallel-wrapper.py', + '--output_dir=%s' % output_dir, + '--dump_json_test_results=%s' % test_results, + '--gtest_color=no', + ] if test_type != 'raw': - extra_files += [ - '../../third_party/gtest-parallel/gtest-parallel', - '../../third_party/gtest-parallel/gtest_parallel.py', - '../../tools_webrtc/gtest-parallel-wrapper.py', - ] - sep = '\\' if self.platform == 'win32' else '/' - output_dir = '${ISOLATED_OUTDIR}' + sep + 'test_logs' - test_results = '${ISOLATED_OUTDIR}' + sep + 'gtest_output.json' + # We tell gtest-parallel to interrupt the test after 900 + # seconds, so it can exit cleanly and report results, + # instead of being interrupted by swarming and not + # reporting anything. timeout = isolate_map[target].get('timeout', 900) - cmdline += [ - '../../tools_webrtc/gtest-parallel-wrapper.py', - '--output_dir=%s' % output_dir, - '--dump_json_test_results=%s' % test_results, - '--gtest_color=no', - # We tell gtest-parallel to interrupt the test after 900 - # seconds, so it can exit cleanly and report results, - # instead of being interrupted by swarming and not - # reporting anything. - '--timeout=%s' % timeout, - ] - if test_type == 'non_parallel_console_test_launcher': - # Still use the gtest-parallel-wrapper.py script since we - # need it to run tests on swarming, but don't execute tests - # in parallel. - cmdline.append('--workers=1') + cmdline.append('--timeout=%s' % timeout) must_retry = True + if test_type in ('raw', 'non_parallel_console_test_launcher'): + # Still use the gtest-parallel-wrapper.py script since we + # need it to run tests on swarming, but don't execute tests + # in parallel. + cmdline.append('--workers=1') asan = 'is_asan=true' in vals['gn_args'] lsan = 'is_lsan=true' in vals['gn_args'] diff --git a/tools_webrtc/mb/mb_unittest.py b/tools_webrtc/mb/mb_unittest.py index a65a55b37b..61786ea7dc 100755 --- a/tools_webrtc/mb/mb_unittest.py +++ b/tools_webrtc/mb/mb_unittest.py @@ -332,8 +332,13 @@ class UnitTest(unittest.TestCase): files = isolate_file_contents['variables']['files'] command = isolate_file_contents['variables']['command'] - self.assertEqual(files, ['../../.vpython', '../../testing/test_env.py', - 'base_unittests']) + self.assertEqual(files, [ + '../../.vpython', '../../testing/test_env.py', + '../../third_party/gtest-parallel/gtest-parallel', + '../../third_party/gtest-parallel/gtest_parallel.py', + '../../tools_webrtc/gtest-parallel-wrapper.py', + 'base_unittests', + ]) self.assertEqual(command, [ 'vpython', '../../build/android/test_wrapper/logdog_wrapper.py', @@ -367,8 +372,13 @@ class UnitTest(unittest.TestCase): files = isolate_file_contents['variables']['files'] command = isolate_file_contents['variables']['command'] - self.assertEqual(files, ['../../.vpython', '../../testing/test_env.py', - 'base_unittests']) + self.assertEqual(files, [ + '../../.vpython', '../../testing/test_env.py', + '../../third_party/gtest-parallel/gtest-parallel', + '../../third_party/gtest-parallel/gtest_parallel.py', + '../../tools_webrtc/gtest-parallel-wrapper.py', + 'base_unittests', + ]) self.assertEqual(command, [ 'vpython', '../../build/android/test_wrapper/logdog_wrapper.py', @@ -457,11 +467,15 @@ class UnitTest(unittest.TestCase): self.assertEqual(files, [ '../../.vpython', '../../testing/test_env.py', + '../../third_party/gtest-parallel/gtest-parallel', + '../../third_party/gtest-parallel/gtest_parallel.py', + '../../tools_webrtc/gtest-parallel-wrapper.py', 'base_unittests', 'base_unittests_script.py', ]) self.assertEqual(command, [ 'vpython', '../../base/base_unittests_script.py', + '--dump_json_test_results=${ISOLATED_OUTDIR}/gtest_output.json', ]) def test_gen_raw(self): @@ -491,14 +505,19 @@ class UnitTest(unittest.TestCase): self.assertEqual(files, [ '../../.vpython', '../../testing/test_env.py', - '../../tools_webrtc/flags_compatibility.py', + '../../third_party/gtest-parallel/gtest-parallel', + '../../third_party/gtest-parallel/gtest_parallel.py', + '../../tools_webrtc/gtest-parallel-wrapper.py', 'base_unittests', ]) self.assertEqual(command, [ - 'vpython', - '../../tools_webrtc/flags_compatibility.py', 'vpython', '../../testing/test_env.py', + '../../tools_webrtc/gtest-parallel-wrapper.py', + '--output_dir=${ISOLATED_OUTDIR}/test_logs', + '--dump_json_test_results=${ISOLATED_OUTDIR}/gtest_output.json', + '--gtest_color=no', + '--workers=1', './base_unittests', '--asan=0', '--lsan=0',