diff options
author | Sree Kuchibhotla <sreek@google.com> | 2018-06-18 09:37:46 -0700 |
---|---|---|
committer | Sree Kuchibhotla <sreek@google.com> | 2018-06-18 09:39:09 -0700 |
commit | 09d7da2652e64c38267bde4ba9c52673e2b97f32 (patch) | |
tree | fbdd9fbc058fa2c55a60044c1bd88bf429e4aa28 /test | |
parent | 16ad9b828073579d0c6364570a3803b26e7fb39d (diff) | |
parent | a2769d5ee4116152eead50678741906b1667923b (diff) |
Merge branch 'master' into epollex-ownerfd-fix
Diffstat (limited to 'test')
50 files changed, 162 insertions, 156 deletions
diff --git a/test/core/channel/BUILD b/test/core/channel/BUILD index e5a9e36457..c554b20148 100644 --- a/test/core/channel/BUILD +++ b/test/core/channel/BUILD @@ -105,7 +105,6 @@ grpc_cc_test( language = "C++", deps = [ "//:grpc", - "//test/core/util:gpr_test_util", ], external_deps = [ "gtest", diff --git a/test/core/channel/channel_trace_test.cc b/test/core/channel/channel_trace_test.cc index 64de05bc0a..d99a32d91d 100644 --- a/test/core/channel/channel_trace_test.cc +++ b/test/core/channel/channel_trace_test.cc @@ -77,13 +77,13 @@ void ValidateChannelTraceData(grpc_json* json, ValidateJsonArraySize(json, "events", actual_num_events_expected); } -void AddSimpleTrace(RefCountedPtr<ChannelTrace> tracer) { +void AddSimpleTrace(const RefCountedPtr<ChannelTrace>& tracer) { tracer->AddTraceEvent(ChannelTrace::Severity::Info, grpc_slice_from_static_string("simple trace")); } // checks for the existence of all the required members of the tracer. -void ValidateChannelTrace(RefCountedPtr<ChannelTrace> tracer, +void ValidateChannelTrace(const RefCountedPtr<ChannelTrace>& tracer, size_t expected_num_event_logged, size_t max_nodes) { if (!max_nodes) return; char* json_str = tracer->RenderTrace(); @@ -95,7 +95,8 @@ void ValidateChannelTrace(RefCountedPtr<ChannelTrace> tracer, gpr_free(json_str); } -void ValidateTraceDataMatchedUuidLookup(RefCountedPtr<ChannelTrace> tracer) { +void ValidateTraceDataMatchedUuidLookup( + const RefCountedPtr<ChannelTrace>& tracer) { intptr_t uuid = tracer->GetUuid(); if (uuid == -1) return; // Doesn't make sense to lookup if tracing disabled char* tracer_json_str = tracer->RenderTrace(); diff --git a/test/core/channel/channelz_registry_test.cc b/test/core/channel/channelz_registry_test.cc index 37696dc0e8..eb6305eb4e 100644 --- a/test/core/channel/channelz_registry_test.cc +++ b/test/core/channel/channelz_registry_test.cc @@ -54,6 +54,7 @@ TEST(ChannelzRegistryTest, UuidStartsAboveZeroTest) { TEST(ChannelzRegistryTest, UuidsAreIncreasing) { int object_to_register; std::vector<intptr_t> uuids; + uuids.reserve(10); for (int i = 0; i < 10; ++i) { // reregister the same object. It's ok since we are just testing uuids uuids.push_back(ChannelzRegistry::Register(&object_to_register)); diff --git a/test/core/end2end/fixtures/h2_http_proxy.cc b/test/core/end2end/fixtures/h2_http_proxy.cc index c2ac209cf9..0af8a29a15 100644 --- a/test/core/end2end/fixtures/h2_http_proxy.cc +++ b/test/core/end2end/fixtures/h2_http_proxy.cc @@ -69,8 +69,9 @@ void chttp2_init_client_fullstack(grpc_end2end_test_fixture* f, char* proxy_uri; /* If testing for proxy auth, add credentials to proxy uri */ - const char* proxy_auth_str = - grpc_channel_args_get_string(client_args, GRPC_ARG_HTTP_PROXY_AUTH_CREDS); + const grpc_arg* proxy_auth_arg = + grpc_channel_args_find(client_args, GRPC_ARG_HTTP_PROXY_AUTH_CREDS); + const char* proxy_auth_str = grpc_channel_arg_get_string(proxy_auth_arg); if (proxy_auth_str == nullptr) { gpr_asprintf(&proxy_uri, "http://%s", grpc_end2end_http_proxy_get_proxy_name(ffd->proxy)); diff --git a/test/core/end2end/fixtures/http_proxy_fixture.cc b/test/core/end2end/fixtures/http_proxy_fixture.cc index 5caddee09e..f02fa9d998 100644 --- a/test/core/end2end/fixtures/http_proxy_fixture.cc +++ b/test/core/end2end/fixtures/http_proxy_fixture.cc @@ -411,8 +411,9 @@ static void on_read_request_done(void* arg, grpc_error* error) { return; } // If proxy auth is being used, check if the header is present and as expected - char* proxy_auth_str = grpc_channel_args_get_string( + const grpc_arg* proxy_auth_arg = grpc_channel_args_find( conn->proxy->channel_args, GRPC_ARG_HTTP_PROXY_AUTH_CREDS); + char* proxy_auth_str = grpc_channel_arg_get_string(proxy_auth_arg); if (proxy_auth_str != nullptr) { bool client_authenticated = false; for (size_t i = 0; i < conn->http_request.hdr_count; i++) { diff --git a/test/core/gprpp/ref_counted_ptr_test.cc b/test/core/gprpp/ref_counted_ptr_test.cc index c810345166..aa30b72282 100644 --- a/test/core/gprpp/ref_counted_ptr_test.cc +++ b/test/core/gprpp/ref_counted_ptr_test.cc @@ -66,14 +66,14 @@ TEST(RefCountedPtr, MoveAssignment) { TEST(RefCountedPtr, CopyConstructor) { RefCountedPtr<Foo> foo(New<Foo>()); - RefCountedPtr<Foo> foo2(foo); + const RefCountedPtr<Foo>& foo2(foo); EXPECT_NE(nullptr, foo.get()); EXPECT_EQ(foo.get(), foo2.get()); } TEST(RefCountedPtr, CopyAssignment) { RefCountedPtr<Foo> foo(New<Foo>()); - RefCountedPtr<Foo> foo2 = foo; + const RefCountedPtr<Foo>& foo2 = foo; EXPECT_NE(nullptr, foo.get()); EXPECT_EQ(foo.get(), foo2.get()); } diff --git a/test/core/security/BUILD b/test/core/security/BUILD index 12aa84d93b..70bcc8c9c3 100644 --- a/test/core/security/BUILD +++ b/test/core/security/BUILD @@ -183,7 +183,6 @@ grpc_cc_test( "//:gpr", "//:gpr_base", "//:grpc", - "//test/core/util:gpr_test_util", ], ) @@ -196,7 +195,6 @@ grpc_cc_test( "//:gpr", "//:gpr_base", "//:grpc", - "//test/core/util:gpr_test_util", ], ) @@ -208,7 +206,6 @@ grpc_cc_test( "//:alts_util", "//:gpr", "//:grpc", - "//test/core/util:gpr_test_util", ], ) @@ -223,6 +220,5 @@ grpc_cc_test( "//:grpc_secure", "//:tsi", "//:tsi_interface", - "//test/core/util:gpr_test_util", ], ) diff --git a/test/core/transport/BUILD b/test/core/transport/BUILD index 7ca1c1d943..84fb3a1421 100644 --- a/test/core/transport/BUILD +++ b/test/core/transport/BUILD @@ -129,7 +129,6 @@ grpc_cc_test( language = "C++", deps = [ "//:grpc", - "//test/core/util:gpr_test_util", ], external_deps = [ "gtest", diff --git a/test/core/tsi/alts/crypt/BUILD b/test/core/tsi/alts/crypt/BUILD index abe1e83656..cf9dbca316 100644 --- a/test/core/tsi/alts/crypt/BUILD +++ b/test/core/tsi/alts/crypt/BUILD @@ -27,7 +27,6 @@ grpc_cc_test( "//:alts_frame_protector", "//:gpr", "//:grpc", - "//test/core/util:gpr_test_util", ], ) diff --git a/test/core/tsi/alts/frame_protector/BUILD b/test/core/tsi/alts/frame_protector/BUILD index 6ff3015f4d..dd1966b379 100644 --- a/test/core/tsi/alts/frame_protector/BUILD +++ b/test/core/tsi/alts/frame_protector/BUILD @@ -27,7 +27,6 @@ grpc_cc_test( "//:gpr", "//:grpc", "//test/core/tsi/alts/crypt:alts_crypt_test_util", - "//test/core/util:gpr_test_util", ], ) @@ -40,7 +39,6 @@ grpc_cc_test( "//:gpr", "//:grpc", "//test/core/tsi/alts/crypt:alts_crypt_test_util", - "//test/core/util:gpr_test_util", ], ) @@ -56,7 +54,6 @@ grpc_cc_test( "//:tsi_interface", "//test/core/tsi/alts/crypt:alts_crypt_test_util", "//test/core/tsi:transport_security_test_lib", - "//test/core/util:gpr_test_util", ], ) @@ -70,6 +67,5 @@ grpc_cc_test( "//:gpr_base", "//:grpc", "//test/core/tsi/alts/crypt:alts_crypt_test_util", - "//test/core/util:gpr_test_util", ], ) diff --git a/test/core/tsi/alts/handshaker/BUILD b/test/core/tsi/alts/handshaker/BUILD index 3f1a681c1a..809742744c 100644 --- a/test/core/tsi/alts/handshaker/BUILD +++ b/test/core/tsi/alts/handshaker/BUILD @@ -37,7 +37,6 @@ grpc_cc_test( "//:tsi", "//:tsi_interface", "//:grpc", - "//test/core/util:gpr_test_util", ], ) @@ -48,7 +47,6 @@ grpc_cc_test( deps = [ ":alts_handshaker_service_api_test_lib", "//:grpc", - "//test/core/util:gpr_test_util", ], ) @@ -62,7 +60,6 @@ grpc_cc_test( "//:gpr_base", "//:grpc", "//:tsi", - "//test/core/util:gpr_test_util", ], ) @@ -74,7 +71,6 @@ grpc_cc_test( ":alts_handshaker_service_api_test_lib", "//:grpc", "//:tsi", - "//test/core/util:gpr_test_util", ], ) @@ -85,7 +81,6 @@ grpc_cc_test( deps = [ "//:alts_util", "//:grpc", - "//test/core/util:gpr_test_util", ], ) diff --git a/test/core/tsi/alts/zero_copy_frame_protector/BUILD b/test/core/tsi/alts/zero_copy_frame_protector/BUILD index a3b797327e..2b41dae043 100644 --- a/test/core/tsi/alts/zero_copy_frame_protector/BUILD +++ b/test/core/tsi/alts/zero_copy_frame_protector/BUILD @@ -28,7 +28,6 @@ grpc_cc_test( "//:grpc", "//:grpc_base_c", "//test/core/tsi/alts/crypt:alts_crypt_test_util", - "//test/core/util:gpr_test_util", ], ) @@ -41,7 +40,6 @@ grpc_cc_test( "//:gpr", "//:grpc", "//test/core/tsi/alts/crypt:alts_crypt_test_util", - "//test/core/util:gpr_test_util", ], ) @@ -55,6 +53,5 @@ grpc_cc_test( "//:grpc", "//:grpc_base_c", "//test/core/tsi/alts/crypt:alts_crypt_test_util", - "//test/core/util:gpr_test_util", ], ) diff --git a/test/core/util/BUILD b/test/core/util/BUILD index be3e204777..f52570cde5 100644 --- a/test/core/util/BUILD +++ b/test/core/util/BUILD @@ -32,11 +32,6 @@ grpc_cc_library( "test_config.h", ], deps = ["//:gpr"], - data = [ - "//tools:lsan_suppressions.txt", - "//tools:tsan_suppressions.txt", - "//tools:ubsan_suppressions.txt", - ], ) grpc_cc_library( diff --git a/test/cpp/client/BUILD b/test/cpp/client/BUILD index c03ea92d34..12825e88c2 100644 --- a/test/cpp/client/BUILD +++ b/test/cpp/client/BUILD @@ -28,7 +28,6 @@ grpc_cc_test( "//:gpr", "//:grpc", "//:grpc++", - "//test/core/util:gpr_test_util", ], ) diff --git a/test/cpp/codegen/golden_file_test.cc b/test/cpp/codegen/golden_file_test.cc index 7e4d15a7c9..bfd3649494 100644 --- a/test/cpp/codegen/golden_file_test.cc +++ b/test/cpp/codegen/golden_file_test.cc @@ -37,8 +37,8 @@ DEFINE_string( const char kGoldenFilePath[] = "test/cpp/codegen/compiler_test_golden"; const char kMockGoldenFilePath[] = "test/cpp/codegen/compiler_test_mock_golden"; -void run_test(std::basic_string<char> generated_file, - std::basic_string<char> golden_file) { +void run_test(const std::basic_string<char>& generated_file, + const std::basic_string<char>& golden_file) { std::ifstream generated(generated_file); std::ifstream golden(golden_file); diff --git a/test/cpp/end2end/async_end2end_test.cc b/test/cpp/end2end/async_end2end_test.cc index e8d2325b7d..3d31c9d810 100644 --- a/test/cpp/end2end/async_end2end_test.cc +++ b/test/cpp/end2end/async_end2end_test.cc @@ -142,7 +142,7 @@ class Verifier { // to call the lambda void Verify(CompletionQueue* cq, std::chrono::system_clock::time_point deadline, - std::function<void(void)> lambda) { + const std::function<void(void)>& lambda) { if (expectations_.empty()) { bool ok; void* got_tag; @@ -891,7 +891,7 @@ TEST_P(AsyncEnd2endTest, ClientInitialMetadataRpc) { cq_.get(), tag(2)); Verifier().Expect(2, true).Verify(cq_.get()); EXPECT_EQ(send_request.message(), recv_request.message()); - auto client_initial_metadata = srv_ctx.client_metadata(); + const auto& client_initial_metadata = srv_ctx.client_metadata(); EXPECT_EQ(meta1.second, ToString(client_initial_metadata.find(meta1.first)->second)); EXPECT_EQ(meta2.second, @@ -937,7 +937,7 @@ TEST_P(AsyncEnd2endTest, ServerInitialMetadataRpc) { srv_ctx.AddInitialMetadata(meta2.first, meta2.second); response_writer.SendInitialMetadata(tag(3)); Verifier().Expect(3, true).Expect(4, true).Verify(cq_.get()); - auto server_initial_metadata = cli_ctx.GetServerInitialMetadata(); + const auto& server_initial_metadata = cli_ctx.GetServerInitialMetadata(); EXPECT_EQ(meta1.second, ToString(server_initial_metadata.find(meta1.first)->second)); EXPECT_EQ(meta2.second, @@ -990,7 +990,7 @@ TEST_P(AsyncEnd2endTest, ServerTrailingMetadataRpc) { EXPECT_EQ(send_response.message(), recv_response.message()); EXPECT_TRUE(recv_status.ok()); - auto server_trailing_metadata = cli_ctx.GetServerTrailingMetadata(); + const auto& server_trailing_metadata = cli_ctx.GetServerTrailingMetadata(); EXPECT_EQ(meta1.second, ToString(server_trailing_metadata.find(meta1.first)->second)); EXPECT_EQ(meta2.second, @@ -1038,7 +1038,7 @@ TEST_P(AsyncEnd2endTest, MetadataRpc) { cq_.get(), tag(2)); Verifier().Expect(2, true).Verify(cq_.get()); EXPECT_EQ(send_request.message(), recv_request.message()); - auto client_initial_metadata = srv_ctx.client_metadata(); + const auto& client_initial_metadata = srv_ctx.client_metadata(); EXPECT_EQ(meta1.second, ToString(client_initial_metadata.find(meta1.first)->second)); EXPECT_EQ(meta2.second, @@ -1049,7 +1049,7 @@ TEST_P(AsyncEnd2endTest, MetadataRpc) { srv_ctx.AddInitialMetadata(meta4.first, meta4.second); response_writer.SendInitialMetadata(tag(3)); Verifier().Expect(3, true).Expect(4, true).Verify(cq_.get()); - auto server_initial_metadata = cli_ctx.GetServerInitialMetadata(); + const auto& server_initial_metadata = cli_ctx.GetServerInitialMetadata(); EXPECT_EQ(meta3.second, ToString(server_initial_metadata.find(meta3.first)->second)); EXPECT_EQ(meta4.second, @@ -1066,7 +1066,7 @@ TEST_P(AsyncEnd2endTest, MetadataRpc) { EXPECT_EQ(send_response.message(), recv_response.message()); EXPECT_TRUE(recv_status.ok()); - auto server_trailing_metadata = cli_ctx.GetServerTrailingMetadata(); + const auto& server_trailing_metadata = cli_ctx.GetServerTrailingMetadata(); EXPECT_EQ(meta5.second, ToString(server_trailing_metadata.find(meta5.first)->second)); EXPECT_EQ(meta6.second, @@ -1144,7 +1144,7 @@ TEST_P(AsyncEnd2endTest, ServerCheckDone) { TEST_P(AsyncEnd2endTest, UnimplementedRpc) { ChannelArguments args; - auto channel_creds = GetCredentialsProvider()->GetChannelCredentials( + const auto& channel_creds = GetCredentialsProvider()->GetChannelCredentials( GetParam().credentials_type, &args); std::shared_ptr<Channel> channel = !(GetParam().inproc) diff --git a/test/cpp/end2end/client_lb_end2end_test.cc b/test/cpp/end2end/client_lb_end2end_test.cc index eeec8e90bd..feea7c3907 100644 --- a/test/cpp/end2end/client_lb_end2end_test.cc +++ b/test/cpp/end2end/client_lb_end2end_test.cc @@ -686,9 +686,11 @@ TEST_F(ClientLbEnd2endTest, RoundRobinReresolve) { const int kNumServers = 3; std::vector<int> first_ports; std::vector<int> second_ports; + first_ports.reserve(kNumServers); for (int i = 0; i < kNumServers; ++i) { first_ports.push_back(grpc_pick_unused_port_or_die()); } + second_ports.reserve(kNumServers); for (int i = 0; i < kNumServers; ++i) { second_ports.push_back(grpc_pick_unused_port_or_die()); } diff --git a/test/cpp/end2end/end2end_test.cc b/test/cpp/end2end/end2end_test.cc index 3c1d48cc1e..fc07681535 100644 --- a/test/cpp/end2end/end2end_test.cc +++ b/test/cpp/end2end/end2end_test.cc @@ -71,8 +71,8 @@ class TestMetadataCredentialsPlugin : public MetadataCredentialsPlugin { static const char kGoodMetadataKey[]; static const char kBadMetadataKey[]; - TestMetadataCredentialsPlugin(grpc::string_ref metadata_key, - grpc::string_ref metadata_value, + TestMetadataCredentialsPlugin(const grpc::string_ref& metadata_key, + const grpc::string_ref& metadata_value, bool is_blocking, bool is_successful) : metadata_key_(metadata_key.data(), metadata_key.length()), metadata_value_(metadata_value.data(), metadata_value.length()), @@ -168,7 +168,7 @@ const char TestAuthMetadataProcessor::kIdentityPropName[] = "novel identity"; class Proxy : public ::grpc::testing::EchoTestService::Service { public: - Proxy(std::shared_ptr<Channel> channel) + Proxy(const std::shared_ptr<Channel>& channel) : stub_(grpc::testing::EchoTestService::NewStub(channel)) {} Status Echo(ServerContext* server_context, const EchoRequest* request, @@ -683,6 +683,7 @@ TEST_P(End2endTest, SimpleRpcWithCustomUserAgentPrefix) { TEST_P(End2endTest, MultipleRpcsWithVariedBinaryMetadataValue) { ResetStub(); std::vector<std::thread> threads; + threads.reserve(10); for (int i = 0; i < 10; ++i) { threads.emplace_back(SendRpc, stub_.get(), 10, true); } @@ -694,6 +695,7 @@ TEST_P(End2endTest, MultipleRpcsWithVariedBinaryMetadataValue) { TEST_P(End2endTest, MultipleRpcs) { ResetStub(); std::vector<std::thread> threads; + threads.reserve(10); for (int i = 0; i < 10; ++i) { threads.emplace_back(SendRpc, stub_.get(), 10, false); } @@ -1272,6 +1274,7 @@ TEST_P(ProxyEnd2endTest, SimpleRpcWithEmptyMessages) { TEST_P(ProxyEnd2endTest, MultipleRpcs) { ResetStub(); std::vector<std::thread> threads; + threads.reserve(10); for (int i = 0; i < 10; ++i) { threads.emplace_back(SendRpc, stub_.get(), 10, false); } diff --git a/test/cpp/end2end/mock_test.cc b/test/cpp/end2end/mock_test.cc index ff49902fea..ba3122c895 100644 --- a/test/cpp/end2end/mock_test.cc +++ b/test/cpp/end2end/mock_test.cc @@ -186,7 +186,7 @@ class TestServiceImpl : public EchoTestService::Service { ServerWriter<EchoResponse>* writer) override { EchoResponse response; vector<grpc::string> tokens = split(request->message()); - for (grpc::string token : tokens) { + for (const grpc::string& token : tokens) { response.set_message(token); writer->Write(response); } diff --git a/test/cpp/end2end/thread_stress_test.cc b/test/cpp/end2end/thread_stress_test.cc index e709a25356..ccf8400a87 100644 --- a/test/cpp/end2end/thread_stress_test.cc +++ b/test/cpp/end2end/thread_stress_test.cc @@ -322,6 +322,7 @@ TYPED_TEST_CASE(End2endTest, CommonTypes); TYPED_TEST(End2endTest, ThreadStress) { this->common_.ResetStub(); std::vector<std::thread> threads; + threads.reserve(kNumThreads); for (int i = 0; i < kNumThreads; ++i) { threads.emplace_back(SendRpc, this->common_.GetStub(), kNumRpcs); } diff --git a/test/cpp/interop/http2_client.cc b/test/cpp/interop/http2_client.cc index 543f159265..bc7f0f5edb 100644 --- a/test/cpp/interop/http2_client.cc +++ b/test/cpp/interop/http2_client.cc @@ -42,16 +42,16 @@ const int kLargeRequestSize = 271828; const int kLargeResponseSize = 314159; } // namespace -Http2Client::ServiceStub::ServiceStub(std::shared_ptr<Channel> channel) - : channel_(channel) { +Http2Client::ServiceStub::ServiceStub(const std::shared_ptr<Channel>& channel) + : channel_(std::move(channel)) { stub_ = TestService::NewStub(channel); } TestService::Stub* Http2Client::ServiceStub::Get() { return stub_.get(); } -Http2Client::Http2Client(std::shared_ptr<Channel> channel) +Http2Client::Http2Client(const std::shared_ptr<Channel>& channel) : serviceStub_(channel), - channel_(channel), + channel_(std::move(channel)), defaultRequest_(BuildDefaultRequest()) {} bool Http2Client::AssertStatusCode(const Status& s, StatusCode expected_code) { @@ -140,7 +140,8 @@ bool Http2Client::DoPing() { return true; } -void Http2Client::MaxStreamsWorker(std::shared_ptr<grpc::Channel> channel) { +void Http2Client::MaxStreamsWorker( + const std::shared_ptr<grpc::Channel>& channel) { SimpleResponse response; AssertStatusCode(SendUnaryCall(&response), grpc::StatusCode::OK); GPR_ASSERT(response.payload().body() == diff --git a/test/cpp/interop/http2_client.h b/test/cpp/interop/http2_client.h index 2bcfdd69db..269d3b32e2 100644 --- a/test/cpp/interop/http2_client.h +++ b/test/cpp/interop/http2_client.h @@ -31,7 +31,7 @@ namespace testing { class Http2Client { public: - explicit Http2Client(std::shared_ptr<Channel> channel); + explicit Http2Client(const std::shared_ptr<Channel>& channel); ~Http2Client() {} bool DoRstAfterHeader(); @@ -44,7 +44,7 @@ class Http2Client { private: class ServiceStub { public: - ServiceStub(std::shared_ptr<Channel> channel); + ServiceStub(const std::shared_ptr<Channel>& channel); TestService::Stub* Get(); @@ -53,7 +53,7 @@ class Http2Client { std::shared_ptr<Channel> channel_; }; - void MaxStreamsWorker(std::shared_ptr<grpc::Channel> channel); + void MaxStreamsWorker(const std::shared_ptr<grpc::Channel>& channel); bool AssertStatusCode(const Status& s, StatusCode expected_code); Status SendUnaryCall(SimpleResponse* response); SimpleRequest BuildDefaultRequest(); diff --git a/test/cpp/interop/interop_client.cc b/test/cpp/interop/interop_client.cc index 68bf1e6dc7..fce99a1697 100644 --- a/test/cpp/interop/interop_client.cc +++ b/test/cpp/interop/interop_client.cc @@ -19,6 +19,7 @@ #include <cinttypes> #include <fstream> #include <memory> +#include <utility> #include <grpc/grpc.h> #include <grpc/support/alloc.h> @@ -73,7 +74,7 @@ void UnaryCompressionChecks(const InteropClientContextInspector& inspector, } } // namespace -InteropClient::ServiceStub::ServiceStub(std::shared_ptr<Channel> channel, +InteropClient::ServiceStub::ServiceStub(const std::shared_ptr<Channel>& channel, bool new_stub_every_call) : channel_(channel), new_stub_every_call_(new_stub_every_call) { // If new_stub_every_call is false, then this is our chance to initialize @@ -99,7 +100,8 @@ InteropClient::ServiceStub::GetUnimplementedServiceStub() { return unimplemented_service_stub_.get(); } -void InteropClient::ServiceStub::Reset(std::shared_ptr<Channel> channel) { +void InteropClient::ServiceStub::Reset( + const std::shared_ptr<Channel>& channel) { channel_ = channel; // Update stub_ as well. Note: If new_stub_every_call_ is true, we can reset @@ -111,14 +113,14 @@ void InteropClient::ServiceStub::Reset(std::shared_ptr<Channel> channel) { } } -void InteropClient::Reset(std::shared_ptr<Channel> channel) { - serviceStub_.Reset(channel); +void InteropClient::Reset(const std::shared_ptr<Channel>& channel) { + serviceStub_.Reset(std::move(channel)); } -InteropClient::InteropClient(std::shared_ptr<Channel> channel, +InteropClient::InteropClient(const std::shared_ptr<Channel>& channel, bool new_stub_every_test_case, bool do_not_abort_on_transient_failures) - : serviceStub_(channel, new_stub_every_test_case), + : serviceStub_(std::move(channel), new_stub_every_test_case), do_not_abort_on_transient_failures_(do_not_abort_on_transient_failures) {} bool InteropClient::AssertStatusOk(const Status& s, @@ -180,7 +182,7 @@ bool InteropClient::PerformLargeUnary(SimpleRequest* request, bool InteropClient::PerformLargeUnary(SimpleRequest* request, SimpleResponse* response, - CheckerFn custom_checks_fn) { + const CheckerFn& custom_checks_fn) { ClientContext context; InteropClientContextInspector inspector(context); request->set_response_size(kLargeResponseSize); diff --git a/test/cpp/interop/interop_client.h b/test/cpp/interop/interop_client.h index 79ff24fc47..480eb3f4b6 100644 --- a/test/cpp/interop/interop_client.h +++ b/test/cpp/interop/interop_client.h @@ -40,12 +40,12 @@ class InteropClient { /// created for every test case /// If do_not_abort_on_transient_failures is true, abort() is not called in /// case of transient failures (like connection failures) - explicit InteropClient(std::shared_ptr<Channel> channel, + explicit InteropClient(const std::shared_ptr<Channel>& channel, bool new_stub_every_test_case, bool do_not_abort_on_transient_failures); ~InteropClient() {} - void Reset(std::shared_ptr<Channel> channel); + void Reset(const std::shared_ptr<Channel>& channel); bool DoEmpty(); bool DoLargeUnary(); @@ -83,12 +83,13 @@ class InteropClient { public: // If new_stub_every_call = true, pointer to a new instance of // TestServce::Stub is returned by Get() everytime it is called - ServiceStub(std::shared_ptr<Channel> channel, bool new_stub_every_call); + ServiceStub(const std::shared_ptr<Channel>& channel, + bool new_stub_every_call); TestService::Stub* Get(); UnimplementedService::Stub* GetUnimplementedServiceStub(); - void Reset(std::shared_ptr<Channel> channel); + void Reset(const std::shared_ptr<Channel>& channel); private: std::unique_ptr<TestService::Stub> stub_; @@ -102,7 +103,7 @@ class InteropClient { /// Run \a custom_check_fn as an additional check. bool PerformLargeUnary(SimpleRequest* request, SimpleResponse* response, - CheckerFn custom_checks_fn); + const CheckerFn& custom_checks_fn); bool AssertStatusOk(const Status& s, const grpc::string& optional_debug_string); bool AssertStatusCode(const Status& s, StatusCode expected_code, diff --git a/test/cpp/interop/interop_server.cc b/test/cpp/interop/interop_server.cc index f55d624b21..6570bbf969 100644 --- a/test/cpp/interop/interop_server.cc +++ b/test/cpp/interop/interop_server.cc @@ -317,25 +317,25 @@ class TestServiceImpl : public TestService::Service { }; void grpc::testing::interop::RunServer( - std::shared_ptr<ServerCredentials> creds) { + const std::shared_ptr<ServerCredentials>& creds) { RunServer(creds, FLAGS_port, nullptr, nullptr); } void grpc::testing::interop::RunServer( - std::shared_ptr<ServerCredentials> creds, + const std::shared_ptr<ServerCredentials>& creds, std::unique_ptr<std::vector<std::unique_ptr<ServerBuilderOption>>> server_options) { RunServer(creds, FLAGS_port, nullptr, std::move(server_options)); } void grpc::testing::interop::RunServer( - std::shared_ptr<ServerCredentials> creds, const int port, + const std::shared_ptr<ServerCredentials>& creds, const int port, ServerStartedCondition* server_started_condition) { RunServer(creds, port, server_started_condition, nullptr); } void grpc::testing::interop::RunServer( - std::shared_ptr<ServerCredentials> creds, const int port, + const std::shared_ptr<ServerCredentials>& creds, const int port, ServerStartedCondition* server_started_condition, std::unique_ptr<std::vector<std::unique_ptr<ServerBuilderOption>>> server_options) { diff --git a/test/cpp/interop/server_helper.h b/test/cpp/interop/server_helper.h index 265874df70..1bfbf8e474 100644 --- a/test/cpp/interop/server_helper.h +++ b/test/cpp/interop/server_helper.h @@ -63,7 +63,7 @@ struct ServerStartedCondition { /// Run gRPC interop server using port FLAGS_port. /// /// \param creds The credentials associated with the server. -void RunServer(std::shared_ptr<ServerCredentials> creds); +void RunServer(const std::shared_ptr<ServerCredentials>& creds); /// Run gRPC interop server. /// @@ -71,7 +71,7 @@ void RunServer(std::shared_ptr<ServerCredentials> creds); /// \param port Port to use for the server. /// \param server_started_condition (optional) Struct holding mutex, condition /// variable, and condition used to notify when the server has started. -void RunServer(std::shared_ptr<ServerCredentials> creds, int port, +void RunServer(const std::shared_ptr<ServerCredentials>& creds, int port, ServerStartedCondition* server_started_condition); /// Run gRPC interop server. @@ -79,7 +79,7 @@ void RunServer(std::shared_ptr<ServerCredentials> creds, int port, /// \param creds The credentials associated with the server. /// \param server_options List of options to set when building the server. void RunServer( - std::shared_ptr<ServerCredentials> creds, + const std::shared_ptr<ServerCredentials>& creds, std::unique_ptr<std::vector<std::unique_ptr<ServerBuilderOption>>> server_options); @@ -91,7 +91,7 @@ void RunServer( /// \param server_started_condition (optional) Struct holding mutex, condition // variable, and condition used to notify when the server has started. void RunServer( - std::shared_ptr<ServerCredentials> creds, const int port, + const std::shared_ptr<ServerCredentials>& creds, const int port, ServerStartedCondition* server_started_condition, std::unique_ptr<std::vector<std::unique_ptr<grpc::ServerBuilderOption>>> server_options); diff --git a/test/cpp/interop/stress_interop_client.cc b/test/cpp/interop/stress_interop_client.cc index 30a8351cfe..9d373c3cd9 100644 --- a/test/cpp/interop/stress_interop_client.cc +++ b/test/cpp/interop/stress_interop_client.cc @@ -68,7 +68,7 @@ TestCaseType WeightedRandomTestSelector::GetNextTest() const { StressTestInteropClient::StressTestInteropClient( int test_id, const grpc::string& server_address, - std::shared_ptr<Channel> channel, + const std::shared_ptr<Channel>& channel, const WeightedRandomTestSelector& test_selector, long test_duration_secs, long sleep_duration_ms, bool do_not_abort_on_transient_failures) : test_id_(test_id), @@ -80,7 +80,8 @@ StressTestInteropClient::StressTestInteropClient( test_duration_secs_(test_duration_secs), sleep_duration_ms_(sleep_duration_ms) {} -void StressTestInteropClient::MainLoop(std::shared_ptr<QpsGauge> qps_gauge) { +void StressTestInteropClient::MainLoop( + const std::shared_ptr<QpsGauge>& qps_gauge) { gpr_log(GPR_INFO, "Running test %d. ServerAddr: %s", test_id_, server_address_.c_str()); diff --git a/test/cpp/interop/stress_interop_client.h b/test/cpp/interop/stress_interop_client.h index a306dc3565..e4fa7d0973 100644 --- a/test/cpp/interop/stress_interop_client.h +++ b/test/cpp/interop/stress_interop_client.h @@ -91,14 +91,14 @@ class WeightedRandomTestSelector { class StressTestInteropClient { public: StressTestInteropClient(int test_id, const grpc::string& server_address, - std::shared_ptr<Channel> channel, + const std::shared_ptr<Channel>& channel, const WeightedRandomTestSelector& test_selector, long test_duration_secs, long sleep_duration_ms, bool do_not_abort_on_transient_failures); // The main function. Use this as the thread entry point. // qps_gauge is the QpsGauge to record the requests per second metric - void MainLoop(std::shared_ptr<QpsGauge> qps_gauge); + void MainLoop(const std::shared_ptr<QpsGauge>& qps_gauge); private: bool RunTest(TestCaseType test_case); diff --git a/test/cpp/microbenchmarks/BUILD b/test/cpp/microbenchmarks/BUILD index d51a0e3dc5..5dcfd94ed3 100644 --- a/test/cpp/microbenchmarks/BUILD +++ b/test/cpp/microbenchmarks/BUILD @@ -24,7 +24,6 @@ grpc_cc_test( external_deps = [ "benchmark", ], - deps = ["//test/core/util:gpr_test_util",] ) grpc_cc_library( diff --git a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc index 823c76f755..6fcf048bf3 100644 --- a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc +++ b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc @@ -208,6 +208,7 @@ class SingleInternedBinaryElem { private: static grpc_slice MakeBytes() { std::vector<char> v; + v.reserve(kLength); for (int i = 0; i < kLength; i++) { v.push_back(static_cast<char>(rand())); } @@ -246,6 +247,7 @@ class SingleNonInternedBinaryElem { private: static grpc_slice MakeBytes() { std::vector<char> v; + v.reserve(kLength); for (int i = 0; i < kLength; i++) { v.push_back(static_cast<char>(rand())); } diff --git a/test/cpp/naming/resolver_component_test.cc b/test/cpp/naming/resolver_component_test.cc index 07ddfd30ee..6ac548120c 100644 --- a/test/cpp/naming/resolver_component_test.cc +++ b/test/cpp/naming/resolver_component_test.cc @@ -90,7 +90,7 @@ namespace { class GrpcLBAddress final { public: GrpcLBAddress(std::string address, bool is_balancer) - : is_balancer(is_balancer), address(address) {} + : is_balancer(is_balancer), address(std::move(address)) {} bool operator==(const GrpcLBAddress& other) const { return this->is_balancer == other.is_balancer && @@ -109,7 +109,7 @@ vector<GrpcLBAddress> ParseExpectedAddrs(std::string expected_addrs) { std::vector<GrpcLBAddress> out; while (expected_addrs.size() != 0) { // get the next <ip>,<port> (v4 or v6) - size_t next_comma = expected_addrs.find(","); + size_t next_comma = expected_addrs.find(','); if (next_comma == std::string::npos) { gpr_log(GPR_ERROR, "Missing ','. Expected_addrs arg should be a semicolon-separated " @@ -120,7 +120,7 @@ vector<GrpcLBAddress> ParseExpectedAddrs(std::string expected_addrs) { std::string next_addr = expected_addrs.substr(0, next_comma); expected_addrs = expected_addrs.substr(next_comma + 1, std::string::npos); // get the next is_balancer 'bool' associated with this address - size_t next_semicolon = expected_addrs.find(";"); + size_t next_semicolon = expected_addrs.find(';'); bool is_balancer = gpr_is_true(expected_addrs.substr(0, next_semicolon).c_str()); out.emplace_back(GrpcLBAddress(next_addr, is_balancer)); diff --git a/test/cpp/naming/resolver_component_tests_runner_invoker.cc b/test/cpp/naming/resolver_component_tests_runner_invoker.cc index 45c1029caa..68be00a67d 100644 --- a/test/cpp/naming/resolver_component_tests_runner_invoker.cc +++ b/test/cpp/naming/resolver_component_tests_runner_invoker.cc @@ -99,21 +99,21 @@ namespace grpc { namespace testing { -void InvokeResolverComponentTestsRunner(std::string test_runner_bin_path, - std::string test_bin_path, - std::string dns_server_bin_path, - std::string records_config_path, - std::string dns_resolver_bin_path, - std::string tcp_connect_bin_path) { +void InvokeResolverComponentTestsRunner( + std::string test_runner_bin_path, const std::string& test_bin_path, + const std::string& dns_server_bin_path, + const std::string& records_config_path, + const std::string& dns_resolver_bin_path, + const std::string& tcp_connect_bin_path) { int dns_server_port = grpc_pick_unused_port_or_die(); - SubProcess* test_driver = - new SubProcess({test_runner_bin_path, "--test_bin_path=" + test_bin_path, - "--dns_server_bin_path=" + dns_server_bin_path, - "--records_config_path=" + records_config_path, - "--dns_server_port=" + std::to_string(dns_server_port), - "--dns_resolver_bin_path=" + dns_resolver_bin_path, - "--tcp_connect_bin_path=" + tcp_connect_bin_path}); + SubProcess* test_driver = new SubProcess( + {std::move(test_runner_bin_path), "--test_bin_path=" + test_bin_path, + "--dns_server_bin_path=" + dns_server_bin_path, + "--records_config_path=" + records_config_path, + "--dns_server_port=" + std::to_string(dns_server_port), + "--dns_resolver_bin_path=" + dns_resolver_bin_path, + "--tcp_connect_bin_path=" + tcp_connect_bin_path}); gpr_mu test_driver_mu; gpr_mu_init(&test_driver_mu); gpr_cv test_driver_cv; diff --git a/test/cpp/qps/client.h b/test/cpp/qps/client.h index 31ae6ca1fb..9d58ea8882 100644 --- a/test/cpp/qps/client.h +++ b/test/cpp/qps/client.h @@ -450,7 +450,7 @@ class ClientImpl : public Client { private: void set_channel_args(const ClientConfig& config, ChannelArguments* args) { - for (auto channel_arg : config.channel_args()) { + for (const auto& channel_arg : config.channel_args()) { if (channel_arg.value_case() == ChannelArg::kStrValue) { args->SetString(channel_arg.name(), channel_arg.str_value()); } else if (channel_arg.value_case() == ChannelArg::kIntValue) { diff --git a/test/cpp/qps/client_async.cc b/test/cpp/qps/client_async.cc index c79a10d1b4..bad24cf04a 100644 --- a/test/cpp/qps/client_async.cc +++ b/test/cpp/qps/client_async.cc @@ -24,6 +24,7 @@ #include <sstream> #include <string> #include <thread> +#include <utility> #include <vector> #include <grpc/grpc.h> @@ -78,7 +79,7 @@ class ClientRpcContextUnaryImpl : public ClientRpcContext { response_(), next_state_(State::READY), callback_(on_done), - next_issue_(next_issue), + next_issue_(std::move(next_issue)), prepare_req_(prepare_req) {} ~ClientRpcContextUnaryImpl() override {} void Start(CompletionQueue* cq, const ClientConfig& config) override { @@ -298,7 +299,7 @@ class AsyncClient : public ClientImpl<StubType, RequestType> { }; static std::unique_ptr<BenchmarkService::Stub> BenchmarkStubCreator( - std::shared_ptr<Channel> ch) { + const std::shared_ptr<Channel>& ch) { return BenchmarkService::NewStub(ch); } @@ -313,7 +314,7 @@ class AsyncUnaryClient final ~AsyncUnaryClient() override {} private: - static void CheckDone(grpc::Status s, SimpleResponse* response, + static void CheckDone(const grpc::Status& s, SimpleResponse* response, HistogramEntry* entry) { entry->set_status(s.error_code()); } @@ -326,7 +327,7 @@ class AsyncUnaryClient final std::function<gpr_timespec()> next_issue, const SimpleRequest& req) { return new ClientRpcContextUnaryImpl<SimpleRequest, SimpleResponse>( - stub, req, next_issue, AsyncUnaryClient::PrepareReq, + stub, req, std::move(next_issue), AsyncUnaryClient::PrepareReq, AsyncUnaryClient::CheckDone); } }; @@ -349,7 +350,7 @@ class ClientRpcContextStreamingPingPongImpl : public ClientRpcContext { response_(), next_state_(State::INVALID), callback_(on_done), - next_issue_(next_issue), + next_issue_(std::move(next_issue)), prepare_req_(prepare_req), coalesce_(false) {} ~ClientRpcContextStreamingPingPongImpl() override {} @@ -497,7 +498,7 @@ class AsyncStreamingPingPongClient final ~AsyncStreamingPingPongClient() override {} private: - static void CheckDone(grpc::Status s, SimpleResponse* response) {} + static void CheckDone(const grpc::Status& s, SimpleResponse* response) {} static std::unique_ptr< grpc::ClientAsyncReaderWriter<SimpleRequest, SimpleResponse>> PrepareReq(BenchmarkService::Stub* stub, grpc::ClientContext* ctx, @@ -510,7 +511,8 @@ class AsyncStreamingPingPongClient final const SimpleRequest& req) { return new ClientRpcContextStreamingPingPongImpl<SimpleRequest, SimpleResponse>( - stub, req, next_issue, AsyncStreamingPingPongClient::PrepareReq, + stub, req, std::move(next_issue), + AsyncStreamingPingPongClient::PrepareReq, AsyncStreamingPingPongClient::CheckDone); } }; @@ -533,7 +535,7 @@ class ClientRpcContextStreamingFromClientImpl : public ClientRpcContext { response_(), next_state_(State::INVALID), callback_(on_done), - next_issue_(next_issue), + next_issue_(std::move(next_issue)), prepare_req_(prepare_req) {} ~ClientRpcContextStreamingFromClientImpl() override {} void Start(CompletionQueue* cq, const ClientConfig& config) override { @@ -628,7 +630,7 @@ class AsyncStreamingFromClientClient final ~AsyncStreamingFromClientClient() override {} private: - static void CheckDone(grpc::Status s, SimpleResponse* response) {} + static void CheckDone(const grpc::Status& s, SimpleResponse* response) {} static std::unique_ptr<grpc::ClientAsyncWriter<SimpleRequest>> PrepareReq( BenchmarkService::Stub* stub, grpc::ClientContext* ctx, SimpleResponse* resp, CompletionQueue* cq) { @@ -640,7 +642,8 @@ class AsyncStreamingFromClientClient final const SimpleRequest& req) { return new ClientRpcContextStreamingFromClientImpl<SimpleRequest, SimpleResponse>( - stub, req, next_issue, AsyncStreamingFromClientClient::PrepareReq, + stub, req, std::move(next_issue), + AsyncStreamingFromClientClient::PrepareReq, AsyncStreamingFromClientClient::CheckDone); } }; @@ -663,7 +666,7 @@ class ClientRpcContextStreamingFromServerImpl : public ClientRpcContext { response_(), next_state_(State::INVALID), callback_(on_done), - next_issue_(next_issue), + next_issue_(std::move(next_issue)), prepare_req_(prepare_req) {} ~ClientRpcContextStreamingFromServerImpl() override {} void Start(CompletionQueue* cq, const ClientConfig& config) override { @@ -742,7 +745,7 @@ class AsyncStreamingFromServerClient final ~AsyncStreamingFromServerClient() override {} private: - static void CheckDone(grpc::Status s, SimpleResponse* response) {} + static void CheckDone(const grpc::Status& s, SimpleResponse* response) {} static std::unique_ptr<grpc::ClientAsyncReader<SimpleResponse>> PrepareReq( BenchmarkService::Stub* stub, grpc::ClientContext* ctx, const SimpleRequest& req, CompletionQueue* cq) { @@ -754,7 +757,8 @@ class AsyncStreamingFromServerClient final const SimpleRequest& req) { return new ClientRpcContextStreamingFromServerImpl<SimpleRequest, SimpleResponse>( - stub, req, next_issue, AsyncStreamingFromServerClient::PrepareReq, + stub, req, std::move(next_issue), + AsyncStreamingFromServerClient::PrepareReq, AsyncStreamingFromServerClient::CheckDone); } }; @@ -775,9 +779,9 @@ class ClientRpcContextGenericStreamingImpl : public ClientRpcContext { req_(req), response_(), next_state_(State::INVALID), - callback_(on_done), - next_issue_(next_issue), - prepare_req_(prepare_req) {} + callback_(std::move(on_done)), + next_issue_(std::move(next_issue)), + prepare_req_(std::move(prepare_req)) {} ~ClientRpcContextGenericStreamingImpl() override {} void Start(CompletionQueue* cq, const ClientConfig& config) override { GPR_ASSERT(!config.use_coalesce_api()); // not supported yet. @@ -891,7 +895,7 @@ class ClientRpcContextGenericStreamingImpl : public ClientRpcContext { }; static std::unique_ptr<grpc::GenericStub> GenericStubCreator( - std::shared_ptr<Channel> ch) { + const std::shared_ptr<Channel>& ch) { return std::unique_ptr<grpc::GenericStub>(new grpc::GenericStub(ch)); } @@ -907,7 +911,7 @@ class GenericAsyncStreamingClient final ~GenericAsyncStreamingClient() override {} private: - static void CheckDone(grpc::Status s, ByteBuffer* response) {} + static void CheckDone(const grpc::Status& s, ByteBuffer* response) {} static std::unique_ptr<grpc::GenericClientAsyncReaderWriter> PrepareReq( grpc::GenericStub* stub, grpc::ClientContext* ctx, const grpc::string& method_name, CompletionQueue* cq) { @@ -918,7 +922,8 @@ class GenericAsyncStreamingClient final std::function<gpr_timespec()> next_issue, const ByteBuffer& req) { return new ClientRpcContextGenericStreamingImpl( - stub, req, next_issue, GenericAsyncStreamingClient::PrepareReq, + stub, req, std::move(next_issue), + GenericAsyncStreamingClient::PrepareReq, GenericAsyncStreamingClient::CheckDone); } }; diff --git a/test/cpp/qps/client_sync.cc b/test/cpp/qps/client_sync.cc index e65e3b43f3..668d9abf5c 100644 --- a/test/cpp/qps/client_sync.cc +++ b/test/cpp/qps/client_sync.cc @@ -44,7 +44,7 @@ namespace grpc { namespace testing { static std::unique_ptr<BenchmarkService::Stub> BenchmarkStubCreator( - std::shared_ptr<Channel> ch) { + const std::shared_ptr<Channel>& ch) { return BenchmarkService::NewStub(ch); } @@ -192,7 +192,7 @@ class SynchronousStreamingClient : public SynchronousClient { new (&context_[thread_idx]) ClientContext(); } - void CleanupAllStreams(std::function<void(size_t)> cleaner) { + void CleanupAllStreams(const std::function<void(size_t)>& cleaner) { std::vector<std::thread> cleanup_threads; for (size_t i = 0; i < num_threads_; i++) { cleanup_threads.emplace_back([this, i, cleaner] { diff --git a/test/cpp/qps/driver.cc b/test/cpp/qps/driver.cc index 34f1291576..cabbd51843 100644 --- a/test/cpp/qps/driver.cc +++ b/test/cpp/qps/driver.cc @@ -96,16 +96,20 @@ static deque<string> get_workers(const string& env_name) { } // helpers for postprocess_scenario_result -static double WallTime(ClientStats s) { return s.time_elapsed(); } -static double SystemTime(ClientStats s) { return s.time_system(); } -static double UserTime(ClientStats s) { return s.time_user(); } -static double CliPollCount(ClientStats s) { return s.cq_poll_count(); } -static double SvrPollCount(ServerStats s) { return s.cq_poll_count(); } -static double ServerWallTime(ServerStats s) { return s.time_elapsed(); } -static double ServerSystemTime(ServerStats s) { return s.time_system(); } -static double ServerUserTime(ServerStats s) { return s.time_user(); } -static double ServerTotalCpuTime(ServerStats s) { return s.total_cpu_time(); } -static double ServerIdleCpuTime(ServerStats s) { return s.idle_cpu_time(); } +static double WallTime(const ClientStats& s) { return s.time_elapsed(); } +static double SystemTime(const ClientStats& s) { return s.time_system(); } +static double UserTime(const ClientStats& s) { return s.time_user(); } +static double CliPollCount(const ClientStats& s) { return s.cq_poll_count(); } +static double SvrPollCount(const ServerStats& s) { return s.cq_poll_count(); } +static double ServerWallTime(const ServerStats& s) { return s.time_elapsed(); } +static double ServerSystemTime(const ServerStats& s) { return s.time_system(); } +static double ServerUserTime(const ServerStats& s) { return s.time_user(); } +static double ServerTotalCpuTime(const ServerStats& s) { + return s.total_cpu_time(); +} +static double ServerIdleCpuTime(const ServerStats& s) { + return s.idle_cpu_time(); +} static int Cores(int n) { return n; } // Postprocess ScenarioResult and populate result summary. @@ -156,7 +160,7 @@ static void postprocess_scenario_result(ScenarioResult* result) { int64_t successes = 0; int64_t failures = 0; for (int i = 0; i < result->request_results_size(); i++) { - RequestResultCount rrc = result->request_results(i); + const RequestResultCount& rrc = result->request_results(i); if (rrc.status_code() == 0) { successes += rrc.count(); } else { @@ -213,7 +217,6 @@ std::unique_ptr<ScenarioResult> RunScenario( // To be added to the result, containing the final configuration used for // client and config (including host, etc.) ClientConfig result_client_config; - const ServerConfig result_server_config = initial_server_config; // Get client, server lists; ignore if inproc test auto workers = (!run_inproc) ? get_workers("QPS_WORKERS") : deque<string>(); @@ -280,7 +283,7 @@ std::unique_ptr<ScenarioResult> RunScenario( local_workers[i]->InProcessChannel(channel_args)); } - ServerConfig server_config = initial_server_config; + const ServerConfig& server_config = initial_server_config; if (server_config.core_limit() != 0) { gpr_log(GPR_ERROR, "server config core limit is set but ignored by driver"); diff --git a/test/cpp/qps/qps_interarrival_test.cc b/test/cpp/qps/qps_interarrival_test.cc index 625b7db426..2cc22e9985 100644 --- a/test/cpp/qps/qps_interarrival_test.cc +++ b/test/cpp/qps/qps_interarrival_test.cc @@ -28,7 +28,8 @@ using grpc::testing::InterarrivalTimer; using grpc::testing::RandomDistInterface; -static void RunTest(RandomDistInterface&& r, int threads, std::string title) { +static void RunTest(RandomDistInterface&& r, int threads, + const std::string& title) { InterarrivalTimer timer; timer.init(r, threads); grpc_histogram* h(grpc_histogram_create(0.01, 60e9)); diff --git a/test/cpp/qps/report.h b/test/cpp/qps/report.h index 8e62f4f449..b00b0a311f 100644 --- a/test/cpp/qps/report.h +++ b/test/cpp/qps/report.h @@ -129,7 +129,7 @@ class JsonReporter : public Reporter { class RpcReporter : public Reporter { public: - RpcReporter(const string& name, std::shared_ptr<grpc::Channel> channel) + RpcReporter(const string& name, const std::shared_ptr<grpc::Channel>& channel) : Reporter(name), stub_(ReportQpsScenarioService::NewStub(channel)) {} private: diff --git a/test/cpp/qps/server_sync.cc b/test/cpp/qps/server_sync.cc index 82a9186989..b8facf9b56 100644 --- a/test/cpp/qps/server_sync.cc +++ b/test/cpp/qps/server_sync.cc @@ -129,7 +129,7 @@ class BenchmarkServiceImpl final : public BenchmarkService::Service { template <class W> static Status ServerPush(ServerContext* context, W* stream, const SimpleResponse& response, - std::function<bool()> done) { + const std::function<bool()>& done) { while ((done == nullptr) || !done()) { // TODO(vjpai): Add potential for rate-pacing on this if (!stream->Write(response)) { diff --git a/test/cpp/server/load_reporter/load_data_store_test.cc b/test/cpp/server/load_reporter/load_data_store_test.cc index 8280dee6a4..aa37b7d6ba 100644 --- a/test/cpp/server/load_reporter/load_data_store_test.cc +++ b/test/cpp/server/load_reporter/load_data_store_test.cc @@ -50,8 +50,8 @@ class LoadDataStoreTest : public ::testing::Test { bool PerBalancerStoresContains( const LoadDataStore& load_data_store, const std::set<PerBalancerStore*>* per_balancer_stores, - const grpc::string hostname, const grpc::string lb_id, - const grpc::string load_key) { + const grpc::string& hostname, const grpc::string& lb_id, + const grpc::string& load_key) { auto original_per_balancer_store = load_data_store.FindPerBalancerStore(hostname, lb_id); EXPECT_NE(original_per_balancer_store, nullptr); @@ -155,7 +155,7 @@ TEST_F(LoadDataStoreTest, OrphanAssignmentIsSticky) { active_lb_ids.erase(orphaned_lb_id); // Find which LB is assigned the orphaned store. grpc::string assigned_lb_id = ""; - for (auto lb_id : active_lb_ids) { + for (const auto& lb_id : active_lb_ids) { if (PerBalancerStoresContains( load_data_store, load_data_store.GetAssignedStores(kHostname1, lb_id), kHostname1, @@ -169,7 +169,7 @@ TEST_F(LoadDataStoreTest, OrphanAssignmentIsSticky) { // orphaned_lb_id shouldn't change. for (size_t _ = 0; _ < 10; ++_) { grpc::string lb_id_to_close = ""; - for (auto lb_id : active_lb_ids) { + for (const auto& lb_id : active_lb_ids) { if (lb_id != assigned_lb_id) { lb_id_to_close = lb_id; break; @@ -187,7 +187,7 @@ TEST_F(LoadDataStoreTest, OrphanAssignmentIsSticky) { load_data_store.ReportStreamClosed(kHostname1, assigned_lb_id); active_lb_ids.erase(assigned_lb_id); size_t orphaned_lb_id_occurences = 0; - for (auto lb_id : active_lb_ids) { + for (const auto& lb_id : active_lb_ids) { if (PerBalancerStoresContains( load_data_store, load_data_store.GetAssignedStores(kHostname1, lb_id), kHostname1, diff --git a/test/cpp/util/.clang-tidy b/test/cpp/util/.clang-tidy new file mode 100644 index 0000000000..f951ee5fea --- /dev/null +++ b/test/cpp/util/.clang-tidy @@ -0,0 +1,6 @@ +--- +Checks: 'modernize-use-nullptr,google-build-namespaces,google-build-explicit-make-pair,readability-function-size,performance-*,-performance-unnecessary-copy-initialization' +WarningsAsErrors: 'modernize-use-nullptr,google-build-namespaces,google-build-explicit-make-pair,readability-function-size,performance-*,-performance-unnecessary-copy-initialization' +CheckOptions: + - key: readability-function-size.StatementThreshold + value: '450' diff --git a/test/cpp/util/BUILD b/test/cpp/util/BUILD index 9b42bb28b1..b1153d2df3 100644 --- a/test/cpp/util/BUILD +++ b/test/cpp/util/BUILD @@ -217,7 +217,6 @@ grpc_cc_test( ], deps = [ "//:grpc++", - "//test/core/util:gpr_test_util", ], ) @@ -261,7 +260,6 @@ grpc_cc_test( deps = [ "//:grpc++_error_details", "//src/proto/grpc/testing:echo_messages_proto", - "//test/core/util:gpr_test_util", ], ) diff --git a/test/cpp/util/byte_buffer_test.cc b/test/cpp/util/byte_buffer_test.cc index b48a53eed1..9bffbf7ac1 100644 --- a/test/cpp/util/byte_buffer_test.cc +++ b/test/cpp/util/byte_buffer_test.cc @@ -41,7 +41,7 @@ class ByteBufferTest : public ::testing::Test {}; TEST_F(ByteBufferTest, CopyCtor) { ByteBuffer buffer1; EXPECT_FALSE(buffer1.Valid()); - ByteBuffer buffer2 = buffer1; + const ByteBuffer& buffer2 = buffer1; EXPECT_FALSE(buffer2.Valid()); } diff --git a/test/cpp/util/cli_call.cc b/test/cpp/util/cli_call.cc index a3992ab278..c258bde908 100644 --- a/test/cpp/util/cli_call.cc +++ b/test/cpp/util/cli_call.cc @@ -19,6 +19,7 @@ #include "test/cpp/util/cli_call.h" #include <iostream> +#include <utility> #include <grpc/grpc.h> #include <grpc/slice.h> @@ -39,7 +40,7 @@ Status CliCall::Call(std::shared_ptr<grpc::Channel> channel, const OutgoingMetadataContainer& metadata, IncomingMetadataContainer* server_initial_metadata, IncomingMetadataContainer* server_trailing_metadata) { - CliCall call(channel, method, metadata); + CliCall call(std::move(channel), method, metadata); call.Write(request); call.WritesDone(); if (!call.Read(response, server_initial_metadata)) { @@ -48,7 +49,7 @@ Status CliCall::Call(std::shared_ptr<grpc::Channel> channel, return call.Finish(server_trailing_metadata); } -CliCall::CliCall(std::shared_ptr<grpc::Channel> channel, +CliCall::CliCall(const std::shared_ptr<grpc::Channel>& channel, const grpc::string& method, const OutgoingMetadataContainer& metadata) : stub_(new grpc::GenericStub(channel)) { diff --git a/test/cpp/util/cli_call.h b/test/cpp/util/cli_call.h index 51ffafd29f..3f279095a4 100644 --- a/test/cpp/util/cli_call.h +++ b/test/cpp/util/cli_call.h @@ -42,7 +42,8 @@ class CliCall final { typedef std::multimap<grpc::string_ref, grpc::string_ref> IncomingMetadataContainer; - CliCall(std::shared_ptr<grpc::Channel> channel, const grpc::string& method, + CliCall(const std::shared_ptr<grpc::Channel>& channel, + const grpc::string& method, const OutgoingMetadataContainer& metadata); ~CliCall(); diff --git a/test/cpp/util/proto_file_parser.cc b/test/cpp/util/proto_file_parser.cc index 3fc96f38ae..a530ed1ffc 100644 --- a/test/cpp/util/proto_file_parser.cc +++ b/test/cpp/util/proto_file_parser.cc @@ -63,7 +63,7 @@ class ErrorPrinter : public protobuf::compiler::MultiFileErrorCollector { ProtoFileParser* parser_; // not owned }; -ProtoFileParser::ProtoFileParser(std::shared_ptr<grpc::Channel> channel, +ProtoFileParser::ProtoFileParser(const std::shared_ptr<grpc::Channel>& channel, const grpc::string& proto_path, const grpc::string& protofiles) : has_error_(false), diff --git a/test/cpp/util/proto_file_parser.h b/test/cpp/util/proto_file_parser.h index 2236b59451..eb1d793c2b 100644 --- a/test/cpp/util/proto_file_parser.h +++ b/test/cpp/util/proto_file_parser.h @@ -36,7 +36,7 @@ class ProtoFileParser { // The parser will search proto files using the server reflection service // provided on the given channel. The given protofiles in a source tree rooted // from proto_path will also be searched. - ProtoFileParser(std::shared_ptr<grpc::Channel> channel, + ProtoFileParser(const std::shared_ptr<grpc::Channel>& channel, const grpc::string& proto_path, const grpc::string& protofiles); diff --git a/test/cpp/util/proto_reflection_descriptor_database.cc b/test/cpp/util/proto_reflection_descriptor_database.cc index 0adbf20ce6..119272ca42 100644 --- a/test/cpp/util/proto_reflection_descriptor_database.cc +++ b/test/cpp/util/proto_reflection_descriptor_database.cc @@ -35,7 +35,7 @@ ProtoReflectionDescriptorDatabase::ProtoReflectionDescriptorDatabase( : stub_(std::move(stub)) {} ProtoReflectionDescriptorDatabase::ProtoReflectionDescriptorDatabase( - std::shared_ptr<grpc::Channel> channel) + const std::shared_ptr<grpc::Channel>& channel) : stub_(ServerReflection::NewStub(channel)) {} ProtoReflectionDescriptorDatabase::~ProtoReflectionDescriptorDatabase() { @@ -79,7 +79,7 @@ bool ProtoReflectionDescriptorDatabase::FindFileByName( AddFileFromResponse(response.file_descriptor_response()); } else if (response.message_response_case() == ServerReflectionResponse::MessageResponseCase::kErrorResponse) { - const ErrorResponse error = response.error_response(); + const ErrorResponse& error = response.error_response(); if (error.error_code() == StatusCode::NOT_FOUND) { gpr_log(GPR_INFO, "NOT_FOUND from server for FindFileByName(%s)", filename.c_str()); @@ -126,7 +126,7 @@ bool ProtoReflectionDescriptorDatabase::FindFileContainingSymbol( AddFileFromResponse(response.file_descriptor_response()); } else if (response.message_response_case() == ServerReflectionResponse::MessageResponseCase::kErrorResponse) { - const ErrorResponse error = response.error_response(); + const ErrorResponse& error = response.error_response(); if (error.error_code() == StatusCode::NOT_FOUND) { missing_symbols_.insert(symbol_name); gpr_log(GPR_INFO, @@ -182,7 +182,7 @@ bool ProtoReflectionDescriptorDatabase::FindFileContainingExtension( AddFileFromResponse(response.file_descriptor_response()); } else if (response.message_response_case() == ServerReflectionResponse::MessageResponseCase::kErrorResponse) { - const ErrorResponse error = response.error_response(); + const ErrorResponse& error = response.error_response(); if (error.error_code() == StatusCode::NOT_FOUND) { if (missing_extensions_.find(containing_type) == missing_extensions_.end()) { @@ -238,7 +238,7 @@ bool ProtoReflectionDescriptorDatabase::FindAllExtensionNumbers( return true; } else if (response.message_response_case() == ServerReflectionResponse::MessageResponseCase::kErrorResponse) { - const ErrorResponse error = response.error_response(); + const ErrorResponse& error = response.error_response(); if (error.error_code() == StatusCode::NOT_FOUND) { gpr_log(GPR_INFO, "NOT_FOUND from server for FindAllExtensionNumbers(%s)", extendee_type.c_str()); @@ -265,14 +265,14 @@ bool ProtoReflectionDescriptorDatabase::GetServices( if (response.message_response_case() == ServerReflectionResponse::MessageResponseCase::kListServicesResponse) { - const ListServiceResponse ls_response = response.list_services_response(); + const ListServiceResponse& ls_response = response.list_services_response(); for (int i = 0; i < ls_response.service_size(); ++i) { (*output).push_back(ls_response.service(i).name()); } return true; } else if (response.message_response_case() == ServerReflectionResponse::MessageResponseCase::kErrorResponse) { - const ErrorResponse error = response.error_response(); + const ErrorResponse& error = response.error_response(); gpr_log(GPR_INFO, "Error on GetServices()\n\tError code: %d\n" "\tError Message: %s", diff --git a/test/cpp/util/proto_reflection_descriptor_database.h b/test/cpp/util/proto_reflection_descriptor_database.h index e4cf2f207e..46190b3217 100644 --- a/test/cpp/util/proto_reflection_descriptor_database.h +++ b/test/cpp/util/proto_reflection_descriptor_database.h @@ -38,7 +38,7 @@ class ProtoReflectionDescriptorDatabase : public protobuf::DescriptorDatabase { std::unique_ptr<reflection::v1alpha::ServerReflection::Stub> stub); explicit ProtoReflectionDescriptorDatabase( - std::shared_ptr<grpc::Channel> channel); + const std::shared_ptr<grpc::Channel>& channel); virtual ~ProtoReflectionDescriptorDatabase(); diff --git a/test/cpp/util/string_ref_test.cc b/test/cpp/util/string_ref_test.cc index 8f7986e64e..031ec33241 100644 --- a/test/cpp/util/string_ref_test.cc +++ b/test/cpp/util/string_ref_test.cc @@ -60,7 +60,7 @@ TEST_F(StringRefTest, FromString) { TEST_F(StringRefTest, CopyConstructor) { string_ref s1(kTestString); ; - string_ref s2(s1); + const string_ref& s2(s1); EXPECT_EQ(s1.length(), s2.length()); EXPECT_EQ(s1.data(), s2.data()); } |