First Timers Only

We know figuring out the process for contributing to an open source project can be intimidating, so we created this issue as a way for you to learn the ropes. (If you feel comfortable contributing to open source projects, please leave this issue for someone else.)

What you will need to know

AMP's test suite includes both unit tests and integration tests. Integration tests are particularly valuable because they allow us to test end-to-end functionality on compiled, production code.

Background

Our integration tests work by loading a local AMP HTML file (a "fixture") into an iframe and verifying behaviors on the fixture. Each test function (a Mocha it(...) function) in an integration test file loads the fixture before execution and unloads it afterwards.

Motivation

amp-bind currently has an integration test suite in test-bind-integration.js that mostly uses a single fixture in bind-integrations.html.

The current implementation has a major drawback: even though each test only depends on a small portion of the fixture, the entire fixture is always loaded. Further, the code being tested runs a moderately expensive scan on the entire fixture every time it's loaded, when only a small subset of the fixture is important to any one test.

Fixing this issue will speed up execution of our integration tests, which improves the productivity of all developers on AMP!

This is also a good way to get familiar with one of our most exciting new extensions, amp-bind.

Step by step

• Claim this issue by adding a comment below. Please only claim this bug if you plan on starting work in the next day or so.
• If you aren't too familiar with Git/GitHub, see the Getting Started End-to-End Guide for an intro to Git & GitHub, and how to get a copy of the code. You can also refer to the Quick Start Guide for the necessary setup steps with less explanation than the End-to-End guide.
• Follow the instructions for building AMP.
• Create a Git branch for making your changes.
• Combine test functions in test-bind-integration.js such that there's a single test function for each AMP component that amp-bind is interacting with. E.g. a single it() for amp-img instead of five currently.
• Split up the bind-integrations.html test fixture into multiple files in test/fixtures/. Each fixture should only contain the markup for a single AMP extension other than amp-bind. Integration with text tags has already been split into its own fixture, which can be found in bind-text-integration.html
• Update test-bind-integration.js such that each describe block loads the new, smaller fixture that corresponds to the AMP extension it's testing. E.g. the single describe() for amp-carousel should load the new fixture in test/fixtures/bind-carousel-integration.html.
• Commit your changes frequently.
• Push your changes to GitHub.
• Create a Pull Request. Mention Fixes #8197 in the description and add @choumx and @kmh287 as reviewers.
• Respond to your reviewer's comments (if any).

Once approved, your changes will be merged. ⚡⚡⚡Congrats on making your first contribution to the AMP Project!⚡⚡⚡ You'll be able to see it live across the web soon!

Thanks, and we hope to see more contributions from you soon.

Questions?

If you have questions ask in this issue or on your Pull Request (mentioning @choumx and @kmh287) or see the How to get help section of the Getting Started guide.