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.

Sign up for GitHub

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

Jump to bottom

Resource scans happen in test iframes after they are removed #3709

Closed
cramforce opened this issue Jun 22, 2016 · 3 comments · Fixed by #3904
Closed

Resource scans happen in test iframes after they are removed #3709

cramforce opened this issue Jun 22, 2016 · 3 comments · Fixed by #3904
Assignees
@dvoytenko
Labels
Related to: Flaky Tests
Milestone
Fixit Week February

Comments

@cramforce
Copy link
Member

Because they get timed on the parent window.

They might just need to check whether the respective document still has a defaultView.

CC @muxin
cramforce added a commit to cramforce/amphtml that referenced this issue Jun 22, 2016
@cramforce
Fix local amp-ad tests that fail due to timing.
562c208 
Related to ampproject#3709
@cramforce cramforce mentioned this issue Jun 22, 2016
Merged
cramforce added a commit that referenced this issue Jun 22, 2016
@cramforce
Fix local amp-ad tests that fail due to timing. (#3710)
e5b0f33 
Related to #3709
@ericlindley-g ericlindley-g modified the milestones: Next, Fixit - Date TBD Jun 29, 2016
@ericlindley-g
Copy link
Contributor

@cramforce — slotting this in the Fixit milestone, but please correct/update as needed

@cramforce cramforce mentioned this issue Jul 1, 2016
Closed
@mkhatib
Copy link
Contributor

mkhatib commented Jul 2, 2016

What is the resource scan? Is that the discoverWork_ loop?

@cramforce
Copy link
Member Author

Yeah, here is what happens:

  • Resource manager is per window.
  • but we have a single timer that we use across all windows.
  • that timer in tests belongs to main iframe
  • when test iframe gets removed, timers belonging to parent iframe still fire (because that wasn't removed)

Fixing might be as simple as giving resource manager its own timer. Likely the timer should just be a real per-window service.

CC @dvoytenko

@cramforce cramforce mentioned this issue Jul 2, 2016
Merged
cramforce added a commit to cramforce/amphtml that referenced this issue Jul 6, 2016
@cramforce
Do not execute pass callbacks if their window went away.
47b34a5 
This could happen in tests if multiple windows had different lifetime. Also turns timer into a service but does not yet fix all callsites.

Fixes ampproject#3709 but all the other timer usages could lead to similar issues.
cramforce added a commit that referenced this issue Jul 6, 2016
@cramforce
Do not execute pass callbacks if their window went away. (#3904)
688ee8d 
This could happen in tests if multiple windows had different lifetime. Also turns timer into a service but does not yet fix all callsites.

Fixes #3709 but all the other timer usages could lead to similar issues.
@muxin muxin mentioned this issue Jul 6, 2016
Merged
ariangibson pushed a commit to Mixpo/amphtml that referenced this issue Sep 7, 2016
@cramforce @ariangibson
Do not execute pass callbacks if their window went away. (ampproject#…
1a93503 
…3904)

This could happen in tests if multiple windows had different lifetime. Also turns timer into a service but does not yet fix all callsites.

Fixes ampproject#3709 but all the other timer usages could lead to similar issues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dvoytenko dvoytenko

Labels
Related to: Flaky Tests
Projects
None yet
Milestone
Fixit Week February
Development

Successfully merging a pull request may close this issue.

Do not execute pass callbacks if their window went away. cramforce/amphtml
4 participants
@mkhatib @cramforce @dvoytenko @ericlindley-g