External Extension for power_LoadTest

External Extension for power_LoadTest

Enable the use of an external extension so we no longer spawn another instance
of chrome to run this test. The external extension is loaded when chrome
logs in.

BUG=none
TEST=power_LoadTest

Signed-off-by: Benson Leung <bleung@chromium.org>;

[Benson Leung, 2010-07-01 08:43:05]

---------- Forwarded message ----------
From: <bleung@chromium.org>
Date: Wed, Jun 30, 2010 at 10:07 PM
Subject: External Extension for power_LoadTest (issue2872027)
To: davidjames@chromium.org
Cc: chromium-os-reviews@chromium.org,
sosa+cc@chromium.org<sosa%2Bcc@chromium.org>,
seano@chromium.org, ericli@chromium.org,
petkov+cc@chromium.org<petkov%2Bcc@chromium.org>


Reviewers: davidjames,

Description:
External Extension for power_LoadTest

Enable the use of an external extension so we no longer spawn another
instance
of chrome to run this test. The external extension is loaded when chrome
logs in.

Signed-off-by: Benson Leung <bleung@chromium.org>

Please review this at http://codereview.chromium.org/2872027/show

SVN Base: ssh://gitrw.chromium.org/autotest.git

Affected files:
 A client/site_tests/power_LoadTest/extension.crx
 A client/site_tests/power_LoadTest/extension.pem
 A client/site_tests/power_LoadTest/external_extensions.json
 M client/site_tests/power_LoadTest/power_LoadTest.py

[davidjames, 2010-07-01 14:27:09]

Looks good w/nit. Interested in hearing Sameer's feedback though before we push.

http://codereview.chromium.org/2872027/diff/3001/4004
File client/site_tests/power_LoadTest/power_LoadTest.py (right):

http://codereview.chromium.org/2872027/diff/3001/4004#newcode27
client/site_tests/power_LoadTest/power_LoadTest.py:27: check_network=False,
loop_time=3600, loop_count=1,
Should this be check_network=True?

[ericli, 2010-07-01 15:09:12]

I like to include Dale Curtis in the discussion of check_network argument.
We had some interesting discussion while working on the ethernet backchannel
for this test.

On Thu, Jul 1, 2010 at 7:27 AM, <davidjames@chromium.org> wrote:

> Looks good w/nit. Interested in hearing Sameer's feedback though before we
> push.
>
>
> http://codereview.chromium.org/2872027/diff/3001/4004
> File client/site_tests/power_LoadTest/power_LoadTest.py (right):
>
> http://codereview.chromium.org/2872027/diff/3001/4004#newcode27
> client/site_tests/power_LoadTest/power_LoadTest.py:27:
> check_network=False, loop_time=3600, loop_count=1,
> Should this be check_network=True?
>
>
> http://codereview.chromium.org/2872027/show
>



-- 
Eric Li
李咏竹
Google Kirkland

[DaleCurtis, 2010-07-01 17:04:04]

http://codereview.chromium.org/2872027/diff/3001/4004
File client/site_tests/power_LoadTest/power_LoadTest.py (right):

http://codereview.chromium.org/2872027/diff/3001/4004#newcode27
client/site_tests/power_LoadTest/power_LoadTest.py:27: check_network=False,
loop_time=3600, loop_count=1,
Agreed. In the lab environment we use a separate control file which overrides
check_network and sets the value to False. I think it's best left as True for
developers who are running the test via run_tests or run_remote_tests. It would
suck to kick this off and then come back hours later and realize the results
were useless because you forgot to unplug.

On 2010/07/01 14:27:10, davidjames wrote:
> Should this be check_network=True?

[Sameer Nanda, 2010-07-01 17:43:26]

Looks good in general, but please address comments before checking in.

http://codereview.chromium.org/2872027/diff/3001/4004
File client/site_tests/power_LoadTest/power_LoadTest.py (right):

http://codereview.chromium.org/2872027/diff/3001/4004#newcode24
client/site_tests/power_LoadTest/power_LoadTest.py:24: pass
why are we doing this? flakiness? please add a comment since its not obvious.

http://codereview.chromium.org/2872027/diff/3001/4004#newcode27
client/site_tests/power_LoadTest/power_LoadTest.py:27: check_network=False,
loop_time=3600, loop_count=1,
On 2010/07/01 17:04:05, dalec wrote:
> Agreed. In the lab environment we use a separate control file which overrides
> check_network and sets the value to False. I think it's best left as True for
> developers who are running the test via run_tests or run_remote_tests. It
would
> suck to kick this off and then come back hours later and realize the results
> were useless because you forgot to unplug.
> 
> On 2010/07/01 14:27:10, davidjames wrote:
> > Should this be check_network=True?
> 
> 
Agreed as well. Lets keep check_network as True.

http://codereview.chromium.org/2872027/diff/3001/4004#newcode135
client/site_tests/power_LoadTest/power_LoadTest.py:135:
self.login(self.username, self.password)
what should happen if self.auto_login is False? Fail the test?

http://codereview.chromium.org/2872027/diff/3001/4004#newcode143
client/site_tests/power_LoadTest/power_LoadTest.py:143: os.mkdir(self.srcdir)
The setup method should no longer be needed since powerd does the required xset
stuff now.

http://codereview.chromium.org/2872027/diff/3001/4004#newcode157
client/site_tests/power_LoadTest/power_LoadTest.py:157: if
site_login.logged_in():
may want to add a comment here explaining something along the lines that the act
of logging in launches chrome with power extension.

http://codereview.chromium.org/2872027/diff/3001/4004#newcode161
client/site_tests/power_LoadTest/power_LoadTest.py:161:
self.login(self.username, self.password)
probably cleaner to move logout/login to the top of the loop and remove it from
initialize.

[Benson Leung, 2010-07-01 19:47:08]

A few of my replies.

http://codereview.chromium.org/2872027/diff/3001/4004
File client/site_tests/power_LoadTest/power_LoadTest.py (right):

http://codereview.chromium.org/2872027/diff/3001/4004#newcode24
client/site_tests/power_LoadTest/power_LoadTest.py:24: pass
On 2010/07/01 17:43:26, sameer wrote:
> why are we doing this? flakiness? please add a comment since its not obvious.

UITest sets up a local authserver in initalize. I've overridden their initialize
to not do this so we can still access the 'net. UITest's ensure_login_complete
spins waiting for the auth server to startup. 

I'll add a comment.

http://codereview.chromium.org/2872027/diff/3001/4004#newcode27
client/site_tests/power_LoadTest/power_LoadTest.py:27: check_network=False,
loop_time=3600, loop_count=1,
On 2010/07/01 17:04:05, dalec wrote:
> Agreed. In the lab environment we use a separate control file which overrides
> check_network and sets the value to False. I think it's best left as True for
> developers who are running the test via run_tests or run_remote_tests. It
would
> suck to kick this off and then come back hours later and realize the results
> were useless because you forgot to unplug.
> 
> On 2010/07/01 14:27:10, davidjames wrote:
> > Should this be check_network=True?
> 
> 

Hi Dale,

Speaking of the control file for this test, do you anticipate modifying the loop
time of this test at all?

The reason I ask is because now that we are pre-packing the extension, we can no
longer simply change the extension's param file from within power_LoadTest.py.

I have some ideas about how to make it dynamic, but I want to feel out if it's
going to be an issue.

Right now each loop is an hour for both loadtest and audiovideo.

[Dale Curtis, 2010-07-01 20:51:30]

Discussed offline. The gist was, not at this time, but we would like to retain
the capability to adjust the loop time. For now it's okay being static, but
either the ability to make it dynamic or a switch to use a 5minute version of
the extension for short-run tests in the future should happen.

On 2010/07/01 19:47:08, Benson Leung wrote:
> A few of my replies.
> 
> http://codereview.chromium.org/2872027/diff/3001/4004
> File client/site_tests/power_LoadTest/power_LoadTest.py (right):
> 
> http://codereview.chromium.org/2872027/diff/3001/4004#newcode24
> client/site_tests/power_LoadTest/power_LoadTest.py:24: pass
> On 2010/07/01 17:43:26, sameer wrote:
> > why are we doing this? flakiness? please add a comment since its not
obvious.
> 
> UITest sets up a local authserver in initalize. I've overridden their
initialize
> to not do this so we can still access the 'net. UITest's ensure_login_complete
> spins waiting for the auth server to startup. 
> 
> I'll add a comment.
> 
> http://codereview.chromium.org/2872027/diff/3001/4004#newcode27
> client/site_tests/power_LoadTest/power_LoadTest.py:27: check_network=False,
> loop_time=3600, loop_count=1,
> On 2010/07/01 17:04:05, dalec wrote:
> > Agreed. In the lab environment we use a separate control file which
overrides
> > check_network and sets the value to False. I think it's best left as True
for
> > developers who are running the test via run_tests or run_remote_tests. It
> would
> > suck to kick this off and then come back hours later and realize the results
> > were useless because you forgot to unplug.
> > 
> > On 2010/07/01 14:27:10, davidjames wrote:
> > > Should this be check_network=True?
> > 
> > 
> 
> Hi Dale,
> 
> Speaking of the control file for this test, do you anticipate modifying the
loop
> time of this test at all?
> 
> The reason I ask is because now that we are pre-packing the extension, we can
no
> longer simply change the extension's param file from within power_LoadTest.py.
> 
> I have some ideas about how to make it dynamic, but I want to feel out if it's
> going to be an issue.
> 
> Right now each loop is an hour for both loadtest and audiovideo.

[Benson Leung, 2010-07-01 22:24:38]

http://codereview.chromium.org/2872027/diff/3001/4004
File client/site_tests/power_LoadTest/power_LoadTest.py (right):

http://codereview.chromium.org/2872027/diff/3001/4004#newcode143
client/site_tests/power_LoadTest/power_LoadTest.py:143: os.mkdir(self.srcdir)
On 2010/07/01 17:43:26, sameer wrote:
> The setup method should no longer be needed since powerd does the required
xset
> stuff now.

Done.

[Benson Leung, 2010-07-01 22:28:49]

http://codereview.chromium.org/2872027/diff/3001/4004
File client/site_tests/power_LoadTest/power_LoadTest.py (right):

http://codereview.chromium.org/2872027/diff/3001/4004#newcode135
client/site_tests/power_LoadTest/power_LoadTest.py:135:
self.login(self.username, self.password)
On 2010/07/01 17:43:26, sameer wrote:
> what should happen if self.auto_login is False? Fail the test?

Actually, we don't need to auto login at all. Let run_once log us in first time
into the loop.

[Sameer Nanda, 2010-07-01 22:34:38]

LGTM. Thanks!