WIP: blink/run-webkit-tests: Add support for gtest API.

WIP: blink/run-webkit-tests: Add support for gtest API.

The gtest library defines a command line / environment variable API for
controlling test running (see
https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md#running-test-programs-advanced-options).

This API is already supported by many of our other tools, including swarming.
To enable run-webkit-tests to be used with these tools, this CL adds this API.

BUG=601332, 524758

[mithro, 2016-06-21 05:00:58]

tansell@chromium.org changed reviewers:
+ qyearsley@chromium.org

[mithro, 2016-06-21 05:00:59]

FYI Quinten...

[Dirk Pranke, 2016-06-21 22:05:24]

dpranke@chromium.org changed reviewers:
+ dpranke@chromium.org

[Dirk Pranke, 2016-06-21 22:05:25]

https://codereview.chromium.org/2086803002/diff/40001/third_party/WebKit/Tool...
File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
(right):

https://codereview.chromium.org/2086803002/diff/40001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:582:
option_group_definitions.append(
I don't necessarily think we should add all of these flags, and I don't know
that they're all needed for swarming. Some of them are maybe just different
names for existing flags, e.g., gtest_repeat, gtest_shard_index. 

Is your plan that swarming will invoke run-webkit-tests directly, rather than
using a wrapper script like we do for telemetry? Life might be easier if we just
had a script in //testing/buildbot that implemented exactly the interface we
needed for swarming, rather than trying to add more overlapping flags to a
command line that is already way too big and complicated.

If you were dead set on adding the flags here and not having a wrapper script,
I'd like to see us remove some of the existing flags that are redundant.

The way you've done the comments for these things is also quite unlike any of
the other comments in the code.

[mithro, 2016-06-22 13:26:25]

On 2016/06/21 22:05:25, Dirk Pranke wrote:
>
https://codereview.chromium.org/2086803002/diff/40001/third_party/WebKit/Tool...
> File
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
> (right):
> 
>
https://codereview.chromium.org/2086803002/diff/40001/third_party/WebKit/Tool...
>
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:582:
> option_group_definitions.append(
> I don't necessarily think we should add all of these flags, and I don't know
> that they're all needed for swarming. Some of them are maybe just different
> names for existing flags, e.g., gtest_repeat, gtest_shard_index. 

Looking so far, it seems all the gtest flags have "similar equivalents" in
run-webkit-tests but some need a bit of massaging.

I do think we want to implement the complete set of gtest APIs as swarming based
functionality is already heading down the path of using that to do things like
provide single click "rerun failed tests again" and "run test multiple times"
type functionality. The more similar to other tests the layout tests are, the
more of this functionality we get for free.

> Is your plan that swarming will invoke run-webkit-tests directly, rather than
> using a wrapper script like we do for telemetry?

> Life might be easier if we just
> had a script in //testing/buildbot that implemented exactly the interface we
> needed for swarming, rather than trying to add more overlapping flags to a
> command line that is already way too big and complicated.

I was originally adding these to a wrapper script, but I changed to adding the
functionality directly for the following reasons;
 * People are more likely to discover and use them.
 * With the issues with wrapper scripts we have had recently, it also seems a
good idea to reduce them.

If you have a compelling argument, I could be convinced to go back to the
wrapper script approach.

> If you were dead set on adding the flags here and not having a wrapper script,
> I'd like to see us remove some of the existing flags that are redundant.

I'm definitely like the idea of removing the existing redundant flags, but I
have a feeling that people are going to complain a lot?

> The way you've done the comments for these things is also quite unlike any of
> the other comments in the code.

This change is still very much a WIP and will probably change quite a bit before
it is ready to land but I mainly uploaded it because Quinten wanted to better
understand the gtest API.

Tim 'mithro' Ansell

[Dirk Pranke, 2016-06-24 00:06:07]

On 2016/06/22 13:26:25, mithro wrote:
> On 2016/06/21 22:05:25, Dirk Pranke wrote:
> >
>
https://codereview.chromium.org/2086803002/diff/40001/third_party/WebKit/Tool...
> > File
> third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
> > (right):
> > 
> >
>
https://codereview.chromium.org/2086803002/diff/40001/third_party/WebKit/Tool...
> >
>
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:582:
> > option_group_definitions.append(
> > I don't necessarily think we should add all of these flags, and I don't know
> > that they're all needed for swarming. Some of them are maybe just different
> > names for existing flags, e.g., gtest_repeat, gtest_shard_index. 
> 
> Looking so far, it seems all the gtest flags have "similar equivalents" in
> run-webkit-tests but some need a bit of massaging.
> 
> I do think we want to implement the complete set of gtest APIs as swarming
based
> functionality is already heading down the path of using that to do things like
> provide single click "rerun failed tests again" and "run test multiple times"
> type functionality. The more similar to other tests the layout tests are, the
> more of this functionality we get for free.

Well, adding code isn't "free" :).

But, yeah, I agree that we should try to make the interfaces identical where
possible,
and that it's good to avoid wrappers where we can.

However, we often already have the wrappers, and you might want to reuse them
instead.

In particular, most of the other python-based test suites (like the telemetry
tests) use a library 
called typ (that I wrote after refactoring stuff out from webkitpy) and there's
more bang for
the buck in sharing code with typ (really, run-webkit-tests should be using typ
directly)
first, before sharing stuff w/ gtest and c++-based.

Though, ideally we should have the same interface for both python- and c++-based
tests.

And, I also find the gtest interface ungainly, and we actually often use our own
flags (via
//base/test/_launcher) anyway, rather than calling gtest directly.

So, my point is that we/you should probably do this in a slightly more visible
way, e.g.,
reach out to phajdan.jr@ and kbr@ and nednguyen@ and others who maintain these
test steps and try to get everyone to agree on an interface. maybe the gtest
interface
is the right thing to do, maybe it isn't.

> I'm definitely like the idea of removing the existing redundant flags, but I
> have a feeling that people are going to complain a lot?

We shouldn't keep arguments around just because we don't want to retrain people.
That leads to unmaintainable code, and we have too much of that in the command
line args for RWT already. And, in these particular examples, I'd be very
surprised
if devs were using any of them at all frequently, so I doubt it'd be a big deal.

To be clear: I'm glad you're working on this stuff, and they seem like good
intentions.
I don't mean to sound too hard here, and apologize if I am.

[qyearsley, 2016-08-29 22:48:29]

Description was changed from

==========
WIP: blink/run-webkit-tests: Add support for gtest API.

The gtest library defines a command line / environment variable API for
controlling test running (see
https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuid...).

This API is already supported by many of our other tools, including swarming.
To enable run-webkit-tests to be used with these tools, this CL adds this API.

BUG=601332,524758
==========

to

==========
WIP: blink/run-webkit-tests: Add support for gtest API.

The gtest library defines a command line / environment variable API for
controlling test running (see
https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuid...).

This API is already supported by many of our other tools, including swarming.
To enable run-webkit-tests to be used with these tools, this CL adds this API.

BUG=601332,524758
==========