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
Rewrite some amp-bind integration tests #19895
Rewrite some amp-bind integration tests #19895
Conversation
945a2f4
to
459e28f
Compare
47ca4ce
to
52d3ebf
Compare
/to @alabiaga |
examples/bind/list.amp.html
Outdated
@@ -10,7 +10,7 @@ | |||
<script async src="https://cdn.ampproject.org/v0.js"></script> | |||
<script async custom-element="amp-bind" src="https://cdn.ampproject.org/v0/amp-bind-0.1.js"></script> | |||
<script async custom-element="amp-list" src="https://cdn.ampproject.org/v0/amp-list-0.1.js"></script> | |||
<script async custom-template="amp-mustache" src="https://cdn.ampproject.org/v0/amp-mustache-latest.js"></script> | |||
<script async custom-template="amp-mustache" src="https://cdn.ampproject.org/v0/amp-mustache-0.1.js"></script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this change? Should we use v0.2 as it is the version that will replace v0.1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted.
} | ||
AMP.registerTemplate('amp-mustache', AmpMustache); | ||
AMP.extension(TAG, '0.2', function(AMP) { | ||
// First, unregister template with to avoid "Duplicate template type" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove "with" in comment.
// First, unregister template with to avoid "Duplicate template type.
Also there is a test failure now that seems to indicate that moving this logic is causing this issue to happen again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove describe.only in test-amp-selector.js.
* Rewrite a few bind integration tests. * Rewrite carousel tests too. * Rewrite list and selector tests. * Skip Edge due to 'Permission denied' errors. * Support extension versions in describes.integration. * Replace weirdly written test-selector-list.js with a unit test. * Aaron's comments. * Remove .only.
Fixes #19789.
describes.integration
instead of iframe fixtures.describes.integration
.