timezone match code - consider canonical id as well

chromeos/settings/timezone_settings_unittest.cc

+// Copyright 2015 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 "base/memory/scoped_ptr.h"
+#include "base/stl_util.h"
+#include "chromeos/settings/timezone_settings_helper.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/icu/source/common/unicode/unistr.h"
+#include "third_party/icu/source/i18n/unicode/timezone.h"
+
+namespace {

[oshima, 2015/04/23 18:30:45]

move this inside chromeos::system

[jungshik at Google, 2015/04/23 19:48:12]

Done.

+
+const static char* kTimeZones[] = {

[oshima, 2015/04/23 18:30:45]

static isn't necessary in anonymous namespace

[jungshik at Google, 2015/04/23 19:48:12]

Acknowledged.

+    "America/Los_Angeles",
+    "America/Vancouver",
+    "America/Chicago",
+    "America/Winnipeg",
+    "America/Mexico_City",
+    "America/Buenos_Aires",
+    "Asia/Ho_Chi_Minh",
+    "Asia/Seoul",
+    "Europe/Athens",
+    "Asia/Ulaanbaatar",
+};
+
+}  // namespace
+
+namespace chromeos {
+namespace system {
+
+using icu::TimeZone;
+using icu::UnicodeString;
+
+class KnownTimeZoneTest : public testing::Test {
+ public:
+  KnownTimeZoneTest() {}
+  ~KnownTimeZoneTest() override {}
+
+  void SetUp() override {
+    for (auto id : kTimeZones) {

[oshima, 2015/04/23 18:30:45]

const auto*

[jungshik at Google, 2015/04/23 19:48:12]

Done. Do you prefer 'const char*' or 'const auto*' here? Down below, I used
'const char*'. I changed that to 'const auto*' to be more consistent. 

[oshima, 2015/04/23 22:01:24]

I'd use auto only if the type is long. More explicit is better, although I'm
fine with auto as well as this is obvious case.

+      timezones_.push_back(TimeZone::createTimeZone(UnicodeString(id)));
+    }
+  }
+
+  void TearDown() override { STLDeleteElements(&timezones_); }
+
+ protected:
+  std::vector<TimeZone*> timezones_;
+};
+
+TEST_F(KnownTimeZoneTest, IdMatch) {
+  static struct {
+    const char* id;
+    const char* matched;
+  } timezone_match_list[] = {
+      // Self matches
+      {"America/Los_Angeles", "America/Los_Angeles"},
+      {"America/Vancouver", "America/Vancouver"},  // Should not be Los_Angeles
+      {"America/Winnipeg", "America/Winnipeg"},
+      {"Asia/Seoul", "Asia/Seoul"},
+      // Canonical ID matches
+      {"Canada/Pacific", "America/Vancouver"},
+      {"US/Pacific", "America/Los_Angeles"},
+      {"US/Central", "America/Chicago"},
+      {"Mexico/General", "America/Mexico_City"},
+      {"Asia/Ulan_Bator", "Asia/Ulaanbaatar"},
+      // Asia/Saigon is canonical, but the list has Asia/Ho_Chi_Minh
+      {"Asia/Saigon", "Asia/Ho_Chi_Minh"},
+  };
+
+  for (const auto& pair : timezone_match_list) {
+    {

[oshima, 2015/04/23 18:30:45]

any reason for this extra block?

[jungshik at Google, 2015/04/23 19:48:12]

oops. what was I thinking? It's not necessary. 

+      scoped_ptr<TimeZone> input(
+          TimeZone::createTimeZone(UnicodeString(pair.id)));
+      scoped_ptr<TimeZone> expected(
+          TimeZone::createTimeZone(UnicodeString(pair.matched)));
+      const TimeZone* actual = GetKnownTimezoneOrNull(*input, timezones_);
+      EXPECT_NE(nullptr, actual) << "input=" << pair.id;
+      if (actual == nullptr)
+        continue;
+      UnicodeString actual_id;
+      actual->getID(actual_id);
+      std::string actual_id_str;
+      actual_id.toUTF8String(actual_id_str);
+      EXPECT_EQ(*expected, *actual) << "input=" << pair.id << ", "
+                                    << "expected=" << pair.matched << ", "
+                                    << "actual=" << actual_id_str;
+    }
+  }
+}
+
+TEST_F(KnownTimeZoneTest, NoMatch) {
+  static const char* no_match_list[] = {
+      "Africa/Juba",                // Not in the list
+      "Africa/Tripoli",             //  UTC+2 with no DST != Europe/Athens
+      "America/Tijuana",            // Historically != America/Los_Angeles
+      "Europe/Sofia",               // Historically != Europe/Athens
+      "America/Argentina/Cordoba",  // Historically != America/Buenos_Aires
+      "Asia/Tokyo",                 // Historically != Asia/Seoul
+  };
+  for (const char* id : no_match_list) {
+    {
+      scoped_ptr<TimeZone> input(TimeZone::createTimeZone(UnicodeString(id)));
+      EXPECT_EQ(NULL, GetKnownTimezoneOrNull(*input, timezones_))
+          << "input=" << id;
+    }
+  }
+}
+
+}  // namespace system
+}  // namespace chromeos