Migrate the BattOr agent test to a MockTimeTaskRunner

This is necessary to migrate the BattOr agent to the new Chrome task
running API (http://bit.ly/2jhO3J6). The reason for this is somewhat
subtle: in the old world, we use a "TestSimpleTaskRunner", which runs
delayed tasks in the order in which they're supposed to run but runs
them immediately. Therefore, RunUntilIdle() effectively fasts forwards
through time.

With the new task posting architecture, TestSimpleTaskRunner no longer
does this, instead by default running non-delayed tasks. This means that
when we wait the 100ms after the last clock sync marker in order to
run stop tracing, we're effectively waiting forever instead of what we
previously did, which is immediately assume that 100ms had elapsed.

In order to get around this, I'm migrating the BattOr agent tests to
what they probably should have used in the first place: a
TestMockTimeTaskRunner. This task runner allows us to programmatically
set the current time, which gives us a lot more control in our unit
tests. As part of this, rather than getting our time from
TimeTicks::Now(), we add a layer of indirection, a TickClock, that
can provide the current TimeTicks of the task runnner clock rather than
the real current clock.

Bug: 761498
Change-Id: I4f59b0bcd7138f11c7b20e659891cbbb066a6b26
Reviewed-on: https://chromium-review.googlesource.com/658259
Reviewed-by: rnephew <rnephew@chromium.org>
Commit-Queue: Charlie Andrews <charliea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500981}

tools/battor_agent/battor_agent.cc

@@ -121,9 +121,8 @@ BattOrAgent::BattOrAgent(
     const std::string& path,
     Listener* listener,
     scoped_refptr<base::SingleThreadTaskRunner> ui_thread_task_runner)
-    : connection_(new BattOrConnectionImpl(path,
-                                           this,
-                                           ui_thread_task_runner)),
+    : connection_(new BattOrConnectionImpl(path, this, ui_thread_task_runner)),
+      tick_clock_(std::make_unique<base::DefaultTickClock>()),
       listener_(listener),
       last_action_(Action::INVALID),
       command_(Command::INVALID),
@@ -315,8 +314,9 @@ void BattOrAgent::OnMessageRead(bool success,
       base::TimeTicks min_request_samples_time =
           last_clock_sync_time_ + base::TimeDelta::FromMilliseconds(
                                       kStopTracingClockSyncDelayMilliseconds);
-      base::TimeDelta request_samples_delay = std::max(
-          min_request_samples_time - base::TimeTicks::Now(), base::TimeDelta());
+      base::TimeDelta request_samples_delay =
+          std::max(min_request_samples_time - tick_clock_->NowTicks(),
+                   base::TimeDelta());
 
       PerformDelayedAction(Action::SEND_SAMPLES_REQUEST, request_samples_delay);
       return;
@@ -373,7 +373,7 @@ void BattOrAgent::OnMessageRead(bool success,
       uint32_t sample_num;
       memcpy(&sample_num, bytes->data(), sizeof(uint32_t));
       clock_sync_markers_[sample_num] = pending_clock_sync_marker_;
-      last_clock_sync_time_ = base::TimeTicks::Now();
+      last_clock_sync_time_ = tick_clock_->NowTicks();
       CompleteCommand(BATTOR_ERROR_NONE);
       return;
 

tools/battor_agent/battor_agent.h

@@ -14,6 +14,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_checker.h"
+#include "base/time/default_tick_clock.h"
 #include "tools/battor_agent/battor_connection.h"
 #include "tools/battor_agent/battor_error.h"
 
@@ -100,6 +101,9 @@ class BattOrAgent : public BattOrConnection::Listener,
   // fake in testing.
   std::unique_ptr<BattOrConnection> connection_;
 
+  // A source of TimeTicks. Protected so that it can be faked in testing.
+  std::unique_ptr<base::TickClock> tick_clock_;
+
   // Timeout for when an action isn't completed within the allotted time. This
   // is virtual and protected so that timeouts can be disabled in testing. The
   // testing task runner that runs delayed tasks immediately deals poorly with

tools/battor_agent/battor_agent_unittest.cc

@@ -6,7 +6,7 @@
 
 #include "tools/battor_agent/battor_agent.h"
 
-#include "base/test/test_simple_task_runner.h"
+#include "base/test/test_mock_time_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -81,10 +81,12 @@ class MockBattOrConnection : public BattOrConnection {
 // TestableBattOrAgent uses a fake BattOrConnection to be testable.
 class TestableBattOrAgent : public BattOrAgent {
  public:
-  TestableBattOrAgent(BattOrAgent::Listener* listener)
+  TestableBattOrAgent(BattOrAgent::Listener* listener,
+                      std::unique_ptr<base::TickClock> tick_clock)
       : BattOrAgent("/dev/test", listener, nullptr) {
     connection_ =
         std::unique_ptr<BattOrConnection>(new MockBattOrConnection(this));
+    tick_clock_ = std::move(tick_clock);
   }
 
   MockBattOrConnection* GetConnection() {
@@ -99,7 +101,7 @@ class TestableBattOrAgent : public BattOrAgent {
 class BattOrAgentTest : public testing::Test, public BattOrAgent::Listener {
  public:
   BattOrAgentTest()
-      : task_runner_(new base::TestSimpleTaskRunner()),
+      : task_runner_(new base::TestMockTimeTaskRunner()),
         thread_task_runner_handle_(task_runner_) {}
 
   void OnStartTracingComplete(BattOrError error) override {
@@ -140,7 +142,8 @@ class BattOrAgentTest : public testing::Test, public BattOrAgent::Listener {
 
  protected:
   void SetUp() override {
-    agent_.reset(new TestableBattOrAgent(this));
+    agent_.reset(
+        new TestableBattOrAgent(this, task_runner_->GetMockTickClock()));
     task_runner_->ClearPendingTasks();
     is_command_complete_ = false;
     command_error_ = BATTOR_ERROR_NONE;
@@ -244,6 +247,7 @@ class BattOrAgentTest : public testing::Test, public BattOrAgent::Listener {
     if (end_state == BattOrAgentState::EEPROM_RECEIVED)
       return;
 
+    GetTaskRunner()->FastForwardBy(base::TimeDelta::FromMilliseconds(100));
     OnBytesSent(true);
     if (end_state == BattOrAgentState::SAMPLES_REQUEST_SENT)
       return;
@@ -310,7 +314,7 @@ class BattOrAgentTest : public testing::Test, public BattOrAgent::Listener {
 
   TestableBattOrAgent* GetAgent() { return agent_.get(); }
 
-  scoped_refptr<base::TestSimpleTaskRunner> GetTaskRunner() {
+  scoped_refptr<base::TestMockTimeTaskRunner> GetTaskRunner() {
     return task_runner_;
   }
 
@@ -320,7 +324,7 @@ class BattOrAgentTest : public testing::Test, public BattOrAgent::Listener {
   std::string GetGitHash() { return firmware_git_hash_; }
 
  private:
-  scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
+  scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;
   // Needed to support ThreadTaskRunnerHandle::Get() in code under test.
   base::ThreadTaskRunnerHandle thread_task_runner_handle_;
 
@@ -948,6 +952,7 @@ TEST_F(BattOrAgentTest, RecordClockSyncMarkerPrintsInStopTracingResult) {
   EXPECT_TRUE(IsCommandComplete());
   EXPECT_EQ(BATTOR_ERROR_NONE, GetCommandError());
 
+  GetTaskRunner()->FastForwardBy(base::TimeDelta::FromMilliseconds(100));
   GetAgent()->StopTracing();
   RunStopTracingTo(BattOrAgentState::SAMPLES_REQUEST_SENT);