Restart of ceee_broker on crash.

ceee/ie/broker/broker_rpc_unittest.cc

Index: ceee/ie/broker/broker_rpc_unittest.cc
===================================================================
--- ceee/ie/broker/broker_rpc_unittest.cc	(revision 67655)
+++ ceee/ie/broker/broker_rpc_unittest.cc	(working copy)
@@ -24,14 +24,14 @@
 
 MOCK_STATIC_CLASS_BEGIN(BrokerRpcMock)
   MOCK_STATIC_INIT_BEGIN(BrokerRpcMock)
-    MOCK_STATIC_INIT(GetRpcEndPointAddress);
+    MOCK_STATIC_INIT(GetRpcEndpointAddress);
     MOCK_STATIC_INIT(BrokerRpcServer_FireEvent);
     MOCK_STATIC_INIT(BrokerRpcServer_SendUmaHistogramTimes);
     MOCK_STATIC_INIT(BrokerRpcServer_SendUmaHistogramData);
   MOCK_STATIC_INIT_END()
-  MOCK_STATIC0(std::wstring, , GetRpcEndPointAddress);
-  MOCK_STATIC3(void, , BrokerRpcServer_FireEvent, handle_t, const char*,
-               const char*);
+  MOCK_STATIC0(std::wstring, , GetRpcEndpointAddress);
+  MOCK_STATIC4(void, , BrokerRpcServer_FireEvent, handle_t, BrokerContextHandle,
+               const char*, const char*);
   MOCK_STATIC3(void, , BrokerRpcServer_SendUmaHistogramTimes, handle_t,
                const char*, int);
   MOCK_STATIC6(void, , BrokerRpcServer_SendUmaHistogramData, handle_t,
@@ -41,7 +41,7 @@
 class BrokerRpcTest : public testing::Test {
  protected:
   virtual void SetUp() {
-    EXPECT_CALL(broker_rpc_mock_, GetRpcEndPointAddress())
+    EXPECT_CALL(broker_rpc_mock_, GetRpcEndpointAddress())
         .WillRepeatedly(Return(L"BrokerRpcTestEP"));
   }
 
@@ -52,7 +52,7 @@
 };
 
 TEST_F(BrokerRpcTest, ConnectNoServer) {
-  BrokerRpcClient client;
+  BrokerRpcClient client(false);
   ASSERT_FALSE(client.is_connected());
   ASSERT_HRESULT_FAILED(client.Connect(false));
   ASSERT_FALSE(client.is_connected());
@@ -63,7 +63,7 @@
   ASSERT_FALSE(server.is_started());
   ASSERT_TRUE(server.Start());
   ASSERT_TRUE(server.is_started());
-  BrokerRpcClient client;
+  BrokerRpcClient client(false);
   ASSERT_HRESULT_SUCCEEDED(client.Connect(false));
   ASSERT_TRUE(client.is_connected());
 }
@@ -72,14 +72,14 @@
   BrokerRpcServer server;
   ASSERT_TRUE(server.Start());
 
-  BrokerRpcClient client;
+  BrokerRpcClient client(false);
   ASSERT_HRESULT_SUCCEEDED(client.Connect(false));
 
   const char* name = "name";
   const char* args = "args";
 
   EXPECT_CALL(broker_rpc_mock_,
-      BrokerRpcServer_FireEvent(_, StrEq(name), StrEq(args)))
+      BrokerRpcServer_FireEvent(_, _, StrEq(name), StrEq(args)))
           .Times(1);
 
   ASSERT_HRESULT_SUCCEEDED(client.FireEvent(name, args));
@@ -95,4 +95,69 @@
   ASSERT_HRESULT_SUCCEEDED(client.SendUmaHistogramData(name, 1, 2, 3, 4));
 }
 
+class BrokerRpcClientMock : public BrokerRpcClient {

[Sigurður Ásgeirsson, 2010/12/01 14:23:54]

Classes that subclass and override implementation for testing are commonly named
TestingXXX, e.g. TestingBrokerRpcClient.

[Vitaly Buka corp, 2010/12/01 19:06:04]

Done.

+ public:
+  explicit BrokerRpcClientMock(bool allow_restarts)
+      : BrokerRpcClient(allow_restarts) {}
+  MOCK_METHOD0(StartServerMock, HRESULT());
+ protected:
+  HRESULT StartServer(IUnknown** server) {

[Sigurður Ásgeirsson, 2010/12/01 14:23:54]

I would suggest mocking this method directly, and adding a  member or two on
this class named 

ExpectStartServer(...)

to set up the various expectations you want. Note you can use testing::Invoke to
invoke server_.Start() where appropriate.

[Vitaly Buka corp, 2010/12/01 19:06:04]

Done without ExpectStartServer. Putting EXPECT_CALL into method is really bad.
EXPECT_CALL provides location where test fails. If we hide it in function
location will be useless.

On 2010/12/01 14:23:54, Ruðrugis wrote:

[Sigurður Ásgeirsson, 2010/12/01 19:29:38]

It's a tradeoff between maintenance and usability, and as unittest failures
should be rare. It's not that bad if you need to hunt down the call stack to the
expectation that failed, and you can mitigate the problem by breaking up the
tests into multiple test cases if needed.
 We do this liberally in other tests.

[Vitaly Buka corp, 2010/12/01 20:50:30]

I know that this is tradeoff. But I prefer to introduce new local macro than
hiding code in function. This code is not so complex that we have to give up on
usability.
Do you insist on ExpectStartServer?
If you do I will change and not argue any more.

On 2010/12/01 19:29:38, Ruðrugis wrote:

[Sigurður Ásgeirsson, 2010/12/01 20:52:46]

No, this is ok as-is.

+    server_.Start();
+    return StartServerMock();
+  }
+
+ public:
+  BrokerRpcServer server_;
+};
+
+TEST_F(BrokerRpcTest, StartServer) {
+  BrokerRpcClientMock client(true);
+  EXPECT_CALL(client, StartServerMock())
+      .Times(0);
+  ASSERT_HRESULT_FAILED(client.Connect(false));
+
+  EXPECT_CALL(client, StartServerMock())
+      .Times(1)
+      .WillOnce(Return(RPC_E_FAULT));
+  ASSERT_HRESULT_FAILED(client.Connect(true));
+
+  EXPECT_CALL(client, StartServerMock())
+      .Times(1)
+      .WillOnce(Return(S_OK));
+  ASSERT_HRESULT_SUCCEEDED(client.Connect(true));
+}
+
+TEST_F(BrokerRpcTest, NoRestartServer) {
+  BrokerRpcClientMock client(false);
+  EXPECT_CALL(client, StartServerMock())
+      .Times(1)
+      .WillOnce(Return(S_OK));
+  ASSERT_HRESULT_SUCCEEDED(client.Connect(true));
+  client.server_.Stop();
+
+  EXPECT_CALL(client, StartServerMock())
+      .Times(0);
+  EXPECT_CALL(broker_rpc_mock_,
+      BrokerRpcServer_FireEvent(_, _, _, _))
+          .Times(0);
+  ASSERT_HRESULT_FAILED(client.FireEvent("A", "B"));
+}
+
+TEST_F(BrokerRpcTest, RestartServer) {

[Sigurður Ásgeirsson, 2010/12/01 14:23:54]

nice set of tests.

[Vitaly Buka corp, 2010/12/01 19:06:04]

Done.

+  BrokerRpcClientMock client(true);
+  EXPECT_CALL(client, StartServerMock())
+      .Times(1)
+      .WillOnce(Return(S_OK));
+  ASSERT_HRESULT_SUCCEEDED(client.Connect(true));
+  client.server_.Stop();
+
+  EXPECT_CALL(client, StartServerMock())
+      .Times(1)
+      .WillOnce(Return(S_OK));
+  EXPECT_CALL(broker_rpc_mock_,
+      BrokerRpcServer_FireEvent(_, _, _, _))
+          .Times(1);
+  ASSERT_HRESULT_SUCCEEDED(client.FireEvent("A", "B"));
+}
+
 }  // namespace