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

RESOLVED FIXED in Firefox 10

Status

()

P1
blocker
RESOLVED FIXED
7 years ago
7 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.
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
No longer blocks: 617414

Comment 11

7 years ago
Removed two more tests which depended on the test directory removed in comment 9 and 10.

Comment 12

7 years ago
(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)
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: 7 years ago
Flags: in-testsuite-
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
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
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.