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

NEW
Unassigned

Status

9 years ago
6 years ago

People

(Reporter: ashughes, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

9 years ago
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]
(Reporter)

Updated

9 years ago
Assignee: nobody → anthony.s.hughes
Status: NEW → ASSIGNED
(Reporter)

Updated

8 years ago
Depends on: 567108
(Reporter)

Updated

8 years ago
Assignee: anthony.s.hughes → nobody
Status: ASSIGNED → NEW

Updated

8 years ago
Assignee: nobody → aaron.train
Created attachment 450565 [details] [diff] [review]
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
Attachment #450565 - Flags: review?(hskupin)

Updated

8 years ago
Attachment #450565 - Attachment is obsolete: true
Attachment #450565 - Flags: review?(hskupin)
Created attachment 450571 [details] [diff] [review]
v1.1 patch

Had a debugging line in there by accident
Attachment #450571 - Flags: review?(hskupin)
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
Status: NEW → ASSIGNED
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.
Attachment #450571 - Flags: review?(hskupin) → review?(anthony.s.hughes)
(Reporter)

Comment 5

8 years ago
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.
Attachment #450571 - Flags: superreview?(hskupin)
Attachment #450571 - Flags: review?(anthony.s.hughes)
Attachment #450571 - Flags: review+
Created attachment 451181 [details] [diff] [review]
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
Attachment #450571 - Attachment is obsolete: true
Attachment #451181 - Flags: superreview?(hskupin)
Attachment #451181 - Flags: review?(anthony.s.hughes)
Attachment #450571 - Flags: superreview?(hskupin)
(Reporter)

Updated

8 years ago
Attachment #451181 - Flags: review?(anthony.s.hughes) → review+
Depends on: 518386
No longer depends on: 567108
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.
Attachment #451181 - Flags: superreview?(hskupin) → superreview-
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?
(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
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?
Is controller.window a valid object?
Created attachment 452071 [details] [diff] [review]
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
Attachment #451181 - Attachment is obsolete: true
Attachment #452071 - Flags: superreview?(hskupin)
Attachment #452071 - Flags: review?(anthony.s.hughes)
(Reporter)

Updated

8 years ago
Attachment #452071 - Flags: review?(anthony.s.hughes) → review+
(Reporter)

Comment 13

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

r+ from me.  Over to you Henrik.
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.
Depends on: 474486
Comment on attachment 452071 [details] [diff] [review]
Patch v1.3

The patch will need an update too by that time.
Attachment #452071 - Flags: superreview?(hskupin)
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.
Component: Session Restore → Mozmill Tests
Flags: superreview-
Product: Firefox → Mozilla QA
QA Contact: session.restore → mozmill-tests

Updated

6 years ago
Assignee: aaron.train → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.