Move test runner to a component

Move test runner to a component

BUG=478250
R=mkwst@chromium.org,torne@chromium.org,jam@chromium.org

Committed: https://crrev.com/73e711c4c67eea6ab339493aa7703a0e29a2f5bd
Cr-Commit-Position: refs/heads/master@{#332583}

[jochen (gone - plz use gerrit), 2015-06-02 09:45:32]

ptal

Mike, file moves & stuff
John, overall DEPS updates
Richard, android_webview/

[Mike West, 2015-06-02 10:30:08]

Move LGTM.

https://codereview.chromium.org/1167703002/diff/1/components/BUILD.gn
File components/BUILD.gn (right):

https://codereview.chromium.org/1167703002/diff/1/components/BUILD.gn#newcode243
components/BUILD.gn:243: "//components/test_runner",
I think you'll want to add a components/test_runner.gypi as well, otherwise I
don't think this will build under gyp. (Or does
components/test_runner/test_runner.gyp cover this?)

https://codereview.chromium.org/1167703002/diff/1/components/test_runner/comm...
File components/test_runner/common/OWNERS (right):

https://codereview.chromium.org/1167703002/diff/1/components/test_runner/comm...
components/test_runner/common/OWNERS:1: abarth@chromium.org
It might be reasonable to move this to components/test_runner/OWNERS, and drop
the common/renderer OWNERS files. Also, if you wouldn't mind adding me, I'd
appreciate it. :)

https://codereview.chromium.org/1167703002/diff/1/content/test/BUILD.gn
File content/test/BUILD.gn (right):

https://codereview.chromium.org/1167703002/diff/1/content/test/BUILD.gn#newco...
content/test/BUILD.gn:222: "//components/test_runner:test_runner",
content_tests.gypi?

[Torne, 2015-06-02 10:54:34]

android_webview LGTM

[jochen (gone - plz use gerrit), 2015-06-02 11:58:42]

https://codereview.chromium.org/1167703002/diff/1/components/BUILD.gn
File components/BUILD.gn (right):

https://codereview.chromium.org/1167703002/diff/1/components/BUILD.gn#newcode243
components/BUILD.gn:243: "//components/test_runner",
On 2015/06/02 10:30:08, Mike West wrote:
> I think you'll want to add a components/test_runner.gypi as well, otherwise I
> don't think this will build under gyp. (Or does
> components/test_runner/test_runner.gyp cover this?)

the gyp file covers that

https://codereview.chromium.org/1167703002/diff/1/content/test/BUILD.gn
File content/test/BUILD.gn (right):

https://codereview.chromium.org/1167703002/diff/1/content/test/BUILD.gn#newco...
content/test/BUILD.gn:222: "//components/test_runner:test_runner",
On 2015/06/02 at 10:30:08, Mike West wrote:
> content_tests.gypi?

what about it? It's somewhere in the middle of the file list...

[Mike West, 2015-06-02 16:28:01]

On 2015/06/02 at 11:58:42, jochen wrote:
>
https://codereview.chromium.org/1167703002/diff/1/content/test/BUILD.gn#newco...
> content/test/BUILD.gn:222: "//components/test_runner:test_runner",
> On 2015/06/02 at 10:30:08, Mike West wrote:
> > content_tests.gypi?
> 
> what about it? It's somewhere in the middle of the file list...

You're right; I didn't see it. Still LGTM.

[jam, 2015-06-02 18:02:59]

https://codereview.chromium.org/1167703002/diff/40001/components/test_runner/...
File components/test_runner/common/DEPS (right):

https://codereview.chromium.org/1167703002/diff/40001/components/test_runner/...
components/test_runner/common/DEPS:3: "+content/shell/common/test_runner",
update?

https://codereview.chromium.org/1167703002/diff/40001/components/test_runner/...
File components/test_runner/common/test_preferences.h (right):

https://codereview.chromium.org/1167703002/diff/40001/components/test_runner/...
components/test_runner/common/test_preferences.h:1: // Copyright 2014 The
Chromium Authors. All rights reserved.
Searching cs
https://code.google.com/p/chromium/codesearch#search/&q=%22/shell/common/test...

i dont' see this being included in directories other than
content/shell/renderer/.

so can we fold components/test_runner/common and components/test_runner/renderer
into one directory (i.e. just move all this code into components/test_runner)? i
ask because having a renderer/common split into a componentized module that's
used by mandoline doesn't make as much sense because there's no "renderer"
process there.

https://codereview.chromium.org/1167703002/diff/40001/components/test_runner/...
components/test_runner/common/test_preferences.h:16: namespace content {
are you planning on updating the namespace to test_runner in all these files in
a followup?

[jochen (gone - plz use gerrit), 2015-06-02 18:36:27]

ptal

https://codereview.chromium.org/1167703002/diff/40001/components/test_runner/...
File components/test_runner/common/DEPS (right):

https://codereview.chromium.org/1167703002/diff/40001/components/test_runner/...
components/test_runner/common/DEPS:3: "+content/shell/common/test_runner",
On 2015/06/02 at 18:02:59, jam wrote:
> update?

done

https://codereview.chromium.org/1167703002/diff/40001/components/test_runner/...
File components/test_runner/common/test_preferences.h (right):

https://codereview.chromium.org/1167703002/diff/40001/components/test_runner/...
components/test_runner/common/test_preferences.h:1: // Copyright 2014 The
Chromium Authors. All rights reserved.
On 2015/06/02 at 18:02:59, jam wrote:
> Searching cs
>
https://code.google.com/p/chromium/codesearch#search/&q=%22/shell/common/test...
> 
> i dont' see this being included in directories other than
content/shell/renderer/.
> 
> so can we fold components/test_runner/common and
components/test_runner/renderer into one directory (i.e. just move all this code
into components/test_runner)? i ask because having a renderer/common split into
a componentized module that's used by mandoline doesn't make as much sense
because there's no "renderer" process there.

good point - IIRC we used to IPC that structure around, but obviously that's no
longer the case.

https://codereview.chromium.org/1167703002/diff/40001/components/test_runner/...
components/test_runner/common/test_preferences.h:16: namespace content {
On 2015/06/02 at 18:02:59, jam wrote:
> are you planning on updating the namespace to test_runner in all these files
in a followup?

yes

(and add export macros)

[jam, 2015-06-02 22:23:29]

lgtm

https://codereview.chromium.org/1167703002/diff/60001/components/test_runner/...
File components/test_runner/DEPS (right):

https://codereview.chromium.org/1167703002/diff/60001/components/test_runner/...
components/test_runner/DEPS:3: "+components/test_runner",
not needed?

[jochen (gone - plz use gerrit), 2015-06-03 07:12:50]

The CQ bit was checked by jochen@chromium.org

[jochen (gone - plz use gerrit), 2015-06-03 07:12:52]

The patchset sent to the CQ was uploaded after l-g-t-m from torne@chromium.org,
mkwst@chromium.org, jam@chromium.org
Link to the patchset: https://codereview.chromium.org/1167703002/#ps80001
(title: "updates")

[commit-bot: I haz the power, 2015-06-03 07:13:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1167703002/80001

[Mike West, 2015-06-03 09:07:01]

On 2015/06/03 at 07:13:40, commit-bot wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/1167703002/80001

Might be worth sending an FYI to blink-dev@ so folks will know where to go to
poke at layout test fundamentals in the future.

[commit-bot: I haz the power, 2015-06-03 10:01:56]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-06-03 10:02:54]

Patchset 5 (id:??) landed as
https://crrev.com/73e711c4c67eea6ab339493aa7703a0e29a2f5bd
Cr-Commit-Position: refs/heads/master@{#332583}