584002 - Mozmill test to check for software update failure: "Update XML file malformed (200)" (Error: channel.securityInfo is null)

Status: VERIFIED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

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

With the fix on bug 471889 the error mentioned above shouldn't be present anymore. We will need a Mozmill restart test to run the following steps and to verify that an update or the no updates available wizard page is correctly shown in the update dialog.

Steps:
* Launch and set home page to about:blank
* Set start page to not restore tabs from the last session
* Set auto-detect in the proxy configuration
* Restart and then check for updates

With the last step the update available or no updates available page should be shown.

Aaron, would you have a free spot to implement this test?

Comment 1

[Aaron Train [:aaronmt]]

Attachment: Test - Check for browser updates with Auto Proxy enabled

Asking for feedback on a couple things. 

a) Test location/name
b) Empty teardown module in test1 (so we dont revert pref changes), empty ok?
c) In the softwareAPI, there is no wizard page constant for updatesFound - I was going to do an assert for this.
d) Proper format for restart? I simply set the prefs in the first file, and in the second file we open the update dialog after the restart (as per Roberts STR)

Comment 2

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

Comment on attachment 462505 [details] [diff] [review]
Test - Check for browser updates with Auto Proxy enabled

>diff -r 5eb639052a6d -r 9a15b33ad3fb firefox/restartTests/testCheckSoftwareUpdateAutoProxy/test1.js

We should start to use one more level of sub folders here to have a better organization of the tests. Can you do restartTests/softwareUpdate/testXMLErrorAutoProxy/?

>+ * The Initial Developer of the Original Code is Mozilla Foundation.
>+ * Portions created by the Initial Developer are Copyright (C) 2009

"the Mozilla Foundation" and 2010.

>+var testCheckSoftwareUpdateAutoProxy = function() {
>+ // Set browser home page to about:blank
>+ PrefsAPI.preferences.setPref(BROWSER_HOME_PAGE, "about:blank");
>+ 
>+ // Set browser start up to display current home page
>+ PrefsAPI.preferences.setPref(BROWSER_STARTUP_PAGE, 1);
>+ 
>+ // Set the proxy type in connection settings to 'Auto-detect proxy settings ...'
>+ PrefsAPI.preferences.setPref(PROXY_TYPE, 4);
>+}

You can move all that stuff to setupModule. There is no ui interaction in that test. You can drop the test and teardownModule function.

>+var teardownModule = function(module) {
>+ PrefsAPI.preferences.clearUserPref(BROWSER_HOME_PAGE);
>+ PrefsAPI.preferences.clearUserPref(BROWSER_STARTUP_PAGE);
>+ PrefsAPI.preferences.clearUserPref(PROXY_TYPE);
>+}

No need to clear the prefs. This profile will not be used for following restart tests.

>+ // Check to see if we have browser updates
>+ if (update.updatesFound) {
>+  update.controller.waitForEval("subject.update.updatesFound == true", TIMEOUT, 100,
>+                                 {update: update});
>+ } else {
>+  update.controller.waitForEval("subject.update.updatesFound == false", TIMEOUT, 100,
>+                                {update: update});
>+  controller.assertJS('subject.currentPage == subject.noUpdatesFoundPage',
>+                      {currentPage: update.currentPage, noUpdatesFoundPage: 
>+                      SoftwareUpdateAPI.WIZARD_PAGES.noUpdatesFound});

You can use update.currentPage to wait for the possible wizard pages: noUpdatesFound, updatesFoundBasic, or updatesFoundBillboard.

Rob would that be enough?

Comment 3

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

Comment on attachment 462505 [details] [diff] [review]
Test - Check for browser updates with Auto Proxy enabled

>diff -r 5eb639052a6d -r 9a15b33ad3fb firefox/restartTests/testCheckSoftwareUpdateAutoProxy/test1.js

We should start to use one more level of sub folders here to have a better organization of the tests. Can you do restartTests/softwareUpdate/testXMLErrorAutoProxy/?

>+ * The Initial Developer of the Original Code is Mozilla Foundation.
>+ * Portions created by the Initial Developer are Copyright (C) 2009

"the Mozilla Foundation" and 2010.

>+var testCheckSoftwareUpdateAutoProxy = function() {
>+ // Set browser home page to about:blank
>+ PrefsAPI.preferences.setPref(BROWSER_HOME_PAGE, "about:blank");
>+ 
>+ // Set browser start up to display current home page
>+ PrefsAPI.preferences.setPref(BROWSER_STARTUP_PAGE, 1);
>+ 
>+ // Set the proxy type in connection settings to 'Auto-detect proxy settings ...'
>+ PrefsAPI.preferences.setPref(PROXY_TYPE, 4);
>+}

You can move all that stuff to setupModule. There is no ui interaction in that test. You can drop the test and teardownModule function.

>+var teardownModule = function(module) {
>+ PrefsAPI.preferences.clearUserPref(BROWSER_HOME_PAGE);
>+ PrefsAPI.preferences.clearUserPref(BROWSER_STARTUP_PAGE);
>+ PrefsAPI.preferences.clearUserPref(PROXY_TYPE);
>+}

No need to clear the prefs. This profile will not be used for following restart tests.

>+ // Check to see if we have browser updates
>+ if (update.updatesFound) {
>+  update.controller.waitForEval("subject.update.updatesFound == true", TIMEOUT, 100,
>+                                 {update: update});
>+ } else {
>+  update.controller.waitForEval("subject.update.updatesFound == false", TIMEOUT, 100,
>+                                {update: update});
>+  controller.assertJS('subject.currentPage == subject.noUpdatesFoundPage',
>+                      {currentPage: update.currentPage, noUpdatesFoundPage: 
>+                      SoftwareUpdateAPI.WIZARD_PAGES.noUpdatesFound});

You can use update.currentPage to wait for the possible wizard pages: noUpdatesFound, updatesFoundBasic, or updatesFoundBillboard.

Rob would that be enough?

Comment 4

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

Aaron, any update for this test?

Comment 5

[Aaron Train [:aaronmt]]

(In reply to comment #4)
> Aaron, any update for this test?

Was waiting for a) any update from Rob, b) an answer on if I should subfolder restart_tests which presently aren't working

Comment 6

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

(In reply to comment #5)
> Was waiting for a) any update from Rob, b) an answer on if I should subfolder
> restart_tests which presently aren't working

For b) don't worry about it now. Simply use a meaningful prefix like testUpdateXXX.

Comment 7

[Robert Strong (they/them - no direct email)]

Aaron, I'm not familiar enough with mozmill to say for certain but as far as verifying that an updates found page was actually seen the "get updatesFound()" in testSoftwareUpdateAPI.js in the mozmill repo handles this properly.

Comment 8

[Aaron Train [:aaronmt]]

(In reply to comment #3)
> >+ // Check to see if we have browser updates
> >+ if (update.updatesFound) {
> >+  update.controller.waitForEval("subject.update.updatesFound == true", TIMEOUT, 100,
> >+                                 {update: update});
> >+ } else {
> >+  update.controller.waitForEval("subject.update.updatesFound == false", TIMEOUT, 100,
> >+                                {update: update});
> >+  controller.assertJS('subject.currentPage == subject.noUpdatesFoundPage',
> >+                      {currentPage: update.currentPage, noUpdatesFoundPage: 
> >+                      SoftwareUpdateAPI.WIZARD_PAGES.noUpdatesFound});
> 
> You can use update.currentPage to wait for the possible wizard pages:
> noUpdatesFound, updatesFoundBasic, or updatesFoundBillboard.

I'm checking for updates in the nightlies on 1.9.2  testing this and the first wizard page appearing is identified as a generic 'updatesfound'. Are you sure this isn't missing as one of the const's in testSoftwareUpdateAPI? It seems like if I use 'updatesFoundBasic', I would eventually timeout.

Comment 9

[Robert Strong (they/them - no direct email)]

Aaron, you should use what I mentioned in comment #8 since it accounts for 1.9.2, trunk, and barring any major changes the future as well

Comment 10

[Aaron Train [:aaronmt]]

Attachment: Patch v1 - (default)

Comment 11

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

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

>+++ b/firefox/restartTests/testCheckSoftwareUpdateAutoProxy/test1.js	Fri Aug 13 11:50:54 2010 -0400

Please name it "testSoftwareUpdateAutoProxy" so we can have virtual subgroups for now. It will make it easier later to move the tests. We should rename the other folders accordingly in another bug and update the litmus tests.

>+var testCheckSoftwareUpdateAutoProxy = function() {
>+ // Set browser home page to about:blank
>+ PrefsAPI.preferences.setPref(BROWSER_HOME_PAGE, "about:blank");
>+ 
>+ // Set browser start up to display current home page
>+ PrefsAPI.preferences.setPref(BROWSER_STARTUP_PAGE, 1);
>+ 
>+ // Set the proxy type in connection settings to 'Auto-detect proxy settings ...'
>+ PrefsAPI.preferences.setPref(PROXY_TYPE, 4);
>+
>+}

Please do the setup of the test in setupModule(). This test module doesn't need a test.

>+// Include necessary modules
>+const RELATIVE_ROOT = '../../../shared-modules';
>+const MODULE_REQUIRES = ['PrefsAPI', 'SoftwareUpdateAPI'];
[..]
>+const BROWSER_HOME_PAGE = 'browser.startup.homepage';
>+const BROWSER_STARTUP_PAGE = 'browser.startup.page';
>+const PROXY_TYPE = 'network.proxy.type';

Please remove those not used lines.

>+var setupModule = function(module) {

nit: remove module from the parameter list.

>+var testCheckSoftwareUpdateAutoProxy = function() {
>+ // Check to see if we have browser updates
>+ if (update.updatesFound) {
>+  update.controller.waitForEval("subject.update.updatesFound == true", TIMEOUT, 100,
>+                                {update: update});
>+ } else {
>+  update.controller.waitForEval("subject.update.updatesFound == false", TIMEOUT, 100,
>+                                {update: update});
>+  controller.assertJS("subject.currentPage == subject.noUpdatesFoundPage",
>+                      {currentPage: update.currentPage, noUpdatesFoundPage: 
>+                       SoftwareUpdateAPI.WIZARD_PAGES.noUpdatesFound});
>+ }

Can we do this with try/catch please? It will save some lines and makes it better readable:

try:
  waitfor updatesfound
catch:
  assert noupdatesfound

Comment 12

[Aaron Train [:aaronmt]]

Attachment: Patch v2 - (default)

Addresses the comments above

Comment 13

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

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

Looks perfect and catched the issue with an older Namoroka build.

Comment 14

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

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/bc08c8248eed (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/874c2f46a0e4 (mozilla1.9.2)

Comment 15

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