tool: continue the unit-tests of the host tool

Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: I51349fc103af2a55f1b83830bf8e74407fc15b9e
diff --git a/tools/test/tools_updater_unittest.cpp b/tools/test/tools_updater_unittest.cpp
index cacf2b0..e54e613 100644
--- a/tools/test/tools_updater_unittest.cpp
+++ b/tools/test/tools_updater_unittest.cpp
@@ -1,4 +1,5 @@
 #include "data_interface_mock.hpp"
+#include "status.hpp"
 #include "updater.hpp"
 #include "updater_mock.hpp"
 #include "util.hpp"
@@ -15,19 +16,23 @@
 using ::testing::_;
 using ::testing::Eq;
 using ::testing::Return;
-using ::testing::StrEq;
 using ::testing::TypedEq;
 
-TEST(UpdaterTest, CheckAvailableSuccess)
+class UpdateHandlerTest : public ::testing::Test
 {
-    /* Call checkAvailable directly() to make sure it works. */
+  protected:
+    const std::uint16_t session = 0xbeef;
+
     DataInterfaceMock handlerMock;
     ipmiblob::BlobInterfaceMock blobMock;
+    UpdateHandler updater{&blobMock, &handlerMock};
+};
 
-    ipmiblob::StatResponse statObj;
+TEST_F(UpdateHandlerTest, CheckAvailableSuccess)
+{
+    ipmiblob::StatResponse statObj = {};
     statObj.blob_state = ipmi_flash::FirmwareBlobHandler::UpdateFlags::ipmi |
                          ipmi_flash::FirmwareBlobHandler::UpdateFlags::lpc;
-    statObj.size = 0;
 
     EXPECT_CALL(blobMock, getBlobList())
         .WillOnce(
@@ -39,104 +44,98 @@
     EXPECT_CALL(handlerMock, supportedType())
         .WillOnce(Return(ipmi_flash::FirmwareBlobHandler::UpdateFlags::lpc));
 
-    UpdateHandler updater(&blobMock, &handlerMock);
     EXPECT_TRUE(updater.checkAvailable(ipmi_flash::staticLayoutBlobId));
 }
 
-TEST(UpdaterTest, SendFileSuccess)
+TEST_F(UpdateHandlerTest, SendFileSuccess)
 {
     /* Call sendFile to verify it does what we expect. */
-    DataInterfaceMock handlerMock;
-    ipmiblob::BlobInterfaceMock blobMock;
-
     std::string firmwareImage = "image.bin";
 
     std::uint16_t supported =
         static_cast<std::uint16_t>(
             ipmi_flash::FirmwareBlobHandler::UpdateFlags::lpc) |
         static_cast<std::uint16_t>(blobs::OpenFlags::write);
-    std::uint16_t session = 0xbeef;
 
     EXPECT_CALL(handlerMock, supportedType())
         .WillOnce(Return(ipmi_flash::FirmwareBlobHandler::UpdateFlags::lpc));
 
-    EXPECT_CALL(
-        blobMock,
-        openBlob(StrEq(ipmi_flash::staticLayoutBlobId.c_str()), supported))
+    EXPECT_CALL(blobMock, openBlob(ipmi_flash::staticLayoutBlobId, supported))
         .WillOnce(Return(session));
 
-    EXPECT_CALL(handlerMock,
-                sendContents(StrEq(firmwareImage.c_str()), session))
+    EXPECT_CALL(handlerMock, sendContents(firmwareImage, session))
         .WillOnce(Return(true));
 
     EXPECT_CALL(blobMock, closeBlob(session)).Times(1);
 
-    UpdateHandler updater(&blobMock, &handlerMock);
     updater.sendFile(ipmi_flash::staticLayoutBlobId, firmwareImage);
 }
 
-#if 0 /* TODO: fix this up. */
-TEST(UpdaterTest, NormalWalkthroughAllHappy)
+TEST_F(UpdateHandlerTest, VerifyFileHandleReturnsTrueOnSuccess)
 {
-    /* Call updaterMain and have everything respond happily. */
-    DataInterfaceMock handlerMock;
-    ipmiblob::BlobInterfaceMock blobMock;
-
-    UpdateHandlerMock updaterMock;
-
-    std::string firmwareImage = "image.bin";
-    std::string signatureFile = "image.sig";
-
-    std::vector<std::string> blobList = {ipmi_flash::staticLayoutBlobId};
-    ipmiblob::StatResponse statObj;
-    statObj.blob_state = ipmi_flash::FirmwareBlobHandler::UpdateFlags::ipmi |
-                         ipmi_flash::FirmwareBlobHandler::UpdateFlags::lpc;
-    statObj.size = 0;
-    std::uint16_t supported =
-        static_cast<std::uint16_t>(
-            ipmi_flash::FirmwareBlobHandler::UpdateFlags::lpc) |
-        static_cast<std::uint16_t>(blobs::OpenFlags::write);
-    std::uint16_t session = 0xbeef;
-
-    EXPECT_CALL(blobMock, getBlobList()).WillOnce(Return(blobList));
-
-    EXPECT_CALL(blobMock, getStat(TypedEq<const std::string&>(ipmi_flash::staticLayoutBlobId)))
-        .WillOnce(Return(statObj));
-
-    EXPECT_CALL(handlerMock, supportedType())
-        .WillOnce(Return(ipmi_flash::FirmwareBlobHandler::UpdateFlags::lpc));
-
-    EXPECT_CALL(blobMock, openBlob(StrEq(ipmi_flash::staticLayoutBlobId.c_str()), Eq(supported)))
+    EXPECT_CALL(blobMock, openBlob(ipmi_flash::verifyBlobId, _))
         .WillOnce(Return(session));
-
-    EXPECT_CALL(handlerMock,
-                sendContents(StrEq(firmwareImage.c_str()), Eq(session)))
-        .WillOnce(Return(true));
-
-    EXPECT_CALL(blobMock, openBlob(StrEq(blobs::hashBlobId.c_str()), Eq(supported)))
-        .WillOnce(Return(session));
-
-    EXPECT_CALL(handlerMock,
-                sendContents(StrEq(signatureFile.c_str()), Eq(session)))
-        .WillOnce(Return(true));
-
-    EXPECT_CALL(blobMock,
-                openBlob(StrEq(blobs::verifyBlobId.c_str()), Eq(supported)))
-        .WillOnce(Return(session));
-
     EXPECT_CALL(blobMock, commit(session, _)).WillOnce(Return());
+    ipmiblob::StatResponse verificationResponse = {};
+    /* the other details of the response are ignored, and should be. */
+    verificationResponse.metadata.push_back(
+        static_cast<std::uint8_t>(ipmi_flash::ActionStatus::success));
 
-    ipmiblob::StatResponse verificationResponse;
-    verificationResponse.blob_state = supported | blobs::StateFlags::committing;
-    verificationResponse.size = 0;
-    verificationResponse.metadata.push_back(static_cast<std::uint8_t>(
-        ipmi_flash::FirmwareBlobHandler::ActionStatus::success));
+    EXPECT_CALL(blobMock, getStat(TypedEq<std::uint16_t>(session)))
+        .WillOnce(Return(verificationResponse));
+    EXPECT_CALL(blobMock, closeBlob(session)).WillOnce(Return());
+
+    EXPECT_TRUE(updater.verifyFile(ipmi_flash::verifyBlobId));
+}
+
+class UpdaterTest : public ::testing::Test
+{
+  protected:
+    ipmiblob::BlobInterfaceMock blobMock;
+    std::uint16_t session = 0xbeef;
+};
+
+TEST_F(UpdaterTest, PollStatusReturnsAfterSuccess)
+{
+    ipmiblob::StatResponse verificationResponse = {};
+    /* the other details of the response are ignored, and should be. */
+    verificationResponse.metadata.push_back(
+        static_cast<std::uint8_t>(ipmi_flash::ActionStatus::success));
 
     EXPECT_CALL(blobMock, getStat(TypedEq<std::uint16_t>(session)))
         .WillOnce(Return(verificationResponse));
 
-    updaterMain(&blobMock, &handlerMock, firmwareImage, signatureFile);
+    EXPECT_TRUE(pollStatus(session, &blobMock));
 }
-#endif
+
+TEST_F(UpdaterTest, PollStatusReturnsAfterFailure)
+{
+    ipmiblob::StatResponse verificationResponse = {};
+    /* the other details of the response are ignored, and should be. */
+    verificationResponse.metadata.push_back(
+        static_cast<std::uint8_t>(ipmi_flash::ActionStatus::failed));
+
+    EXPECT_CALL(blobMock, getStat(TypedEq<std::uint16_t>(session)))
+        .WillOnce(Return(verificationResponse));
+
+    EXPECT_FALSE(pollStatus(session, &blobMock));
+}
+
+TEST_F(UpdaterTest, UpdateMainReturnsSuccessIfAllSuccess)
+{
+    const std::string image = "image.bin";
+    const std::string signature = "signature.bin";
+    UpdateHandlerMock handler;
+
+    EXPECT_CALL(handler, checkAvailable(_)).WillOnce(Return(true));
+    EXPECT_CALL(handler, sendFile(_, image)).WillOnce(Return());
+    EXPECT_CALL(handler, sendFile(_, signature)).WillOnce(Return());
+    EXPECT_CALL(handler, verifyFile(ipmi_flash::verifyBlobId))
+        .WillOnce(Return(true));
+    EXPECT_CALL(handler, verifyFile(ipmi_flash::updateBlobId))
+        .WillOnce(Return(true));
+
+    updaterMain(&handler, image, signature);
+}
 
 } // namespace host_tool
diff --git a/tools/test/updater_mock.hpp b/tools/test/updater_mock.hpp
index d065219..ca4c4b9 100644
--- a/tools/test/updater_mock.hpp
+++ b/tools/test/updater_mock.hpp
@@ -9,7 +9,7 @@
 namespace host_tool
 {
 
-class UpdateHandlerMock : public UpdateHandler
+class UpdateHandlerMock : public UpdateHandlerInterface
 {
   public:
     MOCK_METHOD1(checkAvailable, bool(const std::string&));
diff --git a/tools/updater.cpp b/tools/updater.cpp
index ca9dadb..a5007b2 100644
--- a/tools/updater.cpp
+++ b/tools/updater.cpp
@@ -107,7 +107,8 @@
 }
 
 /* Poll an open verification session.  Handling closing the session is not yet
- * owned by this method. */
+ * owned by this method.
+ */
 bool pollStatus(std::uint16_t session, ipmiblob::BlobInterface* blob)
 {
     using namespace std::chrono_literals;
@@ -230,7 +231,7 @@
     return (success == true);
 }
 
-void updaterMain(UpdateHandler* updater, const std::string& imagePath,
+void updaterMain(UpdateHandlerInterface* updater, const std::string& imagePath,
                  const std::string& signaturePath)
 {
     /* TODO(venture): Add optional parameter to specify the flash type, default
diff --git a/tools/updater.hpp b/tools/updater.hpp
index ac4b11a..670d981 100644
--- a/tools/updater.hpp
+++ b/tools/updater.hpp
@@ -8,16 +8,10 @@
 namespace host_tool
 {
 
-/** Object that actually handles the update itself. */
-class UpdateHandler
+class UpdateHandlerInterface
 {
   public:
-    UpdateHandler(ipmiblob::BlobInterface* blob, DataInterface* handler) :
-        blob(blob), handler(handler)
-    {
-    }
-
-    virtual ~UpdateHandler() = default;
+    virtual ~UpdateHandlerInterface() = default;
 
     /**
      * Check if the goal firmware is listed in the blob_list and that the
@@ -26,16 +20,16 @@
      * @param[in] goalFirmware - the firmware to check /flash/image
      * /flash/tarball, etc.
      */
-    virtual bool checkAvailable(const std::string& goalFirmware);
+    virtual bool checkAvailable(const std::string& goalFirmware) = 0;
 
     /**
      * Send the file contents at path to the blob id, target.
      *
      * @param[in] target - the blob id
      * @param[in] path - the source file path
-     * @throw ToolException on failure.
      */
-    virtual void sendFile(const std::string& target, const std::string& path);
+    virtual void sendFile(const std::string& target,
+                          const std::string& path) = 0;
 
     /**
      * Trigger verification.
@@ -43,9 +37,32 @@
      * @param[in] target - the verification blob id (may support multiple in the
      * future.
      * @return true if verified, false if verification errors.
+     */
+    virtual bool verifyFile(const std::string& target) = 0;
+};
+
+/** Object that actually handles the update itself. */
+class UpdateHandler : public UpdateHandlerInterface
+{
+  public:
+    UpdateHandler(ipmiblob::BlobInterface* blob, DataInterface* handler) :
+        blob(blob), handler(handler)
+    {
+    }
+
+    ~UpdateHandler() = default;
+
+    bool checkAvailable(const std::string& goalFirmware) override;
+
+    /**
+     * @throw ToolException on failure.
+     */
+    void sendFile(const std::string& target, const std::string& path) override;
+
+    /**
      * @throw ToolException on failure (TODO: throw on timeout.)
      */
-    virtual bool verifyFile(const std::string& target);
+    bool verifyFile(const std::string& target) override;
 
   private:
     ipmiblob::BlobInterface* blob;
@@ -69,7 +86,7 @@
  * @param[in] signaturePath - the path to the signature file.
  * @throws ToolException on failures.
  */
-void updaterMain(UpdateHandler* updater, const std::string& imagePath,
+void updaterMain(UpdateHandlerInterface* updater, const std::string& imagePath,
                  const std::string& signaturePath);
 
 } // namespace host_tool