Skip to content

Commit

Permalink
Revert "Make timezone unittest less dependent on environment"
Browse files Browse the repository at this point in the history
This reverts commit b9e3249.

Reason for revert: breaks MSAN base_unittests: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20MSan%20Tests/14315

[ RUN      ] TimezoneTest.CountryCodeForTimezones
==12431==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x55bd41fbd0c1 in icu_63::TZEnumeration::snext(UErrorCode&) ./../../third_party/icu/source/i18n/timezone.cpp:931:31
    #1 0x55bd3e8c9c45 in base::(anonymous namespace)::TimezoneTest_CountryCodeForTimezones_Test::TestBody() ./../../base/i18n/timezone_unittest.cc:20:58
    #2 0x55bd414ee9a0 in HandleExceptionsInMethodIfSupported<testing::Test, void> ./../../third_party/googletest/src/googletest/src/gtest.cc:0:0
    #3 0x55bd414ee9a0 in testing::Test::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2522:0
    #4 0x55bd414f24d3 in testing::TestInfo::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2703:11
    #5 0x55bd414f40e9 in testing::TestCase::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2825:28
    #6 0x55bd4152e9d9 in testing::internal::UnitTestImpl::RunAllTests() ./../../third_party/googletest/src/googletest/src/gtest.cc:5227:43
    #7 0x55bd4152d2c8 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> ./../../third_party/googletest/src/googletest/src/gtest.cc:0:0
    #8 0x55bd4152d2c8 in testing::UnitTest::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:4835:0
    #9 0x55bd41b8f42a in RUN_ALL_TESTS ./../../third_party/googletest/src/googletest/include/gtest/gtest.h:2369:46
    #10 0x55bd41b8f42a in base::TestSuite::Run() ./../../base/test/test_suite.cc:294:0
    #11 0x55bd41bb9d49 in Run ./../../base/callback.h:99:12
    #12 0x55bd41bb9d49 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, bool, base::OnceCallback<void ()>) ./../../base/test/launcher/unit_test_launcher.cc:225:0
    #13 0x55bd41bb94a7 in base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>) ./../../base/test/launcher/unit_test_launcher.cc:575:10
    #14 0x55bd41b522a8 in main ./../../base/test/run_all_base_unittests.cc:12:10
    #15 0x7f1906a7ef44 in __libc_start_main ??:0:0
    #16 0x55bd3dc8d029 in _start ??:0:0


Original change's description:
> Make timezone unittest less dependent on environment
> 
> Instead of only testing the current default timezone, test all
> timezones.
> 
> Noticed this while looking into crbug.com/920094
> 
> Bug: none
> Change-Id: Ida24c063cefb384037fc77c3673ff2d73ab7354f
> Reviewed-on: https://chromium-review.googlesource.com/c/1407437
> Reviewed-by: Jungshik Shin <jshin@chromium.org>
> Commit-Queue: Evan Stade <estade@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#622505}

TBR=thestig@chromium.org,estade@chromium.org,jshin@chromium.org

Change-Id: I86d37bbe6749f3f8a5b81bf3967755e43f441161
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: none
Reviewed-on: https://chromium-review.googlesource.com/c/1409815
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622600}
  • Loading branch information
jeremyroman authored and Commit Bot committed Jan 14, 2019
1 parent 5b9b708 commit 237ecef
Showing 1 changed file with 12 additions and 24 deletions.
36 changes: 12 additions & 24 deletions base/i18n/timezone_unittest.cc
Expand Up @@ -5,34 +5,22 @@
#include "base/i18n/timezone.h"

#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/icu/source/common/unicode/strenum.h"
#include "third_party/icu/source/common/unicode/unistr.h"
#include "third_party/icu/source/i18n/unicode/timezone.h"

namespace base {
namespace {

TEST(TimezoneTest, CountryCodeForTimezones) {
std::unique_ptr<icu::StringEnumeration> timezones(
icu::TimeZone::createEnumeration());

UErrorCode status;
while (const icu::UnicodeString* timezone = timezones->snext(status)) {
icu::TimeZone::adoptDefault(icu::TimeZone::createTimeZone(*timezone));

std::string country_code = CountryCodeForCurrentTimezone();
// On some systems (such as Android or some flavors of Linux), ICU may come
// up empty. With https://chromium-review.googlesource.com/c/512282/ , ICU
// will not fail any more. See also
// http://bugs.icu-project.org/trac/ticket/13208 . Even with that, ICU
// returns '001' (world) for region-agnostic timezones such as Etc/UTC and
// |CountryCodeForCurrentTimezone| returns an empty string so that the next
// fallback can be tried by a customer.
if (!country_code.empty())
EXPECT_EQ(2U, country_code.size()) << "country_code = " << country_code;
}

icu::TimeZone::adoptDefault(nullptr);
TEST(TimezoneTest, CountryCodeForCurrentTimezone) {
std::string country_code = CountryCodeForCurrentTimezone();
// On some systems (such as Android or some flavors of Linux), ICU may come up
// empty. With https://chromium-review.googlesource.com/c/512282/ , ICU will
// not fail any more. See also http://bugs.icu-project.org/trac/ticket/13208 .
// Even with that, ICU returns '001' (world) for region-agnostic timezones
// such as Etc/UTC and |CountryCodeForCurrentTimezone| returns an empty
// string so that the next fallback can be tried by a customer.
// TODO(jshin): Revise this to test for actual timezones using
// use ScopedRestoreICUDefaultTimezone.
if (!country_code.empty())
EXPECT_EQ(2U, country_code.size()) << "country_code = " << country_code;
}

} // namespace
Expand Down

0 comments on commit 237ecef

Please sign in to comment.