Modify power_LoadTest Extension to pull in test parameters.

Modify power_LoadTest Extension to pull in test parameters.

Use a new content script to pull in the test parameters.

BUG=chromium-os:12426, chromium-os:6944
TEST=run power_LoadTest with 5 minute duration, and also
full 9 hour duration. Verify both work.

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

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=7bcd700

[Sameer Nanda, 2010-07-08 20:19:52]

LGTM w/ nits

http://codereview.chromium.org/2844047/diff/2001/3003
File client/site_tests/power_LoadTest/extension/background.html (right):

http://codereview.chromium.org/2844047/diff/2001/3003#newcode21
client/site_tests/power_LoadTest/extension/background.html:21: function dump() {
orphan function?

http://codereview.chromium.org/2844047/diff/2001/3005
File client/site_tests/power_LoadTest/extension/testparams.js (right):

http://codereview.chromium.org/2844047/diff/2001/3005#newcode15
client/site_tests/power_LoadTest/extension/testparams.js:15:
document.getElementById('myCustomEventDiv').addEventListener('myCustomEvent',
function() {
> 80 columns

[Benson Leung, 2010-07-08 20:54:32]

Fixed nits.

http://codereview.chromium.org/2844047/diff/2001/3003
File client/site_tests/power_LoadTest/extension/background.html (right):

http://codereview.chromium.org/2844047/diff/2001/3003#newcode21
client/site_tests/power_LoadTest/extension/background.html:21: function dump() {
On 2010/07/08 20:19:52, sameer wrote:
> orphan function?

Removed.

http://codereview.chromium.org/2844047/diff/2001/3005
File client/site_tests/power_LoadTest/extension/testparams.js (right):

http://codereview.chromium.org/2844047/diff/2001/3005#newcode15
client/site_tests/power_LoadTest/extension/testparams.js:15:
document.getElementById('myCustomEventDiv').addEventListener('myCustomEvent',
function() {
On 2010/07/08 20:19:52, sameer wrote:
> > 80 columns

Done.

[Sameer Nanda, 2010-07-08 20:56:06]

LGTM. Thanks!

[DaleCurtis, 2010-07-08 21:01:26]

Comment from quick once over, will detail more shortly.

http://codereview.chromium.org/2844047/diff/2001/3003
File client/site_tests/power_LoadTest/extension/background.html (right):

http://codereview.chromium.org/2844047/diff/2001/3003#newcode36
client/site_tests/power_LoadTest/extension/background.html:36: for (var i = 0; i
< tasks.length; i++) {
Some more comments here would be helpful. Also, is it reasonable to expect tasks
to always complete in fixed intervals? Slightly off topic, but how accurate is
setTimeout? => Results missed because send_status executes before task
completion. Normally some sort of callback should be used instead.

[DaleCurtis, 2010-07-08 21:26:47]

These two are up to you. I think you can lose some code if you go this way and
I'm always a fan of less code :)

http://codereview.chromium.org/2844047/diff/10001/11005
File client/site_tests/power_LoadTest/extension/testparams.js (right):

http://codereview.chromium.org/2844047/diff/10001/11005#newcode18
client/site_tests/power_LoadTest/extension/testparams.js:18: var eventData =
document.getElementById('myCustomEventDiv').innerText;
In combination with my comment on testparams.html you would use .value here
instead of innerText.

http://codereview.chromium.org/2844047/diff/10001/11008
File client/site_tests/power_LoadTest/testparams.html (right):

http://codereview.chromium.org/2844047/diff/10001/11008#newcode32
client/site_tests/power_LoadTest/testparams.html:32: createDiv('test_time_ms',
test_time_ms, false);
Normally you would put this kind of content into a <input type="hidden"
value="<value>" id="<id>"/> field. Then you wouldn't need to do a style=display:
none, etc.

[Benson Leung, 2011-02-28 08:07:36]

Hi Sameer,

PTAL.

[Sameer Nanda, 2011-02-28 22:11:34]

Also had a chat offline with Benson regarding passing the parameters from test
to the extension.

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
File client/site_tests/power_LoadTest/control (right):

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
client/site_tests/power_LoadTest/control:28: # TODO (bleung): Find a way to do
automatic Facebook in perf test account.
typo: in -> login

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
client/site_tests/power_LoadTest/control:37:
creds='power.loadtest@gmail.com:power_LoadTest')
probably better to pick these creds up from a file on disk or autotest labels
instead of hardcoding.

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
File client/site_tests/power_LoadTest/extension/background.html (right):

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
client/site_tests/power_LoadTest/extension/background.html:56: "from the
extension");
remove logging?

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
client/site_tests/power_LoadTest/extension/background.html:75: }
add else clause with error log?

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
File client/site_tests/power_LoadTest/extension/testparams.js (right):

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
client/site_tests/power_LoadTest/extension/testparams.js:1: // Copyright (c)
2010 The Chromium OS Authors. All rights reserved.
change year to 2011

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
client/site_tests/power_LoadTest/extension/testparams.js:23: scroll_interval_ms
= document.getElementById('scroll_interval_ms').innerText;
 > 80 columns

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
File client/site_tests/power_LoadTest/params.js (right):

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
client/site_tests/power_LoadTest/params.js:1: // Copyright (c) 2010 The Chromium
OS Authors. All rights reserved.
change year to 2011

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
File client/site_tests/power_LoadTest/testparams.html (right):

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
client/site_tests/power_LoadTest/testparams.html:2: Copyright (c) 2010 The
Chromium OS Authors. All rights reserved.
change year to 2011

[Benson Leung, 2011-03-01 02:16:19]

Fixed write params too from power_LoadTest.py.

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
File client/site_tests/power_LoadTest/control (right):

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
client/site_tests/power_LoadTest/control:28: # TODO (bleung): Find a way to do
automatic Facebook in perf test account.
On 2011/02/28 22:11:34, Sameer Nanda wrote:
> typo: in -> login

Done.

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
File client/site_tests/power_LoadTest/extension/background.html (right):

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
client/site_tests/power_LoadTest/extension/background.html:56: "from the
extension");
On 2011/02/28 22:11:34, Sameer Nanda wrote:
> remove logging?

Done.

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
client/site_tests/power_LoadTest/extension/background.html:75: }
On 2011/02/28 22:11:34, Sameer Nanda wrote:
> add else clause with error log?

Done.

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
File client/site_tests/power_LoadTest/extension/testparams.js (right):

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
client/site_tests/power_LoadTest/extension/testparams.js:1: // Copyright (c)
2010 The Chromium OS Authors. All rights reserved.
On 2011/02/28 22:11:34, Sameer Nanda wrote:
> change year to 2011

Done.

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
client/site_tests/power_LoadTest/extension/testparams.js:23: scroll_interval_ms
= document.getElementById('scroll_interval_ms').innerText;
On 2011/02/28 22:11:34, Sameer Nanda wrote:
>  > 80 columns

Done.

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
File client/site_tests/power_LoadTest/params.js (right):

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
client/site_tests/power_LoadTest/params.js:1: // Copyright (c) 2010 The Chromium
OS Authors. All rights reserved.
On 2011/02/28 22:11:34, Sameer Nanda wrote:
> change year to 2011

Done.

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
File client/site_tests/power_LoadTest/testparams.html (right):

http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
client/site_tests/power_LoadTest/testparams.html:2: Copyright (c) 2010 The
Chromium OS Authors. All rights reserved.
On 2011/02/28 22:11:34, Sameer Nanda wrote:
> change year to 2011

Done.

[DaleCurtis, 2011-03-01 02:38:00]

Good stuff. LGTM.

On 2011/03/01 02:16:19, Benson Leung wrote:
> Fixed write params too from power_LoadTest.py.
> 
>
http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
> File client/site_tests/power_LoadTest/control (right):
> 
>
http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
> client/site_tests/power_LoadTest/control:28: # TODO (bleung): Find a way to do
> automatic Facebook in perf test account.
> On 2011/02/28 22:11:34, Sameer Nanda wrote:
> > typo: in -> login
> 
> Done.
> 
>
http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
> File client/site_tests/power_LoadTest/extension/background.html (right):
> 
>
http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
> client/site_tests/power_LoadTest/extension/background.html:56: "from the
> extension");
> On 2011/02/28 22:11:34, Sameer Nanda wrote:
> > remove logging?
> 
> Done.
> 
>
http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
> client/site_tests/power_LoadTest/extension/background.html:75: }
> On 2011/02/28 22:11:34, Sameer Nanda wrote:
> > add else clause with error log?
> 
> Done.
> 
>
http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
> File client/site_tests/power_LoadTest/extension/testparams.js (right):
> 
>
http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
> client/site_tests/power_LoadTest/extension/testparams.js:1: // Copyright (c)
> 2010 The Chromium OS Authors. All rights reserved.
> On 2011/02/28 22:11:34, Sameer Nanda wrote:
> > change year to 2011
> 
> Done.
> 
>
http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
> client/site_tests/power_LoadTest/extension/testparams.js:23:
scroll_interval_ms
> = document.getElementById('scroll_interval_ms').innerText;
> On 2011/02/28 22:11:34, Sameer Nanda wrote:
> >  > 80 columns
> 
> Done.
> 
>
http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
> File client/site_tests/power_LoadTest/params.js (right):
> 
>
http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
> client/site_tests/power_LoadTest/params.js:1: // Copyright (c) 2010 The
Chromium
> OS Authors. All rights reserved.
> On 2011/02/28 22:11:34, Sameer Nanda wrote:
> > change year to 2011
> 
> Done.
> 
>
http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
> File client/site_tests/power_LoadTest/testparams.html (right):
> 
>
http://codereview.chromium.org/2844047/diff/42010/client/site_tests/power_Loa...
> client/site_tests/power_LoadTest/testparams.html:2: Copyright (c) 2010 The
> Chromium OS Authors. All rights reserved.
> On 2011/02/28 22:11:34, Sameer Nanda wrote:
> > change year to 2011
> 
> Done.

[DaleCurtis, 2011-03-01 18:43:20]

http://codereview.chromium.org/2844047/diff/49001/client/site_tests/power_Loa...
File client/site_tests/power_LoadTest/power_LoadTest.py (right):

http://codereview.chromium.org/2844047/diff/49001/client/site_tests/power_Loa...
client/site_tests/power_LoadTest/power_LoadTest.py:228: def cleanup(self):
While looking at another test I realized we don't call self._testServer.stop()
here. Due to the way Autotest calls tests, it's not a big deal, but probably
worth adding.

[Sameer Nanda, 2011-03-01 19:54:06]

LTGM!  Mind updating README.txt with a short blurb about how to manually start
the test with updated parameters?

[Benson Leung, 2011-03-03 04:47:25]

Readme updated. Addressed review feedback.

I've also filed this bug for a future enhancement :
http://code.google.com/p/chromium-os/issues/detail?id=12706

For another day.

http://codereview.chromium.org/2844047/diff/49001/client/site_tests/power_Loa...
File client/site_tests/power_LoadTest/power_LoadTest.py (right):

http://codereview.chromium.org/2844047/diff/49001/client/site_tests/power_Loa...
client/site_tests/power_LoadTest/power_LoadTest.py:228: def cleanup(self):
On 2011/03/01 18:43:20, dalec wrote:
> While looking at another test I realized we don't call self._testServer.stop()
> here. Due to the way Autotest calls tests, it's not a big deal, but probably
> worth adding.

Done.

[Benson Leung, 2011-03-04 07:00:30]

Also increasing to 11 loops instead of 9.