mash: Add catalog/manifest etc. for telemetry tests binary for mash.

mash: Add catalog/manifest etc. for telemetry tests binary for mash.

For telemetry tests, the browser needs to get access to some test api
from the mus window server, which is only exposed to clients marked as
'test' in their manifest. Instead of making default chrome-browser such a
test client, create a separate telemetry-browser catalog, which is used
when run with --use-test-config command-line flag. Instead of adding
dependency on this test catalog from the main chrome executable, create a
separate test_chrome executable that can be used for running the
telemetry tests.

BUG=704681, 721640
Review-Url: https://codereview.chromium.org/2910383002
Cr-Commit-Position: refs/heads/master@{#477326}
Committed: https://chromium.googlesource.com/chromium/src/+/77c67de3a506c39bf84fec3f92585c507f16f29f

[sadrul, 2017-05-31 05:43:05]

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

[commit-bot: I haz the power, 2017-05-31 05:43:19]

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, 2017-05-31 05:48:00]

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

[commit-bot: I haz the power, 2017-05-31 05:48:01]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sadrul, 2017-05-31 06:22:57]

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

[commit-bot: I haz the power, 2017-05-31 06:23:20]

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, 2017-05-31 07:47:38]

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

[commit-bot: I haz the power, 2017-05-31 07:47:39]

Dry run: This issue passed the CQ dry run.

[sadrul, 2017-05-31 07:53:03]

Description was changed from

==========
wip: Add catalog/manifest etc. for telemetry tests binary for mash.

...

BUG=...
==========

to

==========
mash: Add catalog/manifest etc. for telemetry tests binary for mash.

For telemetry tests, the browser needs to get access to some test api
from the mus window server, which is only exposed to clients marked as
'test' in their manifest. Instead of making default chrome-browser such a
test client, create a separate telemetry-browser catalog, which is used
when run with --use-test-config command-line flag.

BUG=704681
==========

[sadrul, 2017-05-31 07:54:30]

sadrul@chromium.org changed reviewers:
+ rockot@chromium.org, sky@chromium.org

[Ken Rockot(use gerrit already), 2017-05-31 11:34:23]

If we're going to add yet another catalog config to maintain for Chrome, maybe
we should at least generalize its name for any kind of testing environment and
not limit it specifically to telemetry.

https://codereview.chromium.org/2910383002/diff/20001/chrome/app/mash/BUILD.gn
File chrome/app/mash/BUILD.gn (right):

https://codereview.chromium.org/2910383002/diff/20001/chrome/app/mash/BUILD.g...
chrome/app/mash/BUILD.gn:103: catalog("catalog_telemetry") {
You should be able to consolidate most of the duplication into a common base
catalog target, and then add that to the catalog_deps of both the ":catalog" and
":catalog_telemetry" targets.

[sadrul, 2017-05-31 13:17:10]

Patchset #3 (id:40001) has been deleted

[sadrul, 2017-05-31 13:18:37]

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

[commit-bot: I haz the power, 2017-05-31 13:18:54]

Dry run: CQ is trying da patch.

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

[sadrul, 2017-05-31 13:19:24]

https://codereview.chromium.org/2910383002/diff/20001/chrome/app/mash/BUILD.gn
File chrome/app/mash/BUILD.gn (right):

https://codereview.chromium.org/2910383002/diff/20001/chrome/app/mash/BUILD.g...
chrome/app/mash/BUILD.gn:103: catalog("catalog_telemetry") {
On 2017/05/31 11:34:23, Ken Rockot(use gerrit already) wrote:
> You should be able to consolidate most of the duplication into a common base
> catalog target, and then add that to the catalog_deps of both the ":catalog"
and
> ":catalog_telemetry" targets.

Done.

[commit-bot: I haz the power, 2017-05-31 14:52:51]

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

[commit-bot: I haz the power, 2017-05-31 14:52:52]

Dry run: This issue passed the CQ dry run.

[sadrul, 2017-05-31 19:38:02]

Description was changed from

==========
mash: Add catalog/manifest etc. for telemetry tests binary for mash.

For telemetry tests, the browser needs to get access to some test api
from the mus window server, which is only exposed to clients marked as
'test' in their manifest. Instead of making default chrome-browser such a
test client, create a separate telemetry-browser catalog, which is used
when run with --use-test-config command-line flag.

BUG=704681
==========

to

==========
mash: Add catalog/manifest etc. for telemetry tests binary for mash.

For telemetry tests, the browser needs to get access to some test api
from the mus window server, which is only exposed to clients marked as
'test' in their manifest. Instead of making default chrome-browser such a
test client, create a separate telemetry-browser catalog, which is used
when run with --use-test-config command-line flag.

BUG=704681, 721640
==========

[sadrul, 2017-06-01 16:58:10]

ping

[Ken Rockot(use gerrit already), 2017-06-01 17:03:24]

Oops, sorry, forgot to stamp. LGTM!

[sky, 2017-06-01 20:45:25]

https://codereview.chromium.org/2910383002/diff/60001/chrome/app/chrome_main_...
File chrome/app/chrome_main_delegate.cc (right):

https://codereview.chromium.org/2910383002/diff/60001/chrome/app/chrome_main_...
chrome/app/chrome_main_delegate.cc:1139: if
(command_line.HasSwitch("use-test-config"))
Should we consider a completely separate build target for this? Having a
separate build target could mean we only expose test-only interfaces in custom
builds and not the main line chrome.

[sadrul, 2017-06-01 20:49:43]

https://codereview.chromium.org/2910383002/diff/60001/chrome/app/chrome_main_...
File chrome/app/chrome_main_delegate.cc (right):

https://codereview.chromium.org/2910383002/diff/60001/chrome/app/chrome_main_...
chrome/app/chrome_main_delegate.cc:1139: if
(command_line.HasSwitch("use-test-config"))
On 2017/06/01 20:45:24, sky wrote:
> Should we consider a completely separate build target for this? Having a
> separate build target could mean we only expose test-only interfaces in custom
> builds and not the main line chrome.

Yep. That's where I was initially going. To do that, we would need to install
some sort of hook into here from before ChromeMain() is called. I thought that
would be too much, but I can do that if you think that's OK.

[sky, 2017-06-01 22:03:35]

I think that would be better in that less test only code is shipped. If you
are concerned about the hook, maybe talk with Tom?

On Thu, Jun 1, 2017 at 1:49 PM, <sadrul@chromium.org> wrote:

>
> https://codereview.chromium.org/2910383002/diff/60001/
> chrome/app/chrome_main_delegate.cc
> File chrome/app/chrome_main_delegate.cc (right):
>
> https://codereview.chromium.org/2910383002/diff/60001/
> chrome/app/chrome_main_delegate.cc#newcode1139
> chrome/app/chrome_main_delegate.cc:1139: if
> (command_line.HasSwitch("use-test-config"))
> On 2017/06/01 20:45:24, sky wrote:
> > Should we consider a completely separate build target for this? Having
> a
> > separate build target could mean we only expose test-only interfaces
> in custom
> > builds and not the main line chrome.
>
> Yep. That's where I was initially going. To do that, we would need to
> install some sort of hook into here from before ChromeMain() is called.
> I thought that would be too much, but I can do that if you think that's
> OK.
>
> https://codereview.chromium.org/2910383002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sadrul, 2017-06-06 04:50:46]

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

[commit-bot: I haz the power, 2017-06-06 04:51:03]

Dry run: CQ is trying da patch.

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

[sadrul, 2017-06-06 04:53:27]

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

[commit-bot: I haz the power, 2017-06-06 04:53:42]

Dry run: CQ is trying da patch.

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

[sadrul, 2017-06-06 04:57:02]

Patchset #5 (id:100001) has been deleted

[sadrul, 2017-06-06 04:57:09]

Patchset #4 (id:80001) has been deleted

[commit-bot: I haz the power, 2017-06-06 04:58:11]

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

[commit-bot: I haz the power, 2017-06-06 04:58:12]

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

[sadrul, 2017-06-06 04:58:24]

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

[commit-bot: I haz the power, 2017-06-06 04:58:37]

Dry run: CQ is trying da patch.

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

[sadrul, 2017-06-06 04:58:55]

Patchset #4 (id:120001) has been deleted

[commit-bot: I haz the power, 2017-06-06 05:13:56]

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

[commit-bot: I haz the power, 2017-06-06 05:13:58]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[sadrul, 2017-06-06 05:16:21]

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

[commit-bot: I haz the power, 2017-06-06 05:16:38]

Dry run: CQ is trying da patch.

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

[sadrul, 2017-06-06 05:19:19]

Description was changed from

==========
mash: Add catalog/manifest etc. for telemetry tests binary for mash.

For telemetry tests, the browser needs to get access to some test api
from the mus window server, which is only exposed to clients marked as
'test' in their manifest. Instead of making default chrome-browser such a
test client, create a separate telemetry-browser catalog, which is used
when run with --use-test-config command-line flag.

BUG=704681, 721640
==========

to

==========
mash: Add catalog/manifest etc. for telemetry tests binary for mash.

For telemetry tests, the browser needs to get access to some test api
from the mus window server, which is only exposed to clients marked as
'test' in their manifest. Instead of making default chrome-browser such a
test client, create a separate telemetry-browser catalog, which is used
when run with --use-test-config command-line flag. Instead of adding
dependency on this test catalog from the main chrome executable, create a
separate test_chrome executable that can be used for running the
telemetry tests.

BUG=704681, 721640
==========

[sadrul, 2017-06-06 05:21:07]

On 2017/06/01 22:03:35, sky wrote:
> I think that would be better in that less test only code is shipped. If you
> are concerned about the hook, maybe talk with Tom?

I wasn't concerned in terms of security, it was more in terms of
code-layering/cleanliness.

I have made this change, where we create a separate binary for running the
telemetry tests, and only this binary gets the dependency on the test catalog.
PTAL.

[commit-bot: I haz the power, 2017-06-06 06:27:18]

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

[commit-bot: I haz the power, 2017-06-06 06:27:19]

Dry run: This issue passed the CQ dry run.

[sky, 2017-06-06 16:12:31]

Nice, LGTM

[sadrul, 2017-06-06 17:18:58]

The CQ bit was checked by sadrul@chromium.org

[sadrul, 2017-06-06 17:18:58]

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

[commit-bot: I haz the power, 2017-06-06 17:19:18]

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-06-06 17:29:08]

CQ is committing da patch.

Bot data: {"patchset_id": 140002, "attempt_start_ts": 1496769538137310,
"parent_rev": "62751ceeb02adc855c96e93127c1bee6c6dc9c33", "commit_rev":
"77c67de3a506c39bf84fec3f92585c507f16f29f"}

[commit-bot: I haz the power, 2017-06-06 17:29:23]

Description was changed from

==========
mash: Add catalog/manifest etc. for telemetry tests binary for mash.

For telemetry tests, the browser needs to get access to some test api
from the mus window server, which is only exposed to clients marked as
'test' in their manifest. Instead of making default chrome-browser such a
test client, create a separate telemetry-browser catalog, which is used
when run with --use-test-config command-line flag. Instead of adding
dependency on this test catalog from the main chrome executable, create a
separate test_chrome executable that can be used for running the
telemetry tests.

BUG=704681, 721640
==========

to

==========
mash: Add catalog/manifest etc. for telemetry tests binary for mash.

For telemetry tests, the browser needs to get access to some test api
from the mus window server, which is only exposed to clients marked as
'test' in their manifest. Instead of making default chrome-browser such a
test client, create a separate telemetry-browser catalog, which is used
when run with --use-test-config command-line flag. Instead of adding
dependency on this test catalog from the main chrome executable, create a
separate test_chrome executable that can be used for running the
telemetry tests.

BUG=704681, 721640

Review-Url: https://codereview.chromium.org/2910383002
Cr-Commit-Position: refs/heads/master@{#477326}
Committed:
https://chromium.googlesource.com/chromium/src/+/77c67de3a506c39bf84fec3f9258...
==========

[commit-bot: I haz the power, 2017-06-06 17:29:25]

Committed patchset #5 (id:140002) as
https://chromium.googlesource.com/chromium/src/+/77c67de3a506c39bf84fec3f9258...