Closed Bug 1236949 Opened 4 years ago Closed 4 years ago
_ext _tabs _move _window .js | This test exceeded the timeout threshold . It should be rewritten or split up .
Any documentation or best practices I can read on how to deal with this?
Heh, documentation. As I understand it, second and third hand, "exceeded the timeout threshold" means that you were still doing fine, making progress, not hanging, just taking longer than we think one test should. If the test is really three separate independent tests in one file, which it looks to my uneducated eye like it might well be, then the proper fix is "split up" - just copy-paste those into browser_ext_tabs_move_window.js and browser_ext_tabs_move_pin.js and browser_ext_tabs_move_multiple.js. And hope it isn't just one of the three which takes all the time. Or, if it's a huge surprise that the three parts are sometimes taking 45965ms, you might want to debug it - the failures all being debug builds, there's rather a lot of stuff being logged already, some of which might actually be related to you rather than just being the normal spew of opening a window, and some of it might say why it sometimes takes 46 seconds to open a few windows and move them around, or you might have to add some logging and push it to try, or worst case if you can't manage to hit a slow instance on try even with lots of retriggers, just land the extra logging in the tree and wait for starring to tell you where an instance with your logging happened. Or, if the three parts really do depend on each other, and taking 45 seconds on Linux debug is reasonable, then there's the third option (which I never copy-paste in the bug summaries, despite its presence in the failure message), "If that's not possible, use requestLongerTimeout(N), but only as a last resort." That actually is, barely, documented, https://developer.mozilla.org/en-US/docs/Browser_chrome_tests#Test_functions
I think you're just running into bad luck here. You're hitting a GC in the middle of your test that winds up reaping about a hundred docshells that were created in previous tests. I think this is probably related to the same problem as bug 1215775. If we were explicitly destroying our hidden windows, most of them would (ideally) be reaped in or near the test that created them, rather than throwing long GC pauses into the middle of random tests.
Depends on: 1215775
I did some testing, and unfortunately none of the obvious changes led to more windows being reaped before we hit a full, non-incremental GC (which tends to fall in the middle of your test). We might be able to make things a bit easier for the cycle collector, but I'm not sure how much time that would take. The easiest thing would probably be to cut the cross-compartment proxies, like we do for sandboxes and some chrome windows. I'm not entirely sure how well it would pay off, though. The best solution for now might be to force cycle collection after some strategic tests, just to make sure it doesn't all hit at once, at unpredictable times.
No longer depends on: 1215775
(In reply to Phil Ringnalda (:philor) from comment #3) Thanks so much for that info.
Andy, do you mind if I take this bug? I want to see if I can do something about our GC problems, since they affect other tests too.
Go for it, thanks mate.
Assignee: amckay → kmaglione+bmo
No failures in the last week. I think this is fixed.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/integration/fx-team/rev/6f37ab432cf08ea6d729161d205d686ffacbff0b Bug 1236949: Split up browser_ext_tab_move_window.js. r=me
You need to log in before you can comment on or make changes to this bug.