Remote assistance on Chrome OS Part IV - It2MeHost

remoting/host/policy_hack/policy_watcher_unittest.cc

Index: remoting/host/policy_hack/policy_watcher_unittest.cc
diff --git a/remoting/host/policy_hack/policy_watcher_unittest.cc b/remoting/host/policy_hack/policy_watcher_unittest.cc
index b19d92f4a3bd224e08a71894d4ed6273a88b42fe..7c5e3e3a230af81d37f26e37bb28a3510b061198 100644
--- a/remoting/host/policy_hack/policy_watcher_unittest.cc
+++ b/remoting/host/policy_hack/policy_watcher_unittest.cc
@@ -99,12 +99,14 @@ class PolicyWatcherTest : public testing::Test {
   }
 
   void StopWatching() {
-    base::WaitableEvent stop_event(false, false);
-    policy_watcher_->StopWatching(&stop_event);
+    EXPECT_CALL(*this, PostPolicyWatcherShutdown()).Times(1);

[Wez, 2014/10/17 17:58:01]

You shouldn't mix call expectations in with actually running the code; they need
to be set up before you start running code. In this case you will already have
called e.g. StartWatching()so the policy watcher will be active at the point
where you're adding this now expectation into the mix.

[kelvinp, 2014/10/20 00:21:18]

I felt the same way when I saw the EXPECT_EQ in the original code as well.  But
then I realized that StopWatching is manually called in the test case.  The test
here is trying to verify that whenever StopWatching is called, the appropriate
callback is invoked (or stop_event is signaled before this change) and not so
much about whether StopWatching is being called.  I am going to leave it as it
is.

+    policy_watcher_->StopWatching(base::Bind(
+        &PolicyWatcherTest::PostPolicyWatcherShutdown, base::Unretained(this)));
     base::RunLoop().RunUntilIdle();
-    EXPECT_EQ(true, stop_event.IsSignaled());
   }
 
+  MOCK_METHOD0(PostPolicyWatcherShutdown, void());
+
   static const char* kHostDomain;
   static const char* kPortRange;
   base::MessageLoop message_loop_;