Skip to content

test(catalog): add 7 new unit tests #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
poyrazK merged 2 commits into from
Apr 15, 2026
+119 −0
Merged

test(catalog): add 7 new unit tests #38

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a73fd1b
test(catalog): add 7 new unit tests
poyrazK Apr 15, 2026
92cccf9
style: automated clang-format fixes
poyrazK Apr 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 119 additions & 0 deletions tests/catalog_coverage_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,4 +155,123 @@ TEST(CatalogCoverageTests, RaftApply) {
catalog->apply(entry);
}

// ============= New Tests =============

/**
* @brief Tests basic table creation and retrieval by ID
*/
TEST(CatalogCoverageTests, CreateAndGetTable) {
auto catalog = Catalog::create();
std::vector<ColumnInfo> cols = {{"id", common::ValueType::TYPE_INT64, 0},
{"name", common::ValueType::TYPE_TEXT, 1}};

oid_t tid = catalog->create_table("users", cols);
ASSERT_NE(tid, 0);

// Retrieve by ID
auto table_opt = catalog->get_table(tid);
ASSERT_TRUE(table_opt.has_value());
EXPECT_EQ((*table_opt)->name, "users");
EXPECT_EQ((*table_opt)->num_columns(), 2);
}

/**
* @brief Tests table creation and retrieval by name
*/
TEST(CatalogCoverageTests, CreateAndGetTableByName) {
auto catalog = Catalog::create();
std::vector<ColumnInfo> cols = {{"id", common::ValueType::TYPE_INT64, 0}};

oid_t tid = catalog->create_table("products", cols);
ASSERT_NE(tid, 0);

// Retrieve by name
auto table_opt = catalog->get_table_by_name("products");
ASSERT_TRUE(table_opt.has_value());
EXPECT_EQ((*table_opt)->table_id, tid);
}

/**
* @brief Tests get_all_tables returns created tables
*/
TEST(CatalogCoverageTests, GetAllTables) {
auto catalog = Catalog::create();
std::vector<ColumnInfo> cols = {{"id", common::ValueType::TYPE_INT64, 0}};

catalog->create_table("table1", cols);
catalog->create_table("table2", cols);
catalog->create_table("table3", cols);

auto tables = catalog->get_all_tables();
EXPECT_EQ(tables.size(), 3);
}

/**
* @brief Tests get_table_indexes returns created indexes
*/
TEST(CatalogCoverageTests, GetTableIndexes) {
auto catalog = Catalog::create();
std::vector<ColumnInfo> cols = {{"id", common::ValueType::TYPE_INT64, 0}};

oid_t tid = catalog->create_table("indexed_table", cols);
ASSERT_NE(tid, 0);

catalog->create_index("idx1", tid, {0}, IndexType::BTree, false);
catalog->create_index("idx2", tid, {0}, IndexType::Hash, true);

auto indexes = catalog->get_table_indexes(tid);
EXPECT_EQ(indexes.size(), 2);
}

/**
* @brief Tests catalog save and load functionality
*
* Note: save() and load() are stubs that don't fully persist table data.
* save() writes a header comment but no table data.
* load() reads but doesn't parse table entries.
*/
TEST(CatalogCoverageTests, SaveAndLoad) {
auto catalog = Catalog::create();
std::vector<ColumnInfo> cols = {{"id", common::ValueType::TYPE_INT64, 0}};
catalog->create_table("persisted_table", cols);

// Save catalog - should succeed
ASSERT_TRUE(catalog->save("/tmp/test_catalog.bin"));

// Create new catalog and load - should succeed (returns true)
auto loaded_catalog = Catalog::create();
ASSERT_TRUE(loaded_catalog->load("/tmp/test_catalog.bin"));

// Note: Due to stub implementation, loaded catalog won't have the table
// This test verifies the save/load cycle works without crashing

// Cleanup
std::remove("/tmp/test_catalog.bin");
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify std::remove usage and direct header presence in this test TU.
rg -n 'std::remove\(' tests/catalog_coverage_tests.cpp -C2
rg -n '^#include <cstdio>' tests/catalog_coverage_tests.cpp -C1

Repository: poyrazK/cloudSQL

Length of output: 140

🏁 Script executed:

rg -n '^#include' tests/catalog_coverage_tests.cpp

Repository: poyrazK/cloudSQL

Length of output: 233

🏁 Script executed:

rg -n '#include <cstdio>' catalog/catalog.hpp common/value.hpp distributed/raft_types.hpp

Repository: poyrazK/cloudSQL

Length of output: 243

🏁 Script executed:

fd -type f 'catalog\.hpp|value\.hpp|raft_types\.hpp' --max-one-result

Repository: poyrazK/cloudSQL

Length of output: 230

🏁 Script executed:

fd 'catalog\.hpp|value\.hpp|raft_types\.hpp'

Repository: poyrazK/cloudSQL

Length of output: 147

🏁 Script executed:

rg '#include <cstdio>' include/catalog/catalog.hpp include/common/value.hpp include/distributed/raft_types.hpp

Repository: poyrazK/cloudSQL

Length of output: 102

Add direct <cstdio> include for std::remove.

Line 249 calls std::remove; while the C standard library function is currently accessible through a transitive include from "common/value.hpp", relying on transitive includes is brittle and can break if that header's dependencies change. Adding an explicit #include <cstdio> makes the dependency clear and robust.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/catalog_coverage_tests.cpp` at line 249, The test uses std::remove but
doesn't include the C stdio header directly; add a direct `#include` <cstdio> at
the top of the test file so std::remove is provided explicitly (ensure the
include is placed with other standard headers and before usage of std::remove to
avoid relying on transitive includes from common/value.hpp).

}
Comment on lines +233 to +250
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid fixed temp filename in SaveAndLoad to prevent test flakiness.

Using "/tmp/test_catalog.bin" on Line [239], Line [243], and Line [249] introduces cross-test collisions in parallel runs and is platform-fragile.

Proposed fix 
+#include <chrono>
+#include <filesystem>
+
 TEST(CatalogCoverageTests, SaveAndLoad) {
     auto catalog = Catalog::create();
     std::vector<ColumnInfo> cols = {{"id", common::ValueType::TYPE_INT64, 0}};
     catalog->create_table("persisted_table", cols);

+    const auto unique_suffix =
+        std::to_string(std::chrono::steady_clock::now().time_since_epoch().count());
+    const auto test_file =
+        (std::filesystem::temp_directory_path() / ("test_catalog_" + unique_suffix + ".bin"))
+            .string();
+
     // Save catalog - should succeed
-    ASSERT_TRUE(catalog->save("/tmp/test_catalog.bin"));
+    ASSERT_TRUE(catalog->save(test_file));

     // Create new catalog and load - should succeed (returns true)
     auto loaded_catalog = Catalog::create();
-    ASSERT_TRUE(loaded_catalog->load("/tmp/test_catalog.bin"));
+    ASSERT_TRUE(loaded_catalog->load(test_file));

     // Note: Due to stub implementation, loaded catalog won't have the table
     // This test verifies the save/load cycle works without crashing

     // Cleanup
-    std::remove("/tmp/test_catalog.bin");
+    EXPECT_EQ(std::remove(test_file.c_str()), 0);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
TEST(CatalogCoverageTests, SaveAndLoad) {
auto catalog = Catalog::create();
std::vector<ColumnInfo> cols = {{"id", common::ValueType::TYPE_INT64, 0}};
catalog->create_table("persisted_table", cols);
// Save catalog - should succeed
ASSERT_TRUE(catalog->save("/tmp/test_catalog.bin"));
// Create new catalog and load - should succeed (returns true)
auto loaded_catalog = Catalog::create();
ASSERT_TRUE(loaded_catalog->load("/tmp/test_catalog.bin"));
// Note: Due to stub implementation, loaded catalog won't have the table
// This test verifies the save/load cycle works without crashing
// Cleanup
std::remove("/tmp/test_catalog.bin");
}
`#include` <chrono>
`#include` <filesystem>
TEST(CatalogCoverageTests, SaveAndLoad) {
auto catalog = Catalog::create();
std::vector<ColumnInfo> cols = {{"id", common::ValueType::TYPE_INT64, 0}};
catalog->create_table("persisted_table", cols);
const auto unique_suffix =
std::to_string(std::chrono::steady_clock::now().time_since_epoch().count());
const auto test_file =
(std::filesystem::temp_directory_path() / ("test_catalog_" + unique_suffix + ".bin"))
.string();
// Save catalog - should succeed
ASSERT_TRUE(catalog->save(test_file));
// Create new catalog and load - should succeed (returns true)
auto loaded_catalog = Catalog::create();
ASSERT_TRUE(loaded_catalog->load(test_file));
// Note: Due to stub implementation, loaded catalog won't have the table
// This test verifies the save/load cycle works without crashing
// Cleanup
EXPECT_EQ(std::remove(test_file.c_str()), 0);
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/catalog_coverage_tests.cpp` around lines 233 - 250, The test
TEST(CatalogCoverageTests, SaveAndLoad) uses a fixed path
"/tmp/test_catalog.bin" for catalog->save and loaded_catalog->load which can
collide in parallel runs; replace the hard-coded filename with a unique
temporary path (e.g., use std::filesystem::temp_directory_path() combined with
std::filesystem::unique_path("test_catalog_%%%%-%%%%.bin") to produce a unique
filename), store it in a local variable (temp_path.string()), use that variable
in catalog->save and loaded_catalog->load, and call
std::filesystem::remove(temp_path) for cleanup; ensure you add the necessary
`#include` <filesystem> and use std::filesystem::path for the temp file handling.


/**
* @brief Tests version increments after catalog operations
*/
TEST(CatalogCoverageTests, VersionIncrement) {
auto catalog = Catalog::create();
uint64_t initial_version = catalog->get_version();

std::vector<ColumnInfo> cols = {{"id", common::ValueType::TYPE_INT64, 0}};
catalog->create_table("versioned_table", cols);

EXPECT_GT(catalog->get_version(), initial_version);
}

/**
* @brief Tests that print() doesn't crash
*/
TEST(CatalogCoverageTests, PrintDoesNotCrash) {
auto catalog = Catalog::create();
std::vector<ColumnInfo> cols = {{"id", common::ValueType::TYPE_INT64, 0}};
catalog->create_table("printed_table", cols);

// Should not throw or crash
EXPECT_NO_THROW(catalog->print());
}

} // namespace