1506390 - ensure the default permissions do not intefer with the permission tests that assume no defaults

Status: RESOLVED FIXED

(Core :: Permission Manager, defect, P3)

Description

[Magnus Melin [:mkmelin]]

As I found out in bug 1072748, the xpcshell test do not run with the Firefox preferences (bug 1168178), so for the permission manager tests they implicitly assume no defaults.

This bug is to ensure these tests explicitly ensure there are no default prefs involved. (Thunderbird xpcshell tests do run with the real prefs, and therefore get test failures due to the assumptions).

Comment 1

[Magnus Melin [:mkmelin]]

Attachment: bug1506390_permmmanager_tests.patch

Comment 2

[Johann Hofmann [:johannh]]

Comment on attachment 9024208 [details] [diff] [review]
bug1506390_permmmanager_tests.patch

Review of attachment 9024208 [details] [diff] [review]:
-----------------------------------------------------------------

Please reset these prefs at the end of the test or use SpecialPowers.pushPrefEnv which I think should also be available in xpcshell.

Comment 3

[Magnus Melin [:mkmelin]]

(In reply to Johann Hofmann [:johannh] from comment #2)
> Please reset these prefs at the end of the test or use

Seems like wasted effort since each xpcshell tests get a clean environment when the start?
https://searchfox.org/mozilla-central/rev/7f7c353e969e61a6a85201cc8ad3c3de12ac30d8/testing/xpcshell/runxpcshelltests.py#803

Since you looked, maybe you want to review?

Comment 4

[Johann Hofmann [:johannh]]

Comment on attachment 9024208 [details] [diff] [review]
bug1506390_permmmanager_tests.patch

Review of attachment 9024208 [details] [diff] [review]:
-----------------------------------------------------------------

Ah, that's run per individual test? In any case, since xpcshell tests aren't run with that pref anyway, this seems like a safe thing to do. Stealing review from Ehsan...

Comment 5

[Magnus Melin [:mkmelin]]

Yes, for each test. I think re-tries would be very messy otherwise.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca52fc256662
ensure the default permissions do not intefer with the permission tests that assume no defaults. r=johannh

Comment 7

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/ca52fc256662

Comment 8

[Jorg K (CEST = GMT+2)]

Comment on attachment 9024208 [details] [diff] [review]
bug1506390_permmmanager_tests.patch

This is "test only". Can we please get this uplifted? Sorry, I forgot what the procedure is here.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/1cb019f04188

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a96813612e0c

Comment 11

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/releases/mozilla-esr60/rev/2d3326755541b844e6c193c3e839d4fb91236444 on THUNDERBIRD_60_VERBRANCH
Thanks Ryan, I put this on our branch since we want to try it before 60.4 comes around.