Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
5379007:
Adding pyauto tests to passwords.py... (Closed)
|
Created:
10 years ago by sunandt Modified:
9 years, 7 months ago CC:
chromium-reviews, anantha, rohitbm Visibility:
Public. |
DescriptionAdding pyauto tests to passwords.py
Added tests
1. testRemovePasswords
2. testFetchAnotherUser
3. testNeverSavePasswords
Added a method for constructing password dictionary
Modified testSavePassword in passwords.py
MOdified GoogleAccountsLogin and VerifyGoogleAccountCredsFilled in test_utils.py to take window index and tab index
BUG=none
TEST=none
Patch Set 1 #Patch Set 2 : '' #
Total comments: 29
Patch Set 3 : '' #
Total comments: 25
Patch Set 4 : '' #
Total comments: 6
Patch Set 5 : '' #
Total comments: 7
Patch Set 6 : '' #Patch Set 7 : '' #Messages
Total messages: 16 (0 generated)
http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py File functional/passwords.py (right): http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:77: # Remove the comment when that gets fixed. add the bug# here http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:108: test_utils.GoogleAccountsLogin(self, creds1['login_url'], You should directly call AddSavedPassword, instead of doing by actually signing in. The use case of actually signing in has been verified in another test. http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:116: creds2 = self.GetPrivateInfo()['etouchqa_google_account'] Please create an internal CL for adding this entry in private_tests_info.txt http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:118: creds2['username'], creds2['password'], 0, 1) pass as named args: tab_index=1, windex=0 Repeat everywhere else http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:123: # Populate the username field with the first saved credentials. This comment is misleading. Wait for username field to get populated http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:125: lambda: self.GetDOMValue('document.getElementById("Email").value', 0, 2), 80+ chars http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:133: def testNeverSavePasswords(self): testNeverSavePasswordsPref http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:144: self.RunCommand(pyauto.IDC_NEW_TAB) use AppendTab http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:147: creds2['username'], creds2['password'], 0, 2) use named args for windex, tab_index http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:151: # TODO: Remove comment when GetSavedPasswords() is fixed. won't this have 1 entry at this point? http://codereview.chromium.org/5379007/diff/2001/functional/test_utils.py File functional/test_utils.py (right): http://codereview.chromium.org/5379007/diff/2001/functional/test_utils.py#new... functional/test_utils.py:31: test.WaitForAllDownloadsToComplete() leave 2 blank lines at global scope http://codereview.chromium.org/5379007/diff/2001/functional/test_utils.py#new... functional/test_utils.py:43: pyauto_utils.RemovePath(downloaded_pkg + '.crdownload') ditto. and repeat everywhere http://codereview.chromium.org/5379007/diff/2001/functional/test_utils.py#new... functional/test_utils.py:45: def GoogleAccountsLogin(test, url, username, password, windex=0, tab_index=0): Please reverse the order. Pass tab_index then windex http://codereview.chromium.org/5379007/diff/2001/functional/test_utils.py#new... functional/test_utils.py:67: def VerifyGoogleAccountCredsFilled(test, username, password, windex=0, same here
Removed the testcase for fetching another user as Chrome wasn't populating the password field when username is populated through javascript. Other suggested changes have been made. http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py File functional/passwords.py (right): http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:77: # Remove the comment when that gets fixed. On 2010/11/29 22:42:58, Nirnimesh wrote: > add the bug# here Done. http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:108: test_utils.GoogleAccountsLogin(self, creds1['login_url'], On 2010/11/29 22:42:58, Nirnimesh wrote: > You should directly call AddSavedPassword, instead of doing by actually signing > in. > > The use case of actually signing in has been verified in another test. Removing this testcase. http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:118: creds2['username'], creds2['password'], 0, 1) On 2010/11/29 22:42:58, Nirnimesh wrote: > pass as named args: > tab_index=1, windex=0 > Repeat everywhere else Removing this testcase. http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:123: # Populate the username field with the first saved credentials. On 2010/11/29 22:42:58, Nirnimesh wrote: > This comment is misleading. > > Wait for username field to get populated Removing this testcase. http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:125: lambda: self.GetDOMValue('document.getElementById("Email").value', 0, 2), On 2010/11/29 22:42:58, Nirnimesh wrote: > 80+ chars Removing this testcase. http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:133: def testNeverSavePasswords(self): On 2010/11/29 22:42:58, Nirnimesh wrote: > testNeverSavePasswordsPref This is not the preference test. http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:144: self.RunCommand(pyauto.IDC_NEW_TAB) On 2010/11/29 22:42:58, Nirnimesh wrote: > use AppendTab Removed. http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:147: creds2['username'], creds2['password'], 0, 2) On 2010/11/29 22:42:58, Nirnimesh wrote: > use named args for windex, tab_index Done. http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:151: # TODO: Remove comment when GetSavedPasswords() is fixed. On 2010/11/29 22:42:58, Nirnimesh wrote: > won't this have 1 entry at this point? GetSavedPasswords() isn't returning anything from exceptions list. http://codereview.chromium.org/5379007/diff/2001/functional/test_utils.py File functional/test_utils.py (right): http://codereview.chromium.org/5379007/diff/2001/functional/test_utils.py#new... functional/test_utils.py:31: test.WaitForAllDownloadsToComplete() On 2010/11/29 22:42:58, Nirnimesh wrote: > leave 2 blank lines at global scope Done. http://codereview.chromium.org/5379007/diff/2001/functional/test_utils.py#new... functional/test_utils.py:43: pyauto_utils.RemovePath(downloaded_pkg + '.crdownload') On 2010/11/29 22:42:58, Nirnimesh wrote: > ditto. and repeat everywhere Done. http://codereview.chromium.org/5379007/diff/2001/functional/test_utils.py#new... functional/test_utils.py:45: def GoogleAccountsLogin(test, url, username, password, windex=0, tab_index=0): On 2010/11/29 22:42:58, Nirnimesh wrote: > Please reverse the order. Pass tab_index then windex Done. http://codereview.chromium.org/5379007/diff/2001/functional/test_utils.py#new... functional/test_utils.py:67: def VerifyGoogleAccountCredsFilled(test, username, password, windex=0, On 2010/11/29 22:42:58, Nirnimesh wrote: > same here Done.
http://codereview.chromium.org/5379007/diff/9001/functional/passwords.py File functional/passwords.py (right): http://codereview.chromium.org/5379007/diff/9001/functional/passwords.py#newc... functional/passwords.py:5: nit: remove this line http://codereview.chromium.org/5379007/diff/9001/functional/passwords.py#newc... functional/passwords.py:37: """Construct a password dictionary with all the required details.""" s/details/fields/ http://codereview.chromium.org/5379007/diff/9001/functional/passwords.py#newc... functional/passwords.py:107: test_utils.GoogleAccountsLogin(self, creds1['login_url'], move all args to next line http://codereview.chromium.org/5379007/diff/9001/functional/passwords.py#newc... functional/passwords.py:118: self.WaitForInfobarCount(1, windex=0, tab_index=1) can skip the windex arg http://codereview.chromium.org/5379007/diff/9001/functional/passwords.py#newc... functional/passwords.py:119: self.PerformActionOnInfobar('cancel', infobar_index=0, Mention (in comment) that this is equivalent to selecting the 'nver for this site' option http://codereview.chromium.org/5379007/diff/9001/functional/passwords.py#newc... functional/passwords.py:120: windex=0, tab_index=1) can skip the windex arg http://codereview.chromium.org/5379007/diff/9001/functional/test_utils.py File functional/test_utils.py (right): http://codereview.chromium.org/5379007/diff/9001/functional/test_utils.py#new... functional/test_utils.py:33: Remove the extra blank space http://codereview.chromium.org/5379007/diff/9001/functional/test_utils.py#new... functional/test_utils.py:46: ditto http://codereview.chromium.org/5379007/diff/9001/functional/test_utils.py#new... functional/test_utils.py:56: password: users login password input. document the args http://codereview.chromium.org/5379007/diff/9001/functional/test_utils.py#new... functional/test_utils.py:57: """ get rid of extra blank spaces http://codereview.chromium.org/5379007/diff/9001/functional/test_utils.py#new... functional/test_utils.py:86: remove extra blank spaces http://codereview.chromium.org/5379007/diff/9001/functional/test_utils.py#new... functional/test_utils.py:91: remove extra blank spaces http://codereview.chromium.org/5379007/diff/9001/functional/test_utils.py#new... functional/test_utils.py:113: remove extra blank spaces
Modified http://codereview.chromium.org/5379007/diff/9001/functional/passwords.py File functional/passwords.py (right): http://codereview.chromium.org/5379007/diff/9001/functional/passwords.py#newc... functional/passwords.py:5: On 2010/12/08 20:59:17, Nirnimesh wrote: > nit: remove this line Done. http://codereview.chromium.org/5379007/diff/9001/functional/passwords.py#newc... functional/passwords.py:37: """Construct a password dictionary with all the required details.""" On 2010/12/08 20:59:17, Nirnimesh wrote: > s/details/fields/ Done. http://codereview.chromium.org/5379007/diff/9001/functional/passwords.py#newc... functional/passwords.py:107: test_utils.GoogleAccountsLogin(self, creds1['login_url'], On 2010/12/08 20:59:17, Nirnimesh wrote: > move all args to next line Done. http://codereview.chromium.org/5379007/diff/9001/functional/passwords.py#newc... functional/passwords.py:118: self.WaitForInfobarCount(1, windex=0, tab_index=1) On 2010/12/08 20:59:17, Nirnimesh wrote: > can skip the windex arg Done. http://codereview.chromium.org/5379007/diff/9001/functional/passwords.py#newc... functional/passwords.py:119: self.PerformActionOnInfobar('cancel', infobar_index=0, On 2010/12/08 20:59:17, Nirnimesh wrote: > Mention (in comment) that this is equivalent to selecting the 'nver for this > site' option Done. http://codereview.chromium.org/5379007/diff/9001/functional/passwords.py#newc... functional/passwords.py:120: windex=0, tab_index=1) On 2010/12/08 20:59:17, Nirnimesh wrote: > can skip the windex arg Done. http://codereview.chromium.org/5379007/diff/9001/functional/test_utils.py File functional/test_utils.py (right): http://codereview.chromium.org/5379007/diff/9001/functional/test_utils.py#new... functional/test_utils.py:33: On 2010/12/08 20:59:17, Nirnimesh wrote: > Remove the extra blank space Done. http://codereview.chromium.org/5379007/diff/9001/functional/test_utils.py#new... functional/test_utils.py:46: On 2010/12/08 20:59:17, Nirnimesh wrote: > ditto Done. http://codereview.chromium.org/5379007/diff/9001/functional/test_utils.py#new... functional/test_utils.py:56: password: users login password input. On 2010/12/08 20:59:17, Nirnimesh wrote: > document the args Done. http://codereview.chromium.org/5379007/diff/9001/functional/test_utils.py#new... functional/test_utils.py:86: On 2010/12/08 20:59:17, Nirnimesh wrote: > remove extra blank spaces Done. http://codereview.chromium.org/5379007/diff/9001/functional/test_utils.py#new... functional/test_utils.py:91: On 2010/12/08 20:59:17, Nirnimesh wrote: > remove extra blank spaces Done. http://codereview.chromium.org/5379007/diff/9001/functional/test_utils.py#new... functional/test_utils.py:113: On 2010/12/08 20:59:17, Nirnimesh wrote: > remove extra blank spaces Done.
http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py File functional/passwords.py (right): http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:5: no need to add another blank line here http://codereview.chromium.org/5379007/diff/15001/functional/test_utils.py File functional/test_utils.py (right): http://codereview.chromium.org/5379007/diff/15001/functional/test_utils.py#ne... functional/test_utils.py:47: def GoogleAccountsLogin(test, url, username, password, tab_index=0, windex=0): I think the url should be hardcoded. It'll only ever work for the google login url anyway. Remove the url parameter http://codereview.chromium.org/5379007/diff/15001/functional/test_utils.py#ne... functional/test_utils.py:55: url: url where user is trying to login. remove this http://codereview.chromium.org/5379007/diff/15001/functional/test_utils.py#ne... functional/test_utils.py:61: test.NavigateToURL(url, windex, tab_index) hardcode the url here
Hopefully everything is done this time. http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py File functional/passwords.py (right): http://codereview.chromium.org/5379007/diff/2001/functional/passwords.py#newc... functional/passwords.py:5: On 2010/12/09 00:25:14, Nirnimesh wrote: > no need to add another blank line here I already did remove this. Forgot to update. :) http://codereview.chromium.org/5379007/diff/15001/functional/test_utils.py File functional/test_utils.py (right): http://codereview.chromium.org/5379007/diff/15001/functional/test_utils.py#ne... functional/test_utils.py:47: def GoogleAccountsLogin(test, url, username, password, tab_index=0, windex=0): On 2010/12/09 00:25:14, Nirnimesh wrote: > I think the url should be hardcoded. It'll only ever work for the google login > url anyway. Remove the url parameter Done. http://codereview.chromium.org/5379007/diff/15001/functional/test_utils.py#ne... functional/test_utils.py:55: url: url where user is trying to login. On 2010/12/09 00:25:14, Nirnimesh wrote: > remove this Done. http://codereview.chromium.org/5379007/diff/15001/functional/test_utils.py#ne... functional/test_utils.py:61: test.NavigateToURL(url, windex, tab_index) On 2010/12/09 00:25:14, Nirnimesh wrote: > hardcode the url here Done.
http://codereview.chromium.org/5379007/diff/20001/functional/passwords.py File functional/passwords.py (right): http://codereview.chromium.org/5379007/diff/20001/functional/passwords.py#new... functional/passwords.py:102: def testNeverSavePasswords(self): This test fails for me. I don't see the infobar ====================================================================== ERROR: __main__.PasswordTest.testNeverSavePasswords: "Verify that we don't save passwords and delete saved passwords" ---------------------------------------------------------------------- Traceback (most recent call last): File "chrome/test/functional/passwords.py", line 109, in testNeverSavePasswords self.PerformActionOnInfobar('accept', infobar_index=0) File "/Volumes/1/chrome/cr-git/src/chrome/test/functional/../pyautolib/pyauto.py", line 804, in PerformActionOnInfobar self._GetResultFromJSONRequest(cmd_dict, windex=windex) File "/Volumes/1/chrome/cr-git/src/chrome/test/functional/../pyautolib/pyauto.py", line 377, in _GetResultFromJSONRequest raise JSONInterfaceError(ret_dict['error']) JSONInterfaceError: No such infobar at index 0 http://codereview.chromium.org/5379007/diff/20001/functional/passwords.py#new... functional/passwords.py:108: self.WaitForInfobarCount(1) wrap inside self.assertTrue() http://codereview.chromium.org/5379007/diff/20001/functional/passwords.py#new... functional/passwords.py:116: self.WaitForInfobarCount(1, tab_index=1) ditto
http://codereview.chromium.org/5379007/diff/20001/functional/passwords.py File functional/passwords.py (right): http://codereview.chromium.org/5379007/diff/20001/functional/passwords.py#new... functional/passwords.py:102: def testNeverSavePasswords(self): On 2010/12/09 02:42:30, Nirnimesh wrote: > This test fails for me. I don't see the infobar > > ====================================================================== > ERROR: __main__.PasswordTest.testNeverSavePasswords: "Verify that we don't save > passwords and delete saved passwords" > ---------------------------------------------------------------------- > Traceback (most recent call last): > File "chrome/test/functional/passwords.py", line 109, in > testNeverSavePasswords > self.PerformActionOnInfobar('accept', infobar_index=0) > File > "/Volumes/1/chrome/cr-git/src/chrome/test/functional/../pyautolib/pyauto.py", > line 804, in PerformActionOnInfobar > self._GetResultFromJSONRequest(cmd_dict, windex=windex) > File > "/Volumes/1/chrome/cr-git/src/chrome/test/functional/../pyautolib/pyauto.py", > line 377, in _GetResultFromJSONRequest > raise JSONInterfaceError(ret_dict['error']) > JSONInterfaceError: No such infobar at index 0 Might be a bug with Chrome on Linux?!?! It works fine for me on Windows. http://codereview.chromium.org/5379007/diff/20001/functional/passwords.py#new... functional/passwords.py:108: self.WaitForInfobarCount(1) On 2010/12/09 02:42:30, Nirnimesh wrote: > wrap inside self.assertTrue() Done. http://codereview.chromium.org/5379007/diff/20001/functional/passwords.py#new... functional/passwords.py:116: self.WaitForInfobarCount(1, tab_index=1) On 2010/12/09 02:42:30, Nirnimesh wrote: > ditto Done.
http://codereview.chromium.org/5379007/diff/20001/functional/passwords.py File functional/passwords.py (right): http://codereview.chromium.org/5379007/diff/20001/functional/passwords.py#new... functional/passwords.py:102: def testNeverSavePasswords(self): On 2010/12/09 03:30:24, sunandt wrote: > On 2010/12/09 02:42:30, Nirnimesh wrote: > > This test fails for me. I don't see the infobar > > > > ====================================================================== > > ERROR: __main__.PasswordTest.testNeverSavePasswords: "Verify that we don't > save > > passwords and delete saved passwords" > > ---------------------------------------------------------------------- > > Traceback (most recent call last): > > File "chrome/test/functional/passwords.py", line 109, in > > testNeverSavePasswords > > self.PerformActionOnInfobar('accept', infobar_index=0) > > File > > "/Volumes/1/chrome/cr-git/src/chrome/test/functional/../pyautolib/pyauto.py", > > line 804, in PerformActionOnInfobar > > self._GetResultFromJSONRequest(cmd_dict, windex=windex) > > File > > "/Volumes/1/chrome/cr-git/src/chrome/test/functional/../pyautolib/pyauto.py", > > line 377, in _GetResultFromJSONRequest > > raise JSONInterfaceError(ret_dict['error']) > > JSONInterfaceError: No such infobar at index 0 > > Might be a bug with Chrome on Linux?!?! It works fine for me on Windows. This works in linux. I just tried it. Possible failure could be that the login was not successful?
It logs in just fine but I don't see the password infobar. Is it broken on Mac?
Ok, the mac problem was something different.
But this CL breaks passwords.PasswordTest.testDisplayAndSavePasswordInfobar
======================================================================
ERROR: passwords.PasswordTest.testDisplayAndSavePasswordInfobar: "Verify
password infobar displays and able to save password."
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Volumes/1/chrome/cr-git/src/chrome/test/functional/passwords.py", line
86, in testDisplayAndSavePasswordInfobar
test_utils.GoogleAccountsLogin(self, ['url'], username, password)
File "/Volumes/1/chrome/cr-git/src/chrome/test/functional/test_utils.py", line
62, in GoogleAccountsLogin
test.NavigateToURL('https://www.google.com/accounts/', windex, tab_index)
File "/Volumes/1/chrome/cr-git/src/out/10.0.605.0/pyautolib.py", line 1096, in
NavigateToURL
return _pyautolib.PyUITestBase_NavigateToURL(self, *args)
NotImplementedError: Wrong number of arguments for overloaded function
'PyUITestBase_NavigateToURL'.
Possible C/C++ prototypes are:
NavigateToURL(PyUITestBase *,char const *)
NavigateToURL(PyUITestBase *,char const *,int,int)
Rohit, is password infobar broken on mac? Could you please check and confirm?
Fixed.
LGTM. Committing.
Thank you. On Fri, Dec 10, 2010 at 3:05 PM, <nirnimesh@chromium.org> wrote: > LGTM. Committing. > > > http://codereview.chromium.org/5379007/ > |
