510621 - [mozmill] - Verify passwords are not saved when we select not to save them

Status: VERIFIED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Aakash Desai [:aakashd]]

Attachment: testPasswordManager/testNoSavedPassword.js

The testscript uses a sample login page from mozilla, submits login information via the sample form and closes the resulting notification bar. It verifies the notification bar pop-ups and closes as well as the information is not saved.



Litmus Testcases Represented:
 * Testcase ID #6486 - Password Manager post-submit bar
 * Testcase ID #6132 - Verify passwords are not saved when we select not to save

Comment 1

[Aakash Desai [:aakashd]]

Attachment: testPasswordManager/testNoSavedPassword.js

Using the review from bug 509125 , the functions for the two references testcases were split in the testscript.

Comment 2

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

Comment on attachment 394855 [details]
testPasswordManager/testNoSavedPassword.js

>// Include necessary modules
>var RELATIVE_ROOT = '../../shared-modules';
>var MODULE_REQUIRES = ['ModalDialogAPI','PrefsAPI', 'UtilsAPI'];

Which part needs the PrefsAPI?

>const gTimeout = 1000;

Please change it to 5000

>  module.panel = new elementslib.Lookup(controller.window.document, '/id("main-window")/id("browser")/id("appcontent")/id("content")/anon({"anonid":"tabbox"})/anon({"anonid":"panelcontainer"})');

No need for that. Please use the new notification bar helper function.

>var testPasswordNotificationBar = function() {

Just a note. We should start to move the { bracket to the next line for function definitions.

>  // Leave only one tab open in the browser
>  UtilsAPI.closeAllTabs(controller);

Your test doesn't depend on that exactly one tab is open. You can remove this line.

>  controller.waitForElement(userField);

Please always add a gTimeout to this call.

>  controller.waitThenClick(new elementslib.ID(controller.tabs.activeTab, "LogIn"));

You can click immediately. No need to wait.

>  controller.sleep(gTimeout);

You wait because of the notification bar? That's not necessary.

>  // After logging in, close the notification bar
>  var xButton = new elementslib.Lookup(controller.window.document, '/id("main-window")/id("browser")/id("appcontent")/id("content")/anon({"anonid":"tabbox"})/anon({"anonid":"panelcontainer"})/{"flex":"1","id":"' + panel.getNode().childNodes[0].id + '"}/{"value":"password-save","priority":"2","type":"info"}/anon({"type":"info"})/{"class":"messageCloseButton tabbable"}');

As said above please use the new fancy way to access the notification bar.

>  UtilsAPI.assertElementVisible(controller, xButton, true);  
>  controller.click(xButton);
>  controller.sleep(gTimeout);
>  controller.assertNodeNotExist(xButton);

Why you are using assertElementVisible and not simply assertNode? Is this button present all the time? If yes then assertNodeNotExist is wrongly used. And replace the gTimeout with a hardcoded value.


>var testVerifyNoSavedPasswords = function() {
>  // Go back verify the login information has not been saved
>  controller.open(testSite);
>  controller.waitForPageLoad();
>  
>  controller.waitForElement(userField);
>  controller.assertValue(userField, "");
>
>  controller.sleep(gTimeout);
>  controller.click(passField);  
>  controller.assertValue(passField, "");
>}

I completely miss the safed password window check inside the preferences. What happens with those checks? I don't trust the automatic fill-in.

Also please add some code to setupModule which clears all saved passwords before the test is started.

I would be really happy when you could supply a real patch the next time.

Comment 3

[Aakash Desai [:aakashd]]

Attachment: patch_file for review

Ok, so gTimeout was added to all waitThenClick and waitForElement functions, assertElementVisible has been removed, shared-module dependencies were removed, the nsILoginManager is called to verify 0 logins are saved in the database...


Also, is there documentation or an example of the new notification bar functionality? Other than that, I'm submitting this patch for review :) (I hope I did it right this time)

Comment 4

[Aakash Desai [:aakashd]]

Attachment: patch_file with new notification bar implementation

I apologize for the many patches today, Henrik. I've found the notification bar implementation and made the changes needed. Here's the patch

Comment 5

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

Comment on attachment 396351 [details] [diff] [review]
patch_file with new notification bar implementation

>+ * Alternatively, the contents of this file may be used under the terms of
>+ * either the GNU General Public License Vertributed under the License is distributed on an "AS IS" basis,
>+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
>+ * for the specific language governing rights and limitations under the
>+ * License.

Something screwed up with the length of the license text line.

>+var setupModule = function(module) {
>+
>+  module.panel = new elementslib.Lookup(controller.window.document, '/id("main-window")/id("browser")/id("appcontent")/id("content")/anon({"anonid":"tabbox"})/anon({"anonid":"panelcontainer"})');

Remove it.

>+  var pm = Components.classes["@mozilla.org/login-manager;1"]
>+                         .getService(Components.interfaces.nsILoginManager);
>+  
>+  pm.removeAllLogins();

Please enclose in try/catch, use CC and Ci which is shorter and put the '.' in the 2nd line directly under the other dot. It should be also called inside teardown.

>+var testPasswordNotificationBar = function() {

Please add a description.

>+  controller.click(new elementslib.ID(controller.tabs.activeTab, "LogIn"));
>+  controller.sleep(500);

No sleep. Please use waitForPageLoad. Further please also test that the password bar doesn't appear before the click.

>+  // After logging in, close the notification bar
>+  //var xButton = new elementslib.Lookup(controller.window.document, '/id("main-window")/id("browser")/id("appcontent")/id("content")/anon({"anonid":"tabbox"})/anon({"anonid":"panelcontainer"})/{"flex":"1","id":"' + panel.getNode().childNodes[0].id + '"}/{"value":"password-save","priority":"2","type":"info"}/anon({"type":"info"})/{"class":"messageCloseButton tabbable"}');

Please remove.

>+var testNoSavedPassword = function() {

Please call it testPasswordsNotSaved (also update the file name)

>+  var pm = Components.classes["@mozilla.org/login-manager;1"]
>+                         .getService(Components.interfaces.nsILoginManager);
>+  
>+  if ( pm.countLogins("", "", "") > 0 ) {
>+    throw "Login was not deleted from the password manager.";	
>+  }
>+}

Why are you using back-end functions here? You should simply do a click in the username field and check that no auto-complete entries are shown.

>\ No newline at end of file

Please add one.

Comment 6

[Aakash Desai [:aakashd]]

Attachment: patch after comment #5

This patch fixed the license, removed panel from the step module, added litmus testcase comments above each test module, replaced sleep with waitForPageLoad after LogIn, removed commented old xButton instantiation, changed testNoSavedPassword to testPasswordNotSaved, and added GUI check for pwd manager having no logins.

Comment 7

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

Comment on attachment 396469 [details] [diff] [review]
patch after comment #5

In general ok, I made some changes. Please check for the difference.

Comment 8

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

Attachment: Test for checkin

Comment 9

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

Landed as:

http://hg.mozilla.org/qa/mozmill-tests/rev/8737db764c9f
http://hg.mozilla.org/qa/mozmill-tests/rev/9462efd6ebba

Comment 10

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

No failures anymore with bug 516729 fixed.

Comment 11

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

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.