585778 - Make Mozmill-test testCloseWindow local

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[u279076]

Module: testPrivateBrowsing/testCloseWindow.js
Test-page: Use any 2 pages from test-files/layout/mozilla*

Comment 1

[Aaron Train [:aaronmt]]

Attachment: Patch v1 - (default)

Comment 2

[u279076]

Comment on attachment 464981 [details] [diff] [review]
Patch v1 - (default)

>-var websites = [
>-                {url: 'https://addons.mozilla.org/', id: 'search-query'},
>-                {url: 'https://bugzilla.mozilla.org', id: 'quicksearch_top'}
>-               ];
>+const LOCAL_TEST_FOLDER = collector.addHttpResource('../test-files/');
>+const LOCAL_PAGES = [
>+                     {url: LOCAL_TEST_FOLDER + 'layout/mozilla.html', 
>+                      name: 'community', index: 0},
>+                     {url: LOCAL_TEST_FOLDER + 'layout/mozilla_mission.html', 
>+                      name: 'mission', index: 1}
>+                    ];
> 

1. Please rename LOCAL_PAGES to LOCAL_TEST_PAGES to keep it the same as the rest of our tests
2. I don't see "index" as particularly useful.  Just use i in the for-loops below.

>+  // Wait until all tabs have finished loading
>+  for each(var localPage in LOCAL_PAGES) {
>+    var elem = new elementslib.Name(controller.tabs.getTab(localPage.index), localPage.name);
>+    controller.waitForElement(elem);
>   }

Convert this to a for-loop with iterator i. 

>+  for each(var page in LOCAL_PAGES) {
>+   var elem = new elementslib.Name(controller.tabs.getTab(localPage.index), localPage.name);
>+   controller.waitForPageLoad(page);
>+   controller.waitForElement(elem);
>   }
> }

Same here.

Comment 3

[Aaron Train [:aaronmt]]

Attachment: Patch v2 - (default)

Comment 4

[u279076]

Comment on attachment 464995 [details] [diff] [review]
Patch v2 - (default)

>+  
>+  for each(var localPage in LOCAL_TEST_PAGES) {
>+    controller.open(localPage.url);
>     controller.click(newTab);
>   }
> 

nit: simply use page instead of localPage
This puts the test inline with what I have been using.

Comment 5

[Aaron Train [:aaronmt]]

Attachment: Patch v3 - (default)

I think we should add a reply  in the meta bug that lists some 'to-be-expected' copy-cat expectations for each test.

Comment 6

[u279076]

Comment on attachment 465008 [details] [diff] [review]
Patch v3 - (default)

Looks good, r+, over to Henrik for final review.

Comment 7

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

Comment on attachment 465008 [details] [diff] [review]
Patch v3 - (default)

>+const LOCAL_TEST_PAGES = [
>+                          {url: LOCAL_TEST_FOLDER + 'layout/mozilla.html', name: 'community'},
>+                          {url: LOCAL_TEST_FOLDER + 'layout/mozilla_mission.html', name: 'mission'}
>+                         ];

2 spaces as indentation for arrays please.

>+  for each(var page in LOCAL_TEST_PAGES) {

nit: Please add a space behind 'each'

>+  for(var i = 0; i < LOCAL_TEST_PAGES.length; i++) {

nit: and here after 'for'.

>+  for(var i = 0; i < LOCAL_TEST_PAGES.length; i++) {

again.

r=me with those comments addressed.

Comment 8

[Aaron Train [:aaronmt]]

Attachment: Patch v4 - (default)

Comment 9

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

Comment on attachment 465029 [details] [diff] [review]
Patch v4 - (default)

>+  for each(var page in LOCAL_TEST_PAGES) {

You missed that space. I'm sure Anthony can correct it before the checkin.

Comment 10

[u279076]

Landed on default:
http://hg.mozilla.org/qa/mozmill-tests/rev/38462c38072c

Please submit follow up patches for 1.9.1 and 1.9.2

Comment 11

[Aaron Train [:aaronmt]]

Attachment: Patch v4 - (1.9.2)

Comment 12

[Aaron Train [:aaronmt]]

Attachment: Patch v4 - (1.9.1)

Comment 13

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

Comment on attachment 465109 [details] [diff] [review]
Patch v4 - (1.9.1)

There is no difference to the patch for 1.9.2. Let's add only one back-port patch in the future if that one is enough.

Comment 14

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

This hasn't been landed yet on older branches.

Comment 15

[u279076]

Sorry for the lateness, these fell off my radar.  The patches have now bitrotted.  Aaron, can you please update the patches?

Thanks.

Comment 16

[Aaron Train [:aaronmt]]

Attachment: Patch v4 - (1.9.2 & 1.9.1) - [bitrot fix]

Comment 17

[u279076]

(In reply to comment #16)
> Created attachment 478113 [details] [diff] [review]
> Patch v4 - (1.9.2 & 1.9.1) - [bitrot fix]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/415fcf456720 [mozilla1.9.2]
http://hg.mozilla.org/qa/mozmill-tests/rev/69ece7d4a5bc [mozilla1.9.1]

Comment 18

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

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.