585775 - Make Mozmill-test testSetToCurrentPage local

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[u279076]

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

Comment 1

[Aaron Train [:aaronmt]]

Attachment: Patch v1 - (default)

Note:

Line 85: waitForPageLoad was replaced with waitForElement. The former was timing out on local data after we click the Home button. Waiting for an element on the local page works.

Comment 2

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

(In reply to comment #1)
> Line 85: waitForPageLoad was replaced with waitForElement. The former was
> timing out on local data after we click the Home button. Waiting for an element
> on the local page works.

Why does it timeout? Is the page loaded too quickly? If that's the case please file a new bug, because we would have to fix that in Mozmill and not in our tests. Sounds like we would need a fallback for the DOMContentLoaded event.

Comment 3

[Aaron Train [:aaronmt]]

I'll get a patch up with the original line shortly now that it should work.

Comment 4

[Aaron Train [:aaronmt]]

Attachment: Patch v2 - (default)

Comment 5

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

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

>   // Verify location bar with the saved home page
>   var locationBar = new elementslib.ID(controller.window.document, "urlbar");
>-  controller.assertValue(locationBar, homepage);
>+  controller.assertValue(locationBar, LOCAL_TEST_PAGES[0].url);
> }

Please use the locationBar class here and assert its value.

Comment 6

[Aaron Train [:aaronmt]]

Attachment: Patch v3 - (default)

(In reply to comment #5)
> Comment on attachment 465374 [details] [diff] [review]
> Patch v2 - (default)
> 
> >   // Verify location bar with the saved home page
> >   var locationBar = new elementslib.ID(controller.window.document, "urlbar");
> >-  controller.assertValue(locationBar, homepage);
> >+  controller.assertValue(locationBar, LOCAL_TEST_PAGES[0].url);
> > }
> 
> Please use the locationBar class here and assert its value.
Done.

Comment 7

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

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

> var setupModule = function(module) {
>   module.controller = mozmill.getBrowserController();
>+  module.locationBar = new ToolbarAPI.locationBar(controller);
> 
>   TabbedBrowsingAPI.closeAllTabs(controller);
> }

Last patch didn't show that up. But please remove all instances of module from that function. Also the parameter.

With this change r=me

Comment 8

[Aaron Train [:aaronmt]]

Attachment: Patch v3 - (default) + removed 'module'

Done.

Comment 9

[u279076]

> Created attachment 465736 [details] [diff] [review]
> Patch v3 - (default) + removed 'module'

http://hg.mozilla.org/qa/mozmill-tests/rev/1c11583e0dd4 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/1bf486a728b1 (mozilla1.9.2)

Does not apply cleanly to mozilla1.9.1, please provide followup patch.

Comment 10

[Aaron Train [:aaronmt]]

Attachment: Patch v3 - (1.9.1)

1.9.1

Comment 11

[u279076]

Comment on attachment 466303 [details] [diff] [review]
Patch v3 - (1.9.1)

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

I think it's probably enough to just use strings, scrap the url member.  We only need to use members when each element consists of 2 or more members (ie. url and id).  This will simplify the code later in this script to use LOCAL_TEST_PAGES[i] instead of LOCAL_TEST_PAGES[i].url

Comment 12

[u279076]

(In reply to comment #11)
> Comment on attachment 466303 [details] [diff] [review]
> Patch v3 - (1.9.1)
> 
> >+const LOCAL_TEST_PAGES = [
> >+  {url: LOCAL_TEST_FOLDER + 'layout/mozilla.html'},
> >+  {url: LOCAL_TEST_FOLDER + 'layout/mozilla_mission.html'}
> >+];
> 
> I think it's probably enough to just use strings, scrap the url member.  We
> only need to use members when each element consists of 2 or more members (ie.
> url and id).  This will simplify the code later in this script to use
> LOCAL_TEST_PAGES[i] instead of LOCAL_TEST_PAGES[i].url

Given this is a 1.9.1 patch, we should fix it on the other branches first.

Comment 13

[Aaron Train [:aaronmt]]

Attachment: Patch v4 - (1.9.1) [removed url member]

Comment 14

[Aaron Train [:aaronmt]]

Attachment: Patch v4 - (default) [removed url member]

Comment 15

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

(In reply to comment #11)
> I think it's probably enough to just use strings, scrap the url member.  We
> only need to use members when each element consists of 2 or more members (ie.
> url and id).  This will simplify the code later in this script to use
> LOCAL_TEST_PAGES[i] instead of LOCAL_TEST_PAGES[i].url

Well, personally I like it when we use the same coding style whether we have multiple items per element or not. That would make the tests better readable and extendable over time. I even would prefer to always have an array for test pages.

Comment 16

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

Comment on attachment 469473 [details] [diff] [review]
Patch v4 - (default) [removed url member]

But for now I'm fine. We can talk about it in the next days.

Comment 17

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

Comment on attachment 469471 [details] [diff] [review]
Patch v4 - (1.9.1) [removed url member]

>+var setupModule = function()
> {

Brackets clean-up for all functions please. With that r=me.

Comment 18

[u279076]

(In reply to comment #14)
> Created attachment 469473 [details] [diff] [review]
> Patch v4 - (default) [removed url member]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/9291c619b050 [default]
http://hg.mozilla.org/qa/mozmill-tests/rev/a6ae7030772b [mozilla1.9.2]

Comment 19

[u279076]

(In reply to comment #13)
> Created attachment 469471 [details] [diff] [review]
> Patch v4 - (1.9.1) [removed url member]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/6dca296e28f6 [mozilla1.9.1]

Comment 20

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

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.