Converts resource_provider apptests to unit tests

Converts resource_provider apptests to unit tests

BUG=594729
TEST=test only change
R=erg@chromium.org, dpranke@chromium.org
TBR=dpranke@chromium.org

Committed: https://crrev.com/4192192a724363263fd878f99c7ca38cc42550f7
Cr-Commit-Position: refs/heads/master@{#382594}

[Elliot Glaysher, 2016-03-21 22:35:45]

lgtm, but you'll need a top level owner for BUILD.gn.

[sky, 2016-03-21 22:40:45]

Description was changed from

==========
Converts resource_provider apptests to unit tests

BUG=594729
TEST=test only change
R=erg@chromium.org
==========

to

==========
Converts resource_provider apptests to unit tests

BUG=594729
TEST=test only change
R=erg@chromium.org, dpranke@chromium.org
TBR=dpranke@chromium.org
==========

[sky, 2016-03-21 22:41:01]

sky@chromium.org changed reviewers:
+ dpranke@chromium.org

[sky, 2016-03-21 22:41:23]

And for that I'll TBR Dirk.

[sky, 2016-03-21 22:41:29]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2016-03-21 22:41:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1816323002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1816323002/20001

[commit-bot: I haz the power, 2016-03-21 22:56:00]

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

[commit-bot: I haz the power, 2016-03-21 22:56:01]

Try jobs failed on following builders:
  linux_blink_oilpan_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_blink_oil...)

[Dirk Pranke, 2016-03-21 22:57:32]

https://codereview.chromium.org/1816323002/diff/20001/components/resource_pro...
File components/resource_provider/BUILD.gn (right):

https://codereview.chromium.org/1816323002/diff/20001/components/resource_pro...
components/resource_provider/BUILD.gn:114: output_name =
"resource_provider_unittests"
GN style is to have the target_name match the binary name for test binaries,
rather than setting the output_name; you should really only use output_name when
you have to for some reason.

[sky, 2016-03-21 23:05:28]

https://codereview.chromium.org/1816323002/diff/20001/components/resource_pro...
File components/resource_provider/BUILD.gn (right):

https://codereview.chromium.org/1816323002/diff/20001/components/resource_pro...
components/resource_provider/BUILD.gn:114: output_name =
"resource_provider_unittests"
On 2016/03/21 22:57:31, Dirk Pranke wrote:
> GN style is to have the target_name match the binary name for test binaries,
> rather than setting the output_name; you should really only use output_name
when
> you have to for some reason.

Is that decision up for discussion? It seems to me typing
components/resource_provider:unittests makes a lot more sense than the redundant
components/resource_provider:resource_provider_unittests.

[sky, 2016-03-21 23:15:04]

https://codereview.chromium.org/1816323002/diff/20001/components/resource_pro...
File components/resource_provider/BUILD.gn (right):

https://codereview.chromium.org/1816323002/diff/20001/components/resource_pro...
components/resource_provider/BUILD.gn:114: output_name =
"resource_provider_unittests"
On 2016/03/21 23:05:28, sky wrote:
> On 2016/03/21 22:57:31, Dirk Pranke wrote:
> > GN style is to have the target_name match the binary name for test binaries,
> > rather than setting the output_name; you should really only use output_name
> when
> > you have to for some reason.
> 
> Is that decision up for discussion? It seems to me typing
> components/resource_provider:unittests makes a lot more sense than the
redundant
> components/resource_provider:resource_provider_unittests.

I talked with Brett and he convinced me to go_with_the_long_name. Updated.

[Dirk Pranke, 2016-03-21 23:36:24]

lgtm

[sky, 2016-03-21 23:37:09]

The CQ bit was checked by sky@chromium.org

[sky, 2016-03-21 23:37:09]

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

[commit-bot: I haz the power, 2016-03-21 23:37:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1816323002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1816323002/40001

[Dirk Pranke, 2016-03-21 23:40:52]

On 2016/03/21 23:15:04, sky wrote:
> I talked with Brett and he convinced me to go_with_the_long_name. Updated.

IIRC, one major reason is that GN will generate a 'short' build target name
that will map to the longer label-based target name for unique executables, 
but I think that we don't take output_name into account for that. I don't
remember if we *can't* do that or not, though, but maybe Brett did?

[commit-bot: I haz the power, 2016-03-22 00:00:24]

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

[commit-bot: I haz the power, 2016-03-22 00:00:26]

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

[sky, 2016-03-22 16:11:36]

The CQ bit was checked by sky@chromium.org

[sky, 2016-03-22 16:11:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, erg@chromium.org
Link to the patchset: https://codereview.chromium.org/1816323002/#ps80001
(title: "add quit to delegate and remove dep")

[commit-bot: I haz the power, 2016-03-22 16:11:48]

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

[Ben Goodger (Google), 2016-03-22 16:33:44]

On 2016/03/21 23:40:52, Dirk Pranke wrote:
> On 2016/03/21 23:15:04, sky wrote:
> > I talked with Brett and he convinced me to go_with_the_long_name. Updated.
> 
> IIRC, one major reason is that GN will generate a 'short' build target name
> that will map to the longer label-based target name for unique executables, 
> but I think that we don't take output_name into account for that. I don't
> remember if we *can't* do that or not, though, but maybe Brett did?

It sure would be nice... since we have a pattern in many places that do things
like:

foo\bar:all
foo\bar:lib
foo\bar:test_support

so it makes sense to have

foo\bar:unittests

FTR, I'm used to typing these fully qualified paths now.

[commit-bot: I haz the power, 2016-03-22 17:20:52]

Description was changed from

==========
Converts resource_provider apptests to unit tests

BUG=594729
TEST=test only change
R=erg@chromium.org, dpranke@chromium.org
TBR=dpranke@chromium.org
==========

to

==========
Converts resource_provider apptests to unit tests

BUG=594729
TEST=test only change
R=erg@chromium.org, dpranke@chromium.org
TBR=dpranke@chromium.org
==========

[commit-bot: I haz the power, 2016-03-22 17:20:53]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-03-22 17:22:21]

Description was changed from

==========
Converts resource_provider apptests to unit tests

BUG=594729
TEST=test only change
R=erg@chromium.org, dpranke@chromium.org
TBR=dpranke@chromium.org
==========

to

==========
Converts resource_provider apptests to unit tests

BUG=594729
TEST=test only change
R=erg@chromium.org, dpranke@chromium.org
TBR=dpranke@chromium.org

Committed: https://crrev.com/4192192a724363263fd878f99c7ca38cc42550f7
Cr-Commit-Position: refs/heads/master@{#382594}
==========

[commit-bot: I haz the power, 2016-03-22 17:22:22]

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