Closed Bug 506100 Opened 10 years ago Closed 10 years ago

[mozmill] Test to check error page when SSL disabled

Categories

(Mozilla QA Graveyard :: Mozmill Tests, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ashughes, Assigned: ashughes)

References

()

Details

Attachments

(1 file, 2 obsolete files)

This is a placeholder bug for writing a Mozmill test script for litmus test 7774 -> https://litmus.mozilla.org/show_test.cgi?id=7774
Depends on: 506098
Attached patch Initial patch (obsolete) — Splinter Review
Attachment #390504 - Flags: review?(hskupin)
Attachment #390504 - Flags: review?(hskupin) → review-
Comment on attachment 390504 [details] [diff] [review]
Initial patch

>+var teardownModule = function(module) {
[...] 
>+  UtilsAPI.closeAllTabs(controller);
>+}

This call is not needed. Probably we should remove it in general from the teardownModule function. Each test module which rely on open tabs should do this before the test is started. That is faster and safer.

>+var testDisableSSL = function() {
>+  PrefsAPI.handlePreferencesDialog(prefDialogCallback);
>+  

>+  // Go to https://www.verisign.com
>+  controller.open("https://www.verisign.com");

Nit: This comment is somewhat useless. Please remove it.

>+  // Sleep for 2s because if we are too fast an alert
>+  // appears instead of the error page
>+  controller.sleep(2000);

Please use waitForPageLoad with 2000ms timeout. AFAIR that would ensure that the former page has been unloaded. Which type of alert do you get here? Normally none should be visible.

>+  if (controller.assertNode(title)) {
>+    if (title.getNode().textContent != "Secure Connection Failed") {
>+      throw "Expected error title 'Secure Connection Failed'!";
>+    }

Which purpose has assertNode here? You only want to check the textContent. So just use assertProperty for it. assertNode is not necessary.

>+  controller.waitForElement(text);
>+  controller.assertNode(text);

Using waitForElement makes the assertNode useless. We should probably remove the UtilsAPI.delayedAssertNode function too.

>+  if (text.getNode().textContent.indexOf("ssl_error_ssl_disabled") == -1) {  
>+    throw "Expected error code ssl_error_ssl_disabled!";
>+  }
>+  if (text.getNode().textContent.indexOf("www.verisign.com") == -1) {
>+    throw "Expected domain www.verisign.com!";  
>+  }
>+  if (text.getNode().textContent.indexOf("SSL protocol has been disabled") == -1) {
>+    throw "Expected SSL Disabled error message!";
>+  }

Nit: Please leave an empty line between each if statement.

>+var prefDialogCallback = function(controller) {
[...]
>+  // Close the Prefs dialog
>+  controller.keypress(null, 'VK_ESCAPE', {});
>+}

I assume you haven't run this test on Windows. By hitting ESC to close the preferences dialog the made changes are discarded. So the test always fails on Windows. Please see the other tests under testPreferences how to close the preferences dialog via the OK button on Windows.
Status: NEW → ASSIGNED
Attached patch Rev1 (obsolete) — Splinter Review
Suggested revisions made...
Attachment #390504 - Attachment is obsolete: true
Attachment #392728 - Flags: review?(hskupin)
Attached patch Rev1aSplinter Review
Add @throws to function header comment.
Attachment #392728 - Attachment is obsolete: true
Attachment #392731 - Flags: review?(hskupin)
Attachment #392728 - Flags: review?(hskupin)
Comment on attachment 392731 [details] [diff] [review]
Rev1a

Looks good. I made a small change I wasn't aware before. We should open a blank page before starting the test so we do not already have the error page open if we run this test several times in a row. With this change we can get rid of the 2s sleep.
Attachment #392731 - Flags: review?(hskupin) → review+
Checked in as http://hg.mozilla.org/qa/mozmill-tests/rev/e7a59d6aca88
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Anthony, due to bug 513129 this test always fails for trunk and 1.9.1. Can you please clear the cache after disabling the prefs and before opening the Versign page? That would make the test pass for the moment.
Status: RESOLVED → REOPENED
Depends on: 513129
Resolution: FIXED → ---
I'd like to stop the practice of reopening test creation bugs whenever a change happens which breaks a particular test.  Once a test has been created, checked-in and passes, we should resolve fixed the bug FOREVER.  Any subsequent problems which arise should be filed in a new bug.

When a new feature goes into Firefox, there is a creation bug for that feature.  We don't reopen that bug every time a new bug is found with that feature.  Mozmill tests should be no different.  In other words, each test is a "feature" being added to Mozmill.

As is the case here, please file a new bug and assign it to me.  Thanks.
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Aye, I agree with Anthony's assessment. This happens the rest of the bugs in Mozilla. We really should do the same here. Like gavin says, bugs are cheap :).
No longer depends on: 513129
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.