Closed Bug 913983 Opened 10 years ago Closed 10 years ago

Intermittent TEST-UNEXPECTED-FAIL | devtools/shared/test/browser_telemetry_buttonsandsidebar.js | This test exceeded the timeout threshold. It should be rewritten or split up.

Categories

(DevTools :: General, defect)

Product:
DevTools
Component:
General
Platform:
All
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox25 fixed, firefox26 fixed, firefox27 fixed, firefox-esr24 wontfix)

Status:
RESOLVED FIXED
Milestone:
Firefox 27
Tracking Flags:
Tracking Status
firefox25 --- fixed
firefox26 --- fixed
firefox27 --- fixed
firefox-esr24 --- wontfix

People

(Reporter: KWierso, Assigned: miker)

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 1 obsolete file)

Patch with added comments
28.44 KB, patch
miker
: review+
Details | Diff | Splinter Review 
I suspect that this is the result of a previous error:
23:34:30     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_637020.js | Console message: [JavaScript Warning: "ReferenceError: reference to undefined property getComputedStyle(...).counterReset" {file: "chrome://browser/content/browser.js" line: 12256}]
23:34:30     INFO -  [Parent 1252] WARNING: NS_ENSURE_TRUE(mTextInputHandler) failed: file ../../../widget/cocoa/nsChildView.mm, line 5011
23:34:30     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_637020.js | the delayed startup has finished
23:34:30     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_637020.js | first window has two tabs
23:34:30     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_637020.js | second window has two tabs
23:34:31     INFO -  System JS : ERROR resource:///modules/sessionstore/SessionStore.jsm:4024
23:34:31     INFO -                       TypeError: this._windows[aWindow.__SSi] is undefined
(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #24)
> I suspect that this is the result of a previous error:
> 23:34:31     INFO -                       TypeError:
> this._windows[aWindow.__SSi] is undefined

Those are likely just parser false-positives (see bug 910614 and dependants).

As test, author can you think of any reason why this has started failing so frequently now?
Flags: needinfo?(mratcliffe)
(In reply to Ed Morley [:edmorley UTC+1] from comment #76)
> (In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #24)
> > I suspect that this is the result of a previous error:
> > 23:34:31     INFO -                       TypeError:
> > this._windows[aWindow.__SSi] is undefined
> 
> Those are likely just parser false-positives (see bug 910614 and dependants).
> 
> As test, author can you think of any reason why this has started failing so
> frequently now?

Am am looking into it.
Flags: needinfo?(mratcliffe)
Full log (debug build):
https://tbpl.mozilla.org/php/getParsedLog.php?id=28278780&tree=Fx-Team&full=1

It seems like there is an unexplained 10 second pause:
03:28:21     INFO -  --DOMWINDOW == 460 (0x16c80b7f8) [serial = 12869] [outer = 0x0] [url = http://example.com/browser/browser/devtools/shared/test/browser_layoutHelpers.html]
03:28:31     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/devtools/shared/test/browser_telemetry_buttonsandsidebar.js | Console message: [JavaScript Warning: "ReferenceError: reference to undefined property display.blinker" {file: "chrome://browser/content/devtools/codemirror/codemirror.js" line: 872}]

We are starting and stopping a bunch of devtools here and using promises as each initializes to run tests. A ten second pause is bad.

This pause is probably down to GC so I will split the test.
Assignee: nobody → mratcliffe
Status: NEW → ASSIGNED
Thank you for looking at this :-)
I have split browser_telemetry_buttonsandsidebar.js into 5:
 - browser_telemetry_sidebar.js
 - browser_telemetry_button_responsive.js
 - browser_telemetry_button_scratchpad.js
 - browser_telemetry_button_tilt.js
 - browser_telemetry_button_paintflashing.js

I also discovered that in browser/devtools/shared/test/head.js we were trying to use console without first declaring it so I fixed that at the same time (I use console.error in the tests).

Try:
https://tbpl.mozilla.org/?tree=Try&rev=343f37e79d37
Attachment #809823 - Flags: review?(jwalker)
The try for this patch is actually green ... the oranges are all about either codemirror or the autocomplete popup.
Comment on attachment 809823 [details] [diff] [review]
buttonsandsidebar-test-timeout-913983.patch

Review of attachment 809823 [details] [diff] [review]:
-----------------------------------------------------------------

One minor thing which is to add a comment in a few places. I think that's safe enough to do without re-try and will only take a few seconds editing.
Sorry for the delay.

::: browser/devtools/shared/test/browser_telemetry_sidebar.js
@@ +40,5 @@
> +
> +    // Concatenate the array with itself so that we can open each tool twice.
> +    sidebarTools.push.apply(sidebarTools, sidebarTools);
> +
> +    setTimeout(function selectSidebarTab() {

Could you add a pointer comment to the justification at the top of the file? e.g:
// See TOOL_DELAY for why we need setTimeout here

Same in other places
Attachment #809823 - Flags: review?(jwalker) → review+
Addressed reviewers comments
Attachment #809823 - Attachment is obsolete: true
Attachment #811205 - Flags: review+
https://hg.mozilla.org/integration/fx-team/rev/7acf8f426b9e
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/7acf8f426b9e
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 27
https://hg.mozilla.org/releases/mozilla-aurora/rev/061938a3b990

I'll also push it to beta as a ride-along the next time I'm pushing other stuff there.
status-firefox26: affected → fixed
status-firefox-esr24: affected → wontfix
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.