Add trace config file for Chrome on CrOS

Add trace config file for Chrome on CrOS

TEST=run_benchmark --browser=cros-chrome --remote=<DUT_IP> page_cycler_v2.intl_ja_zh
BUG=chromium:632815
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/a88a01f8fb632b738c47701a2a86bdd0a3d4d8c4

[bccheng, 2016-08-09 10:06:55]

bccheng@chromium.org changed reviewers:
+ achuith@chromium.org, nednguyen@chromium.org

[nednguyen, 2016-08-09 12:16:56]

nednguyen@google.com changed reviewers:
+ nednguyen@google.com

[nednguyen, 2016-08-09 12:16:57]

Can you add test coverage? 

You can add @Enabled('chromeos') so that the test is only run against cros
platform.

[bccheng, 2016-08-09 14:56:30]

On 2016/08/09 12:16:57, nednguyen wrote:
> Can you add test coverage? 
> 
> You can add @Enabled('chromeos') so that the test is only run against cros
> platform.

OK will look into that.

[achuithb, 2016-08-09 19:47:03]

Thanks for digging into this!

https://codereview.chromium.org/2223963003/diff/1/telemetry/telemetry/interna...
File telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py
(right):

https://codereview.chromium.org/2223963003/diff/1/telemetry/telemetry/interna...
telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py:26:
_CHROME_TRACE_CONFIG_DIR_CROS = '/tmp/'
Would some randomization be better here? mktemp inside /tmp/. In case we have
left-over artifacts from a bad run/crash

[achuithb, 2016-08-09 19:47:22]

Description was changed from

==========
Add trace config file for Chrome on CrOS

TEST=run_benchmark --browser=cros-chrome --remote=<DUT_IP>
page_cycler_v2.intl_ja_zh
BUG=chromium:632815
==========

to

==========
Add trace config file for Chrome on CrOS

TEST=run_benchmark --browser=cros-chrome --remote=<DUT_IP>
page_cycler_v2.intl_ja_zh
BUG=chromium:632815
==========

[achuithb, 2016-08-09 19:47:22]

achuith@chromium.org changed reviewers:
- nednguyen@chromium.org

[achuithb, 2016-08-09 19:47:30]

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

[commit-bot: I haz the power, 2016-08-09 19:47:35]

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-08-09 20:13:59]

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

[commit-bot: I haz the power, 2016-08-09 20:14:00]

Dry run: This issue passed the CQ dry run.

[bccheng, 2016-08-10 05:15:56]

On 2016/08/09 19:47:03, achuithb wrote:
> Thanks for digging into this!
> 
>
https://codereview.chromium.org/2223963003/diff/1/telemetry/telemetry/interna...
> File
telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py
> (right):
> 
>
https://codereview.chromium.org/2223963003/diff/1/telemetry/telemetry/interna...
>
telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py:26:
> _CHROME_TRACE_CONFIG_DIR_CROS = '/tmp/'
> Would some randomization be better here? mktemp inside /tmp/. In case we have
> left-over artifacts from a bad run/crash

Unittests are added for CrOS.

I thought about adding randomization like the desktop case, but the Android
version doesn't do it and I believe it is due to the same reason that adding
randomization will take one more handshake from the DUT to workstation to return
the config file name. I think we should be fine to just use a fixed location
under /tmp for the trace config file.

[nednguyen, 2016-08-10 12:26:54]

lgtm

[bccheng, 2016-08-10 13:53:26]

The CQ bit was checked by bccheng@chromium.org

[commit-bot: I haz the power, 2016-08-10 13:53:38]

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-08-10 16:07:17]

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

[commit-bot: I haz the power, 2016-08-10 16:07:19]

Try jobs failed on following builders:
  Catapult Windows Tryserver on master.tryserver.client.catapult (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Wi...)

[nednguyen, 2016-08-10 16:09:03]

The CQ bit was checked by nednguyen@google.com

[commit-bot: I haz the power, 2016-08-10 16:09:08]

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

[achuithb, 2016-08-10 18:15:02]

On 2016/08/10 13:53:38, commit-bot: I haz the power wrote:
> CQ is trying da patch. Follow status at
>  
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

FYI, you should wait until all reviewers for your CL have had a chance to look
at the CL.

lgtm.

[commit-bot: I haz the power, 2016-08-10 18:26:24]

Description was changed from

==========
Add trace config file for Chrome on CrOS

TEST=run_benchmark --browser=cros-chrome --remote=<DUT_IP>
page_cycler_v2.intl_ja_zh
BUG=chromium:632815
==========

to

==========
Add trace config file for Chrome on CrOS

TEST=run_benchmark --browser=cros-chrome --remote=<DUT_IP>
page_cycler_v2.intl_ja_zh
BUG=chromium:632815

Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2016-08-10 18:26:26]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...

[bccheng, 2016-08-11 02:13:25]

On 2016/08/10 18:15:02, achuithb wrote:
> On 2016/08/10 13:53:38, commit-bot: I haz the power wrote:
> > CQ is trying da patch. Follow status at
> >  
> >
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
> 
> FYI, you should wait until all reviewers for your CL have had a chance to look
> at the CL.
> 
> lgtm.

OK noted!