1136390 - test_TelemetryTimestamps.js/test_conditions.js are missing AddonManager initialization

Status: RESOLVED FIXED

(Firefox Health Report Graveyard :: Client: Desktop, defect)

Description

[Alessio Placitelli [:Dexter]]

A try run highlights that we're missing the AddonManager initialization for those tests.

Comment 1

[Alessio Placitelli [:Dexter]]

Attachment: bug1136390.patch

Comment 2

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8568834 [details] [diff] [review]
bug1136390.patch

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

::: browser/experiments/test/xpcshell/head.js
@@ +154,5 @@
>  }
>  
> +// Starts the addon manager without creating app info. We can't directly use
> +// |loadAddonManager| defined above in test_conditions.js as it would make the test fail.
> +function startAddonManagerOnly() {

Isn't this just startupManager()?

::: browser/experiments/test/xpcshell/test_conditions.js
@@ +68,5 @@
>  
>  add_task(function* test_setup() {
>    createAppInfo();
>    gProfileDir = do_get_profile();
> +  startAddonManagerOnly();

Can't you just use startupManager()?

Comment 3

[Alessio Placitelli [:Dexter]]

Comment on attachment 8568834 [details] [diff] [review]
bug1136390.patch

As per our IRC talk, it's not so straightforward to use |startupManager()| as there's some scoping madness going on. startupManager appears to be triggering the use of gAppInfo from "head_addons.js" instead of the head.js it gets called in:

 0:01.06 PROCESS_OUTPUT: Thread-1 (pid:3869) "*************************"
 0:01.06 PROCESS_OUTPUT: Thread-1 (pid:3869) "A coding exception was thrown and uncaught in a Task."
 0:01.06 PROCESS_OUTPUT: Thread-1 (pid:3869) "Full message: TypeError: gAppInfo is null"
 0:01.06 PROCESS_OUTPUT: Thread-1 (pid:3869) "Full stack: test_simpleFields@/home/dexter/mozilla-central/obj-x86_64-unknown-linux-gnu/_tests/xpcshell/browser/experiments/test/xpcshell/test_conditions.js:131:44"
 0:01.06 PROCESS_OUTPUT: Thread-1 (pid:3869) "_run_next_test@/home/dexter/mozilla-central/testing/xpcshell/head.js:1375:9"
 0:01.06 PROCESS_OUTPUT: Thread-1 (pid:3869) "do_execute_soon/<.run@/home/dexter/mozilla-central/testing/xpcshell/head.js:644:9"
 0:01.06 PROCESS_OUTPUT: Thread-1 (pid:3869) "_do_main@/home/dexter/mozilla-central/testing/xpcshell/head.js:207:5"
 0:01.06 PROCESS_OUTPUT: Thread-1 (pid:3869) "_execute_test@/home/dexter/mozilla-central/testing/xpcshell/head.js:504:5"
 0:01.06 PROCESS_OUTPUT: Thread-1 (pid:3869) "@-e:1:1"
 0:01.06 PROCESS_OUTPUT: Thread-1 (pid:3869) "*************************"
 0:01.06 LOG: Thread-1 INFO (xpcshell/head.js) | test run_next_test 1 finished (2)
 0:01.06 LOG: Thread-1 ERROR Unexpected exception TypeError: gAppInfo is null at /home/dexter/mozilla-central/obj-x86_64-unknown-linux-gnu/_tests/xpcshell/browser/experiments/test/xpcshell/test_conditions.js:131
test_simpleFields@/home/dexter/mozilla-central/obj-x86_64-unknown-linux-gnu/_tests/xpcshell/browser/experiments/test/xpcshell/test_conditions.js:131:44
_run_next_test@/home/dexter/mozilla-central/testing/xpcshell/head.js:1375:9
do_execute_soon/<.run@/home/dexter/mozilla-central/testing/xpcshell/head.js:644:9
_do_main@/home/dexter/mozilla-central/testing/xpcshell/head.js:207:5
_execute_test@/home/dexter/mozilla-central/testing/xpcshell/head.js:504:5
@-e:1:1
 
 0:01.06 LOG: Thread-1 INFO exiting test

Comment 4

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8568834 [details] [diff] [review]
bug1136390.patch

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

::: toolkit/modules/tests/xpcshell/test_TelemetryTimestamps.js
@@ +20,5 @@
>  Cu.import("resource://testing-common/AppInfo.jsm");
>  updateAppInfo();
>  
> +let gGlobalScope = this;
> +function loadAddonManager() {

We already have loadAddonManager in the telemetry head.js

Comment 5

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8568834 [details] [diff] [review]
bug1136390.patch

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

And confusingly test_TelemetryTimestamps.js is not part of the Telemetry tests... so nevermind.

Comment 6

[Georg Fritzsche [:gfritzsche]]

Pushed in: https://hg.mozilla.org/integration/fx-team/pushloghtml?changeset=b83c3fa8c80f

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b83c3fa8c80f