Skip to content

Determine the future of build/test configuration files in flutter/engine. #145185

@matanlurey

Description

@matanlurey

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 gn flags 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:

// Not a 1:1 reproduction.
{
  "builds": [
    {
      "drone_dimensions": [ /* what platform is needed, i.e. Linux, etc */ ],
      "gn": [ /* flags */ ],
      "name": "platform_debug",
      "tests": [ /* executables to run */ ],
    },
  ],
}

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, or BUILDROOT_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 et tool 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

No one assigned

    Labels

    P2Important issues not at the top of the work listc: proposalA detailed proposal for a change to Flutterc: tech-debtTechnical debt, code quality, testing, etc.e: engine-toolEngine-specific tooling (i.e. `tools/engine_tool`).engineflutter/engine related. See also e: labels.team-engineOwned by Engine teamtriaged-engineTriaged by Engine team

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions