Removing GCOVR macro (envoyproxy#19959)

Signed-off-by: Alyssa Wilk <[email protected]>
AppMonet · Feb 16, 2022 · 5187729 · 5187729
1 parent 488f7d4
commit 5187729
Show file tree

Hide file tree

Showing 21 changed files with 54 additions and 60 deletions.
diff --git a/contrib/sip_proxy/filters/network/source/conn_manager.cc b/contrib/sip_proxy/filters/network/source/conn_manager.cc
@@ -207,7 +207,7 @@ void ConnectionManager::sendLocalReply(MessageMetadata& metadata, const DirectRe
     stats_.response_exception_.inc();
     break;
   default:
-    NOT_REACHED_GCOVR_EXCL_LINE;
+    PANIC("not reached");
   }
 }
 

diff --git a/contrib/sip_proxy/filters/network/source/decoder.cc b/contrib/sip_proxy/filters/network/source/decoder.cc
@@ -46,7 +46,7 @@ DecoderStateMachine::DecoderStatus DecoderStateMachine::handleState() {
     return transportEnd();
   default:
     /* test failed report "panic:     not reached" if reach here */
-    NOT_REACHED_GCOVR_EXCL_LINE;
+    PANIC("not reached");
   }
 }
 
@@ -659,7 +659,7 @@ absl::string_view Decoder::domain(absl::string_view sip_header, HeaderType heade
     pattern = ".*sip.*[:@](.*?)[:;].*";
     break;
   default:
-    NOT_REACHED_GCOVR_EXCL_LINE;
+    PANIC("not reached");
   }
 
   re2::RE2::FullMatch(static_cast<std::string>(sip_header), pattern, &domain);

diff --git a/contrib/sip_proxy/filters/network/source/router/router_impl.cc b/contrib/sip_proxy/filters/network/source/router/router_impl.cc
@@ -59,7 +59,7 @@ RouteMatcher::RouteMatcher(
       routes_.emplace_back(new GeneralRouteEntryImpl(route));
       break;
     default:
-      NOT_REACHED_GCOVR_EXCL_LINE;
+      PANIC("not reached");
     }
   }
 }
@@ -487,7 +487,7 @@ void UpstreamRequest::onResetStream(ConnectionPool::PoolFailureReason reason) {
     callbacks_->resetDownstreamConnection();
     break;
   default:
-    NOT_REACHED_GCOVR_EXCL_LINE;
+    PANIC("not reached");
   }
 }
 

diff --git a/source/common/common/assert.h b/source/common/common/assert.h
@@ -250,12 +250,6 @@ void resetEnvoyBugCountersForTest();
 // Always triggers ENVOY_BUG. This is intended for paths that are not expected to be reached.
 #define IS_ENVOY_BUG(...) ENVOY_BUG(false, __VA_ARGS__);
 
-// NOT_REACHED_GCOVR_EXCL_LINE is for spots the compiler insists on having a return, but where we
-// know that it shouldn't be possible to arrive there, assuming no horrendous bugs. For example,
-// after a switch (some_enum) with all enum values included in the cases. The macro name includes
-// "GCOVR_EXCL_LINE" to exclude the macro's usage from code coverage reports.
-#define NOT_REACHED_GCOVR_EXCL_LINE PANIC("not reached")
-
 // It is safer to avoid defaults in switch statements, so that as new enums are added, the compiler
 // checks that new code is added as well. Google's proto library adds 2 sentinel values which should
 // not be used, and this macro allows avoiding using "default:" to handle them.

diff --git a/source/extensions/common/matcher/matcher.cc b/source/extensions/common/matcher/matcher.cc
@@ -56,8 +56,8 @@ void buildMatcher(const envoy::config::common::matcher::v3::MatchPredicate& matc
     new_matcher = std::make_unique<HttpResponseGenericBodyMatcher>(
         match_config.http_response_generic_body_match(), matchers);
     break;
-  default:
-    NOT_REACHED_GCOVR_EXCL_LINE;
+  case envoy::config::common::matcher::v3::MatchPredicate::RuleCase::RULE_NOT_SET:
+    PANIC_DUE_TO_CORRUPT_ENUM;
   }
 
   // Per above, move the matcher into its position.
@@ -148,8 +148,8 @@ HttpGenericBodyMatcher::HttpGenericBodyMatcher(
     case envoy::config::common::matcher::v3::HttpGenericBodyMatch::GenericTextMatch::kStringMatch:
       patterns_->push_back(i.string_match());
       break;
-    default:
-      NOT_REACHED_GCOVR_EXCL_LINE;
+    case envoy::config::common::matcher::v3::HttpGenericBodyMatch::GenericTextMatch::RULE_NOT_SET:
+      PANIC_DUE_TO_CORRUPT_ENUM;
     }
     // overlap_size_ indicates how many bytes from previous data chunk(s) are buffered.
     overlap_size_ = std::max(overlap_size_, patterns_->back().length() - 1);

diff --git a/source/extensions/common/tap/admin.cc b/source/extensions/common/tap/admin.cc
@@ -124,12 +124,17 @@ void AdminHandler::AdminPerTapSinkHandle::submitTrace(
 
     std::string output_string;
     switch (format) {
+      PANIC_ON_PROTO_ENUM_SENTINEL_VALUES;
     case envoy::config::tap::v3::OutputSink::JSON_BODY_AS_STRING:
     case envoy::config::tap::v3::OutputSink::JSON_BODY_AS_BYTES:
       output_string = MessageUtil::getJsonStringFromMessageOrError(*trace, true, true);
       break;
-    default:
-      NOT_REACHED_GCOVR_EXCL_LINE;
+    case envoy::config::tap::v3::OutputSink::PROTO_BINARY:
+      PANIC("not implemented");
+    case envoy::config::tap::v3::OutputSink::PROTO_BINARY_LENGTH_DELIMITED:
+      PANIC("not implemented");
+    case envoy::config::tap::v3::OutputSink::PROTO_TEXT:
+      PANIC("not implemented");
     }
 
     ENVOY_LOG(debug, "admin writing buffered trace to response");

diff --git a/source/extensions/common/tap/extension_config_base.cc b/source/extensions/common/tap/extension_config_base.cc
@@ -37,9 +37,9 @@ ExtensionConfigBase::ExtensionConfigBase(
     ENVOY_LOG(debug, "initializing tap extension with static config");
     break;
   }
-  default: {
-    NOT_REACHED_GCOVR_EXCL_LINE;
-  }
+  case envoy::extensions::common::tap::v3::CommonExtensionConfig::ConfigTypeCase::
+      CONFIG_TYPE_NOT_SET:
+    PANIC_DUE_TO_CORRUPT_ENUM;
   }
 }
 

diff --git a/source/extensions/common/tap/tap_config_base.cc b/source/extensions/common/tap/tap_config_base.cc
@@ -70,8 +70,10 @@ TapConfigBaseImpl::TapConfigBaseImpl(const envoy::config::tap::v3::TapConfig& pr
         std::make_unique<FilePerTapSink>(proto_config.output_config().sinks()[0].file_per_tap());
     sink_to_use_ = sink_.get();
     break;
-  default:
-    NOT_REACHED_GCOVR_EXCL_LINE;
+  case envoy::config::tap::v3::OutputSink::OutputSinkTypeCase::kStreamingGrpc:
+    PANIC("not implemented");
+  case envoy::config::tap::v3::OutputSink::OutputSinkTypeCase::OUTPUT_SINK_TYPE_NOT_SET:
+    PANIC_DUE_TO_CORRUPT_ENUM;
   }
 
   envoy::config::common::matcher::v3::MatchPredicate match;
@@ -152,7 +154,7 @@ void Utility::bodyBytesToString(envoy::data::tap::v3::TraceWrapper& trace,
     break;
   }
   case envoy::data::tap::v3::TraceWrapper::TraceCase::TRACE_NOT_SET:
-    NOT_REACHED_GCOVR_EXCL_LINE;
+    PANIC_DUE_TO_CORRUPT_ENUM;
   }
 }
 
@@ -166,6 +168,7 @@ void FilePerTapSink::FilePerTapSinkHandle::submitTrace(
   if (!output_file_.is_open()) {
     std::string path = fmt::format("{}_{}", parent_.config_.path_prefix(), trace_id_);
     switch (format) {
+      PANIC_ON_PROTO_ENUM_SENTINEL_VALUES;
     case envoy::config::tap::v3::OutputSink::PROTO_BINARY:
       path += MessageUtil::FileExtensions::get().ProtoBinary;
       break;
@@ -179,8 +182,6 @@ void FilePerTapSink::FilePerTapSinkHandle::submitTrace(
     case envoy::config::tap::v3::OutputSink::JSON_BODY_AS_STRING:
       path += MessageUtil::FileExtensions::get().Json;
       break;
-    default:
-      NOT_REACHED_GCOVR_EXCL_LINE;
     }
 
     ENVOY_LOG_MISC(debug, "Opening tap file for [id={}] to {}", trace_id_, path);
@@ -192,6 +193,7 @@ void FilePerTapSink::FilePerTapSinkHandle::submitTrace(
   ENVOY_LOG_MISC(trace, "Tap for [id={}]: {}", trace_id_, trace->DebugString());
 
   switch (format) {
+    PANIC_ON_PROTO_ENUM_SENTINEL_VALUES;
   case envoy::config::tap::v3::OutputSink::PROTO_BINARY:
     trace->SerializeToOstream(&output_file_);
     break;
@@ -209,8 +211,6 @@ void FilePerTapSink::FilePerTapSinkHandle::submitTrace(
   case envoy::config::tap::v3::OutputSink::JSON_BODY_AS_STRING:
     output_file_ << MessageUtil::getJsonStringFromMessageOrError(*trace, true, true);
     break;
-  default:
-    NOT_REACHED_GCOVR_EXCL_LINE;
   }
 }
 

diff --git a/source/extensions/common/wasm/context.cc b/source/extensions/common/wasm/context.cc
@@ -727,7 +727,8 @@ const Http::HeaderMap* Context::getConstMap(WasmHeaderMapType type) {
     return nullptr;
   }
   }
-  NOT_REACHED_GCOVR_EXCL_LINE;
+  IS_ENVOY_BUG("unexpected");
+  return nullptr;
 }
 
 WasmResult Context::addHeaderMapValue(WasmHeaderMapType type, std::string_view key,
@@ -1177,9 +1178,12 @@ WasmResult Context::log(uint32_t level, std::string_view message) {
   case spdlog::level::critical:
     ENVOY_LOG(critical, "wasm log{}: {}", log_prefix(), message);
     return WasmResult::Ok;
-  default:
-    NOT_REACHED_GCOVR_EXCL_LINE;
+  case spdlog::level::off:
+    PANIC("not implemented");
+  case spdlog::level::n_levels:
+    PANIC("not implemented");
   }
+  PANIC_DUE_TO_CORRUPT_ENUM;
 }
 
 uint32_t Context::getLogLevel() {

diff --git a/source/extensions/filters/common/lua/wrappers.cc b/source/extensions/filters/common/lua/wrappers.cc
@@ -111,9 +111,8 @@ void MetadataMapHelper::setValue(lua_State* state, const ProtobufWkt::Value& val
     }
     return;
   }
-
-  default:
-    NOT_REACHED_GCOVR_EXCL_LINE;
+  case ProtobufWkt::Value::KIND_NOT_SET:
+    PANIC("not implemented");
   }
 }
 

diff --git a/source/extensions/health_checkers/redis/redis.cc b/source/extensions/health_checkers/redis/redis.cc
@@ -81,8 +81,6 @@ void RedisHealthChecker::RedisActiveHealthCheckSession::onInterval() {
   case Type::Ping:
     current_request_ = client_->makeRequest(pingHealthCheckRequest(), *this);
     break;
-  default:
-    NOT_REACHED_GCOVR_EXCL_LINE;
   }
 }
 
@@ -107,8 +105,6 @@ void RedisHealthChecker::RedisActiveHealthCheckSession::onResponse(
       handleFailure(envoy::data::core::v3::ACTIVE);
     }
     break;
-  default:
-    NOT_REACHED_GCOVR_EXCL_LINE;
   }
 
   if (!parent_.reuse_connection_) {

diff --git a/source/extensions/stat_sinks/common/statsd/statsd.cc b/source/extensions/stat_sinks/common/statsd/statsd.cc
@@ -152,7 +152,7 @@ const std::string UdpStatsdSink::buildMessage(const Stats::Metric& metric, Value
     return message;
   }
   }
-  NOT_REACHED_GCOVR_EXCL_LINE;
+  PANIC_DUE_TO_CORRUPT_ENUM;
 }
 
 const std::string UdpStatsdSink::getName(const Stats::Metric& metric) const {

diff --git a/source/extensions/stat_sinks/graphite_statsd/config.cc b/source/extensions/stat_sinks/graphite_statsd/config.cc
@@ -36,10 +36,11 @@ GraphiteStatsdSinkFactory::createStatsSink(const Protobuf::Message& config,
                                                            true, statsd_sink.prefix(), max_bytes,
                                                            Common::Statsd::getGraphiteTagFormat());
   }
-  default:
-    // Verified by schema.
-    NOT_REACHED_GCOVR_EXCL_LINE;
+  case envoy::extensions::stat_sinks::graphite_statsd::v3::GraphiteStatsdSink::StatsdSpecifierCase::
+      STATSD_SPECIFIER_NOT_SET:
+    break; // Fall through to PANIC.
   }
+  PANIC_DUE_TO_CORRUPT_ENUM;
 }
 
 ProtobufTypes::MessagePtr GraphiteStatsdSinkFactory::createEmptyConfigProto() {

diff --git a/source/extensions/stat_sinks/statsd/config.cc b/source/extensions/stat_sinks/statsd/config.cc
@@ -34,10 +34,10 @@ StatsdSinkFactory::createStatsSink(const Protobuf::Message& config,
     return std::make_unique<Common::Statsd::TcpStatsdSink>(
         server.localInfo(), statsd_sink.tcp_cluster_name(), server.threadLocal(),
         server.clusterManager(), server.scope(), statsd_sink.prefix());
-  default:
-    // Verified by schema.
-    NOT_REACHED_GCOVR_EXCL_LINE;
+  case envoy::config::metrics::v3::StatsdSink::StatsdSpecifierCase::STATSD_SPECIFIER_NOT_SET:
+    break; // Fall through to PANIC
   }
+  PANIC_DUE_TO_CORRUPT_ENUM;
 }
 
 ProtobufTypes::MessagePtr StatsdSinkFactory::createEmptyConfigProto() {

diff --git a/source/server/admin/profiling_handler.cc b/source/server/admin/profiling_handler.cc
@@ -53,11 +53,8 @@ Http::Code ProfilingHandler::handlerHeapProfiler(absl::string_view url, Http::Re
       response.add("Fail to start heap profiler: already started");
       res = Http::Code::BadRequest;
     } else if (!Profiler::Heap::startProfiler(profile_path_)) {
-      // GCOVR_EXCL_START
-      // TODO(silentdai) remove the GCOVR when startProfiler is better implemented
       response.add("Fail to start the heap profiler");
       res = Http::Code::InternalServerError;
-      // GCOVR_EXCL_STOP
     } else {
       response.add("Starting heap profiler");
       res = Http::Code::OK;

diff --git a/source/server/admin/utils.cc b/source/server/admin/utils.cc
@@ -18,7 +18,7 @@ envoy::admin::v3::ServerInfo::State serverState(Init::Manager::State state,
     return health_check_failed ? envoy::admin::v3::ServerInfo::DRAINING
                                : envoy::admin::v3::ServerInfo::LIVE;
   }
-  NOT_REACHED_GCOVR_EXCL_LINE;
+  PANIC_DUE_TO_CORRUPT_ENUM;
 }
 
 void populateFallbackResponseHeaders(Http::Code code, Http::ResponseHeaderMap& header_map) {

diff --git a/source/server/config_validation/api.cc b/source/server/config_validation/api.cc
@@ -20,12 +20,12 @@ Event::DispatcherPtr ValidationImpl::allocateDispatcher(const std::string& name)
 Event::DispatcherPtr
 ValidationImpl::allocateDispatcher(const std::string&,
                                    const Event::ScaledRangeTimerManagerFactory&) {
-  NOT_REACHED_GCOVR_EXCL_LINE;
+  PANIC("not implemented");
 }
 
 Event::DispatcherPtr ValidationImpl::allocateDispatcher(const std::string&,
                                                         Buffer::WatermarkFactoryPtr&&) {
-  NOT_REACHED_GCOVR_EXCL_LINE;
+  PANIC("not implemented");
 }
 
 } // namespace Api

diff --git a/source/server/connection_handler_impl.cc b/source/server/connection_handler_impl.cc
@@ -48,7 +48,7 @@ void ConnectionHandlerImpl::addListener(absl::optional<uint64_t> overridden_list
         iter->second->internalListener()->get().updateListenerConfig(config);
         return;
       }
-      NOT_REACHED_GCOVR_EXCL_LINE;
+      IS_ENVOY_BUG("unexpected");
     }
     auto internal_listener = std::make_unique<ActiveInternalListener>(*this, dispatcher(), config);
     details->typed_listener_ = *internal_listener;
@@ -60,7 +60,7 @@ void ConnectionHandlerImpl::addListener(absl::optional<uint64_t> overridden_list
         iter->second->tcpListener()->get().updateListenerConfig(config);
         return;
       }
-      NOT_REACHED_GCOVR_EXCL_LINE;
+      IS_ENVOY_BUG("unexpected");
     }
     // worker_index_ doesn't have a value on the main thread for the admin server.
     auto tcp_listener = std::make_unique<ActiveTcpListener>(

diff --git a/source/server/listener_impl.cc b/source/server/listener_impl.cc
@@ -591,8 +591,6 @@ void ListenerImpl::createListenerFilterFactories(Network::Socket::Type socket_ty
       listener_filter_factories_ = parent_.factory_.createListenerFilterFactoryList(
           config_.listener_filters(), *listener_factory_context_);
       break;
-    default:
-      NOT_REACHED_GCOVR_EXCL_LINE;
     }
   }
 }

diff --git a/source/server/overload_manager_impl.cc b/source/server/overload_manager_impl.cc
@@ -256,8 +256,8 @@ OverloadAction::OverloadAction(const envoy::config::overload::v3::OverloadAction
     case envoy::config::overload::v3::Trigger::TriggerOneofCase::kScaled:
       trigger = std::make_unique<ScaledTriggerImpl>(trigger_config.scaled());
       break;
-    default:
-      NOT_REACHED_GCOVR_EXCL_LINE;
+    case envoy::config::overload::v3::Trigger::TriggerOneofCase::TRIGGER_ONEOF_NOT_SET:
+      PANIC_DUE_TO_CORRUPT_ENUM;
     }
 
     if (!triggers_.try_emplace(trigger_config.name(), std::move(trigger)).second) {

diff --git a/test/per_file_coverage.sh b/test/per_file_coverage.sh
@@ -31,8 +31,8 @@ declare -a KNOWN_LOW_COVERAGE=(
 "source/common/watchdog:58.6" # Death tests don't report LCOV
 "source/exe:92.6"
 "source/extensions/common:95.8"
-"source/extensions/common/tap:94.2"
-"source/extensions/common/wasm:95.1" # flaky: be careful adjusting
+"source/extensions/common/tap:92.9"
+"source/extensions/common/wasm:95.0" # flaky: be careful adjusting
 "source/extensions/common/wasm/ext:92.0"
 "source/extensions/filters/common:96.1"
 "source/extensions/filters/common/expr:96.2"
@@ -66,8 +66,8 @@ declare -a KNOWN_LOW_COVERAGE=(
 "source/extensions/io_socket/user_space:96.2"
 "source/extensions/stat_sinks/common:96.4"
 "source/extensions/stat_sinks/common/statsd:96.4"
-"source/extensions/stat_sinks/graphite_statsd:88.5"
-"source/extensions/stat_sinks/statsd:88.0"
+"source/extensions/stat_sinks/graphite_statsd:75.0"
+"source/extensions/stat_sinks/statsd:76.9"
 "source/extensions/tracers/opencensus:94.8"
 "source/extensions/tracers/xray:96.2"
 "source/extensions/tracers/zipkin:95.9"