[mesos] branch master updated: Avoided copying some `Owned` values.

[bb...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

bbannier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git


The following commit(s) were added to refs/heads/master by this push:
     new e8786c2  Avoided copying some `Owned` values.
e8786c2 is described below

commit e8786c26353cddf91e06ff89e9e100cc0b9f4ab1
Author: Benjamin Bannier <be...@mesosphere.io>
AuthorDate: Mon Jan 14 09:50:51 2019 +0100

    Avoided copying some `Owned` values.
    
    The resource provider manager's `start` method takes ownership of a
    passed `Owned` value. Since `Owned` values are not shared (but owned),
    they cannot be copied (at least semantically until MESOS-5122 is
    implemented).
    
    This patch fixes existing code to make sure we do not copy `Owned`
    values when invoking `ResourceProviderManager::start`. This should be a
    non-functional change modulo existing bugs.
    
    Review: https://reviews.apache.org/r/69718/
---
 src/tests/api_tests.cpp                         |  9 +++++----
 src/tests/master_slave_reconciliation_tests.cpp |  3 ++-
 src/tests/master_tests.cpp                      |  5 +++--
 src/tests/operation_reconciliation_tests.cpp    |  6 ++++--
 src/tests/resource_provider_manager_tests.cpp   | 21 +++++++++++++--------
 src/tests/slave_tests.cpp                       |  9 +++++----
 6 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/src/tests/api_tests.cpp b/src/tests/api_tests.cpp
index c597243..79a2d29 100644
--- a/src/tests/api_tests.cpp
+++ b/src/tests/api_tests.cpp
@@ -16,6 +16,7 @@
 
 #include <string>
 #include <tuple>
+#include <utility>
 
 #include <mesos/http.hpp>
 
@@ -236,7 +237,7 @@ TEST_P(MasterAPITest, GetAgents)
 
   updateAgentMessage = FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
 
-  resourceProvider.start(endpointDetector, contentType);
+  resourceProvider.start(std::move(endpointDetector), contentType);
 
   // Wait until the agent's resources have been updated to include the
   // resource provider resources.
@@ -1108,7 +1109,7 @@ TEST_P(MasterAPITest, GetOperations)
 
   const ContentType contentType = GetParam();
 
-  resourceProvider.start(endpointDetector, contentType);
+  resourceProvider.start(std::move(endpointDetector), contentType);
 
   // Wait until the agent's resources have been updated to include the
   // resource provider resources.
@@ -8146,7 +8147,7 @@ TEST_P(AgentAPITest, GetResourceProviders)
 
   updateSlaveMessage = FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
 
-  resourceProvider.start(endpointDetector, contentType);
+  resourceProvider.start(std::move(endpointDetector), contentType);
 
   // Wait until the agent's resources have been updated to include the
   // resource provider resources.
@@ -8239,7 +8240,7 @@ TEST_P(AgentAPITest, GetOperations)
 
   const ContentType contentType = GetParam();
 
-  resourceProvider.start(endpointDetector, contentType);
+  resourceProvider.start(std::move(endpointDetector), contentType);
 
   // Wait until the agent's resources have been updated to include the
   // resource provider resources.
diff --git a/src/tests/master_slave_reconciliation_tests.cpp b/src/tests/master_slave_reconciliation_tests.cpp
index 979048c..002be27 100644
--- a/src/tests/master_slave_reconciliation_tests.cpp
+++ b/src/tests/master_slave_reconciliation_tests.cpp
@@ -14,6 +14,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <utility>
 #include <vector>
 
 #include <gmock/gmock.h>
@@ -523,7 +524,7 @@ TEST_F(
 
   ContentType contentType = ContentType::PROTOBUF;
 
-  resourceProvider->start(endpointDetector, contentType);
+  resourceProvider->start(std::move(endpointDetector), contentType);
 
   // Wait until the agent's resources have been updated to include the
   // resource provider resources.
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index 67713c8..1f8da63 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -18,6 +18,7 @@
 
 #include <memory>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include <gmock/gmock.h>
@@ -8983,7 +8984,7 @@ TEST_F(MasterTest, UpdateSlaveMessageWithPendingOffers)
   Owned<EndpointDetector> endpointDetector(
       resource_provider::createEndpointDetector(agent.get()->pid));
 
-  resourceProvider->start(endpointDetector, ContentType::PROTOBUF);
+  resourceProvider->start(std::move(endpointDetector), ContentType::PROTOBUF);
 
   AWAIT_READY(updateSlaveMessage);
   ASSERT_TRUE(resourceProvider->info.has_id());
@@ -9104,7 +9105,7 @@ TEST_F(MasterTest, OperationUpdateDuringFailover)
 
   updateSlaveMessage = FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
 
-  resourceProvider.start(endpointDetector, ContentType::PROTOBUF);
+  resourceProvider.start(std::move(endpointDetector), ContentType::PROTOBUF);
 
   AWAIT_READY(updateSlaveMessage);
 
diff --git a/src/tests/operation_reconciliation_tests.cpp b/src/tests/operation_reconciliation_tests.cpp
index a5e81a1..9a1fc95 100644
--- a/src/tests/operation_reconciliation_tests.cpp
+++ b/src/tests/operation_reconciliation_tests.cpp
@@ -14,6 +14,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <utility>
+
 #include <gmock/gmock.h>
 
 #include <mesos/mesos.hpp>
@@ -115,7 +117,7 @@ TEST_P(OperationReconciliationTest, PendingOperation)
 
   ContentType contentType = GetParam();
 
-  resourceProvider->start(endpointDetector, contentType);
+  resourceProvider->start(std::move(endpointDetector), contentType);
 
   // Wait until the agent's resources have been updated to include the
   // resource provider resources.
@@ -704,7 +706,7 @@ TEST_P(OperationReconciliationTest, AgentPendingOperationAfterMasterFailover)
 
   ContentType contentType = GetParam();
 
-  resourceProvider->start(endpointDetector, contentType);
+  resourceProvider->start(std::move(endpointDetector), contentType);
 
   // Wait until the agent's resources have been updated to include the
   // resource provider resources.
diff --git a/src/tests/resource_provider_manager_tests.cpp b/src/tests/resource_provider_manager_tests.cpp
index 20cfb34..455ce7d 100644
--- a/src/tests/resource_provider_manager_tests.cpp
+++ b/src/tests/resource_provider_manager_tests.cpp
@@ -15,6 +15,7 @@
 // limitations under the License.
 
 #include <string>
+#include <utility>
 #include <vector>
 
 #include <gtest/gtest.h>
@@ -1023,7 +1024,7 @@ TEST_P(ResourceProviderManagerHttpApiTest, ConvertResources)
 
   const ContentType contentType = GetParam();
 
-  resourceProvider.start(endpointDetector, contentType);
+  resourceProvider.start(std::move(endpointDetector), contentType);
 
   // Wait until the agent's resources have been updated to include the
   // resource provider resources.
@@ -1156,7 +1157,7 @@ TEST_P_TEMP_DISABLED_ON_WINDOWS(
 
   const ContentType contentType = GetParam();
 
-  resourceProvider->start(endpointDetector, contentType);
+  resourceProvider->start(std::move(endpointDetector), contentType);
 
   // Wait until the agent's resources have been updated to include the
   // resource provider resources. At this point the resource provider
@@ -1176,7 +1177,9 @@ TEST_P_TEMP_DISABLED_ON_WINDOWS(
   EXPECT_CALL(*resourceProvider, subscribed(_))
     .WillOnce(FutureArg<0>(&subscribed1));
 
-  resourceProvider->start(endpointDetector, contentType);
+  endpointDetector =
+    resource_provider::createEndpointDetector(agent.get()->pid);
+  resourceProvider->start(std::move(endpointDetector), contentType);
 
   AWAIT_READY(subscribed1);
   EXPECT_EQ(resourceProviderInfo.id(), subscribed1->provider_id());
@@ -1216,7 +1219,7 @@ TEST_P_TEMP_DISABLED_ON_WINDOWS(
   EXPECT_CALL(*resourceProvider, subscribed(_))
     .WillOnce(FutureArg<0>(&subscribed2));
 
-  resourceProvider->start(endpointDetector, contentType);
+  resourceProvider->start(std::move(endpointDetector), contentType);
 
   AWAIT_READY(subscribed2);
   EXPECT_EQ(resourceProviderInfo.id(), subscribed2->provider_id());
@@ -1275,7 +1278,7 @@ TEST_P(ResourceProviderManagerHttpApiTest, ResubscribeUnknownID)
 
   const ContentType contentType = GetParam();
 
-  resourceProvider->start(endpointDetector, contentType);
+  resourceProvider->start(std::move(endpointDetector), contentType);
 
   AWAIT_READY(disconnected);
 }
@@ -1328,7 +1331,7 @@ TEST_P(ResourceProviderManagerHttpApiTest, ResourceProviderDisconnect)
 
   const ContentType contentType = GetParam();
 
-  resourceProvider->start(endpointDetector, contentType);
+  resourceProvider->start(std::move(endpointDetector), contentType);
 
   {
     // Wait until the agent's resources have been updated to include
@@ -1397,7 +1400,7 @@ TEST_F(ResourceProviderManagerHttpApiTest, ResourceProviderSubscribeDisconnect)
   EXPECT_CALL(*resourceProvider1, subscribed(_))
     .WillOnce(FutureArg<0>(&subscribed1));
 
-  resourceProvider1->start(endpointDetector, ContentType::PROTOBUF);
+  resourceProvider1->start(std::move(endpointDetector), ContentType::PROTOBUF);
 
   AWAIT_READY(subscribed1);
 
@@ -1423,7 +1426,9 @@ TEST_F(ResourceProviderManagerHttpApiTest, ResourceProviderSubscribeDisconnect)
   EXPECT_CALL(*resourceProvider2, subscribed(_))
     .WillOnce(FutureArg<0>(&subscribed2));
 
-  resourceProvider2->start(endpointDetector, ContentType::PROTOBUF);
+  endpointDetector =
+    resource_provider::createEndpointDetector(agent.get()->pid);
+  resourceProvider2->start(std::move(endpointDetector), ContentType::PROTOBUF);
 
   AWAIT_READY(disconnected1);
   AWAIT_READY(subscribed2);
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index 4aed5d6..dc711de 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -22,6 +22,7 @@
 #include <map>
 #include <memory>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include <gmock/gmock.h>
@@ -10500,7 +10501,7 @@ TEST_F(SlaveTest, ResourceProviderSubscribe)
   Owned<EndpointDetector> endpointDetector(
       resource_provider::createEndpointDetector(slave.get()->pid));
 
-  resourceProvider.start(endpointDetector, ContentType::PROTOBUF);
+  resourceProvider.start(std::move(endpointDetector), ContentType::PROTOBUF);
 
   AWAIT_READY(connected);
 
@@ -10605,7 +10606,7 @@ TEST_F(SlaveTest, ResourceProviderPublishAll)
 
   updateSlaveMessage = FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
 
-  resourceProvider.start(endpointDetector, ContentType::PROTOBUF);
+  resourceProvider.start(std::move(endpointDetector), ContentType::PROTOBUF);
 
   AWAIT_READY(updateSlaveMessage);
 
@@ -10897,7 +10898,7 @@ TEST_F(SlaveTest, ResourceProviderReconciliation)
 
   updateSlaveMessage = FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
 
-  resourceProvider.start(endpointDetector, ContentType::PROTOBUF);
+  resourceProvider.start(std::move(endpointDetector), ContentType::PROTOBUF);
 
   AWAIT_READY(updateSlaveMessage);
 
@@ -11104,7 +11105,7 @@ TEST_F(SlaveTest, RunTaskResourceVersions)
 
   updateSlaveMessage = FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
 
-  resourceProvider.start(endpointDetector, ContentType::PROTOBUF);
+  resourceProvider.start(std::move(endpointDetector), ContentType::PROTOBUF);
 
   AWAIT_READY(updateSlaveMessage);