Componentize component_updater: Move a bunch of tests to component.

Componentize component_updater: Move a bunch of tests to component.

BUG=371463
TBR=blundell

Committed: https://crrev.com/0409b4df1414ee0fb3e7c1179a86324944f242e3
Cr-Commit-Position: refs/heads/master@{#292026}

[tommycli, 2014-08-21 00:35:29]

sorin/waffles: PTAL. This just moves a bunch of tests into the component.

The checkperms is failing due to codereview not handling patches involving
binary files correctly. We may have to dcommit it once it's ready.

[tommycli, 2014-08-22 16:22:20]

sorin or waffles: ping

[waffles, 2014-08-22 16:29:27]

Yes, I remember there being issues with moving binary files around and the
trybots.

What do you think about splitting this into 3 CLs?:

1) Copy the binary files into components/test/data.
2) Move the tests.
3) Delete the (now unused) old binary files in chrome/test/data.

None of those three CLs should cause any test failures or require dcommit.

[tommycli, 2014-08-22 16:31:35]

On 2014/08/22 16:29:27, waffles wrote:
> Yes, I remember there being issues with moving binary files around and the
> trybots.
> 
> What do you think about splitting this into 3 CLs?:
> 
> 1) Copy the binary files into components/test/data.
> 2) Move the tests.
> 3) Delete the (now unused) old binary files in chrome/test/data.
> 
> None of those three CLs should cause any test failures or require dcommit.

waffles: Good idea. I'll do that.

[tommycli, 2014-08-22 16:44:59]

waffles: PTAL. I've removed all the test data file changes from this CL. This
only moves over the CC/H files now.

Step 3 is in a forthcoming CL.

[waffles, 2014-08-22 17:08:17]

lgtm

I dislike that component_updater_service.cc moved but its test did not.

On the other hand, this highlights the issue that the test was more of a test
for the entire overall behavior in Chrome rather than a test for the code
specifically in component_updater_service.cc. I don't think the test can be made
to move to components in a reasonable amount of effort, so let's leave it.

Honestly, I think we have to rewrite it anyways.

In the meantime, we might want to consider renaming it to better indicate its
function - but we can do that as a separate CL; so this lgtm.

[tommycli, 2014-08-22 17:16:17]

On 2014/08/22 17:08:17, waffles wrote:
> lgtm
> 
> I dislike that component_updater_service.cc moved but its test did not.
> 
> On the other hand, this highlights the issue that the test was more of a test
> for the entire overall behavior in Chrome rather than a test for the code
> specifically in component_updater_service.cc. I don't think the test can be
made
> to move to components in a reasonable amount of effort, so let's leave it.
> 
> Honestly, I think we have to rewrite it anyways.
> 
> In the meantime, we might want to consider renaming it to better indicate its
> function - but we can do that as a separate CL; so this lgtm.

waffles:

Believe me I tried! In my git history there's a split
component_updater_service_unitttest (moved to components) and
component_updater_resource_throtttle_unitttest (still in chrome/), but like you
said, it was too hard to disentangle.

Thanks for understanding.

[Sorin Jianu, 2014-08-22 17:19:59]

Grrrr... lgtm
Thank you Tommy!

https://codereview.chromium.org/493953002/diff/20001/chrome/browser/component...
File chrome/browser/component_updater/test/component_updater_service_unittest.cc
(right):

https://codereview.chromium.org/493953002/diff/20001/chrome/browser/component...
chrome/browser/component_updater/test/component_updater_service_unittest.cc:5:
#include
"chrome/browser/component_updater/test/component_updater_service_unittest.h"
We noticed this unit test is now separated from its production code. We are ok
with this, as we want to rewrite the component updater service, including its
tests in a way that removes its dependencies on src/content.

Until then, is there a practical way to keep the code and the unit test
together?

https://codereview.chromium.org/493953002/diff/20001/chrome/chrome_tests_unit...
File chrome/chrome_tests_unit.gypi (right):

https://codereview.chromium.org/493953002/diff/20001/chrome/chrome_tests_unit...
chrome/chrome_tests_unit.gypi:22:
'../components/components.gyp:component_updater_test_support',
Tommy, do you know if we could further refactor the code so that we will not
have this component_updater_test_support?

I imagine this is needed since it is used in two places by the unit tests. Would
it be possible to further consolidate the code?

https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
File components/component_updater/test/component_patcher_unittest.h (right):

https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
components/component_updater/test/component_patcher_unittest.h:11: #include
"content/public/test/test_browser_thread_bundle.h"
Is it ok to have this content dependency here and in other unit tests in
src/components/component_updater ?

https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
File components/component_updater/test/test_configurator.h (right):

https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
components/component_updater/test/test_configurator.h:1: // Copyright 2014 The
Chromium Authors. All rights reserved.
No changes in this file. Is it intentional?

https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
File components/component_updater/test/test_installer.h (right):

https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
components/component_updater/test/test_installer.h:1: // Copyright 2013 The
Chromium Authors. All rights reserved.
no change?

https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
File components/component_updater/test/update_response_unittest.cc (right):

https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
components/component_updater/test/update_response_unittest.cc:1: // Copyright
2013 The Chromium Authors. All rights reserved.
no change

[tommycli, 2014-08-22 17:31:45]

sorin: Thanks. I replied below.

https://codereview.chromium.org/493953002/diff/20001/chrome/browser/component...
File chrome/browser/component_updater/test/component_updater_service_unittest.cc
(right):

https://codereview.chromium.org/493953002/diff/20001/chrome/browser/component...
chrome/browser/component_updater/test/component_updater_service_unittest.cc:5:
#include
"chrome/browser/component_updater/test/component_updater_service_unittest.h"
On 2014/08/22 17:19:58, Sorin Jianu wrote:
> We noticed this unit test is now separated from its production code. We are ok
> with this, as we want to rewrite the component updater service, including its
> tests in a way that removes its dependencies on src/content.
> 
> Until then, is there a practical way to keep the code and the unit test
> together?

Yeah this is really lame. You could separate the CUS test stuff from the CU
Resource throttle part of the test. 

I started going down that path. It's a lot of work, and started to feel like I
was going down a deep rabbit hole. It would be a good thing to do in a separate
CL.

Maybe rename the test since it's more of an integration test?

https://codereview.chromium.org/493953002/diff/20001/chrome/chrome_tests_unit...
File chrome/chrome_tests_unit.gypi (right):

https://codereview.chromium.org/493953002/diff/20001/chrome/chrome_tests_unit...
chrome/chrome_tests_unit.gypi:22:
'../components/components.gyp:component_updater_test_support',
On 2014/08/22 17:19:58, Sorin Jianu wrote:
> Tommy, do you know if we could further refactor the code so that we will not
> have this component_updater_test_support?
> 
> I imagine this is needed since it is used in two places by the unit tests.
Would
> it be possible to further consolidate the code?

Two ways:

a) you could bake the test support stuff CU in general, but i don't recommend
this.

b) As you mentioned, separate out the CUS test from the CU Resource Throttle
test and move one. You probably need to do more work...

https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
File components/component_updater/test/component_patcher_unittest.h (right):

https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
components/component_updater/test/component_patcher_unittest.h:11: #include
"content/public/test/test_browser_thread_bundle.h"
On 2014/08/22 17:19:58, Sorin Jianu wrote:
> Is it ok to have this content dependency here and in other unit tests in
> src/components/component_updater ?

Yes, I think the tests can use content/. The tests don't run on iOS.

I think there's other components where the production code doesn't use content/
and the tests do.

That being said, it would be nice to remove content/ from the tests. Separate
CL?

https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
File components/component_updater/test/test_configurator.h (right):

https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
components/component_updater/test/test_configurator.h:1: // Copyright 2014 The
Chromium Authors. All rights reserved.
On 2014/08/22 17:19:58, Sorin Jianu wrote:
> No changes in this file. Is it intentional?

Yeah I didn't see any required changes to move this.

https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
File components/component_updater/test/test_installer.h (right):

https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
components/component_updater/test/test_installer.h:1: // Copyright 2013 The
Chromium Authors. All rights reserved.
On 2014/08/22 17:19:58, Sorin Jianu wrote:
> no change?

Same. it's just a straight move.

https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
File components/component_updater/test/update_response_unittest.cc (right):

https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
components/component_updater/test/update_response_unittest.cc:1: // Copyright
2013 The Chromium Authors. All rights reserved.
On 2014/08/22 17:19:59, Sorin Jianu wrote:
> no change

same

[Sorin Jianu, 2014-08-22 17:35:44]

lgtm

Thank you Tommy!

https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
File components/component_updater/test/component_patcher_unittest.h (right):

https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
components/component_updater/test/component_patcher_unittest.h:11: #include
"content/public/test/test_browser_thread_bundle.h"
On 2014/08/22 17:31:45, tommycli wrote:
> On 2014/08/22 17:19:58, Sorin Jianu wrote:
> > Is it ok to have this content dependency here and in other unit tests in
> > src/components/component_updater ?
> 
> Yes, I think the tests can use content/. The tests don't run on iOS.
> 
> I think there's other components where the production code doesn't use
content/
> and the tests do.
> 
> That being said, it would be nice to remove content/ from the tests. Separate
> CL?

Separate CL sg.

[tommycli, 2014-08-22 17:37:42]

sorin/waffles: Thanks!

+blundell: may have a comment on some of the issues we discussed here. He may
correct me on some of the answers I gave today when he gets back.

[tommycli, 2014-08-22 20:36:53]

The CQ bit was checked by tommycli@chromium.org

[commit-bot: I haz the power, 2014-08-22 20:37:29]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tommycli@chromium.org/493953002/20001

[commit-bot: I haz the power, 2014-08-22 20:47:59]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/45351)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/50536)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-22 20:49:37]

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

[commit-bot: I haz the power, 2014-08-22 20:49:37]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tommycli, 2014-08-22 21:19:32]

The CQ bit was checked by tommycli@chromium.org

[commit-bot: I haz the power, 2014-08-22 21:21:05]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tommycli@chromium.org/493953002/60001

[commit-bot: I haz the power, 2014-08-22 22:35:32]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[commit-bot: I haz the power, 2014-08-22 22:43:25]

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

[commit-bot: I haz the power, 2014-08-22 22:43:26]

Try jobs failed on following builders:
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[tommycli, 2014-08-22 23:00:35]

The CQ bit was checked by tommycli@chromium.org

[commit-bot: I haz the power, 2014-08-22 23:01:21]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tommycli@chromium.org/493953002/80001

[commit-bot: I haz the power, 2014-08-23 06:00:37]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[commit-bot: I haz the power, 2014-08-23 06:07:21]

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

[commit-bot: I haz the power, 2014-08-23 06:07:27]

Try jobs failed on following builders:
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[blundell, 2014-08-25 08:10:36]

On 2014/08/22 17:31:45, tommycli wrote:
> sorin: Thanks. I replied below.
> 
>
https://codereview.chromium.org/493953002/diff/20001/chrome/browser/component...
> File
chrome/browser/component_updater/test/component_updater_service_unittest.cc
> (right):
> 
>
https://codereview.chromium.org/493953002/diff/20001/chrome/browser/component...
> chrome/browser/component_updater/test/component_updater_service_unittest.cc:5:
> #include
> "chrome/browser/component_updater/test/component_updater_service_unittest.h"
> On 2014/08/22 17:19:58, Sorin Jianu wrote:
> > We noticed this unit test is now separated from its production code. We are
ok
> > with this, as we want to rewrite the component updater service, including
its
> > tests in a way that removes its dependencies on src/content.
> > 
> > Until then, is there a practical way to keep the code and the unit test
> > together?
> 
> Yeah this is really lame. You could separate the CUS test stuff from the CU
> Resource throttle part of the test. 
> 
> I started going down that path. It's a lot of work, and started to feel like I
> was going down a deep rabbit hole. It would be a good thing to do in a
separate
> CL.
> 
> Maybe rename the test since it's more of an integration test?
> 
>
https://codereview.chromium.org/493953002/diff/20001/chrome/chrome_tests_unit...
> File chrome/chrome_tests_unit.gypi (right):
> 
>
https://codereview.chromium.org/493953002/diff/20001/chrome/chrome_tests_unit...
> chrome/chrome_tests_unit.gypi:22:
> '../components/components.gyp:component_updater_test_support',
> On 2014/08/22 17:19:58, Sorin Jianu wrote:
> > Tommy, do you know if we could further refactor the code so that we will not
> > have this component_updater_test_support?
> > 
> > I imagine this is needed since it is used in two places by the unit tests.
> Would
> > it be possible to further consolidate the code?
> 
> Two ways:
> 
> a) you could bake the test support stuff CU in general, but i don't recommend
> this.
> 
> b) As you mentioned, separate out the CUS test from the CU Resource Throttle
> test and move one. You probably need to do more work...
> 
>
https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
> File components/component_updater/test/component_patcher_unittest.h (right):
> 
>
https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
> components/component_updater/test/component_patcher_unittest.h:11: #include
> "content/public/test/test_browser_thread_bundle.h"
> On 2014/08/22 17:19:58, Sorin Jianu wrote:
> > Is it ok to have this content dependency here and in other unit tests in
> > src/components/component_updater ?
> 
> Yes, I think the tests can use content/. The tests don't run on iOS.

The tests should run on iOS and shouldn't use content/. In general, unittests of
production code that is built on iOS should run on iOS. Thanks!

> 
> I think there's other components where the production code doesn't use
content/
> and the tests do.

Do you have any examples here?

> 
> That being said, it would be nice to remove content/ from the tests. Separate
> CL?
> 
>
https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
> File components/component_updater/test/test_configurator.h (right):
> 
>
https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
> components/component_updater/test/test_configurator.h:1: // Copyright 2014 The
> Chromium Authors. All rights reserved.
> On 2014/08/22 17:19:58, Sorin Jianu wrote:
> > No changes in this file. Is it intentional?
> 
> Yeah I didn't see any required changes to move this.
> 
>
https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
> File components/component_updater/test/test_installer.h (right):
> 
>
https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
> components/component_updater/test/test_installer.h:1: // Copyright 2013 The
> Chromium Authors. All rights reserved.
> On 2014/08/22 17:19:58, Sorin Jianu wrote:
> > no change?
> 
> Same. it's just a straight move.
> 
>
https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
> File components/component_updater/test/update_response_unittest.cc (right):
> 
>
https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
> components/component_updater/test/update_response_unittest.cc:1: // Copyright
> 2013 The Chromium Authors. All rights reserved.
> On 2014/08/22 17:19:59, Sorin Jianu wrote:
> > no change
> 
> same

[blundell, 2014-08-25 08:11:27]

It might have been hard to pick out my response there, so here it is again
snipped out:

https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
> components/component_updater/test/component_patcher_unittest.h:11: #include
> "content/public/test/test_browser_thread_bundle.h"
> On 2014/08/22 17:19:58, Sorin Jianu wrote:
> > Is it ok to have this content dependency here and in other unit tests in
> > src/components/component_updater ?
> 
> Yes, I think the tests can use content/. The tests don't run on iOS.

The tests should run on iOS and shouldn't use content/. In general, unittests of
production code that is built on iOS should run on iOS. Thanks!

> 
> I think there's other components where the production code doesn't use
content/
> and the tests do.

Do you have any examples here?

[tommycli, 2014-08-25 16:08:53]

On 2014/08/25 08:11:27, blundell wrote:
> It might have been hard to pick out my response there, so here it is again
> snipped out:
> 
>
https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
> > components/component_updater/test/component_patcher_unittest.h:11: #include
> > "content/public/test/test_browser_thread_bundle.h"
> > On 2014/08/22 17:19:58, Sorin Jianu wrote:
> > > Is it ok to have this content dependency here and in other unit tests in
> > > src/components/component_updater ?
> > 
> > Yes, I think the tests can use content/. The tests don't run on iOS.
> 
> The tests should run on iOS and shouldn't use content/. In general, unittests
of
> production code that is built on iOS should run on iOS. Thanks!
> 
> > 
> > I think there's other components where the production code doesn't use
> content/
> > and the tests do.
> 
> Do you have any examples here?

Hi blundell - Okay sounds good. I thought I saw some examples of content/ using
tests, but I must have been mistaken.

Are you okay with this patch moving the tests, then we remove content/ in a
separate CL?

[blundell, 2014-08-26 07:41:46]

On 2014/08/25 16:08:53, tommycli wrote:
> On 2014/08/25 08:11:27, blundell wrote:
> > It might have been hard to pick out my response there, so here it is again
> > snipped out:
> > 
> >
>
https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
> > > components/component_updater/test/component_patcher_unittest.h:11:
#include
> > > "content/public/test/test_browser_thread_bundle.h"
> > > On 2014/08/22 17:19:58, Sorin Jianu wrote:
> > > > Is it ok to have this content dependency here and in other unit tests in
> > > > src/components/component_updater ?
> > > 
> > > Yes, I think the tests can use content/. The tests don't run on iOS.
> > 
> > The tests should run on iOS and shouldn't use content/. In general,
unittests
> of
> > production code that is built on iOS should run on iOS. Thanks!
> > 
> > > 
> > > I think there's other components where the production code doesn't use
> > content/
> > > and the tests do.
> > 
> > Do you have any examples here?
> 
> Hi blundell - Okay sounds good. I thought I saw some examples of content/
using
> tests, but I must have been mistaken.
> 
> Are you okay with this patch moving the tests, then we remove content/ in a
> separate CL?

Yep, that's OK.

[blundell, 2014-08-26 07:42:16]

On 2014/08/26 07:41:46, blundell wrote:
> On 2014/08/25 16:08:53, tommycli wrote:
> > On 2014/08/25 08:11:27, blundell wrote:
> > > It might have been hard to pick out my response there, so here it is again
> > > snipped out:
> > > 
> > >
> >
>
https://codereview.chromium.org/493953002/diff/20001/components/component_upd...
> > > > components/component_updater/test/component_patcher_unittest.h:11:
> #include
> > > > "content/public/test/test_browser_thread_bundle.h"
> > > > On 2014/08/22 17:19:58, Sorin Jianu wrote:
> > > > > Is it ok to have this content dependency here and in other unit tests
in
> > > > > src/components/component_updater ?
> > > > 
> > > > Yes, I think the tests can use content/. The tests don't run on iOS.
> > > 
> > > The tests should run on iOS and shouldn't use content/. In general,
> unittests
> > of
> > > production code that is built on iOS should run on iOS. Thanks!
> > > 
> > > > 
> > > > I think there's other components where the production code doesn't use
> > > content/
> > > > and the tests do.
> > > 
> > > Do you have any examples here?
> > 
> > Hi blundell - Okay sounds good. I thought I saw some examples of content/
> using
> > tests, but I must have been mistaken.
> > 
> > Are you okay with this patch moving the tests, then we remove content/ in a
> > separate CL?
> 
> Yep, that's OK.

(as long as you follow up promptly, which I trust that you will given how
efficiently you've been doing this componentization)

[tommycli, 2014-08-26 22:32:07]

The CQ bit was checked by tommycli@chromium.org

[commit-bot: I haz the power, 2014-08-26 22:32:29]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tommycli@chromium.org/493953002/130001

[commit-bot: I haz the power, 2014-08-26 23:31:56]

Committed patchset #7 (130001) as 23917afa854acb98f822bf89c0d8b29c300a1898

[commit-bot: I haz the power, 2014-09-10 02:47:13]

Patchset 7 (id:??) landed as
https://crrev.com/0409b4df1414ee0fb3e7c1179a86324944f242e3
Cr-Commit-Position: refs/heads/master@{#292026}