Update fieldtrial_util To Handle Combined Fieldtrial Config Format

Update fieldtrial_util To Handle Combined Fieldtrial Config Format

BUG=649363, 650705
Committed: https://crrev.com/8da8200a279fcafc4a7c68f2e6ef95f4d721bb3a
Cr-Commit-Position: refs/heads/master@{#421670}

[robliao, 2016-09-27 19:01:58]

Description was changed from

==========
Update fieldtrial_util To Handle Combined Fieldtrial Config Format

BUG=649363
==========

to

==========
Update fieldtrial_util To Handle Combined Fieldtrial Config Format

BUG=649363, 650705
==========

[robliao, 2016-09-27 19:02:04]

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

[commit-bot: I haz the power, 2016-09-27 19:03:19]

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

[robliao, 2016-09-27 19:13:43]

robliao@chromium.org changed reviewers:
+ aiolos@chromium.org, asvitkine@chromium.org

[robliao, 2016-09-27 19:13:44]

aiolos@chromium.org: Please review changes in 
tools/perf/*

asvitkine@chromium.org: Please review changes in 
tools/variations/*

Thanks!

[aiolos (Not reviewing), 2016-09-27 19:26:03]

https://codereview.chromium.org/2373843002/diff/1/tools/perf/core/perf_benchm...
File tools/perf/core/perf_benchmark.py (right):

https://codereview.chromium.org/2373843002/diff/1/tools/perf/core/perf_benchm...
tools/perf/core/perf_benchmark.py:62: os.path.join(variations_dir,
'fieldtrial_testing_config_.json'),
Have the platform-specific configs already been migrated/duplicated over to this
new config?

https://codereview.chromium.org/2373843002/diff/1/tools/variations/fieldtrial...
File tools/variations/fieldtrial_util.py (right):

https://codereview.chromium.org/2373843002/diff/1/tools/variations/fieldtrial...
tools/variations/fieldtrial_util.py:107: (sys.argv[2], supported_platforms))
I'm not sure why you don't exit here. Does it make sense to generate the args
for an unknown platform?

[robliao, 2016-09-27 19:31:23]

https://codereview.chromium.org/2373843002/diff/1/tools/perf/core/perf_benchm...
File tools/perf/core/perf_benchmark.py (right):

https://codereview.chromium.org/2373843002/diff/1/tools/perf/core/perf_benchm...
tools/perf/core/perf_benchmark.py:62: os.path.join(variations_dir,
'fieldtrial_testing_config_.json'),
On 2016/09/27 19:26:03, aiolos wrote:
> Have the platform-specific configs already been migrated/duplicated over to
this
> new config?

Yes, and I missed this usage. 
The migration occurred on
https://codereview.chromium.org/2296493002/

https://codereview.chromium.org/2373843002/diff/1/tools/variations/fieldtrial...
File tools/variations/fieldtrial_util.py (right):

https://codereview.chromium.org/2373843002/diff/1/tools/variations/fieldtrial...
tools/variations/fieldtrial_util.py:107: (sys.argv[2], supported_platforms))
On 2016/09/27 19:26:03, aiolos wrote:
> I'm not sure why you don't exit here. Does it make sense to generate the args
> for an unknown platform?

Nope. Done.

[robliao, 2016-09-27 19:31:28]

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

[commit-bot: I haz the power, 2016-09-27 19:32:43]

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

[aiolos (Not reviewing), 2016-09-27 19:34:48]

lgtm

[commit-bot: I haz the power, 2016-09-27 20:20:45]

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

[commit-bot: I haz the power, 2016-09-27 20:20:46]

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

[Alexei Svitkine (slow), 2016-09-27 20:26:55]

lgtm

https://codereview.chromium.org/2373843002/diff/20001/tools/variations/fieldt...
File tools/variations/fieldtrial_util.py (right):

https://codereview.chromium.org/2373843002/diff/20001/tools/variations/fieldt...
tools/variations/fieldtrial_util.py:49: # fieldtrial_testing_config_*.json.
Update comment.

[robliao, 2016-09-27 20:41:51]

The CQ bit was checked by robliao@chromium.org

[robliao, 2016-09-27 20:41:52]

The patchset sent to the CQ was uploaded after l-g-t-m from aiolos@chromium.org,
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2373843002/#ps40001
(title: "Update Comment")

[commit-bot: I haz the power, 2016-09-27 20:42:16]

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

[robliao, 2016-09-27 20:42:16]

https://codereview.chromium.org/2373843002/diff/20001/tools/variations/fieldt...
File tools/variations/fieldtrial_util.py (right):

https://codereview.chromium.org/2373843002/diff/20001/tools/variations/fieldt...
tools/variations/fieldtrial_util.py:49: # fieldtrial_testing_config_*.json.
On 2016/09/27 20:26:55, Alexei Svitkine (very slow) wrote:
> Update comment.

Done.

[robliao, 2016-09-27 20:58:32]

The CQ bit was unchecked by robliao@chromium.org

[robliao, 2016-09-27 21:14:21]

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

[commit-bot: I haz the power, 2016-09-27 21:14:51]

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

[robliao, 2016-09-27 22:18:16]

Patchset #4 (id:60001) has been deleted

[robliao, 2016-09-27 22:18:20]

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

[commit-bot: I haz the power, 2016-09-27 22:18:38]

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

[robliao, 2016-09-27 22:56:48]

FYI, had to add two dependencies to handle telemetry_perf_unittests 

+    # Field trial dependencies
+    "//tools/json_comment_eater/",
+    "//tools/json_to_struct/",
+

[commit-bot: I haz the power, 2016-09-27 23:31:02]

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

[commit-bot: I haz the power, 2016-09-27 23:31:03]

Dry run: This issue passed the CQ dry run.

[robliao, 2016-09-27 23:31:55]

The CQ bit was checked by robliao@chromium.org

[robliao, 2016-09-27 23:31:55]

The patchset sent to the CQ was uploaded after l-g-t-m from aiolos@chromium.org,
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2373843002/#ps80001
(title: "Add Dependencies to telemetry_perf_unittests")

[commit-bot: I haz the power, 2016-09-27 23:32: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, 2016-09-27 23:38:23]

Description was changed from

==========
Update fieldtrial_util To Handle Combined Fieldtrial Config Format

BUG=649363, 650705
==========

to

==========
Update fieldtrial_util To Handle Combined Fieldtrial Config Format

BUG=649363, 650705
==========

[commit-bot: I haz the power, 2016-09-27 23:38:27]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-09-27 23:41:33]

Description was changed from

==========
Update fieldtrial_util To Handle Combined Fieldtrial Config Format

BUG=649363, 650705
==========

to

==========
Update fieldtrial_util To Handle Combined Fieldtrial Config Format

BUG=649363, 650705

Committed: https://crrev.com/1c70046ee9bfd7f0159b3408331e26b9f61ef49f
Cr-Commit-Position: refs/heads/master@{#421384}
==========

[commit-bot: I haz the power, 2016-09-27 23:41:34]

Patchset 4 (id:??) landed as
https://crrev.com/1c70046ee9bfd7f0159b3408331e26b9f61ef49f
Cr-Commit-Position: refs/heads/master@{#421384}

[robliao, 2016-09-28 20:38:24]

A revert of this CL (patchset #4 id:80001) has been created in
https://codereview.chromium.org/2381663002/ by robliao@chromium.org.

The reason for reverting is: Break in 
https://build.chromium.org/p/chromiumos.chromium/builders/amd64-generic-telem...

This isn't part of the CQ run it appears..

[robliao, 2016-09-28 20:47:43]

Description was changed from

==========
Update fieldtrial_util To Handle Combined Fieldtrial Config Format

BUG=649363, 650705

Committed: https://crrev.com/1c70046ee9bfd7f0159b3408331e26b9f61ef49f
Cr-Commit-Position: refs/heads/master@{#421384}
==========

to

==========
Update fieldtrial_util To Handle Combined Fieldtrial Config Format

BUG=649363, 650705

Committed: https://crrev.com/1c70046ee9bfd7f0159b3408331e26b9f61ef49f
Cr-Commit-Position: refs/heads/master@{#421384}
==========

[robliao, 2016-09-28 21:16:00]

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

[commit-bot: I haz the power, 2016-09-28 21:16:30]

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

[robliao, 2016-09-28 21:19:02]

Description was changed from

==========
Update fieldtrial_util To Handle Combined Fieldtrial Config Format

BUG=649363, 650705

Committed: https://crrev.com/1c70046ee9bfd7f0159b3408331e26b9f61ef49f
Cr-Commit-Position: refs/heads/master@{#421384}
==========

to

==========
Update fieldtrial_util To Handle Combined Fieldtrial Config Format

BUG=649363, 650705
==========

[robliao, 2016-09-28 21:33:18]

FYI, added bootstrap_deps updates too. CrOS runs a parallel test system that
isn't covered in our CQ.

Rudimentary update to bootstrap_deps:
     "src/chrome/test/data/third_party/spaceport":
        
"https://src.chromium.org/chrome/trunk/src/chrome/test/data/third_party/spaceport",
+    "src/tools/json_comment_eater":
+        "https://src.chromium.org/chrome/trunk/src/tools/json_comment_eater",
+    "src/tools/json_to_struct":
+        "https://src.chromium.org/chrome/trunk/src/tools/json_to_struct",
     "src/tools/perf/benchmarks":
         "https://src.chromium.org/chrome/trunk/src/tools/perf/benchmarks",

It also appears that the links are ignored and only the key is used, so we're
okay there and not going to try to refer to Chromium SVN.

[achuithb, 2016-09-28 21:36:03]

On 2016/09/28 20:38:24, robliao wrote:
> A revert of this CL (patchset #4 id:80001) has been created in
> https://codereview.chromium.org/2381663002/ by mailto:robliao@chromium.org.
> 
> The reason for reverting is: Break in 
>
https://build.chromium.org/p/chromiumos.chromium/builders/amd64-generic-telem...
> 
> This isn't part of the CQ run it appears..

Thanks so much for taking care of this.

[aiolos (Not reviewing), 2016-09-28 21:37:12]

On 2016/09/28 21:36:03, achuithb wrote:
> On 2016/09/28 20:38:24, robliao wrote:
> > A revert of this CL (patchset #4 id:80001) has been created in
> > https://codereview.chromium.org/2381663002/ by mailto:robliao@chromium.org.
> > 
> > The reason for reverting is: Break in 
> >
>
https://build.chromium.org/p/chromiumos.chromium/builders/amd64-generic-telem...
> > 
> > This isn't part of the CQ run it appears..
> 
> Thanks so much for taking care of this.

+1

[robliao, 2016-09-28 22:05:27]

On 2016/09/28 21:37:12, aiolos wrote:
> On 2016/09/28 21:36:03, achuithb wrote:
> > On 2016/09/28 20:38:24, robliao wrote:
> > > A revert of this CL (patchset #4 id:80001) has been created in
> > > https://codereview.chromium.org/2381663002/ by
mailto:robliao@chromium.org.
> > > 
> > > The reason for reverting is: Break in 
> > >
> >
>
https://build.chromium.org/p/chromiumos.chromium/builders/amd64-generic-telem...
> > > 
> > > This isn't part of the CQ run it appears..
> > 
> > Thanks so much for taking care of this.
> 
> +1

The CrOS trybot failed to apply the patch (we were partially expecting this), so
we're going to play the submit and see what happens game.

[commit-bot: I haz the power, 2016-09-28 22:40:06]

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

[commit-bot: I haz the power, 2016-09-28 22:40:07]

Dry run: This issue passed the CQ dry run.

[robliao, 2016-09-28 22:40:46]

The CQ bit was checked by robliao@chromium.org

[robliao, 2016-09-28 22:40:47]

The patchset sent to the CQ was uploaded after l-g-t-m from aiolos@chromium.org,
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2373843002/#ps120001
(title: "Update Bootstrap Deps")

[commit-bot: I haz the power, 2016-09-28 22:41:10]

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-09-28 22:47:51]

Description was changed from

==========
Update fieldtrial_util To Handle Combined Fieldtrial Config Format

BUG=649363, 650705
==========

to

==========
Update fieldtrial_util To Handle Combined Fieldtrial Config Format

BUG=649363, 650705
==========

[commit-bot: I haz the power, 2016-09-28 22:47:52]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2016-09-28 22:51:18]

Description was changed from

==========
Update fieldtrial_util To Handle Combined Fieldtrial Config Format

BUG=649363, 650705
==========

to

==========
Update fieldtrial_util To Handle Combined Fieldtrial Config Format

BUG=649363, 650705

Committed: https://crrev.com/8da8200a279fcafc4a7c68f2e6ef95f4d721bb3a
Cr-Commit-Position: refs/heads/master@{#421670}
==========

[commit-bot: I haz the power, 2016-09-28 22:51:19]

Patchset 6 (id:??) landed as
https://crrev.com/8da8200a279fcafc4a7c68f2e6ef95f4d721bb3a
Cr-Commit-Position: refs/heads/master@{#421670}