Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🏗Allow gulp tasks to programmatically build the runtime #28326

Merged
merged 1 commit into from May 12, 2020
Merged

🏗Allow gulp tasks to programmatically build the runtime #28326

merged 1 commit into from May 12, 2020

Conversation

rsimha
Copy link
Contributor

@rsimha rsimha commented May 12, 2020

Background:

Testing tasks like e2e, integration, and visual-diff require fully built AMP binaries. On Travis, we like to run them on minified code, while for local development, we'd like the option of using unminified or minified code.

PR highlights:

  • Adds doBuild() / doDist() to build.js / dist.js to allow for in-process compilation
  • Refactors compilation toolchain with an options param through which args can be plumbed
  • Restricts argv.fortesting usage to top-level entry points (other places use options)
  • Standardizes buildRuntime() and reuses it in e2e, integration, and visual-diff
  • Standardizes --compiled flag for e2e, integration, and visual-diff
  • Updates documentation in TESTING.md

On Travis, this PR is a no-op, and tests will continue to run against minified code. For local testing, the default behavior (without --compiled) is to run on unminified code, which can be considerably faster.

Fixes #28312
Follow up to #28305

@rsimha rsimha self-assigned this May 12, 2020
@rsimha rsimha changed the title 🏗Allow gulp tasks to programmatically build the runtime in-process 🏗Allow gulp tasks to programmatically build the runtime May 12, 2020
@rsimha rsimha requested a review from samouri May 12, 2020 15:57
@rsimha rsimha marked this pull request as ready for review May 12, 2020 15:57
@amp-owners-bot
Copy link

amp-owners-bot bot commented May 12, 2020

Hey @erwinmombay! These files were changed:

build-system/compile/single-pass.js

Hey @estherkim! These files were changed:

build-system/tasks/e2e/index.js

@rsimha rsimha requested a review from rcebulko May 12, 2020 20:36
Copy link
Member

@samouri samouri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, that was quick!

build-system/pr-check/visual-diff-tests.js Show resolved Hide resolved
build-system/tasks/dist.js Show resolved Hide resolved
@rsimha
Copy link
Contributor Author

rsimha commented May 12, 2020

All Travis tests are green.

Tested the following 12 permutations locally, looks good to merge 🤞

gulp {e2e|integration|visual-diff} [--nobuild] [--compiled]

@rsimha rsimha merged commit aca8b11 into ampproject:master May 12, 2020
@rsimha rsimha deleted the 2020-05-11-ExtraArgs branch May 12, 2020 22:19
Jiaming-X pushed a commit to Jiaming-X/amphtml that referenced this pull request May 13, 2020
build-system/tasks/build.js Show resolved Hide resolved
build-system/tasks/css.js Show resolved Hide resolved
build-system/tasks/dist.js Show resolved Hide resolved
build-system/tasks/firebase.js Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fixit: make gulp e2e directly call dist() and build()
5 participants