Fix amp-pixel integration test on Saucelabs. #9130
Conversation
|
|
||
| return withdrawRequest(win, 'no-referrer').then(request => { | ||
| describes.integration('amp-pixel integration test', { | ||
| body: `<amp-pixel src="${depositRequestUrl('no-referrer')}" |
| @@ -333,6 +338,42 @@ class SandboxFixture { | |||
| } | |||
| } | |||
|
|
|||
| /** @implements {Fixture} */ | |||
| class IntegrationFixture { | |||
| /** @param {!{body: string}} spec */ | |||
There was a problem hiding this comment.
As we discussed, can we add a TODO here to support extension and version later
There was a problem hiding this comment.
i don't think TODO is needed, especially we recently introduced a new "TODO rule" that we need to create a github issue for each TODO.
this works as is right now. if anyone want to add tests using extension, they will add them wisely.
There was a problem hiding this comment.
we have such a rule now? OK, I agree they will find it.
There was a problem hiding this comment.
@dvoytenko will tell you about the new rule. :-)
| referrerpolicy="no-referrer">`, | ||
| }, env => { | ||
| it('should remove referrer if referrerpolicy=no-referrer', () => { | ||
| return withdrawRequest(env.win, 'no-referrer').then(request => { |
There was a problem hiding this comment.
Though the new integration test looks good. I really can't tell why it will fix test failure on old Chrome? Can you explain it a bit
There was a problem hiding this comment.
The previous failure was due to the fixture iframe that we created for each test does not have an origin (we use srcdoc). In such case, some browsers will take the top window origin as referrer, but some will use null .
There was a problem hiding this comment.
Make sense. Let's merge this and see : )
The previous failure was due to the fixture iframe that we created for each test does not have an origin (we use
srcdoc). In such case, some browsers will take the top window origin asreferrer, but some will usenull.The fix is to fully mimic the real production world. Use an iframe to load a valid AMP page from a server.
Closes #8853