Closed Bug 731866 Opened 13 years ago Closed 13 years ago

Copy and port current preference tests to run against the in-content preferences

Categories

(Firefox :: Settings UI, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 15

People

(Reporter: joejoevictor, Assigned: joejoevictor)

References

Details

Attachments

(1 file, 6 obsolete files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_2) AppleWebKit/535.11 (KHTML, like Gecko) Chrome/17.0.963.56 Safari/535.11 Steps to reproduce: Testing for in-content preferences Actual results: Testing for in-content preferences Expected results: Testing for in-content preferences
Summary: in-content preferences testing → Testing for In-content Preferences
Blocks: 718011
Status: UNCONFIRMED → RESOLVED
Closed: 13 years ago
Resolution: --- → DUPLICATE
This bug is not a duplicate. It is part of the project to move preferences to in-content UI. There are a number of tests located at /browser/components/preferences/tests that will need to be copied to /browser/components/preferences/in-content/tests
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: DUPLICATE → ---
Status: REOPENED → ASSIGNED
Component: Untriaged → Preferences
OS: Mac OS X → All
QA Contact: untriaged → preferences
Hardware: x86 → All
Version: unspecified → Trunk
Attached patch Tests for in-content preferences (obsolete) (deleted) — Splinter Review
Attachment #605198 - Flags: review?(jwein)
Attachment #605198 - Flags: review?(bmcbride)
Comment on attachment 605198 [details] [diff] [review] Tests for in-content preferences Review of attachment 605198 [details] [diff] [review]: ----------------------------------------------------------------- From a cursory glance, this looks to be what in the right direction, but they're still testing against the old user interface. ::: browser/components/preferences/in-content/tests/Makefile.in @@ +1,2 @@ > +# ***** BEGIN LICENSE BLOCK ***** > +# Version: MPL 1.1/GPL 2.0/LGPL 2.1 MPL 2.0 here. ::: browser/components/preferences/in-content/tests/privacypane_tests.js @@ +56,5 @@ > + } > + }; > + Services.ww.registerNotification(observer); > + > + let dialog = openDialog("chrome://browser/content/preferences/preferences.xul", "Preferences", These tests are opening the old preferences, so they will need to be updated to use the in-content prefs. These tests should fail if you don't have the patch for the privacy preferences applied :)
Attachment #605198 - Flags: review?(jwein) → feedback+
Tried to run the test with the following command: TEST_PATH=browser/components/preferences/in-content/tests/ make -C obj-ff-dbg/ mochitest-plain but it's giving me the error test not found. Any advise? (In reply to Jared Wein [:jaws] from comment #4) > Comment on attachment 605198 [details] [diff] [review] > Tests for in-content preferences > > Review of attachment 605198 [details] [diff] [review]: > ----------------------------------------------------------------- > > From a cursory glance, this looks to be what in the right direction, but > they're still testing against the old user interface. > > ::: browser/components/preferences/in-content/tests/Makefile.in > @@ +1,2 @@ > > +# ***** BEGIN LICENSE BLOCK ***** > > +# Version: MPL 1.1/GPL 2.0/LGPL 2.1 > > MPL 2.0 here. > > ::: browser/components/preferences/in-content/tests/privacypane_tests.js > @@ +56,5 @@ > > + } > > + }; > > + Services.ww.registerNotification(observer); > > + > > + let dialog = openDialog("chrome://browser/content/preferences/preferences.xul", "Preferences", > > These tests are opening the old preferences, so they will need to be updated > to use the in-content prefs. > > These tests should fail if you don't have the patch for the privacy > preferences applied :)
Assignee: nobody → joejoevictor
Joe: Did you sort out the issues with running the tests? Do you have an updated patch? (Sorry, I'm a bit behind on the status of things after being off sick.)
Attachment #605198 - Flags: review?(bmcbride)
Summary: Testing for In-content Preferences → Copy and port current preference tests to run against the in-content preferences
Joe: Have you made any progress on this?
Attached patch individual tests (obsolete) (deleted) — Splinter Review
attempt at getting all the old tests to pass, most seem to be passing but having a few give one or two tests failing
Attached patch in-content tests (obsolete) (deleted) — Splinter Review
Attachment #611680 - Attachment is obsolete: true
Comment on attachment 612562 [details] [diff] [review] in-content tests Review of attachment 612562 [details] [diff] [review]: ----------------------------------------------------------------- Jon, can you push all your patches and this patch to tryserver to see if these tests pass on all platforms? ::: browser/components/preferences/in-content/preferences.js @@ +11,5 @@ > Components.utils.import("resource://gre/modules/XPCOMUtils.jsm"); > > function init_all() { > + var initFinished = document.createEvent("Event"); > + initFinished.initEvent("Initialized", true, true); Move these two lines to the end of init_all, and use |let| instead of |var|. ::: browser/components/preferences/in-content/privacy.js @@ +163,4 @@ > this.dependentControls > .forEach(function (aElement) > document.getElementById(aElement).disabled = disabled); > + Extraneous whitespace got added here. ::: browser/components/preferences/in-content/tests/browser_bug705422.js @@ +36,5 @@ > + cookieSvc.setCookieString(cookieUri, null, name+"="+value+";", null); > + } > + > + // open cookie manager > + var cmd = window.openDialog("chrome://browser/content/preferences/in-content/cookies.xul", Does cookies.xul need to be copied to the in-content folder? Or can we just use the current cookies.xul? ::: browser/components/preferences/in-content/tests/browser_bug731866.js @@ +16,5 @@ > + Services.obs.addObserver(observer, "browser-preferences-initialized", false); > + > + // open about:preferences > + gBrowser.removeCurrentTab(); > + gBrowser.selectedTab = gBrowser.addTab("chrome://browser/components/preferences/in-content/preferences.xul"); This should use ABOUT_PREFERENCES_SPEC instead of the full chrome URL. ::: browser/components/preferences/in-content/tests/privacypane_tests.js @@ +11,5 @@ > + browser.addEventListener("Initialized", function(aEvent) { > + dump("\nInitialized\n"); > + browser.removeEventListener("Initialized", arguments.callee, true); > + setTimeout(function() { > + // is(Services.prefs.getBoolPref("browser.preferences.inContent"), true, "In-content prefs are enabled"); These comments should be removed, @@ +13,5 @@ > + browser.removeEventListener("Initialized", arguments.callee, true); > + setTimeout(function() { > + // is(Services.prefs.getBoolPref("browser.preferences.inContent"), true, "In-content prefs are enabled"); > + is(browser.contentWindow.location.href, "about:preferences", "Checking if the preferences tab was opened"); > + dump("\n here1 \n"); as well as these dump statements, @@ +36,5 @@ > +// testFunc(win.document.defaultView); > +// gBrowser.removeCurrentTab(); > +// dump("\n here2 \n"); > +// testRunner.runNext(); > +// }, false); and these comments too :) @@ +485,5 @@ > +} > + > +let testRunner; > +function run_test_subset(subset) { > + let instantApplyOrig = Services.prefs.getBoolPref("browser.preferences.instantApply"); We don't need to store the original pref value, see below. @@ +496,5 @@ > + counter: 0, > + runNext: function() { > + if (this.counter == this.tests.length) { > + // cleanup > + Services.prefs.setBoolPref("browser.preferences.instantApply", instantApplyOrig); Services.prefs.clearUserPref("browser.preferences.instantApply"); and use registerCleanupFunction at the beginning of test() to reset this pref.
Attachment #612562 - Flags: feedback+
Attached patch copied over tests + browser_bug731866.js patch (obsolete) (deleted) — Splinter Review
permissions and cookies tests were removed and browser_bug731866.js for testing pane switching was added. All pass as well : )
Attachment #612562 - Attachment is obsolete: true
Attachment #613790 - Flags: feedback?(jwein)
Attachment #613790 - Flags: feedback?(bmcbride)
Comment on attachment 613790 [details] [diff] [review] copied over tests + browser_bug731866.js patch Review of attachment 613790 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/preferences/in-content/preferences.js @@ +13,5 @@ > Components.utils.import("resource://services-sync/service.js"); > > function init_all() { > + var initFinished = document.createEvent("Event"); > + initFinished.initEvent("Initialized", true, true); Please move these two lines to be right before the event is dispatched. ::: browser/components/preferences/in-content/tests/privacypane_tests.js @@ +7,5 @@ > + gBrowser.tabContainer.removeEventListener("TabOpen", arguments.callee, true); > + let browser = aEvent.originalTarget.linkedBrowser; > + browser.addEventListener("Initialized", function(aEvent) { > + browser.removeEventListener("Initialized", arguments.callee, true); > +// is(Services.prefs.getBoolPref("browser.preferences.inContent"), true, "In-content prefs are enabled"); Hmm, was this meant to be commented out? Is this check failing for some reason?
Attachment #613790 - Flags: feedback?(jwein) → feedback+
Attached patch patch for testing (obsolete) (deleted) — Splinter Review
Attached patch patch for tests (obsolete) (deleted) — Splinter Review
Attachment #613790 - Attachment is obsolete: true
Attachment #616401 - Attachment is obsolete: true
Attachment #617001 - Flags: feedback?(bmcbride)
Attachment #613790 - Flags: feedback?(bmcbride)
Attachment #605198 - Attachment is obsolete: true
Attached patch patch for tests (deleted) — Splinter Review
Attachment #617001 - Attachment is obsolete: true
Attachment #617048 - Flags: feedback?(bmcbride)
Attachment #617001 - Flags: feedback?(bmcbride)
Attachment #617048 - Flags: feedback?(bmcbride)
\o/ Pushed to mozilla-inbound. These patches should be merged to mozilla-central within the next day or two. Thanks! https://hg.mozilla.org/integration/mozilla-inbound/rev/8b195889f55c
Flags: in-testsuite+
Target Milestone: --- → Firefox 15
Status: ASSIGNED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Depends on: 820815
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: