Add boot time tests to nightly suite, and run multiple iterations

Add boot time tests to nightly suite, and run multiple iterations

BUG=none
TEST=use run_remote_tests.sh on suite_Nightly, and observe new results

[jrbarnette, 2010-06-21 17:02:31]

This change will cause the nightly suite to run platform_BootPerfServer 4 times
per test client.

This is a preliminary change, to start collecting extra boot data with the
nightly run.  Once this change is confirmed effective, the iteration count will
be bumped high enough to allow detection of relatively small changes in boot
time per model of device.  Estimates are that the nightly build has more than
enough slack time to accommodate 50 iterations per client.  The target for
testability is to be able to detect changes at least as small as 50 ms.

[ericli, 2010-06-21 17:53:36]

http://codereview.chromium.org/2830016/diff/1003/2001
File server/site_tests/platform_BootPerfServer/platform_BootPerfServer.py
(right):

http://codereview.chromium.org/2830016/diff/1003/2001#newcode31
server/site_tests/platform_BootPerfServer/platform_BootPerfServer.py:31:
client_results = open(src, "r")
My code review dashboard had rendered this as tab instead of space? Could you
double check?

http://codereview.chromium.org/2830016/diff/1003/2002
File server/site_tests/suite_Nightly/control (right):

http://codereview.chromium.org/2830016/diff/1003/2002#newcode77
server/site_tests/suite_Nightly/control:77:
job.run_test("platform_BootPerfServer", host=host, iterations=4)
you could do boot perf inside run_client_test. so you dont need to call parallel
twice.

And you want to bootperf test at the beginning of the test suite or at the end?
In BVT, boot test is the first test in the suite, but I am not sure which one is
better.

[petkov, 2010-06-21 18:11:31]

http://codereview.chromium.org/2830016/diff/1003/2001
File server/site_tests/platform_BootPerfServer/platform_BootPerfServer.py
(right):

http://codereview.chromium.org/2830016/diff/1003/2001#newcode33
server/site_tests/platform_BootPerfServer/platform_BootPerfServer.py:33: for l
in client_results.readlines():
Use shutil.copyfileobj instead?

Also, I assume you have tested that this appending works OK?

http://codereview.chromium.org/2830016/diff/1003/2002
File server/site_tests/suite_Nightly/control (right):

http://codereview.chromium.org/2830016/diff/1003/2002#newcode77
server/site_tests/suite_Nightly/control:77:
job.run_test("platform_BootPerfServer", host=host, iterations=4)
As ericli suggested, it might be better to run bootperf inside run_client_test.
Feel free to rename run_client_test to run_tests_on_machine or something like
that so that it doesn't imply that only client-side tests are run.

[jrbarnette, 2010-06-21 18:30:26]

On Jun 21, 2010, at 11:11 AM, petkov@chromium.org wrote:

>
> http://codereview.chromium.org/2830016/diff/1003/2001
> File
> server/site_tests/platform_BootPerfServer/platform_BootPerfServer.py
> (right):
>
> http://codereview.chromium.org/2830016/diff/1003/2001#newcode33
> server/site_tests/platform_BootPerfServer/platform_BootPerfServer.py: 
> 33:
> for l in client_results.readlines():
> Use shutil.copyfileobj instead?
>
Ah, better.  I did rummage through the contents of shutil looking for
a routine, but I was looking for something that said "append", so I
missed this...


> Also, I assume you have tested that this appending works OK?
>
Appending is necessary: without it, only the last iteration's results
get recorded.


> http://codereview.chromium.org/2830016/diff/1003/2002
> File server/site_tests/suite_Nightly/control (right):
>
> http://codereview.chromium.org/2830016/diff/1003/2002#newcode77
> server/site_tests/suite_Nightly/control:77:
> job.run_test("platform_BootPerfServer", host=host, iterations=4)
> As ericli suggested, it might be better to run bootperf inside
> run_client_test. Feel free to rename run_client_test to
> run_tests_on_machine or something like that so that it doesn't imply
> that only client-side tests are run.
>
> http://codereview.chromium.org/2830016/show

-- jrb

[petkov, 2010-06-21 18:48:41]

> > Also, I assume you have tested that this appending works OK?
> >
> Appending is necessary: without it, only the last iteration's results
> get recorded.

Yeah, I realize that. Just wanted to make sure that you've tested that it
actually works -- and there's no some other phase, for example, that overwrites
the file or something.

[jrbarnette, 2010-06-21 20:43:54]

On Jun 21, 2010, at 11:48 AM, petkov@chromium.org wrote:

>> > Also, I assume you have tested that this appending works OK?
>> >
>> Appending is necessary: without it, only the last iteration's results
>> get recorded.
>
> Yeah, I realize that. Just wanted to make sure that you've tested  
> that it
> actually works -- and there's no some other phase, for example, that  
> overwrites
> the file or something.
>
Oh, right.  I meant to address that in my original response.  Yes, I've
tested and seen that the file contains the content from all the  
iterations.

I'm now retesting all the requested changes, to make sure it will still
work as expected.


>
> http://codereview.chromium.org/2830016/show

-- jrb

[jrbarnette, 2010-06-22 00:35:05]

OK.  I've updated to address review comments, and retested.
You'll find these changes:
  * Replaced tab characters with equivalent spaces
  * Moved the BootPerf test to join the client tests, and renamed the test
handler to
    describe the function's new role.
  * Changed to use copyfileobj in place of a hand-coded copy loop.

I have no particular opinion as to whether the BootPerfServer test should come
before or after the client tests.  I don't think it really matters.  The only
scenario I can think of would be if the nightly tests ran too long and had to be
killed.  In that case, the BootPerfServer test would be a good candidate to get
dropped because it was last in line.  I'm not sure if this really matters,
though.

[petkov, 2010-06-22 01:01:46]

LGTM