1754536 - Land various intermittent test fixes originally part of bug 1749775 and reenable verify

Status: RESOLVED FIXED

(Firefox :: Messaging System, task, P1)

Description

[Dan Mosedale (:dmosedale, :dmose)]

To land bug 1749775 in a way that is upliftable, we had disabled a test and multiple asserts, that were intermittently failing on all platforms, as well as add a skip-if for verify. The tests need to be fixed and verify should be re-enabled.

Here's how I can reproduce the issues:

• ./mach try fuzzy --no-artifact
• type mochitest-browser-chrome to find all such tests
• press Ctrl-A to select them all
• hit enter to accept

There were lots of problems with the weird interactions and cleanup in the browser_private_browsing_about_nimbus.js file (all the other known ones have been fixed or worked around). It's conceivable that once we get everything migrated to messaging system and away from the bespoke Nimbus feature code (bug 1748124), and all the tests in that file should use setupMSExperimentWithMessage with its associated cleanup code, the problem will disappear.

However, the wiser thing to do may be to simply split that file apart into two files to cut down on the number of possible cross-test interactions.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Bug 1754536 - Fix intermittent failures in private browsing nimbus tests, r?pdahiya

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: WIP: Bug 1754536 - Add more logging to help in debugging

Depends on D138834

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: WIP: Bug 1754536 - Do more aggressive event clearing

Depends on D138835

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: WIP: Bug 1754536 - Request a longer timeout and turn off newtab preloading

Depends on D138836

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: WIP: Bug 1754536 - Make category optional

Depends on D138837

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: WIP: Bug 1754536 - Re-enable dismiss test by uncommenting

Depends on D138838

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: WIP: Bug 1754536 - Add comment about cleanups

Depends on D138839

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

To land bug 1749775 in a way that is upliftable, we had disabled a test and multiple asserts, that were intermittently failing on all platforms, as well as add a skip-if for verify. The tests need to be fixed and verify should be re-enabled.

Here's how I can reproduce the issues:

./mach try fuzzy --no-artifact
type mochitest-browser-chrome to find all such tests
press Ctrl-A to select them all
hit enter to accept

There were lots of problems with the weird interactions and cleanup in the browser_private_browsing_about_nimbus.js file (all the other known ones have been fixed or worked around). It's conceivable that once we get everything migrated to messaging system and away from the bespoke Nimbus feature code (bug 1748124), and all the tests in that file should use setupMSExperimentWithMessage with its associated cleanup code, the problem will disappear.

However, the wiser thing to do may be to simply split that file apart into two files to cut down on the number of possible cross-test interactions.

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

I've attached the work-in-progress patch stack, which I expect to squish down to a smaller number of commits before landing.

This is currently using requestLongerTimeout; we probably want to split it into at least two files instead. That could be a separate bug, though.

We only have a small number of intermittents left (currently 2; there used to be 10-20 or more on this try run):

https://treeherder.mozilla.org/jobs?repo=try&revision=7219b7bd345f313a57b26eec182265c0d51e3cdc

Comment 10

[Dan Mosedale (:dmosedale, :dmose)]

Punam, Andrei and I chatted about this last week, and here are some useful links and a plan that came out of that meeting:

Punam’s debugging work: https://phabricator.services.mozilla.com/D138978

Useful links for telemetry testing and intermittent diagnosis:

• https://firefox-source-docs.mozilla.org/devtools/frontend/telemetry.html?highlight=telemetrytestutils#testing
• https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/collection/events.html?highlight=telemetrytestutils#testing
• https://firefox-source-docs.mozilla.org/devtools/tests/debugging-intermittents.html

Plan to fix intermittents

• Use existing patch stack in this bug
• Remove local branch commits making the category on WaitForEventsToClear be optional: always wait for “normandy”
• Split out each new MS test (and anything with TelemetryTestUtils) into its own file
• If we’re not there yet:
  switch aboutPrivateBrowsing.js to message that parent using RemotePageAccessManager so that the exposure telemetry is sent from AboutPrivateBrowsingParent rather than AboutPrivateBrowsingChild
• change the tests to check “parent” rather than “content”.

Comment 11

[Dan Mosedale (:dmosedale, :dmose)]

Probably want to also have a patch (could be part of a different bug) to add these debugging links to the part of firefox-source-docs about messaging system debugging.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0ab99a4d3c5c
Fix intermittent failures in private browsing nimbus tests, r=pdahiya

Comment 13

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/0ab99a4d3c5c