1950237 - Importing ExperimentManager has side-effects

Status: RESOLVED FIXED

(Firefox :: Nimbus Desktop Client, defect, P4)

Description

[Beth Rennie [:beth] (she/her)]

Importing ExperimentManager.sys.mjs has side-effects; it will instantiate a global ExperimentManager instance that registers pref listeners. This is really only a problem for unit tests, but it is quite frustrating to deal with.

Comment 1

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1950237 - Annotate crash reports synchronously in ExperimentAPI r?#nimbus-reviewers

Comment 2

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1950237 - Register studies and telemetry pref observers on ExperimentAPI r?#nimbus-reviewers!,mstriemer!

Comment 3

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1950237 - Expose ExperimentManager as ExperimentAPI.manager r?#nimbus-reviewers!

Comment 4

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1950237 - Lazily create the ExperimentManager r?#nimbus-reviewers

Comment 5

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1950237 - Use ExperimentAPI.manager in browser/components/preferences r?#settings-reviewers!

As part of this bug, the global ExperimentManager will no longer be
exported from ExperimentManager.sys.mjs and will only be available as a
property on the ExperimentAPI.

Comment 6

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1950237 - Use ExperimentAPI.manager in browser/components/urlbar r?#urlbar-reviewers!

As part of this bug, the global ExperimentManager will no longer be
exported from ExperimentManager.sys.mjs and will only be available as a
property on the ExperimentAPI.

Comment 7

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1950237 - Use ExperimentAPI._manager in browser/extensions/newtab r?#home-newtab-reviewers!

As part of this bug, the global ExperimentManager will no longer be
exported from ExperimentManager.sys.mjs and will only be available as a
property on the ExperimentAPI.

Comment 8

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1950237 - Use ExperimentAPI.manager in services/fxaccounts r?markh!

As part of this bug, the global ExperimentManager will no longer be
exported from ExperimentManager.sys.mjs and will only be available as a
property on the ExperimentAPI.

Comment 9

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1950237 - Use ExperimentAPI.manager in toolkit/components/backgroundtasks r?nalexander,bytesized

As part of this bug, the global ExperimentManager will no longer be
exported from ExperimentManager.sys.mjs and will only be available as a
property on the ExperimentAPI.

Comment 10

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1950237 - Use ExperimentAPI.manager in toolkit/components/messaging-system r?#omc-reviewers!

As part of this bug, the global ExperimentManager will no longer be
exported from ExperimentManager.sys.mjs and will only be available as a
property on the ExperimentAPI.

Comment 11

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1950237 - Use NimbusTestUtils.setupTest in toolkit/components/telemetry r?chutten!,mviar

As part of this bug, the global ExperimentManager will no longer be
exported from ExperimentManager.sys.mjs and will only be available as a
property on the ExperimentAPI.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1ac8526270fe
Annotate crash reports synchronously in ExperimentAPI r=nimbus-reviewers,relud

Comment 13

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/1ac8526270fe

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de62e5a097f3
Register studies and telemetry pref observers on ExperimentAPI r=nimbus-reviewers,settings-reviewers,relud

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/57d0fd242884
Revert "Bug 1950237 - Register studies and telemetry pref observers on ExperimentAPI r=nimbus-reviewers,settings-reviewers,relud" for causing bc failures @browser_experimental_features.js.

Comment 16

[agoloman]

Backed out for causing bc failures @browser_experimental_features.js.

• Backout link
• Push with failures
• Failure Log

Comment 17

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1950237 - Remove ExperimentManager.studiesEnabled r?#nimbus-reviewers

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0d55bf3d56e4
Register studies and telemetry pref observers on ExperimentAPI r=nimbus-reviewers,settings-reviewers,relud

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2ba4ff2cb25e
Expose ExperimentManager as ExperimentAPI.manager r=nimbus-reviewers,relud,nalexander

Comment 20

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/0d55bf3d56e4

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0f058d299380
Use ExperimentAPI.manager in browser/components/preferences r=settings-reviewers,Gijs

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/764e5161e5a2
Use ExperimentAPI._manager in browser/extensions/newtab r=home-newtab-reviewers,mconley

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eb10739459f3
Use ExperimentAPI.manager in services/fxaccounts r=markh

Comment 24

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/2ba4ff2cb25e
https://hg.mozilla.org/mozilla-central/rev/0f058d299380
https://hg.mozilla.org/mozilla-central/rev/764e5161e5a2
https://hg.mozilla.org/mozilla-central/rev/eb10739459f3

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/45fad543e24b
Use ExperimentAPI.manager in toolkit/components/backgroundtasks r=nalexander

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/68a78f816fdf
Use ExperimentAPI.manager in toolkit/components/telemetry r=chutten

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/38d79c19518e
Use ExperimentAPI.manager in toolkit/components/messaging-system r=omc-reviewers,emcminn

Comment 28

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1950237 - Return whether or not the ExperimentAPI was initialized from init() r?#nimbus-reviewers

This is useful in test utilities used across browser tests and xpcshell
tests because in browser tests the ExperimentAPI will be initialized via
category manager (via Normandy.init) and therefore know if we have to
shut down the ExperimentAPI at the end of the test.

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d1a78697ae15
Revert "Bug 1950237 - Use ExperimentAPI.manager in toolkit/components/telemetry r=chutten" for causing xpc failures @ test_dap.js

Comment 30

[Sandor Molnar[:smolnar]]

Backed out for causing xpc failures @ test_dap.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/d1a78697ae15eaf2c7e7f2d97e60c469329e8eb0

Push with failures

Failure log -> TEST-UNEXPECTED-FAIL | toolkit/components/telemetry/dap/tests/xpcshell/test_dap.js

Comment 31

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/45fad543e24b

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e0d264aef05a
Use ExperimentAPI.manager in toolkit/components/telemetry r=chutten

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/870834d81717
Return whether or not the ExperimentAPI was initialized from init() r=nimbus-reviewers,relud

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dcfe2935a4ef
Remove ExperimentManager.studiesEnabled r=nimbus-reviewers,relud

Comment 35

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/70accffee05f
Revert "Bug 1950237 - Use ExperimentAPI.manager in toolkit/components/telemetry r=chutten" for causing xpc failures @test_TelemetryReportingPolicy.js. This reverts commit c93eb1684f5f4c0083e2c1154dfe6e65cab5efe1.

Comment 36

[agoloman]

Backed out for causing xpc failures @test_TelemetryReportingPolicy.js.

• Backout link
• Push with failures
• Failure Log

Comment 37

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1950237 - Only attempt to remove CrashReporter annotation if ExperimentAPI is initialized r?#nimbus-reviewers

Comment 38

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e473aa82ffe1
Only attempt to remove CrashReporter annotation if ExperimentAPI is initialized r=nimbus-reviewers,relud

Comment 39

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c57a338402cf
Use NimbusTestUtils.setupTest in toolkit/components/telemetry r=chutten

Comment 40

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/98562a8b3fdd
Use ExperimentAPI.manager in browser/components/urlbar r=urlbar-reviewers,Standard8

Comment 41

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/e0d264aef05a
https://hg.mozilla.org/mozilla-central/rev/870834d81717
https://hg.mozilla.org/mozilla-central/rev/dcfe2935a4ef
https://hg.mozilla.org/mozilla-central/rev/e473aa82ffe1

Comment 42

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/126f9257d3e4
Lazily create the ExperimentManager r=nimbus-reviewers,relud

Comment 43

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/483375e8ee7a
Revert "Bug 1950237 - Lazily create the ExperimentManager r=nimbus-reviewers,relud" for causing ExperimentManager related mochitest failures and xpcshell failures on test_prefFlips.js

Comment 44

[Norisz Fay [:noriszfay]]

Backed out for causing multiple ExperimentManager related mochitest failures and xpcshell failures on test_prefFlips.js

Backout link

Push with failures

Failure logs can be found and opened selecting from the failures seen in above link.

Comment 45

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/644b7e87a953
Lazily create the ExperimentManager r=nimbus-reviewers,relud

Comment 46

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0655d3e3345d
Revert "Bug 1950237 - Lazily create the ExperimentManager r=nimbus-reviewers,relud" for causing bc failures in browser_windowless_troubleshoot_crash.js and browser_interactions_view_time.js

Comment 47

[Cristian Tuns]

Backed out for causing bc failures

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | devtools/client/styleeditor/test/browser_styleeditor_enabled.js | A promise chain failed to handle a rejection: can't access property "ready", ExperimentManager.store is undefined - stack: normandy@resource://gre/modules/Troubleshoot.sys.mjs:1021:9

Failure log:https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=BrRJZD5lTXa0LwGv1pSDaA.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=644b7e87a953d1532f355d5fd0548c62b42679ad
Line: TEST-UNEXPECTED-FAIL | browser/components/places/tests/browser/interactions/browser_interactions_view_time.js | A promise chain failed to handle a rejection: can't access property "ready", ExperimentManager.store is undefined - stack: normandy@resource://gre/modules/Troubleshoot.sys.mjs:1021:9
And other from the link "Push with failures"

Comment 48

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1950237 - Use ExperimentAPI.manager in browser/components/asrouter r?#omc-reviewers

As part of this bug, the global ExperimentManager will no longer be
exported from ExperimentManager.sys.mjs and will only be available as a
property on the ExperimentAPI.

Comment 49

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1950237 - Use ExperimentAPI.manager in toolkit/mozapps/update r?bytesized

As part of this bug, the global ExperimentManager will no longer be
exported from ExperimentManager.sys.mjs and will only be available as a
property on the ExperimentAPI.

Comment 50

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1950237 - Use ExperimentAPI.manager in toolkit/modules/Troubleshoot.sys.mjs r?mconley

As part of this bug, the global ExperimentManager will no longer be
exported from ExperimentManager.sys.mjs and will only be available as a
property on the ExperimentAPI.

Comment 51

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/03b79e98a18d
Use ExperimentAPI.manager in browser/components/asrouter r=omc-reviewers,emcminn

Comment 52

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c983c77da256
Use ExperimentAPI.manager in toolkit/modules/Troubleshoot.sys.mjs r=mconley

Comment 53

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/03b79e98a18d

Comment 54

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8ae195eb4269
Use ExperimentAPI.manager in toolkit/mozapps/update r=application-update-reviewers,nalexander

Comment 55

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6ecc4f6f627d
Lazily create the ExperimentManager r=nimbus-reviewers,relud

Comment 56

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/c983c77da256
https://hg.mozilla.org/mozilla-central/rev/8ae195eb4269
https://hg.mozilla.org/mozilla-central/rev/6ecc4f6f627d