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

Better A4A/FIE testing, add FIE services regression test #19943

Merged
merged 14 commits into from Dec 19, 2018

Conversation

dreamofabear
Copy link

@dreamofabear dreamofabear commented Dec 18, 2018

  • Refactor amp4test.js to be configurable. Eg. it can now test amp4email spec, A4A FIE with arbitrary content, etc.
  • Add BrowserController.waitForElementReady() to wait for custom elements to finish initializing (layout) before taking action.
  • Add regression test for FIE services refactor in Refactor EmbeddableService #19771.

You can now pass spec: 'amp'|'amp4ads'|'amp4email' to describes.integration and it will generate a correctly formatted AMP document according to the chosen spec.

Test an A4A/FIE with arbitrary markup

const myAdSrc = addParamsToUrl('/amp4test/compose-doc', {
  body: ...,
  extensions: ...,
  spec: 'amp4ads',
});

describes.integration('My A4A test', {
  body: `
    <amp-ad width=300 height=400
        id="i-amphtml-demo-id" type="fake" 
        src="${myAdSrc}">
    </amp-ad>`,
  }, env => {
  ...
});

Test an AMP4EMAIL doc

describes.integration('My AMP4EMAIL test', {
  body: ...,
  spec: 'amp4email',
  }, env => {
  ...
});

@dreamofabear dreamofabear changed the title End-to-end test for amp-bind in FIE Better A4A/FIE testing, add FIE services regression test Dec 18, 2018
@dreamofabear
Copy link
Author

/to @lannka

Still need to fix a few integration test failures likely due to amp4test.js changes.

@dreamofabear
Copy link
Author

I think the problem right now is that test code doesn't wait for extensions to "be ready". E.g. a "click" event can happen before amp-analytics has a chance to setup the "click" trigger.

Copy link
Contributor

@lannka lannka left a comment

Choose a reason for hiding this comment

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

I think the problem right now is that test code doesn't wait for extensions to "be ready". E.g. a "click" event can happen before amp-analytics has a chance to setup the "click" trigger.

Did your waitForReady function help?

I like your refactoring, good work!

@dreamofabear
Copy link
Author

Yup, that was the idea. Thanks!

@dreamofabear dreamofabear merged commit 334755c into ampproject:master Dec 19, 2018
@dreamofabear dreamofabear deleted the test-fie-services branch December 19, 2018 18:17
noranazmy pushed a commit to noranazmy/amphtml that referenced this pull request Mar 22, 2019
…19943)

* Refactor amp4test to use compose helper.

* Add support for cdn mode, fix bugs.

* Add amp-bind in a4a integration test.

* Uncomment other test.

* Support amp-ad-metdata script.

* Runtime char offsets should include runtime script itself.

* Some comments.

* Fix lint.

* Add waitForElementLayout() and fix broken tests.

* Poll readyState instead and decrease analytics timeout to 5s.

* amp-pixel doesn't layout so wait for build instead.

* Tweak timeout message for waitForElementBuild().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants