Logging cleanups

This patch removes some test-specific logging code in favor of death tests, and
makes the OSP_NOTREACHED macro actually be annotated as [[noreturn]], so you
don't have to put a bogus return statement after it.

Change-Id: I6a6a271182061cbd98593ac0ae79347e48da5bc7
Reviewed-on: https://chromium-review.googlesource.com/c/openscreen/+/2555597
Commit-Queue: Jordan Bayles <jophba@chromium.org>
Reviewed-by: Brandon Tolsch <btolsch@chromium.org>

8 files changed, 10 insertions, 16 deletions

diff --git a/cast/common/channel/message_util.cc b/cast/common/channel/message_util.cc
index 79b942bd..44505501 100644
--- a/cast/common/channel/message_util.cc
+++ b/cast/common/channel/message_util.cc
@@ -37,7 +37,6 @@ std::string ToString(AppAvailabilityResult availability) {
       return "Unknown";
     default:
       OSP_NOTREACHED();
-      return "bad value";
   }
 }
 
diff --git a/cast/common/discovery/e2e_test/tests.cc b/cast/common/discovery/e2e_test/tests.cc
index 373d1133..f39c39d5 100644
--- a/cast/common/discovery/e2e_test/tests.cc
+++ b/cast/common/discovery/e2e_test/tests.cc
@@ -105,7 +105,8 @@ class ServiceReceiver : public discovery::DnsSdServiceWatcher<ServiceInfo> {
 
 class FailOnErrorReporting : public discovery::ReportingClient {
   void OnFatalError(Error error) override {
-    OSP_NOTREACHED() << "Fatal error received: '" << error << "'";
+    OSP_LOG_FATAL << "Fatal error received: '" << error << "'";
+    OSP_NOTREACHED();
   }
 
   void OnRecoverableError(Error error) override {
@@ -282,10 +283,8 @@ class DiscoveryE2ETest : public testing::Test {
       return;
     }
 
-    if (attempts++ > kMaxCheckLoopIterations) {
-      OSP_NOTREACHED() << "Service " << service_info.friendly_name
-                       << " publication failed.";
-    }
+    OSP_CHECK_LE(attempts++, kMaxCheckLoopIterations)
+        << "Service " << service_info.friendly_name << " publication failed.";
     task_runner_->PostTaskWithDelay(
         [this, info = std::move(service_info), has_been_seen,
          attempts]() mutable {
@@ -315,8 +314,7 @@ class DiscoveryE2ETest : public testing::Test {
       // TODO(crbug.com/openscreen/110): Log the discovered service instance.
       *has_been_seen = true;
     } else {
-      OSP_NOTREACHED() << "Found instance '" << service_info.friendly_name
-                       << "'!";
+      OSP_LOG_FATAL << "Found instance '" << service_info.friendly_name << "'!";
     }
   }
 };
diff --git a/cast/receiver/channel/receiver_socket_factory.cc b/cast/receiver/channel/receiver_socket_factory.cc
index 5ad7246a..c8ddd691 100644
--- a/cast/receiver/channel/receiver_socket_factory.cc
+++ b/cast/receiver/channel/receiver_socket_factory.cc
@@ -32,7 +32,7 @@ void ReceiverSocketFactory::OnConnected(
     TlsConnectionFactory* factory,
     std::vector<uint8_t> der_x509_peer_cert,
     std::unique_ptr<TlsConnection> connection) {
-  OSP_NOTREACHED() << "This factory is accept-only.";
+  OSP_LOG_FATAL << "This factory is accept-only";
 }
 
 void ReceiverSocketFactory::OnConnectionFailed(
diff --git a/cast/sender/cast_app_discovery_service_impl.cc b/cast/sender/cast_app_discovery_service_impl.cc
index fd071ca2..4ca9a016 100644
--- a/cast/sender/cast_app_discovery_service_impl.cc
+++ b/cast/sender/cast_app_discovery_service_impl.cc
@@ -174,7 +174,6 @@ bool CastAppDiscoveryServiceImpl::ShouldRefreshAppAvailability(
   }
 
   OSP_NOTREACHED();
-  return false;
 }
 
 void CastAppDiscoveryServiceImpl::RemoveAvailabilityCallback(uint32_t id) {
diff --git a/cast/sender/channel/sender_socket_factory.cc b/cast/sender/channel/sender_socket_factory.cc
index a1f24ef0..e971976b 100644
--- a/cast/sender/channel/sender_socket_factory.cc
+++ b/cast/sender/channel/sender_socket_factory.cc
@@ -59,7 +59,8 @@ void SenderSocketFactory::OnAccepted(
     TlsConnectionFactory* factory,
     std::vector<uint8_t> der_x509_peer_cert,
     std::unique_ptr<TlsConnection> connection) {
-  OSP_NOTREACHED() << "This factory is connect-only.";
+  OSP_NOTREACHED();
+  OSP_LOG_FATAL << "This factory is connect-only";
 }
 
 void SenderSocketFactory::OnConnected(
diff --git a/cast/streaming/environment.cc b/cast/streaming/environment.cc
index 16cbb64f..c3e7bea0 100644
--- a/cast/streaming/environment.cc
+++ b/cast/streaming/environment.cc
@@ -61,7 +61,6 @@ int Environment::GetMaxPacketSize() const {
       return kMaxRtpPacketSizeForIpv6UdpOnEthernet;
     default:
       OSP_NOTREACHED();
-      return 0;
   }
 }
 
diff --git a/cast/streaming/offer_messages.cc b/cast/streaming/offer_messages.cc
index 163c0b0c..9ed6e1d9 100644
--- a/cast/streaming/offer_messages.cc
+++ b/cast/streaming/offer_messages.cc
@@ -264,7 +264,6 @@ absl::string_view ToString(Stream::Type type) {
       return kVideoSourceType;
     default: {
       OSP_NOTREACHED();
-      return "";
     }
   }
 }
@@ -317,7 +316,6 @@ std::string CastMode::ToString() const {
       return kCastRemoting;
     default:
       OSP_NOTREACHED();
-      return "";
   }
 }
 
diff --git a/cast/test/cast_socket_e2e_test.cc b/cast/test/cast_socket_e2e_test.cc
index becfdb22..bfd1bbf9 100644
--- a/cast/test/cast_socket_e2e_test.cc
+++ b/cast/test/cast_socket_e2e_test.cc
@@ -67,7 +67,7 @@ class SenderSocketsClient : public SenderSocketFactory::Client,
   void OnError(SenderSocketFactory* factory,
                const IPEndpoint& endpoint,
                Error error) override {
-    OSP_NOTREACHED() << error;
+    OSP_LOG_FATAL << error;
   }
 
   // VirtualConnectionRouter::SocketErrorHandler overrides.
@@ -112,7 +112,7 @@ class ReceiverSocketsClient
   }
 
   void OnError(ReceiverSocketFactory* factory, Error error) override {
-    OSP_NOTREACHED() << error;
+    OSP_LOG_FATAL << error;
   }
 
   // VirtualConnectionRouter::SocketErrorHandler overrides.