Copilot commented on code in PR #3559:
URL: https://github.com/apache/kvrocks/pull/3559#discussion_r3576939478
##########
tests/cppunit/storage_test.cc:
##########
@@ -170,3 +170,90 @@ TEST(Storage, ReplDataManagerRejectsUnsafeFilenames) {
std::filesystem::remove_all("test_repl_file_validation_dir", ec);
ASSERT_FALSE(ec);
}
+
+TEST(Storage, GetFullReplDataInfoRejectsEmptyCheckpoint) {
+ std::error_code ec;
+
+ const std::string test_dir = "test_empty_checkpoint";
+ Config config;
+ config.db_dir = test_dir + "/db";
+ config.checkpoint_dir = test_dir + "/checkpoint";
+ config.slot_id_encoded = false;
+
+ std::filesystem::remove_all(test_dir, ec);
+ ASSERT_FALSE(ec);
+ std::filesystem::create_directory(test_dir, ec);
+ ASSERT_FALSE(ec);
+
+ auto storage = std::make_unique<engine::Storage>(&config);
+ auto s = storage->Open();
+ ASSERT_TRUE(s.IsOK()) << s.Msg();
+
+ auto ctx = engine::Context(storage.get());
+ rocksdb::WriteBatch batch;
+ batch.Put("k", "v");
+ ASSERT_TRUE(storage->Write(ctx, rocksdb::WriteOptions(), &batch).ok());
+
+ std::string files;
+ s = engine::Storage::ReplDataManager::GetFullReplDataInfo(storage.get(),
&files);
+ ASSERT_TRUE(s.IsOK()) << s.Msg();
+
+ std::filesystem::remove_all(config.checkpoint_dir, ec);
+ ASSERT_FALSE(ec);
+ std::filesystem::create_directory(config.checkpoint_dir, ec);
+ ASSERT_FALSE(ec);
+
+ files.clear();
+ s = engine::Storage::ReplDataManager::GetFullReplDataInfo(storage.get(),
&files);
+ EXPECT_FALSE(s.IsOK());
+ EXPECT_TRUE(files.empty());
+
+ std::filesystem::remove_all(test_dir, ec);
+ ASSERT_FALSE(ec);
+}
+
+TEST(Storage, TryPurgeCheckpoint) {
+ std::error_code ec;
+
+ const std::string test_dir = "test_purge_checkpoint";
+ Config config;
+ config.db_dir = test_dir + "/db";
+ config.checkpoint_dir = test_dir + "/checkpoint";
+ config.slot_id_encoded = false;
+
+ std::filesystem::remove_all(test_dir, ec);
+ ASSERT_FALSE(ec);
+ std::filesystem::create_directory(test_dir, ec);
+ ASSERT_FALSE(ec);
+
+ auto storage = std::make_unique<engine::Storage>(&config);
+ auto s = storage->Open();
+ ASSERT_TRUE(s.IsOK()) << s.Msg();
+
+ auto ctx = engine::Context(storage.get());
+ rocksdb::WriteBatch batch;
+ batch.Put("k", "v");
+ ASSERT_TRUE(storage->Write(ctx, rocksdb::WriteOptions(), &batch).ok());
+
+ std::string files;
+ s = engine::Storage::ReplDataManager::GetFullReplDataInfo(storage.get(),
&files);
+ ASSERT_TRUE(s.IsOK()) << s.Msg();
+
+ storage->SetCheckpointAccessTimeSecs(storage->GetCheckpointAccessTimeSecs()
- 60);
+
+ s = storage->TryPurgeCheckpoint(/*fetch_file_threads=*/1);
+ ASSERT_TRUE(s.IsOK()) << s.Msg();
+ EXPECT_TRUE(storage->ExistCheckpoint());
+
+ s = storage->TryPurgeCheckpoint(/*fetch_file_threads=*/0);
+ ASSERT_TRUE(s.IsOK()) << s.Msg();
+ EXPECT_FALSE(storage->ExistCheckpoint());
+
+ files.clear();
+ s = engine::Storage::ReplDataManager::GetFullReplDataInfo(storage.get(),
&files);
+ ASSERT_TRUE(s.IsOK()) << s.Msg();
+ EXPECT_TRUE(storage->ExistCheckpoint());
Review Comment:
This test exercises purge/no-purge paths, but it doesn’t actually assert the
key invariants described in the PR (trash directory removed and
checkpoint_info_ reset). Adding explicit assertions after the purge call would
make the regression protection stronger and ensure TryPurgeCheckpoint clears
state as intended.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
