Enable telemetry_perf_unittests on swarming.

Enable telemetry_perf_unittests on swarming.

BUG=525873, 606066
Committed: https://crrev.com/c6de0f49ae06fd3a2f2459e7161f26c4673c10d0
Cr-Commit-Position: refs/heads/master@{#397216}

[ghost stip (do not use), 2016-06-01 00:34:18]

Description was changed from

==========
Enable telemetry_perf_unittests on swarming.

BUG=525873
==========

to

==========
Enable telemetry_perf_unittests on swarming.

BUG=525873
==========

[ghost stip (do not use), 2016-06-01 00:34:18]

stip@chromium.org changed reviewers:
+ dpranke@chromium.org, kbr@chromium.org, nednguyen@google.com,
sky@chromium.org, thakis@chromium.org

[ghost stip (do not use), 2016-06-01 00:34:38]

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

[commit-bot: I haz the power, 2016-06-01 00:35:06]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2027873002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2027873002/1

[Ken Russell (switch to Gerrit), 2016-06-01 01:18:10]

lgtm overall.

https://codereview.chromium.org/2027873002/diff/1/testing/buildbot/chromium.f...
File testing/buildbot/chromium.fyi.json (right):

https://codereview.chromium.org/2027873002/diff/1/testing/buildbot/chromium.f...
testing/buildbot/chromium.fyi.json:1421: "--xvfb"
This looks wrong. This is a Mac bot and xvfb should not be necessary.

I see that you're folding the "--xvfb" flag into all the bot configurations
which run this test except for Android, but this would be a good opportunity to
fix some of the bots' setups and minimize the diffs in this CL. It should only
be needed on the Linux bots.

https://codereview.chromium.org/2027873002/diff/1/tools/perf/chrome_telemetry...
File tools/perf/chrome_telemetry_build/BUILD.gn (right):

https://codereview.chromium.org/2027873002/diff/1/tools/perf/chrome_telemetry...
tools/perf/chrome_telemetry_build/BUILD.gn:31:
"//chrome/android:chrome_public_apk",
Please add comment:

TODO(stip): remove the content_shell dependency below after the GPU bots are
switched over to use Chromium instead. crbug.com/525873

Thanks.

[commit-bot: I haz the power, 2016-06-01 02:35:58]

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

[commit-bot: I haz the power, 2016-06-01 02:36:00]

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

[ghost stip (do not use), 2016-06-01 08:42:26]

ptal!

nednguyen: review as well as tools/perf OWNERS
thakis/sky: chrome/ OWNERS
dpranke: testing/buildbot and BUILD.gn OWNERS

https://codereview.chromium.org/2027873002/diff/1/testing/buildbot/chromium.f...
File testing/buildbot/chromium.fyi.json (right):

https://codereview.chromium.org/2027873002/diff/1/testing/buildbot/chromium.f...
testing/buildbot/chromium.fyi.json:1421: "--xvfb"
because --xvfb no longer on by default in gn_isolate_map.pyl

https://codereview.chromium.org/2027873002/diff/1/tools/perf/chrome_telemetry...
File tools/perf/chrome_telemetry_build/BUILD.gn (right):

https://codereview.chromium.org/2027873002/diff/1/tools/perf/chrome_telemetry...
tools/perf/chrome_telemetry_build/BUILD.gn:31:
"//chrome/android:chrome_public_apk",
to enable the android-chromium target. a future CL will switch GPU testing over,
removing the requirement for content_shell

https://codereview.chromium.org/2027873002/diff/1/tools/perf/chrome_telemetry...
tools/perf/chrome_telemetry_build/BUILD.gn:31:
"//chrome/android:chrome_public_apk",
On 2016/06/01 01:18:10, Ken Russell wrote:
> Please add comment:
> 
> TODO(stip): remove the content_shell dependency below after the GPU bots are
> switched over to use Chromium instead. crbug.com/525873
> 
> Thanks.

I made crbug.com/616392 to more precisely capture what needs to be done.

https://codereview.chromium.org/2027873002/diff/20001/chrome/test/BUILD.gn
File chrome/test/BUILD.gn (left):

https://codereview.chromium.org/2027873002/diff/20001/chrome/test/BUILD.gn#ol...
chrome/test/BUILD.gn:738: group("telemetry_perf_unittests_run") {
move these out of the if (!is_android) block

https://codereview.chromium.org/2027873002/diff/20001/testing/buildbot/chromi...
File testing/buildbot/chromium.android.json (right):

https://codereview.chromium.org/2027873002/diff/20001/testing/buildbot/chromi...
testing/buildbot/chromium.android.json:959: "--device=android"
--device may not be needed? Ned?

https://codereview.chromium.org/2027873002/diff/20001/testing/buildbot/chromi...
File testing/buildbot/chromium.linux.json (right):

https://codereview.chromium.org/2027873002/diff/20001/testing/buildbot/chromi...
testing/buildbot/chromium.linux.json:1016: "--xvfb"
because --xvfb no longer on by default in gn_isolate_map.pyl

https://codereview.chromium.org/2027873002/diff/20001/testing/buildbot/gn_iso...
File testing/buildbot/gn_isolate_map.pyl (left):

https://codereview.chromium.org/2027873002/diff/20001/testing/buildbot/gn_iso...
testing/buildbot/gn_isolate_map.pyl:556: "--xvfb",
--xvfb breaks on android swarming bots, so we flatten it out to each builder as
needed. It should only be needed for linux bots.

[ghost stip (do not use), 2016-06-01 08:42:40]

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

[commit-bot: I haz the power, 2016-06-01 08:42:51]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2027873002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2027873002/20001

[commit-bot: I haz the power, 2016-06-01 09:53:39]

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

[commit-bot: I haz the power, 2016-06-01 09:53:40]

Dry run: This issue passed the CQ dry run.

[Nico, 2016-06-01 15:12:38]

chrome/ lgtm, good luck

https://codereview.chromium.org/2027873002/diff/20001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2027873002/diff/20001/BUILD.gn#newcode320
BUILD.gn:320: "//chrome/test:telemetry_perf_unittests",
why is this in an is_android block?

[nednguyen, 2016-06-01 16:25:35]

nednguyen@google.com changed reviewers:
+ zhenw@chromium.org

[nednguyen, 2016-06-01 16:40:55]

https://codereview.chromium.org/2027873002/diff/20001/testing/buildbot/chromi...
File testing/buildbot/chromium.android.json (right):

https://codereview.chromium.org/2027873002/diff/20001/testing/buildbot/chromi...
testing/buildbot/chromium.android.json:959: "--device=android"
On 2016/06/01 08:42:25, stip wrote:
> --device may not be needed? Ned?

Let's keep --device=android to be sure. In the future, we may make it a required
argument.

https://codereview.chromium.org/2027873002/diff/20001/testing/buildbot/chromi...
testing/buildbot/chromium.android.json:974: "hard_timeout": 960
This hard_timeout options seems cool. Is this only for Android swarming?

[ghost stip (do not use), 2016-06-01 16:49:32]

https://codereview.chromium.org/2027873002/diff/20001/testing/buildbot/chromi...
File testing/buildbot/chromium.android.json (right):

https://codereview.chromium.org/2027873002/diff/20001/testing/buildbot/chromi...
testing/buildbot/chromium.android.json:974: "hard_timeout": 960
On 2016/06/01 16:40:55, nednguyen wrote:
> This hard_timeout options seems cool. Is this only for Android swarming?

it's for any swarming task, I'm just the only one using it right now

[nednguyen, 2016-06-01 16:51:10]

https://codereview.chromium.org/2027873002/diff/20001/testing/buildbot/chromi...
File testing/buildbot/chromium.android.json (right):

https://codereview.chromium.org/2027873002/diff/20001/testing/buildbot/chromi...
testing/buildbot/chromium.android.json:974: "hard_timeout": 960
On 2016/06/01 16:49:32, stip wrote:
> On 2016/06/01 16:40:55, nednguyen wrote:
> > This hard_timeout options seems cool. Is this only for Android swarming?
> 
> it's for any swarming task, I'm just the only one using it right now

Thanks for this. I will also use this for other telemetry suite on desktops. If
we add sharding, is this the timeout per shard?

[nednguyen, 2016-06-01 16:51:16]

lgtm

[nednguyen, 2016-06-01 16:53:52]

Description was changed from

==========
Enable telemetry_perf_unittests on swarming.

BUG=525873
==========

to

==========
Enable telemetry_perf_unittests on swarming.

BUG=525873, 606066
==========

[Dirk Pranke, 2016-06-01 17:07:30]

lgtm w/ thakis's comment in BUILD.gn addressed.

[ghost stip (do not use), 2016-06-01 17:15:43]

https://codereview.chromium.org/2027873002/diff/20001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2027873002/diff/20001/BUILD.gn#newcode320
BUILD.gn:320: "//chrome/test:telemetry_perf_unittests",
On 2016/06/01 15:12:38, Nico wrote:
> why is this in an is_android block?

seemed like a good idea at the time? should this go in "both_gn_and_gyp" or
"gn_only"? telemetry_perf_unittests is a gyp target, but it's not in
build/gn_migration.gypi

https://codereview.chromium.org/2027873002/diff/20001/testing/buildbot/chromi...
File testing/buildbot/chromium.android.json (right):

https://codereview.chromium.org/2027873002/diff/20001/testing/buildbot/chromi...
testing/buildbot/chromium.android.json:974: "hard_timeout": 960
On 2016/06/01 16:51:10, nednguyen wrote:
> On 2016/06/01 16:49:32, stip wrote:
> > On 2016/06/01 16:40:55, nednguyen wrote:
> > > This hard_timeout options seems cool. Is this only for Android swarming?
> > 
> > it's for any swarming task, I'm just the only one using it right now
> 
> Thanks for this. I will also use this for other telemetry suite on desktops.
If
> we add sharding, is this the timeout per shard?

It's for the whole swarming task. Each swarming 'shard' is a separate task, so
yeah?

[ghost stip (do not use), 2016-06-01 17:19:11]

Moved the target to both_gn_and_gyp, lmk if that makes sense?

[Nico, 2016-06-01 17:25:49]

that looks better, but:

https://codereview.chromium.org/2027873002/diff/40001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2027873002/diff/40001/BUILD.gn#newcode204
BUILD.gn:204: "//chrome/test:unit_tests",
does that target work on ios? if not, it needs to go here

[ghost stip (do not use), 2016-06-01 17:29:50]

Updated, ptal!

https://codereview.chromium.org/2027873002/diff/40001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2027873002/diff/40001/BUILD.gn#newcode204
BUILD.gn:204: "//chrome/test:unit_tests",
On 2016/06/01 17:25:49, Nico wrote:
> does that target work on ios? if not, it needs to go here

AFAIK it doesn't. done.

[Nico, 2016-06-01 18:25:44]

lgtm

[ghost stip (do not use), 2016-06-01 18:27:08]

The CQ bit was checked by stip@chromium.org

[ghost stip (do not use), 2016-06-01 18:27:09]

The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org,
dpranke@chromium.org, nednguyen@google.com
Link to the patchset: https://codereview.chromium.org/2027873002/#ps80001
(title: "Move to !is_ios.")

[commit-bot: I haz the power, 2016-06-01 18:27:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2027873002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2027873002/80001

[commit-bot: I haz the power, 2016-06-01 20:05:17]

Description was changed from

==========
Enable telemetry_perf_unittests on swarming.

BUG=525873, 606066
==========

to

==========
Enable telemetry_perf_unittests on swarming.

BUG=525873, 606066
==========

[commit-bot: I haz the power, 2016-06-01 20:05:18]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-06-01 20:06:51]

Description was changed from

==========
Enable telemetry_perf_unittests on swarming.

BUG=525873, 606066
==========

to

==========
Enable telemetry_perf_unittests on swarming.

BUG=525873, 606066

Committed: https://crrev.com/c6de0f49ae06fd3a2f2459e7161f26c4673c10d0
Cr-Commit-Position: refs/heads/master@{#397216}
==========

[commit-bot: I haz the power, 2016-06-01 20:06:52]

Patchset 5 (id:??) landed as
https://crrev.com/c6de0f49ae06fd3a2f2459e7161f26c4673c10d0
Cr-Commit-Position: refs/heads/master@{#397216}