From 73a7bf69a5d030e4cad698228cfb99e8214604b8 Mon Sep 17 00:00:00 2001 From: dubious90 Date: Fri, 23 Sep 2022 13:43:57 -0400 Subject: [PATCH] Make the fix_format and check_format scripts work on Nighthawk code (#916) Fixes https://github.com/envoyproxy/nighthawk/issues/913 Direct changes: - Modified config.yaml to not exclude the current directory, which was excluding all of NH - Overrode some settings in config.yaml to move past some code changes - Filed https://github.com/envoyproxy/nighthawk/issues/914 and https://github.com/envoyproxy/nighthawk/issues/915 to address these overrides in the future - Moved include_dir_order from check_format.sh over to the config.yaml where it now needs to live. Also ran the format script to fix all current code formatting errors. Signed-off-by: Nathan Perry --- .../adaptive_load_controller_impl.cc | 7 +- source/client/output_formatter_impl.cc | 9 +- source/client/output_formatter_impl.h | 11 +- source/client/process_impl.cc | 173 ++++++++++++------ source/client/stream_decoder.cc | 4 +- source/sink/sink_impl.cc | 3 +- test/BUILD | 2 +- .../fake_metrics_plugin.cc | 1 - test/adaptive_load/metrics_evaluator_test.cc | 2 +- test/options_test.cc | 23 +-- test/output_formatter_test.cc | 2 +- test/sink/nighthawk_sink_client_test.cc | 18 +- test/test_common/mock_stream.h | 3 +- tools/check_format.sh | 1 - tools/code_format/config.yaml | 33 +++- 15 files changed, 183 insertions(+), 109 deletions(-) diff --git a/source/adaptive_load/adaptive_load_controller_impl.cc b/source/adaptive_load/adaptive_load_controller_impl.cc index cc98219e6..07f8fc3a8 100644 --- a/source/adaptive_load/adaptive_load_controller_impl.cc +++ b/source/adaptive_load/adaptive_load_controller_impl.cc @@ -144,7 +144,8 @@ absl::StatusOr AdaptiveLoadControllerImpl::PerformAndAnalyzeNig command_line_options); Envoy::SystemTime end_time = time_source_.systemTime(); if (!nighthawk_response_or.ok()) { - ENVOY_LOG_MISC(error, "Nighthawk Service error: {}: {}", nighthawk_response_or.status().raw_code(), + ENVOY_LOG_MISC(error, "Nighthawk Service error: {}: {}", + nighthawk_response_or.status().raw_code(), nighthawk_response_or.status().message()); return nighthawk_response_or.status(); } @@ -155,8 +156,8 @@ absl::StatusOr AdaptiveLoadControllerImpl::PerformAndAnalyzeNig metrics_evaluator_.AnalyzeNighthawkBenchmark(nighthawk_response, spec, name_to_custom_plugin_map); if (!benchmark_result_or.ok()) { - ENVOY_LOG_MISC(error, "Benchmark scoring error: {}: {}", benchmark_result_or.status().raw_code(), - benchmark_result_or.status().message()); + ENVOY_LOG_MISC(error, "Benchmark scoring error: {}: {}", + benchmark_result_or.status().raw_code(), benchmark_result_or.status().message()); return benchmark_result_or.status(); } BenchmarkResult benchmark_result = benchmark_result_or.value(); diff --git a/source/client/output_formatter_impl.cc b/source/client/output_formatter_impl.cc index 585c74617..2baa6c9d8 100644 --- a/source/client/output_formatter_impl.cc +++ b/source/client/output_formatter_impl.cc @@ -200,7 +200,8 @@ CsvOutputFormatterImpl::formatProto(const nighthawk::client::Output& output) con } std::string s_percentile = fmt::format("{:.{}g}", p, 8); ss << fmt::format("{},{},{},{}", s_percentile, percentile.count(), - Envoy::Protobuf::util::TimeUtil::DurationToMicroseconds(percentile.duration()), + Envoy::Protobuf::util::TimeUtil::DurationToMicroseconds( + percentile.duration()), percentile.has_duration() ? formatProtoDuration(percentile.duration()) : fmt::format("{}", static_cast(percentile.raw_value())), @@ -210,7 +211,7 @@ CsvOutputFormatterImpl::formatProto(const nighthawk::client::Output& output) con }); ss << std::endl; } - + // Counters ss << fmt::format("{},{},{}", "Counter", "Value", "Per second") << std::endl; for (const nighthawk::client::Counter& counter : result.counters()) { @@ -226,8 +227,8 @@ CsvOutputFormatterImpl::formatProto(const nighthawk::client::Output& output) con return ss.str(); } -std::string CsvOutputFormatterImpl::formatProtoDuration( - const Envoy::ProtobufWkt::Duration& duration) const { +std::string +CsvOutputFormatterImpl::formatProtoDuration(const Envoy::ProtobufWkt::Duration& duration) const { int64_t microseconds = Envoy::Protobuf::util::TimeUtil::DurationToMicroseconds(duration); return fmt::format("{}s {:03}ms {:03}us", (microseconds % 1'000'000'000) / 1'000'000, (microseconds % 1'000'000) / 1'000, microseconds % 1'000); diff --git a/source/client/output_formatter_impl.h b/source/client/output_formatter_impl.h index ee3c01de7..b12477821 100644 --- a/source/client/output_formatter_impl.h +++ b/source/client/output_formatter_impl.h @@ -65,17 +65,20 @@ class CsvOutputFormatterImpl : public OutputFormatterImpl { /** * Return name associated with the specified stat id. * - * @param stat_id id that represents a specific stat. Example: "benchmark_http_client.queue_to_connect" + * @param stat_id id that represents a specific stat. Example: + * "benchmark_http_client.queue_to_connect" * @return name or description (as string) associated with the specified stat id */ static std::string statIdtoFriendlyStatName(absl::string_view stat_id); private: /** - * Transforms an Envoy::ProtobufWkt::Duration to a human-readable string in the format of "{}s {:03}ms {:03}us". + * Transforms an Envoy::ProtobufWkt::Duration to a human-readable string in the format of "{}s + * {:03}ms {:03}us". * * @param duration the Envoy::ProtobufWkt::Duration& to transform - * @return the human-readable string representation of the specified duration. Example: "11s 033ms 190us" + * @return the human-readable string representation of the specified duration. Example: "11s 033ms + * 190us" */ std::string formatProtoDuration(const Envoy::ProtobufWkt::Duration& duration) const; }; @@ -167,4 +170,4 @@ class FortioPedanticOutputFormatterImpl : public FortioOutputFormatterImpl { }; } // namespace Client -} // namespace Nighthawk \ No newline at end of file +} // namespace Nighthawk diff --git a/source/client/process_impl.cc b/source/client/process_impl.cc index 6375fbb25..30b91f3b2 100644 --- a/source/client/process_impl.cc +++ b/source/client/process_impl.cc @@ -103,59 +103,107 @@ class BootstrapFactory : public Envoy::Logger::Loggable // Implementation of Envoy::Server::Instance. Only methods used by Envoy's code // when Nighthawk is running are implemented. class NighthawkServerInstance : public Envoy::Server::Instance { - public: - NighthawkServerInstance(Envoy::Server::Admin& admin, Envoy::Api::Api& api, Envoy::Event::Dispatcher& dispatcher, +public: + NighthawkServerInstance(Envoy::Server::Admin& admin, Envoy::Api::Api& api, + Envoy::Event::Dispatcher& dispatcher, Envoy::AccessLog::AccessLogManager& log_manager, - Envoy::Server::Options& options, - Envoy::Runtime::Loader& runtime, Envoy::Singleton::Manager& singleton_manager, Envoy::ThreadLocal::Instance& tls, - Envoy::LocalInfo::LocalInfo& local_info) : - admin_(admin), api_(api), dispatcher_(dispatcher), log_manager_(log_manager), - options_(options), - runtime_(runtime), singleton_manager_(singleton_manager), tls_(tls), local_info_(local_info) {} + Envoy::Server::Options& options, Envoy::Runtime::Loader& runtime, + Envoy::Singleton::Manager& singleton_manager, + Envoy::ThreadLocal::Instance& tls, + Envoy::LocalInfo::LocalInfo& local_info) + : admin_(admin), api_(api), dispatcher_(dispatcher), log_manager_(log_manager), + options_(options), runtime_(runtime), singleton_manager_(singleton_manager), tls_(tls), + local_info_(local_info) {} Envoy::Server::Admin& admin() override { return admin_; } Envoy::Api::Api& api() override { return api_; } - Envoy::Upstream::ClusterManager& clusterManager() override { PANIC("NighthawkServerInstance::clusterManager not implemented"); } + Envoy::Upstream::ClusterManager& clusterManager() override { + PANIC("NighthawkServerInstance::clusterManager not implemented"); + } const Envoy::Upstream::ClusterManager& clusterManager() const override { PANIC("NighthawkServerInstance::clusterManager not implemented"); } - Envoy::Ssl::ContextManager& sslContextManager() override { PANIC("NighthawkServerInstance::sslContextManager not implemented"); } + Envoy::Ssl::ContextManager& sslContextManager() override { + PANIC("NighthawkServerInstance::sslContextManager not implemented"); + } Envoy::Event::Dispatcher& dispatcher() override { return dispatcher_; } - Envoy::Network::DnsResolverSharedPtr dnsResolver() override { PANIC("NighthawkServerInstance::dnsResolver not implemented"); } - void drainListeners() override { PANIC("NighthawkServerInstance::drainListeners not implemented"); } - Envoy::Server::DrainManager& drainManager() override { PANIC("NighthawkServerInstance::drainManager not implemented"); } + Envoy::Network::DnsResolverSharedPtr dnsResolver() override { + PANIC("NighthawkServerInstance::dnsResolver not implemented"); + } + void drainListeners() override { + PANIC("NighthawkServerInstance::drainListeners not implemented"); + } + Envoy::Server::DrainManager& drainManager() override { + PANIC("NighthawkServerInstance::drainManager not implemented"); + } Envoy::AccessLog::AccessLogManager& accessLogManager() override { return log_manager_; } - void failHealthcheck(bool) override { PANIC("NighthawkServerInstance::failHealthcheck not implemented"); } - bool healthCheckFailed() override { PANIC("NighthawkServerInstance::healthCheckFailed not implemented"); } - Envoy::Server::HotRestart& hotRestart() override { PANIC("NighthawkServerInstance::hotRestart not implemented"); } - Envoy::Init::Manager& initManager() override { PANIC("NighthawkServerInstance::initManager not implemented"); } - Envoy::Server::ListenerManager& listenerManager() override { PANIC("NighthawkServerInstance::listenerManager not implemented"); } - Envoy::MutexTracer* mutexTracer() override { PANIC("NighthawkServerInstance::mutexTracer not implemented"); } - Envoy::Server::OverloadManager& overloadManager() override { PANIC("NighthawkServerInstance::overloadManager not implemented"); } - Envoy::Secret::SecretManager& secretManager() override { PANIC("NighthawkServerInstance::secretManager not implemented"); } + void failHealthcheck(bool) override { + PANIC("NighthawkServerInstance::failHealthcheck not implemented"); + } + bool healthCheckFailed() override { + PANIC("NighthawkServerInstance::healthCheckFailed not implemented"); + } + Envoy::Server::HotRestart& hotRestart() override { + PANIC("NighthawkServerInstance::hotRestart not implemented"); + } + Envoy::Init::Manager& initManager() override { + PANIC("NighthawkServerInstance::initManager not implemented"); + } + Envoy::Server::ListenerManager& listenerManager() override { + PANIC("NighthawkServerInstance::listenerManager not implemented"); + } + Envoy::MutexTracer* mutexTracer() override { + PANIC("NighthawkServerInstance::mutexTracer not implemented"); + } + Envoy::Server::OverloadManager& overloadManager() override { + PANIC("NighthawkServerInstance::overloadManager not implemented"); + } + Envoy::Secret::SecretManager& secretManager() override { + PANIC("NighthawkServerInstance::secretManager not implemented"); + } const Envoy::Server::Options& options() override { return options_; } Envoy::Runtime::Loader& runtime() override { return runtime_; } - Envoy::Server::ServerLifecycleNotifier& lifecycleNotifier() override { PANIC("NighthawkServerInstance::lifecycleNotifier not implemented"); } + Envoy::Server::ServerLifecycleNotifier& lifecycleNotifier() override { + PANIC("NighthawkServerInstance::lifecycleNotifier not implemented"); + } void shutdown() override { PANIC("NighthawkServerInstance::shutdown not implemented"); } bool isShutdown() override { PANIC("NighthawkServerInstance::isShutdown not implemented"); } void shutdownAdmin() override { PANIC("NighthawkServerInstance::shutdownAdmin not implemented"); } Envoy::Singleton::Manager& singletonManager() override { return singleton_manager_; } - time_t startTimeCurrentEpoch() override { PANIC("NighthawkServerInstance::startTimeCurrentEpoch not implemented"); } - time_t startTimeFirstEpoch() override { PANIC("NighthawkServerInstance::startTimeFirstEpoch not implemented"); } + time_t startTimeCurrentEpoch() override { + PANIC("NighthawkServerInstance::startTimeCurrentEpoch not implemented"); + } + time_t startTimeFirstEpoch() override { + PANIC("NighthawkServerInstance::startTimeFirstEpoch not implemented"); + } Envoy::Stats::Store& stats() override { PANIC("NighthawkServerInstance::stats not implemented"); } - Envoy::Grpc::Context& grpcContext() override { PANIC("NighthawkServerInstance::grpcContext not implemented"); } - Envoy::Http::Context& httpContext() override { PANIC("NighthawkServerInstance::httpContext not implemented"); } - Envoy::Router::Context& routerContext() override { PANIC("NighthawkServerInstance::routerContext not implemented"); } - Envoy::ProcessContextOptRef processContext() override { PANIC("NighthawkServerInstance::processContext not implemented"); } + Envoy::Grpc::Context& grpcContext() override { + PANIC("NighthawkServerInstance::grpcContext not implemented"); + } + Envoy::Http::Context& httpContext() override { + PANIC("NighthawkServerInstance::httpContext not implemented"); + } + Envoy::Router::Context& routerContext() override { + PANIC("NighthawkServerInstance::routerContext not implemented"); + } + Envoy::ProcessContextOptRef processContext() override { + PANIC("NighthawkServerInstance::processContext not implemented"); + } Envoy::ThreadLocal::Instance& threadLocal() override { return tls_; } Envoy::LocalInfo::LocalInfo& localInfo() const override { return local_info_; } - Envoy::TimeSource& timeSource() override { PANIC("NighthawkServerInstance::timeSource not implemented"); } + Envoy::TimeSource& timeSource() override { + PANIC("NighthawkServerInstance::timeSource not implemented"); + } void flushStats() override { PANIC("NighthawkServerInstance::flushStats not implemented"); } Envoy::ProtobufMessage::ValidationContext& messageValidationContext() override { PANIC("NighthawkServerInstance::messageValidationContext not implemented"); } - Envoy::Server::Configuration::StatsConfig& statsConfig() override { PANIC("NighthawkServerInstance::statsConfig not implemented"); } - envoy::config::bootstrap::v3::Bootstrap& bootstrap() override { PANIC("NighthawkServerInstance::bootstrap not implemented"); } + Envoy::Server::Configuration::StatsConfig& statsConfig() override { + PANIC("NighthawkServerInstance::statsConfig not implemented"); + } + envoy::config::bootstrap::v3::Bootstrap& bootstrap() override { + PANIC("NighthawkServerInstance::bootstrap not implemented"); + } Envoy::Server::Configuration::ServerFactoryContext& serverFactoryContext() override { PANIC("NighthawkServerInstance::serverFactoryContext not implemented"); } @@ -166,12 +214,14 @@ class NighthawkServerInstance : public Envoy::Server::Instance { void setDefaultTracingConfig(const envoy::config::trace::v3::Tracing&) override { PANIC("NighthawkServerInstance::setDefaultTracingConfig not implemented"); } - bool enableReusePortDefault() override { PANIC("NighthawkServerInstance::enableReusePortDefault not implemented"); } + bool enableReusePortDefault() override { + PANIC("NighthawkServerInstance::enableReusePortDefault not implemented"); + } void setSinkPredicates(std::unique_ptr&&) override { PANIC("NighthawkServerInstance::setSinkPredicates not implemented"); } - private: +private: Envoy::Server::Admin& admin_; Envoy::Api::Api& api_; Envoy::Event::Dispatcher& dispatcher_; @@ -181,12 +231,11 @@ class NighthawkServerInstance : public Envoy::Server::Instance { Envoy::Singleton::Manager& singleton_manager_; Envoy::ThreadLocal::Instance& tls_; Envoy::LocalInfo::LocalInfo& local_info_; - }; // Implementation of Envoy::Server::Configuration::ServerFactoryContext. class NighthawkServerFactoryContext : public Envoy::Server::Configuration::ServerFactoryContext { - public: +public: NighthawkServerFactoryContext(Envoy::Server::Instance& server) : server_(server) {} const Envoy::Server::Options& options() override { return server_.options(); }; @@ -207,46 +256,66 @@ class NighthawkServerFactoryContext : public Envoy::Server::Configuration::Serve return Envoy::ProtobufMessage::getStrictValidationVisitor(); }; - Envoy::Stats::Scope& scope() override { PANIC("NighthawkServerFactoryContext::scope not implemented"); }; + Envoy::Stats::Scope& scope() override { + PANIC("NighthawkServerFactoryContext::scope not implemented"); + }; - Envoy::Stats::Scope& serverScope() override { PANIC("NighthawkServerFactoryContext::serverScope not implemented"); }; + Envoy::Stats::Scope& serverScope() override { + PANIC("NighthawkServerFactoryContext::serverScope not implemented"); + }; Envoy::ThreadLocal::SlotAllocator& threadLocal() override { return server_.threadLocal(); } - Envoy::Upstream::ClusterManager& clusterManager() override { PANIC("NighthawkServerFactoryContext::clusterManager not implemented"); }; + Envoy::Upstream::ClusterManager& clusterManager() override { + PANIC("NighthawkServerFactoryContext::clusterManager not implemented"); + }; Envoy::ProtobufMessage::ValidationContext& messageValidationContext() override { PANIC("NighthawkServerFactoryContext::messageValidationContext not implemented"); }; - Envoy::TimeSource& timeSource() override { PANIC("NighthawkServerFactoryContext::timeSource not implemented"); }; + Envoy::TimeSource& timeSource() override { + PANIC("NighthawkServerFactoryContext::timeSource not implemented"); + }; - Envoy::AccessLog::AccessLogManager& accessLogManager() override { return server_.accessLogManager(); } + Envoy::AccessLog::AccessLogManager& accessLogManager() override { + return server_.accessLogManager(); + } Envoy::Server::ServerLifecycleNotifier& lifecycleNotifier() override { PANIC("NighthawkServerFactoryContext::lifecycleNotifier not implemented"); }; - Envoy::Init::Manager& initManager() override { PANIC("NighthawkServerFactoryContext::initManager not implemented"); }; + Envoy::Init::Manager& initManager() override { + PANIC("NighthawkServerFactoryContext::initManager not implemented"); + }; - Envoy::Grpc::Context& grpcContext() override { PANIC("NighthawkServerFactoryContext::grpcContext not implemented"); }; + Envoy::Grpc::Context& grpcContext() override { + PANIC("NighthawkServerFactoryContext::grpcContext not implemented"); + }; - Envoy::Router::Context& routerContext() override { PANIC("NighthawkServerFactoryContext::routerContext not implemented"); }; + Envoy::Router::Context& routerContext() override { + PANIC("NighthawkServerFactoryContext::routerContext not implemented"); + }; - Envoy::Server::DrainManager& drainManager() override { PANIC("NighthawkServerFactoryContext::drainManager not implemented"); }; + Envoy::Server::DrainManager& drainManager() override { + PANIC("NighthawkServerFactoryContext::drainManager not implemented"); + }; - Envoy::Server::Configuration::StatsConfig& statsConfig() override { PANIC("NighthawkServerFactoryContext::statsConfig not implemented"); } + Envoy::Server::Configuration::StatsConfig& statsConfig() override { + PANIC("NighthawkServerFactoryContext::statsConfig not implemented"); + } - envoy::config::bootstrap::v3::Bootstrap& bootstrap() override { PANIC("NighthawkServerFactoryContext::bootstrap not implemented"); } + envoy::config::bootstrap::v3::Bootstrap& bootstrap() override { + PANIC("NighthawkServerFactoryContext::bootstrap not implemented"); + } - private: +private: Envoy::Server::Instance& server_; }; // Disables the hot restart Envoy functionality. -std::string HotRestartDisabled(bool) { - return "hot restart is disabled"; -} +std::string HotRestartDisabled(bool) { return "hot restart is disabled"; } } // namespace @@ -622,7 +691,9 @@ bool ProcessImpl::runInternal(OutputCollector& collector, const UriPtr& tracing_ std::make_unique( time_system_); - server_ = std::make_unique(admin_, *api_, *dispatcher_, access_log_manager_, envoy_options_, runtime_singleton_->instance(), *singleton_manager_, tls_, *local_info_); + server_ = std::make_unique( + admin_, *api_, *dispatcher_, access_log_manager_, envoy_options_, + runtime_singleton_->instance(), *singleton_manager_, tls_, *local_info_); server_factory_context_ = std::make_unique(*server_); cluster_manager_factory_ = std::make_unique( *server_factory_context_, admin_, Envoy::Runtime::LoaderSingleton::get(), store_root_, tls_, diff --git a/source/client/stream_decoder.cc b/source/client/stream_decoder.cc index 1b2e4afd5..3094bda11 100644 --- a/source/client/stream_decoder.cc +++ b/source/client/stream_decoder.cc @@ -2,14 +2,14 @@ #include -#include "fmt/ostream.h" - #include "external/envoy/source/common/http/http1/codec_impl.h" #include "external/envoy/source/common/http/utility.h" #include "external/envoy/source/common/network/address_impl.h" #include "external/envoy/source/common/stream_info/stream_info_impl.h" #include "external/envoy/source/extensions/request_id/uuid/config.h" +#include "fmt/ostream.h" + namespace Nighthawk { namespace Client { diff --git a/source/sink/sink_impl.cc b/source/sink/sink_impl.cc index 3b3b58ff1..1bbd650cd 100644 --- a/source/sink/sink_impl.cc +++ b/source/sink/sink_impl.cc @@ -138,9 +138,8 @@ InMemorySinkImpl::LoadExecutionResult(absl::string_view execution_id) const { } // namespace Nighthawk - // NOLINT(namespace-nighthawk) namespace fmt { // Allow fmtlib to use operator << defined in std::filesystem::path. template <> struct formatter<::std::filesystem::path> : ostream_formatter {}; -} // namespace fmt +} // namespace fmt diff --git a/test/BUILD b/test/BUILD index d20df456b..099380759 100644 --- a/test/BUILD +++ b/test/BUILD @@ -117,8 +117,8 @@ envoy_cc_test( name = "output_formatter_test", srcs = ["output_formatter_test.cc"], data = [ - "test_data/output_formatter.dotted.gold", "test_data/output_formatter.csv.gold", + "test_data/output_formatter.dotted.gold", "test_data/output_formatter.json.gold", "test_data/output_formatter.medium.fortio.gold", "test_data/output_formatter.medium.fortio-noquirks.gold", diff --git a/test/adaptive_load/fake_plugins/fake_metrics_plugin/fake_metrics_plugin.cc b/test/adaptive_load/fake_plugins/fake_metrics_plugin/fake_metrics_plugin.cc index 6e22d97c3..ae354bddd 100644 --- a/test/adaptive_load/fake_plugins/fake_metrics_plugin/fake_metrics_plugin.cc +++ b/test/adaptive_load/fake_plugins/fake_metrics_plugin/fake_metrics_plugin.cc @@ -41,7 +41,6 @@ absl::StatusOr FakeMetricsPlugin::GetMetricByName(absl::string_view metr return value_or_error_from_name_[metric_name]; } - const std::vector FakeMetricsPlugin::GetAllSupportedMetricNames() const { std::vector metric_names; for (const auto& key_value : value_or_error_from_name_) { diff --git a/test/adaptive_load/metrics_evaluator_test.cc b/test/adaptive_load/metrics_evaluator_test.cc index 76dacb0de..45c1a037c 100644 --- a/test/adaptive_load/metrics_evaluator_test.cc +++ b/test/adaptive_load/metrics_evaluator_test.cc @@ -304,7 +304,7 @@ TEST(AnalyzeNighthawkBenchmark, StoresNighthawkResultForSuccessfulMetricEvaluati EXPECT_TRUE(MessageDifferencer::Equivalent(result_or.value().nighthawk_service_output(), nighthawk_response.output())); EXPECT_THAT(result_or.value().nighthawk_service_output(), - EqualsProto(nighthawk_response.output())); + EqualsProto(nighthawk_response.output())); } TEST(AnalyzeNighthawkBenchmark, StoresScoreForSuccessfulMetricEvaluation) { diff --git a/test/options_test.cc b/test/options_test.cc index 1c82d3f2a..f8fb005e0 100644 --- a/test/options_test.cc +++ b/test/options_test.cc @@ -244,9 +244,7 @@ TEST_F(OptionsImplTest, AlmostAll) { } })pb", &expected_transport_socket); - EXPECT_THAT( - options->transportSocket().value(), - EqualsProto(expected_transport_socket)); + EXPECT_THAT(options->transportSocket().value(), EqualsProto(expected_transport_socket)); EXPECT_EQ(10, options->maxPendingRequests()); EXPECT_EQ(11, options->maxActiveRequests()); EXPECT_EQ(12, options->maxRequestsPerConnection()); @@ -774,12 +772,10 @@ TEST_F(OptionsImplTest, BadHttp3ProtocolOptionsSpecification) { client_name_, "{broken_json:")), MalformedArgvException, "Unable to parse JSON as proto"); // Correct JSON, but contents not according to spec. - EXPECT_THROW_WITH_REGEX( - TestUtility::createOptionsImpl( - fmt::format("{} --protocol http3 --http3-protocol-options {} http://foo/", client_name_, - "{invalid_http3_protocol_options:{}}")), - MalformedArgvException, - "INVALID_ARGUMENT"); + EXPECT_THROW_WITH_REGEX(TestUtility::createOptionsImpl(fmt::format( + "{} --protocol http3 --http3-protocol-options {} http://foo/", + client_name_, "{invalid_http3_protocol_options:{}}")), + MalformedArgvException, "INVALID_ARGUMENT"); } TEST_F(OptionsImplTest, FailsWhenHttp3ProtocolOptionsSpecifiedForNonHttp3) { @@ -972,8 +968,7 @@ TEST_F(OptionsImplTest, BadTlsContextSpecification) { EXPECT_THROW_WITH_REGEX( TestUtility::createOptionsImpl(fmt::format("{} --tls-context {} http://foo/", client_name_, "{misspelled_tls_context:{}}")), - MalformedArgvException, - "INVALID_ARGUMENT"); + MalformedArgvException, "INVALID_ARGUMENT"); } TEST_F(OptionsImplTest, BadUpstreamBindConfigSpecification) { @@ -986,8 +981,7 @@ TEST_F(OptionsImplTest, BadUpstreamBindConfigSpecification) { EXPECT_THROW_WITH_REGEX( TestUtility::createOptionsImpl(fmt::format("{} --upstream-bind-config {} http://foo/", client_name_, "{invalid_bind_config:{}}")), - MalformedArgvException, - "INVALID_ARGUMENT"); + MalformedArgvException, "INVALID_ARGUMENT"); } TEST_F(OptionsImplTest, BadTransportSocketSpecification) { @@ -1000,8 +994,7 @@ TEST_F(OptionsImplTest, BadTransportSocketSpecification) { EXPECT_THROW_WITH_REGEX( TestUtility::createOptionsImpl(fmt::format("{} --transport-socket {} http://foo/", client_name_, "{misspelled_transport_socket:{}}")), - MalformedArgvException, - "INVALID_ARGUMENT"); + MalformedArgvException, "INVALID_ARGUMENT"); } TEST_F(OptionsImplTest, BadStatsSinksSpecification) { diff --git a/test/output_formatter_test.cc b/test/output_formatter_test.cc index 47392e407..05eec5721 100644 --- a/test/output_formatter_test.cc +++ b/test/output_formatter_test.cc @@ -136,7 +136,7 @@ TEST_F(OutputCollectorTest, GetLowerCaseOutputFormats) { // When you're looking at this code you probably just added an output format. // This is to point out that you might want to update the list below and add a test above. ASSERT_THAT(output_formats, ElementsAre("json", "human", "yaml", "dotted", "fortio", - "experimental_fortio_pedantic","csv")); + "experimental_fortio_pedantic", "csv")); } class FortioOutputCollectorTest : public OutputCollectorTest { diff --git a/test/sink/nighthawk_sink_client_test.cc b/test/sink/nighthawk_sink_client_test.cc index 8edcad153..85ce37343 100644 --- a/test/sink/nighthawk_sink_client_test.cc +++ b/test/sink/nighthawk_sink_client_test.cc @@ -161,8 +161,7 @@ TEST(SinkRequest, UsesSpecifiedCommandLineOptions) { // test requests a channel. Set call expectations on the inner mock channel. EXPECT_CALL(mock_nighthawk_sink_stub, SinkRequestStreamRaw) .WillOnce([&request](grpc::ClientContext*) { - auto* mock_reader_writer = - new MockClientReaderWriter(); + auto* mock_reader_writer = new MockClientReaderWriter(); // SinkRequest currently expects Read to return true exactly once. EXPECT_CALL(*mock_reader_writer, Read(_)).WillOnce(Return(true)).WillOnce(Return(false)); // Capture the Nighthawk request SinkRequest sends on the channel. @@ -189,8 +188,7 @@ TEST(SinkRequest, ReturnsNighthawkResponseSuccessfully) { // test requests a channel. Set call expectations on the inner mock channel. EXPECT_CALL(mock_nighthawk_sink_stub, SinkRequestStreamRaw) .WillOnce([&expected_response](grpc::ClientContext*) { - auto* mock_reader_writer = - new MockClientReaderWriter(); + auto* mock_reader_writer = new MockClientReaderWriter(); // SinkRequest currently expects Read to return true exactly once. // Capture the gRPC response proto as it is written to the output parameter. EXPECT_CALL(*mock_reader_writer, Read(_)) @@ -215,8 +213,7 @@ TEST(SinkRequest, WillFinishIfNighthawkServiceDoesNotSendResponse) { // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under // test requests a channel. Set call expectations on the inner mock channel. EXPECT_CALL(mock_nighthawk_sink_stub, SinkRequestStreamRaw).WillOnce([](grpc::ClientContext*) { - auto* mock_reader_writer = - new MockClientReaderWriter(); + auto* mock_reader_writer = new MockClientReaderWriter(); EXPECT_CALL(*mock_reader_writer, Read(_)).WillOnce(Return(false)); EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(true)); @@ -235,8 +232,7 @@ TEST(SinkRequest, ReturnsErrorIfNighthawkServiceWriteFails) { // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under // test requests a channel. Set call expectations on the inner mock channel. EXPECT_CALL(mock_nighthawk_sink_stub, SinkRequestStreamRaw).WillOnce([](grpc::ClientContext*) { - auto* mock_reader_writer = - new MockClientReaderWriter(); + auto* mock_reader_writer = new MockClientReaderWriter(); EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(false)); return mock_reader_writer; }); @@ -254,8 +250,7 @@ TEST(SinkRequest, ReturnsErrorIfNighthawkServiceWritesDoneFails) { // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under // test requests a channel. Set call expectations on the inner mock channel. EXPECT_CALL(mock_nighthawk_sink_stub, SinkRequestStreamRaw).WillOnce([](grpc::ClientContext*) { - auto* mock_reader_writer = - new MockClientReaderWriter(); + auto* mock_reader_writer = new MockClientReaderWriter(); EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(false)); return mock_reader_writer; @@ -274,8 +269,7 @@ TEST(SinkRequest, PropagatesErrorIfNighthawkServiceGrpcStreamClosesAbnormally) { // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under // test requests a channel. Set call expectations on the inner mock channel. EXPECT_CALL(mock_nighthawk_sink_stub, SinkRequestStreamRaw).WillOnce([](grpc::ClientContext*) { - auto* mock_reader_writer = - new MockClientReaderWriter(); + auto* mock_reader_writer = new MockClientReaderWriter(); // SinkRequest currently expects Read to return true exactly once. EXPECT_CALL(*mock_reader_writer, Read(_)).WillOnce(Return(true)).WillOnce(Return(false)); EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(true)); diff --git a/test/test_common/mock_stream.h b/test/test_common/mock_stream.h index 565e470d3..9bad4cd1a 100644 --- a/test/test_common/mock_stream.h +++ b/test/test_common/mock_stream.h @@ -1,7 +1,8 @@ #pragma once #include -#include + +#include "grpcpp/support/sync_stream.h" namespace Nighthawk { diff --git a/tools/check_format.sh b/tools/check_format.sh index a7daca6a5..98bd7266f 100755 --- a/tools/check_format.sh +++ b/tools/check_format.sh @@ -15,7 +15,6 @@ TO_CHECK="${2:-$FULL_CHECK}" bazel run @envoy//tools/code_format:check_format.py -- \ --skip_envoy_build_rule_check --namespace_check Nighthawk \ --build_fixer_check_excluded_paths=$TO_CHECK \ - --include_dir_order envoy,nighthawk,external/source/envoy,external,api,common,source,exe,server,client,distributor,sink,grpcpp,request_source,test_common,test \ $1 $TO_CHECK # The include checker doesn't support per-file checking, so we only diff --git a/tools/code_format/config.yaml b/tools/code_format/config.yaml index cb9d3f0cd..fce3e0bf3 100644 --- a/tools/code_format/config.yaml +++ b/tools/code_format/config.yaml @@ -35,7 +35,8 @@ suffixes: paths: # Path prefixes to exclude from checking excluded: - - . + # this exclusion prevents all of our files from being formatted # unique + # - . # unique - bazel/external/http_parser/ - bazel/toolchains/configs/ - bazel- @@ -119,6 +120,8 @@ paths: - ci/prebuilt - source/common/protobuf - test/extensions/bootstrap/wasm/test_data + - . # unique + # TODO(915): Remove this inclusion when all direct references removed # unique # Files that are allowed to use try without main thread assertion. raw_try: @@ -215,14 +218,23 @@ paths: - test/extensions/stats_sinks/wasm/wasm_stat_sink_test.cc - test/test_common/wasm_base.h -dir_order: - - envoy - - common - - source - - exe - - server - - extensions - - test +dir_order: # unique + - envoy # unique + - nighthawk # unique + - external/source/envoy # unique + - external # unique + - api # unique + - common # unique + - source # unique + - exe # unique + - server # unique + - client # unique + - distributor # unique + - sink # unique + - grpcpp # unique + - request_source # unique + - test_common # unique + - test # unique re: codeowners_contrib: (/contrib/[^@]*\s+)(@.*) @@ -239,7 +251,8 @@ re: line_number: ^(\d+)[a|c|d]?\d*(?:,\d+[a|c|d]?\d*)?$ mangled_protobuf_name: envoy::[a-z0-9_:]+::[A-Z][a-z]\w*_\w*_[A-Z]{2} maintainers: .*github.com.(.*)\)\) - old_mock_method: MOCK_METHOD\d + # TODO(914): Stop overriding this when we are using the correct MOCK_METHOD # unique + old_mock_method: dontmatchanything # unique proto_validation_string: \bmin_bytes\b owner: "@\\S+" runtime_guard_flag: RUNTIME_GUARD\((.*)\);