Consolidate Telemetry prefs in head.js

RESOLVED FIXED in Firefox 56

Status

()

enhancement
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Dexter, Assigned: djmdeveloper060796, Mentored)

Tracking

(Blocks 2 bugs)

Trunk
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 wontfix, firefox56 fixed)

Details

(Whiteboard: [measurement:client][lang=js])

Attachments

(1 attachment, 8 obsolete attachments)

Reporter

Description

2 years ago
Some of the Telemetry tests are changing preferences and re-defining them in each test file [1].

We should consolidate all the used preferences in the head.js file [2] and make other test files use these ones.

[1] - https://dxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/tests/unit/head.js
[2] - https://dxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/tests/unit/head.js
Reporter

Updated

2 years ago
Blocks: 1201022
Mentor: alessio.placitelli
Priority: -- → P3
Whiteboard: [measurement:client][lang=js]
BTW there's a fair bit of duplication in the telemetry sources proper. It might be nice to lump them all in a single place.
Assignee

Comment 2

2 years ago
@Alessio 
I would like to work on this. How should I proceed?
Flags: needinfo?(alessio.placitelli)

Comment 3

2 years ago
I am interested in solving this bug.how should i go about it?
Assignee

Comment 5

2 years ago
@Alessio Okay, so all the pref settings needs to be moved from the other test files into the head.js file and remove duplicate values. How do I make the other test files use head.js pref values. Should I export the prefs and import in other files?
Reporter

Comment 6

2 years ago
(In reply to Deepjyoti Mondal from comment #5)
> @Alessio Okay, so all the pref settings needs to be moved from the other
> test files into the head.js file and remove duplicate values. How do I make
> the other test files use head.js pref values. Should I export the prefs and
> import in other files?

Since there are about 200 preference usages between Telemetry code and test, let's restrict this bug to only dealing with the tests. I'll file a follow-up bug to refactor the non-test code.

We could:

1) Add a new object in TelemetryUtils.jsm [1] that will hold the preferences. Something like

> Preferences: Object.freeze({
>   TelemetryEnabled: "toolkit.telemetry.enabled",
> }),

2) For any pref used in the test code (see comment 4) add an entry to the Preferences object.
3) Import and use TelemetryUtils.Preferences.<pref-name> in the tests.

[1] - https://dxr.mozilla.org/mozilla-central/rev/8d026c60151005ad942e3d4389318fe28a0c8c54/toolkit/components/telemetry/TelemetryUtils.jsm#27
Flags: needinfo?(alessio.placitelli)
Reporter

Updated

2 years ago
Blocks: 1344723
Assignee

Comment 7

2 years ago
Thanks a lot for the pointers, I will start working ASAP
Assignee

Comment 8

2 years ago
Posted patch bug1344295.patch (obsolete) — Splinter Review
Moved all the pereferences to its Preferences object in TelemetryUtils.jsm
Attachment #8845223 - Flags: review?(alessio.placitelli)
Reporter

Comment 9

2 years ago
Comment on attachment 8845223 [details] [diff] [review]
bug1344295.patch

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

Thank you for the patch, that's indeed a good start!

A few observations:

- Let's not put test only preferences in TelemetryUtils.
- Don't put the "Branch" entries in TelemetryUtils, as they were mainly used to save some typing.
- Change the commit message to something like "Bug 1344295: Consolidate Telemetry preferences in TelemetryUtils.Preferences. r?dexter"

::: toolkit/components/telemetry/TelemetryUtils.jsm
@@ +24,4 @@
>  })();
>  
>  this.TelemetryUtils = {
> +  Preferences: Object.freeze({

Please group the preferences as follows and sort alphabetically within each group:

// General preferences
ArchiveEnabled: "telemetry...",
...
TelemetryEnabled: "telemetry...",
...

// Data reporting preferences
FhrUploadEnabled: "datareporting....",
...

@@ +25,5 @@
>  
>  this.TelemetryUtils = {
> +  Preferences: Object.freeze({
> +    TelemetryEnabled: "toolkit.telemetry.enabled",
> +    Branch: "toolkit.telemetry.",

Please remove "Branch" as it was just used to build the preference string. We don't need it here.

@@ +27,5 @@
> +  Preferences: Object.freeze({
> +    TelemetryEnabled: "toolkit.telemetry.enabled",
> +    Branch: "toolkit.telemetry.",
> +    ArchiveEnabled: "toolkit.telemetry.archive.enabled",
> +    Test: "toolkit.telemetry.test.pref1",

Please remove all the test preferences from here, we don't need to share them across the different modules.

@@ +28,5 @@
> +    TelemetryEnabled: "toolkit.telemetry.enabled",
> +    Branch: "toolkit.telemetry.",
> +    ArchiveEnabled: "toolkit.telemetry.archive.enabled",
> +    Test: "toolkit.telemetry.test.pref1",
> +    Enabled: "toolkit.telemetry.enabled",

Please check for duplicates, as this is the same of TelemetryEnabled. Let's drop this one as "Enabled" is too vague.

@@ +32,5 @@
> +    Enabled: "toolkit.telemetry.enabled",
> +    FhrUploadEnabled: "datareporting.healthreport.uploadEnabled",
> +    Unified: "toolkit.telemetry.unified",
> +    CachedClientId: "toolkit.telemetry.cachedClientID",
> +    Test1: "toolkit.telemetry.test.pref_new",

Remove these tests preferences: Test1 to Test5.

@@ +37,5 @@
> +    Test2: "toolkit.telemetry.test.pref1",
> +    Test3: "toolkit.telemetry.test.pref2",
> +    Test4: "toolkit.telemetry.test.pref_old",
> +    Test5: "toolkit.telemetry.test.requiresRestart",
> +    PolicyBranch: "datareporting.policy.",

Remove this policy branch for the same reason we're removing the "Branch" entry.

@@ +42,5 @@
> +    BypassNotification: "datareporting.policy.dataSubmissionPolicyBypassNotification",
> +    DataSubmissionEnabled: "datareporting.policy.dataSubmissionEnabled",
> +    CurrentPolicyVersion: "datareporting.policy.currentPolicyVersion",
> +    MinimumPolicyVersion: "datareporting.policy.minimumPolicyVersion",
> +    MinimumChannelPolicyVersion: "datareporting.policy.minimumPolicyVersion.channel-TestChannelABC",

This is a test preference, we don't need to share it here.

::: toolkit/components/telemetry/tests/unit/head.js
@@ +4,4 @@
>  var { classes: Cc, utils: Cu, interfaces: Ci, results: Cr } = Components;
>  
>  Cu.import("resource://gre/modules/TelemetryController.jsm", this);
> +Cu.import("resource://gre/modules/TelemetryUtils.jsm", this);

Let's not include this here. We won't need it if we remove PREF_TELEMETRY_ENABLED. See below.

@@ +32,4 @@
>  const MILLISECONDS_PER_HOUR = 60 * MILLISECONDS_PER_MINUTE;
>  const MILLISECONDS_PER_DAY = 24 * MILLISECONDS_PER_HOUR;
>  
> +const PREF_TELEMETRY_ENABLED = TelemetryUtils.Preferences.TelemetryEnabled;

Let's just remove PREF_TELEMETRY_ENABLED and simply use TelemetryUtils.Preferences.TelemetryEnabled at the call sites.

::: toolkit/components/telemetry/tests/unit/test_SubsessionChaining.js
@@ +15,5 @@
>  const MS_IN_ONE_HOUR  = 60 * 60 * 1000;
>  const MS_IN_ONE_DAY   = 24 * MS_IN_ONE_HOUR;
>  
> +const PREF_BRANCH = TelemetryUtils.Preferences.Branch;
> +const PREF_ARCHIVE_ENABLED = TelemetryUtils.Preferences.ArchiveEnabled;

Let's remove the PREF_* consts and use the one you introduced in TelemetryUtils at the call sites.

@@ +103,4 @@
>      return;
>    }
>  
> +  const PREF_TEST = TelemetryUtils.Preferences.Test;

You don't need to share test preferences in TelemetryUtils. Let's just change this to be const PREF_TEST = "toolkit.telemetry.test.pref1";

::: toolkit/components/telemetry/tests/unit/test_TelemetryController.js
@@ +28,4 @@
>  const APP_VERSION = "1";
>  const APP_NAME = "XPCShell";
>  
> +const PREF_BRANCH = TelemetryUtils.Preferences.Branch;

Let's drop this PREF_* const and the others below. Just use TelemetryUtils... at the call sites.

::: toolkit/components/telemetry/tests/unit/test_TelemetryControllerShutdown.js
@@ +15,4 @@
>  Cu.import("resource://gre/modules/AsyncShutdown.jsm", this);
>  Cu.import("resource://testing-common/httpd.js", this);
>  
> +const PREF_BRANCH = TelemetryUtils.Preferences.Branch;

Same as the other file.

::: toolkit/components/telemetry/tests/unit/test_TelemetryController_idle.js
@@ +12,5 @@
>  Cu.import("resource://gre/modules/TelemetryController.jsm", this);
>  Cu.import("resource://gre/modules/TelemetrySession.jsm", this);
>  Cu.import("resource://gre/modules/TelemetrySend.jsm", this);
>  
> +const PREF_FHR_UPLOAD_ENABLED = TelemetryUtils.Preferences.FhrUploadEnabled;

Same as the other file.

::: toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js
@@ +857,4 @@
>  });
>  
>  add_task(function* test_prefWatchPolicies() {
> +  const PREF_TEST_1 = TelemetryUtils.Preferences.Test1;

Don't change this prefs, as they are test only. You won't be exporting them in TelemetryUtils.

@@ +914,4 @@
>  });
>  
>  add_task(function* test_prefWatch_prefReset() {
> +  const PREF_TEST = TelemetryUtils.Preferences.Test2;

Same here.

@@ +1434,4 @@
>    Services.search.removeEngine(engine);
>  
>    // Define and reset the test preference.
> +  const PREF_TEST = TelemetryUtils.Preferences.Test2;

Same here.

@@ +1507,4 @@
>  
>  add_task(function* test_environmentShutdown() {
>    // Define and reset the test preference.
> +  const PREF_TEST = TelemetryUtils.Preferences.Test2;

Same here.

::: toolkit/components/telemetry/tests/unit/test_TelemetryReportingPolicy.js
@@ +16,4 @@
>  Cu.import("resource://gre/modules/XPCOMUtils.jsm", this);
>  Cu.import("resource://gre/modules/UpdateUtils.jsm", this);
>  
> +const PREF_BRANCH = TelemetryUtils.Preferences.Branch;

As for the other files, just directly use TelemetryUtils.. at the call sites instead of changing the constants here.

::: toolkit/components/telemetry/tests/unit/test_TelemetrySend.js
@@ +15,4 @@
>  Cu.import("resource://gre/modules/Preferences.jsm", this);
>  Cu.import("resource://gre/modules/osfile.jsm", this);
>  
> +const PREF_TELEMETRY_SERVER = TelemetryUtils.Preferences.Server;

Same here.

::: toolkit/components/telemetry/tests/unit/test_TelemetrySendOldPings.js
@@ +34,4 @@
>  
>  const TOTAL_EXPECTED_PINGS = OVERDUE_PINGS + RECENT_PINGS + OLD_FORMAT_PINGS;
>  
> +const PREF_FHR_UPLOAD = TelemetryUtils.Preferences.FhrUploadEnabled;

Same here.

::: toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
@@ +60,5 @@
>  const MS_IN_ONE_HOUR  = 60 * 60 * 1000;
>  const MS_IN_ONE_DAY   = 24 * MS_IN_ONE_HOUR;
>  
> +const PREF_BRANCH = TelemetryUtils.Preferences.Branch;
> +const PREF_SERVER = TelemetryUtils.Preferences.Server;

Same here.

@@ +1208,4 @@
>    let now = fakeNow(2040, 1, 1, 12, 0, 0);
>    gMonotonicNow = fakeMonotonicNow(gMonotonicNow + 10 * MILLISECONDS_PER_MINUTE);
>  
> +  const PREF_TEST = TelemetryUtils.Preferences.Test2;

Don't change this as it's a test only pref.

@@ +1314,4 @@
>    };
>    yield CommonUtils.writeJSON(sessionState, dataFilePath);
>  
> +  const PREF_TEST = TelemetryUtils.Preferences.Test2;

Don't change this as it's a test only pref.

@@ +1695,4 @@
>    }
>  
>    // Reset the test preference.
> +  const PREF_TEST = TelemetryUtils.Preferences.Test2;

Don't change this as it's a test only pref.

@@ +1982,4 @@
>      return TelemetrySession.getPayload().info.subsessionCounter;
>    };
>  
> +  const PREF_TEST = TelemetryUtils.Preferences.Test2;

Don't change this as it's a test only pref.
Attachment #8845223 - Flags: review?(alessio.placitelli) → feedback+
Reporter

Comment 10

2 years ago
Also, it would be great if you could also update the in-tree docs to mention that all the preferences are exposed from TelemetryUtils. The documentation lives here: http://searchfox.org/mozilla-central/source/toolkit/components/telemetry/docs/internals/preferences.rst
Assignee

Comment 11

2 years ago
I will make the required changes and upload a PR soon.
Assignee

Comment 12

2 years ago
Posted patch bug1344295.patch (obsolete) — Splinter Review
Made the changes you asked for.
Attachment #8845223 - Attachment is obsolete: true
Attachment #8848683 - Flags: review?(alessio.placitelli)
Assignee

Comment 13

2 years ago
@Alessio I would love to work on refactoring the non-test code.
Assignee

Comment 14

2 years ago
@Alesso could you also suggest me some other telemetry issues related to JS
Reporter

Comment 15

2 years ago
Comment on attachment 8848683 [details] [diff] [review]
bug1344295.patch

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

Please make sure to replace all the uses of the non test only PREF_* constants, as you're missing a few of them (see this search for PREF_TELEMETRY_ENABLED, but there's more of them - http://searchfox.org/mozilla-central/search?q=PREF_TELEMETRY_ENABLED&case=false&regexp=false&path=telemetry%2Ftest).

Some tests are also using |TelemetryController.Constants.PREF_SERVER|. Make them use the one in TelemetryUtils instead for the tests (and remove it from TelemetryController if it's not used anymore outside of tests).

::: toolkit/components/telemetry/TelemetryUtils.jsm
@@ +24,5 @@
>  })();
>  
>  this.TelemetryUtils = {
> +  Preferences: Object.freeze({
> +

Please remove this empty line.

@@ +42,5 @@
> +    DataSubmissionEnabled: "datareporting.policy.dataSubmissionEnabled",
> +    FhrUploadEnabled: "datareporting.healthreport.uploadEnabled",
> +    MinimumPolicyVersion: "datareporting.policy.minimumPolicyVersion",
> +    OverrideOfficialCheck: "toolkit.telemetry.send.overrideOfficialCheck"
> +  }),

Please leave a whitespace after this line.

::: toolkit/components/telemetry/tests/unit/test_TelemetryControllerShutdown.js
@@ +31,3 @@
>    yield setEmptyPrefWatchlist();
>  
>    Services.prefs.setBoolPref(PREF_TELEMETRY_ENABLED, true);

This is probably TelemetryUtils.Preferences.TelemetryEnabled

::: toolkit/components/telemetry/tests/unit/test_TelemetryController_idle.js
@@ +22,3 @@
>    yield setEmptyPrefWatchlist();
>  
>    Services.prefs.setBoolPref(PREF_TELEMETRY_ENABLED, true);

This is probably TelemetryUtils.Preferences.TelemetryEnabled

::: toolkit/components/telemetry/tests/unit/test_TelemetryReportingPolicy.js
@@ +19,3 @@
>  const TEST_CHANNEL = "TestChannelABC";
>  
> +const PREF_MINIMUM_CHANNEL_POLICY_VERSION = TelemetryUtils.Preferences.MinimumPolicyVersion + 

nit: remove the trailing whitespace

@@ +57,2 @@
>  
>    Services.prefs.setBoolPref(PREF_TELEMETRY_ENABLED, true);

This is probably TelemetryUtils.Preferences.TelemetryEnabled

::: toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
@@ +497,3 @@
>    yield setEmptyPrefWatchlist();
>  
>    Services.prefs.setBoolPref(PREF_TELEMETRY_ENABLED, true);

This is probably TelemetryUtils.Preferences.TelemetryEnabled
Attachment #8848683 - Flags: review?(alessio.placitelli) → feedback+
Assignee

Comment 16

2 years ago
Posted patch bug1344295.patch (obsolete) — Splinter Review
I have removed all the not test PREF_* with the ones I have declared in TelemetryUtils.js
Attachment #8848683 - Attachment is obsolete: true
Attachment #8850514 - Flags: review?(alessio.placitelli)
Reporter

Comment 17

2 years ago
Comment on attachment 8850514 [details] [diff] [review]
bug1344295.patch

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

This looks good now, thanks!
Attachment #8850514 - Flags: review?(alessio.placitelli) → review+
Reporter

Comment 19

2 years ago
Deepjyoti, it looks like the patch makes some xpcshell tests fail (see comment 18). Would you mind checking that?
Status: NEW → ASSIGNED
Flags: needinfo?(djmdeveloper060796)
Assignee

Comment 20

2 years ago
@Alessio yeah sure, I am on it
Flags: needinfo?(djmdeveloper060796)
Assignee

Comment 21

2 years ago
@Alessio the tests are failing due to 3 files, test_PingAPI.js, test_TelemetryController.js and test_TelemetryReportingPolicy.js I checked test_PingAPI.js and found a typo, I have corrected it, now it passes all tests

However test_TelemetryReportingPolicy.js is producing this error:  ERROR Unexpected exception NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIPrefBranch.setStringPref] and
test_TelemetryController.js is producing the following error : TEST_STATUS: Thread-1 test_archivePings FAIL [test_archivePings : 315] Telemetry must not send pings if not allowed to. - false == true
/home_extension/firefox3/firefox/mozilla-central/obj-x86_64-pc-linux-gnu/_tests/xpcshell/toolkit/components/telemetry/tests/unit/test_TelemetryController.js:test_archivePings/<:315
Flags: needinfo?(alessio.placitelli)
Reporter

Comment 22

2 years ago
That's probably another typo. Please attach the updated patch here so that I can have a look.
Flags: needinfo?(alessio.placitelli) → needinfo?(djmdeveloper060796)
Assignee

Comment 23

2 years ago
Posted patch bug1344295.patch (obsolete) — Splinter Review
This is my updated patch
Attachment #8850514 - Attachment is obsolete: true
Flags: needinfo?(djmdeveloper060796)
Attachment #8851954 - Flags: review?(alessio.placitelli)
Reporter

Updated

2 years ago
Attachment #8851954 - Flags: review?(alessio.placitelli)
Reporter

Comment 24

2 years ago
(In reply to Deepjyoti Mondal from comment #21)
> @Alessio the tests are failing due to 3 files, test_PingAPI.js,
> test_TelemetryController.js and test_TelemetryReportingPolicy.js I checked
> test_PingAPI.js and found a typo, I have corrected it, now it passes all
> tests
> 
> However test_TelemetryReportingPolicy.js is producing this error:  ERROR
> Unexpected exception NS_ERROR_ILLEGAL_VALUE: Component returned failure
> code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIPrefBranch.setStringPref] and
> test_TelemetryController.js is producing the following error : TEST_STATUS:
> Thread-1 test_archivePings FAIL [test_archivePings : 315] Telemetry must not
> send pings if not allowed to. - false == true
> /home_extension/firefox3/firefox/mozilla-central/obj-x86_64-pc-linux-gnu/
> _tests/xpcshell/toolkit/components/telemetry/tests/unit/
> test_TelemetryController.js:test_archivePings/<:315

I've applied your patch and, as you reported, it is failing. It's because of some typos :-)
Please follow the trails left by the logs to figure out where the problems are. For example, in test_TelemetryReportingPolicy.js, you're calling:

> Preferences.set(TelemetryUtils.Preferences.server, ...

But the property you defined in TelemetryUtils.Preferences starts with a capital S.

Please make sure all your tests run fine locally before uploading the next patch ;-)
Reporter

Updated

2 years ago
Flags: needinfo?(djmdeveloper060796)
Assignee

Comment 25

2 years ago
Posted patch bug1344295.patch (obsolete) — Splinter Review
@Alessio Thanks a lot for pointing it out :)

I checked test_TelemetryController.js howerver it is till showing : 

Thread-1 test_archivePings FAIL [test_archivePings : 315] Telemetry must not send pings if not allowed to. - false == true
/home_extension/firefox3/firefox/mozilla-central/obj-x86_64-pc-linux-gnu/_tests/xpcshell/toolkit/components/telemetry/tests/unit/test_TelemetryController.js:test_archivePings/<:315


I have checked test_archiveings method for any possible typos. But I have have not found any.I think I am missing something. Would you please have look ?
Flags: needinfo?(djmdeveloper060796)
Attachment #8852754 - Flags: review?(alessio.placitelli)
Reporter

Updated

2 years ago
Attachment #8851954 - Attachment is obsolete: true
Reporter

Comment 26

2 years ago
Comment on attachment 8852754 [details] [diff] [review]
bug1344295.patch

What you could do to figure out the problem is the following:

- Restore this file to its previous state.
- Try to change the prefs one function at a time.
- Run the test.
- See when it starts to fail.

This would allow you to pinpoint the issue.
Flags: needinfo?(djmdeveloper060796)
Attachment #8852754 - Flags: review?(alessio.placitelli)
Assignee

Comment 27

2 years ago
@Alessio Okay I will follow the steps you mentioned :)

There might be some delay in uploading the next patch as I will not be available for some time (travelling).
Flags: needinfo?(djmdeveloper060796)
Reporter

Comment 28

2 years ago
(In reply to Deepjyoti Mondal from comment #27)
> @Alessio Okay I will follow the steps you mentioned :)
> 
> There might be some delay in uploading the next patch as I will not be
> available for some time (travelling).

No problem!
Assignee

Comment 29

2 years ago
Posted patch bug1344295.patch (obsolete) — Splinter Review
@Alessio I have updated my patch, all the tests are now passing.
Attachment #8852754 - Attachment is obsolete: true
Attachment #8855055 - Flags: review?(alessio.placitelli)
Reporter

Comment 30

2 years ago
Comment on attachment 8855055 [details] [diff] [review]
bug1344295.patch

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

This looks good now, thanks. However, a new test file (or a couple) were added meanwhile. Would you mind rebasing this patch so that I can review the new additions too?
Attachment #8855055 - Flags: review?(alessio.placitelli) → feedback+
Assignee

Comment 31

2 years ago
Posted patch bug1344295_v2.patch (obsolete) — Splinter Review
Updated patch.
Attachment #8855055 - Attachment is obsolete: true
Attachment #8856080 - Flags: review?(alessio.placitelli)
Reporter

Comment 32

2 years ago
Comment on attachment 8856080 [details] [diff] [review]
bug1344295_v2.patch

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

This looks good now, thanks!
Attachment #8856080 - Flags: review?(alessio.placitelli) → review+

Comment 34

2 years ago
Pushed by alessio.placitelli@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/16e85d7e7400
Consolidate Telemetry preferences in TelemetryUtils.Preferences. r=dexter
Reporter

Comment 35

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/16e85d7e7400febe85909d6f1baa5b0ff03fb2ab
Bug 1344295 - Consolidate Telemetry preferences in TelemetryUtils.Preferences. r=dexter
backed out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=90102719&repo=mozilla-inbound
Flags: needinfo?(djmdeveloper060796)

Comment 37

2 years ago
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d7af59656111
Backed out changeset 16e85d7e7400 for test failures in test_TelemetrySendOldPings.js
Assignee

Comment 38

2 years ago
@Alessio I ran all the tests locally, all the tests are passing.test_TelemetrySendOldPings.js is not failing any test.alessio.placitelli@gmail.com
Flags: needinfo?(djmdeveloper060796) → needinfo?(alessio.placitelli)
Reporter

Updated

2 years ago
Duplicate of this bug: 1355250
Reporter

Comment 40

2 years ago
(In reply to Deepjyoti Mondal from comment #38)
> @Alessio I ran all the tests locally, all the tests are
> passing.test_TelemetrySendOldPings.js is not failing any
> test.alessio.placitelli@gmail.com

It was a problem with rebasing the patch on mozilla-inbound. I'll rebase it again and attempt a new landing today.
Reporter

Updated

2 years ago
Blocks: 1357749
Reporter

Updated

2 years ago
Flags: needinfo?(alessio.placitelli)
Priority: P3 → P1
Reporter

Comment 41

2 years ago
Posted patch bug1344295_v2.patch (obsolete) — Splinter Review
Just rebased the patch, it's working locally.
Attachment #8856080 - Attachment is obsolete: true
Attachment #8883532 - Flags: review+
Reporter

Comment 43

2 years ago
This should hopefully fix the test fallout.
Attachment #8883532 - Attachment is obsolete: true
Attachment #8883572 - Flags: review+
Reporter

Comment 45

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d9dbeee6130f6acbcd83a10beff2b7cf58d1aa5
Bug 1344295 - Consolidate Telemetry preferences in TelemetryUtils.Preferences. r=dexter

Comment 46

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1d9dbeee6130
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.