Closed Bug 585775 Opened 14 years ago Closed 14 years ago

Make Mozmill-test testSetToCurrentPage local

Categories

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Product:
Mozilla QA Graveyard
Component:
Mozmill Tests
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: u279076, Assigned: aaronmt)

References

Details

(Whiteboard: [litmus-data])

Attachments

(5 files, 2 obsolete files)

Patch v2 - (default)
(deleted), patch
u279076
: review+
whimboo
: review-
Details | Diff | Splinter Review
Patch v3 - (default)
(deleted), patch
whimboo
: review+
Details | Diff | Splinter Review
Patch v3 - (default) + removed 'module'
(deleted), patch
Details | Diff | Splinter Review
Patch v4 - (1.9.1) [removed url member]
(deleted), patch
u279076
: review+
whimboo
: review+
Details | Diff | Splinter Review
Patch v4 - (default) [removed url member]
(deleted), patch
u279076
: review+
whimboo
: review+
Details | Diff | Splinter Review
Module: testPreferences/testSetToCurrentPage.js Test-page: Use any 2 pages from test-files/layout/mozilla*
Blocks: 579965
Assignee: nobody → aaron.train
Status: NEW → ASSIGNED
Attached patch Patch v1 - (default) (obsolete) (deleted) — — Splinter Review
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.
Attachment #465122 - Flags: review?(anthony.s.hughes)
(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.
Depends on: 586640
I'll get a patch up with the original line shortly now that it should work.
Attachment #465122 - Attachment is obsolete: true
Attachment #465122 - Flags: review?(anthony.s.hughes)
Attached patch Patch v2 - (default) (deleted) — — Splinter Review
Attachment #465374 - Flags: review?(anthony.s.hughes)
Attachment #465374 - Flags: review?(hskupin)
Attachment #465374 - Flags: review?(anthony.s.hughes)
Attachment #465374 - Flags: review+
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.
Attachment #465374 - Flags: review?(hskupin) → review-
Attached patch Patch v3 - (default) (deleted) — — Splinter Review
(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.
Attachment #465654 - Flags: review?(hskupin)
Attachment #465654 - Attachment is patch: true
Attachment #465654 - Attachment mime type: application/octet-stream → text/plain
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
Attachment #465654 - Flags: review?(hskupin) → review+
Done.
> 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.
Attached patch Patch v3 - (1.9.1) (obsolete) (deleted) — — Splinter Review
1.9.1
Attachment #466303 - Flags: review?(anthony.s.hughes)
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
Attachment #466303 - Flags: review?(anthony.s.hughes) → review-
(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.
Attachment #466303 - Attachment is obsolete: true
Attachment #469471 - Flags: review?(anthony.s.hughes)
Attachment #469473 - Flags: review?(anthony.s.hughes)
Attachment #469471 - Attachment description: Patch v4 - (1.9.1) → Patch v4 - (1.9.1) [removed url member]
Attachment #469471 - Flags: review?(hskupin)
Attachment #469471 - Flags: review?(anthony.s.hughes)
Attachment #469471 - Flags: review+
Attachment #469473 - Flags: review?(hskupin)
Attachment #469473 - Flags: review?(anthony.s.hughes)
Attachment #469473 - Flags: review+
(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 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.
Attachment #469473 - Flags: review?(hskupin) → review+
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.
Attachment #469471 - Flags: review?(hskupin) → review+
Keywords: checkin-needed
(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]
(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]
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
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.
Product: Testing → Mozilla QA
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: