Closed
Bug 436553
Opened 13 years ago
Closed 13 years ago
Automate litmus test for closing the download manager.
Categories
(Toolkit :: Downloads API, defect)
Toolkit
Downloads API
Tracking
()
RESOLVED
FIXED
mozilla1.9.1b1
People
(Reporter: sdwilsh, Assigned: poonaatsoc)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 10 obsolete files)
5.20 KB,
patch
|
sdwilsh
:
review+
|
Details | Diff | Splinter Review |
This should be a chrome test that lives in toolkit/mozapps/downloads/tests/chrome/ to replace this litmus test: https://litmus.mozilla.org/show_test.cgi?id=3979
Assignee | ||
Comment 1•13 years ago
|
||
I have the Makefile.in in the patch too. Do have a look at the Makefile.in. The issue is, I think a Makefile.in has already been written for this directory by edward. A file that you supplied yesterday, was actually a chrome test for dm, written by edward. That file isn't in my src, maybe because he wrote it recently. If that is the case I can integrate my Makefile.in with the already existant Makefile.in once I update the trunk on my machine
Attachment #327071 -
Flags: review?(sdwilsh)
Reporter | ||
Comment 2•13 years ago
|
||
Comment on attachment 327071 [details] [diff] [review] v1.0 Please update this patch to latest in mozilla-central here are some comments >+/* nit: extra * here please >+ * Litmus Testcase ID #3979 - Various methods to close the Downloads dialog >+ * Verify the various methods to close the download manager >+ * - Press "Esc" >+ * - Ctrl+W (Mac: Cmd+W) >+ * - Click "X" close window button in the titlebar.<-- Can't be implemented since "X" >+ * button is a part of the native widget. >+ * Each of the actions should close the Downloads dialog if it is open and in focus. I'm actually not interested in the litmus testcase id, because the tests are either going to be disabled or modified to cover what we couldn't automate. With that said, please only include what we are actually testing in this block. >+<window title="Download Manager Test" >+ onload="setTimeout(runTest, 0);" >+ xmlns:html="http://www.w3.org/1999/xhtml" >+ xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"> nit: attributes should line up >+ <title>Panel Focus Tests</title> huh? >+ <script> script needs the type set nit: let's not indent all this JS. It doesn't really buy us anything >+ function testCloseDMEscKey() >+ { >+ let dm_win = wm.getMostRecentWindow("Download:Manager"); >+ let [key, event] = getCloseKeys(); >+ >+ EventUtils.synthesizeKey(key, event, dm_win); >+ ok(!dmui.visible, "DMUI fails to close with Esc"); >+ return; just use the ESC key >+ function testCloseDMMetaKey() nit: with Accel-key? >+ function getCloseKeys() >+ { >+ switch (testPhase - 1) { >+ case 0 : >+ return ["VK_ESC", { }]; >+ break; >+ case 1 : >+ if (navigator.platform.search("Mac") == 0) return ["w", { metaKey: true }]; >+ else return ["w", { ctrlKey: true }]; >+ break; >+ } >+ } The whole global state thing is just confusing. Just use this function for the close key.
Attachment #327071 -
Flags: review?(sdwilsh)
Assignee | ||
Comment 3•13 years ago
|
||
Attachment #327071 -
Attachment is obsolete: true
Attachment #329252 -
Flags: review?(sdwilsh)
Assignee | ||
Comment 4•13 years ago
|
||
Attachment #329252 -
Attachment is obsolete: true
Attachment #329296 -
Flags: review?(sdwilsh)
Attachment #329252 -
Flags: review?(sdwilsh)
Assignee | ||
Comment 5•13 years ago
|
||
Attachment #329296 -
Attachment is obsolete: true
Attachment #329320 -
Flags: review?(sdwilsh)
Attachment #329296 -
Flags: review?(sdwilsh)
Assignee | ||
Comment 6•13 years ago
|
||
Attachment #329320 -
Attachment is obsolete: true
Attachment #329325 -
Flags: review?(sdwilsh)
Attachment #329320 -
Flags: review?(sdwilsh)
Assignee | ||
Comment 7•13 years ago
|
||
Attachment #329325 -
Attachment is obsolete: true
Attachment #329441 -
Flags: review?(sdwilsh)
Attachment #329325 -
Flags: review?(sdwilsh)
Assignee | ||
Comment 8•13 years ago
|
||
Have run the test. It works. I checked if dmui.visible before I close it and I get true.
Attachment #329441 -
Attachment is obsolete: true
Attachment #329694 -
Flags: review?(sdwilsh)
Attachment #329441 -
Flags: review?(sdwilsh)
Reporter | ||
Comment 9•13 years ago
|
||
Comment on attachment 329694 [details] [diff] [review] v7.0 >+++ b/toolkit/mozapps/downloads/tests/chrome/Makefile.in >@@ -61,6 +61,7 @@ _CHROME_FILES = \ > test_select_all.xul \ > test_space_key_pauses_resumes.xul \ > test_ui_stays_open_on_alert_clickback.xul \ >+ test_close_download_manager.xul \ > test_bug_429247.xul \ > utils.js \ > $(NULL) Keep this list alphabetized please >+++ b/toolkit/mozapps/downloads/tests/chrome/test_close_download_manager.xul >+/** >+ * This tests bug 436553. This test basically checks the various methods to >+ * close the download manager.- the ESC key and and the accel-key. >+ */ Doesn't quite seem right... >+// Variable to indicate the test that is running. Make it global since we >+// need it in more than 2 functions don't worry about the comment - not needed >+var testPhase = 0; s/var/let/ >+function getCloseKeys() >+{ >+ if (navigator.platform.search("Mac") == 0) >+ return ["w", { metaKey: true }]; >+ else >+ return ["w", { ctrlKey: true }]; >+} Don't need this >+function testCloseDMWithEscKey(aWin) >+{ >+ function dmWindowClosedListener() { >+ ok(!dmui.visible, "DMUI closes with ESC key"); >+ dmui.show(); >+ } >+ aWin.addEventListener("unload", dmWindowClosedListener, false); I think you need to remove the event listener in your function >+function testCloseDMWithAccelKey(aWin) >+{ >+ let [key, event] = getCloseKeys(); >+ >+ function dmWindowClosedListener() { >+ ok(!dmui.visible, "DMUI closes with [ctrl/meta]-w"); just say accel >+ synthesizeKey(key, event, aWin); just do synthesizeKey("w", {accelKey:true}, aWin); >+ let testObs = { >+ observe: function(aSubject, aTopic, aData) { >+ let win = aSubject.QueryInterface(Ci.nsIDOMWindow); >+ if(testPhase == 0) { >+ testPhase++; >+ testCloseDMWithEscKey(win); >+ } >+ else { >+ obs.removeObserver(testObs, DLMGR_UI_DONE); >+ testCloseDMWithAccelKey(win); use a switch please >+ <body xmlns="http://www.w3.org/1999/xhtml"> >+ <p id="display"> >+ </p> >+ <div id="content" style="display: none"> >+ </div> >+ <pre id="test"> >+ </pre> >+ </body> Don't change all this formatting - keep it how all the other test files are please
Attachment #329694 -
Flags: review?(sdwilsh) → review-
Assignee | ||
Comment 10•13 years ago
|
||
Patch updated with the corrections requested. Tested - Passes
Attachment #329694 -
Attachment is obsolete: true
Attachment #329994 -
Flags: review?(sdwilsh)
Assignee | ||
Comment 11•13 years ago
|
||
Solved the issue I have with utils.js :) I have updated the patch now to use the functions from utils.js Test runs
Attachment #329994 -
Attachment is obsolete: true
Attachment #330341 -
Flags: review?(sdwilsh)
Attachment #329994 -
Flags: review?(sdwilsh)
Assignee | ||
Comment 12•13 years ago
|
||
Test updated to accomodate the new utils.js. The test passes.
Attachment #330341 -
Attachment is obsolete: true
Attachment #331506 -
Flags: review?(sdwilsh)
Attachment #330341 -
Flags: review?(sdwilsh)
Updated•13 years ago
|
Product: Firefox → Toolkit
Reporter | ||
Comment 13•13 years ago
|
||
Comment on attachment 331506 [details] [diff] [review] v10.0 >+/** >+ * This tests bug 436553. This test basically checks if the download manager >+ * closes when you press the esy key and accelKey-w. >+ */ lose the bug number (per previous comments) s/esy/esc/ just say accel + w >+function testCloseDMWithAccelKey(aWin) >+{ >+ function dmWindowClosedListener() { >+ aWin.removeEventListener("unload", dmWindowClosedListener, false); >+ ok(!dmui.visible, "DMUI closes with [ctrl/meta]-w"); accel + w instead of the ctrl... r=sdwilsh with those changes
Attachment #331506 -
Flags: review?(sdwilsh) → review+
Assignee | ||
Comment 14•13 years ago
|
||
Test updated to accommodate the reviews :)
Attachment #331506 -
Attachment is obsolete: true
Attachment #334282 -
Flags: review?(sdwilsh)
Reporter | ||
Comment 15•13 years ago
|
||
Comment on attachment 334282 [details] [diff] [review] v11.0 [Checkin: Comment 17] r=sdwilsh
Attachment #334282 -
Flags: review?(sdwilsh) → review+
Reporter | ||
Comment 16•13 years ago
|
||
The litmus test for this can be disabled once this lands.
Flags: in-litmus?
Keywords: checkin-needed
Comment 17•13 years ago
|
||
Comment on attachment 334282 [details] [diff] [review] v11.0 [Checkin: Comment 17] I fixed [ patching file toolkit/mozapps/downloads/tests/chrome/Makefile.in Hunk #1 FAILED at 48 1 out of 1 hunk FAILED -- saving rejects to file [...] ] http://hg.mozilla.org/mozilla-central/rev/c215871b5f6d
Attachment #334282 -
Attachment description: v11.0 → v11.0
[Checkin: Comment 17]
Updated•13 years ago
|
Updated•13 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [ToDo: comment 16]
(In reply to comment #16) > The litmus test for this can be disabled once this lands. Can I disable it, even with the failing hunk? The HG log looks like it all landed...
Reporter | ||
Comment 19•13 years ago
|
||
Yeah - it was just conflicting stuff in the Makefile that Serge fixed at checkin.
https://litmus.mozilla.org/show_test.cgi?id=3979 is around, but disabled; thanks! These tests rock -- not sure how to verify, or I can, yet...
Flags: in-litmus? → in-litmus-
You need to log in
before you can comment on or make changes to this bug.
Description
•