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 <[email protected]> Authored: Mon Sep 19 15:48:02 2016 -0700 Committer: Vinod Kone <[email protected]> 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__
