telemetry: Create telemetry_gtest_wrapper build group.

telemetry: Create telemetry_gtest_wrapper build group.

Refactor chrome/test/BUILD.gn so the scripts needed to run telemetry tests on
swarming as in a target which can be depended on.

This reduces repetition and allows the target to be reused by webkit layout
tests.

BUG=524758

[mithro, 2016-10-27 10:50:36]

The CQ bit was checked by tansell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-27 10:50:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-27 10:54:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-27 10:54:29]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mithro, 2016-10-27 10:56:35]

The CQ bit was checked by tansell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-27 10:56:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mithro, 2016-10-27 10:58:43]

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

[mithro, 2016-10-27 10:58:43]

Hi!

I'm in the process of trying to create a webkit_layout_tests target which can be
isolated (see https://codereview.chromium.org/2452313003/). This requires a
couple of small changes to other build targets which don't seem to be using
data_deps correctly. 

These are the changes needed for the telemetry wrapper scripts.

Tim 'mithro' Ansell

[commit-bot: I haz the power, 2016-10-27 11:38:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-27 11:38:16]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[mithro, 2016-10-27 12:16:48]

The CQ bit was checked by tansell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-27 12:16:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mithro, 2016-10-27 13:10:46]

The CQ bit was checked by tansell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-27 13:11:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-27 14:36:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-27 14:36:13]

Dry run: This issue passed the CQ dry run.

[Dirk Pranke, 2016-10-28 00:24:06]

dpranke@chromium.org changed reviewers:
+ eyaich@chromium.org, kbr@chromium.org, nednguyen@google.com

[Dirk Pranke, 2016-10-28 00:24:07]

+nednguyen, kbr, eyaich ... 

I think this changes are generally fine +/- the comments I have below (which
just relate to naming and locations for things), but I don't touch these files
much and would want to make sure the telemetry folks are okay with this as well.

We really need someone to sort out and document these recipe->script interfaces
better.

https://codereview.chromium.org/2456793003/diff/60001/chrome/test/BUILD.gn
File chrome/test/BUILD.gn (right):

https://codereview.chromium.org/2456793003/diff/60001/chrome/test/BUILD.gn#ne...
chrome/test/BUILD.gn:256: "//testing/xvfb.py",
These two files are actually automatically tacked on into the isolate by MB for
all targets, which may be why you're seeing them inconsistently specified now. I
don't think it's a bad thing to push them into the right place in GN, but that
right place is not in //chrome/test (it should be a target in //testing).

https://codereview.chromium.org/2456793003/diff/60001/chrome/test/BUILD.gn#ne...
chrome/test/BUILD.gn:264: "//testing/scripts/run_telemetry_as_googletest.py",
This is probably a better comment for your other CL, but if we're going to use
run_telemetry_as_googletest.py for non-telemetry-based tests, we should rename
or copy it first (and move it and as per the above). The webkit_layout_tests
target should not have dependencies on //chrome/test or anything in //chrome.

[nednguyen, 2016-10-28 00:26:38]

https://codereview.chromium.org/2456793003/diff/60001/chrome/test/BUILD.gn
File chrome/test/BUILD.gn (right):

https://codereview.chromium.org/2456793003/diff/60001/chrome/test/BUILD.gn#ne...
chrome/test/BUILD.gn:264: "//testing/scripts/run_telemetry_as_googletest.py",
On 2016/10/28 00:24:06, Dirk Pranke wrote:
> This is probably a better comment for your other CL, but if we're going to use
> run_telemetry_as_googletest.py for non-telemetry-based tests, we should rename
> or copy it first (and move it and as per the above). The webkit_layout_tests
> target should not have dependencies on //chrome/test or anything in //chrome.

run_isolated_script_test.py?

[Ken Russell (switch to Gerrit), 2016-10-28 00:27:40]

On 2016/10/28 00:24:07, Dirk Pranke wrote:
> +nednguyen, kbr, eyaich ... 
> 
> I think this changes are generally fine +/- the comments I have below (which
> just relate to naming and locations for things), but I don't touch these files
> much and would want to make sure the telemetry folks are okay with this as
well.
> 
> We really need someone to sort out and document these recipe->script
interfaces
> better.

Sorry about this -- yes, there should be an easy-to-find and centralized
location for this documentation. Right now each of the isolated script test
wrappers in src/testing/scripts/ (such as run_telemetry_as_googletest.py,
run_gpu_integration_test_as_googletest.py) have a comment at the top which is
out of date already. Suggestions about where to put it welcome. README.md in
src/testing/scripts?

[Ken Russell (switch to Gerrit), 2016-10-28 00:28:58]

https://codereview.chromium.org/2456793003/diff/60001/chrome/test/BUILD.gn
File chrome/test/BUILD.gn (right):

https://codereview.chromium.org/2456793003/diff/60001/chrome/test/BUILD.gn#ne...
chrome/test/BUILD.gn:264: "//testing/scripts/run_telemetry_as_googletest.py",
On 2016/10/28 00:26:37, nednguyen wrote:
> On 2016/10/28 00:24:06, Dirk Pranke wrote:
> > This is probably a better comment for your other CL, but if we're going to
use
> > run_telemetry_as_googletest.py for non-telemetry-based tests, we should
rename
> > or copy it first (and move it and as per the above). The webkit_layout_tests
> > target should not have dependencies on //chrome/test or anything in
//chrome.
> 
> run_isolated_script_test.py?

Rather, run_xyz_as_isolated_script_test.py. I think there will need to be
several "xyz"s.

[eyaich1, 2016-10-28 12:46:34]

My larger question in this recipe code, is what is keeping us from consolidating
SwarmingGTestTest and SwarmingIsolatedScriptTest?  Is it just that the
SwarmignGTestTest targets directly output the right format to output and our
tests need a wrapper script to get them into the right result format?

https://codereview.chromium.org/2456793003/diff/60001/chrome/test/BUILD.gn
File chrome/test/BUILD.gn (right):

https://codereview.chromium.org/2456793003/diff/60001/chrome/test/BUILD.gn#ne...
chrome/test/BUILD.gn:264: "//testing/scripts/run_telemetry_as_googletest.py",
On 2016/10/28 00:28:58, Ken Russell wrote:
> On 2016/10/28 00:26:37, nednguyen wrote:
> > On 2016/10/28 00:24:06, Dirk Pranke wrote:
> > > This is probably a better comment for your other CL, but if we're going to
> use
> > > run_telemetry_as_googletest.py for non-telemetry-based tests, we should
> rename
> > > or copy it first (and move it and as per the above). The
webkit_layout_tests
> > > target should not have dependencies on //chrome/test or anything in
> //chrome.
> > 
> > run_isolated_script_test.py?
> 
> Rather, run_xyz_as_isolated_script_test.py. I think there will need to be
> several "xyz"s.

Yes I am actually adding another one in crrev.com/2442663004

https://codereview.chromium.org/2456793003/diff/60001/chrome/test/BUILD.gn#ne...
chrome/test/BUILD.gn:272: group("telemetry_benchmark_as_gtest") {
I don't think a separate group is necessary for each one of the
run_xyz_as_isoalted_script_test.py.  You aren't really wrapping much, and I
think it is clearer for each target to know what script it depends on since that
drives the entire test on the swarming bot.

[Ken Russell (switch to Gerrit), 2016-10-28 18:19:49]

On 2016/10/28 12:46:34, eyaich1 wrote:
> My larger question in this recipe code, is what is keeping us from
consolidating
> SwarmingGTestTest and SwarmingIsolatedScriptTest?  Is it just that the
> SwarmignGTestTest targets directly output the right format to output and our
> tests need a wrapper script to get them into the right result format?

The only hurdles would be getting buy-in from the Infra team, and the technical
aspect of implementing it.

It's not feasible to merge only the Swarming variants of these test types; the
local versions would have to be modified too.

Isolated script tests are strictly more general than GTest tests, because they
provide the possibility of a src-side wrapper script which can massage the
outputs into the right form. However, I don't think they support all the needed
features like running a subset of tests for the "without patch" retries.