I2I: AMP Build Artifacts Storage

[danielrozenberg]

Objective

Use the GitHub API and two new GitHub repositories to maintain and deliver artifacts of past AMP builds on Travis, to enable setting and using dynamic values in the build process (e.g., quickly mark certain tests as flaky to unblock contributors.)

Background

Currently contributors to the AMPHTML repository are often blocked on Travis builds that fail for reasons that are not necessarily relevant to their PR. For example:

• gulp bundle-size compares the compiled bundle size to a hard-coded value in the repository, which might be ok for the PR, but will cause the master branch to go above the allowed bundle size.
• gulp test … executes unit/integration tests, which occasionally start failing for reasons unrelated to the PR (aka flaky tests,) which in turn block PRs until another person fixes or disables those tests, merges the fix into master, and then all contributors have to rebase their PRs on master.

Ideally these checks should become settings that can be quickly toggled by the build cop, to unblock contributors (especially new contributors, who are not likely to be aware of our complicated PR presubmit checks and are less willing to work around unrelated failures.)

Overview

This design doc proposes using two separate new GitHub repositories to maintain build artifacts. Each of these repositories will maintain various values used during Travis build/presubmit checks. The current proposal suggests three gulp <action> checks that can benefit from consulting these repositories, but we can expand that in the future. The gulp actions are: bundle-size, test, visual-diff. The GitHub repositories will be named ampproject/amphtml-bundle-size, ampproject/amphtml-flaky-tests.

Pushing to these repositories should be gated to push builds coming from Travis and to core contributors. Gating on Travis will reduce the risk of abuse, although this risk is expected to be low as these changes will only affect PRs which are inspected by human reviewers.

Changes to AMP's Continuous Integration

gulp bundle-size

When this action is executed on a PR (on Travis or on a local machine)

• Calculate the bundle size [BS[PR]]
• Find the PR's branching point from master and retrieve its bundle size from a file in the amphtml-bundle-size repository [BS[base]]
• If BS[PR] ≤ BS[base], bless this bundle size (exit point)
• If BS[PR] > BS[base], calculate the delta [BS[Δ]] and retrieve the current master's bundle size from the amphtml-bundle-size repository [BR[master]] and the maximum allowed bundle size [BR[max]] which can be configured and updated by AMP maintainers
• If BR[master] + BR[Δ] + hard-coded-buffer ≤ BR[max], bless this bundle size (exit point)
• If BR[master] + BR[Δ] + hard-coded-buffer > BR[max], reject this bundle size (for potentially going above max bundle size)
  • This gives the maintainers an opportunity to increase the max bundle size before merging this PR. If the maintainers increase the bundle size they should re-run the Travis checks on this PR.

(Why a hard coded buffer? Because bundle size is not guaranteed to be accurate. A tiny buffer (e.g., 100 bytes) can allow us to block on the risk, and consider whether to increase the bundle size before it reaches master)

When this action is executed on a push build (on Travis only)

• Calculate the bundle size [BS[master]] and update the amphtml-bundle-size repository with this value paired with the current commit hash
• Retrieve the maximum allowed bundle size [BR[max]]
• If BS[master] ≤ BR[max], bless this bundle size (exit point)
• If BS[master] > BR[max], reject this bundle size
  • This gives the maintainers an opportunity to increase the max bundle size, or to revert the offending PR

If the server is unreachable, ignore this check.

gulp test and gulp visual-diff (on Travis or on a local machine)

• Retrieve a list of unit/integration/visual-diff test names that have been marked as flaky from the amphtml-flaky-tests repository
• Skip those tests

Differences in failure recovery between Travis and workstations

If GitHub is unreachable from Travis, fail the gulp task. Travis starts builds by cloning the PR itself from GitHub, so this is an unrealistic fatal error. If GitHub is unreachable from a developer's local machine who is running the affected gulp task via a gulp pr-check task, warn about the failure but continue to run the other tasks.

Repository Contents

ampproject/amphtml-bundle-size

One of two options:

1. A single JSON file containing a single dictionary, with commit hashes as keys and bundle sizes in bytes as values
2. Multiple flat files, with commit hashes as file names and the bundle size in bytes as values

ampproject/amphtml-flaky-tests

One JSON file for each relevant gulp task, e.g., flaky-unit-tests.json, flaky-integration-tests.json, flaky-visual-diff-tests.json.

For unit/integration tests, the contents would be a list of objects, each object has the keys file and test_name, the pair of which describe which test names in which files are flaky.

For visual diff tests, the contents would be a list of strings, each string is the name of a flaky visual diff test.

Security Considerations

A malicious or faulty PR that has been merged can abuse the restricted Travis access to update the bundle size for commit hashes that are outside of the PR's scope. This is a minor security issue, since a malicious PR can cause much more harm for the project. By preventing the GitHub token from allowing to force-push to the repositories we can maintain the history of the build artifacts and avoid losing data.

Privacy Considerations

No PII is involved.

[danielrozenberg]

Resolved