SERVER-18036 moved getDetectableErrorStatus() to replication executor test fixture

make all setUp/tearDown test fixture functions for data replicator protected

12 files changed, 76 insertions, 72 deletions

diff --git a/src/mongo/db/repl/applier_test.cpp b/src/mongo/db/repl/applier_test.cpp
index 0774b1bed29..f876a884605 100644
--- a/src/mongo/db/repl/applier_test.cpp
+++ b/src/mongo/db/repl/applier_test.cpp
@@ -49,17 +49,14 @@ namespace {
 
     class ApplierTest : public ReplicationExecutorTest {
     public:
-        /**
-         * Creates an initial error status suitable for checking if
-         * applier has modified the 'status' field in test fixture.
-         */
-        static Status getDetectableErrorStatus();
 
-        void setUp() override;
-        void tearDown() override;
         Applier* getApplier() const;
 
     protected:
+
+        void setUp() override;
+        void tearDown() override;
+
         /**
          * Test function to check behavior when we fail to apply one of the operations.
          */
@@ -69,10 +66,6 @@ namespace {
         std::unique_ptr<unittest::Barrier> _barrier;
     };
 
-    Status ApplierTest::getDetectableErrorStatus() {
-        return Status(ErrorCodes::InternalError, "Not mutated");
-    }
-
     void ApplierTest::setUp() {
         ReplicationExecutorTest::setUp();
         launchExecutorThread();
diff --git a/src/mongo/db/repl/base_cloner_test_fixture.cpp b/src/mongo/db/repl/base_cloner_test_fixture.cpp
index f15775d401e..a8df9deb6ba 100644
--- a/src/mongo/db/repl/base_cloner_test_fixture.cpp
+++ b/src/mongo/db/repl/base_cloner_test_fixture.cpp
@@ -43,10 +43,6 @@ namespace repl {
     const BSONObj BaseClonerTest::idIndexSpec =
         BSON("v" << 1 << "key" << BSON("_id" << 1) << "name" << "_id_" << "ns" << nss.ns());
 
-    Status BaseClonerTest::getDetectableErrorStatus() {
-        return Status(ErrorCodes::InternalError, "Not mutated");
-    }
-
     // static
     BSONObj BaseClonerTest::createCursorResponse(CursorId cursorId,
                                                     const std::string& ns,
diff --git a/src/mongo/db/repl/base_cloner_test_fixture.h b/src/mongo/db/repl/base_cloner_test_fixture.h
index cc3ce89c2b7..a7e5f68e448 100644
--- a/src/mongo/db/repl/base_cloner_test_fixture.h
+++ b/src/mongo/db/repl/base_cloner_test_fixture.h
@@ -57,12 +57,6 @@ namespace repl {
         typedef executor::NetworkInterfaceMock::NetworkOperationIterator NetworkOperationIterator;
 
         /**
-         * Creates an initial error status suitable for checking if
-         * cloner has modified the 'status' field in test fixture.
-         */
-        static Status getDetectableErrorStatus();
-
-        /**
          * Creates a cursor response with given array of documents.
          */
         static BSONObj createCursorResponse(CursorId cursorId,
@@ -101,9 +95,6 @@ namespace repl {
 
         BaseClonerTest();
 
-        void setUp() override;
-        void tearDown() override;
-
         virtual void clear();
 
         void setStatus(const Status& status);
@@ -127,6 +118,9 @@ namespace repl {
 
     protected:
 
+        void setUp() override;
+        void tearDown() override;
+
         std::unique_ptr<ClonerStorageInterfaceMock> storageInterface;
 
     private:
diff --git a/src/mongo/db/repl/collection_cloner_test.cpp b/src/mongo/db/repl/collection_cloner_test.cpp
index 9b6927c4b43..4a6c4d2bc03 100644
--- a/src/mongo/db/repl/collection_cloner_test.cpp
+++ b/src/mongo/db/repl/collection_cloner_test.cpp
@@ -45,11 +45,13 @@ namespace {
     class CollectionClonerTest : public BaseClonerTest {
     public:
 
-        void setUp() override;
-        void tearDown() override;
         BaseCloner* getCloner() const override;
 
     protected:
+
+        void setUp() override;
+        void tearDown() override;
+
         CollectionOptions options;
         std::unique_ptr<CollectionCloner> collectionCloner;
     };
diff --git a/src/mongo/db/repl/data_replicator.cpp b/src/mongo/db/repl/data_replicator.cpp
index 37bd1fe8073..287f81bca30 100644
--- a/src/mongo/db/repl/data_replicator.cpp
+++ b/src/mongo/db/repl/data_replicator.cpp
@@ -1296,7 +1296,7 @@ std::string toString(DataReplicatorState s) {
         if (status.code() == ErrorCodes::CallbackCanceled)
             return;
         if (status.isOK()) {
-            const auto docs = fetchResult.getValue().documents;
+            const auto& docs = fetchResult.getValue().documents;
             if (docs.begin() != docs.end()) {
                 LockGuard lk(_mutex);
                 std::for_each(docs.cbegin(),
diff --git a/src/mongo/db/repl/data_replicator_test.cpp b/src/mongo/db/repl/data_replicator_test.cpp
index 22989734ada..0b8aa6887f5 100644
--- a/src/mongo/db/repl/data_replicator_test.cpp
+++ b/src/mongo/db/repl/data_replicator_test.cpp
@@ -70,28 +70,11 @@ namespace {
 
     class DataReplicatorTest : public ReplicationExecutorTest {
     public:
-        DataReplicatorTest() {}
-        void setUp() override {
-            ReplicationExecutorTest::setUp();
-            reset();
 
-            _settings.replSet = "foo"; // We are a replica set :)
-            _repl.reset(new ReplicationCoordinatorMock(_settings));
-            launchExecutorThread();
-            DataReplicatorOptions options;
-            options.initialSyncRetryWait = Milliseconds(0);
-            createDataReplicator(options);
-        }
+        DataReplicatorTest() {}
 
         void postExecutorThreadLaunch() override {};
 
-        void tearDown() override {
-            ReplicationExecutorTest::tearDown();
-            _dr.reset();
-            _repl.reset();
-            // Executor may still invoke callback before shutting down.
-        }
-
         void reset() {
             // clear/reset state
         }
@@ -147,6 +130,26 @@ namespace {
         DataReplicator& getDR() { return *_dr; }
         ReplicationCoordinator& getRepl() { return *_repl; }
 
+    protected:
+
+        void setUp() override {
+            ReplicationExecutorTest::setUp();
+            reset();
+
+            _settings.replSet = "foo"; // We are a replica set :)
+            _repl.reset(new ReplicationCoordinatorMock(_settings));
+            launchExecutorThread();
+            DataReplicatorOptions options;
+            options.initialSyncRetryWait = Milliseconds(0);
+            createDataReplicator(options);
+        }
+
+        void tearDown() override {
+            ReplicationExecutorTest::tearDown();
+            _dr.reset();
+            _repl.reset();
+            // Executor may still invoke callback before shutting down.
+        }
 
     private:
         std::unique_ptr<DataReplicator> _dr;
diff --git a/src/mongo/db/repl/database_cloner_test.cpp b/src/mongo/db/repl/database_cloner_test.cpp
index eb305fbd0fa..1cb772bc898 100644
--- a/src/mongo/db/repl/database_cloner_test.cpp
+++ b/src/mongo/db/repl/database_cloner_test.cpp
@@ -46,14 +46,17 @@ namespace {
 
     class DatabaseClonerTest : public BaseClonerTest {
     public:
+
         DatabaseClonerTest();
         void collectionWork(const Status& status, const NamespaceString& sourceNss);
-        void setUp() override;
-        void tearDown() override;
         void clear() override;
         BaseCloner* getCloner() const override;
 
     protected:
+
+        void setUp() override;
+        void tearDown() override;
+
         std::list<std::pair<Status, NamespaceString> > collectionWorkResults;
         std::unique_ptr<DatabaseCloner> databaseCloner;
     };
diff --git a/src/mongo/db/repl/replication_executor_test.cpp b/src/mongo/db/repl/replication_executor_test.cpp
index 144a1519c89..d299706f75b 100644
--- a/src/mongo/db/repl/replication_executor_test.cpp
+++ b/src/mongo/db/repl/replication_executor_test.cpp
@@ -98,7 +98,7 @@ namespace {
 
     TEST_F(ReplicationExecutorTest, RunOne) {
         ReplicationExecutor& executor = getExecutor();
-        Status status(ErrorCodes::InternalError, "Not mutated");
+        Status status = getDetectableErrorStatus();
         ASSERT_OK(executor.scheduleWork(stdx::bind(setStatusAndShutdown,
                                                      stdx::placeholders::_1,
                                                      &status)).getStatus());
@@ -108,7 +108,7 @@ namespace {
 
     TEST_F(ReplicationExecutorTest, Schedule1ButShutdown) {
         ReplicationExecutor& executor = getExecutor();
-        Status status(ErrorCodes::InternalError, "Not mutated");
+        Status status = getDetectableErrorStatus();
         ASSERT_OK(executor.scheduleWork(stdx::bind(setStatusAndShutdown,
                                                    stdx::placeholders::_1,
                                                    &status)).getStatus());
@@ -119,8 +119,8 @@ namespace {
 
     TEST_F(ReplicationExecutorTest, Schedule2Cancel1) {
         ReplicationExecutor& executor = getExecutor();
-        Status status1(ErrorCodes::InternalError, "Not mutated");
-        Status status2(ErrorCodes::InternalError, "Not mutated");
+        Status status1 = getDetectableErrorStatus();
+        Status status2 = getDetectableErrorStatus();
         ReplicationExecutor::CallbackHandle cb = unittest::assertGet(
             executor.scheduleWork(stdx::bind(setStatusAndShutdown,
                                              stdx::placeholders::_1,
@@ -136,8 +136,8 @@ namespace {
 
     TEST_F(ReplicationExecutorTest, OneSchedulesAnother) {
         ReplicationExecutor& executor = getExecutor();
-        Status status1(ErrorCodes::InternalError, "Not mutated");
-        Status status2(ErrorCodes::InternalError, "Not mutated");
+        Status status1 = getDetectableErrorStatus();
+        Status status2 = getDetectableErrorStatus();
         ASSERT_OK(executor.scheduleWork(stdx::bind(scheduleSetStatusAndShutdown,
                                                    stdx::placeholders::_1,
                                                    &status1,
@@ -283,9 +283,9 @@ namespace {
         NetworkInterfaceMock* net = getNet();
         ReplicationExecutor& executor = getExecutor();
         launchExecutorThread();
-        Status status1(ErrorCodes::InternalError, "Not mutated");
-        Status status2(ErrorCodes::InternalError, "Not mutated");
-        Status status3(ErrorCodes::InternalError, "Not mutated");
+        Status status1 = getDetectableErrorStatus();
+        Status status2 = getDetectableErrorStatus();
+        Status status3 = getDetectableErrorStatus();
         const Date_t now = net->now();
         const ReplicationExecutor::CallbackHandle cb1 =
             unittest::assertGet(executor.scheduleWorkAt(now + Milliseconds(100),
@@ -338,7 +338,7 @@ namespace {
         NetworkInterfaceMock* net = getNet();
         ReplicationExecutor& executor = getExecutor();
         launchExecutorThread();
-        Status status1(ErrorCodes::InternalError, "Not mutated");
+        Status status1 = getDetectableErrorStatus();
         const RemoteCommandRequest request(
                 HostAndPort("localhost", 27017),
                 "mydb",
@@ -365,7 +365,7 @@ namespace {
 
     TEST_F(ReplicationExecutorTest, ScheduleAndCancelRemoteCommand) {
         ReplicationExecutor& executor = getExecutor();
-        Status status1(ErrorCodes::InternalError, "Not mutated");
+        Status status1 = getDetectableErrorStatus();
         const RemoteCommandRequest request(
                 HostAndPort("localhost", 27017),
                 "mydb",
@@ -390,7 +390,7 @@ namespace {
         unittest::Barrier barrier(2U);
         NamespaceString nss("mydb", "mycoll");
         ReplicationExecutor& executor = getExecutor();
-        Status status1(ErrorCodes::InternalError, "Not mutated");
+        Status status1 = getDetectableErrorStatus();
         OperationContext* txn = nullptr;
         using CallbackData = ReplicationExecutor::CallbackArgs;
         ASSERT_OK(executor.scheduleDBWork([&](const CallbackData& cbData) {
@@ -411,7 +411,7 @@ namespace {
     TEST_F(ReplicationExecutorTest, ScheduleDBWorkWithCollectionLock) {
         NamespaceString nss("mydb", "mycoll");
         ReplicationExecutor& executor = getExecutor();
-        Status status1(ErrorCodes::InternalError, "Not mutated");
+        Status status1 = getDetectableErrorStatus();
         OperationContext* txn = nullptr;
         bool collectionIsLocked = false;
         using CallbackData = ReplicationExecutor::CallbackArgs;
@@ -432,7 +432,7 @@ namespace {
 
     TEST_F(ReplicationExecutorTest, ScheduleExclusiveLockOperation) {
         ReplicationExecutor& executor = getExecutor();
-        Status status1(ErrorCodes::InternalError, "Not mutated");
+        Status status1 = getDetectableErrorStatus();
         OperationContext* txn = nullptr;
         bool lockIsW = false;
         using CallbackData = ReplicationExecutor::CallbackArgs;
@@ -452,7 +452,7 @@ namespace {
     TEST_F(ReplicationExecutorTest, ShutdownBeforeRunningSecondExclusiveLockOperation) {
         ReplicationExecutor& executor = getExecutor();
         using CallbackData = ReplicationExecutor::CallbackArgs;
-        Status status1(ErrorCodes::InternalError, "Not mutated");
+        Status status1 = getDetectableErrorStatus();
         ASSERT_OK(executor.scheduleWorkWithGlobalExclusiveLock([&](const CallbackData& cbData) {
             status1 = cbData.status;
             if (cbData.status != ErrorCodes::CallbackCanceled)
@@ -461,7 +461,7 @@ namespace {
         // Second db work item is invoked by the main executor thread because the work item is
         // moved from the exclusive lock queue to the ready work item queue when the first callback
         // cancels the executor.
-        Status status2(ErrorCodes::InternalError, "Not mutated");
+        Status status2 = getDetectableErrorStatus();
         ASSERT_OK(executor.scheduleWorkWithGlobalExclusiveLock([&](const CallbackData& cbData) {
             status2 = cbData.status;
             if (cbData.status != ErrorCodes::CallbackCanceled)
diff --git a/src/mongo/db/repl/replication_executor_test_fixture.cpp b/src/mongo/db/repl/replication_executor_test_fixture.cpp
index 33b850597bf..bcd07d0db68 100644
--- a/src/mongo/db/repl/replication_executor_test_fixture.cpp
+++ b/src/mongo/db/repl/replication_executor_test_fixture.cpp
@@ -43,6 +43,11 @@ namespace {
 
 } // namespace
 
+    // static
+    Status ReplicationExecutorTest::getDetectableErrorStatus() {
+        return Status(ErrorCodes::InternalError, "Not mutated");
+    }
+
     void ReplicationExecutorTest::launchExecutorThread() {
         ASSERT(!_executorThread);
         _executorThread.reset(
diff --git a/src/mongo/db/repl/replication_executor_test_fixture.h b/src/mongo/db/repl/replication_executor_test_fixture.h
index 8612db5a1a7..a6fec40ebd2 100644
--- a/src/mongo/db/repl/replication_executor_test_fixture.h
+++ b/src/mongo/db/repl/replication_executor_test_fixture.h
@@ -49,6 +49,14 @@ namespace repl {
      * a NetworkInterfaceMock.
      */
     class ReplicationExecutorTest : public unittest::Test {
+    public:
+
+        /**
+         * Creates an initial error status suitable for checking if
+         * component has modified the 'status' field in test fixture.
+         */
+        static Status getDetectableErrorStatus();
+
     protected:
         executor::NetworkInterfaceMock* getNet() { return _net; }
         ReplicationExecutor& getExecutor() { return *_executor; }
@@ -77,14 +85,14 @@ namespace repl {
          * To run the executor in the background, tests should invoke launchExecutorThread() or
          * override this function() to achieve the same effect.
          */
-        virtual void setUp();
+        void setUp() override;
 
         /**
          * Destroys the replication executor.
          *
          * Shuts down running background executor.
          */
-        virtual void tearDown();
+        void tearDown() override;
 
 
     private:
diff --git a/src/mongo/db/repl/reporter_test.cpp b/src/mongo/db/repl/reporter_test.cpp
index c4bd0b6176e..50070134ec0 100644
--- a/src/mongo/db/repl/reporter_test.cpp
+++ b/src/mongo/db/repl/reporter_test.cpp
@@ -63,15 +63,17 @@ namespace {
 
     class ReporterTest : public ReplicationExecutorTest {
     public:
-        static Status getDetectableErrorStatus();
+
         ReporterTest();
-        void setUp() override;
-        void tearDown() override;
         void scheduleNetworkResponse(const BSONObj& obj);
         void scheduleNetworkResponse(ErrorCodes::Error code, const std::string& reason);
         void finishProcessingNetworkResponse();
 
     protected:
+
+        void setUp() override;
+        void tearDown() override;
+
         std::unique_ptr<Reporter> reporter;
         std::unique_ptr<MockProgressManager> posUpdater;
 
@@ -79,10 +81,6 @@ namespace {
 
     ReporterTest::ReporterTest() {}
 
-    Status ReporterTest::getDetectableErrorStatus() {
-        return Status(ErrorCodes::InternalError, "Not mutated");
-    }
-
     void ReporterTest::setUp() {
         ReplicationExecutorTest::setUp();
         posUpdater.reset(new MockProgressManager());
diff --git a/src/mongo/db/repl/task_runner_test_fixture.h b/src/mongo/db/repl/task_runner_test_fixture.h
index 3863410d64c..70b25923498 100644
--- a/src/mongo/db/repl/task_runner_test_fixture.h
+++ b/src/mongo/db/repl/task_runner_test_fixture.h
@@ -68,6 +68,8 @@ namespace repl {
         void resetTaskRunner(TaskRunner* taskRunner);
         void destroyTaskRunner();
 
+    protected:
+
         void setUp() override;
         void tearDown() override;