Make `TelemetryReportingPolicy` more async
Categories
(Toolkit :: Telemetry, enhancement)
Tracking
()
People
(Reporter: nalexander, Assigned: nalexander)
References
Details
Attachments
(4 files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
phab-bot
:
approval-mozilla-beta+
|
Details | Review |
48 bytes,
text/x-phabricator-request
|
phab-bot
:
approval-mozilla-beta+
|
Details | Review |
Right now, TelemetryReportingPolicy.sys.mjs
is a mix of old-style callbacks and new-style async code. This ticket tracks moving some of the callbacks to async/Promise style code.
Assignee | ||
Comment 1•16 days ago
|
||
This partially reverts the changes of an earlier commit, which made
some things asynchronous. The NimbusFeatures.getAllVariables()
API
is not, in fact, asynchronous, so opening the internals is not
strictly necessary. This simplifies the expression and testing for
later.
N.b.: the existing implementation did not correctly test
_openFirstRunPage
: that function was async and there was no await
.
Therefore existing tests were asserting things about the function
definitions and not the function behaviours.
Assignee | ||
Comment 2•16 days ago
|
||
The goal here is to separate the display of policy (tabs or infobars)
from marking the user notified. This is mostly moving the deck chairs
around.
Assignee | ||
Comment 3•16 days ago
|
||
For testing this:
- a new profile will show a background tab;
- a new profile with
datareporting.policy.firstRunURL=""
will show an infobar after 60s; - quitting before seeing that first infobar will show an infobar after 10s on the next invocation.
N.b.: the data reporting policy state machine determines "New profile" by toolkit.telemetry.reportingpolicy.firstRun=true
, so you can do all the testing in one profile with a little effort.
Backed out for causing xpcshell failures @ test_TelemetryReportingPolicy.js
- Backout link
- Push with failures
- Failure Log
- Failure line:
TEST-UNEXPECTED-FAIL | toolkit/components/telemetry/tests/unit/test_TelemetryReportingPolicy.js | xpcshell return code: 0
https://hg.mozilla.org/mozilla-central/rev/636011fb8288
https://hg.mozilla.org/mozilla-central/rev/2d16ee415c72
Assignee | ||
Comment 9•10 days ago
|
||
This partially reverts the changes of an earlier commit, which made
some things asynchronous. The NimbusFeatures.getAllVariables()
API
is not, in fact, asynchronous, so opening the internals is not
strictly necessary. This simplifies the expression and testing for
later.
N.b.: the existing implementation did not correctly test
_openFirstRunPage
: that function was async and there was no await
.
Therefore existing tests were asserting things about the function
definitions and not the function behaviours.
Original Revision: https://phabricator.services.mozilla.com/D235886
Updated•10 days ago
|
Assignee | ||
Comment 10•10 days ago
|
||
The goal here is to separate the display of policy (tabs or infobars)
from marking the user notified. This is mostly moving the deck chairs
around.
Original Revision: https://phabricator.services.mozilla.com/D236443
Updated•10 days ago
|
Updated•5 days ago
|
Updated•5 days ago
|
Comment 11•5 days ago
|
||
uplift |
Updated•5 days ago
|
Description
•