From 9500d90d1ba9212150b4c1bc053235466a69b4b6 Mon Sep 17 00:00:00 2001 From: "mayue.fight" Date: Fri, 14 Apr 2023 10:44:42 -0700 Subject: [PATCH] Fix serval bugs in ImportColumnFamilyTest (#11372) Summary: **Context/Summary:** ASSERT_EQ will only verify the code of Status, but will not check the state message of Status. - Assert by checking Status state in `ImportColumnFamilyTest` - Forgot to set db_comparator_name when creating ExportImportFilesMetaData in `ImportColumnFamilyNegativeTest` Pull Request resolved: https://github.com/facebook/rocksdb/pull/11372 Reviewed By: ajkr Differential Revision: D45004343 Pulled By: cbi42 fbshipit-source-id: a13d45521df17ead3d6d4c1c1fe1e4c95397ce8b --- db/import_column_family_job.cc | 9 ++++++--- db/import_column_family_test.cc | 36 ++++++++++++++++----------------- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/db/import_column_family_job.cc b/db/import_column_family_job.cc index 68e54ab69..690a3b617 100644 --- a/db/import_column_family_job.cc +++ b/db/import_column_family_job.cc @@ -44,16 +44,19 @@ Status ImportColumnFamilyJob::Prepare(uint64_t next_file_number, auto num_files = files_to_import_.size(); if (num_files == 0) { - return Status::InvalidArgument("The list of files is empty"); + status = Status::InvalidArgument("The list of files is empty"); + return status; } for (const auto& f : files_to_import_) { if (f.num_entries == 0) { - return Status::InvalidArgument("File contain no entries"); + status = Status::InvalidArgument("File contain no entries"); + return status; } if (!f.smallest_internal_key.Valid() || !f.largest_internal_key.Valid()) { - return Status::Corruption("File has corrupted keys"); + status = Status::Corruption("File has corrupted keys"); + return status; } } diff --git a/db/import_column_family_test.cc b/db/import_column_family_test.cc index c7940a374..241e69cbe 100644 --- a/db/import_column_family_test.cc +++ b/db/import_column_family_test.cc @@ -628,22 +628,22 @@ TEST_F(ImportColumnFamilyTest, ImportColumnFamilyNegativeTest) { { // Create column family with existing cf name. ExportImportFilesMetaData metadata; - - ASSERT_EQ(db_->CreateColumnFamilyWithImport(ColumnFamilyOptions(), "koko", - ImportColumnFamilyOptions(), - metadata, &import_cfh_), - Status::InvalidArgument("Column family already exists")); + metadata.db_comparator_name = options.comparator->Name(); + Status s = db_->CreateColumnFamilyWithImport(ColumnFamilyOptions(), "koko", + ImportColumnFamilyOptions(), + metadata, &import_cfh_); + ASSERT_TRUE(std::strstr(s.getState(), "Column family already exists")); ASSERT_EQ(import_cfh_, nullptr); } { // Import with no files specified. ExportImportFilesMetaData metadata; - - ASSERT_EQ(db_->CreateColumnFamilyWithImport(ColumnFamilyOptions(), "yoyo", - ImportColumnFamilyOptions(), - metadata, &import_cfh_), - Status::InvalidArgument("The list of files is empty")); + metadata.db_comparator_name = options.comparator->Name(); + Status s = db_->CreateColumnFamilyWithImport(ColumnFamilyOptions(), "yoyo", + ImportColumnFamilyOptions(), + metadata, &import_cfh_); + ASSERT_TRUE(std::strstr(s.getState(), "The list of files is empty")); ASSERT_EQ(import_cfh_, nullptr); } @@ -693,10 +693,10 @@ TEST_F(ImportColumnFamilyTest, ImportColumnFamilyNegativeTest) { LiveFileMetaDataInit(file1_sst_name, sst_files_dir_, 1, 10, 19)); metadata.db_comparator_name = mismatch_options.comparator->Name(); - ASSERT_EQ(db_->CreateColumnFamilyWithImport(ColumnFamilyOptions(), "coco", - ImportColumnFamilyOptions(), - metadata, &import_cfh_), - Status::InvalidArgument("Comparator name mismatch")); + Status s = db_->CreateColumnFamilyWithImport(ColumnFamilyOptions(), "coco", + ImportColumnFamilyOptions(), + metadata, &import_cfh_); + ASSERT_TRUE(std::strstr(s.getState(), "Comparator name mismatch")); ASSERT_EQ(import_cfh_, nullptr); } @@ -718,10 +718,10 @@ TEST_F(ImportColumnFamilyTest, ImportColumnFamilyNegativeTest) { LiveFileMetaDataInit(file3_sst_name, sst_files_dir_, 1, 10, 19)); metadata.db_comparator_name = options.comparator->Name(); - ASSERT_EQ(db_->CreateColumnFamilyWithImport(ColumnFamilyOptions(), "yoyo", - ImportColumnFamilyOptions(), - metadata, &import_cfh_), - Status::IOError("No such file or directory")); + Status s = db_->CreateColumnFamilyWithImport(ColumnFamilyOptions(), "yoyo", + ImportColumnFamilyOptions(), + metadata, &import_cfh_); + ASSERT_TRUE(std::strstr(s.getState(), "No such file or directory")); ASSERT_EQ(import_cfh_, nullptr); // Test successful import after a failure with the same CF name. Ensures