1977567 - Thunderbird test regressions caused by new FF pref reset in mochitests

Status: RESOLVED FIXED

(Thunderbird :: Testing Infrastructure, defect)

Description

[Kai Engert [:KaiE:]]

We are experiencing test failures, caused by bug 1975980.

Comment 1

[Rob Wu [:robwu]]

Could you link the relevant test files and/or log entries? That would help with understanding the impact of the regression.

Comment 2

[Martin Giger [:freaktechnik]]

The main problem from what I've been able to tell is that the change now resets unexpected prefs between test runs. Sadly our entire account infrastructure (still) relies on prefs, so we get a whole bunch of non-default prefs that are expected to persist between test files. We also can't affect the ignorePrefs.json at this time from what I can tell. Even just being able to pass a different file name might be helpful enough, since we should be able to bundle an alternate file from our build - however the file name (and source) is currently hard coded in mach as far as I understand it.

Comment 3

[Rob Wu [:robwu]]

The change does two things:

• loudly complaining about changed prefs between tests. Previously, this only happened in TV (--verify) mode.
• reset changed prefs to minimize the impact of one test on another: tests should not depend on prefs set by other tests.

The latter could potentially affect test behavior if there are preference observers with side effects.

Depending on the extent of test failures, the fix could be as simple as adding cleanup logic to some head.js file, or listing some prefs in ignorePrefs.json.

Comment 4

[Martin Giger [:freaktechnik]]

(In reply to Rob Wu [:robwu] from comment #3)

Depending on the extent of test failures, the fix could be as simple as adding cleanup logic to some head.js file

I'm sure some of the failures are our own fault and we need to fix when we're setting and resetting prefs - though forgetting to reset shouldn't fail tests yet (expected failure), the thing we're struggling with primarily is prefs getting resets between test files that we're expecting to stick around as far as I understand.

listing some prefs in ignorePrefs.json.

Which means we'd have to list our prefs in the m-c file currently which is certainly not great...

Comment 5

[Florian Quèze [:florian]]

Attachment: Preference changed by tests in push 1683472.html

(In reply to Rob Wu [:robwu] from comment #1)

Could you link the relevant test files and/or log entries? That would help with understanding the impact of the regression.

https://treeherder.mozilla.org/jobs?repo=comm-central&revision=5da6e60eb093b7c2df73aab979ece1e95dbf0630

Comment 6

[Rob Wu [:robwu]]

Ideally we'd have the ability to specify a separate TB-specific set of preferences to ignore, but since that does not exist yet, the fastest way to unblock you is to add it to ignorePrefs.json

The file supports wildcards, so you can add mail.* to revert back to the original (non-cleanup) behavior for the time being.

A quick inventory of modified prefs according to comment 5's data:

• 468 prefs are in the mail.* branch.
• 895 are in the calendar.* branch, 890 of which are in calendar.registry.*.
• 71 prefs are not in mail or calendar.

Here is an example of a bug revealed by the change: The extensions.webextensions.warnings-as-errors is reported as changed. This pref is set by default in tests to enable stricter extension schema validation checks (e.g. use of deprecated or misspelled properties in manifest.json). If the pref were to be turned off, then this stricter validation would be disabled for all unrelated tests that follow.

There are three files listing these, at https://bug1977567.bmoattachments.org/attachment.cgi?id=9500923#pref-extensions.webextensions.warnings-as-errors . These manually set the preferences and clear them afterwards (e.g. browser_ext_commands_execute_browser_action.js). In this specific case, ExtensionTestUtils.failOnSchemaWarnings(false); should be used instead (examples of uses in m-c: https://searchfox.org/mozilla-central/search?q=ExtensionTestUtils.failOnSchemaWarnings&path=browser%2F&case=false&regexp=false ). This advice applies to all of the tests that manually touch the pref, at https://searchfox.org/comm-central/search?q=extensions.webextensions.warnings-as-errors&path=mail%2F&case=true&regexp=false

In general, in browser tests and mochitests you should use SpecialPowers.pushPrefEnv instead of using Services.prefs API, because the test-only SpecialPowers.pushPrefEnv API automatically cleans up preferences at the end of the test, even if you forget (or intentionally omit) the countering SpecialPowers.popPrefEnv call

Comment 7

[Kai Engert [:KaiE:]]

Attachment: Bug 1977567 - Add mail.* to ignorePrefs.json for Thunderbird. r=florian

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/d3617b8e7642
https://hg.mozilla.org/integration/autoland/rev/5e7b0679badd
Add mail.* to ignorePrefs.json for Thunderbird. r=florian

Comment 9

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/5e7b0679badd