coverage

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
alyssawilk · Aug 22, 2023 · e294dcc · e294dcc
1 parent 90ad5f2
commit e294dcc
Show file tree

Hide file tree

Showing 7 changed files with 24 additions and 24 deletions.
diff --git a/envoy/router/router.h b/envoy/router/router.h
@@ -1395,6 +1395,11 @@ class GenericConnPool {
    * @return optionally returns the host for the connection pool.
    */
   virtual Upstream::HostDescriptionConstSharedPtr host() const PURE;
+
+  /**
+   * @return returns if the connection pool was iniitalized successfully.
+   */
+  virtual bool valid() const PURE;
 };
 
 /**

diff --git a/source/extensions/upstreams/http/generic/config.cc b/source/extensions/upstreams/http/generic/config.cc
@@ -17,22 +17,19 @@ Router::GenericConnPoolPtr GenericGenericConnPoolFactory::createGenericConnPool(
     const Router::RouteEntry& route_entry,
     absl::optional<Envoy::Http::Protocol> downstream_protocol,
     Upstream::LoadBalancerContext* ctx) const {
+  Router::GenericConnPoolPtr conn_pool;
   switch (upstream_protocol) {
-  case UpstreamProtocol::HTTP: {
-    auto http_conn_pool = std::make_unique<Upstreams::Http::Http::HttpConnPool>(
+  case UpstreamProtocol::HTTP:
+    conn_pool = std::make_unique<Upstreams::Http::Http::HttpConnPool>(
         thread_local_cluster, route_entry, downstream_protocol, ctx);
-    return (http_conn_pool->valid() ? std::move(http_conn_pool) : nullptr);
-  }
-  case UpstreamProtocol::TCP: {
-    auto tcp_conn_pool =
+    return (conn_pool->valid() ? std::move(conn_pool) : nullptr);
+  case UpstreamProtocol::TCP:
+    conn_pool =
         std::make_unique<Upstreams::Http::Tcp::TcpConnPool>(thread_local_cluster, route_entry, ctx);
-    return (tcp_conn_pool->valid() ? std::move(tcp_conn_pool) : nullptr);
-  }
-  case UpstreamProtocol::UDP: {
-    auto udp_conn_pool =
-        std::make_unique<Upstreams::Http::Udp::UdpConnPool>(thread_local_cluster, ctx);
-    return (udp_conn_pool->valid() ? std::move(udp_conn_pool) : nullptr);
-  }
+    return (conn_pool->valid() ? std::move(conn_pool) : nullptr);
+  case UpstreamProtocol::UDP:
+    conn_pool = std::make_unique<Upstreams::Http::Udp::UdpConnPool>(thread_local_cluster, ctx);
+    return (conn_pool->valid() ? std::move(conn_pool) : nullptr);
   }
 
   return nullptr;

diff --git a/source/extensions/upstreams/http/http/upstream_request.h b/source/extensions/upstreams/http/http/upstream_request.h
@@ -20,7 +20,6 @@ namespace Http {
 
 class HttpConnPool : public Router::GenericConnPool, public Envoy::Http::ConnectionPool::Callbacks {
 public:
-  // GenericConnPool
   HttpConnPool(Upstream::ThreadLocalCluster& thread_local_cluster,
                const Router::RouteEntry& route_entry,
                absl::optional<Envoy::Http::Protocol> downstream_protocol,
@@ -31,8 +30,13 @@ class HttpConnPool : public Router::GenericConnPool, public Envoy::Http::Connect
   ~HttpConnPool() override {
     ASSERT(conn_pool_stream_handle_ == nullptr, "conn_pool_stream_handle not null");
   }
+  // GenericConnPool
   void newStream(Router::GenericConnectionPoolCallbacks* callbacks) override;
   bool cancelAnyPendingStream() override;
+  bool valid() const override { return pool_data_.has_value(); }
+  Upstream::HostDescriptionConstSharedPtr host() const override {
+    return pool_data_.value().host();
+  }
 
   // Http::ConnectionPool::Callbacks
   void onPoolFailure(ConnectionPool::PoolFailureReason reason,
@@ -41,11 +45,6 @@ class HttpConnPool : public Router::GenericConnPool, public Envoy::Http::Connect
   void onPoolReady(Envoy::Http::RequestEncoder& callbacks_encoder,
                    Upstream::HostDescriptionConstSharedPtr host, StreamInfo::StreamInfo& info,
                    absl::optional<Envoy::Http::Protocol> protocol) override;
-  Upstream::HostDescriptionConstSharedPtr host() const override {
-    return pool_data_.value().host();
-  }
-
-  bool valid() { return pool_data_.has_value(); }
 
 protected:
   // Points to the actual connection pool to create streams from.

diff --git a/source/extensions/upstreams/http/tcp/upstream_request.h b/source/extensions/upstreams/http/tcp/upstream_request.h
@@ -26,11 +26,11 @@ class TcpConnPool : public Router::GenericConnPool, public Envoy::Tcp::Connectio
               const Router::RouteEntry& route_entry, Upstream::LoadBalancerContext* ctx) {
     conn_pool_data_ = thread_local_cluster.tcpConnPool(route_entry.priority(), ctx);
   }
+  // Router::GenericConnPool
   void newStream(Router::GenericConnectionPoolCallbacks* callbacks) override {
     callbacks_ = callbacks;
     upstream_handle_ = conn_pool_data_.value().newConnection(*this);
   }
-
   bool cancelAnyPendingStream() override {
     if (upstream_handle_) {
       upstream_handle_->cancel(Envoy::Tcp::ConnectionPool::CancelPolicy::Default);
@@ -42,8 +42,7 @@ class TcpConnPool : public Router::GenericConnPool, public Envoy::Tcp::Connectio
   Upstream::HostDescriptionConstSharedPtr host() const override {
     return conn_pool_data_.value().host();
   }
-
-  bool valid() { return conn_pool_data_.has_value(); }
+  bool valid() const override { return conn_pool_data_.has_value(); }
 
   // Tcp::ConnectionPool::Callbacks
   void onPoolFailure(ConnectionPool::PoolFailureReason reason,

diff --git a/source/extensions/upstreams/http/udp/upstream_request.h b/source/extensions/upstreams/http/udp/upstream_request.h
@@ -49,7 +49,7 @@ class UdpConnPool : public Router::GenericConnPool {
                                                  Network::SocketCreationOptions{});
   }
 
-  bool valid() { return host_ != nullptr; }
+  bool valid() const override { return host_ != nullptr; }
 
 private:
   Upstream::HostConstSharedPtr host_;

diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h
@@ -626,6 +626,7 @@ class MockGenericConnPool : public GenericConnPool {
               (Upstream::ClusterManager&, const RouteEntry&, Http::Protocol,
                Upstream::LoadBalancerContext*));
   MOCK_METHOD(Upstream::HostDescriptionConstSharedPtr, host, (), (const));
+  MOCK_METHOD(bool, valid, (), (const));
 };
 
 class MockUpstreamToDownstream : public UpstreamToDownstream {

diff --git a/test/per_file_coverage.sh b/test/per_file_coverage.sh
@@ -58,7 +58,6 @@ declare -a KNOWN_LOW_COVERAGE=(
 "source/extensions/transport_sockets/tls:95.0"
 "source/extensions/transport_sockets/tls/cert_validator:95.2"
 "source/extensions/transport_sockets/tls/private_key:88.9"
-"source/extensions/upstreams/http/generic:85.0" # Braces in switch statements are considered uncovered
 "source/extensions/wasm_runtime/wamr:0.0" # Not enabled in coverage build
 "source/extensions/wasm_runtime/wasmtime:0.0" # Not enabled in coverage build
 "source/extensions/wasm_runtime/wavm:0.0" # Not enabled in coverage build