Make Hands-Off Zero-Touch Enrollment compatibile with tests

chrome/browser/chromeos/login/screens/update_screen_unittest.cc

Index: chrome/browser/chromeos/login/screens/update_screen_unittest.cc
diff --git a/chrome/browser/chromeos/login/screens/update_screen_unittest.cc b/chrome/browser/chromeos/login/screens/update_screen_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..6ea4239ddd47f0dc3be2b9f2d39620173e3c7746
--- /dev/null
+++ b/chrome/browser/chromeos/login/screens/update_screen_unittest.cc
@@ -0,0 +1,296 @@
+// Copyright 2017 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/chromeos/login/screens/update_screen.h"
+#include "base/command_line.h"
+#include "base/message_loop/message_loop.h"
+#include "base/test/scoped_mock_time_message_loop_task_runner.h"
+#include "chrome/browser/chromeos/login/screens/mock_base_screen_delegate.h"
+#include "chrome/browser/chromeos/login/screens/mock_error_screen.h"
+#include "chrome/browser/chromeos/login/screens/mock_update_screen.h"
+#include "chrome/browser/chromeos/login/startup_utils.h"
+#include "chrome/browser/chromeos/settings/cros_settings.h"
+#include "chrome/browser/chromeos/settings/device_settings_service.h"
+#include "chrome/test/base/scoped_testing_local_state.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chromeos/chromeos_switches.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/fake_update_engine_client.h"
+#include "chromeos/dbus/update_engine_client.h"
+#include "chromeos/network/network_handler.h"
+#include "chromeos/network/portal_detector/mock_network_portal_detector.h"
+#include "chromeos/network/portal_detector/network_portal_detector.h"
+#include "components/pairing/fake_host_pairing_controller.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using testing::_;
+using testing::AnyNumber;
+using testing::Return;
+
+namespace chromeos {
+
+class UpdateScreenUnitTest : public testing::Test {
+ public:
+  UpdateScreenUnitTest() : local_state_(TestingBrowserProcess::GetGlobal()) {}
+
+  // Fast-forwards time by the specified amount.
+  void FastForwardTime(base::TimeDelta time) {
+    runner_.task_runner()->FastForwardBy(time);

[xiyuan, 2017/05/15 17:24:23]

Is this relevant? 

[kumarniranjan, 2017/05/15 22:08:22]

Good catch! It's no longer needed.

+    base::Time last = StartupUtils::GetTimeOfLastUpdateCheckWithoutUpdate();

[xiyuan, 2017/05/15 17:24:23]

ASSERT_FALSE(last.is_null());

[kumarniranjan, 2017/05/15 22:08:21]

In StartupUtils::RegisterPrefs, I set this pref to have initial value "0" (since
Time is encoded as int64_t which is in turn encoded as a string). So,
technically, this does translate to a real time (when I was debugging the test
with print statements, I saw that this translated to sometime in the year 1601).
But adding the assert can't hurt, so I put it in anyway.

+    base::Time modified_last = last - time;
+    StartupUtils::SaveTimeOfLastUpdateCheckWithoutUpdate(modified_last);
+  }
+
+  // Simulates an update being available (or not).
+  // The parameter "available" indicates whether an update is available.
+  // The parameter "critical" indicates whether that update is critical.
+  void SimulateUpdateAvailable(bool available, bool critical = false) {

[xiyuan, 2017/05/15 17:24:23]

Avoid using default arg.

[kumarniranjan, 2017/05/15 22:08:21]

Done.

+    update_engine_status_.status =
+        UpdateEngineClient::UPDATE_STATUS_CHECKING_FOR_UPDATE;
+    fake_update_engine_client_->NotifyObserversThatStatusChanged(
+        update_engine_status_);
+    if (critical) {
+      ASSERT_TRUE(available) << "Does not make sense for an update to be "
+                                "critical if one is not even available.";
+      first_update_screen_->is_ignore_update_deadlines_ = true;
+    }
+    update_engine_status_.status =
+        available ? UpdateEngineClient::UPDATE_STATUS_UPDATE_AVAILABLE
+                  : UpdateEngineClient::UPDATE_STATUS_IDLE;
+    fake_update_engine_client_->NotifyObserversThatStatusChanged(
+        update_engine_status_);
+  }
+
+  // testing::Test:
+  void SetUp() override {
+    // Configure the browser to use Hands-Off Enrollment.
+    base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+        switches::kEnterpriseEnableZeroTouchEnrollment, "hands-off");
+
+    // Initialize objects needed by UpdateScreen
+    fake_update_engine_client_ = new FakeUpdateEngineClient();
+    DBusThreadManager::GetSetterForTesting()->SetUpdateEngineClient(
+        std::unique_ptr<UpdateEngineClient>(fake_update_engine_client_));
+    NetworkHandler::Initialize();
+    mock_network_portal_detector_ = new MockNetworkPortalDetector();
+    network_portal_detector::InitializeForTesting(
+        mock_network_portal_detector_);
+    mock_error_screen_ =
+        new MockErrorScreen(&mock_base_screen_delegate_, &mock_error_view_);
+    fake_controller_ = new pairing_chromeos::FakeHostPairingController("");
+
+    // Verify that we are using the right TaskRunner.
+    EXPECT_EQ(runner_.task_runner(),
+              base::MessageLoop::current()->task_runner().get());
+
+    // Ensure proper behavior of UpdateScreen's supporting objects.
+    EXPECT_CALL(*mock_network_portal_detector_, IsEnabled())
+        .Times(AnyNumber())
+        .WillRepeatedly(Return(false));
+    EXPECT_CALL(mock_base_screen_delegate_, GetErrorScreen())
+        .Times(AnyNumber())
+        .WillRepeatedly(Return(mock_error_screen_));
+
+    // Later verifies that UpdateScreen successfully exits both times.
+    EXPECT_CALL(mock_base_screen_delegate_,
+                OnExit(_, ScreenExitCode::UPDATE_NOUPDATE, _))
+        .Times(2);
+  }
+
+  void TearDown() override {
+    TestingBrowserProcess::GetGlobal()->SetShuttingDown(true);
+    delete first_update_screen_;
+    delete second_update_screen_;
+    delete mock_error_screen_;

[xiyuan, 2017/05/15 17:24:23]

Use std::uniqe_ptr<> and call .reset() here instead.

[kumarniranjan, 2017/05/15 22:08:21]

Done.

+    network_portal_detector::Shutdown();
+    network_portal_detector::EndTest();
+    NetworkHandler::Shutdown();
+    DBusThreadManager::Shutdown();
+  }
+
+ protected:
+  // A pointer to the UpdateScreen that shows up during the first OOBE.
+  UpdateScreen* first_update_screen_;
+
+  // A pointer to the UpdateScreen which shows up during the second OOBE.
+  // This test verifies proper behavior if the device is restarted before
+  // OOBE is complete, which is why there is a second OOBE.
+  UpdateScreen* second_update_screen_;
+
+  // Accessory objects needed by UpdateScreen.
+  MockBaseScreenDelegate mock_base_screen_delegate_;
+  MockUpdateView mock_view_;
+  MockNetworkErrorView mock_error_view_;
+  UpdateEngineClient::Status update_engine_status_;
+  MockErrorScreen* mock_error_screen_;
+  pairing_chromeos::FakeHostPairingController* fake_controller_;
+  MockNetworkPortalDetector* mock_network_portal_detector_;
+  FakeUpdateEngineClient* fake_update_engine_client_;
+
+ private:
+  // Test versions of core browser infrastructure.
+  content::TestBrowserThreadBundle threads_;
+  base::ScopedMockTimeMessageLoopTaskRunner runner_;
+  ScopedTestingLocalState local_state_;
+
+  DISALLOW_COPY_AND_ASSIGN(UpdateScreenUnitTest);
+};
+
+// Test Scenario Description:
+// In this description, "will" refers to an external event, and "should" refers
+// to the expected behavior of the DUT in response to external events.
+//
+// The DUT boots up and starts OOBE. Since it is a Hands-Off device, it
+// proceeds through OOBE automatically. When it hits the UpdateScreen,
+// it checks for updates. It will find that there is indeed an update
+// available, will then install it. After installing the update, it should
+// continue with Hands-Off OOBE. Then, before OOBE is complete, something
+// (could be user, environment, anything) will cause the DUT to reboot.
+// Since OOBE is not complete, the DUT goes through OOBE again.
+// When the DUT hits the UpdateScreen during this second OOBE run-through,
+// it should check for updates again.
+TEST_F(UpdateScreenUnitTest, ChecksForUpdateBothTimesIfFirstIsInstalled) {
+  // DUT reaches UpdateScreen for the first time.
+  first_update_screen_ = new UpdateScreen(&mock_base_screen_delegate_,
+                                          &mock_view_, fake_controller_);
+  first_update_screen_->StartNetworkCheck();
+
+  // Verify that the DUT checks for an update.
+  EXPECT_EQ(fake_update_engine_client_->request_update_check_call_count(), 1);
+
+  // An update is available.
+  SimulateUpdateAvailable(true);
+
+  // DUT reboots...
+  // After rebooting, the DUT reaches UpdateScreen for the second time.
+  second_update_screen_ = new UpdateScreen(&mock_base_screen_delegate_,
+                                           &mock_view_, fake_controller_);
+  second_update_screen_->StartNetworkCheck();

[xiyuan, 2017/05/15 17:24:23]

Looks like |first_update_screen_| and |second_update_screen_| can be locals of
the test case instead of being member of the test fixture.

[kumarniranjan, 2017/05/15 22:08:22]

Done.

+
+  // Verify that the DUT checks for updates again.
+  EXPECT_EQ(fake_update_engine_client_->request_update_check_call_count(), 2);
+
+  // No updates available this time.
+  SimulateUpdateAvailable(false);

[xiyuan, 2017/05/15 17:24:22]

Is this call necessary? There is no follow up test code. Why do we need it?

[kumarniranjan, 2017/05/15 22:08:22]

It is necessary to satisfy the expectation defined at the end of SetUp:
  96     // Later verifies that UpdateScreen successfully exits both times.
  97     EXPECT_CALL(mock_base_screen_delegate_,
  98                 OnExit(_, ScreenExitCode::UPDATE_NOUPDATE, _))
  99         .Times(2);

It is also more similar to the scenario the UpdateScreen will face in real use.

+}
+
+// Test Scenario Description:
+// In this description, "will" refers to an external event, and "should" refers
+// to the expected behavior of the DUT in response to external events.
+//
+// The DUT boots up and starts OOBE. Since it is a Hands-Off device, it
+// proceeds through OOBE automatically. When it hits the UpdateScreen,
+// it checks for updates. It will find that there are no updates
+// available, and it should leave the UpdateScreen without installing any
+// updates. It continues with OOBE. Then, before OOBE is complete, something
+// (could be user, environment, anything) will cause the DUT to reboot.
+// Since OOBE is not complete, the DUT goes through OOBE again.
+// When the DUT hits the UpdateScreen during this second OOBE run-through,
+// more than one hour will have passed since the previous update check.
+// Therefore, the DUT should check for updates again.
+TEST_F(UpdateScreenUnitTest, ChecksForUpdateBothTimesIfEnoughTimePasses) {
+  // DUT reaches UpdateScreen for the first time.
+  first_update_screen_ = new UpdateScreen(&mock_base_screen_delegate_,
+                                          &mock_view_, fake_controller_);
+  first_update_screen_->StartNetworkCheck();
+
+  // Verify that the DUT checks for updates.
+  EXPECT_EQ(fake_update_engine_client_->request_update_check_call_count(), 1);
+
+  // No updates are available.
+  SimulateUpdateAvailable(false);
+
+  // Fast-forward time by one hour.
+  FastForwardTime(base::TimeDelta::FromHours(1));
+
+  // DUT reboots...
+  // After rebooting, the DUT reaches UpdateScreen for the second time.
+  second_update_screen_ = new UpdateScreen(&mock_base_screen_delegate_,
+                                           &mock_view_, fake_controller_);
+  second_update_screen_->StartNetworkCheck();
+
+  // Verify that the DUT checks for updates again.
+  EXPECT_EQ(fake_update_engine_client_->request_update_check_call_count(), 2);
+
+  // No updates available this time either.
+  SimulateUpdateAvailable(false);
+}
+
+// Test Scenario Description:
+// In this description, "will" refers to an external event, and "should" refers
+// to the expected behavior of the DUT in response to external events.
+//
+// The DUT boots up and starts OOBE. Since it is a Hands-Off device, it
+// proceeds through OOBE automatically. When it hits the UpdateScreen,
+// it checks for updates. It will find that there are no updates
+// available, and it should leave the UpdateScreen without installing any
+// updates. It continues with OOBE. Then, before OOBE is complete, something
+// (could be user, environment, anything) will cause the DUT to reboot.
+// Since OOBE is not complete, the DUT goes through OOBE again.
+// When the DUT hits the UpdateScreen during this second OOBE run-through,
+// less than one hour will have passed since the previous update check.
+// Therefore, the DUT should not check for updates again.
+TEST_F(UpdateScreenUnitTest, ChecksForUpdateOnceButNotAgainIfTooSoon) {
+  // DUT reaches UpdateScreen for the first time.
+  first_update_screen_ = new UpdateScreen(&mock_base_screen_delegate_,
+                                          &mock_view_, fake_controller_);
+  first_update_screen_->StartNetworkCheck();
+
+  // Verify that the DUT checks for updates.
+  EXPECT_EQ(fake_update_engine_client_->request_update_check_call_count(), 1);
+
+  // No update available.
+  SimulateUpdateAvailable(false);
+
+  // DUT reboots...
+  // After rebooting, the DUT reaches UpdateScreen for the second time.
+  second_update_screen_ = new UpdateScreen(&mock_base_screen_delegate_,
+                                           &mock_view_, fake_controller_);
+  second_update_screen_->StartNetworkCheck();
+
+  // Verify that the DUT did not check for updates again.
+  EXPECT_EQ(fake_update_engine_client_->request_update_check_call_count(), 1);
+
+  // No update available this time either.
+  SimulateUpdateAvailable(false);
+}
+
+// Test Scenario Description:
+// In this description, "will" refers to an external event, and "should" refers
+// to the expected behavior of the DUT in response to external events.
+//
+// The DUT boots up and starts OOBE. Since it is a Hands-Off device, it
+// proceeds through OOBE automatically. When it hits the UpdateScreen,
+// it checks for updates. It will find that a critical update is available.
+// The DUT installs the update, and because the update is critical, it reboots.
+// Since OOBE is not complete, the DUT goes through OOBE again after reboot.
+// When the DUT hits the UpdateScreen during this second OOBE run-through,
+// it should check for updates again.
+TEST_F(UpdateScreenUnitTest, ChecksForUpdateBothTimesIfCriticalUpdate) {
+  // DUT reaches UpdateScreen for the first time.
+  first_update_screen_ = new UpdateScreen(&mock_base_screen_delegate_,
+                                          &mock_view_, fake_controller_);
+  first_update_screen_->StartNetworkCheck();
+
+  // Verify that the DUT checks for updates.
+  EXPECT_EQ(fake_update_engine_client_->request_update_check_call_count(), 1);
+
+  // An update is available, and it's critical!
+  SimulateUpdateAvailable(true, true);
+
+  // DUT reboots...
+  // After rebooting, the DUT reaches UpdateScreen for the second time.
+  second_update_screen_ = new UpdateScreen(&mock_base_screen_delegate_,
+                                           &mock_view_, fake_controller_);
+  second_update_screen_->StartNetworkCheck();
+
+  // Verify that the DUT checks for updates again.
+  EXPECT_EQ(fake_update_engine_client_->request_update_check_call_count(), 2);
+
+  // No update available this time.
+  SimulateUpdateAvailable(false);
+}
+
+}  // namespace chromeos