544980 - [mozmill] Create a Mozmill script for "Undo Close Window" test case

Status: RESOLVED WONTFIX

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[u279076]

This is a tracking bug for creating a Mozmill test script for the following Mozmill test script:

[sessionstore] Undo Close Window
https://litmus.mozilla.org/show_test.cgi?id=7794 [Firefox 3.5]
https://litmus.mozilla.org/show_test.cgi?id=9360 [Firefox 3.6]

Comment 1

[Aaron Train [:aaronmt]]

Attachment: patch v1

This unit test covers the litmus tests covered in comment #0. Ideally, I would like to add another assert that checks the dynamically generated menupopup, i.e, (historyUndoWindowPopup) see [1] for appropriate population and correct tab display. I am not sure if this is possible in mozmill currently as through an inspection I see a large messy xpath string. 

[1] http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-menubar.inc#514

Comment 2

[Aaron Train [:aaronmt]]

Attachment: v1.1 patch

Had a debugging line in there by accident

Comment 3

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Comment on attachment 450571 [details] [diff] [review]
v1.1 patch

> /**
>+ * Resets the list of recently closed windows by setting and clearing the user preference
>+ */
>+function resetRecentlyClosedWindows()
>+{
>+  var prefs = collector.getModule('PrefsAPI').preferences;
>+  
>+  prefs.setPref(SESSIONSTORE_MAXWINDOWS_PREF, 0);
>+  prefs.clearUserPref(SESSIONSTORE_MAXWINDOWS_PREF);
>+}

FWIW, this might not work exactly as expected if there are popup windows involved... see bug 491590

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 450571 [details] [diff] [review]
v1.1 patch

Anthony is the one who own that subgroup for Mozmill test creation. Lets move the review request to him first.

Comment 5

[u279076]

Comment on attachment 450571 [details] [diff] [review]
v1.1 patch

r+ with two nits:

>+const gTimeout = 5000;
>+const localTestFolder = collector.addHttpResource('../test-files');
Please use '../test-files/'

>+
>+  for (var i = 0; i < 3; i++) {
>+   controller2.open(localTestFolder + '/tabbedbrowsing/openinnewtab_target.html?id=' + i);
Please use 'tabbedbrowsing/openinnewtab_target.html?id='

Failure to do so will result in a // in the locationbar.

Comment 6

[Aaron Train [:aaronmt]]

Attachment: v1.2 patch

Thanks for the review Anthony. 

v1.2 
- Nits fixed in comment #5
- Added a missing check that I caught, that wasn't caught in your review in line #125

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 451181 [details] [diff] [review]
v1.2 patch

>+// Holds the instance of the newly created window
>+var newWindow = null;

Today we have landed the window helper function on bug 518386. Please check the new window test under tabbed browsing how to handle that function. It should be similar to what we have in this test.

>+var teardownModule = function(module) {
>+  SessionStoreAPI.resetRecentlyClosedWindows();
>+  
>+  if (newWindow != controller.window) {
>+    newWindow.close();
>+  }

Same applies to the teardownModule function.

>+var testUndoCloseWindow = function() {

Please add a comment for the test function.

>+  controller.sleep(1000);
>+
>+  // Check if a new window has opened
>+  newWindow = mozmill.wm.getMostRecentWindow("navigator:browser");
>+  var controller2 = new mozmill.controller.MozMillController(newWindow);
>+  controller2.waitForPageLoad();
>+
>+  controller.waitForEval("subject.getWindows().length == " + (windowCount + 1), 
>+                         gTimeout, 100, mozmill.utils);
>+

All that can be replaced now with a single line of code.

>+  // Open 3 tabs with pages in the local test folder in the new window
>+  tabBrowser = new TabbedBrowsingAPI.tabBrowser(controller2);
>+  tabBrowser.closeAllTabs();

A new window should only have one tab open per default. There is no need to close all tabs before.

>+  // Check tab count of new window, should be 4 (3 + 1 blank)
>+  controller.waitForEval('subject.length == 4', gTimeout, 100, tabBrowser);

Please pass tabBrowser as object into waitForEval.

>+  // Close the recently opened window
>+  newWindow.close();

You should use a keyboard event here.

>+  // Check that 'Recently Closed Windows' is enabled
>+  controller.assertPropertyNotExist(new elementslib.ID(controller.window.document, 
>+                                   'historyUndoWindowMenu', 'disabled'));

Without running the test, do we update the status of the menu item directly? Means you do not have to click the history menu item first? I thought the states get updated with the onpopupshown event.

>+  // Check the window count
>+  controller.waitForEval("subject.getWindows().length == " + windowCount,
>+                         gTimeout, 100, mozmill.utils);

Shouldn't this be placed before checking the menu above?

>+  // Restore recently closed window
>+  SessionStoreAPI.undoClosedWindow(controller, {type: 'shortcut'});



>+  controller.sleep(1000);
>+
>+  // Check if a new window has opened
>+  newWindow = mozmill.wm.getMostRecentWindow("navigator:browser");
>+  controller2 = new mozmill.controller.MozMillController(newWindow);
>+  controller2.waitForPageLoad();

This can also be replaced now.

>+  // Check that the original 4 (3 + 1 blank) tabs are restored
>+  tabBrowser = new TabbedBrowsingAPI.tabBrowser(controller2);
>+  controller.waitForEval('subject.length == 4', gTimeout, 100, tabBrowser);

Please pass in as object.

>+  // Check that 'Recently Closed Windows' is disabled
>+  controller.assertProperty(new elementslib.ID(controller.window.document, 
>+                           'historyUndoWindowMenu'), 'disabled', true);
>+
>+  // Check that 'Recently Closed Windows' count is 0
>+  controller.assertJS("subject.closedWindowCount == 0",
>+                     {closedWindowCount: SessionStoreAPI.getClosedWindowCount(controller)});

Flipping both checks?


Also please update undoClosedWindow to use our internal getClosedWindowCount function now. And the assertJS call will also have to be better replaced with waitForEval.

Comment 8

[Aaron Train [:aaronmt]]

With the new handleWindow function, I am attempting something like 

controller.click(new elementslib.Elem(controller.menus['file-menu'].menu_newNavigator));

controller2 = UtilsAPI.handleWindow("type", "navigator:browser");
tabBrowser = new TabbedBrowsingAPI.tabBrowser(controller2);

But controller2 ends up being null and I cant get a tabBrowser. Thus, no assignment from the handleWindow function. Any ideas?

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #8)
> controller2 = UtilsAPI.handleWindow("type", "navigator:browser");
> tabBrowser = new TabbedBrowsingAPI.tabBrowser(controller2);
> 
> But controller2 ends up being null and I cant get a tabBrowser. Thus, no
> assignment from the handleWindow function. Any ideas?

Create a callback same as for modal windows and pass it in as a 3rd parameter to the handleWindow function. See the example I have pointed out:

http://hg.mozilla.org/qa/mozmill-tests/file/tip/firefox/testTabbedBrowsing/testNewWindow.js

Comment 10

[Aaron Train [:aaronmt]]

Trying to restore a recent window

SessionStoreAPI.undoClosedWindow(controller, {type: 'shortcut'});

ERROR:mozmill:Test Failure: {u'exception': {u'message': u'Synthesizing key event failed. \n[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMWindowUtils.sendKeyEvent]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)

Any ideas?

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Is controller.window a valid object?

Comment 12

[Aaron Train [:aaronmt]]

Attachment: Patch v1.3

v1.3, 
- removed the checks for the menu popup as we can't test those elements
+ changes from previous review as well as recent template changes

Comment 13

[u279076]

Comment on attachment 452071 [details] [diff] [review]
Patch v1.3

r+ from me.  Over to you Henrik.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Before I want to review the test we should have support for dynamic menu entries. This work is done on bug 474486. The reason is that the Litmus tests mention checks for menu entries and we don't have those in our test right now.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 452071 [details] [diff] [review]
Patch v1.3

The patch will need an update too by that time.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Mass move of Mozmill Test related project bugs to newly created components. You can filter out those emails by using "Mozmill-Tests-to-MozillaQA" as criteria.

Comment 17

[Sylvestre Ledru [:Sylvestre]]

Mozmill is dead, WONTFIX the remaining bugs.