[5318] Improved the tests for long commands/responses.

src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc

@@ -48,6 +48,32 @@ using namespace isc::test;
 
 namespace {
 
+/// @brief Simple RAII class which stops IO service upon destruction
+/// of the object.
+class IOServiceWork {
+public:
+
+    /// @brief Constructor.
+    ///
+    /// @param io_service Pointer to the IO service to be stopped.
+    IOServiceWork(const IOServicePtr& io_service)
+        : io_service_(io_service) {
+    }
+
+    /// @brief Destructor.
+    ///
+    /// Stops IO service.
+    ~IOServiceWork() {
+        io_service_->stop();
+    }
+
+private:
+
+    /// @brief Pointer to the IO service to be stopped upon destruction.
+    IOServicePtr io_service_;
+
+};
+
 class NakedControlledDhcpv4Srv: public ControlledDhcpv4Srv {
     // "Naked" DHCPv4 server, exposes internal fields
 public:
@@ -1123,55 +1149,97 @@ TEST_F(CtrlChannelDhcpv4SrvTest, concurrentConnections) {
     ASSERT_NO_THROW(getIOService()->poll());
 }
 
+// This test verifies that the server can receive and process a large command.
 TEST_F(CtrlChannelDhcpv4SrvTest, longCommand) {
     createUnixChannelServer();
 
-    boost::scoped_ptr<UnixControlClient> client(new UnixControlClient());
-    ASSERT_TRUE(client);
+    std::string response;
+    std::thread th([this, &response]() {
 
-    ASSERT_TRUE(client->connectToServer(socket_path_));
-    getIOService()->run_one();
-    getIOService()->poll();
+        // IO service will be stopped automatically when this object goes
+        // out of scope and is destroyed. This is useful because we use
+        // asserts which may break the thread in various exit points.
+        IOServiceWork work(getIOService());
 
-    size_t bytes_transferred = 0;
-    const size_t payload_size = 1024 * 1000;
-    bool first_payload = true;
-    while (bytes_transferred < payload_size) {
-        if (bytes_transferred == 0) {
-            std::string preamble = "{ \"command\": \"foo\", \"arguments\": [ ";
-            ASSERT_TRUE(client->sendCommand(preamble));
-            bytes_transferred += preamble.size();
+        // Create client which we will use to send command to the server.
+        boost::scoped_ptr<UnixControlClient> client(new UnixControlClient());
+        ASSERT_TRUE(client);
 
-        } else {
-            std::ostringstream payload;
-            if (!first_payload) {
-                payload << ", ";
-            }
-            first_payload = false;
-            payload << "\"blablablablablablablablablablablablablablablabla\"";
+        // Connect to the server. This will trigger acceptor handler on the
+        // server side and create a new connection.
+        ASSERT_TRUE(client->connectToServer(socket_path_));
 
-            if (bytes_transferred + payload.tellp() > payload_size) {
-                payload << "] }";
+        // This counter will hold the number of bytes transferred to the server
+        // so far.
+        size_t bytes_transferred = 0;
+        // This is the desired size of the command sent to the server (1MB). The
+        // actual size sent will be slightly greater than that.
+        const size_t command_size = 1024 * 1000;
+        bool first_payload = true;
+
+        // If we still haven't sent the entire command, continue sending.
+        while (bytes_transferred < command_size) {
+
+            // We're sending command 'foo' with arguments being a list of
+            // strings. If this is the first transmission, send command name
+            // and open the arguments list.
+            if (bytes_transferred == 0) {
+                std::string preamble = "{ \"command\": \"foo\", \"arguments\": [ ";
+                ASSERT_TRUE(client->sendCommand(preamble));
+                // Store the number of bytes sent.
+                bytes_transferred += preamble.size();
+
+            } else {
+                // We have already transmitted command name and arguments. Now
+                // we send the list of 'blabla' strings.
+                std::ostringstream payload;
+                // If this is not the first parameter in on the list it must be
+                // prefixed with a comma.
+                if (!first_payload) {
+                    payload << ", ";
+                }
+
+                first_payload = false;
+                payload << "\"blablablablablablablablablablablablablablablabla\"";
+
+                // If we have hit the limit of the command size, close braces to
+                // get appropriate JSON.
+                if (bytes_transferred + payload.tellp() > command_size) {
+                    payload << "] }";
+                }
+                // Send the payload.
+                ASSERT_TRUE(client->sendCommand(payload.str()));
+                // Update the number of bytes sent.
+                bytes_transferred += payload.tellp();
             }
-            ASSERT_TRUE(client->sendCommand(payload.str()));
 
-            bytes_transferred += payload.tellp();
         }
 
-        getIOService()->run_one();
-        getIOService()->poll();
-    }
+        // Set timeout to 5 seconds to allow the time for the server to send
+        // a response.
+        const unsigned int timeout = 5;
+        ASSERT_TRUE(client->getResponse(response, timeout));
 
-    std::string response;
-    ASSERT_TRUE(client->getResponse(response));
+        // We're done. Close the connection to the server.
+        client->disconnectFromServer();
+    });
+
+    // Run the server until the command has been processed and response
+    // received.
+    getIOService()->run();
+
+    // Wait for the thread to complete.
+    th.join();
 
     EXPECT_EQ("{ \"result\": 2, \"text\": \"'foo' command not supported.\" }",
               response);
-
-    client->disconnectFromServer();
 }
 
+// This test verifies that the server can send long response to the client.
 TEST_F(CtrlChannelDhcpv4SrvTest, longResponse) {
+    // We need to generate large response. The simplest way is to create
+    // a command and a handler which will generate some static response
+    // of a desired size.
     ASSERT_NO_THROW(
         CommandMgr::instance().registerCommand("foo",
              boost::bind(&CtrlChannelDhcpv4SrvTest::longResponseHandler, _1, _2));
@@ -1179,35 +1247,57 @@ TEST_F(CtrlChannelDhcpv4SrvTest, longResponse) {
 
     createUnixChannelServer();
 
+    // The UnixControlClient doesn't have any means to check that the entire
+    // response has been received. What we want to do is to generate a
+    // reference response using our command handler and then compare
+    // what we have received over the unix domain socket with this reference
+    // response to figure out when to stop receiving.
     std::string reference_response = longResponseHandler("foo", ConstElementPtr())->str();
+
+    // In this stream we're going to collect out partial responses.
     std::ostringstream response;
+
+    // The client is synchronous so it is useful to run it in a thread.
     std::thread th([this, &response, reference_response]() {
 
-        size_t long_response_size = reference_response.size();
+        // IO service will be stopped automatically when this object goes
+        // out of scope and is destroyed. This is useful because we use
+        // asserts which may break the thread in various exit points.
+        IOServiceWork work(getIOService());
+
+        // Remember the response size so as we know when we should stop
+        // receiving.
+        const size_t long_response_size = reference_response.size();
 
+        // Create the client and connect it to the server.
         boost::scoped_ptr<UnixControlClient> client(new UnixControlClient());
         ASSERT_TRUE(client);
-
         ASSERT_TRUE(client->connectToServer(socket_path_));
 
+        // Send the stub command.
         std::string command = "{ \"command\": \"foo\", \"arguments\": { }  }";
         ASSERT_TRUE(client->sendCommand(command));
 
+        // Keep receiving response data until we have received the full answer.
         while (response.tellp() < long_response_size) {
             std::string partial;
-            client->getResponse(partial);
+            const unsigned int timeout = 5;
+            ASSERT_TRUE(client->getResponse(partial, 5));
             response << partial;
         }
 
+        // We have received the entire response, so close the connection and
+        // stop the IO service.
         client->disconnectFromServer();
-
-        getIOService()->stop();
     });
 
+    // Run the server until the entire response has been received.
     getIOService()->run();
 
+    // Wait for the thread to complete.
     th.join();
 
+    // Make sure we have received correct response.
     EXPECT_EQ(reference_response, response.str());
 }
 

src/lib/testutils/unix_control_client.cc

@@ -84,11 +84,12 @@ bool UnixControlClient::sendCommand(const std::string& command) {
     return (true);
 }
 
-bool UnixControlClient::getResponse(std::string& response) {
+bool UnixControlClient::getResponse(std::string& response,
+                                    const unsigned int timeout_sec) {
     // Receive response
     char buf[65536];
     memset(buf, 0, sizeof(buf));
-    switch (selectCheck()) {
+    switch (selectCheck(timeout_sec)) {
     case -1: {
         const char* errmsg = strerror(errno);
         ADD_FAILURE() << "getResponse - select failed: " << errmsg;
@@ -119,7 +120,7 @@ bool UnixControlClient::getResponse(std::string& response) {
     return (true);
 }
 
-int UnixControlClient::selectCheck() {
+int UnixControlClient::selectCheck(const unsigned int timeout_sec) {
     int maxfd = 0;
 
     fd_set read_fds;
@@ -130,7 +131,7 @@ int UnixControlClient::selectCheck() {
     maxfd = socket_fd_;
 
     struct timeval select_timeout;
-    select_timeout.tv_sec = 0;
+    select_timeout.tv_sec = static_cast<time_t>(timeout_sec);
     select_timeout.tv_usec = 0;
 
     return (select(maxfd + 1, &read_fds, NULL, NULL, &select_timeout));

src/lib/testutils/unix_control_client.h

@@ -44,13 +44,16 @@ public:
     /// @brief Reads the response text from the open Control Channel
     /// @param response variable into which the received response should be
     /// placed.
+    /// @param timeout_sec Timeout for receiving response in seconds.
     /// @return true if data was successfully read from the socket,
     /// false otherwise
-    bool getResponse(std::string& response);
+    bool getResponse(std::string& response, const unsigned int timeout_sec = 0);
 
     /// @brief Uses select to poll the Control Channel for data waiting
+    ///
+    /// @param timeout_sec Select timeout in seconds
     /// @return -1 on error, 0 if no data is available,  1 if data is ready
-    int selectCheck();
+    int selectCheck(const unsigned int timeout_sec);
 
     /// @brief Retains the fd of the open socket
     int socket_fd_;