Skip to content

Commit d023edd

Browse files
poyrazKpoyrazK
authored
Merge pull request #15 from poyrazK/feature/improve-coverage
Improve Test Coverage to >85%
2 parents 6c8ea98 + 0941745 commit d023edd

4 files changed

Lines changed: 460 additions & 0 deletions

File tree

‎CMakeLists.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,9 @@ endmacro()
8484
# Tests
8585
if(BUILD_TESTS)
8686
enable_testing()
87+
add_cloudsql_test(catalog_coverage_tests tests/catalog_coverage_tests.cpp)
88+
add_cloudsql_test(transaction_coverage_tests tests/transaction_coverage_tests.cpp)
89+
add_cloudsql_test(utils_coverage_tests tests/utils_coverage_tests.cpp)
8790
add_cloudsql_test(cloudSQL_tests tests/cloudSQL_tests.cpp)
8891
add_cloudsql_test(server_tests tests/server_tests.cpp)
8992
add_cloudsql_test(statement_tests tests/statement_tests.cpp)

‎tests/catalog_coverage_tests.cpp‎

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
/**
2+
* @file catalog_coverage_tests.cpp
3+
* @brief Targeted unit tests to increase coverage of the Catalog module
4+
*/
5+
6+
#include <gtest/gtest.h>
7+
8+
#include <cstring>
9+
#include <vector>
10+
11+
#include "catalog/catalog.hpp"
12+
#include "common/value.hpp"
13+
#include "distributed/raft_types.hpp"
14+
15+
using namespace cloudsql;
16+
17+
namespace {
18+
19+
/**
20+
* @brief Tests Catalog behavior with missing entities and invalid lookups.
21+
*/
22+
TEST(CatalogCoverageTests, MissingEntities) {
23+
auto catalog = Catalog::create();
24+
25+
// Invalid table lookup
26+
EXPECT_FALSE(catalog->get_table(9999).has_value());
27+
EXPECT_FALSE(catalog->table_exists(9999));
28+
EXPECT_FALSE(catalog->table_exists_by_name("non_existent"));
29+
EXPECT_FALSE(catalog->get_table_by_name("non_existent").has_value());
30+
31+
// Invalid index lookup
32+
EXPECT_FALSE(catalog->get_index(8888).has_value());
33+
EXPECT_TRUE(catalog->get_table_indexes(9999).empty());
34+
35+
// Dropping non-existent entities
36+
EXPECT_FALSE(catalog->drop_table(9999));
37+
EXPECT_FALSE(catalog->drop_index(8888));
38+
39+
// Update stats for non-existent table
40+
EXPECT_FALSE(catalog->update_table_stats(9999, 100));
41+
}
42+
43+
/**
44+
* @brief Tests Catalog behavior with duplicate entities and creation edge cases.
45+
*/
46+
TEST(CatalogCoverageTests, DuplicateEntities) {
47+
auto catalog = Catalog::create();
48+
std::vector<ColumnInfo> cols = {{"id", common::ValueType::TYPE_INT64, 0}};
49+
50+
oid_t tid = catalog->create_table("test_table", cols);
51+
ASSERT_NE(tid, 0);
52+
53+
// Duplicate table creation should throw
54+
EXPECT_THROW(catalog->create_table("test_table", cols), std::runtime_error);
55+
56+
// Create an index
57+
oid_t iid = catalog->create_index("idx_id", tid, {0}, IndexType::BTree, true);
58+
ASSERT_NE(iid, 0);
59+
60+
// Duplicate index creation should throw
61+
EXPECT_THROW(catalog->create_index("idx_id", tid, {0}, IndexType::BTree, true),
62+
std::runtime_error);
63+
64+
// Creating index on missing table
65+
EXPECT_EQ(catalog->create_index("idx_missing", 9999, {0}, IndexType::BTree, false), 0);
66+
}
67+
68+
/**
69+
* @brief Helper to serialize CreateTable command for Raft simulation
70+
*/
71+
std::vector<uint8_t> serialize_create_table(const std::string& name,
72+
const std::vector<ColumnInfo>& columns) {
73+
std::vector<uint8_t> data;
74+
data.push_back(1); // Type 1
75+
76+
uint32_t name_len = name.size();
77+
size_t off = data.size();
78+
data.resize(off + 4 + name_len);
79+
std::memcpy(data.data() + off, &name_len, 4);
80+
std::memcpy(data.data() + off + 4, name.data(), name_len);
81+
82+
uint32_t col_count = columns.size();
83+
off = data.size();
84+
data.resize(off + 4);
85+
std::memcpy(data.data() + off, &col_count, 4);
86+
87+
for (const auto& col : columns) {
88+
uint32_t cname_len = col.name.size();
89+
off = data.size();
90+
data.resize(off + 4 + cname_len + 1 + 2);
91+
std::memcpy(data.data() + off, &cname_len, 4);
92+
std::memcpy(data.data() + off + 4, col.name.data(), cname_len);
93+
data[off + 4 + cname_len] = static_cast<uint8_t>(col.type);
94+
std::memcpy(data.data() + off + 4 + cname_len + 1, &col.position, 2);
95+
}
96+
97+
uint32_t shard_count = 1;
98+
off = data.size();
99+
data.resize(off + 4);
100+
std::memcpy(data.data() + off, &shard_count, 4);
101+
102+
std::string addr = "127.0.0.1";
103+
uint32_t addr_len = addr.size();
104+
uint32_t sid = 0;
105+
uint16_t port = 6441;
106+
107+
off = data.size();
108+
data.resize(off + 4 + addr_len + 4 + 2);
109+
std::memcpy(data.data() + off, &addr_len, 4);
110+
std::memcpy(data.data() + off + 4, addr.data(), addr_len);
111+
std::memcpy(data.data() + off + 4 + addr_len, &sid, 4);
112+
std::memcpy(data.data() + off + 4 + addr_len + 4, &port, 2);
113+
114+
return data;
115+
}
116+
117+
/**
118+
* @brief Tests the Raft state machine application (apply) in the Catalog.
119+
*/
120+
TEST(CatalogCoverageTests, RaftApply) {
121+
auto catalog = Catalog::create();
122+
123+
// 1. Replay CreateTable
124+
std::vector<ColumnInfo> cols = {{"id", common::ValueType::TYPE_INT64, 0}};
125+
std::vector<uint8_t> create_data = serialize_create_table("raft_table", cols);
126+
127+
raft::LogEntry entry;
128+
entry.term = 1;
129+
entry.index = 1;
130+
entry.data = create_data;
131+
132+
catalog->apply(entry);
133+
EXPECT_TRUE(catalog->table_exists_by_name("raft_table"));
134+
135+
auto table_opt = catalog->get_table_by_name("raft_table");
136+
ASSERT_TRUE(table_opt.has_value());
137+
oid_t tid = (*table_opt)->table_id;
138+
139+
// 2. Replay DropTable
140+
std::vector<uint8_t> drop_data;
141+
drop_data.push_back(2); // Type 2
142+
drop_data.resize(5);
143+
std::memcpy(drop_data.data() + 1, &tid, 4);
144+
145+
entry.index = 2;
146+
entry.data = drop_data;
147+
148+
catalog->apply(entry);
149+
EXPECT_FALSE(catalog->table_exists(tid));
150+
EXPECT_FALSE(catalog->table_exists_by_name("raft_table"));
151+
152+
// 3. Replay with empty data (should do nothing)
153+
entry.index = 3;
154+
entry.data.clear();
155+
catalog->apply(entry);
156+
}
157+
158+
} // namespace

‎tests/transaction_coverage_tests.cpp‎

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
/**
2+
* @file transaction_coverage_tests.cpp
3+
* @brief Targeted unit tests to increase coverage of Transaction and Lock Manager
4+
*/
5+
6+
#include <gtest/gtest.h>
7+
8+
#include <atomic>
9+
#include <chrono>
10+
#include <cstdio>
11+
#include <thread>
12+
#include <vector>
13+
14+
#include "catalog/catalog.hpp"
15+
#include "common/config.hpp"
16+
#include "storage/buffer_pool_manager.hpp"
17+
#include "storage/heap_table.hpp"
18+
#include "storage/storage_manager.hpp"
19+
#include "transaction/lock_manager.hpp"
20+
#include "transaction/transaction.hpp"
21+
#include "transaction/transaction_manager.hpp"
22+
23+
using namespace cloudsql;
24+
using namespace cloudsql::transaction;
25+
using namespace cloudsql::storage;
26+
27+
namespace {
28+
29+
/**
30+
* @class TransactionCoverageTests
31+
* @brief Fixture for transaction-related coverage tests to ensure proper resource management.
32+
*/
33+
class TransactionCoverageTests : public ::testing::Test {
34+
protected:
35+
void SetUp() override {
36+
catalog_ptr = Catalog::create();
37+
disk_manager_ptr = std::make_unique<StorageManager>("./test_data");
38+
bpm_ptr = std::make_unique<BufferPoolManager>(config::Config::DEFAULT_BUFFER_POOL_SIZE,
39+
*disk_manager_ptr);
40+
lm_ptr = std::make_unique<LockManager>();
41+
tm_ptr = std::make_unique<TransactionManager>(*lm_ptr, *catalog_ptr, *bpm_ptr, nullptr);
42+
43+
std::vector<ColumnInfo> cols = {{"id", common::ValueType::TYPE_INT64, 0},
44+
{"val", common::ValueType::TYPE_TEXT, 1}};
45+
catalog_ptr->create_table("rollback_stress", cols);
46+
47+
executor::Schema schema;
48+
schema.add_column("id", common::ValueType::TYPE_INT64);
49+
schema.add_column("val", common::ValueType::TYPE_TEXT);
50+
51+
table_ptr = std::make_unique<HeapTable>("rollback_stress", *bpm_ptr, schema);
52+
table_ptr->create();
53+
54+
txn = nullptr;
55+
}
56+
57+
void TearDown() override {
58+
if (txn != nullptr) {
59+
tm_ptr->abort(txn);
60+
}
61+
table_ptr.reset();
62+
tm_ptr.reset();
63+
lm_ptr.reset();
64+
bpm_ptr.reset();
65+
disk_manager_ptr.reset();
66+
catalog_ptr.reset();
67+
68+
static_cast<void>(std::remove("./test_data/rollback_stress.heap"));
69+
}
70+
71+
// Pointers managed by the fixture
72+
std::unique_ptr<Catalog> catalog_ptr;
73+
std::unique_ptr<StorageManager> disk_manager_ptr;
74+
std::unique_ptr<BufferPoolManager> bpm_ptr;
75+
std::unique_ptr<LockManager> lm_ptr;
76+
std::unique_ptr<TransactionManager> tm_ptr;
77+
std::unique_ptr<HeapTable> table_ptr;
78+
79+
// Live transaction pointer for cleanup
80+
Transaction* txn;
81+
};
82+
83+
/**
84+
* @brief Stress tests the LockManager with concurrent shared and exclusive requests.
85+
*/
86+
TEST(TransactionCoverageTestsStandalone, LockManagerConcurrency) {
87+
LockManager lm;
88+
const int num_readers = 5;
89+
std::vector<std::thread> readers;
90+
std::atomic<int> shared_granted{0};
91+
std::atomic<bool> stop{false};
92+
93+
Transaction writer_txn(100);
94+
95+
// Writers holds exclusive lock initially
96+
ASSERT_TRUE(lm.acquire_exclusive(&writer_txn, "RESOURCE"));
97+
98+
for (int i = 0; i < num_readers; ++i) {
99+
readers.emplace_back([&, i]() {
100+
Transaction reader_txn(i);
101+
if (lm.acquire_shared(&reader_txn, "RESOURCE")) {
102+
shared_granted++;
103+
while (!stop) {
104+
std::this_thread::yield();
105+
}
106+
lm.unlock(&reader_txn, "RESOURCE");
107+
}
108+
});
109+
}
110+
111+
// Readers should be blocked by the writer
112+
std::this_thread::sleep_for(std::chrono::milliseconds(200));
113+
EXPECT_EQ(shared_granted.load(), 0);
114+
115+
// Release writer lock, readers should proceed
116+
lm.unlock(&writer_txn, "RESOURCE");
117+
118+
// Wait for all readers to get the lock
119+
for (int i = 0; i < 50 && shared_granted.load() < num_readers; ++i) {
120+
std::this_thread::sleep_for(std::chrono::milliseconds(50));
121+
}
122+
EXPECT_EQ(shared_granted.load(), num_readers);
123+
124+
stop = true;
125+
for (auto& t : readers) {
126+
t.join();
127+
}
128+
}
129+
130+
/**
131+
* @brief Tests deep rollback functionality via the Undo Log.
132+
* Uses the TransactionCoverageTests fixture for automated cleanup.
133+
*/
134+
TEST_F(TransactionCoverageTests, DeepRollback) {
135+
// Expose symbols to reuse existing test body logic
136+
TransactionManager& tm = *tm_ptr;
137+
HeapTable& table = *table_ptr;
138+
139+
txn = tm.begin();
140+
141+
// 1. Insert some data
142+
auto rid1 =
143+
table.insert(executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A")}),
144+
txn->get_id());
145+
txn->add_undo_log(UndoLog::Type::INSERT, "rollback_stress", rid1);
146+
147+
auto rid2 =
148+
table.insert(executor::Tuple({common::Value::make_int64(2), common::Value::make_text("B")}),
149+
txn->get_id());
150+
txn->add_undo_log(UndoLog::Type::INSERT, "rollback_stress", rid2);
151+
152+
// 2. Update data
153+
table.remove(rid1, txn->get_id()); // Mark old version deleted
154+
auto rid1_new = table.insert(
155+
executor::Tuple({common::Value::make_int64(1), common::Value::make_text("A_NEW")}),
156+
txn->get_id());
157+
txn->add_undo_log(UndoLog::Type::UPDATE, "rollback_stress", rid1_new, rid1);
158+
159+
// 3. Delete data
160+
table.remove(rid2, txn->get_id());
161+
txn->add_undo_log(UndoLog::Type::DELETE, "rollback_stress", rid2);
162+
163+
EXPECT_EQ(table.tuple_count(), 1U); // rid1_new is active, rid1 and rid2 are logically deleted
164+
165+
// 4. Abort
166+
tm.abort(txn);
167+
txn = nullptr; // Marked as aborted and handled by TearDown if still set
168+
169+
// 5. Verify restoration
170+
EXPECT_EQ(table.tuple_count(),
171+
0U); // Inserted rows should be physically removed or logically invisible
172+
173+
// The table should be empty because we aborted the inserts
174+
auto iter = table.scan();
175+
executor::Tuple t;
176+
EXPECT_FALSE(iter.next(t));
177+
}
178+
179+
} // namespace

0 commit comments

Comments
 (0)