From efad7dba5f3d3ec27763926a7dc9dc5701c15f05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Poyraz=20K=C3=BC=C3=A7=C3=BCkarslan?= <83272398+PoyrazK@users.noreply.github.com> Date: Sat, 14 Mar 2026 22:29:40 +0300 Subject: [PATCH 1/8] test: implement catalog module coverage tests (Phase 1) --- CMakeLists.txt | 1 + tests/catalog_coverage_tests.cpp | 154 +++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 tests/catalog_coverage_tests.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index e0d03f8..ab5c552 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -84,6 +84,7 @@ endmacro() # Tests if(BUILD_TESTS) enable_testing() + add_cloudsql_test(catalog_coverage_tests tests/catalog_coverage_tests.cpp) add_cloudsql_test(cloudSQL_tests tests/cloudSQL_tests.cpp) add_cloudsql_test(server_tests tests/server_tests.cpp) add_cloudsql_test(statement_tests tests/statement_tests.cpp) diff --git a/tests/catalog_coverage_tests.cpp b/tests/catalog_coverage_tests.cpp new file mode 100644 index 0000000..79d02da --- /dev/null +++ b/tests/catalog_coverage_tests.cpp @@ -0,0 +1,154 @@ +/** + * @file catalog_coverage_tests.cpp + * @brief Targeted unit tests to increase coverage of the Catalog module + */ + +#include +#include +#include +#include "catalog/catalog.hpp" +#include "common/value.hpp" +#include "distributed/raft_types.hpp" + +using namespace cloudsql; + +namespace { + +/** + * @brief Tests Catalog behavior with missing entities and invalid lookups. + */ +TEST(CatalogCoverageTests, MissingEntities) { + auto catalog = Catalog::create(); + + // Invalid table lookup + EXPECT_FALSE(catalog->get_table(9999).has_value()); + EXPECT_FALSE(catalog->table_exists(9999)); + EXPECT_FALSE(catalog->table_exists_by_name("non_existent")); + EXPECT_FALSE(catalog->get_table_by_name("non_existent").has_value()); + + // Invalid index lookup + EXPECT_FALSE(catalog->get_index(8888).has_value()); + EXPECT_TRUE(catalog->get_table_indexes(9999).empty()); + + // Dropping non-existent entities + EXPECT_FALSE(catalog->drop_table(9999)); + EXPECT_FALSE(catalog->drop_index(8888)); + + // Update stats for non-existent table + EXPECT_FALSE(catalog->update_table_stats(9999, 100)); +} + +/** + * @brief Tests Catalog behavior with duplicate entities and creation edge cases. + */ +TEST(CatalogCoverageTests, DuplicateEntities) { + auto catalog = Catalog::create(); + std::vector cols = {{"id", common::ValueType::TYPE_INT64, 0}}; + + oid_t tid = catalog->create_table("test_table", cols); + ASSERT_NE(tid, 0); + + // Duplicate table creation should throw + EXPECT_THROW(catalog->create_table("test_table", cols), std::runtime_error); + + // Create an index + oid_t iid = catalog->create_index("idx_id", tid, {0}, IndexType::BTree, true); + ASSERT_NE(iid, 0); + + // Duplicate index creation should throw + EXPECT_THROW(catalog->create_index("idx_id", tid, {0}, IndexType::BTree, true), std::runtime_error); + + // Creating index on missing table + EXPECT_EQ(catalog->create_index("idx_missing", 9999, {0}, IndexType::BTree, false), 0); +} + +/** + * @brief Helper to serialize CreateTable command for Raft simulation + */ +std::vector serialize_create_table(const std::string& name, const std::vector& columns) { + std::vector data; + data.push_back(1); // Type 1 + + uint32_t name_len = name.size(); + size_t off = data.size(); + data.resize(off + 4 + name_len); + std::memcpy(data.data() + off, &name_len, 4); + std::memcpy(data.data() + off + 4, name.data(), name_len); + + uint32_t col_count = columns.size(); + off = data.size(); + data.resize(off + 4); + std::memcpy(data.data() + off, &col_count, 4); + + for (const auto& col : columns) { + uint32_t cname_len = col.name.size(); + off = data.size(); + data.resize(off + 4 + cname_len + 1 + 2); + std::memcpy(data.data() + off, &cname_len, 4); + std::memcpy(data.data() + off + 4, col.name.data(), cname_len); + data[off + 4 + cname_len] = static_cast(col.type); + std::memcpy(data.data() + off + 4 + cname_len + 1, &col.position, 2); + } + + uint32_t shard_count = 1; + off = data.size(); + data.resize(off + 4); + std::memcpy(data.data() + off, &shard_count, 4); + + std::string addr = "127.0.0.1"; + uint32_t addr_len = addr.size(); + uint32_t sid = 0; + uint16_t port = 6441; + + off = data.size(); + data.resize(off + 4 + addr_len + 4 + 2); + std::memcpy(data.data() + off, &addr_len, 4); + std::memcpy(data.data() + off + 4, addr.data(), addr_len); + std::memcpy(data.data() + off + 4 + addr_len, &sid, 4); + std::memcpy(data.data() + off + 4 + addr_len + 4, &port, 2); + + return data; +} + +/** + * @brief Tests the Raft state machine application (apply) in the Catalog. + */ +TEST(CatalogCoverageTests, RaftApply) { + auto catalog = Catalog::create(); + + // 1. Replay CreateTable + std::vector cols = {{"id", common::ValueType::TYPE_INT64, 0}}; + std::vector create_data = serialize_create_table("raft_table", cols); + + raft::LogEntry entry; + entry.term = 1; + entry.index = 1; + entry.data = create_data; + + catalog->apply(entry); + EXPECT_TRUE(catalog->table_exists_by_name("raft_table")); + + auto table_opt = catalog->get_table_by_name("raft_table"); + ASSERT_TRUE(table_opt.has_value()); + oid_t tid = (*table_opt)->table_id; + + // 2. Replay DropTable + std::vector drop_data; + drop_data.push_back(2); // Type 2 + drop_data.resize(5); + std::memcpy(drop_data.data() + 1, &tid, 4); + + entry.index = 2; + entry.data = drop_data; + + catalog->apply(entry); + EXPECT_FALSE(catalog->table_exists(tid)); + EXPECT_FALSE(catalog->table_exists_by_name("raft_table")); + + // 3. Replay with empty data (should do nothing) + entry.index = 3; + entry.data.clear(); + catalog->apply(entry); +} + +} // namespace From b5a3db8b6e00d2ad54fcf998b34f13c308415836 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Poyraz=20K=C3=BC=C3=A7=C3=BCkarslan?= <83272398+PoyrazK@users.noreply.github.com> Date: Sat, 14 Mar 2026 22:52:51 +0300 Subject: [PATCH 2/8] test: implement transaction module coverage tests (Phase 2) --- CMakeLists.txt | 1 + tests/transaction_coverage_tests.cpp | 129 +++++++++++++++++++++++++++ 2 files changed, 130 insertions(+) create mode 100644 tests/transaction_coverage_tests.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index ab5c552..1a30d53 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -85,6 +85,7 @@ endmacro() if(BUILD_TESTS) enable_testing() add_cloudsql_test(catalog_coverage_tests tests/catalog_coverage_tests.cpp) + add_cloudsql_test(transaction_coverage_tests tests/transaction_coverage_tests.cpp) add_cloudsql_test(cloudSQL_tests tests/cloudSQL_tests.cpp) add_cloudsql_test(server_tests tests/server_tests.cpp) add_cloudsql_test(statement_tests tests/statement_tests.cpp) diff --git a/tests/transaction_coverage_tests.cpp b/tests/transaction_coverage_tests.cpp new file mode 100644 index 0000000..b549330 --- /dev/null +++ b/tests/transaction_coverage_tests.cpp @@ -0,0 +1,129 @@ +/** + * @file transaction_coverage_tests.cpp + * @brief Targeted unit tests to increase coverage of Transaction and Lock Manager + */ + +#include +#include +#include +#include +#include + +#include "catalog/catalog.hpp" +#include "common/config.hpp" +#include "storage/buffer_pool_manager.hpp" +#include "storage/storage_manager.hpp" +#include "storage/heap_table.hpp" +#include "transaction/lock_manager.hpp" +#include "transaction/transaction.hpp" +#include "transaction/transaction_manager.hpp" + +using namespace cloudsql; +using namespace cloudsql::transaction; +using namespace cloudsql::storage; + +namespace { + +/** + * @brief Stress tests the LockManager with concurrent shared and exclusive requests. + */ +TEST(TransactionCoverageTests, LockManagerConcurrency) { + LockManager lm; + const int num_readers = 5; + std::vector readers; + std::atomic shared_granted{0}; + std::atomic stop{false}; + + Transaction writer_txn(100); + + // Writers holds exclusive lock initially + ASSERT_TRUE(lm.acquire_exclusive(&writer_txn, "RESOURCE")); + + for (int i = 0; i < num_readers; ++i) { + readers.emplace_back([&, i]() { + Transaction reader_txn(i); + if (lm.acquire_shared(&reader_txn, "RESOURCE")) { + shared_granted++; + while (!stop) { + std::this_thread::yield(); + } + lm.unlock(&reader_txn, "RESOURCE"); + } + }); + } + + // Readers should be blocked by the writer + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + EXPECT_EQ(shared_granted.load(), 0); + + // Release writer lock, readers should proceed + lm.unlock(&writer_txn, "RESOURCE"); + + // Wait for all readers to get the lock + for (int i = 0; i < 50 && shared_granted.load() < num_readers; ++i) { + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + } + EXPECT_EQ(shared_granted.load(), num_readers); + + stop = true; + for (auto& t : readers) { + t.join(); + } +} + +/** + * @brief Tests deep rollback functionality via the Undo Log. + */ +TEST(TransactionCoverageTests, DeepRollback) { + auto catalog = Catalog::create(); + StorageManager disk_manager("./test_data"); + BufferPoolManager bpm(config::Config::DEFAULT_BUFFER_POOL_SIZE, disk_manager); + LockManager lm; + TransactionManager tm(lm, *catalog, bpm, nullptr); + + std::vector cols = {{"id", common::ValueType::TYPE_INT64, 0}, + {"val", common::ValueType::TYPE_TEXT, 1}}; + oid_t tid = catalog->create_table("rollback_stress", cols); + + executor::Schema schema; + schema.add_column("id", common::ValueType::TYPE_INT64); + schema.add_column("val", common::ValueType::TYPE_TEXT); + + HeapTable table("rollback_stress", bpm, schema); + table.create(); + + Transaction* txn = tm.begin(); + + // 1. Insert some data + auto rid1 = table.insert(executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A")}), txn->get_id()); + txn->add_undo_log(UndoLog::Type::INSERT, "rollback_stress", rid1); + + auto rid2 = table.insert(executor::Tuple({common::Value::make_int64(2), common::Value::make_text("B")}), txn->get_id()); + txn->add_undo_log(UndoLog::Type::INSERT, "rollback_stress", rid2); + + // 2. Update data + table.remove(rid1, txn->get_id()); // Mark old version deleted + auto rid1_new = table.insert(executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A_NEW")}), txn->get_id()); + txn->add_undo_log(UndoLog::Type::UPDATE, "rollback_stress", rid1_new, rid1); + + // 3. Delete data + table.remove(rid2, txn->get_id()); + txn->add_undo_log(UndoLog::Type::DELETE, "rollback_stress", rid2); + + EXPECT_EQ(table.tuple_count(), 1U); // rid1_new is active, rid1 and rid2 are logically deleted + + // 4. Abort + tm.abort(txn); + + // 5. Verify restoration + EXPECT_EQ(table.tuple_count(), 0U); // Inserted rows should be physically removed or logically invisible + + // The table should be empty because we aborted the inserts + auto iter = table.scan(); + executor::Tuple t; + EXPECT_FALSE(iter.next(t)); + + static_cast(std::remove("./test_data/rollback_stress.heap")); +} + +} // namespace From d475e24a16eab7868d35db345d4ba219f62fdfc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Poyraz=20K=C3=BC=C3=A7=C3=BCkarslan?= <83272398+PoyrazK@users.noreply.github.com> Date: Sun, 15 Mar 2026 15:37:34 +0300 Subject: [PATCH 3/8] test: implement utils and network coverage tests (Phase 3) --- CMakeLists.txt | 1 + tests/utils_coverage_tests.cpp | 111 +++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+) create mode 100644 tests/utils_coverage_tests.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 1a30d53..8e8f3b6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -86,6 +86,7 @@ if(BUILD_TESTS) enable_testing() add_cloudsql_test(catalog_coverage_tests tests/catalog_coverage_tests.cpp) add_cloudsql_test(transaction_coverage_tests tests/transaction_coverage_tests.cpp) + add_cloudsql_test(utils_coverage_tests tests/utils_coverage_tests.cpp) add_cloudsql_test(cloudSQL_tests tests/cloudSQL_tests.cpp) add_cloudsql_test(server_tests tests/server_tests.cpp) add_cloudsql_test(statement_tests tests/statement_tests.cpp) diff --git a/tests/utils_coverage_tests.cpp b/tests/utils_coverage_tests.cpp new file mode 100644 index 0000000..4ca3429 --- /dev/null +++ b/tests/utils_coverage_tests.cpp @@ -0,0 +1,111 @@ +/** + * @file utils_coverage_tests.cpp + * @brief Targeted unit tests to increase coverage of Common Values and Network Client + */ + +#include +#include +#include +#include "common/value.hpp" +#include "network/rpc_client.hpp" + +using namespace cloudsql::common; +using namespace cloudsql::network; + +namespace { + +/** + * @brief Tests Value accessor failure paths and boundary conversions. + */ +TEST(UtilsCoverageTests, ValueEdgeCases) { + // 1. Accessor failure paths + Value v_int(ValueType::TYPE_INT32); // Default 0 + EXPECT_THROW(v_int.as_bool(), std::runtime_error); + EXPECT_THROW(v_int.as_float64(), std::runtime_error); + EXPECT_THROW(v_int.as_text(), std::runtime_error); + + Value v_bool(true); + EXPECT_THROW(v_bool.as_int64(), std::runtime_error); + + Value v_text("123"); + EXPECT_THROW(v_text.as_int32(), std::runtime_error); + + // 2. boundary to_* conversions + EXPECT_EQ(v_text.to_int64(), 0); // Text doesn't auto-convert to int in to_int64() + EXPECT_EQ(v_text.to_float64(), 0.0); + + Value v_null = Value::make_null(); + EXPECT_EQ(v_null.to_int64(), 0); + EXPECT_EQ(v_null.to_float64(), 0.0); + EXPECT_STREQ(v_null.to_string().c_str(), "NULL"); + + Value v_f(1.23); + EXPECT_EQ(v_f.to_int64(), 1); + + // 3. Numeric check + EXPECT_TRUE(v_int.is_numeric()); + EXPECT_TRUE(v_f.is_numeric()); + EXPECT_FALSE(v_text.is_numeric()); + EXPECT_FALSE(v_bool.is_numeric()); +} + +/** + * @brief Tests complex Value comparisons including mixed types and NULLs. + */ +TEST(UtilsCoverageTests, ValueComparisons) { + Value v_null = Value::make_null(); + Value v_int(10); + Value v_float(10.0); + Value v_text("A"); + + // Equality + EXPECT_TRUE(v_int == v_float); // Numeric equality + EXPECT_FALSE(v_int == v_text); + EXPECT_FALSE(v_int == v_null); + + // Less than + EXPECT_FALSE(v_null < v_int); // NULL is not less than anything + EXPECT_TRUE(v_int < v_null); // non-NULL is less than NULL (by convention in this impl) + + Value v_int_small(5); + EXPECT_TRUE(v_int_small < v_float); + EXPECT_FALSE(v_float < v_int_small); + + Value v_text_b("B"); + EXPECT_TRUE(v_text < v_text_b); + + // Mixed numeric/non-numeric comparison + EXPECT_FALSE(v_int < v_text); +} + +/** + * @brief Tests RpcClient behavior on connection failures. + */ +TEST(UtilsCoverageTests, RpcClientFailure) { + // Attempt to connect to an unreachable port on localhost + RpcClient client("127.0.0.1", 1); // Port 1 is usually privileged and closed + + EXPECT_FALSE(client.connect()); + EXPECT_FALSE(client.is_connected()); + + std::vector resp; + EXPECT_FALSE(client.call(RpcType::AppendEntries, {1, 2, 3}, resp)); + EXPECT_FALSE(client.send_only(RpcType::AppendEntries, {1, 2, 3})); +} + +/** + * @brief Tests Value Hashing. + */ +TEST(UtilsCoverageTests, ValueHash) { + Value::Hash hasher; + Value v1(10); + Value v2(10); + Value v3("10"); + Value v_null = Value::make_null(); + + EXPECT_EQ(hasher(v1), hasher(v2)); + EXPECT_NE(hasher(v1), hasher(v3)); + EXPECT_NE(hasher(v1), hasher(v_null)); +} + +} // namespace From c1b467ced431ec23f46def2e5d68138ce06a82f8 Mon Sep 17 00:00:00 2001 From: poyrazK <83272398+poyrazK@users.noreply.github.com> Date: Sun, 15 Mar 2026 12:44:25 +0000 Subject: [PATCH 4/8] style: automated clang-format fixes --- tests/catalog_coverage_tests.cpp | 26 +++++++++++-------- tests/transaction_coverage_tests.cpp | 38 +++++++++++++++++----------- tests/utils_coverage_tests.cpp | 28 ++++++++++---------- 3 files changed, 53 insertions(+), 39 deletions(-) diff --git a/tests/catalog_coverage_tests.cpp b/tests/catalog_coverage_tests.cpp index 79d02da..7470997 100644 --- a/tests/catalog_coverage_tests.cpp +++ b/tests/catalog_coverage_tests.cpp @@ -4,8 +4,10 @@ */ #include + #include #include + #include "catalog/catalog.hpp" #include "common/value.hpp" #include "distributed/raft_types.hpp" @@ -19,7 +21,7 @@ namespace { */ TEST(CatalogCoverageTests, MissingEntities) { auto catalog = Catalog::create(); - + // Invalid table lookup EXPECT_FALSE(catalog->get_table(9999).has_value()); EXPECT_FALSE(catalog->table_exists(9999)); @@ -56,7 +58,8 @@ TEST(CatalogCoverageTests, DuplicateEntities) { ASSERT_NE(iid, 0); // Duplicate index creation should throw - EXPECT_THROW(catalog->create_index("idx_id", tid, {0}, IndexType::BTree, true), std::runtime_error); + EXPECT_THROW(catalog->create_index("idx_id", tid, {0}, IndexType::BTree, true), + std::runtime_error); // Creating index on missing table EXPECT_EQ(catalog->create_index("idx_missing", 9999, {0}, IndexType::BTree, false), 0); @@ -65,9 +68,10 @@ TEST(CatalogCoverageTests, DuplicateEntities) { /** * @brief Helper to serialize CreateTable command for Raft simulation */ -std::vector serialize_create_table(const std::string& name, const std::vector& columns) { +std::vector serialize_create_table(const std::string& name, + const std::vector& columns) { std::vector data; - data.push_back(1); // Type 1 + data.push_back(1); // Type 1 uint32_t name_len = name.size(); size_t off = data.size(); @@ -99,7 +103,7 @@ std::vector serialize_create_table(const std::string& name, const std:: uint32_t addr_len = addr.size(); uint32_t sid = 0; uint16_t port = 6441; - + off = data.size(); data.resize(off + 4 + addr_len + 4 + 2); std::memcpy(data.data() + off, &addr_len, 4); @@ -115,11 +119,11 @@ std::vector serialize_create_table(const std::string& name, const std:: */ TEST(CatalogCoverageTests, RaftApply) { auto catalog = Catalog::create(); - + // 1. Replay CreateTable std::vector cols = {{"id", common::ValueType::TYPE_INT64, 0}}; std::vector create_data = serialize_create_table("raft_table", cols); - + raft::LogEntry entry; entry.term = 1; entry.index = 1; @@ -127,20 +131,20 @@ TEST(CatalogCoverageTests, RaftApply) { catalog->apply(entry); EXPECT_TRUE(catalog->table_exists_by_name("raft_table")); - + auto table_opt = catalog->get_table_by_name("raft_table"); ASSERT_TRUE(table_opt.has_value()); oid_t tid = (*table_opt)->table_id; // 2. Replay DropTable std::vector drop_data; - drop_data.push_back(2); // Type 2 + drop_data.push_back(2); // Type 2 drop_data.resize(5); std::memcpy(drop_data.data() + 1, &tid, 4); entry.index = 2; entry.data = drop_data; - + catalog->apply(entry); EXPECT_FALSE(catalog->table_exists(tid)); EXPECT_FALSE(catalog->table_exists_by_name("raft_table")); @@ -151,4 +155,4 @@ TEST(CatalogCoverageTests, RaftApply) { catalog->apply(entry); } -} // namespace +} // namespace diff --git a/tests/transaction_coverage_tests.cpp b/tests/transaction_coverage_tests.cpp index b549330..eaa727f 100644 --- a/tests/transaction_coverage_tests.cpp +++ b/tests/transaction_coverage_tests.cpp @@ -4,6 +4,7 @@ */ #include + #include #include #include @@ -12,8 +13,8 @@ #include "catalog/catalog.hpp" #include "common/config.hpp" #include "storage/buffer_pool_manager.hpp" -#include "storage/storage_manager.hpp" #include "storage/heap_table.hpp" +#include "storage/storage_manager.hpp" #include "transaction/lock_manager.hpp" #include "transaction/transaction.hpp" #include "transaction/transaction_manager.hpp" @@ -35,7 +36,7 @@ TEST(TransactionCoverageTests, LockManagerConcurrency) { std::atomic stop{false}; Transaction writer_txn(100); - + // Writers holds exclusive lock initially ASSERT_TRUE(lm.acquire_exclusive(&writer_txn, "RESOURCE")); @@ -58,7 +59,7 @@ TEST(TransactionCoverageTests, LockManagerConcurrency) { // Release writer lock, readers should proceed lm.unlock(&writer_txn, "RESOURCE"); - + // Wait for all readers to get the lock for (int i = 0; i < 50 && shared_granted.load() < num_readers; ++i) { std::this_thread::sleep_for(std::chrono::milliseconds(50)); @@ -84,40 +85,47 @@ TEST(TransactionCoverageTests, DeepRollback) { std::vector cols = {{"id", common::ValueType::TYPE_INT64, 0}, {"val", common::ValueType::TYPE_TEXT, 1}}; oid_t tid = catalog->create_table("rollback_stress", cols); - + executor::Schema schema; schema.add_column("id", common::ValueType::TYPE_INT64); schema.add_column("val", common::ValueType::TYPE_TEXT); - + HeapTable table("rollback_stress", bpm, schema); table.create(); Transaction* txn = tm.begin(); - + // 1. Insert some data - auto rid1 = table.insert(executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A")}), txn->get_id()); + auto rid1 = + table.insert(executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A")}), + txn->get_id()); txn->add_undo_log(UndoLog::Type::INSERT, "rollback_stress", rid1); - - auto rid2 = table.insert(executor::Tuple({common::Value::make_int64(2), common::Value::make_text("B")}), txn->get_id()); + + auto rid2 = + table.insert(executor::Tuple({common::Value::make_int64(2), common::Value::make_text("B")}), + txn->get_id()); txn->add_undo_log(UndoLog::Type::INSERT, "rollback_stress", rid2); // 2. Update data - table.remove(rid1, txn->get_id()); // Mark old version deleted - auto rid1_new = table.insert(executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A_NEW")}), txn->get_id()); + table.remove(rid1, txn->get_id()); // Mark old version deleted + auto rid1_new = table.insert( + executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A_NEW")}), + txn->get_id()); txn->add_undo_log(UndoLog::Type::UPDATE, "rollback_stress", rid1_new, rid1); // 3. Delete data table.remove(rid2, txn->get_id()); txn->add_undo_log(UndoLog::Type::DELETE, "rollback_stress", rid2); - EXPECT_EQ(table.tuple_count(), 1U); // rid1_new is active, rid1 and rid2 are logically deleted + EXPECT_EQ(table.tuple_count(), 1U); // rid1_new is active, rid1 and rid2 are logically deleted // 4. Abort tm.abort(txn); // 5. Verify restoration - EXPECT_EQ(table.tuple_count(), 0U); // Inserted rows should be physically removed or logically invisible - + EXPECT_EQ(table.tuple_count(), + 0U); // Inserted rows should be physically removed or logically invisible + // The table should be empty because we aborted the inserts auto iter = table.scan(); executor::Tuple t; @@ -126,4 +134,4 @@ TEST(TransactionCoverageTests, DeepRollback) { static_cast(std::remove("./test_data/rollback_stress.heap")); } -} // namespace +} // namespace diff --git a/tests/utils_coverage_tests.cpp b/tests/utils_coverage_tests.cpp index 4ca3429..30d449f 100644 --- a/tests/utils_coverage_tests.cpp +++ b/tests/utils_coverage_tests.cpp @@ -4,8 +4,10 @@ */ #include + #include #include + #include "common/value.hpp" #include "network/rpc_client.hpp" @@ -19,7 +21,7 @@ namespace { */ TEST(UtilsCoverageTests, ValueEdgeCases) { // 1. Accessor failure paths - Value v_int(ValueType::TYPE_INT32); // Default 0 + Value v_int(ValueType::TYPE_INT32); // Default 0 EXPECT_THROW(v_int.as_bool(), std::runtime_error); EXPECT_THROW(v_int.as_float64(), std::runtime_error); EXPECT_THROW(v_int.as_text(), std::runtime_error); @@ -31,7 +33,7 @@ TEST(UtilsCoverageTests, ValueEdgeCases) { EXPECT_THROW(v_text.as_int32(), std::runtime_error); // 2. boundary to_* conversions - EXPECT_EQ(v_text.to_int64(), 0); // Text doesn't auto-convert to int in to_int64() + EXPECT_EQ(v_text.to_int64(), 0); // Text doesn't auto-convert to int in to_int64() EXPECT_EQ(v_text.to_float64(), 0.0); Value v_null = Value::make_null(); @@ -41,7 +43,7 @@ TEST(UtilsCoverageTests, ValueEdgeCases) { Value v_f(1.23); EXPECT_EQ(v_f.to_int64(), 1); - + // 3. Numeric check EXPECT_TRUE(v_int.is_numeric()); EXPECT_TRUE(v_f.is_numeric()); @@ -59,23 +61,23 @@ TEST(UtilsCoverageTests, ValueComparisons) { Value v_text("A"); // Equality - EXPECT_TRUE(v_int == v_float); // Numeric equality + EXPECT_TRUE(v_int == v_float); // Numeric equality EXPECT_FALSE(v_int == v_text); EXPECT_FALSE(v_int == v_null); // Less than - EXPECT_FALSE(v_null < v_int); // NULL is not less than anything - EXPECT_TRUE(v_int < v_null); // non-NULL is less than NULL (by convention in this impl) - + EXPECT_FALSE(v_null < v_int); // NULL is not less than anything + EXPECT_TRUE(v_int < v_null); // non-NULL is less than NULL (by convention in this impl) + Value v_int_small(5); EXPECT_TRUE(v_int_small < v_float); EXPECT_FALSE(v_float < v_int_small); Value v_text_b("B"); EXPECT_TRUE(v_text < v_text_b); - + // Mixed numeric/non-numeric comparison - EXPECT_FALSE(v_int < v_text); + EXPECT_FALSE(v_int < v_text); } /** @@ -83,11 +85,11 @@ TEST(UtilsCoverageTests, ValueComparisons) { */ TEST(UtilsCoverageTests, RpcClientFailure) { // Attempt to connect to an unreachable port on localhost - RpcClient client("127.0.0.1", 1); // Port 1 is usually privileged and closed - + RpcClient client("127.0.0.1", 1); // Port 1 is usually privileged and closed + EXPECT_FALSE(client.connect()); EXPECT_FALSE(client.is_connected()); - + std::vector resp; EXPECT_FALSE(client.call(RpcType::AppendEntries, {1, 2, 3}, resp)); EXPECT_FALSE(client.send_only(RpcType::AppendEntries, {1, 2, 3})); @@ -108,4 +110,4 @@ TEST(UtilsCoverageTests, ValueHash) { EXPECT_NE(hasher(v1), hasher(v_null)); } -} // namespace +} // namespace From f629c4a9ef05a022c70681512be461ebd29cdcc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Poyraz=20K=C3=BC=C3=A7=C3=BCkarslan?= <83272398+PoyrazK@users.noreply.github.com> Date: Sun, 15 Mar 2026 16:19:44 +0300 Subject: [PATCH 5/8] test: refactor coverage tests for better resource management and deterministic network failure --- tests/transaction_coverage_tests.cpp | 117 ++++++++++++++++----------- tests/utils_coverage_tests.cpp | 35 ++++---- 2 files changed, 91 insertions(+), 61 deletions(-) diff --git a/tests/transaction_coverage_tests.cpp b/tests/transaction_coverage_tests.cpp index eaa727f..9e0d125 100644 --- a/tests/transaction_coverage_tests.cpp +++ b/tests/transaction_coverage_tests.cpp @@ -4,17 +4,17 @@ */ #include - #include #include #include #include +#include #include "catalog/catalog.hpp" #include "common/config.hpp" #include "storage/buffer_pool_manager.hpp" -#include "storage/heap_table.hpp" #include "storage/storage_manager.hpp" +#include "storage/heap_table.hpp" #include "transaction/lock_manager.hpp" #include "transaction/transaction.hpp" #include "transaction/transaction_manager.hpp" @@ -25,10 +25,60 @@ using namespace cloudsql::storage; namespace { +/** + * @class TransactionCoverageTests + * @brief Fixture for transaction-related coverage tests to ensure proper resource management. + */ +class TransactionCoverageTests : public ::testing::Test { +protected: + void SetUp() override { + catalog = Catalog::create(); + disk_manager = std::make_unique("./test_data"); + bpm = std::make_unique(config::Config::DEFAULT_BUFFER_POOL_SIZE, *disk_manager); + lm = std::make_unique(); + tm = std::make_unique(*lm, *catalog, *bpm, nullptr); + + std::vector cols = {{"id", common::ValueType::TYPE_INT64, 0}, + {"val", common::ValueType::TYPE_TEXT, 1}}; + catalog->create_table("rollback_stress", cols); + + executor::Schema schema; + schema.add_column("id", common::ValueType::TYPE_INT64); + schema.add_column("val", common::ValueType::TYPE_TEXT); + + table = std::make_unique("rollback_stress", *bpm, schema); + table->create(); + + txn = nullptr; + } + + void TearDown() override { + if (txn != nullptr) { + tm->abort(txn); + } + table.reset(); + tm.reset(); + lm.reset(); + bpm.reset(); + disk_manager.reset(); + catalog.reset(); + + static_cast(std::remove("./test_data/rollback_stress.heap")); + } + + std::unique_ptr catalog; + std::unique_ptr disk_manager; + std::unique_ptr bpm; + std::unique_ptr lm; + std::unique_ptr tm; + std::unique_ptr table; + Transaction* txn; +}; + /** * @brief Stress tests the LockManager with concurrent shared and exclusive requests. */ -TEST(TransactionCoverageTests, LockManagerConcurrency) { +TEST(TransactionCoverageTestsStandalone, LockManagerConcurrency) { LockManager lm; const int num_readers = 5; std::vector readers; @@ -36,7 +86,7 @@ TEST(TransactionCoverageTests, LockManagerConcurrency) { std::atomic stop{false}; Transaction writer_txn(100); - + // Writers holds exclusive lock initially ASSERT_TRUE(lm.acquire_exclusive(&writer_txn, "RESOURCE")); @@ -59,7 +109,7 @@ TEST(TransactionCoverageTests, LockManagerConcurrency) { // Release writer lock, readers should proceed lm.unlock(&writer_txn, "RESOURCE"); - + // Wait for all readers to get the lock for (int i = 0; i < 50 && shared_granted.load() < num_readers; ++i) { std::this_thread::sleep_for(std::chrono::milliseconds(50)); @@ -75,63 +125,38 @@ TEST(TransactionCoverageTests, LockManagerConcurrency) { /** * @brief Tests deep rollback functionality via the Undo Log. */ -TEST(TransactionCoverageTests, DeepRollback) { - auto catalog = Catalog::create(); - StorageManager disk_manager("./test_data"); - BufferPoolManager bpm(config::Config::DEFAULT_BUFFER_POOL_SIZE, disk_manager); - LockManager lm; - TransactionManager tm(lm, *catalog, bpm, nullptr); - - std::vector cols = {{"id", common::ValueType::TYPE_INT64, 0}, - {"val", common::ValueType::TYPE_TEXT, 1}}; - oid_t tid = catalog->create_table("rollback_stress", cols); - - executor::Schema schema; - schema.add_column("id", common::ValueType::TYPE_INT64); - schema.add_column("val", common::ValueType::TYPE_TEXT); - - HeapTable table("rollback_stress", bpm, schema); - table.create(); - - Transaction* txn = tm.begin(); - +TEST_F(TransactionCoverageTests, DeepRollback) { + txn = tm->begin(); + // 1. Insert some data - auto rid1 = - table.insert(executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A")}), - txn->get_id()); + auto rid1 = table->insert(executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A")}), txn->get_id()); txn->add_undo_log(UndoLog::Type::INSERT, "rollback_stress", rid1); - - auto rid2 = - table.insert(executor::Tuple({common::Value::make_int64(2), common::Value::make_text("B")}), - txn->get_id()); + + auto rid2 = table->insert(executor::Tuple({common::Value::make_int64(2), common::Value::make_text("B")}), txn->get_id()); txn->add_undo_log(UndoLog::Type::INSERT, "rollback_stress", rid2); // 2. Update data - table.remove(rid1, txn->get_id()); // Mark old version deleted - auto rid1_new = table.insert( - executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A_NEW")}), - txn->get_id()); + table->remove(rid1, txn->get_id()); // Mark old version deleted + auto rid1_new = table->insert(executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A_NEW")}), txn->get_id()); txn->add_undo_log(UndoLog::Type::UPDATE, "rollback_stress", rid1_new, rid1); // 3. Delete data - table.remove(rid2, txn->get_id()); + table->remove(rid2, txn->get_id()); txn->add_undo_log(UndoLog::Type::DELETE, "rollback_stress", rid2); - EXPECT_EQ(table.tuple_count(), 1U); // rid1_new is active, rid1 and rid2 are logically deleted + EXPECT_EQ(table->tuple_count(), 1U); // rid1_new is active, rid1 and rid2 are logically deleted // 4. Abort - tm.abort(txn); + tm->abort(txn); + txn = nullptr; // Marked as aborted and handled by TearDown if still set // 5. Verify restoration - EXPECT_EQ(table.tuple_count(), - 0U); // Inserted rows should be physically removed or logically invisible - + EXPECT_EQ(table->tuple_count(), 0U); // Inserted rows should be physically removed or logically invisible + // The table should be empty because we aborted the inserts - auto iter = table.scan(); + auto iter = table->scan(); executor::Tuple t; EXPECT_FALSE(iter.next(t)); - - static_cast(std::remove("./test_data/rollback_stress.heap")); } -} // namespace +} // namespace diff --git a/tests/utils_coverage_tests.cpp b/tests/utils_coverage_tests.cpp index 30d449f..3be5e6c 100644 --- a/tests/utils_coverage_tests.cpp +++ b/tests/utils_coverage_tests.cpp @@ -4,10 +4,8 @@ */ #include - #include #include - #include "common/value.hpp" #include "network/rpc_client.hpp" @@ -21,7 +19,7 @@ namespace { */ TEST(UtilsCoverageTests, ValueEdgeCases) { // 1. Accessor failure paths - Value v_int(ValueType::TYPE_INT32); // Default 0 + Value v_int(ValueType::TYPE_INT32); // Default 0 EXPECT_THROW(v_int.as_bool(), std::runtime_error); EXPECT_THROW(v_int.as_float64(), std::runtime_error); EXPECT_THROW(v_int.as_text(), std::runtime_error); @@ -33,7 +31,7 @@ TEST(UtilsCoverageTests, ValueEdgeCases) { EXPECT_THROW(v_text.as_int32(), std::runtime_error); // 2. boundary to_* conversions - EXPECT_EQ(v_text.to_int64(), 0); // Text doesn't auto-convert to int in to_int64() + EXPECT_EQ(v_text.to_int64(), 0); // Text doesn't auto-convert to int in to_int64() EXPECT_EQ(v_text.to_float64(), 0.0); Value v_null = Value::make_null(); @@ -43,7 +41,7 @@ TEST(UtilsCoverageTests, ValueEdgeCases) { Value v_f(1.23); EXPECT_EQ(v_f.to_int64(), 1); - + // 3. Numeric check EXPECT_TRUE(v_int.is_numeric()); EXPECT_TRUE(v_f.is_numeric()); @@ -61,36 +59,43 @@ TEST(UtilsCoverageTests, ValueComparisons) { Value v_text("A"); // Equality - EXPECT_TRUE(v_int == v_float); // Numeric equality + EXPECT_TRUE(v_int == v_float); // Numeric equality EXPECT_FALSE(v_int == v_text); EXPECT_FALSE(v_int == v_null); // Less than - EXPECT_FALSE(v_null < v_int); // NULL is not less than anything - EXPECT_TRUE(v_int < v_null); // non-NULL is less than NULL (by convention in this impl) - + EXPECT_FALSE(v_null < v_int); // NULL is not less than anything + EXPECT_TRUE(v_int < v_null); // non-NULL is less than NULL (by convention in this impl) + Value v_int_small(5); EXPECT_TRUE(v_int_small < v_float); EXPECT_FALSE(v_float < v_int_small); Value v_text_b("B"); EXPECT_TRUE(v_text < v_text_b); - + // Mixed numeric/non-numeric comparison - EXPECT_FALSE(v_int < v_text); + EXPECT_FALSE(v_int < v_text); } /** * @brief Tests RpcClient behavior on connection failures. + * + * We use localhost on a reserved/privileged port (1) to trigger an immediate + * "Connection refused" from the OS. This provides a deterministic failure + * without the multi-second timeouts associated with non-routable external + * IP addresses (like 192.0.2.1), ensuring the CI pipeline remains fast while + * still covering the error handling logic in RpcClient. */ TEST(UtilsCoverageTests, RpcClientFailure) { // Attempt to connect to an unreachable port on localhost - RpcClient client("127.0.0.1", 1); // Port 1 is usually privileged and closed - + RpcClient client("127.0.0.1", 1); + EXPECT_FALSE(client.connect()); EXPECT_FALSE(client.is_connected()); - + std::vector resp; + // These should return false as they require a valid connection EXPECT_FALSE(client.call(RpcType::AppendEntries, {1, 2, 3}, resp)); EXPECT_FALSE(client.send_only(RpcType::AppendEntries, {1, 2, 3})); } @@ -110,4 +115,4 @@ TEST(UtilsCoverageTests, ValueHash) { EXPECT_NE(hasher(v1), hasher(v_null)); } -} // namespace +} // namespace From 065a609ad8d2b38453acb76e60c899820227a2e2 Mon Sep 17 00:00:00 2001 From: poyrazK <83272398+poyrazK@users.noreply.github.com> Date: Tue, 17 Mar 2026 13:51:43 +0000 Subject: [PATCH 6/8] style: automated clang-format fixes --- tests/transaction_coverage_tests.cpp | 51 ++++++++++++++++------------ tests/utils_coverage_tests.cpp | 36 ++++++++++---------- 2 files changed, 49 insertions(+), 38 deletions(-) diff --git a/tests/transaction_coverage_tests.cpp b/tests/transaction_coverage_tests.cpp index 9e0d125..0bc90c1 100644 --- a/tests/transaction_coverage_tests.cpp +++ b/tests/transaction_coverage_tests.cpp @@ -4,17 +4,18 @@ */ #include + #include #include +#include #include #include -#include #include "catalog/catalog.hpp" #include "common/config.hpp" #include "storage/buffer_pool_manager.hpp" -#include "storage/storage_manager.hpp" #include "storage/heap_table.hpp" +#include "storage/storage_manager.hpp" #include "transaction/lock_manager.hpp" #include "transaction/transaction.hpp" #include "transaction/transaction_manager.hpp" @@ -30,25 +31,26 @@ namespace { * @brief Fixture for transaction-related coverage tests to ensure proper resource management. */ class TransactionCoverageTests : public ::testing::Test { -protected: + protected: void SetUp() override { catalog = Catalog::create(); disk_manager = std::make_unique("./test_data"); - bpm = std::make_unique(config::Config::DEFAULT_BUFFER_POOL_SIZE, *disk_manager); + bpm = std::make_unique(config::Config::DEFAULT_BUFFER_POOL_SIZE, + *disk_manager); lm = std::make_unique(); tm = std::make_unique(*lm, *catalog, *bpm, nullptr); std::vector cols = {{"id", common::ValueType::TYPE_INT64, 0}, {"val", common::ValueType::TYPE_TEXT, 1}}; catalog->create_table("rollback_stress", cols); - + executor::Schema schema; schema.add_column("id", common::ValueType::TYPE_INT64); schema.add_column("val", common::ValueType::TYPE_TEXT); - + table = std::make_unique("rollback_stress", *bpm, schema); table->create(); - + txn = nullptr; } @@ -62,7 +64,7 @@ class TransactionCoverageTests : public ::testing::Test { bpm.reset(); disk_manager.reset(); catalog.reset(); - + static_cast(std::remove("./test_data/rollback_stress.heap")); } @@ -86,7 +88,7 @@ TEST(TransactionCoverageTestsStandalone, LockManagerConcurrency) { std::atomic stop{false}; Transaction writer_txn(100); - + // Writers holds exclusive lock initially ASSERT_TRUE(lm.acquire_exclusive(&writer_txn, "RESOURCE")); @@ -109,7 +111,7 @@ TEST(TransactionCoverageTestsStandalone, LockManagerConcurrency) { // Release writer lock, readers should proceed lm.unlock(&writer_txn, "RESOURCE"); - + // Wait for all readers to get the lock for (int i = 0; i < 50 && shared_granted.load() < num_readers; ++i) { std::this_thread::sleep_for(std::chrono::milliseconds(50)); @@ -127,36 +129,43 @@ TEST(TransactionCoverageTestsStandalone, LockManagerConcurrency) { */ TEST_F(TransactionCoverageTests, DeepRollback) { txn = tm->begin(); - + // 1. Insert some data - auto rid1 = table->insert(executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A")}), txn->get_id()); + auto rid1 = table->insert( + executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A")}), + txn->get_id()); txn->add_undo_log(UndoLog::Type::INSERT, "rollback_stress", rid1); - - auto rid2 = table->insert(executor::Tuple({common::Value::make_int64(2), common::Value::make_text("B")}), txn->get_id()); + + auto rid2 = table->insert( + executor::Tuple({common::Value::make_int64(2), common::Value::make_text("B")}), + txn->get_id()); txn->add_undo_log(UndoLog::Type::INSERT, "rollback_stress", rid2); // 2. Update data - table->remove(rid1, txn->get_id()); // Mark old version deleted - auto rid1_new = table->insert(executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A_NEW")}), txn->get_id()); + table->remove(rid1, txn->get_id()); // Mark old version deleted + auto rid1_new = table->insert( + executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A_NEW")}), + txn->get_id()); txn->add_undo_log(UndoLog::Type::UPDATE, "rollback_stress", rid1_new, rid1); // 3. Delete data table->remove(rid2, txn->get_id()); txn->add_undo_log(UndoLog::Type::DELETE, "rollback_stress", rid2); - EXPECT_EQ(table->tuple_count(), 1U); // rid1_new is active, rid1 and rid2 are logically deleted + EXPECT_EQ(table->tuple_count(), 1U); // rid1_new is active, rid1 and rid2 are logically deleted // 4. Abort tm->abort(txn); - txn = nullptr; // Marked as aborted and handled by TearDown if still set + txn = nullptr; // Marked as aborted and handled by TearDown if still set // 5. Verify restoration - EXPECT_EQ(table->tuple_count(), 0U); // Inserted rows should be physically removed or logically invisible - + EXPECT_EQ(table->tuple_count(), + 0U); // Inserted rows should be physically removed or logically invisible + // The table should be empty because we aborted the inserts auto iter = table->scan(); executor::Tuple t; EXPECT_FALSE(iter.next(t)); } -} // namespace +} // namespace diff --git a/tests/utils_coverage_tests.cpp b/tests/utils_coverage_tests.cpp index 3be5e6c..bfea0b1 100644 --- a/tests/utils_coverage_tests.cpp +++ b/tests/utils_coverage_tests.cpp @@ -4,8 +4,10 @@ */ #include + #include #include + #include "common/value.hpp" #include "network/rpc_client.hpp" @@ -19,7 +21,7 @@ namespace { */ TEST(UtilsCoverageTests, ValueEdgeCases) { // 1. Accessor failure paths - Value v_int(ValueType::TYPE_INT32); // Default 0 + Value v_int(ValueType::TYPE_INT32); // Default 0 EXPECT_THROW(v_int.as_bool(), std::runtime_error); EXPECT_THROW(v_int.as_float64(), std::runtime_error); EXPECT_THROW(v_int.as_text(), std::runtime_error); @@ -31,7 +33,7 @@ TEST(UtilsCoverageTests, ValueEdgeCases) { EXPECT_THROW(v_text.as_int32(), std::runtime_error); // 2. boundary to_* conversions - EXPECT_EQ(v_text.to_int64(), 0); // Text doesn't auto-convert to int in to_int64() + EXPECT_EQ(v_text.to_int64(), 0); // Text doesn't auto-convert to int in to_int64() EXPECT_EQ(v_text.to_float64(), 0.0); Value v_null = Value::make_null(); @@ -41,7 +43,7 @@ TEST(UtilsCoverageTests, ValueEdgeCases) { Value v_f(1.23); EXPECT_EQ(v_f.to_int64(), 1); - + // 3. Numeric check EXPECT_TRUE(v_int.is_numeric()); EXPECT_TRUE(v_f.is_numeric()); @@ -59,41 +61,41 @@ TEST(UtilsCoverageTests, ValueComparisons) { Value v_text("A"); // Equality - EXPECT_TRUE(v_int == v_float); // Numeric equality + EXPECT_TRUE(v_int == v_float); // Numeric equality EXPECT_FALSE(v_int == v_text); EXPECT_FALSE(v_int == v_null); // Less than - EXPECT_FALSE(v_null < v_int); // NULL is not less than anything - EXPECT_TRUE(v_int < v_null); // non-NULL is less than NULL (by convention in this impl) - + EXPECT_FALSE(v_null < v_int); // NULL is not less than anything + EXPECT_TRUE(v_int < v_null); // non-NULL is less than NULL (by convention in this impl) + Value v_int_small(5); EXPECT_TRUE(v_int_small < v_float); EXPECT_FALSE(v_float < v_int_small); Value v_text_b("B"); EXPECT_TRUE(v_text < v_text_b); - + // Mixed numeric/non-numeric comparison - EXPECT_FALSE(v_int < v_text); + EXPECT_FALSE(v_int < v_text); } /** * @brief Tests RpcClient behavior on connection failures. - * + * * We use localhost on a reserved/privileged port (1) to trigger an immediate - * "Connection refused" from the OS. This provides a deterministic failure - * without the multi-second timeouts associated with non-routable external - * IP addresses (like 192.0.2.1), ensuring the CI pipeline remains fast while + * "Connection refused" from the OS. This provides a deterministic failure + * without the multi-second timeouts associated with non-routable external + * IP addresses (like 192.0.2.1), ensuring the CI pipeline remains fast while * still covering the error handling logic in RpcClient. */ TEST(UtilsCoverageTests, RpcClientFailure) { // Attempt to connect to an unreachable port on localhost - RpcClient client("127.0.0.1", 1); - + RpcClient client("127.0.0.1", 1); + EXPECT_FALSE(client.connect()); EXPECT_FALSE(client.is_connected()); - + std::vector resp; // These should return false as they require a valid connection EXPECT_FALSE(client.call(RpcType::AppendEntries, {1, 2, 3}, resp)); @@ -115,4 +117,4 @@ TEST(UtilsCoverageTests, ValueHash) { EXPECT_NE(hasher(v1), hasher(v_null)); } -} // namespace +} // namespace From edaa01aeeba583abdecb1f7d3e69a8cd2d690afb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Poyraz=20K=C3=BC=C3=A7=C3=BCkarslan?= <83272398+PoyrazK@users.noreply.github.com> Date: Sun, 15 Mar 2026 16:19:44 +0300 Subject: [PATCH 7/8] test: refactor coverage tests for better resource management and deterministic network failure --- tests/transaction_coverage_tests.cpp | 105 +++++++++++++-------------- tests/utils_coverage_tests.cpp | 36 +++++---- 2 files changed, 69 insertions(+), 72 deletions(-) diff --git a/tests/transaction_coverage_tests.cpp b/tests/transaction_coverage_tests.cpp index 0bc90c1..1a07fad 100644 --- a/tests/transaction_coverage_tests.cpp +++ b/tests/transaction_coverage_tests.cpp @@ -4,18 +4,17 @@ */ #include - #include #include -#include #include #include +#include #include "catalog/catalog.hpp" #include "common/config.hpp" #include "storage/buffer_pool_manager.hpp" -#include "storage/heap_table.hpp" #include "storage/storage_manager.hpp" +#include "storage/heap_table.hpp" #include "transaction/lock_manager.hpp" #include "transaction/transaction.hpp" #include "transaction/transaction_manager.hpp" @@ -31,49 +30,51 @@ namespace { * @brief Fixture for transaction-related coverage tests to ensure proper resource management. */ class TransactionCoverageTests : public ::testing::Test { - protected: +protected: void SetUp() override { - catalog = Catalog::create(); - disk_manager = std::make_unique("./test_data"); - bpm = std::make_unique(config::Config::DEFAULT_BUFFER_POOL_SIZE, - *disk_manager); - lm = std::make_unique(); - tm = std::make_unique(*lm, *catalog, *bpm, nullptr); + catalog_ptr = Catalog::create(); + disk_manager_ptr = std::make_unique("./test_data"); + bpm_ptr = std::make_unique(config::Config::DEFAULT_BUFFER_POOL_SIZE, *disk_manager_ptr); + lm_ptr = std::make_unique(); + tm_ptr = std::make_unique(*lm_ptr, *catalog_ptr, *bpm_ptr, nullptr); std::vector cols = {{"id", common::ValueType::TYPE_INT64, 0}, {"val", common::ValueType::TYPE_TEXT, 1}}; - catalog->create_table("rollback_stress", cols); - + catalog_ptr->create_table("rollback_stress", cols); + executor::Schema schema; schema.add_column("id", common::ValueType::TYPE_INT64); schema.add_column("val", common::ValueType::TYPE_TEXT); - - table = std::make_unique("rollback_stress", *bpm, schema); - table->create(); - + + table_ptr = std::make_unique("rollback_stress", *bpm_ptr, schema); + table_ptr->create(); + txn = nullptr; } void TearDown() override { if (txn != nullptr) { - tm->abort(txn); + tm_ptr->abort(txn); } - table.reset(); - tm.reset(); - lm.reset(); - bpm.reset(); - disk_manager.reset(); - catalog.reset(); - + table_ptr.reset(); + tm_ptr.reset(); + lm_ptr.reset(); + bpm_ptr.reset(); + disk_manager_ptr.reset(); + catalog_ptr.reset(); + static_cast(std::remove("./test_data/rollback_stress.heap")); } - std::unique_ptr catalog; - std::unique_ptr disk_manager; - std::unique_ptr bpm; - std::unique_ptr lm; - std::unique_ptr tm; - std::unique_ptr table; + // Pointers managed by the fixture + std::unique_ptr catalog_ptr; + std::unique_ptr disk_manager_ptr; + std::unique_ptr bpm_ptr; + std::unique_ptr lm_ptr; + std::unique_ptr tm_ptr; + std::unique_ptr table_ptr; + + // Live transaction pointer for cleanup Transaction* txn; }; @@ -88,7 +89,7 @@ TEST(TransactionCoverageTestsStandalone, LockManagerConcurrency) { std::atomic stop{false}; Transaction writer_txn(100); - + // Writers holds exclusive lock initially ASSERT_TRUE(lm.acquire_exclusive(&writer_txn, "RESOURCE")); @@ -111,7 +112,7 @@ TEST(TransactionCoverageTestsStandalone, LockManagerConcurrency) { // Release writer lock, readers should proceed lm.unlock(&writer_txn, "RESOURCE"); - + // Wait for all readers to get the lock for (int i = 0; i < 50 && shared_granted.load() < num_readers; ++i) { std::this_thread::sleep_for(std::chrono::milliseconds(50)); @@ -126,46 +127,44 @@ TEST(TransactionCoverageTestsStandalone, LockManagerConcurrency) { /** * @brief Tests deep rollback functionality via the Undo Log. + * Uses the TransactionCoverageTests fixture for automated cleanup. */ TEST_F(TransactionCoverageTests, DeepRollback) { - txn = tm->begin(); + // Expose symbols to reuse existing test body logic + TransactionManager& tm = *tm_ptr; + HeapTable& table = *table_ptr; + txn = tm.begin(); + // 1. Insert some data - auto rid1 = table->insert( - executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A")}), - txn->get_id()); + auto rid1 = table.insert(executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A")}), txn->get_id()); txn->add_undo_log(UndoLog::Type::INSERT, "rollback_stress", rid1); - - auto rid2 = table->insert( - executor::Tuple({common::Value::make_int64(2), common::Value::make_text("B")}), - txn->get_id()); + + auto rid2 = table.insert(executor::Tuple({common::Value::make_int64(2), common::Value::make_text("B")}), txn->get_id()); txn->add_undo_log(UndoLog::Type::INSERT, "rollback_stress", rid2); // 2. Update data - table->remove(rid1, txn->get_id()); // Mark old version deleted - auto rid1_new = table->insert( - executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A_NEW")}), - txn->get_id()); + table.remove(rid1, txn->get_id()); // Mark old version deleted + auto rid1_new = table.insert(executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A_NEW")}), txn->get_id()); txn->add_undo_log(UndoLog::Type::UPDATE, "rollback_stress", rid1_new, rid1); // 3. Delete data - table->remove(rid2, txn->get_id()); + table.remove(rid2, txn->get_id()); txn->add_undo_log(UndoLog::Type::DELETE, "rollback_stress", rid2); - EXPECT_EQ(table->tuple_count(), 1U); // rid1_new is active, rid1 and rid2 are logically deleted + EXPECT_EQ(table.tuple_count(), 1U); // rid1_new is active, rid1 and rid2 are logically deleted // 4. Abort - tm->abort(txn); - txn = nullptr; // Marked as aborted and handled by TearDown if still set + tm.abort(txn); + txn = nullptr; // Marked as aborted and handled by TearDown if still set // 5. Verify restoration - EXPECT_EQ(table->tuple_count(), - 0U); // Inserted rows should be physically removed or logically invisible - + EXPECT_EQ(table.tuple_count(), 0U); // Inserted rows should be physically removed or logically invisible + // The table should be empty because we aborted the inserts - auto iter = table->scan(); + auto iter = table.scan(); executor::Tuple t; EXPECT_FALSE(iter.next(t)); } -} // namespace +} // namespace diff --git a/tests/utils_coverage_tests.cpp b/tests/utils_coverage_tests.cpp index bfea0b1..3be5e6c 100644 --- a/tests/utils_coverage_tests.cpp +++ b/tests/utils_coverage_tests.cpp @@ -4,10 +4,8 @@ */ #include - #include #include - #include "common/value.hpp" #include "network/rpc_client.hpp" @@ -21,7 +19,7 @@ namespace { */ TEST(UtilsCoverageTests, ValueEdgeCases) { // 1. Accessor failure paths - Value v_int(ValueType::TYPE_INT32); // Default 0 + Value v_int(ValueType::TYPE_INT32); // Default 0 EXPECT_THROW(v_int.as_bool(), std::runtime_error); EXPECT_THROW(v_int.as_float64(), std::runtime_error); EXPECT_THROW(v_int.as_text(), std::runtime_error); @@ -33,7 +31,7 @@ TEST(UtilsCoverageTests, ValueEdgeCases) { EXPECT_THROW(v_text.as_int32(), std::runtime_error); // 2. boundary to_* conversions - EXPECT_EQ(v_text.to_int64(), 0); // Text doesn't auto-convert to int in to_int64() + EXPECT_EQ(v_text.to_int64(), 0); // Text doesn't auto-convert to int in to_int64() EXPECT_EQ(v_text.to_float64(), 0.0); Value v_null = Value::make_null(); @@ -43,7 +41,7 @@ TEST(UtilsCoverageTests, ValueEdgeCases) { Value v_f(1.23); EXPECT_EQ(v_f.to_int64(), 1); - + // 3. Numeric check EXPECT_TRUE(v_int.is_numeric()); EXPECT_TRUE(v_f.is_numeric()); @@ -61,41 +59,41 @@ TEST(UtilsCoverageTests, ValueComparisons) { Value v_text("A"); // Equality - EXPECT_TRUE(v_int == v_float); // Numeric equality + EXPECT_TRUE(v_int == v_float); // Numeric equality EXPECT_FALSE(v_int == v_text); EXPECT_FALSE(v_int == v_null); // Less than - EXPECT_FALSE(v_null < v_int); // NULL is not less than anything - EXPECT_TRUE(v_int < v_null); // non-NULL is less than NULL (by convention in this impl) - + EXPECT_FALSE(v_null < v_int); // NULL is not less than anything + EXPECT_TRUE(v_int < v_null); // non-NULL is less than NULL (by convention in this impl) + Value v_int_small(5); EXPECT_TRUE(v_int_small < v_float); EXPECT_FALSE(v_float < v_int_small); Value v_text_b("B"); EXPECT_TRUE(v_text < v_text_b); - + // Mixed numeric/non-numeric comparison - EXPECT_FALSE(v_int < v_text); + EXPECT_FALSE(v_int < v_text); } /** * @brief Tests RpcClient behavior on connection failures. - * + * * We use localhost on a reserved/privileged port (1) to trigger an immediate - * "Connection refused" from the OS. This provides a deterministic failure - * without the multi-second timeouts associated with non-routable external - * IP addresses (like 192.0.2.1), ensuring the CI pipeline remains fast while + * "Connection refused" from the OS. This provides a deterministic failure + * without the multi-second timeouts associated with non-routable external + * IP addresses (like 192.0.2.1), ensuring the CI pipeline remains fast while * still covering the error handling logic in RpcClient. */ TEST(UtilsCoverageTests, RpcClientFailure) { // Attempt to connect to an unreachable port on localhost - RpcClient client("127.0.0.1", 1); - + RpcClient client("127.0.0.1", 1); + EXPECT_FALSE(client.connect()); EXPECT_FALSE(client.is_connected()); - + std::vector resp; // These should return false as they require a valid connection EXPECT_FALSE(client.call(RpcType::AppendEntries, {1, 2, 3}, resp)); @@ -117,4 +115,4 @@ TEST(UtilsCoverageTests, ValueHash) { EXPECT_NE(hasher(v1), hasher(v_null)); } -} // namespace +} // namespace From 0941745577d05e863ecc75ee52ea817e4ee11ef2 Mon Sep 17 00:00:00 2001 From: poyrazK <83272398+poyrazK@users.noreply.github.com> Date: Tue, 17 Mar 2026 14:23:02 +0000 Subject: [PATCH 8/8] style: automated clang-format fixes --- tests/transaction_coverage_tests.cpp | 53 ++++++++++++++++------------ tests/utils_coverage_tests.cpp | 36 ++++++++++--------- 2 files changed, 50 insertions(+), 39 deletions(-) diff --git a/tests/transaction_coverage_tests.cpp b/tests/transaction_coverage_tests.cpp index 1a07fad..0cfbad8 100644 --- a/tests/transaction_coverage_tests.cpp +++ b/tests/transaction_coverage_tests.cpp @@ -4,17 +4,18 @@ */ #include + #include #include +#include #include #include -#include #include "catalog/catalog.hpp" #include "common/config.hpp" #include "storage/buffer_pool_manager.hpp" -#include "storage/storage_manager.hpp" #include "storage/heap_table.hpp" +#include "storage/storage_manager.hpp" #include "transaction/lock_manager.hpp" #include "transaction/transaction.hpp" #include "transaction/transaction_manager.hpp" @@ -30,25 +31,26 @@ namespace { * @brief Fixture for transaction-related coverage tests to ensure proper resource management. */ class TransactionCoverageTests : public ::testing::Test { -protected: + protected: void SetUp() override { catalog_ptr = Catalog::create(); disk_manager_ptr = std::make_unique("./test_data"); - bpm_ptr = std::make_unique(config::Config::DEFAULT_BUFFER_POOL_SIZE, *disk_manager_ptr); + bpm_ptr = std::make_unique(config::Config::DEFAULT_BUFFER_POOL_SIZE, + *disk_manager_ptr); lm_ptr = std::make_unique(); tm_ptr = std::make_unique(*lm_ptr, *catalog_ptr, *bpm_ptr, nullptr); std::vector cols = {{"id", common::ValueType::TYPE_INT64, 0}, {"val", common::ValueType::TYPE_TEXT, 1}}; catalog_ptr->create_table("rollback_stress", cols); - + executor::Schema schema; schema.add_column("id", common::ValueType::TYPE_INT64); schema.add_column("val", common::ValueType::TYPE_TEXT); - + table_ptr = std::make_unique("rollback_stress", *bpm_ptr, schema); table_ptr->create(); - + txn = nullptr; } @@ -62,7 +64,7 @@ class TransactionCoverageTests : public ::testing::Test { bpm_ptr.reset(); disk_manager_ptr.reset(); catalog_ptr.reset(); - + static_cast(std::remove("./test_data/rollback_stress.heap")); } @@ -73,7 +75,7 @@ class TransactionCoverageTests : public ::testing::Test { std::unique_ptr lm_ptr; std::unique_ptr tm_ptr; std::unique_ptr table_ptr; - + // Live transaction pointer for cleanup Transaction* txn; }; @@ -89,7 +91,7 @@ TEST(TransactionCoverageTestsStandalone, LockManagerConcurrency) { std::atomic stop{false}; Transaction writer_txn(100); - + // Writers holds exclusive lock initially ASSERT_TRUE(lm.acquire_exclusive(&writer_txn, "RESOURCE")); @@ -112,7 +114,7 @@ TEST(TransactionCoverageTestsStandalone, LockManagerConcurrency) { // Release writer lock, readers should proceed lm.unlock(&writer_txn, "RESOURCE"); - + // Wait for all readers to get the lock for (int i = 0; i < 50 && shared_granted.load() < num_readers; ++i) { std::this_thread::sleep_for(std::chrono::milliseconds(50)); @@ -135,36 +137,43 @@ TEST_F(TransactionCoverageTests, DeepRollback) { HeapTable& table = *table_ptr; txn = tm.begin(); - + // 1. Insert some data - auto rid1 = table.insert(executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A")}), txn->get_id()); + auto rid1 = + table.insert(executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A")}), + txn->get_id()); txn->add_undo_log(UndoLog::Type::INSERT, "rollback_stress", rid1); - - auto rid2 = table.insert(executor::Tuple({common::Value::make_int64(2), common::Value::make_text("B")}), txn->get_id()); + + auto rid2 = + table.insert(executor::Tuple({common::Value::make_int64(2), common::Value::make_text("B")}), + txn->get_id()); txn->add_undo_log(UndoLog::Type::INSERT, "rollback_stress", rid2); // 2. Update data - table.remove(rid1, txn->get_id()); // Mark old version deleted - auto rid1_new = table.insert(executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A_NEW")}), txn->get_id()); + table.remove(rid1, txn->get_id()); // Mark old version deleted + auto rid1_new = table.insert( + executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A_NEW")}), + txn->get_id()); txn->add_undo_log(UndoLog::Type::UPDATE, "rollback_stress", rid1_new, rid1); // 3. Delete data table.remove(rid2, txn->get_id()); txn->add_undo_log(UndoLog::Type::DELETE, "rollback_stress", rid2); - EXPECT_EQ(table.tuple_count(), 1U); // rid1_new is active, rid1 and rid2 are logically deleted + EXPECT_EQ(table.tuple_count(), 1U); // rid1_new is active, rid1 and rid2 are logically deleted // 4. Abort tm.abort(txn); - txn = nullptr; // Marked as aborted and handled by TearDown if still set + txn = nullptr; // Marked as aborted and handled by TearDown if still set // 5. Verify restoration - EXPECT_EQ(table.tuple_count(), 0U); // Inserted rows should be physically removed or logically invisible - + EXPECT_EQ(table.tuple_count(), + 0U); // Inserted rows should be physically removed or logically invisible + // The table should be empty because we aborted the inserts auto iter = table.scan(); executor::Tuple t; EXPECT_FALSE(iter.next(t)); } -} // namespace +} // namespace diff --git a/tests/utils_coverage_tests.cpp b/tests/utils_coverage_tests.cpp index 3be5e6c..bfea0b1 100644 --- a/tests/utils_coverage_tests.cpp +++ b/tests/utils_coverage_tests.cpp @@ -4,8 +4,10 @@ */ #include + #include #include + #include "common/value.hpp" #include "network/rpc_client.hpp" @@ -19,7 +21,7 @@ namespace { */ TEST(UtilsCoverageTests, ValueEdgeCases) { // 1. Accessor failure paths - Value v_int(ValueType::TYPE_INT32); // Default 0 + Value v_int(ValueType::TYPE_INT32); // Default 0 EXPECT_THROW(v_int.as_bool(), std::runtime_error); EXPECT_THROW(v_int.as_float64(), std::runtime_error); EXPECT_THROW(v_int.as_text(), std::runtime_error); @@ -31,7 +33,7 @@ TEST(UtilsCoverageTests, ValueEdgeCases) { EXPECT_THROW(v_text.as_int32(), std::runtime_error); // 2. boundary to_* conversions - EXPECT_EQ(v_text.to_int64(), 0); // Text doesn't auto-convert to int in to_int64() + EXPECT_EQ(v_text.to_int64(), 0); // Text doesn't auto-convert to int in to_int64() EXPECT_EQ(v_text.to_float64(), 0.0); Value v_null = Value::make_null(); @@ -41,7 +43,7 @@ TEST(UtilsCoverageTests, ValueEdgeCases) { Value v_f(1.23); EXPECT_EQ(v_f.to_int64(), 1); - + // 3. Numeric check EXPECT_TRUE(v_int.is_numeric()); EXPECT_TRUE(v_f.is_numeric()); @@ -59,41 +61,41 @@ TEST(UtilsCoverageTests, ValueComparisons) { Value v_text("A"); // Equality - EXPECT_TRUE(v_int == v_float); // Numeric equality + EXPECT_TRUE(v_int == v_float); // Numeric equality EXPECT_FALSE(v_int == v_text); EXPECT_FALSE(v_int == v_null); // Less than - EXPECT_FALSE(v_null < v_int); // NULL is not less than anything - EXPECT_TRUE(v_int < v_null); // non-NULL is less than NULL (by convention in this impl) - + EXPECT_FALSE(v_null < v_int); // NULL is not less than anything + EXPECT_TRUE(v_int < v_null); // non-NULL is less than NULL (by convention in this impl) + Value v_int_small(5); EXPECT_TRUE(v_int_small < v_float); EXPECT_FALSE(v_float < v_int_small); Value v_text_b("B"); EXPECT_TRUE(v_text < v_text_b); - + // Mixed numeric/non-numeric comparison - EXPECT_FALSE(v_int < v_text); + EXPECT_FALSE(v_int < v_text); } /** * @brief Tests RpcClient behavior on connection failures. - * + * * We use localhost on a reserved/privileged port (1) to trigger an immediate - * "Connection refused" from the OS. This provides a deterministic failure - * without the multi-second timeouts associated with non-routable external - * IP addresses (like 192.0.2.1), ensuring the CI pipeline remains fast while + * "Connection refused" from the OS. This provides a deterministic failure + * without the multi-second timeouts associated with non-routable external + * IP addresses (like 192.0.2.1), ensuring the CI pipeline remains fast while * still covering the error handling logic in RpcClient. */ TEST(UtilsCoverageTests, RpcClientFailure) { // Attempt to connect to an unreachable port on localhost - RpcClient client("127.0.0.1", 1); - + RpcClient client("127.0.0.1", 1); + EXPECT_FALSE(client.connect()); EXPECT_FALSE(client.is_connected()); - + std::vector resp; // These should return false as they require a valid connection EXPECT_FALSE(client.call(RpcType::AppendEntries, {1, 2, 3}, resp)); @@ -115,4 +117,4 @@ TEST(UtilsCoverageTests, ValueHash) { EXPECT_NE(hasher(v1), hasher(v_null)); } -} // namespace +} // namespace