Adjusted existing tests to use MockRegistrar. This means that `Master::_reregisterSlave` can be made private.
Review: https://reviews.apache.org/r/51376/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/87b94314 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/87b94314 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/87b94314 Branch: refs/heads/master Commit: 87b94314b1490ef7035e9ec5e8e071dfc1f12309 Parents: 80993da Author: Neil Conway <neil.con...@gmail.com> Authored: Mon Sep 19 15:48:06 2016 -0700 Committer: Vinod Kone <vinodk...@gmail.com> Committed: Mon Sep 19 15:48:06 2016 -0700 ---------------------------------------------------------------------- src/master/master.hpp | 27 +++++++++++---------------- src/tests/master_tests.cpp | 16 ++++++++++++---- src/tests/reconciliation_tests.cpp | 21 ++++++++++++++------- 3 files changed, 37 insertions(+), 27 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/87b94314/src/master/master.hpp ---------------------------------------------------------------------- diff --git a/src/master/master.hpp b/src/master/master.hpp index 714aa79..370e0f0 100644 --- a/src/master/master.hpp +++ b/src/master/master.hpp @@ -555,22 +555,6 @@ public: // Made public for testing purposes. process::Future<Nothing> _recover(const Registry& registry); - // Continuation of reregisterSlave(). - // Made public for testing purposes. - // TODO(vinod): Instead of doing this create and use a - // MockRegistrar. - // TODO(dhamon): Consider FRIEND_TEST macro from gtest. - void _reregisterSlave( - const SlaveInfo& slaveInfo, - const process::UPID& pid, - const std::vector<Resource>& checkpointedResources, - const std::vector<ExecutorInfo>& executorInfos, - const std::vector<Task>& tasks, - const std::vector<FrameworkInfo>& frameworks, - const std::vector<Archive::Framework>& completedFrameworks, - const std::string& version, - const process::Future<bool>& readmit); - MasterInfo info() const { return info_; @@ -620,6 +604,17 @@ protected: const std::string& version, const process::Future<bool>& admit); + void _reregisterSlave( + const SlaveInfo& slaveInfo, + const process::UPID& pid, + const std::vector<Resource>& checkpointedResources, + const std::vector<ExecutorInfo>& executorInfos, + const std::vector<Task>& tasks, + const std::vector<FrameworkInfo>& frameworks, + const std::vector<Archive::Framework>& completedFrameworks, + const std::string& version, + const process::Future<bool>& readmit); + void __reregisterSlave( Slave* slave, const std::vector<Task>& tasks, http://git-wip-us.apache.org/repos/asf/mesos/blob/87b94314/src/tests/master_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp index 85ebd57..737f589 100644 --- a/src/tests/master_tests.cpp +++ b/src/tests/master_tests.cpp @@ -535,9 +535,6 @@ TEST_F(MasterTest, KillUnknownTaskSlaveInTransition) EXPECT_CALL(exec, shutdown(_)) .Times(AtMost(1)); - Future<Nothing> _reregisterSlave = - DROP_DISPATCH(_, &Master::_reregisterSlave); - // Stop master and slave. master->reset(); slave.get()->terminate(); @@ -551,6 +548,14 @@ TEST_F(MasterTest, KillUnknownTaskSlaveInTransition) master = StartMaster(); ASSERT_SOME(master); + // Intercept the first registrar operation that is attempted; this + // should be the registry operation that reregisters the slave. + Future<Owned<master::Operation>> reregister; + Promise<bool> promise; // Never satisfied. + EXPECT_CALL(*master.get()->registrar.get(), apply(_)) + .WillOnce(DoAll(FutureArg<0>(&reregister), + Return(promise.future()))); + frameworkId = Future<FrameworkID>(); EXPECT_CALL(sched, registered(&driver, _, _)) .WillOnce(FutureArg<1>(&frameworkId)); @@ -567,7 +572,10 @@ TEST_F(MasterTest, KillUnknownTaskSlaveInTransition) ASSERT_SOME(slave); // Wait for the slave to start reregistration. - AWAIT_READY(_reregisterSlave); + AWAIT_READY(reregister); + EXPECT_NE( + nullptr, + dynamic_cast<master::MarkSlaveReachable*>(reregister.get().get())); // As Master::killTask isn't doing anything, we shouldn't get a status update. EXPECT_CALL(sched, statusUpdate(&driver, _)) http://git-wip-us.apache.org/repos/asf/mesos/blob/87b94314/src/tests/reconciliation_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/reconciliation_tests.cpp b/src/tests/reconciliation_tests.cpp index a043356..9828ab9 100644 --- a/src/tests/reconciliation_tests.cpp +++ b/src/tests/reconciliation_tests.cpp @@ -28,6 +28,7 @@ #include <process/clock.hpp> #include <process/future.hpp> +#include <process/gmock.hpp> #include <process/owned.hpp> #include <process/pid.hpp> #include <process/process.hpp> @@ -412,15 +413,18 @@ TEST_F(ReconciliationTest, SlaveInTransition) EXPECT_CALL(sched, statusUpdate(&driver, _)) .Times(0); - // Drop '&Master::_reregisterSlave' dispatch so that the slave is - // in 'reregistering' state. - Future<Nothing> _reregisterSlave = - DROP_DISPATCH(_, &Master::_reregisterSlave); - // Restart the master. master = StartMaster(masterFlags); ASSERT_SOME(master); + // Intercept the first registrar operation that is attempted; this + // should be the registry operation that reregisters the slave. + Future<Owned<master::Operation>> reregister; + Promise<bool> promise; // Never satisfied. + EXPECT_CALL(*master.get()->registrar.get(), apply(_)) + .WillOnce(DoAll(FutureArg<0>(&reregister), + Return(promise.future()))); + driver.start(); // Wait for the framework to register. @@ -431,8 +435,11 @@ TEST_F(ReconciliationTest, SlaveInTransition) slave = StartSlave(detector.get(), slaveFlags); ASSERT_SOME(slave); - // Slave will be in 'reregistering' state here. - AWAIT_READY(_reregisterSlave); + // Wait for the slave to start reregistration. + AWAIT_READY(reregister); + EXPECT_NE( + nullptr, + dynamic_cast<master::MarkSlaveReachable*>(reregister.get().get())); Future<mesos::scheduler::Call> reconcileCall = FUTURE_CALL( mesos::scheduler::Call(), mesos::scheduler::Call::RECONCILE, _ , _);
