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

Amp-Video Bind Integration Test #7727

Merged
merged 99 commits into from
Mar 9, 2017

Conversation

kmh287
Copy link
Contributor

@kmh287 kmh287 commented Feb 22, 2017

Added an integration test for amp-bind with amp-video. I've tested the src, alt, and controls attributes, and for src, tested allowed protocols and blocked URLs..

@kmh287
Copy link
Contributor Author

kmh287 commented Mar 8, 2017

@choumx PTAL

.equal('https://www.google.com/unbound.webm');
button.click();
return waitForBindApplication().then(() => {
// Only HTTPS is allowed

Choose a reason for hiding this comment

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

+2 spaces

Copy link
Contributor Author

@kmh287 kmh287 Mar 9, 2017

Choose a reason for hiding this comment

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

Shouldn't blocks only be indented by two?

EDIT: Oh, you mean the .equal part. Done.

Choose a reason for hiding this comment

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

No I meant this comment. Overflow should be +4 spaces so that change should be reverted.

Nitty nit nits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Overflow is at +4. Will fix the comment on this PR

@kmh287 kmh287 merged commit d65970f into ampproject:master Mar 9, 2017
kmh287 added a commit to kmh287/amphtml that referenced this pull request Mar 13, 2017
mrjoro pushed a commit to mrjoro/amphtml that referenced this pull request Apr 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants