Closed Bug 1506390 Opened 9 months ago Closed 9 months ago

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

Categories

(Core :: Permission Manager, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox-esr60 --- fixed
firefox63 --- wontfix
firefox64 --- fixed
firefox65 --- fixed

People

(Reporter: mkmelin, Assigned: mkmelin)

References

Details

Attachments

(1 file)

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).
Attachment #9024208 - Flags: review?(ehsan)
Blocks: 1072748
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.
Priority: -- → P3
(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?
Flags: needinfo?(jhofmann)
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...
Attachment #9024208 - Flags: review?(ehsan) → review+
Flags: needinfo?(jhofmann)
Yes, for each test. I think re-tries would be very messy otherwise.
Pushed by mkmelin@iki.fi:
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
https://hg.mozilla.org/mozilla-central/rev/ca52fc256662
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
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.
Flags: needinfo?(ryanvm)
Attachment #9024208 - Flags: approval-mozilla-esr60?
Attachment #9024208 - Flags: approval-mozilla-beta?
Attachment #9024208 - Flags: approval-mozilla-esr60?
Attachment #9024208 - Flags: approval-mozilla-beta?
Whiteboard: [checkin-needed-beta][checkin-needed-esr60]
Flags: needinfo?(ryanvm)
https://hg.mozilla.org/releases/mozilla-beta/rev/a96813612e0c
Whiteboard: [checkin-needed-beta][checkin-needed-esr60]
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.
You need to log in before you can comment on or make changes to this bug.