Reduce the set of prefs in the default unittest set to only those required for stable/reliable tests

RESOLVED FIXED in Firefox 66


Last year
7 months ago


(Reporter: jgraham, Assigned: KWierso)


Version 3

Firefox Tracking Flags

(firefox66 fixed)



(1 attachment)

The uniitest user.js file seems to be a mix of things that are required to make running tests run (and do so reliably), and optional features that are enabled in tests. For wpt we would like to be able to run against stable builds with a featureset that's close to what we ship. Therefore it would be good to move all the prefs that are just enabling non-shipping features into some other harness-specific location so that the minimal unittest set is changing only what is necessary to make the tests run.
Priority: -- → P3

FWIW, I did a bunch of try pushes with the various user.js prefs disabled.

This was just a baseline push of the current tip of m-c:

This commented out browser.sessionstore.resume_from_crash:

This commented out extensions.autoDisableScopes:

This commented out extensions.update.notifyUser:

This commented out focusmanager.testmode:

This commented out media.navigator.streams.fake:

This commented out network.preload:

This commented out network.proxy.type:

This commented out toolkit.startup.max_resumed_crashes:

This commented out gfx.font_ahem_antialias_none:

This commented out network.http.phishy-userpass-length:

This commented out the whole block of safebrowsing prefs:

And this commented out dom.disable_beforeunload:

I didn't do a try push for commenting out places.history.enabled, as the comment above it makes it sound like that would do a lot of I/O. It may not need to be set in user.js, but it should probably be set somewhere for sure.

I haven't looked closely at the results, but some things jump out:

Commenting out the phishy userpass length pref seems to cause /fetch/security/embedded-credentials.tentative.sub.html to fail everywhere, and /html/webappapis/dynamic-markup-insertion/opening-the-input-stream/ignore-opens-during-unload.window.html crashed on both linux debug runs, but not on other platforms. There's also maybe an ASAN leak?

Commenting out the ahem antialiasing pref seems to have broken all non-linux wpt-reftest chunks in multiple tests.

Commenting out the media.navigator.streams.fake pref seems to cause a few test failures (/mediacapture-image/ImageCapture-creation.https.html and /feature-policy/reporting/camera-report-only.https.html, maybe more).

Wes, since you have started looking at this can you finish it off

Assignee: nobody → wkocher

So to be clear the goal is to move out any prefs that are just enabling features into a seperate pref set. So I think this is about knowing intent rather than something that can be figured out from try pushes. All the ones you mentioned are about test correctness/reliability rather than enabling non-default features, so they seem fine to apply to stable builds.

So for example in the unittest set, the following look like they are enabling features and so should be in a different pref set:,173-182,208-210,238

That list isn't exhaustive, but hopefully it's indicative of the kind of thing we want to look for here.

So this would involve adding an additional file referenced like these[1], putting these prefs into that new file, and then only having the harnesses that need those prefs look up those files?

Any chance you know what controls which user.js files get applied in the various harnesses? The closest I can see is just the search results for [2], but that doesn't really help me.

Flags: needinfo?(james)

Just found which I assume is where I'd need to also reference this new testing/profiles/[non-shipping-features-profile]/user.js file for the various harnesses that currently depend on unittest?

The unittest/user.js file contained a mix of preferences that ensure tests can run (switching web services to dummy servers to prevent hitting networks, etc) and preferences that enable features that aren't shipping by default (turning on touch events, enabling experimental css features, etc).

In the future, we're going to want to run tests of only the features that are being shipped in release builds (or in beta builds, or in esr builds, etc), so we need to move feature-enabling preferences into a different file to make it possible to run tests with various sets of features enabled.

This commit just moves feature-enabling prefs into a new file and then includes that file everywhere unittest prefs were already being included, so it should have no functional difference in the set of preferences being set in test runs.
Attachment #9035495 - Attachment description: Bug 1485386 - Move feature-enabling preferences out of unittest/user.js into a separate file f?jgraham → Bug 1485386 - Move feature-enabling preferences out of unittest/user.js into a separate file
Flags: needinfo?(james)

Just for fun, I did a try push where these moved prefs are not included in the various profiles[1].

Lots of failures, unsurprisingly. Wonder how many of these should just be set by tests that need them?

Pushed by
Move feature-enabling preferences out of unittest/user.js into a separate file r=ahal

So, what would be the next step(s)? Actually moving these prefs into test/harness manifest metadata until we get to the point where the new unittest-features profile can be removed since it doesn't contain anything?

Flags: needinfo?(james)

Do you know why mozprofile would say the renamed unittest-stability profile wouldn't exist? Is there somewhere else I'd need to edit?

Flags: needinfo?(wkocher) → needinfo?(ahal)

Oh right, sorry I should have caught this in review (I forgot it needed to be done). You'll also have to make sure it gets copied into the

Just copy all profiles to all harnesses even if the harness doesn't use it (like wpt). At runtime only the profiles in profiles.json will be used anyway.

Flags: needinfo?(ahal)

If we have to do another round of fixes here anyway, could I bikeshed that we rename unittest-stability to unitest-required. Some of the prefs are to make the harnesses work at all, not just for stability. (I'm not super bothered if this doesn't happen, but I think it's epsilon clearer).

Flags: needinfo?(james)

(In reply to James Graham [:jgraham] from comment #13)

If we have to do another round of fixes here anyway, could I bikeshed that we rename unittest-stability to unitest-required


Pushed by
Move feature-enabling preferences out of unittest/user.js into a separate file r=ahal
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

Could someone please look at out documentation and update it?

I'm looking into Bug 1519538 where some prefs that we currently set in, don't seem to apply anymore to some test harnesses, and I have no idea where I should move those prefs, because there are too many locations (common, perf, unittest-required, unittest-features, ...).

Keywords: dev-doc-needed

Thanks for pointing that out! I'll spend some time updating that.

Tl;dr look at testing/profiles/profiles.json to see which profiles apply to which test suites, and choose accordingly. The current profile structure isn't super well thought out or anything, so feel free to add new profiles and/or re-organize as needed.

I updated it.

Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.