test(storage_manager): add 9 new unit tests#36
Conversation
- PageSizeConstant: verify PAGE_SIZE == 4096 - ReadNonOpenedFileAutoOpens: verify read_page auto-opens files - WriteNonOpenedFileAutoOpens: verify write_page auto-opens files - DataPersistenceAcrossOpenClose: verify data persists across open/close - StatsBytesAccurate: verify bytes_read and bytes_written stats - MultipleFilesDataIsolation: verify data isolation between files - DeallocatePageStub: verify deallocate_page doesn't crash - ReadAfterWriteDifferentPatterns: verify complex patterns persist - FilesOpenedStatAccurate: verify files_opened stat increments
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 42 minutes and 11 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA comprehensive test suite was added to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/storage_manager_tests.cpp (1)
248-268: Strengthen auto-open tests with side-effect checksThese two tests only verify a
truereturn value. Add postconditions (file_existsandfiles_openeddelta) so they prove auto-open behavior, not just success.Suggested test hardening
TEST_F(StorageManagerTests, ReadNonOpenedFileAutoOpens) { const std::string filename = "non_opened_read.db"; cleanup_file("./test_data", filename); + const auto& stats = sm_->get_stats(); + const auto initial_files_opened = stats.files_opened.load(); + // StorageManager auto-opens files, so read should succeed char buf[StorageManager::PAGE_SIZE]; - EXPECT_TRUE(sm_->read_page(filename, 0, buf)); + ASSERT_TRUE(sm_->read_page(filename, 0, buf)); + EXPECT_TRUE(sm_->file_exists(filename)); + EXPECT_EQ(stats.files_opened.load(), initial_files_opened + 1); } TEST_F(StorageManagerTests, WriteNonOpenedFileAutoOpens) { const std::string filename = "non_opened_write.db"; cleanup_file("./test_data", filename); + const auto& stats = sm_->get_stats(); + const auto initial_files_opened = stats.files_opened.load(); + char buf[StorageManager::PAGE_SIZE]; std::memset(buf, 0xAB, StorageManager::PAGE_SIZE); // StorageManager auto-opens files, so write should succeed - EXPECT_TRUE(sm_->write_page(filename, 0, buf)); + ASSERT_TRUE(sm_->write_page(filename, 0, buf)); + EXPECT_TRUE(sm_->file_exists(filename)); + EXPECT_EQ(stats.files_opened.load(), initial_files_opened + 1); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/storage_manager_tests.cpp` around lines 248 - 268, Update the two tests ReadNonOpenedFileAutoOpens and WriteNonOpenedFileAutoOpens to assert the side-effects of auto-opening: before calling sm_->read_page / sm_->write_page capture the current open-file count (e.g., auto before = sm_->files_opened();) then after the operation assert the file was created on disk (use file_exists("./test_data", filename)) and that the open-file count increased by 1 (EXPECT_EQ(sm_->files_opened(), before + 1)); keep existing buffer checks and cleanup_file calls intact and use StorageManager::PAGE_SIZE for buffer sizing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/storage_manager_tests.cpp`:
- Around line 248-268: Update the two tests ReadNonOpenedFileAutoOpens and
WriteNonOpenedFileAutoOpens to assert the side-effects of auto-opening: before
calling sm_->read_page / sm_->write_page capture the current open-file count
(e.g., auto before = sm_->files_opened();) then after the operation assert the
file was created on disk (use file_exists("./test_data", filename)) and that the
open-file count increased by 1 (EXPECT_EQ(sm_->files_opened(), before + 1));
keep existing buffer checks and cleanup_file calls intact and use
StorageManager::PAGE_SIZE for buffer sizing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 48ed6046-2446-44b4-96af-8ec568fe866c
📒 Files selected for processing (1)
tests/storage_manager_tests.cpp
Update ReadNonOpenedFileAutoOpens and WriteNonOpenedFileAutoOpens to: - Capture open-file count before operation - Assert file was created on disk - Assert open-file count increased by 1
1 participant
Summary
Add 9 new unit tests for StorageManager to tests/storage_manager_tests.cpp.
New Tests
Test plan
Summary by CodeRabbit