The default bug view has changed. See this FAQ.

Addons Manager tests should have AMO-related prefs reset after each test to disallow network access

RESOLVED FIXED in Firefox 10

Status

()

Toolkit
Add-ons Manager
P1
blocker
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Unfocused, Assigned: Unfocused)

Tracking

unspecified
mozilla11
Points:
---
Bug Flags:
in-testsuite -
in-litmus -

Firefox Tracking Flags

(firefox10 fixed)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment, 1 obsolete attachment)

We've had a few cases where a test doesn't correctly reset all AMO-related prefs, and therefore subsequent tests end up hitting the network, which is not only a bad idea, but it can lead to test failures. eg:

https://tbpl.mozilla.org/php/getParsedLog.php?id=7870276&tree=Firefox&full=1

The head.js file should just reset these each time.
https://tbpl.mozilla.org/php/getParsedLog.php?id=7907285&tree=Firefox
https://tbpl.mozilla.org/php/getParsedLog.php?id=7907246&tree=Firefox
https://tbpl.mozilla.org/php/getParsedLog.php?id=7907240&tree=Firefox

Hitting the network is pretty much the last thing on earth you want to be doing right now :)
https://tbpl.mozilla.org/php/getParsedLog.php?id=7921482&tree=Mozilla-Inbound

Even more so right now.
https://tbpl.mozilla.org/php/getParsedLog.php?id=7921925&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=7928463&tree=Firefox
https://tbpl.mozilla.org/php/getParsedLog.php?id=7930467&tree=Firefox
https://tbpl.mozilla.org/php/getParsedLog.php?id=7930397&tree=Firefox
https://tbpl.mozilla.org/php/getParsedLog.php?id=7930402&tree=Firefox
https://tbpl.mozilla.org/php/getParsedLog.php?id=7930309&tree=Firefox
Severity: normal → blocker
Priority: -- → P1
https://tbpl.mozilla.org/php/getParsedLog.php?id=7928172&tree=Firefox
https://tbpl.mozilla.org/php/getParsedLog.php?id=7930506&tree=Firefox
https://tbpl.mozilla.org/php/getParsedLog.php?id=7929092&tree=Firefox
https://tbpl.mozilla.org/php/getParsedLog.php?id=7929811&tree=Firefox
https://tbpl.mozilla.org/php/getParsedLog.php?id=7929747&tree=Firefox
https://tbpl.mozilla.org/php/getParsedLog.php?id=7929139&tree=Firefox
https://tbpl.mozilla.org/php/getParsedLog.php?id=7929867&tree=Firefox
Yes, please. Our tests should be runnable without internet access. And they should continue to work if AMO goes down temporarily, or changes location permanently.

We're planning on shutting off outside network access to the build and test machines in the near future, to help enforce things like this.
Blocks: 617414
Tests disabled

https://hg.mozilla.org/mozilla-central/rev/ce998a59ee1d
No longer blocks: 617414
And the rest of them too, for good measure

https://hg.mozilla.org/mozilla-central/rev/a07143afa2d0
Removed two more tests which depended on the test directory removed in comment 9 and 10.
(In reply to Ehsan Akhgari [:ehsan] from comment #11)
> Removed two more tests which depended on the test directory removed in
> comment 9 and 10.

https://hg.mozilla.org/mozilla-central/rev/4d2921df8f34
Created attachment 581780 [details] [diff] [review]
patch rev 1

Most of the prefs are already reset correctly, caching isn't though
Assignee: bmcbride → dtownsend
Attachment #581780 - Flags: review?(bmcbride)
https://tbpl.mozilla.org/php/getParsedLog.php?id=7934810&tree=Mozilla-Aurora
https://tbpl.mozilla.org/php/getParsedLog.php?id=7934850&tree=Mozilla-Aurora
https://tbpl.mozilla.org/php/getParsedLog.php?id=7935162&tree=Mozilla-Aurora
https://tbpl.mozilla.org/php/getParsedLog.php?id=7935304&tree=Mozilla-Aurora
https://tbpl.mozilla.org/php/getParsedLog.php?id=7935500&tree=Mozilla-Aurora
Created attachment 581817 [details] [diff] [review]
Patch v2

New and improved!

And yes, the stuff in head.js is being paranoid on purpose.
Assignee: dtownsend → bmcbride
Attachment #581780 - Attachment is obsolete: true
Attachment #581780 - Flags: review?(bmcbride)
Attachment #581817 - Flags: review?(dtownsend)
And I couldn't finds anything in the xpinstall tests that actually set any relevant prefs, so AFAIK they're fine.
Attachment #581817 - Flags: review?(dtownsend) → review+
http://hg.mozilla.org/mozilla-central/rev/243ff1422033
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
https://tbpl.mozilla.org/php/getParsedLog.php?id=7942014&tree=Mozilla-Aurora - Hello from Aurora's xpinstall/ tests!
Comment on attachment 581817 [details] [diff] [review]
Patch v2

Lets fix this on Aurora too :) No change to any code we ship, just test-related fixed.
Attachment #581817 - Flags: approval-mozilla-aurora?
This may or may not have caused an android test orange (bug 710956), which showed up on this push, but no one knows why/how.
Blocks: 710956

Updated

5 years ago
Duplicate of this bug: 711084
Comment on attachment 581817 [details] [diff] [review]
Patch v2

[Triage Comment]
Stop the insanity :)! Approved for Aurora.
Attachment #581817 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Merged in the followup fix to the mobile browser_addons.js test too, to avoid that going orange.

https://hg.mozilla.org/releases/mozilla-aurora/rev/a7ca1d99832b
status-firefox10: --- → fixed
Marking qa- as I don't think this is something QA needs to verify. Please mark qa+ if this is not the case.
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.