Introduced MockRegistrar.

This allows test cases to inspect how the master interacts with
the registry.

This commit changes `tests::cluster::Master` to use `MockRegistrar`
exclusively, even for test cases that aren't interested in mocking
aspects of the master <-> registrar interaction. However, this should be
harmless, since by default `MockRegistrar` behaves identically to the
normal registrar.

Review: https://reviews.apache.org/r/51375/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/80993da4
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/80993da4
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/80993da4

Branch: refs/heads/master
Commit: 80993da4cefa88a60a63134fadbc2a3f183dfd46
Parents: 88b2533
Author: Neil Conway <neil.con...@gmail.com>
Authored: Mon Sep 19 15:48:02 2016 -0700
Committer: Vinod Kone <vinodk...@gmail.com>
Committed: Mon Sep 19 15:48:02 2016 -0700

----------------------------------------------------------------------
 src/Makefile.am              |  2 ++
 src/master/registrar.hpp     |  4 +--
 src/tests/cluster.cpp        |  3 +-
 src/tests/cluster.hpp        | 17 ++++++++--
 src/tests/mock_registrar.cpp | 71 +++++++++++++++++++++++++++++++++++++++
 src/tests/mock_registrar.hpp | 60 +++++++++++++++++++++++++++++++++
 6 files changed, 151 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/80993da4/src/Makefile.am
----------------------------------------------------------------------
diff --git a/src/Makefile.am b/src/Makefile.am
index 6fb095f..478fb5a 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -991,6 +991,7 @@ libmesos_no_3rdparty_la_SOURCES +=                          
        \
   tests/mesos.hpp                                                      \
   tests/mock_docker.hpp                                                        
\
   tests/mock_slave.hpp                                                 \
+  tests/mock_registrar.hpp                                             \
   tests/module.hpp                                                     \
   tests/script.hpp                                                     \
   tests/utils.hpp                                                      \
@@ -2107,6 +2108,7 @@ mesos_tests_SOURCES =                                     
        \
   tests/metrics_tests.cpp                                      \
   tests/mock_docker.cpp                                                \
   tests/mock_slave.cpp                                         \
+  tests/mock_registrar.cpp                                     \
   tests/module.cpp                                             \
   tests/module_tests.cpp                                       \
   tests/oversubscription_tests.cpp                             \

http://git-wip-us.apache.org/repos/asf/mesos/blob/80993da4/src/master/registrar.hpp
----------------------------------------------------------------------
diff --git a/src/master/registrar.hpp b/src/master/registrar.hpp
index c39dd1b..238a960 100644
--- a/src/master/registrar.hpp
+++ b/src/master/registrar.hpp
@@ -106,7 +106,7 @@ public:
   Registrar(const Flags& flags,
             mesos::state::protobuf::State* state,
             const Option<std::string>& authenticationRealm = None());
-  ~Registrar();
+  virtual ~Registrar();
 
   // Recovers the Registry, persisting the new Master information.
   // The Registrar must be recovered to allow other operations to
@@ -123,7 +123,7 @@ public:
   //   false if the operation is not permitted.
   //   Failure if the operation fails (possibly lost log leadership),
   //     or recovery failed.
-  process::Future<bool> apply(process::Owned<Operation> operation);
+  virtual process::Future<bool> apply(process::Owned<Operation> operation);
 
   // Gets the pid of the underlying process.
   // Used in tests.

http://git-wip-us.apache.org/repos/asf/mesos/blob/80993da4/src/tests/cluster.cpp
----------------------------------------------------------------------
diff --git a/src/tests/cluster.cpp b/src/tests/cluster.cpp
index b04653a..7152ac3 100644
--- a/src/tests/cluster.cpp
+++ b/src/tests/cluster.cpp
@@ -88,6 +88,7 @@
 #include "slave/containerizer/fetcher.hpp"
 
 #include "tests/cluster.hpp"
+#include "tests/mock_registrar.hpp"
 
 using mesos::master::contender::StandaloneMasterContender;
 using mesos::master::contender::ZooKeeperMasterContender;
@@ -243,7 +244,7 @@ Try<process::Owned<Master>> Master::start(
 
   // Instantiate some other master dependencies.
   master->state.reset(new 
mesos::state::protobuf::State(master->storage.get()));
-  master->registrar.reset(new master::Registrar(
+  master->registrar.reset(new MockRegistrar(
       flags, master->state.get(), master::READONLY_HTTP_AUTHENTICATION_REALM));
 
   if (slaveRemovalLimiter.isNone() && flags.agent_removal_rate_limit.isSome()) 
{

http://git-wip-us.apache.org/repos/asf/mesos/blob/80993da4/src/tests/cluster.hpp
----------------------------------------------------------------------
diff --git a/src/tests/cluster.hpp b/src/tests/cluster.hpp
index c6fbbf2..250b12f 100644
--- a/src/tests/cluster.hpp
+++ b/src/tests/cluster.hpp
@@ -53,7 +53,6 @@
 #include "master/constants.hpp"
 #include "master/flags.hpp"
 #include "master/master.hpp"
-#include "master/registrar.hpp"
 
 #include "slave/constants.hpp"
 #include "slave/flags.hpp"
@@ -64,6 +63,7 @@
 #include "slave/containerizer/containerizer.hpp"
 #include "slave/containerizer/fetcher.hpp"
 
+#include "tests/mock_registrar.hpp"
 
 namespace mesos {
 namespace internal {
@@ -111,7 +111,12 @@ private:
   Option<zookeeper::URL> zookeeperUrl;
   Files files;
 
-  // Dependencies that are created by the factory method.
+  // Dependencies that are created by the factory method. The order in
+  // which these fields are declared should match the order in which
+  // they are initialized by the constructor; this ensures that
+  // dependencies between these fields are handled correctly during
+  // destruction.
+
   process::Owned<mesos::allocator::Allocator> allocator;
   process::Owned<Authorizer> authorizer;
   process::Owned<mesos::master::contender::MasterContender> contender;
@@ -119,8 +124,14 @@ private:
   process::Owned<mesos::log::Log> log;
   process::Owned<mesos::state::Storage> storage;
   process::Owned<mesos::state::protobuf::State> state;
-  process::Owned<master::Registrar> registrar;
+public:
+  // Exposed for testing and mocking purposes. We always use a
+  // `MockRegistrar` in case the test case wants to inspect how the
+  // master interacts with the registrar; by default, the mock
+  // registrar behaves identically to the normal registrar.
+  process::Owned<MockRegistrar> registrar;
 
+private:
   Option<std::shared_ptr<process::RateLimiter>> slaveRemovalLimiter;
 
   // Indicates whether or not authorization callbacks were set when this master

http://git-wip-us.apache.org/repos/asf/mesos/blob/80993da4/src/tests/mock_registrar.cpp
----------------------------------------------------------------------
diff --git a/src/tests/mock_registrar.cpp b/src/tests/mock_registrar.cpp
new file mode 100644
index 0000000..8643e4c
--- /dev/null
+++ b/src/tests/mock_registrar.cpp
@@ -0,0 +1,71 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <string>
+
+#include <gmock/gmock.h>
+
+#include <mesos/state/protobuf.hpp>
+
+#include <process/future.hpp>
+#include <process/owned.hpp>
+
+#include <stout/none.hpp>
+#include <stout/option.hpp>
+
+#include "master/flags.hpp"
+#include "master/registrar.hpp"
+
+#include "tests/mock_registrar.hpp"
+
+using std::string;
+
+using testing::_;
+using testing::DoDefault;
+using testing::Invoke;
+
+using process::Future;
+using process::Owned;
+
+namespace mesos {
+namespace internal {
+namespace tests {
+
+MockRegistrar::MockRegistrar(
+    const master::Flags& flags,
+    mesos::state::protobuf::State* state,
+    const Option<string>& authenticationRealm)
+  : Registrar(flags, state, authenticationRealm)
+{
+  // The default behavior is to call the original method.
+  ON_CALL(*this, apply(_))
+    .WillByDefault(Invoke(this, &MockRegistrar::unmocked_apply));
+  EXPECT_CALL(*this, apply(_))
+    .WillRepeatedly(DoDefault());
+}
+
+
+MockRegistrar::~MockRegistrar() {}
+
+
+Future<bool> MockRegistrar::unmocked_apply(Owned<master::Operation> operation)
+{
+  return master::Registrar::apply(operation);
+}
+
+} // namespace tests {
+} // namespace internal {
+} // namespace mesos {

http://git-wip-us.apache.org/repos/asf/mesos/blob/80993da4/src/tests/mock_registrar.hpp
----------------------------------------------------------------------
diff --git a/src/tests/mock_registrar.hpp b/src/tests/mock_registrar.hpp
new file mode 100644
index 0000000..cdcc699
--- /dev/null
+++ b/src/tests/mock_registrar.hpp
@@ -0,0 +1,60 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef __TESTS_MOCK_REGISTRAR_HPP__
+#define __TESTS_MOCK_REGISTRAR_HPP__
+
+#include <string>
+
+#include <gmock/gmock.h>
+
+#include <mesos/state/protobuf.hpp>
+
+#include <process/future.hpp>
+#include <process/owned.hpp>
+
+#include <stout/none.hpp>
+#include <stout/option.hpp>
+
+#include "master/flags.hpp"
+#include "master/registrar.hpp"
+
+namespace mesos {
+namespace internal {
+namespace tests {
+
+class MockRegistrar : public mesos::internal::master::Registrar
+{
+public:
+  MockRegistrar(const master::Flags& flags,
+                mesos::state::protobuf::State* state,
+                const Option<std::string>& authenticationRealm = None());
+
+  virtual ~MockRegistrar();
+
+  MOCK_METHOD1(
+      apply,
+      process::Future<bool>(process::Owned<master::Operation> operation));
+
+  process::Future<bool> unmocked_apply(
+      process::Owned<master::Operation> operation);
+};
+
+} // namespace tests {
+} // namespace internal {
+} // namespace mesos {
+
+#endif // __TESTS_MOCK_REGISTRAR_HPP__

Reply via email to