Closed
Bug 585769
Opened 14 years ago
Closed 14 years ago
Make Mozmill-test testBackForwardButtons local
Categories
(Mozilla QA Graveyard :: Mozmill Tests, defect)
Mozilla QA Graveyard
Mozmill Tests
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: u279076, Assigned: aaronmt)
References
Details
(Whiteboard: [litmus-data])
Attachments
(5 files)
7.25 KB,
patch
|
u279076
:
review+
whimboo
:
review-
|
Details | Diff | Splinter Review |
7.33 KB,
patch
|
u279076
:
review-
|
Details | Diff | Splinter Review |
7.33 KB,
patch
|
u279076
:
review+
|
Details | Diff | Splinter Review |
7.35 KB,
patch
|
u279076
:
review+
|
Details | Diff | Splinter Review |
7.35 KB,
patch
|
Details | Diff | Splinter Review |
Module: testGeneral/testBackForwardButtons.js Test-page: Use any 3 pages from test-files/layout/mozilla* NOTE: Please move test module from testGeneral to testToolbar
Assignee | ||
Comment 1•14 years ago
|
||
test move to firefox/testToolbar (as requested in comment #0) + cleanup + converting test to local-data
Assignee: nobody → aaron.train
Status: NEW → ASSIGNED
Attachment #466709 -
Flags: review?(anthony.s.hughes)
Comment on attachment 466709 [details] [diff] [review] Patch v1 - (default) Looks good, r+. Over to Henrik for additional review.
Attachment #466709 -
Flags: review?(hskupin)
Attachment #466709 -
Flags: review?(anthony.s.hughes)
Attachment #466709 -
Flags: review+
Comment 3•14 years ago
|
||
Comment on attachment 466709 [details] [diff] [review] Patch v1 - (default) First, please avoid moving of files and changes at the same time. It makes the review too complex. Instead handle those kind of tasks separately. >+const LOCAL_TEST_FOLDER = collector.addHttpResource('../test-files/'); >+const LOCAL_TEST_PAGES = [ >+ {url: LOCAL_TEST_FOLDER + 'layout/mozilla.html', id: 'community'}, >+ {url: LOCAL_TEST_FOLDER + 'layout/mozilla_mission.html', id: 'mission_statement'} >+]; I would like to see three pages here as what we had in the original code. That way we can add more tests to this function to check the enabled/disabled states for both buttons. Would be good to have in any way. >+var testBackandForward = function() >+{ nit: Please fix the bracing.
Attachment #466709 -
Flags: review?(hskupin) → review-
Assignee | ||
Comment 4•14 years ago
|
||
Thanks for catching the two as opposed to three test files! Fixes as per comment #3
Attachment #471573 -
Flags: review?(anthony.s.hughes)
Comment on attachment 471573 [details] [diff] [review] Patch v1.1 - (default) 2 things: 1. Please rename testBackandForward to testBackAndForward() 2. As Henrik already asked, please do the move and the revisions in separate patches Thanks!
Attachment #471573 -
Flags: review?(anthony.s.hughes) → review-
Assignee | ||
Comment 6•14 years ago
|
||
1. Ok. 2. Since he already reviewed it, there shouldn't be a need. I'll do so accordingly in the future.
(In reply to comment #6) > 1. Ok. > 2. Since he already reviewed it, there shouldn't be a need. I'll do so > accordingly in the future. Henrik, do you want this broken into 2 patches or is it ok in a single patch this time?
Comment 8•14 years ago
|
||
It's fine for this time. Just keep an eye on it in the future.
Assignee | ||
Comment 9•14 years ago
|
||
Renamed testBackandForward() to testBackAndForward()
Attachment #471620 -
Flags: review?(anthony.s.hughes)
Reporter | ||
Comment 10•14 years ago
|
||
Comment on attachment 471620 [details] [diff] [review] Patch v1.2 - (default) r+, assuming you don't need to re-review this Henrik.
Attachment #471620 -
Flags: review?(anthony.s.hughes) → review+
Keywords: checkin-needed
Reporter | ||
Comment 11•14 years ago
|
||
(In reply to comment #10) > Comment on attachment 471620 [details] [diff] [review] > Patch v1.2 - (default) > > r+, assuming you don't need to re-review this Henrik. Landed on default: http://hg.mozilla.org/qa/mozmill-tests/rev/1ed816089b1e Please followup with 1.9.2 and 1.9.1 patches as appropriate.
Assignee | ||
Comment 12•14 years ago
|
||
Attachment #473620 -
Flags: review?(anthony.s.hughes)
Attachment #473620 -
Flags: review?(anthony.s.hughes) → review+
Reporter | ||
Comment 13•14 years ago
|
||
(In reply to comment #12) > Created attachment 473620 [details] [diff] [review] > Patch v1.2 - (1.9.2) Landed on mozilla1.9.2: http://hg.mozilla.org/qa/mozmill-tests/rev/8e80d946710b
Keywords: checkin-needed
Assignee | ||
Comment 14•14 years ago
|
||
Keywords: checkin-needed
Reporter | ||
Comment 15•14 years ago
|
||
(In reply to comment #14) > Created attachment 474210 [details] [diff] [review] > Patch v1.2 - (1.9.1) Landed: http://hg.mozilla.org/qa/mozmill-tests/rev/81cb146ae2f3 [mozilla1.9.1]
Comment 16•14 years ago
|
||
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
Updated•5 years ago
|
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•