From 76dce91cf2bfb703db447bd9b49f517d813e66af Mon Sep 17 00:00:00 2001 From: "zijiehe@" <68449066+zijiehe-google-com@users.noreply.github.com> Date: Tue, 6 Feb 2024 09:34:53 -0800 Subject: [PATCH] Revert "Revert "[Fuchsia] Execute most of the testing/fuchsia/test_suites.yaml on debug and release builds"" (flutter/engine#50295) Reverts flutter/engine#50291, https://github.com/flutter/flutter/issues/142811 Following is the original change description. This change implements a BundledTestRunner to run most of the tests in testing/fuchsia/test_suites.yaml as ExecutableTestRunner. - Tests with packages out of out/fuchsia_*_x64/ are ignored for now. - Tests with extra test command line parameters are ignored for now. The BundledTestRunner can share most of the logic in ExecutableTestRunner and avoid reinventing the wheel. This change also fixes the build break of fuchsia_tests in fuchsia_release_x64 which allows tests to run on the build as well. - Tests not built with AOT are filtered out with variant field in test_suites.yaml. Bug: https://github.com/flutter/flutter/issues/140179 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat --- engine/src/flutter/.ci.yaml | 7 +- .../flutter/ci/builders/linux_fuchsia.json | 24 +++- .../fuchsia/dart_runner/embedder/BUILD.gn | 3 +- .../fuchsia/dart_runner/kernel/BUILD.gn | 3 +- .../fuchsia/dart_runner/vmservice/BUILD.gn | 2 +- .../platform/fuchsia/flutter/kernel/BUILD.gn | 3 +- .../src/flutter/testing/fuchsia/run_tests.py | 115 ++++++++++++++---- .../flutter/testing/fuchsia/test_suites.yaml | 4 + 8 files changed, 128 insertions(+), 33 deletions(-) diff --git a/engine/src/flutter/.ci.yaml b/engine/src/flutter/.ci.yaml index 5b25a136b70..fa5bd99a109 100644 --- a/engine/src/flutter/.ci.yaml +++ b/engine/src/flutter/.ci.yaml @@ -195,7 +195,12 @@ targets: - name: Linux linux_fuchsia recipe: engine_v2/engine_v2 - timeout: 60 + # Temporarily increase the timeout of this builder to 75 minutes to avoid + # being penetrated by cold goma / rbclient cache. + # TODO(zijiehe-google-com): Drop the timeout to 60 minutes once the release + # builder can finish within a reasonable time. + # https://github.com/flutter/flutter/issues/142932 + timeout: 75 properties: release_build: "true" config_name: linux_fuchsia diff --git a/engine/src/flutter/ci/builders/linux_fuchsia.json b/engine/src/flutter/ci/builders/linux_fuchsia.json index 011a13618aa..facbae0827b 100644 --- a/engine/src/flutter/ci/builders/linux_fuchsia.json +++ b/engine/src/flutter/ci/builders/linux_fuchsia.json @@ -115,10 +115,12 @@ { "drone_dimensions": [ "device_type=none", + "kvm=1", "os=Linux" ], "gclient_variables": { - "download_android_deps": false + "download_android_deps": false, + "run_fuchsia_emu": true }, "gn": [ "--fuchsia", @@ -131,9 +133,22 @@ "ninja": { "config": "fuchsia_release_x64", "targets": [ - "flutter/shell/platform/fuchsia:fuchsia" + "flutter/shell/platform/fuchsia:fuchsia", + "flutter/shell/platform/fuchsia/dart_runner:dart_runner_tests", + "fuchsia_tests" ] - } + }, + "tests": [ + { + "name": "x64 emulator based release tests", + "language": "python3", + "script": "flutter/tools/fuchsia/with_envs.py", + "parameters": [ + "testing/fuchsia/run_tests.py", + "fuchsia_release_x64" + ] + } + ] }, { "drone_dimensions": [ @@ -158,6 +173,7 @@ "config": "fuchsia_debug_x64", "targets": [ "flutter/shell/platform/fuchsia:fuchsia", + "flutter/shell/platform/fuchsia/dart_runner:dart_runner_tests", "fuchsia_tests" ] }, @@ -176,7 +192,7 @@ ] }, { - "name": "x64 emulator based tests", + "name": "x64 emulator based debug tests", "language": "python3", "script": "flutter/tools/fuchsia/with_envs.py", "parameters": [ diff --git a/engine/src/flutter/shell/platform/fuchsia/dart_runner/embedder/BUILD.gn b/engine/src/flutter/shell/platform/fuchsia/dart_runner/embedder/BUILD.gn index afd12c4b51b..562191cbcf6 100644 --- a/engine/src/flutter/shell/platform/fuchsia/dart_runner/embedder/BUILD.gn +++ b/engine/src/flutter/shell/platform/fuchsia/dart_runner/embedder/BUILD.gn @@ -54,8 +54,7 @@ template("create_aot_snapshot") { # No asserts in debug or release product. # No asserts in release with flutter_profile=true (non-product) # Yes asserts in non-product debug. - if (!invoker.product && - (!(flutter_runtime_mode == "profile") || is_debug)) { + if (!invoker.product && (flutter_runtime_mode == "debug" || is_debug)) { args += [ "--enable_asserts" ] } args += [ rebase_path(shim_kernel) ] diff --git a/engine/src/flutter/shell/platform/fuchsia/dart_runner/kernel/BUILD.gn b/engine/src/flutter/shell/platform/fuchsia/dart_runner/kernel/BUILD.gn index af896f65e28..c9907424047 100644 --- a/engine/src/flutter/shell/platform/fuchsia/dart_runner/kernel/BUILD.gn +++ b/engine/src/flutter/shell/platform/fuchsia/dart_runner/kernel/BUILD.gn @@ -69,8 +69,7 @@ template("create_kernel_core_snapshot") { # No asserts in debug or release product. # No asserts in release with flutter_profile=true (non-product) # Yes asserts in non-product debug. - if (!invoker.product && - (is_debug || !(flutter_runtime_mode == "profile"))) { + if (!invoker.product && (is_debug || flutter_runtime_mode == "debug")) { args += [ "--enable_asserts" ] } args += [ rebase_path(platform_dill) ] diff --git a/engine/src/flutter/shell/platform/fuchsia/dart_runner/vmservice/BUILD.gn b/engine/src/flutter/shell/platform/fuchsia/dart_runner/vmservice/BUILD.gn index 8aa7da550ba..c299edb5f3f 100644 --- a/engine/src/flutter/shell/platform/fuchsia/dart_runner/vmservice/BUILD.gn +++ b/engine/src/flutter/shell/platform/fuchsia/dart_runner/vmservice/BUILD.gn @@ -65,7 +65,7 @@ template("aot_snapshot") { # No asserts in debug or release product. # No asserts in release with flutter_profile=true (non-product) # Yes asserts in non-product debug. - if (!product && (!(flutter_runtime_mode == "profile") || is_debug)) { + if (!product && (flutter_runtime_mode == "debug" || is_debug)) { args += [ "--enable_asserts" ] } diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/kernel/BUILD.gn b/engine/src/flutter/shell/platform/fuchsia/flutter/kernel/BUILD.gn index bc9853d5cd7..dd77441bfd3 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/kernel/BUILD.gn +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/kernel/BUILD.gn @@ -73,8 +73,7 @@ template("core_snapshot") { # No asserts in debug or release product. # No asserts in release with flutter_profile=true (non-product) # Yes asserts in non-product debug. - if (!invoker.product && - (is_debug || !(flutter_runtime_mode == "profile"))) { + if (!invoker.product && (is_debug || flutter_runtime_mode == "debug")) { args += [ "--enable_asserts" ] } args += [ rebase_path(platform_dill) ] diff --git a/engine/src/flutter/testing/fuchsia/run_tests.py b/engine/src/flutter/testing/fuchsia/run_tests.py index 2a301c82829..40c66be2b96 100755 --- a/engine/src/flutter/testing/fuchsia/run_tests.py +++ b/engine/src/flutter/testing/fuchsia/run_tests.py @@ -1,12 +1,28 @@ -#!/usr/bin/env python3 +#!/usr/bin/env vpython3 + +# [VPYTHON:BEGIN] +# python_version: "3.8" +# wheel < +# name: "infra/python/wheels/pyyaml/${platform}_${py_python}_${py_abi}" +# version: "version:5.4.1.chromium.1" +# > +# [VPYTHON:END] + # Copyright (c) 2013, the Flutter project authors. All rights reserved. # Use of this source code is governed by a BSD-style license that can be found # in the LICENSE file. import argparse +import logging import os import sys +from subprocess import CompletedProcess +from typing import List + +# The import is coming from vpython wheel and pylint cannot find it. +import yaml # pylint: disable=import-error + # The imports are coming from fuchsia/test_scripts and pylint cannot find them # without setting a global init-hook which is less favorable. # But this file will be executed as part of the CI, its correctness of importing @@ -25,32 +41,89 @@ from common import DIR_SRC_ROOT from run_executable_test import ExecutableTestRunner from test_runner import TestRunner -# TODO(https://github.com/flutter/flutter/issues/140179): Respect build -# configurations. -OUT_DIR = os.path.join(DIR_SRC_ROOT, 'out/fuchsia_debug_x64') +if len(sys.argv) == 2: + VARIANT = sys.argv[1] + sys.argv.pop() +elif len(sys.argv) == 1: + VARIANT = 'fuchsia_debug_x64' +else: + assert False, 'Expect only one parameter as the compile output directory.' +OUT_DIR = os.path.join(DIR_SRC_ROOT, 'out', VARIANT) -# TODO(https://github.com/flutter/flutter/issues/140179): Execute all the tests -# in -# https://github.com/flutter/engine/blob/main/testing/fuchsia/test_suites.yaml -# and avoid hardcoded paths. -def _get_test_runner(runner_args: argparse.Namespace, *_) -> TestRunner: - return ExecutableTestRunner( - OUT_DIR, [], - 'fuchsia-pkg://fuchsia.com/dart_runner_tests#meta/dart_runner_tests.cm', - runner_args.target_id, None, '/tmp/log', - [os.path.join(OUT_DIR, 'dart_runner_tests.far')], None +class BundledTestRunner(TestRunner): + + # private, use bundled_test_runner_of function instead. + def __init__( + self, target_id: str, package_deps: List[str], tests: List[str], + logs_dir: str + ): + super().__init__(OUT_DIR, [], None, target_id, package_deps) + self.tests = tests + self.logs_dir = logs_dir + + def run_test(self) -> CompletedProcess: + returncode = 0 + for test in self.tests: + # pylint: disable=protected-access + test_runner = ExecutableTestRunner( + OUT_DIR, [], test, self._target_id, None, self.logs_dir, [], None + ) + test_runner._package_deps = self._package_deps + result = test_runner.run_test().returncode + logging.info('Result of test %s is %s', test, result) + if result != 0: + returncode = result + return CompletedProcess(args='', returncode=returncode) + + +def bundled_test_runner_of(target_id: str) -> BundledTestRunner: + log_dir = os.environ.get('FLUTTER_LOGS_DIR', '/tmp/log') + with open(os.path.join(os.path.dirname(__file__), 'test_suites.yaml'), + 'r') as file: + tests = yaml.safe_load(file) + # TODO(zijiehe-google-com): Run tests with multiple packages or with extra + # test arguments, https://github.com/flutter/flutter/issues/140179. + tests = list( + filter( + lambda test: test['test_command'].startswith('test run ') and test[ + 'test_command'].endswith('.cm'), tests + ) ) + tests = list( + filter( + lambda test: 'package' in test and test['package'].endswith('-0.far'), + tests + ) + ) + tests = list( + filter( + lambda test: not 'variant' in test or VARIANT == test['variant'], + tests + ) + ) + for test in tests: + original_package = test['package'] + test['package'] = os.path.join( + OUT_DIR, test['package'].replace('-0.far', '.far') + ) + try: + os.remove(test['package']) + except FileNotFoundError: + pass + os.symlink(original_package, test['package']) + return BundledTestRunner( + target_id, [test['package'] for test in tests], + [test['test_command'][len('test run '):] for test in tests], log_dir + ) + + +def _get_test_runner(runner_args: argparse.Namespace, *_) -> TestRunner: + return bundled_test_runner_of(runner_args.target_id) if __name__ == '__main__': - try: - os.remove(os.path.join(OUT_DIR, 'dart_runner_tests.far')) - except FileNotFoundError: - pass - os.symlink( - 'dart_runner_tests-0.far', os.path.join(OUT_DIR, 'dart_runner_tests.far') - ) + logging.info('Running tests in %s', OUT_DIR) sys.argv.append('--out-dir=' + OUT_DIR) # The 'flutter-test-type' is a place holder and has no specific meaning; the # _get_test_runner is overrided. diff --git a/engine/src/flutter/testing/fuchsia/test_suites.yaml b/engine/src/flutter/testing/fuchsia/test_suites.yaml index c311fe2fc28..876a51ad891 100644 --- a/engine/src/flutter/testing/fuchsia/test_suites.yaml +++ b/engine/src/flutter/testing/fuchsia/test_suites.yaml @@ -19,16 +19,20 @@ package: flow_tests-0.far - test_command: test run fuchsia-pkg://fuchsia.com/runtime_tests#meta/runtime_tests.cm package: runtime_tests-0.far + variant: fuchsia_debug_x64 - test_command: test run fuchsia-pkg://fuchsia.com/shell_tests#meta/shell_tests.cm package: shell_tests-0.far + variant: fuchsia_debug_x64 - test_command: test run fuchsia-pkg://fuchsia.com/testing_tests#meta/testing_tests.cm package: testing_tests-0.far - test_command: test run fuchsia-pkg://fuchsia.com/txt_tests#meta/txt_tests.cm -- --gtest_filter=-ParagraphTest.* package: txt_tests-0.far - test_command: test run fuchsia-pkg://fuchsia.com/ui_tests#meta/ui_tests.cm package: ui_tests-0.far + variant: fuchsia_debug_x64 - test_command: test run fuchsia-pkg://fuchsia.com/embedder_tests#meta/embedder_tests.cm package: embedder_tests-0.far + variant: fuchsia_debug_x64 - test_command: test run fuchsia-pkg://fuchsia.com/dart_utils_tests#meta/dart_utils_tests.cm package: dart_utils_tests-0.far - test_command: test run fuchsia-pkg://fuchsia.com/dart-jit-runner-integration-test#meta/dart-jit-runner-integration-test.cm