1519480 - Replace browser_webauthn_telemetry.js's getParentProcessScalars with a call to the one in TelemetryTestUtils

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P3)

Description

[Chris H-C :chutten]

Thanks to bug 1518152 we now have a TelemetryTestUtils module that contains some common test code. browser_webauthn_telemetry.js should be changed to use the common code instead of its own getParentProcessScalars implementation.

To help Mozilla out with this bug, here's the steps:

0. Comment here on the bug that you want to volunteer to help. I (or someone else) will assign it to you.
1. Download and build the Firefox source code: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build
   • If you have any problems, please ask on IRC (https://wiki.mozilla.org/Irc) in the #introduction channel. They're there to help you get started.
   • You can also read the Developer Guide, which has answers to most development questions: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction
2. Start working on this bug. You'll be working in the browser_webauthn_telemetry.js file, replacing its own getParentProcessScalars with a call to TelemetryTestUtils.getParentProcessScalars. The function signatures are slightly different, so you may need to add Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTOUT to the call.
   • If you have any problems with this bug, please comment on this bug and set the needinfo flag for me. Also, you can find me and my teammates on the #telemetry channel on IRC (https://wiki.mozilla.org/Irc) most hours of most days.
3. Build your change with mach build and test your change with mach test dom/webauthn/tests/browser/browser_webauthn_telemetry.js. Also check your changes for adherence to our style guidelines by using mach lint
4. Submit the patch for review. Mark me as a reviewer so I'll get an email to come look at your code.
   • Here's the guide: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch
5. After a series of reviews and changes to your patch, I'll mark it for checkin or push it to autoland. Your code will soon be shipping to Firefox users worldwide!
6. ...now you get to think about what kind of bug you'd like to work on next. Let me know what you're interested in and I can help you find your next contribution.

Comment 1

[Varun [he/him IST]]

Hey @chutten I would like to take it up!

Comment 2

[Chris H-C :chutten]

Alright, I have assigned this to you. Since this is your first bug, I'll leave bug 1519476 unassigned for now. We can get to it after this one's done.

Comment 3

[Varun [he/him IST]]

Attachment: Bug 1519480 - Update browser_webauthn_telemetry.js getParentProcessScalars with TelemetryTestUtils

Replacing browser_webauthn_telemetry.js's custom getParentProcessScalars method with the method defined in TelemetryTestUtils module

Comment 4

[Varun [he/him IST]]

Hi Chris! Can you update this as well.

Comment 5

[Chris H-C :chutten]

I'm not sure what you mean by updating this? When the patch makes its way from autoland to mozilla-central it should trigger this to be RESOLVED>FIXED. Is that what you meant?

Comment 6

[Chris H-C :chutten]

Ohhh, this is the first bug. That's what I get for not looking closely enough.

Did you get a failure email when the landing failed? I was expecting a status update to Phabricator or Bugzilla to tip me off, but I didn't receive anything.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/60ee07f3171f
Update browser_webauthn_telemetry.js getParentProcessScalars with TelemetryTestUtils r=chutten,jcj

Comment 8

[Stefan Hindli [:stefan_hindli]]

Backed out changeset 60ee07f3171f (Bug 1519480) for TV and bc failures in browser_webauthn_telemetry.js CLOSED TREE

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=223380946

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=223380946&repo=autoland&lineNumber=4997

task 2019-01-22T22:14:31.019Z] 22:14:31 INFO - TEST-INFO | screentopng: exit 0
[task 2019-01-22T22:14:31.020Z] 22:14:31 INFO - Buffered messages logged at 22:14:28
[task 2019-01-22T22:14:31.020Z] 22:14:31 INFO - Entering test bound test_setup
[task 2019-01-22T22:14:31.020Z] 22:14:31 INFO - Buffered messages logged at 22:14:29
[task 2019-01-22T22:14:31.020Z] 22:14:31 INFO - Leaving test bound test_setup
[task 2019-01-22T22:14:31.021Z] 22:14:31 INFO - Entering test bound test
[task 2019-01-22T22:14:31.021Z] 22:14:31 INFO - Buffered messages logged at 22:14:30
[task 2019-01-22T22:14:31.022Z] 22:14:31 INFO - TEST-PASS | dom/webauthn/tests/browser/browser_webauthn_telemetry.js | Assertion's user presence byte set correctly -
[task 2019-01-22T22:14:31.024Z] 22:14:31 INFO - TEST-PASS | dom/webauthn/tests/browser/browser_webauthn_telemetry.js | signature is valid -
[task 2019-01-22T22:14:31.025Z] 22:14:31 INFO - Buffered messages finished
[task 2019-01-22T22:14:31.028Z] 22:14:31 INFO - TEST-UNEXPECTED-FAIL | dom/webauthn/tests/browser/browser_webauthn_telemetry.js | Uncaught exception - at chrome://mochitests/content/browser/dom/webauthn/tests/browser/browser_webauthn_telemetry.js:10 - ReferenceError: TelemetryTestUtils is not defined
[task 2019-01-22T22:14:31.029Z] 22:14:31 INFO - Stack trace:
[task 2019-01-22T22:14:31.030Z] 22:14:31 INFO - getTelemetryForScalar@chrome://mochitests/content/browser/dom/webauthn/tests/browser/browser_webauthn_telemetry.js:10:7
[task 2019-01-22T22:14:31.031Z] 22:14:31 INFO - test@chrome://mochitests/content/browser/dom/webauthn/tests/browser/browser_webauthn_telemetry.js:141:23
[task 2019-01-22T22:14:31.032Z] 22:14:31 INFO - AsyncTester_execTest/<@chrome://mochikit/content/browser-test.js:1108:34
[task 2019-01-22T22:14:31.033Z] 22:14:31 INFO - asyncTester_execTest@chrome://mochikit/content/browser-test.js:1099:16
[task 2019-01-22T22:14:31.033Z] 22:14:31 INFO - nextTest/<@chrome://mochikit/content/browser-test.js:997:9
[task 2019-01-22T22:14:31.034Z] 22:14:31 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:803:59
[task 2019-01-22T22:14:31.035Z] 22:14:31 INFO - Leaving test bound test
[task 2019-01-22T22:14:31.036Z] 22:14:31 INFO - GECKO(4085) | MEMORY STAT | vsize 20973994MB | residentFast 1184MB
[task 2019-01-22T22:14:31.037Z] 22:14:31 INFO - TEST-OK | dom/webauthn/tests/browser/browser_webauthn_telemetry.js | took 1146ms
[task 2019-01-22T22:14:31.038Z] 22:14:31 INFO - Not taking screenshot here: see the one that was previously logged
[task 2019-01-22T22:14:31.039Z] 22:14:31 INFO - TEST-UNEXPECTED-FAIL | dom/webauthn/tests/browser/browser_webauthn_telemetry.js | Found an unexpected tab at the end of test run: https://example.com/ -
[task 2019-01-22T22:14:31.040Z] 22:14:31 INFO - checking window state
[task 2019-01-22T22:14:32.906Z] 22:14:32 INFO - GECKO(4085) | Completed ShutdownLeaks collections in process 4315
[task 2019-01-22T22:14:32.934Z] 22:14:32 INFO - GECKO(4085) | Completed ShutdownLeaks collections in process 4283
[task 2019-01-22T22:14:33.004Z] 22:14:33 INFO - GECKO(4085) | Completed ShutdownLeaks collections in process 4253
[task 2019-01-22T22:14:33.022Z] 22:14:33 INFO - GECKO(4085) | Completed ShutdownLeaks collections in process 4177
[task 2019-01-22T22:14:33.106Z] 22:14:33 INFO - GECKO(4085) | Completed ShutdownLeaks collections in process 4196
[task 2019-01-22T22:14:33.938Z] 22:14:33 INFO - GECKO(4085) | Completed ShutdownLeaks collections in process 4085
[task 2019-01-22T22:14:33.938Z] 22:14:33 INFO - TEST-START | Shutdown
[task 2019-01-22T22:14:33.939Z] 22:14:33 INFO - Browser Chrome Test Summary
[task 2019-01-22T22:14:33.939Z] 22:14:33 INFO - Passed: 41
[task 2019-01-22T22:14:33.939Z] 22:14:33 INFO - Failed: 2
[task 2019-01-22T22:14:33.940Z] 22:14:33 INFO - Todo: 0
[task 2019-01-22T22:14:33.941Z] 22:14:33 INFO - Mode: e10s
[task 2019-01-22T22:14:33.943Z] 22:14:33 INFO - *** End BrowserChrome Test Results ***
[task 2019-01-22T22:14:34.601Z] 22:14:34 INFO - GECKO(4085) | ###!!! [Parent][RunMessage] Error: Channel closing: too late to send/recv, messages will be lost
[task 2019-01-22T22:14:34.622Z] 22:14:34 INFO - GECKO(4085) | ###!!! [Parent][RunMessage] Error: Channel closing: too late to send/recv, messages will be lost
[task 2019-01-22T22:14:36.609Z] 22:14:36 INFO - GECKO(4085) | 1548195276602 Marionette TRACE Received observer notification xpcom-will-shutdown
[task 2019-01-22T22:14:36.610Z] 22:14:36 INFO - GECKO(4085) | 1548195276607 Marionette INFO Stopped listening on port 2828
[task 2019-01-22T22:14:36.618Z] 22:14:36 INFO - GECKO(4085) | 1548195276613 Marionette DEBUG Remote service is inactive

Comment 9

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0216ba36d52e
Backed out changeset 60ee07f3171f for TV and bc failures in browser_webauthn_telemetry.js CLOSED TREE

Comment 10

[Chris H-C :chutten]

Hey, Varun. This stuff means that there was a problem landing your patch. The most relevant line from the failure is probably

[task 2019-01-22T22:14:31.028Z] 22:14:31 INFO - TEST-UNEXPECTED-FAIL | dom/webauthn/tests/browser/browser_webauthn_telemetry.js | Uncaught exception - at chrome://mochitests/content/browser/dom/webauthn/tests/browser/browser_webauthn_telemetry.js:10 - ReferenceError: TelemetryTestUtils is not defined

Which we've seen before. I'm a little confused as to why this doesn't pop locally when you run the tests, but it's a given that the environment on the tryservers is not the environment on dev machines.

The next step is to adjust your patch to fix this error and put it back up for review. You may need to go to Phabricator and reopen the revision before it'll let you push a new version. I will run this new one through try before pushing to autoland this time to ensure we've got it right, and then we'll push again.

Sound good?

Comment 11

[Varun [he/him IST]]

Hey Chris, I have updated the PR. Should work as expected now!

Comment 12

[Chris H-C :chutten]

I've pushed it to try to run xpcshell and mochitests on a few platforms: https://treeherder.mozilla.org/#/jobs?repo=try&revision=630de568de7f9895436571f1be69d1d214362af7

If that comes back green (or mostly green with the couple of oranges due to unrelated intermittent failures), we're good to go.

Comment 13

[Chris H-C :chutten]

Came back green! (The oranges turned green when retried) I'll go ahead and give this a push.

Do you want to file the bug I mentioned in https://phabricator.services.mozilla.com/D17032 for your next, somewhat-larger contribution?

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7013cb0056a2
Update browser_webauthn_telemetry.js getParentProcessScalars with TelemetryTestUtils r=chutten,jcj

Comment 15

[Varun [he/him IST]]

A bit confused here. You want me to file the bug or work on the bug once it is filed?

Comment 16

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/7013cb0056a2

Comment 17

[Chris H-C :chutten]

I think it would be great if you wanted to do both :)

Comment 18

[Varun [he/him IST]]

Please review https://bugzilla.mozilla.org/show_bug.cgi?id=1524227 and feel free to add/edit/remove any information. I'll start once you have made any necessary changes.