Skip to content

Engine pre‐submits and post‐submits

Matan Lurey edited this page Dec 7, 2023 · 2 revisions

The Flutter engine repo runs both pre-submit (before merging) and post-submit (after merging) suites of tests and checks, defined in .ci.yaml.

Tip

See Cocoon Scheduler for details.

Failure to run appropriate tests for changes can (and do) result in the engine tree turning red, which in turn causes an expensive cascade of developers being blocked, investigative work, reverts, and roll-forwards. Where possible, all attempts should be made to run any/all tests before merging a PR. See nuances (below) for exceptional cases.

Pre-submit

Presubmits run (and are required to be passing) to merge a PR:

Checks

For example, the linux_host_engine target above runs based on the configuration in ci/builders/linux_host_engine.json.

Nuances

Typically, pre-submits always run on every PR, and don't need any special attention (other than keeping them green). There are two exceptions:

  1. Targets that provide a runIf: ... configuration
  2. Changes that impact Clang Tidy

Warning

runIf: ... is a powerful (but dangerous) feature that trades predictability for speed.

runIf will skip certain targets if a particular file (or commonly, sets of files) are not changed in a given PR.

For example, the linux_clang_tidy_presubmit target will not run if only markdown (*.md) files are changed.

Clang Tidy, on the other hand, is only run on files that have changed in a given PR. For example, if you have:

// impeller/a.h
struct A {}

... files that import that header, such as impeller/foo/bar/baz.cc, will not be run in pre-submit. This means that changing (or updating the DEPS of libraries that provide) headers is not a safe change, and will not be detected in pre-submit. As an example, #48705 had to be reverted (despite passing all pre-submit checks), because the Clang Tidy post-submit caught a failure.

See post-submit below for options to run post-submits eagerly (i.e. as a pre-submit).

Post-submit

Some (albeit fewer) targets are configured with the property presubmit: false:

  - name: Mac mac_clang_tidy
    recipe: engine_v2/engine_v2
    presubmit: false

These targets will not show up during a PR, and will not be executed, but can (and do) turn the tree red.

Running post-submits eagerly

We've intentionally chosen to make it easier to land PRs, at the cost of turning the tree red periodically because post-submit checks catch something that the developer did not intend (or even know about). As a code author (or reviewer), you can optionally turn on post-submits to run eagerly (during pre-submit) by adding the label test: all (available only in the flutter/engine repo).

Add the label, and push the PR (or a new commit, the scheduler will not understand the label being added without a commit):

Screenshot 2023-12-07 at 1 55 48 PM

For example, #48158 ran all of the checks, including what is typically post-submits:

Screenshot 2023-12-07 at 1 55 39 PM

Warning

This increases the use of workers/capacity, and should be discouraged to be used on all PRs.

Flutter Wiki

Process

Framework repo

The Flutter CLI Tool

Engine repo

Packages repo

Engineering Productivity

User documentation

Clone this wiki locally