Fix DeviceStatusCollectorTest.MaxStoredPeriods.

chrome/browser/policy/device_status_collector_unittest.cc

Index: chrome/browser/policy/device_status_collector_unittest.cc
diff --git a/chrome/browser/policy/device_status_collector_unittest.cc b/chrome/browser/policy/device_status_collector_unittest.cc
index 245b933aa8497ceb4b6bfe3fc789b711f3a0041a..a112f653ae6a2aecb41651f93e6da93b397ed470 100644
--- a/chrome/browser/policy/device_status_collector_unittest.cc
+++ b/chrome/browser/policy/device_status_collector_unittest.cc
@@ -8,6 +8,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/threading/sequenced_worker_pool.h"
+#include "base/test/scoped_environment_variable.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/settings/cros_settings_names.h"
 #include "chrome/browser/chromeos/settings/cros_settings_provider.h"
@@ -37,8 +38,15 @@ namespace {
 
 const int64 kMillisecondsPerDay = Time::kMicrosecondsPerDay / 1000;
 
+const char kTimezoneEnvVariable[] = "TZ";
+const char kTestTimezone[] = "UTC";
+
 scoped_ptr<content::Geoposition> mock_position_to_return_next;
 
+Time TestBaselineTime() {
+  return Time::UnixEpoch() + TimeDelta::FromDays(42) + TimeDelta::FromHours(5);

[jar (doing other things), 2012/11/16 20:29:23]

Can you add a comment indicating how these numbers were chosen, and what the
implications are?

[Joao da Silva, 2012/11/20 09:10:52]

This has been removed. It was a misguided attempt to make this test pass under
any timezone.

+}
+
 void SetMockPositionToReturnNext(const content::Geoposition &position) {
   mock_position_to_return_next.reset(new content::Geoposition(position));
 }
@@ -67,9 +75,12 @@ class TestingDeviceStatusCollector : public policy::DeviceStatusCollector {
       : policy::DeviceStatusCollector(local_state,
                                       provider,
                                       &MockPositionUpdateRequester) {
-    // Set the baseline time to a fixed value (1 AM) to prevent test flakiness
+    // Set the baseline time to a fixed value to prevent test flakiness
     // due to a single activity period spanning two days.
-    SetBaselineTime(Time::Now().LocalMidnight() + TimeDelta::FromHours(1));
+    SetBaselineTime(TestBaselineTime());
+
+    // Initialize the collector after setting the baseline time.
+    Init();
   }
 
   void Simulate(IdleState* states, int len) {
@@ -94,7 +105,7 @@ class TestingDeviceStatusCollector : public policy::DeviceStatusCollector {
  protected:
   virtual void CheckIdleState() OVERRIDE {
     // This should never be called in testing, as it results in a dbus call.
-    NOTREACHED();
+    ADD_FAILURE();
   }
 
   // Each time this is called, returns a time that is a fixed increment
@@ -133,7 +144,8 @@ class DeviceStatusCollectorTest : public testing::Test {
     : message_loop_(MessageLoop::TYPE_UI),
       ui_thread_(content::BrowserThread::UI, &message_loop_),
       file_thread_(content::BrowserThread::FILE, &message_loop_),
-      io_thread_(content::BrowserThread::IO, &message_loop_) {
+      io_thread_(content::BrowserThread::IO, &message_loop_),
+      timezone_override_(kTimezoneEnvVariable, kTestTimezone) {
     TestingDeviceStatusCollector::RegisterPrefs(&prefs_);
 
     EXPECT_CALL(statistics_provider_, GetMachineStatistic(_, NotNull()))
@@ -198,8 +210,8 @@ class DeviceStatusCollectorTest : public testing::Test {
     EXPECT_DOUBLE_EQ(-7.8, location.longitude());
     EXPECT_DOUBLE_EQ(3., location.accuracy());
     // Check that the timestamp is not older than ten minutes.
-    EXPECT_TRUE(Time::Now() - Time::FromDoubleT(location.timestamp() / 1000.) <
-                TimeDelta::FromMinutes(10));
+    Time timestamp = Time::FromDoubleT(location.timestamp() / 1000.0);
+    EXPECT_TRUE(TestBaselineTime() - timestamp < TimeDelta::FromMinutes(10));
   }
 
   void CheckThatALocationErrorIsReported() {
@@ -222,6 +234,7 @@ class DeviceStatusCollectorTest : public testing::Test {
   content::TestBrowserThread file_thread_;
   content::TestBrowserThread io_thread_;
 
+  base::ScopedEnvironmentVariable timezone_override_;
   TestingPrefService prefs_;
   chromeos::system::MockStatisticsProvider statistics_provider_;
   scoped_ptr<TestingDeviceStatusCollector> status_collector_;
@@ -338,9 +351,7 @@ TEST_F(DeviceStatusCollectorTest, Times) {
   EXPECT_EQ(3 * ActivePeriodMilliseconds(), GetActiveMilliseconds(status_));
 }
 
-// Fails after after WebKit roll [132375:132450]
-// http://crbug.com/157848
-TEST_F(DeviceStatusCollectorTest, DISABLED_MaxStoredPeriods) {
+TEST_F(DeviceStatusCollectorTest, MaxStoredPeriods) {
   IdleState test_states[] = {
     IDLE_STATE_ACTIVE,
     IDLE_STATE_IDLE
@@ -350,7 +361,7 @@ TEST_F(DeviceStatusCollectorTest, DISABLED_MaxStoredPeriods) {
   cros_settings_->SetBoolean(chromeos::kReportDeviceActivityTimes, true);
   status_collector_->set_max_stored_past_activity_days(max_days - 1);
   status_collector_->set_max_stored_future_activity_days(1);
-  Time baseline = Time::Now().LocalMidnight();
+  Time baseline = TestBaselineTime();
 
   // Simulate 12 active periods.
   for (int i = 0; i < static_cast<int>(max_days) + 2; i++) {
@@ -383,7 +394,7 @@ TEST_F(DeviceStatusCollectorTest, DISABLED_MaxStoredPeriods) {
   // Check that we don't exceed the max number of periods.
   status_.clear_active_period();
   GetStatus();
-  EXPECT_LT(status_.active_period_size(), static_cast<int>(max_days));
+  EXPECT_LE(status_.active_period_size(), static_cast<int>(max_days));

[jar (doing other things), 2012/11/16 20:29:23]

nit: It is strange that all but one use of |max_days| undergoes a cast.  Why is
this unsigned?... or perhaps, why is the slot that needs to be set in line 362
unsigned?

[Joao da Silva, 2012/11/20 09:10:52]

The casts have been removed from the test. Why the slot is unsigned has to be
asked to the original authors :-) @dubroy, do you have an idea about that?

 }
 
 TEST_F(DeviceStatusCollectorTest, ActivityTimesDisabledByDefault) {
@@ -410,7 +421,7 @@ TEST_F(DeviceStatusCollectorTest, ActivityCrossingMidnight) {
 
   // Set the baseline time to 10 seconds after midnight.
   status_collector_->SetBaselineTime(
-      Time::Now().LocalMidnight() + TimeDelta::FromSeconds(10));
+      TestBaselineTime().LocalMidnight() + TimeDelta::FromSeconds(10));
 
   status_collector_->Simulate(test_states, 1);
   GetStatus();
@@ -495,12 +506,12 @@ TEST_F(DeviceStatusCollectorTest, Location) {
   valid_fix.latitude = 4.3;
   valid_fix.longitude = -7.8;
   valid_fix.accuracy = 3.;
-  valid_fix.timestamp = Time::Now();
+  valid_fix.timestamp = TestBaselineTime() - TimeDelta::FromMinutes(5);
 
   content::Geoposition invalid_fix;
   invalid_fix.error_code =
       content::Geoposition::ERROR_CODE_POSITION_UNAVAILABLE;
-  invalid_fix.timestamp = Time::Now();
+  invalid_fix.timestamp = TestBaselineTime() - TimeDelta::FromMinutes(4);
 
   // Check that when device location reporting is disabled, no location is
   // reported.