From ba46afe3e2b4e8dc00fec8347979f2ad9c1427dd Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Thu, 15 Mar 2018 11:16:58 -0700 Subject: [PATCH] Delete status_test that has no coverage; replace with static_asserts --- CMakeLists.txt | 38 ------- Makefile | 48 -------- build.yaml | 12 -- include/grpcpp/impl/codegen/status.h | 56 +++++++++- test/cpp/util/BUILD | 104 ++++++++---------- test/cpp/util/status_test.cc | 62 ----------- .../generated/sources_and_headers.json | 18 --- tools/run_tests/generated/tests.json | 24 ---- 8 files changed, 102 insertions(+), 260 deletions(-) delete mode 100644 test/cpp/util/status_test.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index 3e58593fb19..77e84a0df80 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -620,7 +620,6 @@ add_dependencies(buildtests_cxx slice_hash_table_test) add_dependencies(buildtests_cxx slice_weak_hash_table_test) add_dependencies(buildtests_cxx stats_test) add_dependencies(buildtests_cxx status_metadata_test) -add_dependencies(buildtests_cxx status_test) add_dependencies(buildtests_cxx status_util_test) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) add_dependencies(buildtests_cxx streaming_throughput_test) @@ -13073,43 +13072,6 @@ target_link_libraries(status_metadata_test endif (gRPC_BUILD_TESTS) if (gRPC_BUILD_TESTS) -add_executable(status_test - test/cpp/util/status_test.cc - third_party/googletest/googletest/src/gtest-all.cc - third_party/googletest/googlemock/src/gmock-all.cc -) - - -target_include_directories(status_test - PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} - PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include - PRIVATE ${_gRPC_SSL_INCLUDE_DIR} - PRIVATE ${_gRPC_PROTOBUF_INCLUDE_DIR} - PRIVATE ${_gRPC_ZLIB_INCLUDE_DIR} - PRIVATE ${_gRPC_BENCHMARK_INCLUDE_DIR} - PRIVATE ${_gRPC_CARES_INCLUDE_DIR} - PRIVATE ${_gRPC_GFLAGS_INCLUDE_DIR} - PRIVATE third_party/googletest/googletest/include - PRIVATE third_party/googletest/googletest - PRIVATE third_party/googletest/googlemock/include - PRIVATE third_party/googletest/googlemock - PRIVATE ${_gRPC_PROTO_GENS_DIR} -) - -target_link_libraries(status_test - ${_gRPC_PROTOBUF_LIBRARIES} - ${_gRPC_ALLTARGETS_LIBRARIES} - grpc_test_util - grpc++ - grpc - gpr_test_util - gpr - ${_gRPC_GFLAGS_LIBRARIES} -) - -endif (gRPC_BUILD_TESTS) -if (gRPC_BUILD_TESTS) - add_executable(status_util_test test/core/client_channel/status_util_test.cc third_party/googletest/googletest/src/gtest-all.cc diff --git a/Makefile b/Makefile index 625248ea301..3fa0f6617b3 100644 --- a/Makefile +++ b/Makefile @@ -1202,7 +1202,6 @@ slice_hash_table_test: $(BINDIR)/$(CONFIG)/slice_hash_table_test slice_weak_hash_table_test: $(BINDIR)/$(CONFIG)/slice_weak_hash_table_test stats_test: $(BINDIR)/$(CONFIG)/stats_test status_metadata_test: $(BINDIR)/$(CONFIG)/status_metadata_test -status_test: $(BINDIR)/$(CONFIG)/status_test status_util_test: $(BINDIR)/$(CONFIG)/status_util_test streaming_throughput_test: $(BINDIR)/$(CONFIG)/streaming_throughput_test stress_test: $(BINDIR)/$(CONFIG)/stress_test @@ -1681,7 +1680,6 @@ buildtests_cxx: privatelibs_cxx \ $(BINDIR)/$(CONFIG)/slice_weak_hash_table_test \ $(BINDIR)/$(CONFIG)/stats_test \ $(BINDIR)/$(CONFIG)/status_metadata_test \ - $(BINDIR)/$(CONFIG)/status_test \ $(BINDIR)/$(CONFIG)/status_util_test \ $(BINDIR)/$(CONFIG)/streaming_throughput_test \ $(BINDIR)/$(CONFIG)/stress_test \ @@ -1848,7 +1846,6 @@ buildtests_cxx: privatelibs_cxx \ $(BINDIR)/$(CONFIG)/slice_weak_hash_table_test \ $(BINDIR)/$(CONFIG)/stats_test \ $(BINDIR)/$(CONFIG)/status_metadata_test \ - $(BINDIR)/$(CONFIG)/status_test \ $(BINDIR)/$(CONFIG)/status_util_test \ $(BINDIR)/$(CONFIG)/streaming_throughput_test \ $(BINDIR)/$(CONFIG)/stress_test \ @@ -2312,8 +2309,6 @@ test_cxx: buildtests_cxx $(Q) $(BINDIR)/$(CONFIG)/stats_test || ( echo test stats_test failed ; exit 1 ) $(E) "[RUN] Testing status_metadata_test" $(Q) $(BINDIR)/$(CONFIG)/status_metadata_test || ( echo test status_metadata_test failed ; exit 1 ) - $(E) "[RUN] Testing status_test" - $(Q) $(BINDIR)/$(CONFIG)/status_test || ( echo test status_test failed ; exit 1 ) $(E) "[RUN] Testing status_util_test" $(Q) $(BINDIR)/$(CONFIG)/status_util_test || ( echo test status_util_test failed ; exit 1 ) $(E) "[RUN] Testing streaming_throughput_test" @@ -19115,49 +19110,6 @@ endif endif -STATUS_TEST_SRC = \ - test/cpp/util/status_test.cc \ - -STATUS_TEST_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(STATUS_TEST_SRC)))) -ifeq ($(NO_SECURE),true) - -# You can't build secure targets if you don't have OpenSSL. - -$(BINDIR)/$(CONFIG)/status_test: openssl_dep_error - -else - - - - -ifeq ($(NO_PROTOBUF),true) - -# You can't build the protoc plugins or protobuf-enabled targets if you don't have protobuf 3.0.0+. - -$(BINDIR)/$(CONFIG)/status_test: protobuf_dep_error - -else - -$(BINDIR)/$(CONFIG)/status_test: $(PROTOBUF_DEP) $(STATUS_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a - $(E) "[LD] Linking $@" - $(Q) mkdir -p `dirname $@` - $(Q) $(LDXX) $(LDFLAGS) $(STATUS_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBSXX) $(LDLIBS_PROTOBUF) $(LDLIBS) $(LDLIBS_SECURE) $(GTEST_LIB) -o $(BINDIR)/$(CONFIG)/status_test - -endif - -endif - -$(OBJDIR)/$(CONFIG)/test/cpp/util/status_test.o: $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a - -deps_status_test: $(STATUS_TEST_OBJS:.o=.dep) - -ifneq ($(NO_SECURE),true) -ifneq ($(NO_DEPS),true) --include $(STATUS_TEST_OBJS:.o=.dep) -endif -endif - - STATUS_UTIL_TEST_SRC = \ test/core/client_channel/status_util_test.cc \ diff --git a/build.yaml b/build.yaml index 02a1e6186e8..a4875df9e53 100644 --- a/build.yaml +++ b/build.yaml @@ -5122,18 +5122,6 @@ targets: deps: - grpc uses_polling: false -- name: status_test - build: test - language: c++ - src: - - test/cpp/util/status_test.cc - deps: - - grpc_test_util - - grpc++ - - grpc - - gpr_test_util - - gpr - uses_polling: false - name: status_util_test gtest: true cpu_cost: 0.1 diff --git a/include/grpcpp/impl/codegen/status.h b/include/grpcpp/impl/codegen/status.h index 9f409eb9bd8..e625a76b15d 100644 --- a/include/grpcpp/impl/codegen/status.h +++ b/include/grpcpp/impl/codegen/status.h @@ -19,6 +19,7 @@ #ifndef GRPCPP_IMPL_CODEGEN_STATUS_H #define GRPCPP_IMPL_CODEGEN_STATUS_H +#include #include #include @@ -30,7 +31,60 @@ namespace grpc { class Status { public: /// Construct an OK instance. - Status() : code_(StatusCode::OK) {} + Status() : code_(StatusCode::OK) { + // Static assertions to make sure that the C++ API value correctly + // maps to the core surface API value + static_assert(StatusCode::OK == static_cast(GRPC_STATUS_OK), + "Mismatched status code"); + static_assert( + StatusCode::CANCELLED == static_cast(GRPC_STATUS_CANCELLED), + "Mismatched status code"); + static_assert( + StatusCode::UNKNOWN == static_cast(GRPC_STATUS_UNKNOWN), + "Mismatched status code"); + static_assert(StatusCode::INVALID_ARGUMENT == + static_cast(GRPC_STATUS_INVALID_ARGUMENT), + "Mismatched status code"); + static_assert(StatusCode::DEADLINE_EXCEEDED == + static_cast(GRPC_STATUS_DEADLINE_EXCEEDED), + "Mismatched status code"); + static_assert( + StatusCode::NOT_FOUND == static_cast(GRPC_STATUS_NOT_FOUND), + "Mismatched status code"); + static_assert(StatusCode::ALREADY_EXISTS == + static_cast(GRPC_STATUS_ALREADY_EXISTS), + "Mismatched status code"); + static_assert(StatusCode::PERMISSION_DENIED == + static_cast(GRPC_STATUS_PERMISSION_DENIED), + "Mismatched status code"); + static_assert(StatusCode::UNAUTHENTICATED == + static_cast(GRPC_STATUS_UNAUTHENTICATED), + "Mismatched status code"); + static_assert(StatusCode::RESOURCE_EXHAUSTED == + static_cast(GRPC_STATUS_RESOURCE_EXHAUSTED), + "Mismatched status code"); + static_assert(StatusCode::FAILED_PRECONDITION == + static_cast(GRPC_STATUS_FAILED_PRECONDITION), + "Mismatched status code"); + static_assert( + StatusCode::ABORTED == static_cast(GRPC_STATUS_ABORTED), + "Mismatched status code"); + static_assert(StatusCode::OUT_OF_RANGE == + static_cast(GRPC_STATUS_OUT_OF_RANGE), + "Mismatched status code"); + static_assert(StatusCode::UNIMPLEMENTED == + static_cast(GRPC_STATUS_UNIMPLEMENTED), + "Mismatched status code"); + static_assert( + StatusCode::INTERNAL == static_cast(GRPC_STATUS_INTERNAL), + "Mismatched status code"); + static_assert(StatusCode::UNAVAILABLE == + static_cast(GRPC_STATUS_UNAVAILABLE), + "Mismatched status code"); + static_assert( + StatusCode::DATA_LOSS == static_cast(GRPC_STATUS_DATA_LOSS), + "Mismatched status code"); + } /// Construct an instance with associated \a code and \a error_message. /// It is an error to construct an OK status with non-empty \a error_message. diff --git a/test/cpp/util/BUILD b/test/cpp/util/BUILD index d092ba348ef..4f84c738203 100644 --- a/test/cpp/util/BUILD +++ b/test/cpp/util/BUILD @@ -16,7 +16,10 @@ licenses(["notice"]) # Apache v2 load("//bazel:grpc_build_system.bzl", "grpc_cc_library", "grpc_cc_binary", "grpc_cc_test", "grpc_package") -grpc_package(name = "test/cpp/util", visibility = "public") +grpc_package( + name = "test/cpp/util", + visibility = "public", +) grpc_cc_library( name = "test_config", @@ -49,7 +52,7 @@ grpc_cc_library( ], ) -GRPCXX_TESTUTIL_SRCS = [ +GRPCXX_TESTUTIL_SRCS = [ "byte_buffer_proto_helper.cc", "string_ref_helper.cc", "subprocess.cc", @@ -71,27 +74,27 @@ grpc_cc_library( "create_test_channel.h", "test_credentials_provider.h", ], + external_deps = [ + "protobuf", + ], deps = [ "//:grpc++", "//test/core/end2end:ssl_test_data", "//test/core/util:grpc_test_util", ], - external_deps = [ - "protobuf", - ], ) grpc_cc_library( name = "test_util_unsecure", srcs = GRPCXX_TESTUTIL_SRCS, hdrs = GRPCXX_TESTUTIL_HDRS, + external_deps = [ + "protobuf", + ], deps = [ "//:grpc++_unsecure", "//test/core/util:grpc_test_util", ], - external_deps = [ - "protobuf", - ], ) grpc_cc_library( @@ -111,16 +114,16 @@ grpc_cc_library( "proto_file_parser.h", "service_describer.h", ], - deps = [ - "//:grpc++", - "//src/proto/grpc/reflection/v1alpha:reflection_proto", - ":grpc++_proto_reflection_desc_db", - ], external_deps = [ "gflags", "protobuf", "protobuf_clib", ], + deps = [ + ":grpc++_proto_reflection_desc_db", + "//:grpc++", + "//src/proto/grpc/reflection/v1alpha:reflection_proto", + ], ) grpc_cc_library( @@ -132,8 +135,8 @@ grpc_cc_library( "metrics_server.h", ], deps = [ - "//src/proto/grpc/testing:metrics_proto", "//:grpc++", + "//src/proto/grpc/testing:metrics_proto", ], ) @@ -144,19 +147,19 @@ grpc_cc_test( ], data = [ "//src/proto/grpc/testing:echo.proto", - "//src/proto/grpc/testing:echo_messages.proto" - ], - deps = [ - ":grpc_cli_libs", - ":test_util", - "//:grpc++_reflection", - "//src/proto/grpc/testing:echo_proto", - "//src/proto/grpc/testing:echo_messages_proto", - "//test/core/util:grpc_test_util", + "//src/proto/grpc/testing:echo_messages.proto", ], external_deps = [ "gtest", ], + deps = [ + ":grpc_cli_libs", + ":test_util", + "//:grpc++_reflection", + "//src/proto/grpc/testing:echo_messages_proto", + "//src/proto/grpc/testing:echo_proto", + "//test/core/util:grpc_test_util", + ], ) grpc_cc_test( @@ -164,12 +167,12 @@ grpc_cc_test( srcs = [ "byte_buffer_test.cc", ], - deps = [ - ":test_util", - ], external_deps = [ "gtest", ], + deps = [ + ":test_util", + ], ) grpc_cc_test( @@ -177,12 +180,12 @@ grpc_cc_test( srcs = [ "slice_test.cc", ], - deps = [ - ":test_util", - ], external_deps = [ "gtest", ], + deps = [ + ":test_util", + ], ) grpc_cc_test( @@ -190,12 +193,12 @@ grpc_cc_test( srcs = [ "string_ref_test.cc", ], - deps = [ - "//:grpc++", - ], external_deps = [ "gtest", ], + deps = [ + "//:grpc++", + ], ) grpc_cc_test( @@ -203,24 +206,11 @@ grpc_cc_test( srcs = [ "time_test.cc", ], - deps = [ - ":test_util", - ], external_deps = [ "gtest", ], -) - -grpc_cc_test( - name = "status_test", - srcs = [ - "status_test.cc", - ], deps = [ - ":test_util", - ], - external_deps = [ - "gtest", + ":test_util", ], ) @@ -229,14 +219,14 @@ grpc_cc_test( srcs = [ "cli_call_test.cc", ], + external_deps = [ + "gtest", + ], deps = [ ":grpc_cli_libs", - ":test_util", + ":test_util", "//src/proto/grpc/testing:echo_proto", - "//test/core/util:grpc_test_util", - ], - external_deps = [ - "gtest", + "//test/core/util:grpc_test_util", ], ) @@ -245,13 +235,13 @@ grpc_cc_test( srcs = [ "error_details_test.cc", ], + external_deps = [ + "gtest", + ], deps = [ "//:grpc++_error_details", "//src/proto/grpc/testing:echo_messages_proto", ], - external_deps = [ - "gtest", - ], ) grpc_cc_binary( @@ -274,11 +264,11 @@ grpc_cc_binary( "test_config.h", "test_config_cc.cc", ], + external_deps = [ + "gflags", + ], deps = [ "//:grpc++", "//src/proto/grpc/reflection/v1alpha:reflection_proto", ], - external_deps = [ - "gflags", - ], ) diff --git a/test/cpp/util/status_test.cc b/test/cpp/util/status_test.cc deleted file mode 100644 index 2188723db80..00000000000 --- a/test/cpp/util/status_test.cc +++ /dev/null @@ -1,62 +0,0 @@ -/* - * - * Copyright 2015 gRPC authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ - -#include - -#include -#include - -// Make sure the existing grpc_status_code match with grpc::Code. -int main(int argc, char** argv) { - GPR_ASSERT(grpc::StatusCode::OK == - static_cast(GRPC_STATUS_OK)); - GPR_ASSERT(grpc::StatusCode::CANCELLED == - static_cast(GRPC_STATUS_CANCELLED)); - GPR_ASSERT(grpc::StatusCode::UNKNOWN == - static_cast(GRPC_STATUS_UNKNOWN)); - GPR_ASSERT(grpc::StatusCode::INVALID_ARGUMENT == - static_cast(GRPC_STATUS_INVALID_ARGUMENT)); - GPR_ASSERT(grpc::StatusCode::DEADLINE_EXCEEDED == - static_cast(GRPC_STATUS_DEADLINE_EXCEEDED)); - GPR_ASSERT(grpc::StatusCode::NOT_FOUND == - static_cast(GRPC_STATUS_NOT_FOUND)); - GPR_ASSERT(grpc::StatusCode::ALREADY_EXISTS == - static_cast(GRPC_STATUS_ALREADY_EXISTS)); - GPR_ASSERT(grpc::StatusCode::PERMISSION_DENIED == - static_cast(GRPC_STATUS_PERMISSION_DENIED)); - GPR_ASSERT(grpc::StatusCode::UNAUTHENTICATED == - static_cast(GRPC_STATUS_UNAUTHENTICATED)); - GPR_ASSERT(grpc::StatusCode::RESOURCE_EXHAUSTED == - static_cast(GRPC_STATUS_RESOURCE_EXHAUSTED)); - GPR_ASSERT(grpc::StatusCode::FAILED_PRECONDITION == - static_cast(GRPC_STATUS_FAILED_PRECONDITION)); - GPR_ASSERT(grpc::StatusCode::ABORTED == - static_cast(GRPC_STATUS_ABORTED)); - GPR_ASSERT(grpc::StatusCode::OUT_OF_RANGE == - static_cast(GRPC_STATUS_OUT_OF_RANGE)); - GPR_ASSERT(grpc::StatusCode::UNIMPLEMENTED == - static_cast(GRPC_STATUS_UNIMPLEMENTED)); - GPR_ASSERT(grpc::StatusCode::INTERNAL == - static_cast(GRPC_STATUS_INTERNAL)); - GPR_ASSERT(grpc::StatusCode::UNAVAILABLE == - static_cast(GRPC_STATUS_UNAVAILABLE)); - GPR_ASSERT(grpc::StatusCode::DATA_LOSS == - static_cast(GRPC_STATUS_DATA_LOSS)); - - return 0; -} diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index d754c5d6fc1..f80cd9f2b5c 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -4473,24 +4473,6 @@ "third_party": false, "type": "target" }, - { - "deps": [ - "gpr", - "gpr_test_util", - "grpc", - "grpc++", - "grpc_test_util" - ], - "headers": [], - "is_filegroup": false, - "language": "c++", - "name": "status_test", - "src": [ - "test/cpp/util/status_test.cc" - ], - "third_party": false, - "type": "target" - }, { "deps": [ "grpc" diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 1e320fc15dc..3d6b6caa2ad 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -4914,30 +4914,6 @@ ], "uses_polling": false }, - { - "args": [], - "benchmark": false, - "ci_platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "gtest": false, - "language": "c++", - "name": "status_test", - "platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "uses_polling": false - }, { "args": [], "benchmark": false,