Closed
Bug 1340586
Opened 7 years ago
Closed 7 years ago
Permissions tests cleanups
Categories
(Toolkit :: Add-ons Manager, defect)
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: aswan, Assigned: aswan)
References
Details
Attachments
(3 files)
The unit tests for webextension permission prompts started small but they've grown fairly large with a lot of repeated code across files and an unfortunate requestLongerTimeout() in browser_extension_permissions.js since it wasn't easy to split that file up without copying a ton of code. This bug is to consider moving all the permissions tests to their own directory (or at least their own .ini file) so they can have their own custom head.js with all the shared code and we can make the individual test files much more fine grained.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → aswan
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8840681 -
Flags: review?(florian)
Attachment #8840682 -
Flags: review?(florian)
Attachment #8840683 -
Flags: review?(florian)
Assignee | ||
Updated•7 years ago
|
Attachment #8840681 -
Flags: review?(florian) → review?(dtownsend)
Attachment #8840682 -
Flags: review?(florian) → review?(dtownsend)
Attachment #8840683 -
Flags: review?(florian) → review?(dtownsend)
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8840681 [details] Bug 1340586 Part 1 Move webextensions tests to their own directory https://reviewboard.mozilla.org/r/115114/#review116900
Attachment #8840681 -
Flags: review?(dtownsend) → review+
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8840682 [details] Bug 1340586 Part 2 Move common code to head.js https://reviewboard.mozilla.org/r/115116/#review116906 ::: browser/base/content/test/webextensions/browser_extension_sideloading.js:246 (Diff revision 1) > is(addon2.userDisabled, false, "Addon 2 should now be enabled"); > > yield BrowserTestUtils.removeTab(gBrowser.selectedTab); > }); > + > +add_task(() => cleanup()); So rather than doing this how about having a global cleanup variable in head.js that you can assign a function to to be run at the start of the cleanup function?
Attachment #8840682 -
Flags: review?(dtownsend)
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8840683 [details] Bug 1340586 Part 3 Split up browser_extension_permissions.js https://reviewboard.mozilla.org/r/115118/#review116908 I didn't look too closely at this as I'm assuming it is all just copy and paste.
Attachment #8840683 -
Flags: review?(dtownsend) → review+
Assignee | ||
Comment 7•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8840682 [details] Bug 1340586 Part 2 Move common code to head.js https://reviewboard.mozilla.org/r/115116/#review116906 > So rather than doing this how about having a global cleanup variable in head.js that you can assign a function to to be run at the start of the cleanup function? Good idea, this is much nicer.
Assignee | ||
Comment 8•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8840683 [details] Bug 1340586 Part 3 Split up browser_extension_permissions.js https://reviewboard.mozilla.org/r/115118/#review116908 Yep
Comment 9•7 years ago
|
||
excited about the r+'s here- looking forward to this landing as it should help out a lot with bug 1333713.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 12•7 years ago
|
||
mozreview-review |
Comment on attachment 8840682 [details] Bug 1340586 Part 2 Move common code to head.js https://reviewboard.mozilla.org/r/115116/#review117182
Attachment #8840682 -
Flags: review?(dtownsend) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 16•7 years ago
|
||
Pushed by aswan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/096a249febe7 Part 1 Move webextensions tests to their own directory r=mossop https://hg.mozilla.org/integration/autoland/rev/c8bcd3a86de6 Part 2 Move common code to head.js r=mossop https://hg.mozilla.org/integration/autoland/rev/06efe0295e1e Part 3 Split up browser_extension_permissions.js r=mossop
Comment 17•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/096a249febe7 https://hg.mozilla.org/mozilla-central/rev/c8bcd3a86de6 https://hg.mozilla.org/mozilla-central/rev/06efe0295e1e
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•