Add testing library to running tests.

Add testing library to running tests.

Depends on internal code review for slave DEPS,
which depends on http://codereview.chromium.org/9600031/

BUG=113583
TEST=Manual.  More GPU tests should pass on normal linux bots.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=126423

[Peter Mayo, 2012-03-06 20:14:49]

This is dependent on the DEPS changes in internal to pull the new directories
onto our slaves.

The DEPS cleanup is dependent on this change becoming active.

Peter, John.  I am proposing the initial tests be the simple ones you have
pointed at before.

Do we also need to separate the gpu_tests from gpu results accordingly in the
XML generation?

[Peter Mayo, 2012-03-06 21:46:27]

http://codereview.chromium.org/9601021/diff/5003/scripts/master/factory/chrom...
File scripts/master/factory/chromium_commands.py (right):

http://codereview.chromium.org/9601021/diff/5003/scripts/master/factory/chrom...
scripts/master/factory/chromium_commands.py:962:
'--gtest_filter=GpuPixelBrowserTest.*'],

Jonathan:
I've been looking at the target gpu_test
and gpu_crash_browsertest would be useful too...

[pkotwicz, 2012-03-06 21:58:56]

http://codereview.chromium.org/9601021/diff/5003/scripts/master/factory/chrom...
File scripts/master/factory/chromium_commands.py (right):

http://codereview.chromium.org/9601021/diff/5003/scripts/master/factory/chrom...
scripts/master/factory/chromium_commands.py:962:
'--gtest_filter=GpuPixelBrowserTest.*'],
For completeness, you probably want to include Canvas2DPixelTestSD.* and
Canvas2DPixelTestHD.*

[Peter Mayo, 2012-03-06 23:25:08]

This seems to test OK now.

[Marc-Antoine Ruel (Google), 2012-03-06 23:38:03]

http://codereview.chromium.org/9601021/diff/10002/scripts/master/factory/chro...
File scripts/master/factory/chromium_commands.py (right):

http://codereview.chromium.org/9601021/diff/10002/scripts/master/factory/chro...
scripts/master/factory/chromium_commands.py:944: # Put gpu data in
/b/build/slave/SLAVE_NAME/gpu_data
you mean <slavebuilddir> instead of SLAVE_NAME?

Note that I dislike having stuff outside slave/<slavebuilddir>/build/ since it
tends to stay there literally for years.

What's the plan to delete stale files?

http://codereview.chromium.org/9601021/diff/10002/scripts/master/factory/chro...
scripts/master/factory/chromium_commands.py:948: tests =
':'.join(['GpuPixelBrowserTest.*', 'GpuFeatureTest.*',
fix alignment

http://codereview.chromium.org/9601021/diff/10002/scripts/master/factory/chro...
scripts/master/factory/chromium_commands.py:954: arg_list=['--generated-dir=%s'
% gen_dir,
fix alignment

http://codereview.chromium.org/9601021/diff/10002/scripts/master/factory/chro...
File scripts/master/factory/chromium_factory.py (right):

http://codereview.chromium.org/9601021/diff/10002/scripts/master/factory/chro...
scripts/master/factory/chromium_factory.py:292: if R('aura_shell') or R('ash'):
??

http://codereview.chromium.org/9601021/diff/10002/scripts/slave/runtest.py
File scripts/slave/runtest.py (right):

http://codereview.chromium.org/9601021/diff/10002/scripts/slave/runtest.py#ne...
scripts/slave/runtest.py:447: import platform
Why function local?

http://codereview.chromium.org/9601021/diff/10002/scripts/slave/runtest.py#ne...
scripts/slave/runtest.py:496: option_parser.add_option('', '--llvmpipe',
action='store_true',
Why add this flag at all?

http://codereview.chromium.org/9601021/diff/10002/scripts/slave/runtest.py#ne...
scripts/slave/runtest.py:501: option_parser.add_option('', '--llvmpipe-dir',
default=os.path.join(
Shouldn't just specifying --lvmpipe-dir enable it?
Also, alignment

[Peter Mayo, 2012-03-07 16:00:35]

http://codereview.chromium.org/9601021/diff/10002/scripts/master/factory/chro...
File scripts/master/factory/chromium_commands.py (right):

http://codereview.chromium.org/9601021/diff/10002/scripts/master/factory/chro...
scripts/master/factory/chromium_commands.py:944: # Put gpu data in
/b/build/slave/SLAVE_NAME/gpu_data
On 2012/03/06 23:38:03, Marc-Antoine Ruel (Google) wrote:
> you mean <slavebuilddir> instead of SLAVE_NAME?

Guilty of blatant copying from below.

PeterK:  We are aiming at broad testing with the softGPU, so we shouldn't be
needing the same sort of collection as the GPU bots.  What would it take to make
these local?  Are teh defaults actually good enough?  If not, can we make them
sensible/normal?

> 
> Note that I dislike having stuff outside slave/<slavebuilddir>/build/ since it
> tends to stay there literally for years.


Good point.  I agree.

http://codereview.chromium.org/9601021/diff/10002/scripts/master/factory/chro...
scripts/master/factory/chromium_commands.py:948: tests =
':'.join(['GpuPixelBrowserTest.*', 'GpuFeatureTest.*',
On 2012/03/06 23:38:03, Marc-Antoine Ruel (Google) wrote:
> fix alignment

Done.

http://codereview.chromium.org/9601021/diff/10002/scripts/master/factory/chro...
scripts/master/factory/chromium_commands.py:954: arg_list=['--generated-dir=%s'
% gen_dir,
On 2012/03/06 23:38:03, Marc-Antoine Ruel (Google) wrote:
> fix alignment

Done.

[M-A Ruel, 2012-03-07 16:14:23]

Did you forget to upload again?

[Peter Mayo, 2012-03-07 16:22:42]

On 2012/03/07 16:14:23, Marc-Antoine Ruel wrote:
> Did you forget to upload again?

I hit send when I was moving from file to file.

[Peter Mayo, 2012-03-07 16:22:48]

http://codereview.chromium.org/9601021/diff/10002/scripts/master/factory/chro...
File scripts/master/factory/chromium_factory.py (right):

http://codereview.chromium.org/9601021/diff/10002/scripts/master/factory/chro...
scripts/master/factory/chromium_factory.py:292: if R('aura_shell') or R('ash'):
On 2012/03/06 23:38:03, Marc-Antoine Ruel (Google) wrote:
> ??
leakage from http://codereview.chromium.org/9197019/

removed.

http://codereview.chromium.org/9601021/diff/10002/scripts/slave/runtest.py
File scripts/slave/runtest.py (right):

http://codereview.chromium.org/9601021/diff/10002/scripts/slave/runtest.py#ne...
scripts/slave/runtest.py:447: import platform
On 2012/03/06 23:38:03, Marc-Antoine Ruel (Google) wrote:
> Why function local?

Conflict with line 132 variable name.  This seemed cleaner than refactoring that
interface.

http://codereview.chromium.org/9601021/diff/10002/scripts/slave/runtest.py#ne...
scripts/slave/runtest.py:496: option_parser.add_option('', '--llvmpipe',
action='store_true',
On 2012/03/06 23:38:03, Marc-Antoine Ruel (Google) wrote:
> Why add this flag at all?
i) I don't believe in adding options that don't have inverses, and no-llvmpipe
seems prudent.
ii) I like having defaults that are just 'right' almost all the time. 
Specifying the correct arch directory in all of the call sites seems like
distributing the business logic to much.

hence no-args:  the usual llvmpipe directory.
   --llvmpipe:  the usual directory.
--no-llvmpipe:  nothing weird in LD_LIBRARY_PATH
--lvmpipe-dir:  weird directory in LD_LIBRARY_PATH

http://codereview.chromium.org/9601021/diff/10002/scripts/slave/runtest.py#ne...
scripts/slave/runtest.py:499: default=False, dest='llvmpipe',
Contradictory defaults.

http://codereview.chromium.org/9601021/diff/10002/scripts/slave/runtest.py#ne...
scripts/slave/runtest.py:501: option_parser.add_option('', '--llvmpipe-dir',
default=os.path.join(
On 2012/03/06 23:38:03, Marc-Antoine Ruel (Google) wrote:
> Shouldn't just specifying --lvmpipe-dir enable it?

I'd like to leave that to the person who implements default=False for llvmpipe.


> Also, alignment

The function-argument alignment starts to look bad with these lengths & nesting.
Refactored the path out.  It is easier to read.

[M-A Ruel, 2012-03-07 20:29:44]

lgtm

http://codereview.chromium.org/9601021/diff/12002/scripts/master/factory/chro...
File scripts/master/factory/chromium_commands.py (right):

http://codereview.chromium.org/9601021/diff/12002/scripts/master/factory/chro...
scripts/master/factory/chromium_commands.py:944: # Put gpu data in
/b/build/slave/SLAVE_NAME/gpu_data
Do you want to keep this comment?

http://codereview.chromium.org/9601021/diff/12002/scripts/master/factory/chro...
scripts/master/factory/chromium_commands.py:955: # '--generated-dir=%s' %
gen_dir,
same

http://codereview.chromium.org/9601021/diff/12002/scripts/slave/runtest.py
File scripts/slave/runtest.py (right):

http://codereview.chromium.org/9601021/diff/12002/scripts/slave/runtest.py#ne...
scripts/slave/runtest.py:499: option_parser.add_option('', '--llvmpipe',
action='store_true', default=True,
I don't care enough to block you but I think it just adds noise. Either you use
the default or you don't. It makes both the implementation and users simpler
since there can't be conflicting flags on the command line.

[Peter Mayo, 2012-03-08 21:17:35]

re-testing and waiting a little for the discussion of what the invoked tests are
supposed to be.

http://codereview.chromium.org/9601021/diff/12002/scripts/master/factory/chro...
File scripts/master/factory/chromium_commands.py (right):

http://codereview.chromium.org/9601021/diff/12002/scripts/master/factory/chro...
scripts/master/factory/chromium_commands.py:944: # Put gpu data in
/b/build/slave/SLAVE_NAME/gpu_data
On 2012/03/07 20:29:44, Marc-Antoine Ruel wrote:
> Do you want to keep this comment?

These were a hook to the key open question of where the test expectations come
from, and where failures get saved:  There is no correct value for these as of
yet.

http://codereview.chromium.org/9601021/diff/12002/scripts/master/factory/chro...
scripts/master/factory/chromium_commands.py:955: # '--generated-dir=%s' %
gen_dir,
On 2012/03/07 20:29:44, Marc-Antoine Ruel wrote:
> same

Done.

http://codereview.chromium.org/9601021/diff/12002/scripts/slave/runtest.py
File scripts/slave/runtest.py (right):

http://codereview.chromium.org/9601021/diff/12002/scripts/slave/runtest.py#ne...
scripts/slave/runtest.py:499: option_parser.add_option('', '--llvmpipe',
action='store_true', default=True,
On 2012/03/07 20:29:44, Marc-Antoine Ruel wrote:
> I don't care enough to block you but I think it just adds noise. Either you
use
> the default or you don't. It makes both the implementation and users simpler
> since there can't be conflicting flags on the command line.

I think you are right that a single variable makes the implementation cleaner. 
Done.  Each of the three clear options are then at best redundant, and usually
contradictory.

[commit-bot: I haz the power, 2012-03-13 18:29:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/petermayo@chromium.org/9601021/21001

[commit-bot: I haz the power, 2012-03-13 18:32:05]

Change committed as 126423