bmc: firmware updatePending: commit(session)

Commit(updateBlobId) will trigger the update.
Now you can only trigger if you're in the correct state, therefore
delete obsolete unit-tests that didn't set up the correct starting
condition.

Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: If97d1db21269027dc61c9cd295f022499e949106

5 files changed, 61 insertions, 47 deletions

diff --git a/firmware_handler.cpp b/firmware_handler.cpp
index 15dbee8..f648d42 100644
--- a/firmware_handler.cpp
+++ b/firmware_handler.cpp
@@ -572,14 +572,21 @@ bool FirmwareBlobHandler::commit(uint16_t session,
         return false;
     }
 
-    /* You can only commit on the verifyBlodId */
-    if (item->second->activePath != verifyBlobId)
+    /* You can only commit on the verifyBlodId or updateBlobId */
+    if (item->second->activePath != verifyBlobId &&
+        item->second->activePath != updateBlobId)
     {
+        std::fprintf(stderr, "path: '%s' not expected for commit\n",
+                     item->second->activePath.c_str());
         return false;
     }
 
     switch (state)
     {
+        case UpdateState::verificationPending:
+            /* Set state to committing. */
+            item->second->flags |= blobs::StateFlags::committing;
+            return triggerVerification();
         case UpdateState::verificationStarted:
             /* Calling repeatedly has no effect within an update process. */
             return true;
@@ -587,10 +594,11 @@ bool FirmwareBlobHandler::commit(uint16_t session,
             /* Calling after the verification process has completed returns
              * failure. */
             return false;
-        default:
-            /* Set state to committing. */
+        case UpdateState::updatePending:
             item->second->flags |= blobs::StateFlags::committing;
-            return triggerVerification();
+            return triggerUpdate();
+        default:
+            return false;
     }
 }
 
@@ -698,4 +706,15 @@ bool FirmwareBlobHandler::triggerVerification()
     return result;
 }
 
+bool FirmwareBlobHandler::triggerUpdate()
+{
+    bool result = update->triggerUpdate();
+    if (result)
+    {
+        state = UpdateState::updateStarted;
+    }
+
+    return result;
+}
+
 } // namespace ipmi_flash
diff --git a/firmware_handler.hpp b/firmware_handler.hpp
index bfae529..b2f5a64 100644
--- a/firmware_handler.hpp
+++ b/firmware_handler.hpp
@@ -169,6 +169,7 @@ class FirmwareBlobHandler : public blobs::GenericBlobInterface
     bool expire(uint16_t session) override;
 
     bool triggerVerification();
+    bool triggerUpdate();
 
     /** Allow grabbing the current state. */
     UpdateState getCurrentState() const
diff --git a/test/firmware_commit_unittest.cpp b/test/firmware_commit_unittest.cpp
index 87dba97..37e619e 100644
--- a/test/firmware_commit_unittest.cpp
+++ b/test/firmware_commit_unittest.cpp
@@ -83,43 +83,4 @@ TEST_F(FirmwareHandlerCommitTest, VerifyCannotCommitOnHashFile)
     EXPECT_FALSE(handler->commit(0, {}));
 }
 
-TEST_F(FirmwareHandlerCommitTest, VerifyCommitAcceptedOnVerifyBlob)
-{
-    /* Verify the verify blob lets you call this command, and it returns
-     * success.
-     */
-    auto verifyMock = CreateVerifyMock();
-    auto verifyMockPtr = reinterpret_cast<VerificationMock*>(verifyMock.get());
-
-    auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
-        blobs, data, std::move(verifyMock), CreateUpdateMock());
-
-    EXPECT_TRUE(handler->open(0, blobs::OpenFlags::write, verifyBlobId));
-
-    EXPECT_CALL(*verifyMockPtr, triggerVerification())
-        .WillRepeatedly(Return(true));
-
-    EXPECT_TRUE(handler->commit(0, {}));
-}
-
-TEST_F(FirmwareHandlerCommitTest, VerifyCommitCanOnlyBeCalledOnceForEffect)
-{
-    /* Verify you cannot call the commit() command once verification is
-     * started, after which it will just return true.
-     */
-    auto verifyMock = CreateVerifyMock();
-    auto verifyMockPtr = reinterpret_cast<VerificationMock*>(verifyMock.get());
-
-    auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
-        blobs, data, std::move(verifyMock), CreateUpdateMock());
-
-    EXPECT_TRUE(handler->open(0, blobs::OpenFlags::write, verifyBlobId));
-
-    EXPECT_CALL(*verifyMockPtr, triggerVerification())
-        .WillRepeatedly(Return(true));
-
-    EXPECT_TRUE(handler->commit(0, {}));
-    EXPECT_TRUE(handler->commit(0, {}));
-}
-
 } // namespace ipmi_flash
diff --git a/test/firmware_state_updatepending_unittest.cpp b/test/firmware_state_updatepending_unittest.cpp
index eb9e437..7a1e480 100644
--- a/test/firmware_state_updatepending_unittest.cpp
+++ b/test/firmware_state_updatepending_unittest.cpp
@@ -237,11 +237,39 @@ TEST_F(FirmwareHandlerUpdatePendingTest,
 }
 
 /*
- * TODO: deleteBlob(blob)
+ * commit(session)
  */
+TEST_F(FirmwareHandlerUpdatePendingTest,
+       CommitOnUpdateBlobTriggersUpdateAndChangesState)
+{
+    /* Commit triggers the update mechanism (similarly for the verifyBlobId) and
+     * changes state to updateStarted.
+     */
+    getToUpdatePending();
+    EXPECT_TRUE(handler->open(session, flags, updateBlobId));
+    expectedState(FirmwareBlobHandler::UpdateState::updatePending);
+
+    EXPECT_CALL(*updateMockPtr, triggerUpdate()).WillOnce(Return(true));
+
+    EXPECT_TRUE(handler->commit(session, {}));
+    expectedState(FirmwareBlobHandler::UpdateState::updateStarted);
+}
+
+TEST_F(FirmwareHandlerUpdatePendingTest,
+       CommitOnUpdateBlobTriggersUpdateAndReturnsFailureDoesNotChangeState)
+{
+    getToUpdatePending();
+    EXPECT_TRUE(handler->open(session, flags, updateBlobId));
+    expectedState(FirmwareBlobHandler::UpdateState::updatePending);
+
+    EXPECT_CALL(*updateMockPtr, triggerUpdate()).WillOnce(Return(false));
+
+    EXPECT_FALSE(handler->commit(session, {}));
+    expectedState(FirmwareBlobHandler::UpdateState::updatePending);
+}
 
 /*
- * commit(session)
+ * TODO: deleteBlob(blob)
  */
 
 } // namespace
diff --git a/test/firmware_unittest.hpp b/test/firmware_unittest.hpp
index 2427b82..0aa7e12 100644
--- a/test/firmware_unittest.hpp
+++ b/test/firmware_unittest.hpp
@@ -29,8 +29,12 @@ class IpmiOnlyFirmwareStaticTest : public ::testing::Test
             std::make_unique<VerificationMock>();
         verifyMockPtr = reinterpret_cast<VerificationMock*>(verifyMock.get());
 
+        std::unique_ptr<UpdateInterface> updateMock =
+            std::make_unique<UpdateMock>();
+        updateMockPtr = reinterpret_cast<UpdateMock*>(updateMock.get());
+
         handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
-            blobs, data, std::move(verifyMock), CreateUpdateMock());
+            blobs, data, std::move(verifyMock), std::move(updateMock));
     }
 
     void expectedState(FirmwareBlobHandler::UpdateState state)
@@ -45,6 +49,7 @@ class IpmiOnlyFirmwareStaticTest : public ::testing::Test
         {FirmwareBlobHandler::UpdateFlags::ipmi, nullptr}};
     std::unique_ptr<blobs::GenericBlobInterface> handler;
     VerificationMock* verifyMockPtr;
+    UpdateMock* updateMockPtr;
 };
 
 class IpmiOnlyFirmwareTest : public ::testing::Test