Closed Bug 1539830 Opened 5 months ago Closed 3 months ago

Develop an integration test for the telemetry optout ping

Categories

(Toolkit :: Telemetry, enhancement, P1)

enhancement
Points:
3

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: raphael, Assigned: chutten)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Extend the telemetry-tests-client suite with a test focused on verifying that Firefox attempts to send out an optout ping when a user changes the Firefox's data choices preference. If we receive an optout ping it must not contain a client id and environment data. Also check that any pending pings are discarded.

See:
https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/data/optout-ping.html
https://searchfox.org/mozilla-central/source/toolkit/components/telemetry/tests/unit/test_TelemetryController.js

Priority: -- → P3

Pre-condition: Ensure that Firefox doesn't think it's running around local midnight (avoids throttling)

  1. Take a look at that new-profile ping
    • It has a client_id. Remember that for later.
  2. Disable the Data Choices Preference (about:preferences#privacy under "Data Collection and Use")
    • Await an optout ping. ( this is a "best effort" ping, but I think in the controlled environment of our tests we can make the stronger assert that it will be sent )
    • validate its contents (has no client_id, has no environment, has empty payload)
  3. Shut down Firefox
    • Ensure no pings are sent.
    • After Firefox has shut down, ensure no pings are in the pending pings directory on disk (which is in the user app data dir under the "Pending Pings" subdirectory)
  4. Start Firefox
  5. Enable the Data Choices Preference
    • Just before this, ensure that we've still sent no pings.
  6. Shut down Firefox
    • (This ensures that at least one main ping is sent)
    • Ensure that the main ping with reason shutdown contains a client_id that DOES NOT match the client_id from Step 0
    • Examine all pings from the suite:
      • none should have the canary client_id of "c0ffeec0-ffee-c0ff-eec0-ffeec0ffeec0"
      • exactly one should have doctype optout

ni?janerik - Can you think of any holes in this test plan? Any behaviours I'll miss? Any assertions that won't always hold?

ni?raphael - Does the framework have the capabilities this plan will need? Configuring the client clock and turning off a pref in the UI are two areas I'm specifically not sure about.

Assignee: nobody → chutten
Status: NEW → ASSIGNED
Points: --- → 3
Flags: needinfo?(rpierzina)
Flags: needinfo?(jrediger)
Priority: P3 → P1

The plan looks good so far.
While optout is "best effort" we control the environment here and, if we assume the local network holds, the ping should get out.

Do we have a chance to uncleanly shut down the browser (e.g. crash it)? Would make for a good other test to trigger the aborted-session ping (we had issues where we did save it with a c0ffee client ID and it went out afterwards).

Otherwise this seems like a good first approach.

Flags: needinfo?(jrediger)

Hey :chutten,

I don't think the framework currently supports turning off a preference in the UI. Marionette uses a JS call to set preferences. We might have to add a tool like Selenium to the suite for that and use Marionette's set_pref for the time being.

As for the setting the client clock to avoid ping throttling, :janerik added a testing preference that disables throttling for the test run. See disableFuzzingDelay.

Please note that the testcase base class in the suite implements a helper method for verifying UUIDs named assertIsValidUUID.

Flags: needinfo?(rpierzina)

Comment on attachment 9061710 [details]
Bug 1539830 - Ensure Environment is init'd before trying to install addon in telemetry-tests-client r?raphael

Revision D29388 was moved to bug 1547750. Setting attachment 9061710 [details] to obsolete.

Attachment #9061710 - Attachment is obsolete: true

I'm running into an issue where the Firefox started in Step 3 has data upload re-enabled before I get to enable it in Step 4.

Raphael, would the driver override the profile's preferences when the app starts up that second time? Do you know of a way to stop it?

Otherwise, maybe I'll need to rewrite the test plan to avoid restarting.

Flags: needinfo?(rpierzina)

Clearing the ni request, since :chutten and I discussed this on IRC.

Summary

We verified that setting preferences via marionette.set_pref() works, however we also found that restarting the browser marionette.restart(clean=False, in_app=True) resets the preferences. When then confirmed with :whimboo and he suggested to use marionette.enforce_gecko_prefs() when wanting to overwrite user preferences, which were originally set when the test runner is being initialized.

Flags: needinfo?(rpierzina)

There have been some issues doing that. It seems as though enforce_gecko_prefs() doesn't actually get the job done (when it restarts it once again has upload enabled.) This seems to be because I already manually set the pref I was hoping to enforce.

If I enforce the pref without setting it, it properly resets with upload disabled (but because it was disabled while Firefox wasn't running, it still had a clientId present that needed to be cleared).

So maybe I need to break this up into two separate tests:

  1. Test that turning off datareporting.healthreport.uploadEnabled sends an "optout" ping of correct and valid dimensions and that the Pending Pings directory is empty after Firefox closes.

  2. Test that starting a browser with datareporting.healthreport.uploadEnabled set to false doesn't send pings until after it is set to True.

I'd really rather not do this as it doesn't properly mimic user behaviour.

But I've tried many other things that don't work so my hand might be forced.

Attachment #9062248 - Attachment description: Bug 1539830 - WIP optout ping test → Bug 1539830 - Test "optout" ping. r?raphael
See Also: → 1550718
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
See Also: → 1552111
You need to log in before you can comment on or make changes to this bug.