Fix browser test to work with dynamic CLD

Fix browser test to work with dynamic CLD

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

[Andrew Hayden (chromium.org), 2014-05-19 12:55:36]

Dear owner: 
NB: This change depends upon https://codereview.chromium.org/285293004/.

We are making it possible for the Compact Language Detector v2 (CLD2) data to be
split out of the binary and be loaded dynamically at runtime. This change to
your browser test will allow it to run when this dynamic behavior is turned on.
A scoped object is created that copies CLD2 data from the test directory and
sets internal pointers to it; the destructor clears the pointers and cleans up
the files. For more information, please see crbug/367239.

[msw, 2014-05-19 18:56:42]

https://codereview.chromium.org/292723003/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/translate/translate_bubble_view_browsertest.cc
(right):

https://codereview.chromium.org/292723003/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/translate/translate_bubble_view_browsertest.cc:31:
test::ScopedCLDDynamicDataHarness dynamic_data_scope;
As I asked in https://codereview.chromium.org/285293004, can
TranslateBubbleViewBrowserTest own the ScopedCLDDynamicDataHarness object and
call init in a SetUp() override, so each of these tests don't have to run all
this file copying and cleanup separately (and apparently redundantly)?

[Andrew Hayden (chromium.org), 2014-05-21 09:09:47]

This is done. PTAL, the bots seem like they are happy with this version.

[Andrew Hayden (chromium.org), 2014-05-21 09:14:42]

Also FWIW I was curious as to whether ASSERT would work in SetUp. Indeed, it
seems to be explicitly supported - although I haven't found any documentation to
this effect in the FAQ:

https://code.google.com/p/googletest/source/browse/trunk/src/gtest.cc#2146

Bottom line: This construct is valid gtest.

[Andrew Hayden (chromium.org), 2014-05-21 14:23:08]

I had to make one more adjustment to get the setup work done correctly; we have
to wait until the USER_DATA_DIRECTORY has been created in order to do anything.

The translate_browser_test_utils themselves have a bug here, because they assert
that the directory exists - which it won't, the first time the test runs in a
clean environment :)

I'll fix that in a separate checkin, since it will block us from turning the
flags on for real. This change, however, does work and has been verified on my
local machine in all 3 combinations of flags.

[msw, 2014-05-21 17:08:31]

lgtm with nits.

https://codereview.chromium.org/292723003/diff/80001/chrome/browser/ui/views/...
File chrome/browser/ui/views/translate/translate_bubble_view_browsertest.cc
(right):

https://codereview.chromium.org/292723003/diff/80001/chrome/browser/ui/views/...
chrome/browser/ui/views/translate/translate_bubble_view_browsertest.cc:24: bool
SetUpUserDataDirectory() OVERRIDE {
nit: virtual

https://codereview.chromium.org/292723003/diff/80001/chrome/browser/ui/views/...
chrome/browser/ui/views/translate/translate_bubble_view_browsertest.cc:28:
return !(::testing::Test::HasFatalFailure());
nit: I think you can just return !HasFatalFailure();

[Andrew Hayden (chromium.org), 2014-05-21 17:46:55]

Nits addressed, trybots cranking. Thanks.

[Andrew Hayden (chromium.org), 2014-05-21 19:44:54]

I have spawned https://codereview.chromium.org/296943002 to include some cleanup
that I did in the test util class while researching the problems encountered
getting this shoehorned into test setup. You may be interested in reviewing it,
as I've adapted the test setup from THIS review into a comment in THAT code
review.

Thanks again. Since this is all no-ops till we get the flags turned on, I'm
going to go ahead and submit this one first.

[Andrew Hayden (chromium.org), 2014-05-21 19:45:11]

The CQ bit was checked by andrewhayden@chromium.org

[commit-bot: I haz the power, 2014-05-21 19:58:26]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/andrewhayden@chromium.org/292723003/12...

[commit-bot: I haz the power, 2014-05-22 03:20:35]

Change committed as 272068