redfish-core tests: use matcher correctly
This commit corrects the usage of ASSERT, EXPECT, and all the matchers.
It also fixes cases where a cleaner matcher can be used.
This commit increases readability (with correct and cleaner matcher) and
corrects bugs (access iterator before checking validity).
Typical incorrect usage is that when a function returns a boolean value
to indicated whether the function succeeds or not, unless the function
has clear behavior when it fails, we shouldn't continue the test that
inspects the output parameters. A typical test codes look like this,
```
ASSERT_TRUE(fooBar(output));
EXPECT_EQ(output, 123);
```
Reference:
https://testing.googleblog.com/2008/07/tott-expect-vs-assert.html
https://github.com/google/googletest/blob/main/docs/reference/matchers.md
Tested: unit test passed.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Ia2cf9922bd4cb2fe8b4b3912e9153e9ae4eab134
diff --git a/redfish-core/ut/lock_test.cpp b/redfish-core/ut/lock_test.cpp
index 10c51d3..9c37da3 100644
--- a/redfish-core/ut/lock_test.cpp
+++ b/redfish-core/ut/lock_test.cpp
@@ -2,7 +2,8 @@
#include <string>
-#include "gmock/gmock.h"
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
namespace crow::ibm_mc_lock
{
@@ -22,6 +23,7 @@
using RcReleaseLockApi = std::pair<bool, std::variant<bool, RcRelaseLock>>;
using SessionFlags = std::pair<SType, SType>;
using ListOfSessionIds = std::vector<std::string>;
+using ::testing::IsEmpty;
class LockTest : public ::testing::Test
{
@@ -113,7 +115,7 @@
{
MockLock lockManager;
const LockRequest& t = record;
- ASSERT_EQ(1, lockManager.isValidLockRequest(t));
+ EXPECT_TRUE(lockManager.isValidLockRequest(t));
}
TEST_F(LockTest, ValidationBadTestWithLocktype)
@@ -122,7 +124,7 @@
// Corrupt the lock type
std::get<2>(record) = "rwrite";
const LockRequest& t = record;
- ASSERT_EQ(0, lockManager.isValidLockRequest(t));
+ EXPECT_FALSE(lockManager.isValidLockRequest(t));
}
TEST_F(LockTest, ValidationBadTestWithlockFlags)
@@ -131,7 +133,7 @@
// Corrupt the lockflag
std::get<4>(record)[0].first = "lock";
const LockRequest& t = record;
- ASSERT_EQ(0, lockManager.isValidLockRequest(t));
+ EXPECT_FALSE(lockManager.isValidLockRequest(t));
}
TEST_F(LockTest, ValidationBadTestWithSegmentlength)
@@ -140,14 +142,14 @@
// Corrupt the Segment length
std::get<4>(record)[0].second = 7;
const LockRequest& t = record;
- ASSERT_EQ(0, lockManager.isValidLockRequest(t));
+ EXPECT_FALSE(lockManager.isValidLockRequest(t));
}
TEST_F(LockTest, MultiRequestWithoutConflict)
{
MockLock lockManager;
const LockRequests& t = request;
- ASSERT_EQ(0, lockManager.isConflictRequest(t));
+ EXPECT_FALSE(lockManager.isConflictRequest(t));
}
TEST_F(LockTest, MultiRequestWithConflictduetoSameSegmentLength)
@@ -159,7 +161,7 @@
// resource
std::get<4>(request[0])[0].first = "LockAll";
const LockRequests& t = request;
- ASSERT_EQ(1, lockManager.isConflictRequest(t));
+ EXPECT_TRUE(lockManager.isConflictRequest(t));
}
TEST_F(LockTest, MultiRequestWithoutConflictduetoDifferentSegmentData)
@@ -177,7 +179,7 @@
std::get<3>(request[0]) = 216179379183550464; // HEX 03 00 06 00 00 00 00 00
std::get<3>(request[1]) = 288236973221478400; // HEX 04 00 06 00 00 00 00 00
const LockRequests& t = request;
- ASSERT_EQ(0, lockManager.isConflictRequest(t));
+ EXPECT_FALSE(lockManager.isConflictRequest(t));
}
TEST_F(LockTest, MultiRequestWithConflictduetoSameSegmentData)
@@ -189,12 +191,13 @@
// resource
std::get<4>(request[0])[0].first = "DontLock";
std::get<4>(request[0])[1].first = "LockAll";
- // Dont Change the resource id(1st & 2nd byte) at all, so that the conflict
- // occurs from the second segment which is trying to lock all the resources.
+ // Dont Change the resource id(1st & 2nd byte) at all, so that the
+ // conflict occurs from the second segment which is trying to lock all
+ // the resources.
std::get<3>(request[0]) = 216173882346831872; // 03 00 01 00 2B 00 00 00
std::get<3>(request[1]) = 216173882346831872; // 03 00 01 00 2B 00 00 00
const LockRequests& t = request;
- ASSERT_EQ(1, lockManager.isConflictRequest(t));
+ EXPECT_TRUE(lockManager.isConflictRequest(t));
}
TEST_F(LockTest, MultiRequestWithoutConflictduetoDifferentSegmentLength)
@@ -210,7 +213,7 @@
std::get<4>(request[0])[0].second = 3;
const LockRequests& t = request;
// Return No Conflict
- ASSERT_EQ(0, lockManager.isConflictRequest(t));
+ EXPECT_FALSE(lockManager.isConflictRequest(t));
}
TEST_F(LockTest, MultiRequestWithoutConflictduetoReadLocktype)
@@ -221,7 +224,7 @@
std::get<4>(request[0])[0].first = "LockAll";
const LockRequests& t = request;
// Return No Conflict
- ASSERT_EQ(0, lockManager.isConflictRequest(t));
+ EXPECT_FALSE(lockManager.isConflictRequest(t));
}
TEST_F(LockTest, MultiRequestWithoutConflictduetoReadLocktypeAndLockall)
@@ -233,7 +236,7 @@
std::get<4>(request[0])[1].first = "LockAll";
const LockRequests& t = request;
// Return No Conflict
- ASSERT_EQ(0, lockManager.isConflictRequest(t));
+ EXPECT_FALSE(lockManager.isConflictRequest(t));
}
TEST_F(LockTest, RequestConflictedWithLockTableEntries)
@@ -248,7 +251,7 @@
const LockRequests& p = request;
auto rc2 = lockManager.isConflictWithTable(p);
// Return a Conflict
- ASSERT_EQ(1, rc2.first);
+ EXPECT_TRUE(rc2.first);
}
TEST_F(LockTest, RequestNotConflictedWithLockTableEntries)
@@ -264,15 +267,15 @@
const LockRequests& p = request;
auto rc2 = lockManager.isConflictWithTable(p);
// Return No Conflict
- ASSERT_EQ(0, rc2.first);
+ EXPECT_FALSE(rc2.first);
}
TEST_F(LockTest, TestGenerateTransactionIDFunction)
{
MockLock lockManager;
- uint32_t transactionid1 = lockManager.generateTransactionId();
- uint32_t transactionid2 = lockManager.generateTransactionId();
- EXPECT_TRUE(transactionid2 == ++transactionid1);
+ uint32_t transactionId1 = lockManager.generateTransactionId();
+ uint32_t transactionId2 = lockManager.generateTransactionId();
+ EXPECT_EQ(transactionId2, ++transactionId1);
}
TEST_F(LockTest, ValidateTransactionIDsGoodTestCase)
@@ -283,7 +286,7 @@
auto rc1 = lockManager.isConflictWithTable(t);
std::vector<uint32_t> tids = {1};
const std::vector<uint32_t>& p = tids;
- ASSERT_EQ(1, lockManager.validateRids(p));
+ EXPECT_TRUE(lockManager.validateRids(p));
}
TEST_F(LockTest, ValidateTransactionIDsBadTestCase)
@@ -294,7 +297,7 @@
auto rc1 = lockManager.isConflictWithTable(t);
std::vector<uint32_t> tids = {10};
const std::vector<uint32_t>& p = tids;
- ASSERT_EQ(0, lockManager.validateRids(p));
+ EXPECT_FALSE(lockManager.validateRids(p));
}
TEST_F(LockTest, ValidateisItMyLockGoodTestCase)
@@ -309,7 +312,7 @@
std::string sessionid = "xxxxx";
std::pair<SType, SType> ids = std::make_pair(hmcid, sessionid);
auto rc = lockManager.isItMyLock(p, ids);
- ASSERT_EQ(1, rc.first);
+ EXPECT_TRUE(rc.first);
}
TEST_F(LockTest, ValidateisItMyLockBadTestCase)
@@ -326,7 +329,7 @@
std::string sessionid = "random";
std::pair<SType, SType> ids = std::make_pair(hmcid, sessionid);
auto rc = lockManager.isItMyLock(p, ids);
- ASSERT_EQ(0, rc.first);
+ EXPECT_FALSE(rc.first);
}
TEST_F(LockTest, ValidateSessionIDForGetlocklistBadTestCase)
@@ -339,7 +342,7 @@
auto status = lockManager.getLockList(sessionid);
auto result =
std::get<std::vector<std::pair<uint32_t, LockRequests>>>(status);
- ASSERT_EQ(0, result.size());
+ EXPECT_THAT(result, IsEmpty());
}
TEST_F(LockTest, ValidateSessionIDForGetlocklistGoodTestCase)
@@ -352,7 +355,7 @@
auto status = lockManager.getLockList(sessionid);
auto result =
std::get<std::vector<std::pair<uint32_t, LockRequests>>>(status);
- ASSERT_EQ(1, result.size());
+ EXPECT_EQ(result.size(), 1);
}
} // namespace