[i18n] Reinit icu_util state, correctly

The old code cleared ICU library internal state between test runs, but
not the icu_util internal state.  This caused the new ICU configurations
never to be loaded, since the default behavior of the loader is to make
loads lazy and idempotent.

Clearing the internal state of icu_util enabled loading new
ICU data in each individual test case.

This initialization issue may have contributed to arm64 test failures
that mysteriously came and went a few days back, possibly related to
test run ordering.

Bug: fuchsia:45491, 1047475
Change-Id: Ib9290952ea1955b27c752dcaf78a8a6de7fbf77b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2042057
Commit-Queue: Filip Filmar <fmil@google.com>
Reviewed-by: David Dorwin <ddorwin@chromium.org>
Reviewed-by: Kevin Marshall <kmarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740833}

base/fuchsia/time_zone_data_unittest.cc

@@ -6,6 +6,7 @@
 
 #include "base/environment.h"
 #include "base/files/file_util.h"
+#include "base/strings/string_util.h"
 #include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/icu/source/common/unicode/uclean.h"
@@ -14,13 +15,33 @@
 namespace base {
 namespace i18n {
 
+namespace {
+
+// Directory path to the tzdata configuration files.
+const char kTzDataDirPath[] = "/pkg/base/test/data/tzdata/2019a/44/le";
+
+// File path to the text file containing the expected ICU library revision, for
+// example "2019c".
+const char kRevisionFilePath[] = "/config/data/tzdata/revision.txt";
+
+}  // namespace
+
 class TimeZoneDataTest : public testing::Test {
  protected:
   TimeZoneDataTest() : env_(base::Environment::Create()) {}
 
-  void SetUp() override { u_cleanup(); }
+  void SetUp() override { ResetIcu(); }
 
-  void TearDown() override { u_cleanup(); }
+  void TearDown() override { ResetIcu(); }
+
+  // Needed to enable loading of ICU config files that are different from what
+  // is available in Chromium.  Both icu_util and ICU library keep internal
+  // state so clear both.
+  void ResetIcu() {
+    // Clears the state in the reverse order of construction.
+    u_cleanup();
+    ResetGlobalsForTesting();
+  }
 
   void GetActualRevision(std::string* icu_version) {
     UErrorCode err = U_ZERO_ERROR;
@@ -31,44 +52,59 @@ class TimeZoneDataTest : public testing::Test {
   std::unique_ptr<base::Environment> env_;
 };
 
-TEST_F(TimeZoneDataTest, DISABLED_RevisionFromConfig) {
-  // Config data is not available on Chromium test runners, so don't actually
-  // run this test there.  A marker for this is the presence of revision.txt.
-  if (!base::PathExists(base::FilePath("/config/data/tzdata/revision.txt")))
+// Loads a file revision.txt from the actual underlying filesystem, which
+// contains the tzdata version we expect to be able to load.  It then attempts
+// to load this configuration from the default path and compares the version it
+// obtained from the load with the expected version, failing on version
+// mismatch.
+//
+// In Fuchsia build bot setup, we ensure that the file revision.txt exists, so
+// that this test is not skipped. In Chromium build bot setup, this file may
+// not be present, in which case we skip running this test.
+TEST_F(TimeZoneDataTest, CompareSystemRevisionWithExpected) {
+  if (!base::PathExists(base::FilePath(kRevisionFilePath))) {
+    LOG(INFO) << "Skipped test because tzdata config is not present";
     return;
+  }
 
   // Ensure that timezone data is loaded from the default location.
   ASSERT_TRUE(env_->UnSetVar("ICU_TIMEZONE_FILES_DIR"));
 
-  InitializeICU();
+  ASSERT_TRUE(InitializeICU());
   std::string expected;
-  ASSERT_TRUE(base::ReadFileToString(
-      base::FilePath("/config/data/tzdata/revision.txt"), &expected));
+  ASSERT_TRUE(
+      base::ReadFileToString(base::FilePath(kRevisionFilePath), &expected));
   std::string actual;
   GetActualRevision(&actual);
   EXPECT_EQ(expected, actual);
 }
 
-TEST_F(TimeZoneDataTest, DISABLED_RevisionFromTestData) {
-  // TODO(1047475): Adding more asserts in case they give more hints to the
-  // buildbot failures.
-  ASSERT_TRUE(base::DirectoryExists(
-      base::FilePath("/pkg/base/test/data/tzdata/2019a/44/le")));
-  ASSERT_TRUE(base::PathExists(
-      base::FilePath("/pkg/base/test/data/tzdata/2019a/44/le/zoneinfo64.res")));
-  ASSERT_TRUE(base::PathExists(base::FilePath(
-      "/pkg/base/test/data/tzdata/2019a/44/le/timezoneTypes.res")));
-  ASSERT_TRUE(base::PathExists(
-      base::FilePath("/pkg/base/test/data/tzdata/2019a/44/le/metaZones.res")));
-
-  // Ensure that timezone data is loaded from test data.
-  ASSERT_TRUE(env_->SetVar("ICU_TIMEZONE_FILES_DIR",
-                           "/pkg/base/test/data/tzdata/2019a/44/le"));
+// Verifies that the current version of the ICU library in use can load ICU
+// data in a specific version format (in this case 44).  Designed to fail if
+// the ICU library version used in Chromium drifts from version 44 so much that
+// the library is no longer able to load the old tzdata.  If the test fails,
+// this could be a sign that all platforms Chromium runs on need to upgrade the
+// ICU library versions.
+TEST_F(TimeZoneDataTest, TestLoadingTimeZoneDataFromKnownConfigs) {
+  ASSERT_TRUE(base::DirectoryExists(base::FilePath(kTzDataDirPath)));
+  ASSERT_TRUE(env_->SetVar("ICU_TIMEZONE_FILES_DIR", kTzDataDirPath));
+
+  EXPECT_TRUE(InitializeICU());
+  std::string actual;
+  GetActualRevision(&actual);
+  EXPECT_EQ("2019a", actual) << "If ICU no longer supports this tzdata "
+                                "version, tzdata version needs to be upgraded";
+}
+
+TEST_F(TimeZoneDataTest, DoesNotCrashWithInvalidPath) {
+  ASSERT_TRUE(env_->SetVar("ICU_TIMEZONE_FILES_DIR", "/some/nonexistent/path"));
 
   EXPECT_TRUE(InitializeICU());
   std::string actual;
   GetActualRevision(&actual);
-  EXPECT_EQ("2019a", actual);
+  EXPECT_TRUE(
+      base::StartsWith(actual, "20", base::CompareCase::INSENSITIVE_ASCII))
+      << "Got version: " << actual;
 }
 
 }  // namespace i18n