Fix DeviceStatusCollectorTest.MaxStoredPeriods.

chrome/browser/policy/device_status_collector_unittest.cc

 // Copyright (c) 2012 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 "chrome/browser/policy/device_status_collector.h"
 
 #include "base/logging.h"
 #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"
 #include "chrome/browser/chromeos/settings/stub_cros_settings_provider.h"
 #include "chrome/browser/chromeos/system/mock_statistics_provider.h"
 #include "chrome/browser/chromeos/system/statistics_provider.h"
 #include "chrome/browser/policy/proto/device_management_backend.pb.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_pref_service.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_browser_thread.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::testing::_;
 using ::testing::DoAll;
 using ::testing::NotNull;
 using ::testing::Return;
 using ::testing::SetArgPointee;
 using base::TimeDelta;
 using base::Time;
 
 namespace em = enterprise_management;
 
 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));
 }
 
 void MockPositionUpdateRequester(
     const content::GeolocationUpdateCallback& callback) {
   if (!mock_position_to_return_next.get())
     return;
 
   // If the fix is invalid, the DeviceStatusCollector will immediately request
   // another update when it receives the callback. This is desirable and safe in
   // real life where geolocation updates arrive asynchronously. In this testing
   // harness, the callback is invoked synchronously upon request, leading to a
   // request-callback loop. The loop is broken by returning the mock position
   // only once.
   scoped_ptr<content::Geoposition> position(
       mock_position_to_return_next.release());
   callback.Run(*position);
 }
 
 class TestingDeviceStatusCollector : public policy::DeviceStatusCollector {
  public:
   TestingDeviceStatusCollector(
       PrefService* local_state,
       chromeos::system::StatisticsProvider* provider)
       : 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) {
     for (int i = 0; i < len; i++)
       IdleStateCallback(states[i]);
   }
 
   void set_max_stored_past_activity_days(unsigned int value) {
     max_stored_past_activity_days_ = value;
   }
 
   void set_max_stored_future_activity_days(unsigned int value) {
     max_stored_future_activity_days_ = value;
   }
 
   // Reset the baseline time.
   void SetBaselineTime(Time time) {
     baseline_time_ = time;
     baseline_offset_periods_ = 0;
   }
 
  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
   // later than the previous time.
   virtual Time GetCurrentTime() OVERRIDE {
     int poll_interval = policy::DeviceStatusCollector::kIdlePollIntervalSeconds;
     return baseline_time_ +
         TimeDelta::FromSeconds(poll_interval * baseline_offset_periods_++);
   }
 
@@ skipping 18 matching lines @@
 }  // namespace
 
 namespace policy {
 
 class DeviceStatusCollectorTest : public testing::Test {
  public:
   DeviceStatusCollectorTest()
     : 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()))
         .WillRepeatedly(Return(false));
 
     // Remove the real DeviceSettingsProvider and replace it with a stub.
     cros_settings_ = chromeos::CrosSettings::Get();
     device_settings_provider_ =
         cros_settings_->GetProvider(chromeos::kReportDeviceVersionInfo);
     EXPECT_TRUE(device_settings_provider_ != NULL);
@@ skipping 44 matching lines @@
     EXPECT_TRUE(location.has_timestamp());
     EXPECT_FALSE(location.has_altitude());
     EXPECT_FALSE(location.has_altitude_accuracy());
     EXPECT_FALSE(location.has_heading());
     EXPECT_FALSE(location.has_speed());
     EXPECT_FALSE(location.has_error_message());
     EXPECT_DOUBLE_EQ(4.3, location.latitude());
     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() {
     GetStatus();
     EXPECT_TRUE(status_.has_device_location());
     em::DeviceLocation location = status_.device_location();
     EXPECT_TRUE(location.has_error_code());
     EXPECT_EQ(em::DeviceLocation::ERROR_CODE_POSITION_UNAVAILABLE,
               location.error_code());
   }
 
  protected:
   // Convenience method.
   int64 ActivePeriodMilliseconds() {
     return policy::DeviceStatusCollector::kIdlePollIntervalSeconds * 1000;
   }
 
   MessageLoop message_loop_;
   content::TestBrowserThread ui_thread_;
   content::TestBrowserThread file_thread_;
   content::TestBrowserThread io_thread_;
 
+  base::ScopedEnvironmentVariable timezone_override_;
   TestingPrefService prefs_;
   chromeos::system::MockStatisticsProvider statistics_provider_;
   scoped_ptr<TestingDeviceStatusCollector> status_collector_;
   em::DeviceStatusReportRequest status_;
   chromeos::CrosSettings* cros_settings_;
   chromeos::CrosSettingsProvider* device_settings_provider_;
   chromeos::StubCrosSettingsProvider stub_settings_provider_;
 };
 
 TEST_F(DeviceStatusCollectorTest, AllIdle) {
@@ skipping 96 matching lines @@
     IDLE_STATE_IDLE,
     IDLE_STATE_IDLE
   };
   cros_settings_->SetBoolean(chromeos::kReportDeviceActivityTimes, true);
   status_collector_->Simulate(test_states,
                               sizeof(test_states) / sizeof(IdleState));
   GetStatus();
   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
   };
   unsigned int max_days = 10;
 
   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++) {
     status_collector_->Simulate(test_states,
                                 sizeof(test_states) / sizeof(IdleState));
     // Advance the simulated clock by a day.
     baseline += TimeDelta::FromDays(1);
     status_collector_->SetBaselineTime(baseline);
   }
 
@@ skipping 12 matching lines @@
   // Set the clock back so the previous simulated times are in the future.
   baseline -= TimeDelta::FromDays(20);
   status_collector_->SetBaselineTime(baseline);
 
   // Collect one more data point to trigger pruning.
   status_collector_->Simulate(test_states, 1);
 
   // 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) {
   // If the pref for collecting device activity times isn't explicitly turned
   // on, no data on activity times should be reported.
 
   IdleState test_states[] = {
     IDLE_STATE_ACTIVE,
     IDLE_STATE_ACTIVE,
     IDLE_STATE_ACTIVE
   };
   status_collector_->Simulate(test_states,
                               sizeof(test_states) / sizeof(IdleState));
   GetStatus();
   EXPECT_EQ(0, status_.active_period_size());
   EXPECT_EQ(0, GetActiveMilliseconds(status_));
 }
 
 TEST_F(DeviceStatusCollectorTest, ActivityCrossingMidnight) {
   IdleState test_states[] = {
     IDLE_STATE_ACTIVE
   };
   cros_settings_->SetBoolean(chromeos::kReportDeviceActivityTimes, true);
 
   // 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();
   ASSERT_EQ(2, status_.active_period_size());
 
   em::ActiveTimePeriod period0 = status_.active_period(0);
   em::ActiveTimePeriod period1 = status_.active_period(1);
   EXPECT_EQ(ActivePeriodMilliseconds() - 10000, period0.active_duration());
   EXPECT_EQ(10000, period1.active_duration());
 
@@ skipping 64 matching lines @@
   // don't have any reasonable values inside the unit test, so those
   // aren't checked.
   EXPECT_NE("", status_.browser_version());
 }
 
 TEST_F(DeviceStatusCollectorTest, Location) {
   content::Geoposition valid_fix;
   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.
   SetMockPositionToReturnNext(valid_fix);
   CheckThatNoLocationIsReported();
 
   // Check that when device location reporting is enabled and a valid fix is
   // available, the location is reported and is stored in local state.
   SetMockPositionToReturnNext(valid_fix);
   cros_settings_->SetBoolean(chromeos::kReportDeviceLocation, true);
@@ skipping 19 matching lines @@
   // Check that after enabling location reporting again, an error is reported
   // if no valid fix is available.
   SetMockPositionToReturnNext(invalid_fix);
   cros_settings_->SetBoolean(chromeos::kReportDeviceLocation, true);
   // Allow the new pref to propagate to the status collector.
   message_loop_.RunAllPending();
   CheckThatALocationErrorIsReported();
 }
 
 }  // namespace policy