You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by be...@apache.org on 2016/10/04 02:20:49 UTC

[3/4] mesos git commit: Merged TestIsolator and MockIsolator and added a comment.

Merged TestIsolator and MockIsolator and added a comment.

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


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

Branch: refs/heads/master
Commit: 8ecea16118f957e358f96ccb0981b2fec18c5656
Parents: 66d4b26
Author: Benjamin Hindman <be...@gmail.com>
Authored: Sun Oct 2 20:06:10 2016 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Mon Oct 3 19:20:26 2016 -0700

----------------------------------------------------------------------
 src/tests/containerizer/isolator.hpp            | 87 +++++++++++++-------
 .../containerizer/mesos_containerizer_tests.cpp | 75 ++---------------
 2 files changed, 64 insertions(+), 98 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/8ecea161/src/tests/containerizer/isolator.hpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/isolator.hpp b/src/tests/containerizer/isolator.hpp
index fbe80aa..86664a8 100644
--- a/src/tests/containerizer/isolator.hpp
+++ b/src/tests/containerizer/isolator.hpp
@@ -25,16 +25,62 @@ namespace mesos {
 namespace internal {
 namespace tests {
 
-class TestIsolatorProcess : public slave::MesosIsolatorProcess
+// Provides a mock Isolator that by default expects calls to
+// Isolator::prepare, Isolator::isolate, Isolator::watch, and
+// Isolator::cleanup and simply returns "nothing" as appropriate for
+// each call. This behavior can be overriden by adding EXPECT_CALL as
+// necessary. For example, if you don't expect any calls to
+// Isolator::cleanup you can do:
+//
+//   MockIsolator isolator;
+//   EXPECT_CALL(isolator, prepare(_, _))
+//    .Times(0);
+//
+// Or if you want to override that only a single invocation should
+// occur you can do:
+//
+//   MockIsolator isolator;
+//   EXPECT_CALL(isolator, prepare(_, _))
+//    .WillOnce(Return(ContainerLaunchInfo(...)));
+//
+// But note that YOU MUST use exactly `prepare(_, _)` otherwise gmock
+// will not properly match this new expectation with the default
+// expectation created by MockIsolator.
+//
+// In the event you want to override a single invocation but let all
+// subsequent invocations return the "default" you can do:
+//
+//   MockIsolator isolator;
+//   EXPECT_CALL(isolator, prepare(_, _))
+//    .WillOnce(Return(ContainerLaunchInfo(...)))
+//    .RetiresOnSaturation();
+//
+// Another example, if you want to override what gets returned for a
+// every invocation you can do:
+//
+//   MockIsolator isolator;
+//   EXPECT_CALL(isolator, prepare(_, _))
+//    .WillRepeatedly(Return(Failure(...)));
+//
+// Again, YOU MUST use exactly `prepare(_, _)` to override the default
+// expectation.
+class MockIsolator : public mesos::slave::Isolator
 {
 public:
-  static Try<mesos::slave::Isolator*> create(
-      const Option<mesos::slave::ContainerLaunchInfo>& prepare)
+  MockIsolator()
   {
-    process::Owned<MesosIsolatorProcess> process(
-        new TestIsolatorProcess(prepare));
+    EXPECT_CALL(*this, prepare(_, _))
+      .WillRepeatedly(Return(None()));
+
+    EXPECT_CALL(*this, isolate(_, _))
+      .WillRepeatedly(Return(Nothing()));
+
+    EXPECT_CALL(*this, watch(_))
+      .WillRepeatedly(
+          Return(process::Future<mesos::slave::ContainerLimitation>()));
 
-    return new slave::MesosIsolator(process);
+    EXPECT_CALL(*this, cleanup(_))
+      .WillRepeatedly(Return(Nothing()));
   }
 
   MOCK_METHOD2(
@@ -43,12 +89,11 @@ public:
           const std::list<mesos::slave::ContainerState>&,
           const hashset<ContainerID>&));
 
-  virtual process::Future<Option<mesos::slave::ContainerLaunchInfo>> prepare(
-      const ContainerID& containerId,
-      const mesos::slave::ContainerConfig& containerConfig)
-  {
-    return launchInfo;
-  }
+  MOCK_METHOD2(
+      prepare,
+      process::Future<Option<mesos::slave::ContainerLaunchInfo>>(
+          const ContainerID&,
+          const mesos::slave::ContainerConfig&));
 
   MOCK_METHOD2(
       isolate,
@@ -69,24 +114,6 @@ public:
   MOCK_METHOD1(
       cleanup,
       process::Future<Nothing>(const ContainerID&));
-
-private:
-  TestIsolatorProcess( const Option<mesos::slave::ContainerLaunchInfo>& info)
-    : launchInfo(info)
-  {
-    EXPECT_CALL(*this, watch(testing::_))
-      .WillRepeatedly(testing::Return(promise.future()));
-
-    EXPECT_CALL(*this, isolate(testing::_, testing::_))
-      .WillRepeatedly(testing::Return(Nothing()));
-
-    EXPECT_CALL(*this, cleanup(testing::_))
-      .WillRepeatedly(testing::Return(Nothing()));
-  }
-
-  const Option<mesos::slave::ContainerLaunchInfo> launchInfo;
-
-  process::Promise<mesos::slave::ContainerLimitation> promise;
 };
 
 } // namespace tests {

http://git-wip-us.apache.org/repos/asf/mesos/blob/8ecea161/src/tests/containerizer/mesos_containerizer_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/mesos_containerizer_tests.cpp b/src/tests/containerizer/mesos_containerizer_tests.cpp
index f64ec5d..7872c74 100644
--- a/src/tests/containerizer/mesos_containerizer_tests.cpp
+++ b/src/tests/containerizer/mesos_containerizer_tests.cpp
@@ -201,8 +201,8 @@ TEST_F(MesosContainerizerTest, Destroy)
 class MesosContainerizerIsolatorPreparationTest : public MesosTest
 {
 public:
-  // Construct a MesosContainerizer with TestIsolator(s) which use the provided
-  // 'prepare' command(s).
+  // Construct a MesosContainerizer with MockIsolator(s) which return
+  // the provided ContainerLaunchInfo for Isolator::prepare.
   Try<Owned<MesosContainerizer>> CreateContainerizer(
       Fetcher* fetcher,
       const vector<Option<ContainerLaunchInfo>>& launchInfos)
@@ -210,12 +210,12 @@ public:
     vector<Owned<Isolator>> isolators;
 
     foreach (const Option<ContainerLaunchInfo>& launchInfo, launchInfos) {
-      Try<Isolator*> isolator = TestIsolatorProcess::create(launchInfo);
-      if (isolator.isError()) {
-        return Error(isolator.error());
-      }
+      MockIsolator* isolator = new MockIsolator();
 
-      isolators.push_back(Owned<Isolator>(isolator.get()));
+      EXPECT_CALL(*isolator, prepare(_, _))
+        .WillOnce(Return(launchInfo));
+
+      isolators.push_back(Owned<Isolator>(isolator));
     }
 
     slave::Flags flags = CreateSlaveFlags();
@@ -628,67 +628,6 @@ public:
 };
 
 
-class MockIsolator : public mesos::slave::Isolator
-{
-public:
-  MockIsolator()
-  {
-    EXPECT_CALL(*this, watch(_))
-      .WillRepeatedly(Return(watchPromise.future()));
-
-    EXPECT_CALL(*this, isolate(_, _))
-      .WillRepeatedly(Return(Nothing()));
-
-    EXPECT_CALL(*this, cleanup(_))
-      .WillRepeatedly(Return(Nothing()));
-
-    EXPECT_CALL(*this, prepare(_, _))
-      .WillRepeatedly(Invoke(this, &MockIsolator::_prepare));
-  }
-
-  MOCK_METHOD2(
-      recover,
-      Future<Nothing>(
-          const list<ContainerState>&,
-          const hashset<ContainerID>&));
-
-  MOCK_METHOD2(
-      prepare,
-      Future<Option<ContainerLaunchInfo>>(
-          const ContainerID&,
-          const ContainerConfig&));
-
-  virtual Future<Option<ContainerLaunchInfo>> _prepare(
-      const ContainerID& containerId,
-      const ContainerConfig& containerConfig)
-  {
-    return None();
-  }
-
-  MOCK_METHOD2(
-      isolate,
-      Future<Nothing>(const ContainerID&, pid_t));
-
-  MOCK_METHOD1(
-      watch,
-      Future<mesos::slave::ContainerLimitation>(const ContainerID&));
-
-  MOCK_METHOD2(
-      update,
-      Future<Nothing>(const ContainerID&, const Resources&));
-
-  MOCK_METHOD1(
-      usage,
-      Future<ResourceStatistics>(const ContainerID&));
-
-  MOCK_METHOD1(
-      cleanup,
-      Future<Nothing>(const ContainerID&));
-
-  Promise<mesos::slave::ContainerLimitation> watchPromise;
-};
-
-
 // Destroying a mesos containerizer while it is fetching should
 // complete without waiting for the fetching to finish.
 TEST_F(MesosContainerizerDestroyTest, DestroyWhileFetching)