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

RESOLVED FIXED in Firefox 66

Status

()

defect
P3
normal
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: chutten, Assigned: vdey, Mentored)

Tracking

(Blocks 1 bug)

Trunk
mozilla66
Points:
---

Firefox Tracking Flags

(firefox66 fixed)

Details

(Whiteboard: [good first bug][lang=js])

Attachments

(1 attachment)

(Reporter)

Description

3 months ago

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:

  1. Comment here on the bug that you want to volunteer to help. I (or someone else) will assign it to you.
  2. Download and build the Firefox source code: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build
  3. 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.
  4. 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
  5. Submit the patch for review. Mark me as a reviewer so I'll get an email to come look at your code.
  6. 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!
  7. ...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.
(Assignee)

Comment 1

3 months ago

Hey @chutten I would like to take it up!

(Reporter)

Comment 2

3 months ago

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.

Assignee: nobody → varundey20
Status: NEW → ASSIGNED
Priority: -- → P3
(Assignee)

Comment 3

3 months ago
Replacing browser_webauthn_telemetry.js's custom getParentProcessScalars method with the method defined in TelemetryTestUtils module
Attachment #9036607 - Attachment description: Fix 1519480 - Update browser_webauthn_telemetry.js getParentProcessScalars with TelemetryTestUtils → Bug 1519480 - Update browser_webauthn_telemetry.js getParentProcessScalars with TelemetryTestUtils
(Assignee)

Comment 4

3 months ago

Hi Chris! Can you update this as well.

Flags: needinfo?(chutten)
(Reporter)

Comment 5

3 months ago

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?

Flags: needinfo?(chutten)
(Reporter)

Comment 6

3 months ago

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

3 months ago
Pushed by chutten@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/60ee07f3171f
Update browser_webauthn_telemetry.js getParentProcessScalars with TelemetryTestUtils r=chutten,jcj

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 - async
Tester_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

Flags: needinfo?(varundey20)

Comment 9

3 months ago
Backout by shindli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0216ba36d52e
Backed out changeset 60ee07f3171f for TV and bc failures in browser_webauthn_telemetry.js CLOSED TREE
(Reporter)

Comment 10

3 months ago

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?

(Assignee)

Comment 11

3 months ago

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

Flags: needinfo?(varundey20)
(Reporter)

Comment 12

3 months ago

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.

(Reporter)

Comment 13

3 months ago

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

3 months ago
Pushed by chutten@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7013cb0056a2
Update browser_webauthn_telemetry.js getParentProcessScalars with TelemetryTestUtils r=chutten,jcj
(Assignee)

Comment 15

3 months ago

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

Comment 16

3 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
(Reporter)

Comment 17

3 months ago

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

(Assignee)

Comment 18

3 months ago

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.

You need to log in before you can comment on or make changes to this bug.