Closed Bug 504178 Opened 15 years ago Closed 15 years ago

[mozmill] Test default security preferences

Categories

(Mozilla QA Graveyard :: Mozmill Tests, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: u279076, Assigned: u279076)

Details

Attachments

(1 file, 5 obsolete files)

This is a placeholder bug for creating a Mozmill test for the Default Security Preferences BFT in Litmus (https://litmus.mozilla.org/show_test.cgi?id=6018)
Attachment #388553 - Flags: review?(hskupin)
Assignee: nobody → ashughes
Attached patch Rev1 (obsolete) — Splinter Review
A few personal nits...like missing comments and @throws statements added.
Attachment #388553 - Attachment is obsolete: true
Attachment #388862 - Flags: review?(hskupin)
Attachment #388553 - Flags: review?(hskupin)
Status: NEW → ASSIGNED
OS: Mac OS X → All
Hardware: x86 → All
Attachment #388862 - Flags: review?(hskupin) → review-
Comment on attachment 388862 [details] [diff] [review]
Rev1

>+ * The Original Code is Mozilla Mozmill Test Code.

Please remove Mozilla from that line. It was accidentally added by a test in the past and has been carried all the time.

>+var teardownModule = function(module) {
>+  // Reset the SSL & TLS prefs
>+  try {
>+    PrefsAPI.preferences.branch.clearUserPref("security.enable_ssl3");
>+    PrefsAPI.preferences.branch.clearUserPref("security.enable_tls");
>+  } catch(e) {
>+  }
>+
>+  // Close all open tabs
>+  UtilsAPI.closeAllTabs(controller);
>+}

The teardownModule function is not necessary for this test. We don't change any pref and don't open any tab. So please remove it.

>+var testDefaultSecurityPreferences = function() {
>+  UtilsAPI.closeAllTabs(controller);

No need to call this function. You don't rely on multiple tabs in this test.

>+var prefDialogCallback = function(controller) {
>+  // Get the Advanced Pane
>+  var pane = '/id("BrowserPreferences")/anon({"orient":"vertical"})/anon({"anonid":"selector"})/{"pane":"paneAdvanced"}';
>+  controller.click(new elementslib.Lookup(controller.window.document, pane));

Can you please change it to waitThenClick? That makes us safer since we can't be 100% sure that the element exists immediately.

>+  // Get the Encryption tab
>+  controller.click(new elementslib.ID(controller.window.document, "encryptionTab"));

Same here.

>+  controller.waitForElement(sslPref, null, null);
>+  controller.waitForElement(tlsPref, null, null);

Nit: There is no need to pass null values as params to the functions. At some point we should decide about a good timeout value here but for now we should use the default and check how all the tests run from the command line.

>+  var sslChecked = sslPref.getNode().checked;
>+  var tlsChecked = tlsPref.getNode().checked;
>+  
>+  if (!sslChecked) {
>+    throw "Use SSL 3.0 is not checked but should be!";
>+  }
>+  if (!tlsChecked) {
>+    throw "Use TLS 1.0 is not checked but should be!";
>+  }

Can you please eliminate the extra variables? You can put the code directly into the if statement.

>+  // Close the preferences dialog
>+  if (mozmill.isWindows) {
>+    var okButton = new elementslib.Lookup(controller.window.document, '/id("BrowserPreferences")/anon({"anonid":"dlg-buttons"})/{"dlgtype":"accept"}');
>+    controller.click(okButton);
>+  } else {
>+    controller.keypress(null, 'VK_ESCAPE', {});
>+  }

Given the fact that we don't do any changes to the prefs you can always hit the ESC key. No need to differentiate between the platforms.
Attached patch Rev2 (obsolete) — Splinter Review
Attachment #388862 - Attachment is obsolete: true
Attachment #388979 - Flags: review?(hskupin)
Attachment #388979 - Flags: review?(hskupin) → review+
Comment on attachment 388979 [details] [diff] [review]
Rev2

Thanks Anthony! That looks good. There is only one small thing I missed in the last review.

>+  if (!sslPref.getNode().checked) {
>+    throw "Use SSL 3.0 is not checked but should be!";
>+  }
>+  if (!tlsPref.getNode().checked) {
>+    throw "Use TLS 1.0 is not checked but should be!";
>+  }

We have a controller.assertChecked function you should use here. With the change, r=me.
Attached patch Rev3 (obsolete) — Splinter Review
Attachment #388979 - Attachment is obsolete: true
Attachment #389728 - Flags: review?(hskupin)
Attached patch Rev4 (obsolete) — Splinter Review
Forgot to remove the @throws comments
Attachment #389728 - Attachment is obsolete: true
Attachment #389729 - Flags: review?(hskupin)
Attachment #389728 - Flags: review?(hskupin)
Attached patch Rev4aSplinter Review
Patch corrupt...new version of same patch
Attachment #389729 - Attachment is obsolete: true
Attachment #389989 - Flags: review?(hskupin)
Attachment #389729 - Flags: review?(hskupin)
Comment on attachment 389989 [details] [diff] [review]
Rev4a

The throws have returned since rev4. I will remove them and check it in.
Attachment #389989 - Flags: review?(hskupin) → review+
Checked in as http://hg.mozilla.org/qa/mozmill-tests/rev/e0557fc8b321

I removed the throw comments and updated the license block as noted on IRC. Thanks Anthony.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Mass 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.
Component: Security → Mozmill Tests
Product: Firefox → Mozilla QA
QA Contact: firefox → mozmill-tests
Version: Trunk → unspecified
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: