Run angle_perftests on the GPU FYI bots.

Run angle_perftests on the GPU FYI bots.

This will help safeguard against future breakage, and be a place to
test the tests before we add them to other bots.

BUG=675997
R=kbr@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2591813002
Cr-Commit-Position: refs/heads/master@{#442605}
Committed: https://chromium.googlesource.com/chromium/src/+/ecbaf9756e7cc6f7e4743a614a32ff2e6c3ed2a9

[Jamie Madill, 2016-12-20 16:56:29]

Description was changed from

==========
Run angle_perftests on the GPU FYI bots.

This will help safeguard against future breakage, and be a place to
test the tests before we add them to other bots.

BUG=675997
R=kbr@chromium.org
==========

to

==========
Run angle_perftests on the GPU FYI bots.

This will help safeguard against future breakage, and be a place to
test the tests before we add them to other bots.

BUG=675997
R=kbr@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
==========

[Jamie Madill, 2016-12-20 16:56:45]

Description was changed from

==========
Run angle_perftests on the GPU FYI bots.

This will help safeguard against future breakage, and be a place to
test the tests before we add them to other bots.

BUG=675997
R=kbr@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
==========

to

==========
Run angle_perftests on the GPU FYI bots.

This will help safeguard against future breakage, and be a place to
test the tests before we add them to other bots.

BUG=675997
R=kbr@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
==========

[Jamie Madill, 2016-12-20 16:56:57]

Ken PTAL. (Yuly, FYI)

[Ken Russell (switch to Gerrit), 2016-12-21 17:32:17]

lgtm if this passes the trybots.

How long do these tests take to run?

[Jamie Madill, 2016-12-21 17:41:12]

a couple minutes or so. nothing onerous.

[Jamie Madill, 2016-12-21 17:41:17]

The CQ bit was checked by jmadill@chromium.org

[commit-bot: I haz the power, 2016-12-21 17:41:51]

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-12-21 17:48:22]

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

[commit-bot: I haz the power, 2016-12-21 17:48:23]

Try jobs failed on following builders:
  linux_optional_gpu_tests_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional_...)

[Jamie Madill, 2016-12-22 16:32:32]

The CQ bit was checked by jmadill@chromium.org

[Jamie Madill, 2016-12-22 16:32:32]

The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org
Link to the patchset: https://codereview.chromium.org/2591813002/#ps20001
(title: "Rebase")

[commit-bot: I haz the power, 2016-12-22 16:32:43]

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-12-22 17:11:08]

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

[commit-bot: I haz the power, 2016-12-22 17:11:09]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  win_optional_gpu_tests_rel on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_optional_gpu_...)

[Jamie Madill, 2016-12-22 20:44:17]

Ken, I'm a bit stumped - I need to pick a path for the perf harness to write to,
and I don't know what to put. I know I can add extra test args in the generator,
just not sure which path to use. Any suggestion?

[Jamie Madill, 2016-12-22 20:45:15]

Relevant error:

usage: run_gtest_perf_test.py [-h] --isolated-script-test-output
                              ISOLATED_SCRIPT_TEST_OUTPUT
                              --isolated-script-test-chartjson-output
                              ISOLATED_SCRIPT_TEST_CHARTJSON_OUTPUT [--xvfb]
run_gtest_perf_test.py: error: argument --isolated-script-test-output is
required

[Ken Russell (switch to Gerrit), 2017-01-04 03:13:06]

On 2016/12/22 20:45:15, Jamie Madill wrote:
> Relevant error:
> 
> usage: run_gtest_perf_test.py [-h] --isolated-script-test-output
>                               ISOLATED_SCRIPT_TEST_OUTPUT
>                               --isolated-script-test-chartjson-output
>                               ISOLATED_SCRIPT_TEST_CHARTJSON_OUTPUT [--xvfb]
> run_gtest_perf_test.py: error: argument --isolated-script-test-output is
> required

This error is happening because this is the first non-Telemetry-based isolated
script test that generate_buildbot_json has dealt with so far. It'll be
necessary to put this in the "isolated_scripts" section of the resulting JSON
files.

However, the "generate_telemetry_test" function is a little over-specialized,
and I wouldn't recommend contorting it to make it work for this case. So, I
recommend:

1) Add a new section to the file, similar to COMMON_GTESTS, like
ISOLATED_SCRIPT_TESTS, or NON_TELEMETRY_ISOLATED_SCRIPT_TESTS. Put the new entry
for angle_perftests in there.

2) Write a new generator, similar to generate_telemetry_test, which outputs the
desired data into the JSON file. Note that the new generator shouldn't have any
references to telemetry_gpu_test or telemetry_gpu_integration_test; it should
pick up the name of the isolate from the name of the test (in this case,
angle_perftests).

3) Invoke this generator when assembling the isolated_scripts array in
generate_all_tests.

[Jamie Madill, 2017-01-05 13:46:52]

Thanks for the detailed guide Ken, but I'm confused about one thing.

Your instructions seem to read like you expect the json to be an input to the
perf tests. I think they're actually a temporary output file for the results of
the run. Just wanted to confirm I was understanding you correctly. Do you know
where I should place this temporary garbage file? Or how to clean it up? Even if
I follow your steps this isn't clear to me.

[Ken Russell (switch to Gerrit), 2017-01-05 16:02:35]

On 2017/01/05 13:46:52, Jamie Madill wrote:
> Thanks for the detailed guide Ken, but I'm confused about one thing.
> 
> Your instructions seem to read like you expect the json to be an input to the
> perf tests. I think they're actually a temporary output file for the results
of
> the run. Just wanted to confirm I was understanding you correctly. Do you know
> where I should place this temporary garbage file? Or how to clean it up? Even
if
> I follow your steps this isn't clear to me.

The recipes choose the name for this temporary output file and pass the
--isolated-script-test-output flag to the target test when it's the
"isolated_script" type rather than the "gtest" type.

Sorry, I now realize the description above was confusing. In step (2) above I
was talking about the chromium.gpu.fyi.json file. The main thing is to modify
generate_buildbot_json.py to have a new section for ISOLATED_SCRIPT_TESTS.

[Jamie Madill, 2017-01-05 16:06:58]

On 2017/01/05 16:02:35, Ken Russell wrote:
> On 2017/01/05 13:46:52, Jamie Madill wrote:
> > Thanks for the detailed guide Ken, but I'm confused about one thing.
> > 
> > Your instructions seem to read like you expect the json to be an input to
the
> > perf tests. I think they're actually a temporary output file for the results
> of
> > the run. Just wanted to confirm I was understanding you correctly. Do you
know
> > where I should place this temporary garbage file? Or how to clean it up?
Even
> if
> > I follow your steps this isn't clear to me.
> 
> The recipes choose the name for this temporary output file and pass the
> --isolated-script-test-output flag to the target test when it's the
> "isolated_script" type rather than the "gtest" type.
> 
> Sorry, I now realize the description above was confusing. In step (2) above I
> was talking about the chromium.gpu.fyi.json file. The main thing is to modify
> generate_buildbot_json.py to have a new section for ISOLATED_SCRIPT_TESTS.

Ah, yes, thank you, I was confused by the use of the term json. I'll take a
look, thanks for the advice!

[Jamie Madill, 2017-01-10 00:15:42]

CQ'ing this, thanks for the help Ken. Feel free to de-CQ if there's something
objectionable, but for expediency I am going to CQ now.

[Jamie Madill, 2017-01-10 00:15:45]

The CQ bit was checked by jmadill@chromium.org

[Jamie Madill, 2017-01-10 00:15:45]

The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org
Link to the patchset: https://codereview.chromium.org/2591813002/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2017-01-10 00:17:34]

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

[Ken Russell (switch to Gerrit), 2017-01-10 00:22:34]

LGTM. Thanks for taking care of this. Two minor comments which you don't have to
address in this commit.

https://codereview.chromium.org/2591813002/diff/80001/content/test/gpu/genera...
File content/test/gpu/generate_buildbot_json.py (right):

https://codereview.chromium.org/2591813002/diff/80001/content/test/gpu/genera...
content/test/gpu/generate_buildbot_json.py:1625: def
generate_non_telemetry_isolated_test(tester_name, tester_config,
Could you add a TODO about refactoring this and generate_telemetry_test? There
could be a base function "generate_isolated_test" which takes in the additional
information that generate_telemetry_test computes (like
extra_browser_args_string and the isolate name), and both this one and the other
could call it, reducing duplicated code.

https://codereview.chromium.org/2591813002/diff/80001/content/test/gpu/genera...
content/test/gpu/generate_buildbot_json.py:1645: # Prepend Telemetry
GPU-specific flags.
Update: "Prepend GPU-specific flags."

[commit-bot: I haz the power, 2017-01-10 02:19:52]

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

[commit-bot: I haz the power, 2017-01-10 02:19:53]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)

[Jamie Madill, 2017-01-10 15:09:59]

On 2017/01/10 00:22:34, Ken Russell wrote:
> LGTM. Thanks for taking care of this. Two minor comments which you don't have
to
> address in this commit.
> 
>
https://codereview.chromium.org/2591813002/diff/80001/content/test/gpu/genera...
> File content/test/gpu/generate_buildbot_json.py (right):
> 
>
https://codereview.chromium.org/2591813002/diff/80001/content/test/gpu/genera...
> content/test/gpu/generate_buildbot_json.py:1625: def
> generate_non_telemetry_isolated_test(tester_name, tester_config,
> Could you add a TODO about refactoring this and generate_telemetry_test? There
> could be a base function "generate_isolated_test" which takes in the
additional
> information that generate_telemetry_test computes (like
> extra_browser_args_string and the isolate name), and both this one and the
other
> could call it, reducing duplicated code.
> 
>
https://codereview.chromium.org/2591813002/diff/80001/content/test/gpu/genera...
> content/test/gpu/generate_buildbot_json.py:1645: # Prepend Telemetry
> GPU-specific flags.
> Update: "Prepend GPU-specific flags."

Thanks Ken, addressed feedback in Patch Set 6.

[Jamie Madill, 2017-01-10 15:10:05]

The CQ bit was checked by jmadill@chromium.org

[Jamie Madill, 2017-01-10 15:10:06]

The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org
Link to the patchset: https://codereview.chromium.org/2591813002/#ps100001
(title: "Use shared base generator func.")

[commit-bot: I haz the power, 2017-01-10 15:10:20]

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, 2017-01-10 16:21:52]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1484061005935870,
"parent_rev": "3893dd4061e1c702a96da38124c229adfe6bb441", "commit_rev":
"ecbaf9756e7cc6f7e4743a614a32ff2e6c3ed2a9"}

[commit-bot: I haz the power, 2017-01-10 16:22:21]

Description was changed from

==========
Run angle_perftests on the GPU FYI bots.

This will help safeguard against future breakage, and be a place to
test the tests before we add them to other bots.

BUG=675997
R=kbr@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
==========

to

==========
Run angle_perftests on the GPU FYI bots.

This will help safeguard against future breakage, and be a place to
test the tests before we add them to other bots.

BUG=675997
R=kbr@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2591813002
Cr-Commit-Position: refs/heads/master@{#442605}
Committed:
https://chromium.googlesource.com/chromium/src/+/ecbaf9756e7cc6f7e4743a614a32...
==========

[commit-bot: I haz the power, 2017-01-10 16:22:22]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/ecbaf9756e7cc6f7e4743a614a32...