1011652 - mochitest-devtools fails when shared and tilt folders are run together

Status: RESOLVED FIXED

(DevTools :: Framework, defect)

Description

[Brian Grinstead [:bgrins]]

Still trying to narrow down the cause, but I get a failure:

 9:16.85 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/tilt/test/browser_tilt_02_notifications-seq.js | uncaught exception - TypeError: this.telemetryInfo is undefined at chrome://mochitests/content/browser/browser/devtools/shared/test/browser_telemetry_button_paintflashing.js:21

This is blocking itchpad from landing, as the new folder changes the chunking to run these tests together on windows and osx: https://tbpl.mozilla.org/?tree=Try&rev=c96e4a3fa349.

Comment 1

[Brian Grinstead [:bgrins]]

Attachment: less-tests.patch

quick patch to speed up the failure by removing tests in between shared/ and tilt/

Comment 2

[Brian Grinstead [:bgrins]]

Narrowed this down the smallest range of tests that still causes the failure:

--start-at=browser/devtools/shared/test/browser_telemetry_toolboxtabs_webconsole.js
--end-at=browser/devtools/tilt/test/browser_tilt_02_notifications-seq.js

Comment 3

[Brian Grinstead [:bgrins]]

What's weird is that the tilt test fails, but the error comes from shared/test/browser_telemetry_toolboxtabs_webconsole:

 0:20.31 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/tilt/test/browser_tilt_02_notifications-seq.js | uncaught exception - TypeError: this.telemetryInfo is undefined at chrome://mochitests/content/browser/browser/devtools/shared/test/browser_telemetry_toolboxtabs_webconsole.js:21

Comment 4

[Brian Grinstead [:bgrins]]

Attachment: telemetry-mochitest.patch

Push with current chunking and this patch applied: https://tbpl.mozilla.org/?tree=Try&rev=103697536bfc
Push with stubbed out itchpad folder (causes new chunking) and this patch applied: https://tbpl.mozilla.org/?tree=Try&rev=197096e304e3

Comment 5

[Brian Grinstead [:bgrins]]

And finally, a push that includes the full itchpad patch: https://tbpl.mozilla.org/?tree=Try&rev=080a7ff5aecb

Comment 6

[Brian Grinstead [:bgrins]]

Comment on attachment 8424099 [details] [diff] [review]
telemetry-mochitest.patch

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

Can you take a look at this?  For some reason, trying to access this.telemetryInfo causes errors, but only when run in the same suite as Tilt.  Catching the case where this.telemetryInfo is nonexistent and just returning fixes the error and doesn't cause any failures in the tests themselves.

Comment 7

[Brian Grinstead [:bgrins]]

My second push from Comment 4 has an error, best to see these two:

This patch only on the current m-c: https://tbpl.mozilla.org/?tree=Try&rev=103697536bfc
A push with this patch plus the new itchpad folder also: https://tbpl.mozilla.org/?tree=Try&rev=080a7ff5aecb

Comment 8

[Brian Grinstead [:bgrins]]

The only relation this bug has to itchpad is that landing a new folder starting with 'i' bumps the shared and tilt tests together (presumably) on windows and osx.  If the new folder was called 'zpad' then this woudl not be a problem.

Comment 9

[Brian Grinstead [:bgrins]]

This patch fixes the failures, and doesn't cause any tests to be skipped.  It is a workaround for a bug with the test suite that I'm not understanding, but Bug 992911 will likely fix the problem for good.

Comment 10

[Brian Grinstead [:bgrins]]

OK, so I spent some more time looking at this just to make sure I understood what was going on.  Since the test harness doesn't have access to the instances of the Telemetry objects, it modifies the prototype of the object to change the log function: http://dxr.mozilla.org/mozilla-central/source/browser/devtools/shared/test/browser_telemetry_sidebar.js#17.

The problem is that tilt builds a new Telemetry instance on toolbox open (see [call stack]).  This builds a new object with the modified prototype, so that when tries to log telemetry during the tilt test, we see the error.  I can't think of an easy way around this since the tests doesn't which telemetry instances are logging - during the test you *do* want everything that got logged even if it didn't come from an instance you expect.

I suspect this issue will be permanently resolved by Bug 992911, so I think this patch is an OK workaround in the interim.  I'll update the patch with a comment indicating that it can be removed once 992911 lands.

[call stack]:
_vtools/tilt/tilt.js 98 Tilt
_vtools/tilt/tilt.js 63 TiltManager.getTiltForBrowser
_lt/tilt-commands.js 50 exports.items<.state.onChange
_eveloperToolbar.jsm 151 CommandUtils.createButtons/<
_eveloperToolbar.jsm 93 CommandUtils.createButtons
_ramework/toolbox.js 542 Toolbox.prototype._buildButtons
_ramework/toolbox.js 248 Toolbox.prototype.open/</domReady

Comment 11

[Brian Grinstead [:bgrins]]

Attachment: telemetry-mochitest.patch

Added comments with more info

Comment 12

[Brian Grinstead [:bgrins]]

Attachment: telemetry-mochitest.patch

Missed one comment

Comment 13

[Brian Grinstead [:bgrins]]

https://hg.mozilla.org/integration/fx-team/rev/209b90cb1173
https://tbpl.mozilla.org/?tree=Fx-Team&rev=209b90cb1173

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/209b90cb1173