1350941 - Changing TelemetryController._onUploadPrefChange to async/await causes xpcshell tests to fail

Status: NEW

(Toolkit :: Telemetry, defect, P4)

Description

[Deepjyoti Mondal]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20170317213149

Steps to reproduce:

In TelemetryController.jsm, in _onUploadPrefChange function change Task.spawn to async function ,i.e, Task.spawn(function*() {}) to (async function() {})() and change yield to await

run ./mach xpcshell-test toolkit/components/telemetry/tests/unit/test_TelemetryController.js


Actual results:

The test should have passed


Expected results:

The test fails. However in the Task.spawn function if the order of ping removal is changed that is first the pending pings are removed and then the current pings are removed then the test passes

Comment 1

[Deepjyoti Mondal]

@Alessio I would love to work on this bug, how should I proceed?

Comment 2

[Alessio Placitelli [:Dexter]]

I warn you, this bug is potentially involving and troublesome. Feel free to pick another bug if it doesn't sound too interesting! :-)

This is an investigation bug, the expected outcome of it should be:

1) Figure out why the change makes the test fail.
2) Fix the problem.

I'd love to stress how the first point is important: we don't want to change the order of any operation unless we understand exactly what is going on behind the scenes and why that fixes the issue.

With that said, there's no fixed set of steps to fix it. What I would do:

- Run the test without the change, and save the log output to a file.
- Change the code to reproduce the problem.
- Dump the log output of the failing test to a different file.
- Analyse the logs to figure out what's going on, how is that behaving different.

Comment 3

[Deepjyoti Mondal]

@Alessio I would like to give it a try, I will start working on it ASAP. I will let you know if I am stuck :)