Translate: split language code typo correction to apply unit tests.

Translate: split language code typo correction to apply unit tests.

Split language code type correction logic to an independent static
function. Then, apply unit tests to confirm that the logic correct
well-known mal-formed language code.

BUG=175673
TEST=unit_tests --gtest_filter'TranslateHelperTest.LanguageCodeTypoCorrection'

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

[Takashi Toyoshima, 2013-02-08 11:48:01]

Hi mad,

This is an extra CL to apply unit tests against well-known language code typo
correction.

[MAD, 2013-02-08 13:46:41]

LGTM with a small optional nit...

Thanks!

BYE
MAD

https://codereview.chromium.org/12221085/diff/1/chrome/renderer/translate_hel...
File chrome/renderer/translate_helper_browsertest.cc (right):

https://codereview.chromium.org/12221085/diff/1/chrome/renderer/translate_hel...
chrome/renderer/translate_helper_browsertest.cc:32: void
CorrectLanguageCodeTypo(std::string* code) {
The way typically do this is by making the test a friend of the class (using a
dedicated macro) so it can access protected/private stuff to be tested...

[Takashi Toyoshima, 2013-02-11 06:59:33]

https://codereview.chromium.org/12221085/diff/1/chrome/renderer/translate_hel...
File chrome/renderer/translate_helper_browsertest.cc (right):

https://codereview.chromium.org/12221085/diff/1/chrome/renderer/translate_hel...
chrome/renderer/translate_helper_browsertest.cc:32: void
CorrectLanguageCodeTypo(std::string* code) {
I see.
I think FRIEND_TEST_ALL_PREFIXES is the macro you mean here.
I'll make another CL which focus on replacing them with the macro.

[commit-bot: I haz the power, 2013-02-11 06:59:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/toyoshim@chromium.org/12221085/1

[commit-bot: I haz the power, 2013-02-11 06:59:56]

Presubmit check for 12221085-1 failed and returned exit status 1.

INFO:root:Found 3 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[Takashi Toyoshima, 2013-02-11 07:01:05]

+sky@ for chrome/ OWNER check.

[sky, 2013-02-11 21:10:14]

https://codereview.chromium.org/12221085/diff/1/chrome/renderer/translate_hel...
File chrome/renderer/translate_helper.h (right):

https://codereview.chromium.org/12221085/diff/1/chrome/renderer/translate_hel...
chrome/renderer/translate_helper.h:31: // Correct language coed if it contains
well-known mistakes.
coed->code

https://codereview.chromium.org/12221085/diff/1/chrome/renderer/translate_hel...
File chrome/renderer/translate_helper_browsertest.cc (right):

https://codereview.chromium.org/12221085/diff/1/chrome/renderer/translate_hel...
chrome/renderer/translate_helper_browsertest.cc:315: TEST_F(TranslateHelperTest,
LanguageCodeTypoCorrection) {
Why do you need a browser test to verify CorrectLanguageCodeTypo? Can't this be
a unit test?

[Takashi Toyoshima, 2013-02-12 06:46:02]

Thanks.
I update this change in order to implement new test as unit test.
This change depends on following CL which introduce unit tests for
TranslateHelper.
https://codereview.chromium.org/12209114/

https://codereview.chromium.org/12221085/diff/1/chrome/renderer/translate_hel...
File chrome/renderer/translate_helper.h (right):

https://codereview.chromium.org/12221085/diff/1/chrome/renderer/translate_hel...
chrome/renderer/translate_helper.h:31: // Correct language coed if it contains
well-known mistakes.
On 2013/02/11 21:10:14, sky wrote:
> coed->code

Done.

https://codereview.chromium.org/12221085/diff/1/chrome/renderer/translate_hel...
File chrome/renderer/translate_helper_browsertest.cc (right):

https://codereview.chromium.org/12221085/diff/1/chrome/renderer/translate_hel...
chrome/renderer/translate_helper_browsertest.cc:315: TEST_F(TranslateHelperTest,
LanguageCodeTypoCorrection) {
On 2013/02/11 21:10:14, sky wrote:
> Why do you need a browser test to verify CorrectLanguageCodeTypo? Can't this
be
> a unit test?

Done.

[sky, 2013-02-12 17:30:06]

LGTM - but see comments in other patch as you should consolidate test files.

https://codereview.chromium.org/12221085/diff/3003/chrome/renderer/translate_...
File chrome/renderer/translate_helper_unittest.cc (right):

https://codereview.chromium.org/12221085/diff/3003/chrome/renderer/translate_...
chrome/renderer/translate_helper_unittest.cc:18: EXPECT_EQ(0,
language.compare("ja"));
EXPECT_EQ("ja", language) same pattern for other tests.

[Takashi Toyoshima, 2013-02-14 08:00:18]

I'll submit this because dependent change is submitted now.
Thanks.

https://codereview.chromium.org/12221085/diff/3003/chrome/renderer/translate_...
File chrome/renderer/translate_helper_unittest.cc (right):

https://codereview.chromium.org/12221085/diff/3003/chrome/renderer/translate_...
chrome/renderer/translate_helper_unittest.cc:18: EXPECT_EQ(0,
language.compare("ja"));
On 2013/02/12 17:30:07, sky wrote:
> EXPECT_EQ("ja", language) same pattern for other tests.

Done.

[commit-bot: I haz the power, 2013-02-14 08:07:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/toyoshim@chromium.org/12221085/15001