Move the tests in ui/chromeos to a new target: ui_chromeos_unittests

Move the tests in ui/chromeos to a new target: ui_chromeos_unittests

BUG=432538
TEST=None
R=skuhne,phajdan.jr,oshima,thestig
TBR=sky (For reverting changes to ui/base added in https://codereview.chromium.org/680383008/)

Committed: https://crrev.com/48074e104908abf4a329d949660f209a8a09b1eb
Cr-Commit-Position: refs/heads/master@{#304825}

[pkotwicz, 2014-11-17 20:25:22]

Patchset #1 (id:1) has been deleted

[pkotwicz, 2014-11-17 20:34:29]

Patchset #1 (id:20001) has been deleted

[pkotwicz, 2014-11-17 20:37:35]

Patchset #1 (id:40001) has been deleted

[pkotwicz, 2014-11-17 20:38:44]

Patchset #1 (id:60001) has been deleted

[pkotwicz, 2014-11-17 20:56:57]

Patchset #1 (id:80001) has been deleted

[pkotwicz, 2014-11-17 20:57:34]

pkotwicz@chromium.org changed reviewers:
+ skuhne@chromium.org

[pkotwicz, 2014-11-17 20:57:34]

skuhne@ can you please take an initial look? I mostly want to make sure that I
am not missing anything

[Mr4D (OOO till 08-26), 2014-11-17 21:22:36]

I think this looks good - you should run tests on ios/android/mac/win in
addition (just to make sure).

https://codereview.chromium.org/738513002/diff/100001/build/all.gyp
File build/all.gyp (right):

https://codereview.chromium.org/738513002/diff/100001/build/all.gyp#newcode1315
build/all.gyp:1315: '../ui/chromeos/ui_chromeos.gyp:ui_chromeos_unittests',
Sorting - can you please move this one line down?

https://codereview.chromium.org/738513002/diff/100001/testing/buildbot/chromi...
File testing/buildbot/chromium.chromiumos.json (right):

https://codereview.chromium.org/738513002/diff/100001/testing/buildbot/chromi...
testing/buildbot/chromium.chromiumos.json:64: "ui_chromeos_unittests",
Could this be swarmed? If so - please do!

[pkotwicz, 2014-11-18 03:03:12]

Stefan, can you please take another look?

https://codereview.chromium.org/738513002/diff/100001/testing/buildbot/chromi...
File testing/buildbot/chromium.chromiumos.json (right):

https://codereview.chromium.org/738513002/diff/100001/testing/buildbot/chromi...
testing/buildbot/chromium.chromiumos.json:64: "ui_chromeos_unittests",
ui_base_unittests does not seem to be swarmed. If ui_base_unittests is not
swarmed, it is probably best not to swarm ui_chromeos_unittests in this CL

[Mr4D (OOO till 08-26), 2014-11-18 14:53:07]

lgtm

https://codereview.chromium.org/738513002/diff/100001/testing/buildbot/chromi...
File testing/buildbot/chromium.chromiumos.json (right):

https://codereview.chromium.org/738513002/diff/100001/testing/buildbot/chromi...
testing/buildbot/chromium.chromiumos.json:64: "ui_chromeos_unittests",
On 2014/11/18 03:03:12, pkotwicz wrote:
> ui_base_unittests does not seem to be swarmed. If ui_base_unittests is not
> swarmed, it is probably best not to swarm ui_chromeos_unittests in this CL

Acknowledged.

[pkotwicz, 2014-11-18 15:42:31]

pkotwicz@chromium.org changed reviewers:
+ oshima@chromium.org, phajdan.jr@chromium.org, thestig@chromium.org

[pkotwicz, 2014-11-18 15:42:32]

phajdan.jr@ for build/ and testing/
thestig@ for tools/valgrind
oshima@ for ui/chromeos/

[oshima, 2014-11-18 16:20:38]

ui/chromeos lgtm

https://codereview.chromium.org/738513002/diff/120001/ui/base/BUILD.gn
File ui/base/BUILD.gn (left):

https://codereview.chromium.org/738513002/diff/120001/ui/base/BUILD.gn#oldcod...
ui/base/BUILD.gn:920: "//ui/views:test_support",
nice!

[Lei Zhang, 2014-11-18 18:38:11]

lgtm

https://codereview.chromium.org/738513002/diff/120001/tools/valgrind/chrome_t...
File tools/valgrind/chrome_tests.py (right):

https://codereview.chromium.org/738513002/diff/120001/tools/valgrind/chrome_t...
tools/valgrind/chrome_tests.py:476: def TestUIChromeOS(self):
nit: alphabetical order please

[pkotwicz, 2014-11-18 18:45:24]

https://codereview.chromium.org/738513002/diff/120001/tools/valgrind/chrome_t...
File tools/valgrind/chrome_tests.py (right):

https://codereview.chromium.org/738513002/diff/120001/tools/valgrind/chrome_t...
tools/valgrind/chrome_tests.py:476: def TestUIChromeOS(self):
On 2014/11/18 18:38:11, Lei Zhang wrote:
> nit: alphabetical order please

Done.

[Paweł Hajdan Jr., 2014-11-19 11:21:24]

LGTM

[pkotwicz, 2014-11-19 15:52:13]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-11-19 15:52:53]

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

[commit-bot: I haz the power, 2014-11-19 16:49:51]

Committed patchset #4 (id:160001)

[commit-bot: I haz the power, 2014-11-19 16:50:43]

Patchset 4 (id:??) landed as
https://crrev.com/48074e104908abf4a329d949660f209a8a09b1eb
Cr-Commit-Position: refs/heads/master@{#304825}

[tfarina, 2014-11-19 17:32:18]

some postmortem comments.

https://codereview.chromium.org/738513002/diff/160001/testing/buildbot/chromi...
File testing/buildbot/chromium_trybot.json (right):

https://codereview.chromium.org/738513002/diff/160001/testing/buildbot/chromi...
testing/buildbot/chromium_trybot.json:169: "test": "ui_chromeos_unittests",
this isn't running on main waterfall. Is it?

https://codereview.chromium.org/738513002/diff/160001/ui/base/ui_base_tests.gypi
File ui/base/ui_base_tests.gypi (left):

https://codereview.chromium.org/738513002/diff/160001/ui/base/ui_base_tests.g...
ui/base/ui_base_tests.gypi:198: '../aura/aura.gyp:aura_test_support',
thanks

https://codereview.chromium.org/738513002/diff/160001/ui/chromeos/run_all_uni...
File ui/chromeos/run_all_unittests.cc (right):

https://codereview.chromium.org/738513002/diff/160001/ui/chromeos/run_all_uni...
ui/chromeos/run_all_unittests.cc:21: ~UIChromeOSTestSuite() override {}
why did you need to override the dtor?

https://codereview.chromium.org/738513002/diff/160001/ui/chromeos/run_all_uni...
ui/chromeos/run_all_unittests.cc:30:
ASSERT_TRUE(PathService::Get(ui::UI_TEST_PAK, &ui_test_pak_path));
Are you sure ui_chromeos_unittests needs to setup the pak stuff?

[pkotwicz, 2014-11-20 02:17:42]

https://codereview.chromium.org/738513002/diff/160001/testing/buildbot/chromi...
File testing/buildbot/chromium_trybot.json (right):

https://codereview.chromium.org/738513002/diff/160001/testing/buildbot/chromi...
testing/buildbot/chromium_trybot.json:169: "test": "ui_chromeos_unittests",
Yes, ui_chromeos_unittests is running on the main waterfall. See Linux
ChromiumOS Tests (1)
http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%2...

My understanding is that it should run on the main waterfall.
ui_chromeos_unittests contains some of the tests which were in
ui_base_unittests. ui_base_unittests runs on the main waterfall.

I am not sure where I can check the impact of the change in this particular
file.

https://codereview.chromium.org/738513002/diff/160001/ui/chromeos/run_all_uni...
File ui/chromeos/run_all_unittests.cc (right):

https://codereview.chromium.org/738513002/diff/160001/ui/chromeos/run_all_uni...
ui/chromeos/run_all_unittests.cc:30:
ASSERT_TRUE(PathService::Get(ui::UI_TEST_PAK, &ui_test_pak_path));
You are right, I could have used
ui::ResourceBundle::InitSharedInstanceWithLocale() instead.

[tfarina, 2014-12-19 16:58:30]

https://codereview.chromium.org/738513002/diff/160001/ui/chromeos/BUILD.gn
File ui/chromeos/BUILD.gn (right):

https://codereview.chromium.org/738513002/diff/160001/ui/chromeos/BUILD.gn#ne...
ui/chromeos/BUILD.gn:58: "../chromeos/ime/candidate_view_unittest.cc",
just noticed this now.

I think could have been simply:

"ime/candidate_view_unittest.cc",
"ime/candidate_window_view_unittest.cc",
"ime/input_method_menu_item_unittest.cc",

like the source list above.

I will try later tonight and a send a patch for it.

[tfarina, 2015-01-04 19:30:32]

https://codereview.chromium.org/738513002/diff/160001/ui/chromeos/BUILD.gn
File ui/chromeos/BUILD.gn (right):

https://codereview.chromium.org/738513002/diff/160001/ui/chromeos/BUILD.gn#ne...
ui/chromeos/BUILD.gn:58: "../chromeos/ime/candidate_view_unittest.cc",
On 2014/12/19 16:58:30, tfarina wrote:
> just noticed this now.
> 
> I think could have been simply:
> 
> "ime/candidate_view_unittest.cc",
> "ime/candidate_window_view_unittest.cc",
> "ime/input_method_menu_item_unittest.cc",
> 
> like the source list above.
> 
> I will try later tonight and a send a patch for it.

Done. https://codereview.chromium.org/833043002/diff/1/ui/chromeos/BUILD.gn