1723198 - prefs option in xpcshell manifest causes Android tests to run without testing/profiles/xpcshell/user.js

Status: RESOLVED FIXED

(Testing :: XPCShell Harness, defect)

Description

[Rob Wu [:robwu]]

In bug 1722966 the patch was backed out because of an unexpected failure on Android. It turns out that a pref that is supposedly set on all platforms and all test types wasn't set, because somehow the prefs set in testing/profiles/xpcshell/user.js were missing.

The prefs were missing because since bug 1635227 the presence of the prefs option in a xpcshell test manifest triggers logic to generate a new user.js file in updateTestPrefsFile. The logic assumes that the profile directory contains a user.js file. In the default case, this file is generated at testing/profiles/xpcshell/user.js generated in buildPrefsFile. In the remote case (e.g. when Android xpcshell tests are run), the subclass in remotexpcshelltests.py overrides the buildPrefsFile method with one that deletes the local user.js file.
Consequently, when updateTestPrefsFile attempts to generate a new user.js file, it will derive the file from an empty set of preferences (due to the missing user.js).

This can be fixed by not deleting the local user.js file.

And once fixed, we could revisit the changes from bug 1614966 (which added prefs = in some xpcshell manifests (example of xpcshell.ini)), because services.settings.server is already present in testing/profiles/xpcshell/user.js. The prefs added in bug 1614966 didn't do anything until bug 1635227 landed, so the base set of preferences in the user.js file is probably sufficient. This will also make extension tests run faster on Android, because there is no longer a need for pushing a new user.js file for each test to the device.

Comment 1

[Rob Wu [:robwu]]

As mentioned at https://bugzilla.mozilla.org/show_bug.cgi?id=1723189#c4, the unit test at https://searchfox.org/mozilla-central/rev/4b88e0b8cca115009e82fdd65e5bf5812ff99128/toolkit/components/extensions/test/xpcshell/test_ext_schema.js#36-50 was failing on Android because of this bug.

That test verifies that the extensions.webextensions.warnings-as-errors pref set at https://searchfox.org/mozilla-central/rev/4b88e0b8cca115009e82fdd65e5bf5812ff99128/testing/profiles/common/user.js#50 works as expected.

Comment 2

[Rob Wu [:robwu]]

Attachment: Bug 1723198 - Don't delete user.js in remotexpcshelltests.py + tests

The new tests in testing/xpcshell/example/ cover the following scenarios:

• Local AND remote xpcshell tests. In contrast, modifications to
  selftest.py in bug 1635227 only covered runxpcshelltests.

• prefs= inheritance from ancestor manifests as implemented by
  https://hg.mozilla.org/mozilla-central/rev/ac51a01611dc

• Per-file prefs setters (used in bug 1638099).

• Multiple prefs and comments per prefs line.

Comment 3

[Rob Wu [:robwu]]

Try run is green where we care: https://treeherder.mozilla.org/jobs?repo=try&revision=028b837991ba975f1864ef3047e01605754142c4
The new tests pass locally, and I can confirm that they pass on try too (e.g. the X1 job of android-em-7-0-x86_64-lite-qr opt).

I classified the orange failures as pre-existing issues. The x-fis tests are a non-standard configuration (with pre-existing failures) and can be ignored according to nika in the Fission channel on Matrix.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/43a0e54578b1
Don't delete user.js in remotexpcshelltests.py + tests r=jmaher

Comment 5

[Alexandru Michis [:malexandru]]

Backed out 2 changesets (Bug 1723198, Bug 1723536) for causing xpcshell failures in test_SocketScalars.js and test_ext_cookieBehaviors.js
Backout link: https://hg.mozilla.org/integration/autoland/rev/5d3b6ace0a3b0a38f16f92688998e1d0f55ea3cb
Push with failures, failure log:

• test_SocketScalars.js
  (Update) The cause for the failures in test_ext_cookieBehaviors.js was actually Bug 1650089.

Comment 6

[Rob Wu [:robwu]]

The try push was good as seen in comment 3.

The test failure in test_SocketScalars.js is bug 1716307, and unrelated to the change here.

Comment 7

[Rob Wu [:robwu]]

Soft freeze is over, I'm going to reland.

The failure in test_SocketScalars.js is independent of the work here, and bug 1716307 has a patch, so if there are any newly reported failures, associate them with bug 1716307 rather than backing out my patches again.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/16c398716c60
Don't delete user.js in remotexpcshelltests.py + tests r=jmaher

Comment 9

[Iulian Moraru]

Backed out for causing xpcshell failures on test_SocketScalars.js.

Push with failures

Failure log

Backout link

Comment 10

[Rob Wu [:robwu]]

Looking good now with the patch for test_SocketScalars.js at bug 1716307:
https://treeherder.mozilla.org/jobs?repo=try&revision=2baa26aae2202482c4c943999b8dac867000ff49

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/127d855f7cf9
Don't delete user.js in remotexpcshelltests.py + tests r=jmaher

Comment 12

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/127d855f7cf9