Closed
Bug 810627
Opened 12 years ago
Closed 12 years ago
elementsLib's radiobutton test causes Application disconnect error
Categories
(Testing Graveyard :: Mozmill, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: xabolcs, Assigned: xabolcs)
Details
Attachments
(1 file, 2 obsolete files)
3.08 KB,
patch
|
xabolcs
:
review+
|
Details | Diff | Splinter Review |
Testing mutt/mutt/tests/js/elementslib/radio_buttons.js with
current version of mozmill master [1] running on Nightly (Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121110030714 CSet: a47525b93528) ends with:
TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed
If I close the preference window, I got:
ERROR | Test Failure | {
"exception": {
"stack": [
"createInstance@resource://mozmill/driver/mozelement.js:45",
"ID@resource://mozmill/driver/mozelement.js:57",
"test@resource://mozmill/modules/frame.js -> file:///path/to/mozmill/mutt/mutt/tests/js/elementslib/radio_buttons.js:28",
"Runner.prototype.wrapper@resource://mozmill/modules/frame.js:634",
"Runner.prototype.runTestModule@resource://mozmill/modules/frame.js:704",
"Runner.prototype.runTestFile@resource://mozmill/modules/frame.js:602",
"runTestFile@resource://mozmill/modules/frame.js:749",
"Bridge.prototype._execFunction@resource://jsbridge/modules/Bridge.jsm:140",
"Bridge.prototype.execFunction@resource://jsbridge/modules/Bridge.jsm:147",
"@resource://jsbridge/modules/Server.jsm:33",
""
],
"message": "could not find element ID: saveWhere",
"fileName": "resource://mozmill/driver/mozelement.js",
"name": "Error",
"lineNumber": 45
}
}
STR:
1. checkout mozmill/master
2. get Nightly
3. start mozmill with parameters: test=mutt/mutt/tests/js/elementslib/radio_buttons.js --binary=/path/to/Nightly
4. close preferences window optionally
[1]: https://github.com/mozilla/mozmill/tree/027aacbb70e9083949341d1927d314f11c92d606
Assignee | ||
Comment 1•12 years ago
|
||
Adding these two lines before opening prefs window will help to pass the test.
> Cu.import("resource://gre/modules/Services.jsm");
> Services.prefs.setBoolPref("browser.preferences.instantApply", true);
So yes, this is Windows specific, because "instantApply" is false by default on that platform.
And pref window opens as a modal window, and the execution will hang till that window closes.
How to fix this?
I think mozmill.getPreferencesController() shouldn't block the execution.
So it should open preferences in another "thread" - for example with a timeout callback + waitFor.
Another way is the instantApply stuff, above.
Of course there are other resolutions, surely.
Comment 2•12 years ago
|
||
We shouldn't use 'mozmill.getPreferencesController()' at all. Sadly we do not have the Modal Dialog handler available yet in Mozmill core (see bug 525187).
So I think we should create a testpage which we can use to proof the functionality.
Assignee | ||
Comment 3•12 years ago
|
||
The failing section is about testing chrome (XUL) stuff [1].
There are lot of <radiogroup> in the chrome. [2]
For example the following windows open non-modal:
- Sync setup from Tools / Set up Sync...
- Page Info from Tools / Page Info
Could I change the ids to work with one of those dialogs?
Sorry for the newbie question, but are there helper functions in Mozmill to open up a menuitem?
I hope so.
[1]: https://github.com/mozilla/mozmill/blob/931efcab0c4885f96a405b92825149390982bafa/mutt/mutt/tests/js/elementslib/radio_buttons.js#L25
[2]: https://mxr.mozilla.org/mozilla-central/search?string=radiogroup+&find=\.xul%24&findi=\.xul%24&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
Comment 4•12 years ago
|
||
(In reply to Szabolcs Hubai (:xabolcs) from comment #3)
> The failing section is about testing chrome (XUL) stuff [1].
>
> There are lot of <radiogroup> in the chrome. [2]
> For example the following windows open non-modal:
> - Sync setup from Tools / Set up Sync...
> - Page Info from Tools / Page Info
>
> Could I change the ids to work with one of those dialogs?
I still don't think we should do that. Instead we should create XUL test files which we can stick in the extension folder (see content/test/ or so) and then load in content.
> Sorry for the newbie question, but are there helper functions in Mozmill to
> open up a menuitem?
You can use controller.Menu or controller.mainMenu().
Assignee | ||
Comment 5•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #4)
> (In reply to Szabolcs Hubai (:xabolcs) from comment #3)
> > The failing section is about testing chrome (XUL) stuff [1].
> >
> > There are lot of <radiogroup> in the chrome. [2]
> > For example the following windows open non-modal:
> > - Sync setup from Tools / Set up Sync...
> > - Page Info from Tools / Page Info
> >
> > Could I change the ids to work with one of those dialogs?
>
> I still don't think we should do that. Instead we should create XUL test
> files which we can stick in the extension folder (see content/test/ or so)
> and then load in content.
>
Above the failing part in the radio_buttons.js the content is tested. [1]
The failing part tests chrome document currently, as I said.
Loading anything (even chrome://browser/content/preferences/preferences.xul) into tab will not be a content? Testing that would be needless, I think.
Are there any QueryInterface magic to make these content tabs into a chrome tab?
[1]: https://github.com/mozilla/mozmill/blob/027aacbb70e9083949341d1927d314f11c92d606/mutt/mutt/tests/js/elementslib/radio_buttons.js#L13
Comment 6•12 years ago
|
||
You can make use of XUL elements in content. See the add-on manager which lives in-content. What the test has to do is to check those elements. It's not necessary to have those in chrome scope.
Assignee | ||
Comment 7•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #6)
> You can make use of XUL elements in content. See the add-on manager which
> lives in-content. What the test has to do is to check those elements. It's
> not necessary to have those in chrome scope.
Ohh. Thanks!
By the way, I had no luck with xul elements inside a xhtml. :(
What about just loading up the general tab of preferences?
> // Test chrome (XUL)
>- prefs = mozmill.getPreferencesController();
>+ controller.open("chrome://browser/content/preferences/main.xul");
>+ controller.waitForPageLoad();
>
>- var radiogroup = findElement.ID(prefs.window.document, "saveWhere");
>+ var radiogroup = findElement.ID(controller.tabs.activeTab, "saveWhere");
> radiogroup.select(0);
> expect.equal(radiogroup.getNode().selectedIndex, 0, "First radio button has been selected.");
Not an eye-candy thing, but it works.
Comment 8•12 years ago
|
||
Please see the following test file for an example:
https://github.com/whimboo/mozmill/blob/master/mozmill/mozmill/extension/content/test/test.xul
We should add another file which contains all the supported elements which need to be tested.
Assignee | ||
Comment 9•12 years ago
|
||
This patch is the easiest fix, IMHO.
It opens the first tab of preference dialog in a content tab instead in a modal window.
The development branch is at GitHub. [1]
[1]: https://github.com/xabolcs/mozmill/compare/branch-bug-810627-radiobutton-instantapply
Assignee: nobody → xabolcs
Attachment #680857 -
Flags: feedback?(hskupin)
Assignee | ||
Comment 10•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #8)
> Please see the following test file for an example:
>
> https://github.com/whimboo/mozmill/blob/master/mozmill/mozmill/extension/
> content/test/test.xul
>
> We should add another file which contains all the supported elements which
> need to be tested.
I didn't noted Your comment. Sorry!
Thanks for the example.
Will provide another patch soon!
Assignee | ||
Comment 11•12 years ago
|
||
Attachment #680857 -
Attachment is obsolete: true
Attachment #680857 -
Flags: feedback?(hskupin)
Attachment #680883 -
Flags: review?(hskupin)
Updated•12 years ago
|
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment 12•12 years ago
|
||
Comment on attachment 680883 [details] [diff] [review]
Patch v2 - now with own radio_buttons.xul
Review of attachment 680883 [details] [diff] [review]:
-----------------------------------------------------------------
Looks great! r+ from me with the white-space issue fixed.
::: mozmill/mozmill/extension/content/test/radio_buttons.xul
@@ +10,5 @@
> +%mainDTD;
> +]>
> +
> +<window id="radio_buttons_test"
> + xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
nit: I can see trailing whitespaces at the end of those two lines.
Attachment #680883 -
Flags: review?(hskupin) → review+
Assignee | ||
Comment 13•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #12)
> Comment on attachment 680883 [details] [diff] [review]
> Patch v2 - now with own radio_buttons.xul
>
> Review of attachment 680883 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Looks great! r+ from me with the white-space issue fixed.
>
Thanks!
Whitespace fixed (https://github.com/xabolcs/mozmill/commit/a81ef0756226),
pull requested:
https://github.com/mozilla/mozmill/pull/112
Keywords: checkin-needed
Comment 14•12 years ago
|
||
So this was checked in, right? (Trying to figure out why checkin-needed was set in comment #13)
Assignee | ||
Comment 15•12 years ago
|
||
(In reply to Ryan VanderMeulen from comment #14)
> So this was checked in, right? (Trying to figure out why checkin-needed was
> set in comment #13)
It wasn't yet. A "checkin-needed"-like stuff (the pull request) was set up on the MozMill repository.
And yes, this should be check in to GitHub.
It seems it wasn't a good choice to set "checkin-needed" flag here too.
Clearing it now.
Sorry for that.
Offtopic:
There is a stabilization process in MozBaze issue tracking policy [1],
but I can't find the same directions for MozMill.
[1]: https://groups.google.com/d/topic/mozilla.tools/OtVhl8tK3NU/discussion
Keywords: checkin-needed
Comment 16•12 years ago
|
||
Please do not request a pull request if you have started to use patches on Bugzilla. We will need the final patch attached to this bug. Probably we should update the Mozmill docs to follow the mozbase policy in not using github pull requests.
Assignee | ||
Comment 17•12 years ago
|
||
I recently updated the patch, addressing the white-space nits.
Carrying forward :whimboo's r+.
By the way, I closed the requested pull #112.
Attachment #680883 -
Attachment is obsolete: true
Attachment #683075 -
Flags: review+
Comment 18•12 years ago
|
||
Thanks for the patch! I just landed it:
https://github.com/mozilla/mozmill/commit/aadced721cc91a762ee25020e2f1f95ccf01d19e
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•8 years ago
|
Product: Testing → Testing Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•