Pyauto test for Quasar(Google Talk Extension)

Pyauto test for Google Talk Extension. Also add test to chrome pyauto test suite to run with chrome continuous build.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=123303

[jqian1, 2012-02-06 21:18:51]

Hey Nirnimesh & Frank,

Would you please review the pyauto test for Google Talk Chrome Extension?

Thanks.
Jeanne

[Nirnimesh, 2012-02-06 22:22:44]

A few comments

https://chromiumcodereview.appspot.com/9317067/diff/2001/chrome/test/function...
File chrome/test/functional/gtalk/gtalk_base_test.py (right):

https://chromiumcodereview.appspot.com/9317067/diff/2001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:1: #!/usr/bin/python
Please remove executable permissions from all .crx and .py files

https://chromiumcodereview.appspot.com/9317067/diff/2001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:1: #!/usr/bin/python
Change this line to match other .py files in the repository
#!/usr/bin/env python

https://chromiumcodereview.appspot.com/9317067/diff/2001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:266: def
action_max_timeout_ms(self):
why are you overriding this?

https://chromiumcodereview.appspot.com/9317067/diff/2001/chrome/test/function...
File chrome/test/functional/gtalk/test_basic.py (right):

https://chromiumcodereview.appspot.com/9317067/diff/2001/chrome/test/function...
chrome/test/functional/gtalk/test_basic.py:6: """Basic sanity tests for the
GTalk extension (http://go/quasar)
Remove internal link

https://chromiumcodereview.appspot.com/9317067/diff/2001/chrome/test/function...
chrome/test/functional/gtalk/test_basic.py:50:
self.TriggerBrowserActionById(extension['id'], 0, 0)
use named args. tab_index=0, windex=0

OR
skip them altogether since 0, 0 is the default

https://chromiumcodereview.appspot.com/9317067/diff/2001/chrome/test/function...
chrome/test/functional/gtalk/test_basic.py:100: self.RunInLoginPage(
Are you sure you want to keep username/passwd in the test script?

Other tests keep private data in chrome/test/data/pyauto_private/..

https://chromiumcodereview.appspot.com/9317067/diff/2001/chrome/test/function...
chrome/test/functional/gtalk/test_basic.py:138: # TODO(wud): fix issue where
mole doesn't open when clicked too quickly.
rename todo?

[jqian1, 2012-02-09 01:04:58]

Hey Frank and Nirnimesh,

I modified the code per your comments. I also have one question about how to put
userid/password to a private data folder and I would like to know your
suggestion.

Thanks.
Jeanne

https://chromiumcodereview.appspot.com/9317067/diff/2001/chrome/test/function...
File chrome/test/functional/gtalk/gtalk_base_test.py (right):

https://chromiumcodereview.appspot.com/9317067/diff/2001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:1: #!/usr/bin/python
On 2012/02/06 22:22:44, Nirnimesh wrote:
> Please remove executable permissions from all .crx and .py files

Done.

https://chromiumcodereview.appspot.com/9317067/diff/2001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:1: #!/usr/bin/python
On 2012/02/06 22:22:44, Nirnimesh wrote:
> Change this line to match other .py files in the repository
> #!/usr/bin/env python

Done.

https://chromiumcodereview.appspot.com/9317067/diff/2001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:266: def
action_max_timeout_ms(self):
Removed.
On 2012/02/06 22:22:44, Nirnimesh wrote:
> why are you overriding this?

https://chromiumcodereview.appspot.com/9317067/diff/2001/chrome/test/function...
File chrome/test/functional/gtalk/test_basic.py (right):

https://chromiumcodereview.appspot.com/9317067/diff/2001/chrome/test/function...
chrome/test/functional/gtalk/test_basic.py:6: """Basic sanity tests for the
GTalk extension (http://go/quasar)
On 2012/02/06 22:22:44, Nirnimesh wrote:
> Remove internal link

Done.

https://chromiumcodereview.appspot.com/9317067/diff/2001/chrome/test/function...
chrome/test/functional/gtalk/test_basic.py:50:
self.TriggerBrowserActionById(extension['id'], 0, 0)
On 2012/02/06 22:22:44, Nirnimesh wrote:
> use named args. tab_index=0, windex=0
> 
> OR
> skip them altogether since 0, 0 is the default

Done.

https://chromiumcodereview.appspot.com/9317067/diff/2001/chrome/test/function...
chrome/test/functional/gtalk/test_basic.py:100: self.RunInLoginPage(
Yes, it will be better to protect the userid/password info. Is it ok if I add my
test account to private_tests_info.txt? Or I need to create a folder for gtalk
specifically?

On 2012/02/06 22:22:44, Nirnimesh wrote:
> Are you sure you want to keep username/passwd in the test script?
> 
> Other tests keep private data in chrome/test/data/pyauto_private/..

https://chromiumcodereview.appspot.com/9317067/diff/2001/chrome/test/function...
chrome/test/functional/gtalk/test_basic.py:138: # TODO(wud): fix issue where
mole doesn't open when clicked too quickly.
On 2012/02/06 22:22:44, Nirnimesh wrote:
> rename todo?

Done.

[Nirnimesh, 2012-02-10 01:36:51]

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
File chrome/test/functional/gtalk/gtalk_base_test.py (right):

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:1: #!/usr/bin/env python
All the files still have execute permissions.

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:6: """The base test class for
GTalk tests.
Base of GTalk tests.

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:8: This module contains a set of
common utilities used by unittest for querying
remove 'used by unittest'

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:27: return
pyauto.PyUITest.ExtraChromeFlags(self) + ['--no-sandbox']
why --no-sandbox?
The behavior without the sandbox is quite different from regular use. This has
the potential to miss real bugs.

If it's just because you cannot run the test locally on your local linux machine
without --no-sandbox, try using a chromium build instead of 'Google Chrome' one

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:36:
raw_input('--------------------> ' + text.encode('utf-8'))
why encode to utf-8? you're just going to print

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:40:
self.UninstallGTalkExtension()
why uninstall? This is unexpected. Neither the function name nor the description
mentions it.

Note that every test begins with a clean profile anyway, so you don't need to
uninstall anything.

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:159: def WaitUntilResult(self,
result, func, msg):
Do you really need this helper method? It's a one-liner anyway

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:199: """ %
(self._GetInjectedJs(), statement)
align """ under return

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
File chrome/test/functional/gtalk/test_basic.py (right):

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/test_basic.py:6: """Basic sanity tests for the
GTalk extension
End with .

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/test_basic.py:101:
'document.forms[0].Email.value="quasar.test.1@gmail.com"')
Add a TODO to move it to a private file.

[frankf, 2012-02-10 03:09:36]

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
File chrome/test/functional/gtalk/gtalk_base_test.py (right):

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:47:
self.InstallExtension(extension_path, False)
nit, False is the default value.

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:138: 'chrome-extension://' +
extension['id'] + '/panel.html', mole_index)
'chrome-extension://%s/panel.html' % extension['id'].

Same for others.

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:220: frame_xpath = xpath)
http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Indentation

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:233: if not isinstance(value,
basestring):
you can probably get rid of the if and do str(value) regardless.

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:263: self._injected_js =
open(os.path.dirname(__file__) + '/jsutils.js').read()
Use os.path.join

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
File chrome/test/functional/gtalk/pyauto_gtalk.py (right):

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/pyauto_gtalk.py:2: # Copyright (c) 2011 The
Chromium Authors. All rights reserved.
2012

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
File chrome/test/functional/gtalk/test_basic.py (right):

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/test_basic.py:26: def Prompt(self, text):
Isn't this identical to the base class's?

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/test_basic.py:60: msg='Timed out waiting for
iframes to load.')
msg should be aligned with 'window..

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/test_basic.py:88: '$Click($FindByText($BODY(),
"Sign In"))', '//iframe[1]'))
optional: define '//iframe...' as constant once.

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/test_basic.py:255: tries = 0
don't need the initialization.

[jqian1, 2012-02-15 18:41:27]

Hey Frank and Nirnimesh,

I made the most of the changes per your comments.

One thing I am not sure is how to remove the execution permission of the files.

I ran "chmod -x" against those files and run "gcl upload". Is this the right way
to remove the execute permissions? If not, would you tell me what is the right
way? Thanks.

Thanks.
Jeanne

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
File chrome/test/functional/gtalk/gtalk_base_test.py (right):

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:1: #!/usr/bin/env python
On 2012/02/10 01:36:51, Nirnimesh wrote:
> All the files still have execute permissions.
I ran "chmod -x" against those files and run "gcl upload". Is this the right way
to remove the execute permissions? If not, would you tell me what is the right
way? Thanks.

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:6: """The base test class for
GTalk tests.
On 2012/02/10 01:36:51, Nirnimesh wrote:
> Base of GTalk tests.

Done.

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:8: This module contains a set of
common utilities used by unittest for querying
On 2012/02/10 01:36:51, Nirnimesh wrote:
> remove 'used by unittest'

Done.

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:27: return
pyauto.PyUITest.ExtraChromeFlags(self) + ['--no-sandbox']
I will remove "--no-sandbox" when I check in the code. I still need that flag to
run pyauto tests on my local machine.

On 2012/02/10 01:36:51, Nirnimesh wrote:
> why --no-sandbox?
> The behavior without the sandbox is quite different from regular use. This has
> the potential to miss real bugs.
> 
> If it's just because you cannot run the test locally on your local linux
machine
> without --no-sandbox, try using a chromium build instead of 'Google Chrome'
one

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:36:
raw_input('--------------------> ' + text.encode('utf-8'))
Removed "utf8"
On 2012/02/10 01:36:51, Nirnimesh wrote:
> why encode to utf-8? you're just going to print

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:40:
self.UninstallGTalkExtension()
I removed the uninstall part.thanks.
On 2012/02/10 01:36:51, Nirnimesh wrote:
> why uninstall? This is unexpected. Neither the function name nor the
description
> mentions it.
> 
> Note that every test begins with a clean profile anyway, so you don't need to
> uninstall anything.

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:138: 'chrome-extension://' +
extension['id'] + '/panel.html', mole_index)
On 2012/02/10 03:09:36, frankf wrote:
> 'chrome-extension://%s/panel.html' % extension['id'].
> 
> Same for others.

Done.

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:159: def WaitUntilResult(self,
result, func, msg):
I would prefer to having this helper method for now. It still make the code a
little bit neat.
On 2012/02/10 01:36:51, Nirnimesh wrote:
> Do you really need this helper method? It's a one-liner anyway

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:199: """ %
(self._GetInjectedJs(), statement)
On 2012/02/10 01:36:51, Nirnimesh wrote:
> align """ under return

Done.

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:220: frame_xpath = xpath)
On 2012/02/10 03:09:36, frankf wrote:
> http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Indentation

Done.

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/gtalk_base_test.py:263: self._injected_js =
open(os.path.dirname(__file__) + '/jsutils.js').read()
On 2012/02/10 03:09:36, frankf wrote:
> Use os.path.join

Done.

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
File chrome/test/functional/gtalk/pyauto_gtalk.py (right):

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/pyauto_gtalk.py:2: # Copyright (c) 2011 The
Chromium Authors. All rights reserved.
On 2012/02/10 03:09:36, frankf wrote:
> 2012

Done.

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
File chrome/test/functional/gtalk/test_basic.py (right):

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/test_basic.py:6: """Basic sanity tests for the
GTalk extension
On 2012/02/10 01:36:51, Nirnimesh wrote:
> End with .

Done.

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/test_basic.py:26: def Prompt(self, text):
Yes, thanks for pointing this out. I removed it from this file.
On 2012/02/10 03:09:36, frankf wrote:
> Isn't this identical to the base class's?

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/test_basic.py:60: msg='Timed out waiting for
iframes to load.')
On 2012/02/10 03:09:36, frankf wrote:
> msg should be aligned with 'window..

Done.

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/test_basic.py:101:
'document.forms[0].Email.value="quasar.test.1@gmail.com"')
On 2012/02/10 01:36:51, Nirnimesh wrote:
> Add a TODO to move it to a private file.

I have moved the userid/password to a private file.

https://chromiumcodereview.appspot.com/9317067/diff/8001/chrome/test/function...
chrome/test/functional/gtalk/test_basic.py:255: tries = 0
On 2012/02/10 03:09:36, frankf wrote:
> don't need the initialization.

Done.

[Nirnimesh, 2012-02-16 01:04:38]

LGTM, after you add your test 'gtalk.test_basic' to PYAUTO_TESTS, so that it can
actually be fired up by the buildbot slaves.

[jqian1, 2012-02-22 00:49:08]

Hey Nirnimesh,

I put the changes in pyauto_test to this big cl.

Jeanne

[Nirnimesh, 2012-02-22 02:13:42]

How come the description now shows as "Hey Frank and Nirnimesh .....
"""

Please use a real description, something that explains this CL.

[frankf, 2012-02-22 03:27:47]

LGTM after description fix.

[jqian1, 2012-02-23 05:47:47]

On 2012/02/22 03:27:47, frankf wrote:
> LGTM after description fix.

Thanks a lot. I fixed the description. 
I do not think I have the commit permission. Can you guys check in the code for
me?

Thanks.
Jeanne

[commit-bot: I haz the power, 2012-02-23 07:08:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jqian@google.com/9317067/19001

[commit-bot: I haz the power, 2012-02-23 07:08:18]

Failed to request the patch to try. Please note that binary filesare still
unsupported at the moment, this is being worked on.

Thanks for your patience.

HTTP Error 302: Found

[Nirnimesh, 2012-02-23 07:09:22]

LGTM still.

BTW, Checking the 'commit' checkbox allows you to commit even without being a
committer (after approval from a committer). I've checked it just now.

[Nirnimesh, 2012-02-23 07:10:41]

Ah.. ok. binary files are not supported yet by the commit queue. I'll submit it
by hand on your behalf. The codereview server does not keep copies of binary
files, so could you please provide the .crx to me some other way?

[jqian1, 2012-02-23 07:16:03]

I just copied the .crx file to /home/jqian/www/gtalk.

Jeanne

On Wed, Feb 22, 2012 at 11:10 PM, <nirnimesh@chromium.org> wrote:

> Ah.. ok. binary files are not supported yet by the commit queue. I'll
> submit it
> by hand on your behalf. The codereview server does not keep copies of
> binary
> files, so could you please provide the .crx to me some other way?
>
>
http://codereview.chromium.**org/9317067/<http://codereview.chromium.org/9317...
>

[Nirnimesh, 2012-02-23 19:58:33]

Committed as:  https://src.chromium.org/viewvc/chrome?view=rev&revision=123303