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
Fix unit tests for amp-viewer-integration #32937
Conversation
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.
Thank you!
const event = { | ||
source: window, | ||
source: env.win, |
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.
I love this change. It prevents the pollution of the global window
object.
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.
👍🏽
const ampDocSrc = '/test/fixtures/served/ampdoc-with-messaging.html'; | ||
// TODO(aghassemi): Investigate failure in beforeEach. #10974. |
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.
Was there a failure in the beforeEach
?
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.
Not really, not sure why this was skipped (issue #10974 doesn't offer much context).
However, while looking into this, I discovered that 'should handle unload correctly'
test was incorrect and got a chance to fix it & move it to the correct location since it is testing AmpViewerIntegration
. So thanks for this comment!
Fixes #32938