607198 - Update tests to use CommonJS [group 4]

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[u279076]

This bug tracks the work needed to move the current tests over to use CommonJS
instead of the old module system. Specifically, this bug deals with conversion of the following test groups:

testSearch: 9 tests
testSecurity: 15 tests
testSessionStore: 1 test
------------------------
25 tests total

Comment 1

[u279076]

Attachment: Patch v1 (default)

Comment 2

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

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

>+var modaldialog = require("../../shared-modules/testModalDialogAPI");
[..]
>+var tabbedbrowser = require("../../shared-modules/testTabbedBrowsingAPI");
[..]
>+var sessionstore = require("../../shared-modules/testSessionStoreAPI");

Only skimmed the patch but have seen that the entries above do not use the naming convention given by my email from yesterday. Please update the patch to match those. Otherwise it's even harder for me to do the final patch. Thanks.

Comment 3

[u279076]

Attachment: Patch v2 (default)

Comment 4

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

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

Anthony, looks pretty good, but you use "searchbar" all over the place when consistency with existing code and camelcaps would call for "searchBar."

With that change, r=me. 

Do make sure you do a quick test run across the changed tests, though--it's hard to make sure you caught everything in a code review.

Comment 5

[u279076]

Attachment: Patch v3 (default) [checked-in]

Patch tested, no regressions found.

Comment 6

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Comment on attachment 486160 [details] [diff] [review]
Patch v3 (default) [checked-in]

LGTM, thanks for the change! Please go ahead and land it.

Comment 7

[u279076]

Thanks for the review.  Can you please land this as I am on the road to Toronto tomorrow.  I will b afk for the next 24h.

Thanks

Comment 8

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

Comment on attachment 486160 [details] [diff] [review]
Patch v3 (default) [checked-in]

Landed on default:
http://hg.mozilla.org/qa/mozmill-tests/rev/390d7b2f8e20

Anthony, will you be able to work on the backports today? Otherwise please hand of to Aaron.

Comment 9

[u279076]

Later today, yes.  Considering the tree is closed for 3.5/3.6 releases, I think it's fine.

Comment 10

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

I would like to see patch up today, even reviewed if possible. That way I can check-in everything once we are done with the release testing. Thanks.

Comment 11

[u279076]

Then you better get Aaron to do it because I am afk until tomorrow.

Comment 12

[Aaron Train [:aaronmt]]

Attachment: Patch v1 - (1.9.2)

1.9.2 backport

Will get 1.9.1 tomorrow

Comment 13

[Aaron Train [:aaronmt]]

Attachment: Patch v1 - (1.9.1)

Comment 14

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Comment on attachment 486609 [details] [diff] [review]
Patch v1 - (1.9.1)

Really minor indentation problems.

testSearch/GetMoreSearchEngines, look for var confirmTitle.

testSecurity/testGreyLarry.js and testBlueLarry.js, look for var l10n* and var security*

testSecurity/testSafeBrowsingNotificationBar.js, look for var label (twice)

With those fixed, r=me.

Comment 15

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Comment on attachment 486490 [details] [diff] [review]
Patch v1 - (1.9.2)

>diff --git a/firefox/testSearch/testAddMozSearchProvider.js b/firefox/testSearch/testAddMozSearchProvider.js
>
>+  var confirmTitle = utils.getProperty("chrome://global/locale/search/search.properties",
>                                           "addEngineConfirmTitle");


>diff --git a/firefox/testSearch/testGetMoreSearchEngines.js b/firefox/testSearch/testGetMoreSearchEngines.js
>
>+  var confirmTitle = utils.getProperty("chrome://global/locale/search/search.properties",
>                                           "addEngineConfirmTitle");
>diff --git a/firefox/testSecurity/testBlueLarry.js b/firefox/testSecurity/testBlueLarry.js
>
>+  var property = utils.getProperty("chrome://browser/locale/browser.properties",
>
>+  var l10nVerifierLabel = utils.getProperty("chrome://browser/locale/browser.properties",
>                                                "identity.identified.verifier");
>
>+  var l10nEncryptionLabel = utils.getProperty("chrome://browser/locale/browser.properties",
>                                                  "identity.encrypted");
>
>+  var securityOwner = utils.getProperty("chrome://browser/locale/pageInfo.properties",
>                                            "securityNoOwner");

>diff --git a/firefox/testSecurity/testEncryptedPageWarning.js b/firefox/testSecurity/testEncryptedPageWarning.js
>
>+  var enterSecureMessage = utils.getProperty("chrome://pipnss/locale/security.properties",
>                                                 "EnterSecureMessage");

>diff --git a/firefox/testSecurity/testGreenLarry.js b/firefox/testSecurity/testGreenLarry.js
>
>+  var l10nVerifierLabel = utils.getProperty("chrome://browser/locale/browser.properties",
>                                                "identity.identified.verifier");

>+  var l10nEncryptionLabel = utils.getProperty("chrome://browser/locale/browser.properties",
>                                                  "identity.encrypted");

>diff --git a/firefox/testSecurity/testGreyLarry.js b/firefox/testSecurity/testGreyLarry.js
>
>+  var securityOwner = utils.getProperty("chrome://browser/locale/pageInfo.properties",
>                                            "securityNoOwner");

>+  var securityIdentifier = utils.getProperty("chrome://browser/locale/pageInfo.properties",
>                                                 "notset");

>diff --git a/firefox/testSecurity/testSafeBrowsingNotificationBar.js b/firefox/testSecurity/testSafeBrowsingNotificationBar.js
>
>+    var label = utils.getProperty("chrome://browser/locale/browser.properties",
>                                      "safebrowsing.notAForgeryButton.label");

>+    var label = utils.getProperty("chrome://browser/locale/browser.properties",
>                                      "safebrowsing.notAnAttackButton.label");

>+  var label = utils.getProperty("chrome://browser/locale/browser.properties",
>                                    "safebrowsing.getMeOutOfHereButton.label");

>diff --git a/firefox/testSecurity/testSubmitUnencryptedInfoWarning.js b/firefox/testSecurity/testSubmitUnencryptedInfoWarning.js
>
>+  var message = utils.getProperty("chrome://pipnss/locale/security.properties",
>                                      "PostToInsecureFromInsecureMessage");

Same indent issues. r=me when fixed. Also, doublecheck 1.9.1 against these--I may have missed some of them there.

Comment 16

[Aaron Train [:aaronmt]]

Attachment: Patch v2 - (1.9.2) [checked-in]

Thanks for catching these indent changes, there were a few more as well fixed.

Comment 17

[Aaron Train [:aaronmt]]

Attachment: Patch v2 - (1.9.1) [checked-in]

Same fixes as above

Comment 18

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

Backports landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/716a1229a3d9 (1.9.2)
http://hg.mozilla.org/qa/mozmill-tests/rev/129c9b95ad59 (1.9.1)

Comment 19

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