585769 - Make Mozmill-test testBackForwardButtons local

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[u279076]

Module: testGeneral/testBackForwardButtons.js
Test-page: Use any 3 pages from test-files/layout/mozilla*

NOTE: Please move test module from testGeneral to testToolbar

Comment 1

[Aaron Train [:aaronmt]]

Attachment: Patch v1 - (default)

test move to firefox/testToolbar (as requested in comment #0) + cleanup + converting test to local-data

Comment 2

[u279076]

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

Looks good, r+.  Over to Henrik for additional review.

Comment 3

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

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.

Comment 4

[Aaron Train [:aaronmt]]

Attachment: Patch v1.1 - (default)

Thanks for catching the two as opposed to three test files! Fixes as per comment #3

Comment 5

[u279076]

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!

Comment 6

[Aaron Train [:aaronmt]]

1. Ok.
2. Since he already reviewed it, there shouldn't be a need. I'll do so accordingly in the future.

Comment 7

[u279076]

(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

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

It's fine for this time. Just keep an eye on it in the future.

Comment 9

[Aaron Train [:aaronmt]]

Attachment: Patch v1.2 - (default)

Renamed testBackandForward() to testBackAndForward()

Comment 10

[u279076]

Comment on attachment 471620 [details] [diff] [review]
Patch v1.2 - (default)

r+, assuming you don't need to re-review this Henrik.

Comment 11

[u279076]

(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.

Comment 12

[Aaron Train [:aaronmt]]

Attachment: Patch v1.2 - (1.9.2)

Comment 13

[u279076]

(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

Comment 14

[Aaron Train [:aaronmt]]

Attachment: Patch v1.2 - (1.9.1)

Comment 15

[u279076]

(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

[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.