base: Add a couple Pref unit tests to base_unittests target.

base: Add a couple Pref unit tests to base_unittests target.

The added unit tests does not depend on anything but base, so we can run the on base_unittests right now.

BUG=155525
TEST=base_unittests --gtest_filter=*Pref*
R=joi@chromium.org
TBR=darin@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=177139

[tfarina, 2013-01-14 21:26:12]

Is this fine?

[Jói, 2013-01-14 21:28:29]

Hi Thiago,

The intent is to add the Prefs unit tests to the base_unittests target
once their dependencies have been broken, rather than creating a
separate base_prefs_unittests target.  This is in line with how
base_i18n unit tests are handled, and is also due to the fact that
there is some overhead to each new unit tests target, so we don't want
to add too many.

Cheers,
Jói


On Mon, Jan 14, 2013 at 1:26 PM,  <tfarina@chromium.org> wrote:
> Reviewers: Jói,
>
> Message:
> Is this fine?
>
> Description:
> base: Add base_prefs_unittests target.
>
> Add a minimal unittests target that runs a couple of Prefs unit tests that
> does
> not depend on anything but base.
>
> BUG=155525
> TEST=base_prefs_unittests
> R=joi@chromium.org
>
>
> Please review this at https://codereview.chromium.org/11879023/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files:
>   M base/base.gyp
>
>
> Index: base/base.gyp
> diff --git a/base/base.gyp b/base/base.gyp
> index
>
08cf7f4dcf2aa251fb14a16e0e128aef642cdcde..cca05e1cd5eeb6c690a95371279aebed92911897
> 100644
> --- a/base/base.gyp
> +++ b/base/base.gyp
> @@ -354,6 +354,38 @@
>        ],
>      },
>      {
> +      'target_name': 'base_prefs_test_support',
> +      'type': 'static_library',
> +      'dependencies': [
> +        'base',
> +        'base_prefs',
> +        '../testing/gmock.gyp:gmock',
> +      ],
> +      'sources': [
> +        'prefs/pref_store_observer_mock.cc',
> +        'prefs/pref_store_observer_mock.h',
> +        'prefs/testing_pref_store.cc',
> +        'prefs/testing_pref_store.h',
> +      ],
> +    },
> +    {
> +      'target_name': 'base_prefs_unittests',
> +      'type': '<(gtest_target_type)',
> +      'dependencies': [
> +        'base',
> +        'base_prefs',
> +        'base_prefs_test_support',
> +        'run_all_unittests',
> +        '../testing/gmock.gyp:gmock',
> +        '../testing/gtest.gyp:gtest',
> +      ],
> +      'sources': [
> +        'prefs/json_pref_store_unittest.cc',
> +        'prefs/overlay_user_pref_store_unittest.cc',
> +        'prefs/pref_value_map_unittest.cc',
> +      ],
> +    },
> +    {
>        # This is the subset of files from base that should not be used with
> a
>        # dynamic library. Note that this library cannot depend on base
> because
>        # base depends on base_static.
>
>

[tfarina, 2013-01-14 21:37:58]

Does patch set 3 works for you?

[Jói, 2013-01-14 21:45:46]

On 2013/01/14 21:37:58, tfarina wrote:
> Does patch set 3 works for you?

Yes, except these are also being built in chrome/chrome_tests_unit.gypi and so
should be removed from there in the same change.

Cheers,
Jói

[tfarina, 2013-01-14 22:14:18]

Done. PTAL.

TBRing Darin for base.gyp changes.

[Jói, 2013-01-14 22:22:06]

LGTM, thanks Thiago.

[darin (slow to review), 2013-01-14 22:38:39]

LGTM

[commit-bot: I haz the power, 2013-01-15 23:10:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/11879023/9001

[commit-bot: I haz the power, 2013-01-15 23:51:25]

Retried try job too often on ios_dbg_simulator for step(s) base_unittests

[commit-bot: I haz the power, 2013-01-16 00:22:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/11879023/9001

[tfarina, 2013-01-16 00:35:25]

ios_dbg_simulator (base_unittests) is playing dirty games on me!

I don't know why the hell he is refusing to pass.

[tfarina, 2013-01-16 00:50:03]

Looks like ios_dbg_simulator isn't running/doesn't run unit_tests yet, so I'm
going to move json_pref_store_unittest.cc back to unit_tests.

[commit-bot: I haz the power, 2013-01-16 00:58:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/11879023/24005

[commit-bot: I haz the power, 2013-01-16 03:16:11]

Retried try job too often on win for step(s) compile

[commit-bot: I haz the power, 2013-01-16 11:28:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/11879023/24005

[commit-bot: I haz the power, 2013-01-16 13:11:37]

Change committed as 177139

[tfarina, 2013-01-19 20:50:59]

https://codereview.chromium.org/11879023/diff/24005/chrome/chrome_tests_unit....
File chrome/chrome_tests_unit.gypi (right):

https://codereview.chromium.org/11879023/diff/24005/chrome/chrome_tests_unit....
chrome/chrome_tests_unit.gypi:1590: '../base/prefs/json_pref_store_unittest.cc',
We can move it back to base_unittests and exclude it when OS equal ios.

Would be that fine, Joi?

[Jói, 2013-01-21 10:33:22]

https://codereview.chromium.org/11879023/diff/24005/chrome/chrome_tests_unit....
File chrome/chrome_tests_unit.gypi (right):

https://codereview.chromium.org/11879023/diff/24005/chrome/chrome_tests_unit....
chrome/chrome_tests_unit.gypi:1590: '../base/prefs/json_pref_store_unittest.cc',
On 2013/01/19 20:50:59, tfarina wrote:
> We can move it back to base_unittests and exclude it when OS equal ios.
> 
> Would be that fine, Joi?

If that passes all bots then I have no issue with it.