Closed Bug 969965 Opened 6 years ago Closed 6 years ago

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

Categories

(Toolkit :: Telemetry, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30
Tracking Status
firefox27 --- unaffected
firefox28 --- unaffected
firefox29 + verified
firefox30 --- fixed
firefox-esr24 --- unaffected
b2g18 --- unaffected
b2g-v1.1hd --- unaffected
b2g-v1.2 --- unaffected
b2g-v1.3 --- unaffected
b2g-v1.3T --- unaffected
b2g-v1.4 --- unaffected

People

(Reporter: philor, Assigned: rnewman)

References

Details

Attachments

(1 file)

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.
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
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.
Attachment #8373081 - Flags: review?(mark.finkle)
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.
Status: NEW → ASSIGNED
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
Attachment #8373081 - Flags: review?(mark.finkle) → review+
(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 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.
Attachment #8373081 - Flags: approval-mozilla-aurora?
Except if it is urgent, I am going to wait for the patch to be landed first on m-c.
(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!
https://hg.mozilla.org/integration/fx-team/rev/0008aed2c971
Hardware: ARM → All
Target Milestone: --- → mozilla30
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 on attachment 8373081 [details] [diff] [review]
avoid testUITelemetry dependency on telemetry being enabled.

wfm!
Attachment #8373081 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/0008aed2c971
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.