-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Closed as not planned
Closed as not planned
Copy link
Labels
P2Important issues not at the top of the work listImportant issues not at the top of the work listc: proposalA detailed proposal for a change to FlutterA detailed proposal for a change to Flutterc: tech-debtTechnical debt, code quality, testing, etc.Technical debt, code quality, testing, etc.e: engine-toolEngine-specific tooling (i.e. `tools/engine_tool`).Engine-specific tooling (i.e. `tools/engine_tool`).engineflutter/engine related. See also e: labels.flutter/engine related. See also e: labels.team-engineOwned by Engine teamOwned by Engine teamtriaged-engineTriaged by Engine teamTriaged by Engine team
Description
Background
Today we "encode" how to build and test the engine in at least three different places:
ci/builders/*.json
Encodes:
- What platform is needed to run a "build"
- What
gnflags to use (i.e.--runtime-mode, and so on) for a build - What target(s) to build (i.e. delegates to
ninja) for a build - What target(s) to run as tests (i.e. a non-zero exit code is a failure) for a build
For example:
Note that many, but not all of the "tests": [ ... ] invocations are to testing/run_tests.py.
testing/run_tests.py
Encodes:
- A number of directories or utilities that are used multiple times (i.e.
FONTS_DIR, orBUILDROOT_DIR) - Environment variables that are conditionally added or modified
- Minor file operations (moving, copying, sym-linking, deleting) to prepare a test (or cleanup after)
- A combination of both C++ and non-C++ targets to execute tests with
For example:
# Not a 1:1 reproduction.
# Example of top-level variables.
BUILDROOT_DIR = os.path.abspath(os.path.join(os.path.realpath(__file__), '..', '..', '..'))
OUT_DIR = os.path.join(BUILDROOT_DIR, 'out')
GOLDEN_DIR = os.path.join(BUILDROOT_DIR, 'flutter', 'testing', 'resources')
FONTS_DIR = os.path.join(BUILDROOT_DIR, 'flutter', 'third_party', 'txt', 'third_party', 'fonts')
# Example of platform checks.
def is_asan(build_dir):
with open(os.path.join(build_dir, 'args.gn')) as args:
if 'is_asan = true' in args.read():
return True
return False
# Example of conditional environment variables.
def metal_validation_env():
extra_env = {
'MTL_SHADER_VALIDATION': '1', # Enables all shader validation tests.
'MTL_SHADER_VALIDATION_GLOBAL_MEMORY': '1', # Validates accesses to device and constant memory.
'MTL_SHADER_VALIDATION_THREADGROUP_MEMORY': '1', # Validates accesses to threadgroup memory.
'MTL_SHADER_VALIDATION_TEXTURE_USAGE': '1', # Validates that texture references are not nil.
}
if is_aarm64():
extra_env.update({
'METAL_DEBUG_ERROR_MODE': '0', # Enables metal validation.
'METAL_DEVICE_WRAPPER_TYPE': '1', # Enables metal validation.
})
return extra_env
# Example of declaring paths to run tests on (there are other specific executable cases).
def build_dart_host_test_list(build_dir):
dart_host_tests = [
(os.path.join('flutter', 'testing', 'litetest'), []),
(os.path.join('flutter', 'testing', 'skia_gold_client'), []),
(os.path.join('flutter', 'testing', 'scenario_app'), []),
]
# Based on the flags, runs variants of tests.
def main():
if 'dart-host' in types:
dart_filter = args.dart_host_filter.split(',') if args.dart_host_filter else None
dart_host_packages = build_dart_host_test_list(build_dir)
tasks = []
for dart_host_package, extra_opts in dart_host_packages:
if dart_filter is None or dart_host_package in dart_filter:
tasks += list(
gather_dart_package_tests(
build_dir,
os.path.join(BUILDROOT_DIR, dart_host_package),
extra_opts,
)
)
success = success and run_engine_tasks_in_parallel(tasks).ci.yaml
Encodes:
- How to run pre/post-submit checks
- Which builder (i.e.
platform_debug) to build and run tests for - Optional other arguments around timeouts, conditional runs, and conditional branches
For example:
# Not a 1:1 reproduction.
targets:
- name: Linux linux_android_emulator_tests
enabled_branches:
- main
recipe: engine_v2/engine_v2
properties:
config_name: linux_android_emulator
dependencies: >-
[
{"dependency": "goldctl", "version": "git_revision:720a542f6fe4f92922c3b8f0fdcc4d2ac6bb83cd"}
]
timeout: 90
runIf:
- .ci.yaml
- ci/builders/linux_android_emulator.json
- DEPS
- lib/ui/**
- shell/platform/android/**
- testing/scenario_app/**
- testing/skia_gold_client/**Proposal
There can be more than one proposal.
On end-state I think we all agree that:
- This (^) is not sustainable, we already run the risk of under-testing or over-testing;
- We need to stop leaking implementation details about how builds and tests work across the entire repository;
- We can "know", for a given platform, all possible build and test targets;
- Our CI and local development process, as much as possible, uses the same workflows.
As a straw-man, you can imagine writing GN rules for every single build and test target (including non-C++), and just using GN throughout the system (behind the et tool). There might be other approaches worth considering (like our own metadata format), but we should keep the end state in mind.
Goals:
- Avoid divergence between CI and local development, that is, our
ettool should be used in production; - Deliver incremental milestones that add value to the repo (instead of just more code), in other words, make users happy;
- Have a kick-ass engine development workflow.
Metadata
Metadata
Assignees
Labels
P2Important issues not at the top of the work listImportant issues not at the top of the work listc: proposalA detailed proposal for a change to FlutterA detailed proposal for a change to Flutterc: tech-debtTechnical debt, code quality, testing, etc.Technical debt, code quality, testing, etc.e: engine-toolEngine-specific tooling (i.e. `tools/engine_tool`).Engine-specific tooling (i.e. `tools/engine_tool`).engineflutter/engine related. See also e: labels.flutter/engine related. See also e: labels.team-engineOwned by Engine teamOwned by Engine teamtriaged-engineTriaged by Engine teamTriaged by Engine team