rocksdb: Replace ASSERT* with EXPECT* in functions that does not retu…

…rn void value

Summary:
gtest does not use exceptions to fail a unit test by design, and `ASSERT*`s are implemented using `return`. As a consequence we cannot use `ASSERT*` in a function that does not return `void` value ([[ https://code.google.com/p/googletest/wiki/AdvancedGuide#Assertion_Placement | 1]]), and have to fix our existing code. This diff does this in a generic way, with no manual changes.

In order to detect all existing `ASSERT*` that are used in functions that doesn't return void value, I change the code to generate compile errors for such cases.

In `util/testharness.h` I defined `EXPECT*` assertions, the same way as `ASSERT*`, and redefined `ASSERT*` to return `void`. Then executed:

```lang=bash
% USE_CLANG=1 make all -j55 -k 2> build.log
% perl -naF: -e 'print "-- -number=".$F[1]." ".$F[0]."\n" if  /: error:/' \
build.log | xargs -L 1 perl -spi -e 's/ASSERT/EXPECT/g if $. == $number'
% make format
```
After that I reverted back change to `ASSERT*` in `util/testharness.h`. But preserved introduced `EXPECT*`, which is the same as `ASSERT*`. This will be deleted once switched to gtest.

This diff is independent and contains manual changes only in `util/testharness.h`.

Test Plan:
Make sure all tests are passing.
```lang=bash
% USE_CLANG=1 make check
```

Reviewers: igor, lgalanis, sdong, yufei.zhu, rven, meyering

Reviewed By: meyering

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D33333

db/column_family_test.cc

@@ -115,7 +115,7 @@ class ColumnFamilyTest {
 
   int GetProperty(int cf, std::string property) {
     std::string value;
-    ASSERT_TRUE(dbfull()->GetProperty(handles_[cf], property, &value));
+    EXPECT_TRUE(dbfull()->GetProperty(handles_[cf], property, &value));
     return std::stoi(value);
   }
 
@@ -274,7 +274,7 @@ class ColumnFamilyTest {
         break;
       }
     }
-    ASSERT_OK(s);
+    EXPECT_OK(s);
     for (const auto& wal : wal_files) {
       if (wal->Type() == kAliveLogFile) {
         ++ret;

db/compaction_job_test.cc

@@ -33,16 +33,15 @@ class CompactionJobTest {
                                  &write_controller_)),
         shutting_down_(false),
         mock_table_factory_(new mock::MockTableFactory()) {
-    ASSERT_OK(env_->CreateDirIfMissing(dbname_));
+    EXPECT_OK(env_->CreateDirIfMissing(dbname_));
     db_options_.db_paths.emplace_back(dbname_,
                                       std::numeric_limits<uint64_t>::max());
     NewDB();
     std::vector<ColumnFamilyDescriptor> column_families;
     cf_options_.table_factory = mock_table_factory_;
     column_families.emplace_back(kDefaultColumnFamilyName, cf_options_);
 
-
-    ASSERT_OK(versions_->Recover(column_families, false));
+    EXPECT_OK(versions_->Recover(column_families, false));
   }
 
   std::string GenerateFileName(uint64_t file_number) {
@@ -82,7 +81,7 @@ class CompactionJobTest {
       }
 
       uint64_t file_number = versions_->NewFileNumber();
-      ASSERT_OK(mock_table_factory_->CreateMockTable(
+      EXPECT_OK(mock_table_factory_->CreateMockTable(
           env_, GenerateFileName(file_number), std::move(contents)));
 
       VersionEdit edit;

db/comparator_db_test.cc

@@ -260,12 +260,12 @@ class ComparatorDBTest {
   ComparatorDBTest() : env_(Env::Default()), db_(nullptr) {
     comparator = BytewiseComparator();
     dbname_ = test::TmpDir() + "/comparator_db_test";
-    ASSERT_OK(DestroyDB(dbname_, last_options_));
+    EXPECT_OK(DestroyDB(dbname_, last_options_));
   }
 
   ~ComparatorDBTest() {
     delete db_;
-    ASSERT_OK(DestroyDB(dbname_, last_options_));
+    EXPECT_OK(DestroyDB(dbname_, last_options_));
     comparator = BytewiseComparator();
   }
 

db/cuckoo_table_db_test.cc

@@ -23,14 +23,14 @@ class CuckooTableDBTest {
  public:
   CuckooTableDBTest() : env_(Env::Default()) {
     dbname_ = test::TmpDir() + "/cuckoo_table_db_test";
-    ASSERT_OK(DestroyDB(dbname_, Options()));
+    EXPECT_OK(DestroyDB(dbname_, Options()));
     db_ = nullptr;
     Reopen();
   }
 
   ~CuckooTableDBTest() {
     delete db_;
-    ASSERT_OK(DestroyDB(dbname_, Options()));
+    EXPECT_OK(DestroyDB(dbname_, Options()));
   }
 
   Options CurrentOptions() {
@@ -83,9 +83,8 @@ class CuckooTableDBTest {
 
   int NumTableFilesAtLevel(int level) {
     std::string property;
-    ASSERT_TRUE(
-        db_->GetProperty("rocksdb.num-files-at-level" + NumberToString(level),
-                         &property));
+    EXPECT_TRUE(db_->GetProperty(
+        "rocksdb.num-files-at-level" + NumberToString(level), &property));
     return atoi(property.c_str());
   }
 

db/db_test.cc

@@ -464,7 +464,7 @@ class DBTest {
     env_->SetBackgroundThreads(1, Env::HIGH);
     dbname_ = test::TmpDir(env_) + "/db_test";
     auto options = CurrentOptions();
-    ASSERT_OK(DestroyDB(dbname_, options));
+    EXPECT_OK(DestroyDB(dbname_, options));
     db_ = nullptr;
     Reopen(options);
   }
@@ -476,7 +476,7 @@ class DBTest {
     options.db_paths.emplace_back(dbname_ + "_2", 0);
     options.db_paths.emplace_back(dbname_ + "_3", 0);
     options.db_paths.emplace_back(dbname_ + "_4", 0);
-    ASSERT_OK(DestroyDB(dbname_, options));
+    EXPECT_OK(DestroyDB(dbname_, options));
     delete env_;
   }
 
@@ -747,7 +747,7 @@ class DBTest {
       const std::vector<std::string>& cfs,
       const std::vector<Options>& options) {
     Close();
-    ASSERT_EQ(cfs.size(), options.size());
+    EXPECT_EQ(cfs.size(), options.size());
     std::vector<ColumnFamilyDescriptor> column_families;
     for (size_t i = 0; i < cfs.size(); ++i) {
       column_families.push_back(ColumnFamilyDescriptor(cfs[i], options[i]));
@@ -861,13 +861,13 @@ class DBTest {
 
   uint64_t GetNumSnapshots() {
     uint64_t int_num;
-    ASSERT_TRUE(dbfull()->GetIntProperty("rocksdb.num-snapshots", &int_num));
+    EXPECT_TRUE(dbfull()->GetIntProperty("rocksdb.num-snapshots", &int_num));
     return int_num;
   }
 
   uint64_t GetTimeOldestSnapshots() {
     uint64_t int_num;
-    ASSERT_TRUE(
+    EXPECT_TRUE(
         dbfull()->GetIntProperty("rocksdb.oldest-snapshot-time", &int_num));
     return int_num;
   }
@@ -890,11 +890,11 @@ class DBTest {
     // Check reverse iteration results are the reverse of forward results
     unsigned int matched = 0;
     for (iter->SeekToLast(); iter->Valid(); iter->Prev()) {
-      ASSERT_LT(matched, forward.size());
-      ASSERT_EQ(IterStatus(iter), forward[forward.size() - matched - 1]);
+      EXPECT_LT(matched, forward.size());
+      EXPECT_EQ(IterStatus(iter), forward[forward.size() - matched - 1]);
       matched++;
     }
-    ASSERT_EQ(matched, forward.size());
+    EXPECT_EQ(matched, forward.size());
 
     delete iter;
     return result;
@@ -958,10 +958,10 @@ class DBTest {
     std::string property;
     if (cf == 0) {
       // default cfd
-      ASSERT_TRUE(db_->GetProperty(
+      EXPECT_TRUE(db_->GetProperty(
           "rocksdb.num-files-at-level" + NumberToString(level), &property));
     } else {
-      ASSERT_TRUE(db_->GetProperty(
+      EXPECT_TRUE(db_->GetProperty(
           handles_[cf], "rocksdb.num-files-at-level" + NumberToString(level),
           &property));
     }
@@ -1137,8 +1137,8 @@ class DBTest {
       const SequenceNumber seq) {
     unique_ptr<TransactionLogIterator> iter;
     Status status = dbfull()->GetUpdatesSince(seq, &iter);
-    ASSERT_OK(status);
-    ASSERT_TRUE(iter->Valid());
+    EXPECT_OK(status);
+    EXPECT_TRUE(iter->Valid());
     return std::move(iter);
   }
 
@@ -3762,8 +3762,8 @@ class KeepFilterFactory : public CompactionFilterFactory {
   virtual std::unique_ptr<CompactionFilter> CreateCompactionFilter(
       const CompactionFilter::Context& context) override {
     if (check_context_) {
-      ASSERT_EQ(expect_full_compaction_.load(), context.is_full_compaction);
-      ASSERT_EQ(expect_manual_compaction_.load(), context.is_manual_compaction);
+      EXPECT_EQ(expect_full_compaction_.load(), context.is_full_compaction);
+      EXPECT_EQ(expect_manual_compaction_.load(), context.is_manual_compaction);
     }
     return std::unique_ptr<CompactionFilter>(new KeepFilter());
   }
@@ -6409,11 +6409,11 @@ TEST(DBTest, CustomComparator) {
    private:
     static int ToNumber(const Slice& x) {
       // Check that there are no extra characters.
-      ASSERT_TRUE(x.size() >= 2 && x[0] == '[' && x[x.size()-1] == ']')
+      EXPECT_TRUE(x.size() >= 2 && x[0] == '[' && x[x.size() - 1] == ']')
           << EscapeString(x);
       int val;
       char ignored;
-      ASSERT_TRUE(sscanf(x.ToString().c_str(), "[%i]%c", &val, &ignored) == 1)
+      EXPECT_TRUE(sscanf(x.ToString().c_str(), "[%i]%c", &val, &ignored) == 1)
           << EscapeString(x);
       return val;
     }
@@ -7652,10 +7652,10 @@ SequenceNumber ReadRecords(
   BatchResult res;
   while (iter->Valid()) {
     res = iter->GetBatch();
-    ASSERT_TRUE(res.sequence > lastSequence);
+    EXPECT_TRUE(res.sequence > lastSequence);
     ++count;
     lastSequence = res.sequence;
-    ASSERT_OK(iter->status());
+    EXPECT_OK(iter->status());
     iter->Next();
   }
   return res.sequence;

db/deletefile_test.cc

@@ -57,7 +57,7 @@ class DeleteFileTest {
 
     DestroyDB(dbname_, options_);
     numlevels_ = 7;
-    ASSERT_OK(ReopenDB(true));
+    EXPECT_OK(ReopenDB(true));
   }
 
   Status ReopenDB(bool create) {

db/fault_injection_test.cc

@@ -174,7 +174,7 @@ class FaultInjectionTestEnv : public EnvWrapper {
                       unique_ptr<Directory>* result) override {
     unique_ptr<Directory> r;
     Status s = target()->NewDirectory(name, &r);
-    ASSERT_OK(s);
+    EXPECT_OK(s);
     if (!s.ok()) {
       return s;
     }
@@ -206,7 +206,7 @@ class FaultInjectionTestEnv : public EnvWrapper {
       fprintf(stderr, "Cannot delete file %s: %s\n", f.c_str(),
               s.ToString().c_str());
     }
-    ASSERT_OK(s);
+    EXPECT_OK(s);
     if (s.ok()) {
       UntrackFile(f);
     }
@@ -450,7 +450,7 @@ class FaultInjectionTest {
     NewDB();
   }
 
-  ~FaultInjectionTest() { ASSERT_OK(TearDown()); }
+  ~FaultInjectionTest() { EXPECT_OK(TearDown()); }
 
   bool ChangeOptions() {
     option_config_++;
@@ -524,7 +524,7 @@ class FaultInjectionTest {
 
     dbname_ = test::TmpDir() + "/fault_test";
 
-    ASSERT_OK(DestroyDB(dbname_, options_));
+    EXPECT_OK(DestroyDB(dbname_, options_));
 
     options_.create_if_missing = true;
     Status s = OpenDB();
@@ -581,7 +581,7 @@ class FaultInjectionTest {
       Value(i, &value_space);
       s = ReadValue(i, &val);
       if (s.ok()) {
-        ASSERT_EQ(value_space, val);
+        EXPECT_EQ(value_space, val);
       }
       if (expected == kValExpectFound) {
         if (!s.ok()) {

db/flush_job_test.cc

@@ -32,7 +32,7 @@ class FlushJobTest {
                                  &write_controller_)),
         shutting_down_(false),
         mock_table_factory_(new mock::MockTableFactory()) {
-    ASSERT_OK(env_->CreateDirIfMissing(dbname_));
+    EXPECT_OK(env_->CreateDirIfMissing(dbname_));
     db_options_.db_paths.emplace_back(dbname_,
                                       std::numeric_limits<uint64_t>::max());
     // TODO(icanadi) Remove this once we mock out VersionSet
@@ -41,7 +41,7 @@ class FlushJobTest {
     cf_options_.table_factory = mock_table_factory_;
     column_families.emplace_back(kDefaultColumnFamilyName, cf_options_);
 
-    ASSERT_OK(versions_->Recover(column_families, false));
+    EXPECT_OK(versions_->Recover(column_families, false));
   }
 
   void NewDB() {

db/listener_test.cc

@@ -39,7 +39,7 @@ class EventListenerTest {
  public:
   EventListenerTest() {
     dbname_ = test::TmpDir() + "/listener_test";
-    ASSERT_OK(DestroyDB(dbname_, Options()));
+    EXPECT_OK(DestroyDB(dbname_, Options()));
     db_ = nullptr;
     Reopen();
   }
@@ -51,7 +51,7 @@ class EventListenerTest {
     options.db_paths.emplace_back(dbname_ + "_2", 0);
     options.db_paths.emplace_back(dbname_ + "_3", 0);
     options.db_paths.emplace_back(dbname_ + "_4", 0);
-    ASSERT_OK(DestroyDB(dbname_, options));
+    EXPECT_OK(DestroyDB(dbname_, options));
   }
 
   void CreateColumnFamilies(const std::vector<std::string>& cfs,
@@ -91,7 +91,7 @@ class EventListenerTest {
       const std::vector<std::string>& cfs,
       const std::vector<const Options*>& options) {
     Close();
-    ASSERT_EQ(cfs.size(), options.size());
+    EXPECT_EQ(cfs.size(), options.size());
     std::vector<ColumnFamilyDescriptor> column_families;
     for (size_t i = 0; i < cfs.size(); ++i) {
       column_families.push_back(ColumnFamilyDescriptor(cfs[i], *options[i]));

db/log_test.cc

@@ -57,7 +57,7 @@ class LogTest {
 
     virtual Status Close() override { return Status::OK(); }
     virtual Status Flush() override {
-      ASSERT_TRUE(reader_contents_.size() <= last_flush_);
+      EXPECT_TRUE(reader_contents_.size() <= last_flush_);
       size_t offset = last_flush_ - reader_contents_.size();
       reader_contents_ = Slice(
           contents_.data() + offset,
@@ -100,7 +100,7 @@ class LogTest {
       returned_partial_(false) { }
 
     virtual Status Read(size_t n, Slice* result, char* scratch) override {
-      ASSERT_TRUE(!returned_partial_) << "must not Read() after eof/error";
+      EXPECT_TRUE(!returned_partial_) << "must not Read() after eof/error";
 
       if (force_error_) {
         if (force_error_position_ >= n) {

db/perf_context_test.cc

@@ -55,7 +55,7 @@ std::shared_ptr<DB> OpenDb(bool read_only = false) {
     } else {
       s = DB::OpenForReadOnly(options, kDbName, &db);
     }
-    ASSERT_OK(s);
+    EXPECT_OK(s);
     return std::shared_ptr<DB>(db);
 }