TranslatePrefTest: Added test case for UpdateLanguageList.

components/translate/core/browser/translate_prefs_unittest.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/translate/core/browser/translate_prefs.h"
 
 #include <algorithm>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/json/json_reader.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "components/translate/core/browser/translate_download_manager.h"
 #include "components/translate/core/browser/translate_prefs.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
 const char kTestLanguage[] = "en";
 
+#if defined(OS_CHROMEOS)

[groby-ooo-7-16, 2017/03/08 00:49:15]

If you're refactoring this anyways - could you factor this out into a function?
There are _numerous_ tests that do exactly this, and it's... annoying. (It's
also annoying that tests in components/ depend on things defined in chrome/, but
that's a larger story :)

[Matt Giuca, 2017/03/15 02:45:18]

Hmm, I had a bit of a think about it but it's not quite clear where this would
go (it doesn't really belong in translate_prefs.h, because it's only used in
tests). It isn't clear to me why this pref name is parameterised in the code
anyway (and only concrete in tests); why isn't it just hard-coded in the code
itself?

I'd rather not have to solve this issue: this CL is a side-yak on a
codebase-wide refactor (https://codereview.chromium.org/2695883003) and I can't
stop to clean up nearby problems in all of those places. I already went one step
further here and added a test. (This is kind of time-critical because the longer
it takes to land that CL, the more merge conflicts I have to deal with.)

+const char kPreferredLanguagesPref[] = "settings.language.preferred_languages";
+#else
+const char* kPreferredLanguagesPref = nullptr;
+#endif
+const char kAcceptLanguagesPref[] = "intl.accept_languages";
+
 }  // namespace
 
 namespace translate {
 
 class TranslatePrefTest : public testing::Test {
  protected:
   TranslatePrefTest()
       : prefs_(new sync_preferences::TestingPrefServiceSyncable()) {
-#if defined(OS_CHROMEOS)
-    const char* preferred_languages_prefs =
-        "settings.language.preferred_languages";
-#else
-    const char* preferred_languages_prefs = NULL;
-#endif
     translate_prefs_.reset(new translate::TranslatePrefs(
-        prefs_.get(), "intl.accept_languages", preferred_languages_prefs));
+        prefs_.get(), kAcceptLanguagesPref, kPreferredLanguagesPref));
     TranslatePrefs::RegisterProfilePrefs(prefs_->registry());
     now_ = base::Time::Now();
     two_days_ago_ = now_ - base::TimeDelta::FromDays(2);
   }
 
+  void SetUp() override {
+    prefs_->registry()->RegisterStringPref(kAcceptLanguagesPref, std::string());
+#if defined(OS_CHROMEOS)
+    prefs_->registry()->RegisterStringPref(kPreferredLanguagesPref,

[groby-ooo-7-16, 2017/03/08 00:49:15]

I'd really like to break the defined(OS_CHROMEOS) dependency - would you mind
changing this to 

if(kPreferredLanguagesPref)
  ...RegisterStringPref(kPreferred...

[Matt Giuca, 2017/03/15 02:45:18]

Done.

[Matt Giuca, 2017/03/15 07:45:14]

Undone... unfortunately I get this compile error on ChromeOS:

error: address of array 'kPreferredLanguagesPref' will always evaluate to 'true'
[-Werror,-Wpointer-bool-conversion]
    if (kPreferredLanguagesPref) {
    ~~  ^~~~~~~~~~~~~~~~~~~~~~~

So this has to remain a compile-time check.

+                                           std::string());
+#endif
+  }
+
   void SetLastDeniedTime(const std::string& language, base::Time time) {
     DenialTimeUpdate update(prefs_.get(), language, 2);
     update.AddDenialTime(time);
   }
 
   base::Time GetLastDeniedTime(const std::string& language) {
     DenialTimeUpdate update(prefs_.get(), language, 2);
     return update.GetOldestDenialTime();
   }
 
@@ skipping 148 matching lines @@
 
   update.AddDenialTime(now_);
   EXPECT_EQ(update.GetOldestDenialTime(),
             now_ - base::TimeDelta::FromMinutes(3));
 
   update.AddDenialTime(now_);
   EXPECT_EQ(update.GetOldestDenialTime(),
             now_ - base::TimeDelta::FromMinutes(2));
 }
 
+TEST_F(TranslatePrefTest, UpdateLanguageList) {
+  // Test with basic set of languages (no country codes).
+  std::vector<std::string> languages{"en", "ja"};
+  translate_prefs_->UpdateLanguageList(languages);
+  EXPECT_EQ("en,ja", prefs_->GetString(kAcceptLanguagesPref));
+
+  // Test with languages that have country codes. Expect accepted languages both
+  // with and without a country code. (See documentation for
+  // ExpandLanguageCodes.)
+  languages = {"en-US", "ja", "en-CA"};
+  translate_prefs_->UpdateLanguageList(languages);
+#if defined(OS_CHROMEOS)
+  EXPECT_EQ("en-US,ja,en-CA", prefs_->GetString(kPreferredLanguagesPref));
+#endif
+  EXPECT_EQ("en-US,en,ja,en-CA", prefs_->GetString(kAcceptLanguagesPref));
+}
+
 TEST_F(TranslatePrefTest, ULPPrefs) {
   // Mock the pref.
   // Case 1: well formed ULP.
   const char json1[] =
       "{\n"
       "  \"reading\": {\n"
       "    \"confidence\": 0.8,\n"
       "    \"preference\": [\n"
       "      {\n"
       "        \"language\": \"en-AU\",\n"
@@ skipping 106 matching lines @@
   EXPECT_EQ(0.8, translate_prefs_->GetReadingFromUserLanguageProfile(&list));
   EXPECT_EQ(2UL, list.size());
   EXPECT_EQ("en", list[0].first);  // en-US and en-GB will be normalize into en
   EXPECT_EQ(0.6,
             list[0].second);  // and their probability will add to gether be 0.6
   EXPECT_EQ("fr", list[1].first);  // fr will move down to the 2nd one
   EXPECT_EQ(0.4, list[1].second);
 }
 
 }  // namespace translate