Fix and enable testPasswordAutofilledInIncognito.

Fix and enable testPasswordAutofilledInIncognito.

TEST=Tested on chromium bots and passed.
BUG=none
NOTRY=true
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=138289

[Ilya Sherman, 2012-05-18 03:14:27]

LGTM with nits addressed.  (Please wait for Nirnimesh's review as well prior to
committing.)  Thanks :)

http://codereview.chromium.org/10383241/diff/1/chrome/test/functional/passwor...
File chrome/test/functional/passwords.py (right):

http://codereview.chromium.org/10383241/diff/1/chrome/test/functional/passwor...
chrome/test/functional/passwords.py:221: SIGNON_REALM =
'https://accounts.google.com/'
nit: It would be good if you could compute this from the URL constant declared
above (by trimming the path from the url).  If not, I think you should declare
this as HOSTNAME or DOMAIN or REALM above, with the other url constants.

http://codereview.chromium.org/10383241/diff/1/chrome/test/functional/passwor...
chrome/test/functional/passwords.py:222: ORIGIN_URL =
'https://accounts.google.com/ServiceLogin'
nit: You can use URL for this.

http://codereview.chromium.org/10383241/diff/1/chrome/test/functional/passwor...
chrome/test/functional/passwords.py:224: PASSWORD_ELEM = 'Passwd'
nit: These should probably be defined with the other constants at the top of the
class, since they are used by other tests as well, and depend on the specific
login page.

http://codereview.chromium.org/10383241/diff/1/chrome/test/functional/passwor...
chrome/test/functional/passwords.py:225: ACTION_TARGET =
'https://accounts.google.com/'
nit: Same comment as for the sign-on realm.

[Nirnimesh, 2012-05-18 03:20:23]

LGTM provided you fix the casing.

http://codereview.chromium.org/10383241/diff/1/chrome/test/functional/passwor...
File chrome/test/functional/passwords.py (right):

http://codereview.chromium.org/10383241/diff/1/chrome/test/functional/passwor...
chrome/test/functional/passwords.py:219: USERNAME = 'test@google.com'
please use lowercase for local vars. CAPS looks weird.
You can have CAPS if these are used by other tests as well, in which case make
these class vars. But it doesn't look like these are used elsewhere.

[dyu1, 2012-05-18 23:44:37]

Can you have a quick look at the changes?

http://codereview.chromium.org/10383241/diff/1/chrome/test/functional/passwor...
File chrome/test/functional/passwords.py (right):

http://codereview.chromium.org/10383241/diff/1/chrome/test/functional/passwor...
chrome/test/functional/passwords.py:219: USERNAME = 'test@google.com'
On 2012/05/18 03:20:23, Nirnimesh wrote:
> please use lowercase for local vars. CAPS looks weird.
> You can have CAPS if these are used by other tests as well, in which case make
> these class vars. But it doesn't look like these are used elsewhere.

Done.

http://codereview.chromium.org/10383241/diff/1/chrome/test/functional/passwor...
chrome/test/functional/passwords.py:221: SIGNON_REALM =
'https://accounts.google.com/'
On 2012/05/18 03:14:27, Ilya Sherman wrote:
> nit: It would be good if you could compute this from the URL constant declared
> above (by trimming the path from the url).  If not, I think you should declare
> this as HOSTNAME or DOMAIN or REALM above, with the other url constants.

Done.

http://codereview.chromium.org/10383241/diff/1/chrome/test/functional/passwor...
chrome/test/functional/passwords.py:222: ORIGIN_URL =
'https://accounts.google.com/ServiceLogin'
On 2012/05/18 03:14:27, Ilya Sherman wrote:
> nit: You can use URL for this.

Done.

http://codereview.chromium.org/10383241/diff/1/chrome/test/functional/passwor...
chrome/test/functional/passwords.py:224: PASSWORD_ELEM = 'Passwd'
On 2012/05/18 03:14:27, Ilya Sherman wrote:
> nit: These should probably be defined with the other constants at the top of
the
> class, since they are used by other tests as well, and depend on the specific
> login page.

Done.

http://codereview.chromium.org/10383241/diff/1/chrome/test/functional/passwor...
chrome/test/functional/passwords.py:225: ACTION_TARGET =
'https://accounts.google.com/'
On 2012/05/18 03:14:27, Ilya Sherman wrote:
> nit: Same comment as for the sign-on realm.

Done.

[Ilya Sherman, 2012-05-19 01:08:56]

http://codereview.chromium.org/10383241/diff/8001/chrome/test/functional/pass...
File chrome/test/functional/passwords.py (right):

http://codereview.chromium.org/10383241/diff/8001/chrome/test/functional/pass...
chrome/test/functional/passwords.py:24: HOSTNAME = '/'.join(SLASHPARTS[:3]) +
'/'
Hmm, you should probably use something like the urlparse module [1] to parse out
the hostname, rather than doing it manually.  Or, if you prefer to do it
manually, I would just go ahead and hardcode it as
'https://accounts.google.com/' -- anyone editing the file will easily be able to
tell that they need to update that as well, since it'll be right next to the
other URL constants.
[1] http://docs.python.org/library/urlparse.html

http://codereview.chromium.org/10383241/diff/8001/chrome/test/functional/pass...
chrome/test/functional/passwords.py:242: msg='Password creds did not autofill in
incognito mode.')
nit: indent one more space

http://codereview.chromium.org/10383241/diff/8001/chrome/test/functional/pass...
chrome/test/functional/passwords.py:280: 'Passwd': 'test12345'}
nit: Please update these to use the named constants as well.

[dyu1, 2012-05-19 02:21:47]

http://codereview.chromium.org/10383241/diff/8001/chrome/test/functional/pass...
File chrome/test/functional/passwords.py (right):

http://codereview.chromium.org/10383241/diff/8001/chrome/test/functional/pass...
chrome/test/functional/passwords.py:24: HOSTNAME = '/'.join(SLASHPARTS[:3]) +
'/'
On 2012/05/19 01:08:57, Ilya Sherman wrote:
> Hmm, you should probably use something like the urlparse module [1] to parse
out
> the hostname, rather than doing it manually.  Or, if you prefer to do it
> manually, I would just go ahead and hardcode it as
> 'https://accounts.google.com/' -- anyone editing the file will easily be able
to
> tell that they need to update that as well, since it'll be right next to the
> other URL constants.
> [1] http://docs.python.org/library/urlparse.html

Done.

http://codereview.chromium.org/10383241/diff/8001/chrome/test/functional/pass...
chrome/test/functional/passwords.py:242: msg='Password creds did not autofill in
incognito mode.')
On 2012/05/19 01:08:57, Ilya Sherman wrote:
> nit: indent one more space

Done.

http://codereview.chromium.org/10383241/diff/8001/chrome/test/functional/pass...
chrome/test/functional/passwords.py:280: 'Passwd': 'test12345'}
On 2012/05/19 01:08:57, Ilya Sherman wrote:
> nit: Please update these to use the named constants as well.

Done.

[Ilya Sherman, 2012-05-19 07:47:34]

LGTM with nits

http://codereview.chromium.org/10383241/diff/8001/chrome/test/functional/pass...
File chrome/test/functional/passwords.py (right):

http://codereview.chromium.org/10383241/diff/8001/chrome/test/functional/pass...
chrome/test/functional/passwords.py:280: 'Passwd': 'test12345'}
On 2012/05/19 02:21:47, dyu1 wrote:
> On 2012/05/19 01:08:57, Ilya Sherman wrote:
> > nit: Please update these to use the named constants as well.
> 
> Done.

I mean the 'Email' and 'Passwd' strings, though these are also fine to
canonicalize.

http://codereview.chromium.org/10383241/diff/13001/chrome/test/functional/pas...
File chrome/test/functional/passwords.py (right):

http://codereview.chromium.org/10383241/diff/13001/chrome/test/functional/pas...
chrome/test/functional/passwords.py:22: HOSTNAME = 'https://'+urlparse(URL)[1]
nit: HOSTNAME = 'https://' + urlparse(URL).netloc

[dyu1, 2012-05-22 17:31:38]

http://codereview.chromium.org/10383241/diff/13001/chrome/test/functional/pas...
File chrome/test/functional/passwords.py (right):

http://codereview.chromium.org/10383241/diff/13001/chrome/test/functional/pas...
chrome/test/functional/passwords.py:22: HOSTNAME = 'https://'+urlparse(URL)[1]
On 2012/05/19 07:47:34, Ilya Sherman wrote:
> nit: HOSTNAME = 'https://' + urlparse(URL).netloc

Done.