Eliminate openscreen_unittests log spam, fix some gmock warnings.

Use ::testing::NiceMock and/or correct unit tests that were missing
EXPECT_CALL()'s that were actually relevant to the test. Also, raise the
default log level to kWarning (was kInfo); but all command-line tools
and demo apps explicitly set this back down to kInfo.

Background: Many of the tests were spamming the logs with "uninteresting
mock" warnings from GMock. Also, there was a ton of spam due to INFO
logging meant for the command-line tools. This made it difficult for
humans to find the logging output associated with test failures.
Change-Id: I2560a7324130727c25e0593b86d08b4753e0570e
Reviewed-on: https://chromium-review.googlesource.com/c/openscreen/+/1674509
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Reviewed-by: Max Yakimakha <yakimakha@chromium.org>
Reviewed-by: Ryan Keane <rwkeane@google.com>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Commit-Queue: Yuri Wiitala <miu@chromium.org>

6 files changed, 42 insertions, 18 deletions

diff --git a/osp/impl/presentation/presentation_connection_unittest.cc b/osp/impl/presentation/presentation_connection_unittest.cc
index e52fed48..0c5096cf 100644
--- a/osp/impl/presentation/presentation_connection_unittest.cc
+++ b/osp/impl/presentation/presentation_connection_unittest.cc
@@ -22,8 +22,11 @@ namespace presentation {
 
 using ::testing::_;
 using ::testing::Invoke;
+using ::testing::NiceMock;
 
-class MockParentDelegate final : public Connection::ParentDelegate {
+namespace {
+
+class MockParentDelegate : public Connection::ParentDelegate {
  public:
   MockParentDelegate() = default;
   ~MockParentDelegate() override = default;
@@ -49,6 +52,8 @@ class MockConnectRequest final
   MOCK_METHOD1(OnConnectionFailed, void(uint64_t request_id));
 };
 
+}  // namespace
+
 class ConnectionTest : public ::testing::Test {
  protected:
   void SetUp() override {
@@ -76,8 +81,8 @@ class ConnectionTest : public ::testing::Test {
       quic_bridge_.controller_demuxer.get()};
   ConnectionManager receiver_connection_manager_{
       quic_bridge_.receiver_demuxer.get()};
-  MockParentDelegate mock_controller_;
-  MockParentDelegate mock_receiver_;
+  NiceMock<MockParentDelegate> mock_controller_;
+  NiceMock<MockParentDelegate> mock_receiver_;
 };
 
 TEST_F(ConnectionTest, ConnectAndSend) {
diff --git a/osp/impl/presentation/presentation_controller.cc b/osp/impl/presentation/presentation_controller.cc
index de10290b..fa62a3df 100644
--- a/osp/impl/presentation/presentation_controller.cc
+++ b/osp/impl/presentation/presentation_controller.cc
@@ -679,8 +679,8 @@ ErrorOr<size_t> Controller::TerminationListener::OnStreamMessage(
 // static
 std::string Controller::MakePresentationId(const std::string& url,
                                            const std::string& service_id) {
-  OSP_UNIMPLEMENTED();
-  // TODO(btolsch): This is just a placeholder for the demo.
+  // TODO(btolsch): This is just a placeholder for the demo. It should
+  // eventually become a GUID/unguessable token routine.
   std::string safe_id = service_id;
   for (auto& c : safe_id)
     if (c < ' ' || c > '~')
diff --git a/osp/impl/presentation/presentation_controller_unittest.cc b/osp/impl/presentation/presentation_controller_unittest.cc
index e1758c85..5b09d10c 100644
--- a/osp/impl/presentation/presentation_controller_unittest.cc
+++ b/osp/impl/presentation/presentation_controller_unittest.cc
@@ -23,6 +23,9 @@ namespace presentation {
 using std::chrono::seconds;
 using ::testing::_;
 using ::testing::Invoke;
+using ::testing::NiceMock;
+
+namespace {
 
 const char kTestUrl[] = "https://example.foo";
 
@@ -68,6 +71,8 @@ class MockRequestDelegate final : public RequestDelegate {
   MOCK_METHOD1(OnError, void(const Error& error));
 };
 
+}  // namespace
+
 class ControllerTest : public ::testing::Test {
  protected:
   void SetUp() override {
@@ -381,7 +386,7 @@ TEST_F(ControllerTest, ReceiverWatchCancel) {
 
 TEST_F(ControllerTest, StartPresentation) {
   MockMessageCallback mock_callback;
-  MockConnectionDelegate mock_connection_delegate;
+  NiceMock<MockConnectionDelegate> mock_connection_delegate;
   std::unique_ptr<Connection> connection;
   StartPresentation(&mock_callback, &mock_connection_delegate, &connection);
 }
@@ -461,7 +466,7 @@ TEST_F(ControllerTest, CloseConnection) {
 
 TEST_F(ControllerTest, Reconnect) {
   MockMessageCallback mock_callback;
-  MockConnectionDelegate mock_connection_delegate;
+  NiceMock<MockConnectionDelegate> mock_connection_delegate;
   std::unique_ptr<Connection> connection;
   StartPresentation(&mock_callback, &mock_connection_delegate, &connection);
 
diff --git a/osp/impl/presentation/presentation_receiver_unittest.cc b/osp/impl/presentation/presentation_receiver_unittest.cc
index 9a6e0dbd..51e01aa9 100644
--- a/osp/impl/presentation/presentation_receiver_unittest.cc
+++ b/osp/impl/presentation/presentation_receiver_unittest.cc
@@ -20,10 +20,12 @@
 
 namespace openscreen {
 namespace presentation {
+
 namespace {
 
 using ::testing::_;
 using ::testing::Invoke;
+using ::testing::NiceMock;
 
 class MockConnectRequest final
     : public ProtocolConnectionClient::ConnectionRequestCallback {
@@ -170,7 +172,7 @@ TEST_F(PresentationReceiverTest, StartPresentation) {
   EXPECT_EQ(presentation_id, info.id);
   EXPECT_EQ(url1_, info.url);
 
-  MockConnectionDelegate null_connection_delegate;
+  NiceMock<MockConnectionDelegate> null_connection_delegate;
   Connection connection(Connection::PresentationInfo{presentation_id, url1_},
                         &null_connection_delegate, Receiver::Get());
   Receiver::Get()->OnPresentationStarted(presentation_id, &connection,
diff --git a/osp/impl/presentation/testing/mock_connection_delegate.h b/osp/impl/presentation/testing/mock_connection_delegate.h
index 85c3c113..08960686 100644
--- a/osp/impl/presentation/testing/mock_connection_delegate.h
+++ b/osp/impl/presentation/testing/mock_connection_delegate.h
@@ -11,7 +11,7 @@
 namespace openscreen {
 namespace presentation {
 
-class MockConnectionDelegate final : public Connection::Delegate {
+class MockConnectionDelegate : public Connection::Delegate {
  public:
   ~MockConnectionDelegate() override = default;
 
diff --git a/osp/impl/presentation/url_availability_requester_unittest.cc b/osp/impl/presentation/url_availability_requester_unittest.cc
index 5b8c2f8f..9732fc38 100644
--- a/osp/impl/presentation/url_availability_requester_unittest.cc
+++ b/osp/impl/presentation/url_availability_requester_unittest.cc
@@ -24,9 +24,12 @@ namespace presentation {
 
 using ::testing::_;
 using ::testing::Invoke;
+using ::testing::Mock;
 using ::testing::Test;
 
-class MockReceiverObserver final : public ReceiverObserver {
+namespace {
+
+class MockReceiverObserver : public ReceiverObserver {
  public:
   ~MockReceiverObserver() override = default;
 
@@ -37,14 +40,7 @@ class MockReceiverObserver final : public ReceiverObserver {
                void(const std::string&, const std::string&));
 };
 
-class NullObserver final : public ProtocolConnectionServiceObserver {
- public:
-  ~NullObserver() override = default;
-  void OnRunning() override {}
-  void OnStopped() override {}
-  void OnMetrics(const NetworkMetrics& metrics) override {}
-  void OnError(const Error& error) override {}
-};
+}  // namespace
 
 class UrlAvailabilityRequesterTest : public Test {
  public:
@@ -367,12 +363,15 @@ TEST_F(UrlAvailabilityRequesterTest, RemoveObserverUrls) {
   EXPECT_CALL(mock_observer1, OnReceiverUnavailable(url1_, service_id_))
       .Times(0);
   quic_bridge_.RunTasksUntilIdle();
+  Mock::VerifyAndClearExpectations(&mock_observer1);
 
   MockReceiverObserver mock_observer2;
+  EXPECT_CALL(mock_observer2, OnReceiverAvailable(url1_, service_id_));
   listener_.AddObserver({url1_, url2_}, &mock_observer2);
 
   ExpectStreamMessage(&mock_callback_, &request);
   quic_bridge_.RunTasksUntilIdle();
+  Mock::VerifyAndClearExpectations(&mock_observer2);
 
   EXPECT_EQ(std::vector<std::string>{url2_}, request.urls);
 
@@ -385,6 +384,8 @@ TEST_F(UrlAvailabilityRequesterTest, RemoveObserverUrls) {
   EXPECT_CALL(mock_observer2, OnReceiverAvailable(_, service_id_)).Times(0);
   EXPECT_CALL(mock_observer2, OnReceiverUnavailable(url2_, service_id_));
   quic_bridge_.RunTasksUntilIdle();
+  Mock::VerifyAndClearExpectations(&mock_observer1);
+  Mock::VerifyAndClearExpectations(&mock_observer2);
 
   SendAvailabilityEvent(
       url1_watch_id,
@@ -395,6 +396,8 @@ TEST_F(UrlAvailabilityRequesterTest, RemoveObserverUrls) {
       .Times(0);
   EXPECT_CALL(mock_observer2, OnReceiverUnavailable(url1_, service_id_));
   quic_bridge_.RunTasksUntilIdle();
+  Mock::VerifyAndClearExpectations(&mock_observer1);
+  Mock::VerifyAndClearExpectations(&mock_observer2);
 }
 
 TEST_F(UrlAvailabilityRequesterTest, RemoveObserver) {
@@ -421,12 +424,15 @@ TEST_F(UrlAvailabilityRequesterTest, RemoveObserver) {
   EXPECT_CALL(mock_observer1, OnReceiverUnavailable(url1_, service_id_))
       .Times(0);
   quic_bridge_.RunTasksUntilIdle();
+  Mock::VerifyAndClearExpectations(&mock_observer1);
 
   MockReceiverObserver mock_observer2;
+  EXPECT_CALL(mock_observer2, OnReceiverAvailable(url1_, service_id_));
   listener_.AddObserver({url1_, url2_}, &mock_observer2);
 
   ExpectStreamMessage(&mock_callback_, &request);
   quic_bridge_.RunTasksUntilIdle();
+  Mock::VerifyAndClearExpectations(&mock_observer2);
 
   uint64_t url2_watch_id = request.watch_id;
   EXPECT_EQ(std::vector<std::string>{url2_}, request.urls);
@@ -440,6 +446,8 @@ TEST_F(UrlAvailabilityRequesterTest, RemoveObserver) {
   EXPECT_CALL(mock_observer2, OnReceiverAvailable(_, service_id_)).Times(0);
   EXPECT_CALL(mock_observer2, OnReceiverUnavailable(url2_, service_id_));
   quic_bridge_.RunTasksUntilIdle();
+  Mock::VerifyAndClearExpectations(&mock_observer1);
+  Mock::VerifyAndClearExpectations(&mock_observer2);
 
   SendAvailabilityEvent(
       url1_watch_id,
@@ -450,6 +458,8 @@ TEST_F(UrlAvailabilityRequesterTest, RemoveObserver) {
       .Times(0);
   EXPECT_CALL(mock_observer2, OnReceiverUnavailable(url1_, service_id_));
   quic_bridge_.RunTasksUntilIdle();
+  Mock::VerifyAndClearExpectations(&mock_observer1);
+  Mock::VerifyAndClearExpectations(&mock_observer2);
 
   listener_.RemoveObserver(&mock_observer2);
 
@@ -466,6 +476,8 @@ TEST_F(UrlAvailabilityRequesterTest, RemoveObserver) {
   EXPECT_CALL(mock_observer1, OnReceiverUnavailable(_, service_id_)).Times(0);
   EXPECT_CALL(mock_observer2, OnReceiverUnavailable(_, service_id_)).Times(0);
   quic_bridge_.RunTasksUntilIdle();
+  Mock::VerifyAndClearExpectations(&mock_observer1);
+  Mock::VerifyAndClearExpectations(&mock_observer2);
 }
 
 TEST_F(UrlAvailabilityRequesterTest, EventUpdate) {