hillsp commented on a change in pull request #1945: Grpc RpcHandler: Fix finish before start behavior URL: https://github.com/apache/incubator-pagespeed-mod/pull/1945#discussion_r319294818
########## File path: pagespeed/controller/rpc_handler_test.cc ########## @@ -559,17 +559,16 @@ TEST_F(RpcHandlerTest, FinishAfterCompletion) { // Check nothing bad happens (leaks or stalls) if Finish is called before the // first call to HandleRequest. -// XXX(oschaaf): this one still fails. -TEST_F(RpcHandlerTest, DISABLED_FinishBeforeStart) { +TEST_F(RpcHandlerTest, FinishBeforeStart) { MockRpcHandler* handler = new MockRpcHandler(&service_, queue_.get()); - EXPECT_CALL(*handler, HandleRequest(_)).Times(0); + EXPECT_CALL(*handler, HandleRequest(_)).Times(1); Review comment: This doesn't look right. The RpcHandler subclass isn't expecting a call for this case. If it gets one, the Proto won't be populated so suddenly a whole bunch of other edge cases are introduced. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services