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

Longtask unit test refactor #12437

Merged

Conversation

jonkeller
Copy link
Contributor

Makes the unit test test with a cross-origin iframe, instead of a same-origin one, to be more realistic.

@jonkeller jonkeller self-assigned this Dec 12, 2017
@@ -135,52 +136,6 @@ describes.realWin('amp-analytics.iframe-transport', {amp: true}, env => {
expect(createPerformanceObserverSpy).to.be.called;
});


it('logs poor performance of vendor iframe', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not deleted, just reordered and then changed a quite a bit.
The main change is the value of frameUrl2, and that we're no longer setting srcdoc. That (and some other deleted stuff) was hoop-jumping necessitated by doNotLoadExternalResourcesInTest() in testing/iframe.js, which is no longer needed because of the new allowExternalResources: true on line 155.

@keithwrightbos keithwrightbos merged commit a16ae31 into ampproject:master Dec 13, 2017
gzgogo pushed a commit to gzgogo/amphtml that referenced this pull request Jan 26, 2018
* Longtask unit test now uses cross-origin

* Longtask unit test now uses cross-origin

* Fix mistakes made by gulp lint --fix
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.

None yet

5 participants