969965 - testUITelemetry will be permaorange when 29 merges to beta on March 17th

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[Phil Ringnalda (:philor)]

https://tbpl.mozilla.org/?tree=Try&rev=bc445a9dbee6&jobname=robocop is current aurora pushed as though it had already been merged to beta. Looks like maybe the feature knows it's supposed to turn itself off when it hits release builds, but the test doesn't bother to check whether the feature exists before testing it.

Comment 1

[Richard Newman [:rnewman]]

Aye, UITelemetry prefs itself off to match telem.

const PREF_BRANCH = "toolkit.telemetry.";
#ifdef MOZ_TELEMETRY_ON_BY_DEFAULT
const PREF_ENABLED = PREF_BRANCH + "enabledPreRelease";
#else
const PREF_ENABLED = PREF_BRANCH + "enabled";
#endif

Comment 2

[Richard Newman [:rnewman]]

Attachment: avoid testUITelemetry dependency on telemetry being enabled.

Per philor, we prefer to turn on a feature to test it than to skip the test. This patch does exactly that (from Java, because we need the pref set before we do the Java-side work), then verifies on the JS side.

Comment 3

[Richard Newman [:rnewman]]

Incidentally, this test fails on local m-c builds if MOZ_TELEMETRY_REPORTING isn't set in your .mozconfig. That means that either everyone has that set, or nobody is running or paying attention to Robocop tests on their local machines. Hmm.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8373081 [details] [diff] [review]
avoid testUITelemetry dependency on telemetry being enabled.

Can't we enable telemetry using the profile that's used for the tests themselves?

http://mxr.mozilla.org/mozilla-central/source/testing/profiles/prefs_general.js#45

Comment 5

[Richard Newman [:rnewman]]

(In reply to Mark Finkle (:mfinkle) from comment #4)

> Can't we enable telemetry using the profile that's used for the tests
> themselves?

I don't think so -- we want the majority of tests to tests what we're actually shipping, and that means telemetry off and no pref set.

(I would love to have per-test profiles, though.)

Comment 6

[Richard Newman [:rnewman]]

Comment on attachment 8373081 [details] [diff] [review]
avoid testUITelemetry dependency on telemetry being enabled.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
  Bug 932092

User impact if declined: 
  None. Tests will be orange on Beta.

Testing completed (on m-c, etc.): 
  Tested locally. Landing on fx-team when trees reopen.

Risk to taking this patch (and alternatives if risky): 
  Test-only.

String or IDL/UUID changes made by this patch:
  None.

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Except if it is urgent, I am going to wait for the patch to be landed first on m-c.

Comment 8

[Richard Newman [:rnewman]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #7)
> Except if it is urgent, I am going to wait for the patch to be landed first
> on m-c.

Oh, for sure; I just didn't want to forget to request uplift!

Comment 9

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/fx-team/rev/0008aed2c971

Comment 10

[Phil Ringnalda (:philor)]

It's test-only, you don't need to request approval - the vast swathes of landings on m-a that say "a=test-only" aren't kidding or lying or edging around the rules, them's the rules.

Comment 11

[Richard Newman [:rnewman]]

Comment on attachment 8373081 [details] [diff] [review]
avoid testUITelemetry dependency on telemetry being enabled.

wfm!

Comment 12

[Richard Newman [:rnewman]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7a6043b28459

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0008aed2c971