Enable cc_perftests through chromium.perf.json.

Enable cc_perftests through chromium.perf.json.


BUG=392620

[shatch, 2015-02-27 19:34:18]

simonhatch@chromium.org changed reviewers:
+ phajdan.jr@chromium.org

[shatch, 2015-02-27 19:34:49]

simonhatch@chromium.org changed reviewers:
+ danakj@chromium.org

[shatch, 2015-02-27 19:55:37]

Patchset #1 (id:1) has been deleted

[shatch, 2015-02-27 19:56:11]

danakj: Are these bots ok? I just enabled this on everything.

[danakj, 2015-02-27 19:59:59]

LGTM -gpu bots

https://codereview.chromium.org/963203002/diff/20001/testing/buildbot/chromiu...
File testing/buildbot/chromium.perf.json (right):

https://codereview.chromium.org/963203002/diff/20001/testing/buildbot/chromiu...
testing/buildbot/chromium.perf.json:74: "Win 7 ATI GPU Perf": {
GPU isn't necessary, there are no GPU related tests.

[shatch, 2015-02-27 20:05:46]

https://codereview.chromium.org/963203002/diff/20001/testing/buildbot/chromiu...
File testing/buildbot/chromium.perf.json (right):

https://codereview.chromium.org/963203002/diff/20001/testing/buildbot/chromiu...
testing/buildbot/chromium.perf.json:74: "Win 7 ATI GPU Perf": {
On 2015/02/27 19:59:59, danakj wrote:
> GPU isn't necessary, there are no GPU related tests.

Done.

[danakj, 2015-02-27 20:12:40]

danakj@chromium.org changed reviewers:
+ enne@chromium.org

[danakj, 2015-02-27 20:12:40]

LGTM

[sullivan, 2015-02-27 20:22:09]

The CQ bit was checked by sullivan@chromium.org

[sullivan, 2015-02-27 20:22:13]

The CQ bit was unchecked by sullivan@chromium.org

[shatch, 2015-03-02 15:22:25]

On 2015/02/27 20:12:40, danakj wrote:
> LGTM

phajdan: Need owners approval, ptal

[Paweł Hajdan Jr., 2015-03-02 18:52:25]

https://codereview.chromium.org/963203002/diff/40001/testing/buildbot/chromiu...
File testing/buildbot/chromium.perf.json (right):

https://codereview.chromium.org/963203002/diff/40001/testing/buildbot/chromiu...
testing/buildbot/chromium.perf.json:7: "args": ["cc_perftests",
"--single-process-tests"]
Please don't pass this from command line (--single-process-tests). Instead the
test binary should specify tests should run serially, see e.g.
interactive_ui_tests or media_perftests.

[shatch, 2015-03-02 21:03:58]

https://codereview.chromium.org/963203002/diff/40001/testing/buildbot/chromiu...
File testing/buildbot/chromium.perf.json (right):

https://codereview.chromium.org/963203002/diff/40001/testing/buildbot/chromiu...
testing/buildbot/chromium.perf.json:7: "args": ["cc_perftests",
"--single-process-tests"]
On 2015/03/02 18:52:25, Paweł Hajdan Jr. wrote:
> Please don't pass this from command line (--single-process-tests). Instead the
> test binary should specify tests should run serially, see e.g.
> interactive_ui_tests or media_perftests.

Ah I see they run serially anyway (crbug.com/236893). Added a stdio flag so that
the output is still parsed/sent to dashboard.

[Nico, 2015-03-02 22:59:18]

On 2015/03/02 21:03:58, shatch wrote:
>
https://codereview.chromium.org/963203002/diff/40001/testing/buildbot/chromiu...
> File testing/buildbot/chromium.perf.json (right):
> 
>
https://codereview.chromium.org/963203002/diff/40001/testing/buildbot/chromiu...
> testing/buildbot/chromium.perf.json:7: "args": ["cc_perftests",
> "--single-process-tests"]
> On 2015/03/02 18:52:25, Paweł Hajdan Jr. wrote:
> > Please don't pass this from command line (--single-process-tests). Instead
the
> > test binary should specify tests should run serially, see e.g.
> > interactive_ui_tests or media_perftests.
> 
> Ah I see they run serially anyway (crbug.com/236893). Added a stdio flag so
that
> the output is still parsed/sent to dashboard.

Repeating my question on https://codereview.chromium.org/962863002/: Why is this
stdio flag needed? Is this something devs would want to set when running the
test too? If so, should ti just be on by default?

[shatch, 2015-03-03 14:59:29]

On 2015/03/02 22:59:18, Nico wrote:
> On 2015/03/02 21:03:58, shatch wrote:
> >
>
https://codereview.chromium.org/963203002/diff/40001/testing/buildbot/chromiu...
> > File testing/buildbot/chromium.perf.json (right):
> > 
> >
>
https://codereview.chromium.org/963203002/diff/40001/testing/buildbot/chromiu...
> > testing/buildbot/chromium.perf.json:7: "args": ["cc_perftests",
> > "--single-process-tests"]
> > On 2015/03/02 18:52:25, Paweł Hajdan Jr. wrote:
> > > Please don't pass this from command line (--single-process-tests). Instead
> the
> > > test binary should specify tests should run serially, see e.g.
> > > interactive_ui_tests or media_perftests.
> > 
> > Ah I see they run serially anyway (crbug.com/236893). Added a stdio flag so
> that
> > the output is still parsed/sent to dashboard.
> 
> Repeating my question on https://codereview.chromium.org/962863002/: Why is
this
> stdio flag needed? Is this something devs would want to set when running the
> test too? If so, should ti just be on by default?

So my understanding is that there are only 2 supported paths to generating the
data for perf dashboard upload. In runtest.py we eventually call
_SendResultsToDashboard, which takes a log_processor. Telemetry tests always
generate a TelemetryResultsProcessor which outputs the new Telemetry ChartJson
format which is supported by the dashboard. Otherwise, the only log_processor
with support for generating data for the dashboard right now is
GraphingLogProcessor, which expects to parse the stdio of the test line by line.

It's not something that devs need when running the test, only needed so that the
results get sent to the perf dashboard.

[Nico, 2015-03-03 20:04:09]

Cool, thanks.

Should this flag be in the "which tests to run" metadata file, or should it be
in the bot config? The latter seems like the "more correct" place to me, but I'm
not sure.

[shatch, 2015-03-03 20:10:36]

On 2015/03/03 20:04:09, Nico wrote:
> Cool, thanks.
> 
> Should this flag be in the "which tests to run" metadata file, or should it be
> in the bot config? The latter seems like the "more correct" place to me, but
I'm
> not sure.

Maybe the metadata file isn't the best spot, but another option is we could also
add it to the gtest_perf_test.py script since I'm guessing all callers will want
the stdio so that this is uploaded to the dashboard.

I'm not super familiar with the whole build system, what do you mean by the bot
config?

[Paweł Hajdan Jr., 2015-03-04 14:23:10]

LGTM

Please move away from scraping the log output, and use JSON format for this
instead.

[Nico, 2015-03-04 19:30:25]

On 2015/03/03 20:10:36, shatch wrote:
> On 2015/03/03 20:04:09, Nico wrote:
> > Cool, thanks.
> > 
> > Should this flag be in the "which tests to run" metadata file, or should it
be
> > in the bot config? The latter seems like the "more correct" place to me, but
> I'm
> > not sure.
> 
> Maybe the metadata file isn't the best spot, but another option is we could
also
> add it to the gtest_perf_test.py script since I'm guessing all callers will
want
> the stdio so that this is uploaded to the dashboard.
> 
> I'm not super familiar with the whole build system, what do you mean by the
bot
> config?

There are lots of files that explain to the bots what build steps to run, which
flags to pass for each step, which env vars to set, and whatnot.

But if Pawel i shappy with this cl, go ahead as-is and ignore me :-)