fw: FaceTable iterator dereferences to Face& instead of shared_ptr

This commit also improves the speed and reliability of
Mgmt/TestFaceManager/CreateFace test suite.

refs #3205

Change-Id: Idd013488ced2d7f8072ef8a3d910f94da2e0c8ac
diff --git a/tests/daemon/fw/face-table.t.cpp b/tests/daemon/fw/face-table.t.cpp
index 4616efb..e8962b8 100644
--- a/tests/daemon/fw/face-table.t.cpp
+++ b/tests/daemon/fw/face-table.t.cpp
@@ -113,41 +113,31 @@
   BOOST_CHECK_EQUAL(std::distance(faceTable.begin(), faceTable.end()), faceTable.size());
   hasFace1 = hasFace2 = false;
   for (FaceTable::const_iterator it = faceTable.begin(); it != faceTable.end(); ++it) {
-    if (*it == face1) {
-      hasFace1 = true;
-    }
+    hasFace1 = hasFace1 || &*it == face1.get();
   }
-  BOOST_CHECK(hasFace1);
+  BOOST_CHECK_EQUAL(hasFace1, true);
 
   faceTable.add(face2);
   BOOST_CHECK_EQUAL(faceTable.size(), 2);
   BOOST_CHECK_EQUAL(std::distance(faceTable.begin(), faceTable.end()), faceTable.size());
   hasFace1 = hasFace2 = false;
   for (FaceTable::const_iterator it = faceTable.begin(); it != faceTable.end(); ++it) {
-    if (*it == face1) {
-      hasFace1 = true;
-    }
-    if (*it == face2) {
-      hasFace2 = true;
-    }
+    hasFace1 = hasFace1 || &*it == face1.get();
+    hasFace2 = hasFace2 || &*it == face2.get();
   }
-  BOOST_CHECK(hasFace1);
-  BOOST_CHECK(hasFace2);
+  BOOST_CHECK_EQUAL(hasFace1, true);
+  BOOST_CHECK_EQUAL(hasFace2, true);
 
   face1->close();
   BOOST_CHECK_EQUAL(faceTable.size(), 1);
   BOOST_CHECK_EQUAL(std::distance(faceTable.begin(), faceTable.end()), faceTable.size());
   hasFace1 = hasFace2 = false;
   for (FaceTable::const_iterator it = faceTable.begin(); it != faceTable.end(); ++it) {
-    if (*it == face1) {
-      hasFace1 = true;
-    }
-    if (*it == face2) {
-      hasFace2 = true;
-    }
+    hasFace1 = hasFace1 || &*it == face1.get();
+    hasFace2 = hasFace2 || &*it == face2.get();
   }
-  BOOST_CHECK(!hasFace1);
-  BOOST_CHECK(hasFace2);
+  BOOST_CHECK_EQUAL(hasFace1, false);
+  BOOST_CHECK_EQUAL(hasFace2, true);
 }
 
 BOOST_AUTO_TEST_SUITE_END() // TestFaceTable
diff --git a/tests/daemon/fw/strategy.t.cpp b/tests/daemon/fw/strategy.t.cpp
index 90654d7..7a15df4 100644
--- a/tests/daemon/fw/strategy.t.cpp
+++ b/tests/daemon/fw/strategy.t.cpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /**
- * Copyright (c) 2014-2015,  Regents of the University of California,
+ * Copyright (c) 2014-2016,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -27,6 +27,8 @@
 #include "dummy-strategy.hpp"
 #include "tests/daemon/face/dummy-face.hpp"
 #include <boost/range/adaptor/filtered.hpp>
+#include <boost/range/adaptor/transformed.hpp>
+#include <boost/range/algorithm/copy.hpp>
 
 #include "tests/test-common.hpp"
 
@@ -58,14 +60,15 @@
   getLocalFaces()
   {
     auto enumerable = this->getFaceTable() |
-                      boost::adaptors::filtered([] (shared_ptr<Face> face) {
-                        return face->getScope() == ndn::nfd::FACE_SCOPE_LOCAL;
+                      boost::adaptors::filtered([] (Face& face) {
+                        return face.getScope() == ndn::nfd::FACE_SCOPE_LOCAL;
+                      }) |
+                      boost::adaptors::transformed([] (Face& face) {
+                        return face.getId();
                       });
 
     std::vector<FaceId> results;
-    for (shared_ptr<Face> face : enumerable) {
-      results.push_back(face->getId());
-    }
+    boost::copy(enumerable, std::back_inserter(results));
     return results;
   }
 
diff --git a/tests/daemon/mgmt/face-manager-create-face.t.cpp b/tests/daemon/mgmt/face-manager-create-face.t.cpp
index fe6d147..8c67b3a 100644
--- a/tests/daemon/mgmt/face-manager-create-face.t.cpp
+++ b/tests/daemon/mgmt/face-manager-create-face.t.cpp
@@ -25,19 +25,18 @@
 
 #include "mgmt/face-manager.hpp"
 #include "fw/forwarder.hpp"
+#include <ndn-cxx/mgmt/dispatcher.hpp>
+#include <ndn-cxx/util/dummy-client-face.hpp>
+
+#include <thread>
+#include <boost/property_tree/info_parser.hpp>
 
 #include "tests/test-common.hpp"
 #include "tests/identity-management-fixture.hpp"
 
-#include <ndn-cxx/mgmt/dispatcher.hpp>
-#include <ndn-cxx/util/dummy-client-face.hpp>
-
-#include <boost/property_tree/info_parser.hpp>
-
 namespace nfd {
 namespace tests {
 
-
 BOOST_AUTO_TEST_SUITE(Mgmt)
 BOOST_AUTO_TEST_SUITE(TestFaceManager)
 
@@ -47,9 +46,10 @@
 {
 public:
   FaceManagerNode(ndn::KeyChain& keyChain, const std::string& port = "6363")
-    : face(getGlobalIoService(), keyChain, {true, true})
+    : faceTable(forwarder.getFaceTable())
+    , face(getGlobalIoService(), keyChain, {true, true})
     , dispatcher(face, keyChain, ndn::security::SigningInfo())
-    , manager(forwarder.getFaceTable(), dispatcher, validator)
+    , manager(faceTable, dispatcher, validator)
   {
     dispatcher.addTopPrefix("/localhost/nfd");
 
@@ -95,16 +95,17 @@
   void
   closeFaces()
   {
-    std::vector<shared_ptr<Face>> facesToClose;
+    std::vector<std::reference_wrapper<Face>> facesToClose;
     std::copy(forwarder.getFaceTable().begin(), forwarder.getFaceTable().end(),
               std::back_inserter(facesToClose));
-    for (auto face : facesToClose) {
-      face->close();
+    for (Face& face : facesToClose) {
+      face.close();
     }
   }
 
 public:
   Forwarder forwarder;
+  FaceTable& faceTable;
   ndn::util::DummyClientFace face;
   ndn::mgmt::Dispatcher dispatcher;
   CommandValidator validator;
@@ -119,14 +120,16 @@
     : node1(m_keyChain, "16363")
     , node2(m_keyChain, "26363")
   {
-    advanceClocks(time::milliseconds(1), 100);
+    advanceClocks(time::milliseconds(1), 5);
   }
 
   ~FaceManagerFixture()
   {
+    // Explicitly closing faces is necessary. Otherwise, in a subsequent test case,
+    // incoming packets may be delivered to an old socket from previous test cases.
     node1.closeFaces();
     node2.closeFaces();
-    advanceClocks(time::milliseconds(1), 100);
+    advanceClocks(time::milliseconds(1), 5);
   }
 
 public:
@@ -262,13 +265,11 @@
   Name commandName("/localhost/nfd/faces");
   commandName.append("create");
   commandName.append(FaceType().getParameters().wireEncode());
-
-  shared_ptr<Interest> command(make_shared<Interest>(commandName));
+  auto command = makeInterest(commandName);
   m_keyChain.sign(*command);
 
   bool hasCallbackFired = false;
   this->node1.face.onSendData.connect([this, command, &hasCallbackFired] (const Data& response) {
-      // std::cout << response << std::endl;
       if (!command->getName().isPrefixOf(response.getName())) {
         return;
       }
@@ -289,7 +290,7 @@
     });
 
   this->node1.face.receive(*command);
-  this->advanceClocks(time::milliseconds(1), 100);
+  this->advanceClocks(time::milliseconds(1), 5);
 
   BOOST_CHECK(hasCallbackFired);
 }
@@ -313,12 +314,11 @@
     Name commandName("/localhost/nfd/faces");
     commandName.append("create");
     commandName.append(FaceType1().getParameters().wireEncode());
-
-    shared_ptr<Interest> command(make_shared<Interest>(commandName));
+    auto command = makeInterest(commandName);
     m_keyChain.sign(*command);
 
     this->node1.face.receive(*command);
-    this->advanceClocks(time::milliseconds(1), 10);
+    this->advanceClocks(time::milliseconds(1), 5);
   }
 
   //
@@ -328,8 +328,7 @@
     Name commandName("/localhost/nfd/faces");
     commandName.append("create");
     commandName.append(FaceType2().getParameters().wireEncode());
-
-    shared_ptr<Interest> command(make_shared<Interest>(commandName));
+    auto command = makeInterest(commandName);
     m_keyChain.sign(*command);
 
     bool hasCallbackFired = false;
@@ -349,7 +348,7 @@
       });
 
     this->node1.face.receive(*command);
-    this->advanceClocks(time::milliseconds(1), 10);
+    this->advanceClocks(time::milliseconds(1), 5);
 
     BOOST_CHECK(hasCallbackFired);
   }
@@ -389,8 +388,7 @@
     Name commandName("/localhost/nfd/faces");
     commandName.append("create");
     commandName.append(OtherNodeFace().getParameters().wireEncode());
-
-    shared_ptr<Interest> command(make_shared<Interest>(commandName));
+    auto command = makeInterest(commandName);
     m_keyChain.sign(*command);
 
     ndn::util::signal::ScopedConnection connection =
@@ -411,19 +409,21 @@
         });
 
     this->node2.face.receive(*command);
-    this->advanceClocks(time::milliseconds(1), 10);
+    this->advanceClocks(time::milliseconds(1), 5); // let node2 process command and send Interest
+    std::this_thread::sleep_for(std::chrono::milliseconds(100)); // allow wallclock time for socket IO
+    this->advanceClocks(time::milliseconds(1), 5); // let node1 accept Interest and create on-demand face
   }
 
   // make sure there is on-demand face
-  bool onDemandFaceFound = false;
   FaceUri onDemandFaceUri(FaceType().getParameters().getUri());
-  for (auto face : this->node1.forwarder.getFaceTable()) {
-    if (face->getRemoteUri() == onDemandFaceUri) {
-      onDemandFaceFound = true;
+  const Face* foundFace = nullptr;
+  for (const Face& face : this->node1.faceTable) {
+    if (face.getRemoteUri() == onDemandFaceUri) {
+      foundFace = &face;
       break;
     }
   }
-  BOOST_REQUIRE(onDemandFaceFound);
+  BOOST_REQUIRE_MESSAGE(foundFace != nullptr, "on-demand face is not created");
 
   //
   {
@@ -432,12 +432,12 @@
     Name commandName("/localhost/nfd/faces");
     commandName.append("create");
     commandName.append(FaceType().getParameters().wireEncode());
-
-    shared_ptr<Interest> command(make_shared<Interest>(commandName));
+    auto command = makeInterest(commandName);
     m_keyChain.sign(*command);
 
     bool hasCallbackFired = false;
-    this->node1.face.onSendData.connect([this, command, &hasCallbackFired] (const Data& response) {
+    this->node1.face.onSendData.connect(
+      [this, command, &hasCallbackFired, foundFace] (const Data& response) {
         if (!command->getName().isPrefixOf(response.getName())) {
           return;
         }
@@ -447,13 +447,15 @@
 
         ControlParameters expectedParams(FaceType().getParameters());
         ControlParameters actualParams(actual.getBody());
-        BOOST_CHECK_EQUAL(expectedParams.getFacePersistency(), actualParams.getFacePersistency());
+        BOOST_CHECK_EQUAL(actualParams.getFacePersistency(), expectedParams.getFacePersistency());
+        BOOST_CHECK_EQUAL(actualParams.getFaceId(), foundFace->getId());
+        BOOST_CHECK_EQUAL(foundFace->getPersistency(), expectedParams.getFacePersistency());
 
         hasCallbackFired = true;
       });
 
     this->node1.face.receive(*command);
-    this->advanceClocks(time::milliseconds(1), 10);
+    this->advanceClocks(time::milliseconds(1), 5);
 
     BOOST_CHECK(hasCallbackFired);
   }